mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net v3 0/2] net: psp: preserve SKB_EXT_PSP across packet scrubbing
@ 2026-11-06 18:20 Wei Wang
  2026-11-06 18:20 ` [PATCH net v3 1/2] net: Make PSP skb extension survive " Wei Wang
  2026-11-06 18:20 ` [PATCH net v3 2/2] selftests/net: psp: test bidirectional data transfer Wei Wang
  0 siblings, 2 replies; 7+ messages in thread
From: Wei Wang @ 2026-11-06 18:20 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni, Andrew Lunn, Shuah Khan
  Cc: Simon Horman, Jakub Sitnicki, Daniel Zahka, Willem de Bruijn,
	netdev, linux-kselftest, linux-kernel, bpf, Wei Wang

When a virtual interface such as netkit is associated with a PSP device
in another network namespace via PSP_CMD_DEV_ASSOC, incoming PSP packets
decapsulated on the physical device lose their SKB_EXT_PSP extension
when scrubbed during redirection (e.g. skb_do_redirect() or
netkit_xmit() calling skb_scrub_packet()). As a result,
__psp_sk_rx_policy_check() rejects incoming PSP-encrypted TCP data
segments at the socket layer.

This gap slipped through the existing psp.py selftests because both
_data_basic_send() and _data_basic_send_netkit_psp_assoc() only sent a
unidirectional data stream from the test socket to psp_responder; the
returning pure TCP ACKs had SKB_EXT_PSP stripped by skb_scrub_packet(),
but were still accepted by __psp_sk_rx_policy_check() while peer_tx was
unset.

Patch 1 (originally from Jakub Sitnicki's BPF skb extension series [1])
introduces skb_ext_scrub() to selectively preserve SKB_EXT_PSP during
skb_scrub_packet().

Patch 2 updates the psp.py selftests to exercise bidirectional PSP data
transfer using psp_responder's "data echo" command in both direct and
netkit dev-assoc test cases.

Changes in v3:
- Patch 1: Move tmp = *pse; after the fast-path early return, just
  before skb_ext_reset(skb).
- Patch 1: Do not clear skb->decrypted if skb_ext_add() fails so
  sk_validate_xmit_skb() still drops socketless decrypted packets on
  egress.
- Patch 2: Replace s.settimeout() in _check_data_echo() with
  non-blocking _recv_careful() and _req_echo() helpers.

Changes in v2:
- Patch 1: Replace the skb_ext_del() loop in skb_ext_scrub() with
  skb_ext_reset() + skb_ext_add() (and a fast path when SKB_EXT_PSP is
  the only extension in the skb_ext buffer) so shared skb_ext buffers
  with refcounted extensions (SKB_EXT_SEC_PATH / SKB_EXT_MCTP) are not
  left half-deleted.
- Patch 1: Make skb_ext_scrub() static in net/core/skbuff.c and drop
  EXPORT_SYMBOL() / public header prototype.

Link: https://lore.kernel.org/netdev/20260910-bpf-meta-inside-skb-ext-v2-3-0b21e42180b0@cloudflare.com/ [1]

Wei Wang (2):
  net: Make PSP skb extension survive packet scrubbing
  selftests/net: psp: test bidirectional data transfer

 net/core/skbuff.c                          | 30 ++++++++++++++++++++--
 tools/testing/selftests/drivers/net/psp.py | 22 ++++++++++++++++
 2 files changed, 50 insertions(+), 2 deletions(-)

-- 
2.56.0.455.g6c5a503b38-goog


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH net v3 1/2] net: Make PSP skb extension survive packet scrubbing
  2026-11-06 18:20 [PATCH net v3 0/2] net: psp: preserve SKB_EXT_PSP across packet scrubbing Wei Wang
@ 2026-11-06 18:20 ` Wei Wang
  2026-11-07 18:23   ` netdev-bot+sashiko
  2026-11-06 18:20 ` [PATCH net v3 2/2] selftests/net: psp: test bidirectional data transfer Wei Wang
  1 sibling, 1 reply; 7+ messages in thread
From: Wei Wang @ 2026-11-06 18:20 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni, Andrew Lunn, Shuah Khan
  Cc: Simon Horman, Jakub Sitnicki, Daniel Zahka, Willem de Bruijn,
	netdev, linux-kselftest, linux-kernel, bpf, Wei Wang

skb_scrub_packet() drops all skb extensions unconditionally via
skb_ext_reset(). It runs on tunnel encap/decap (ip_tunnel_rcv, vxlan_rcv,
etc.) and virtual device forwarding/redirection (dev_forward_skb,
netkit_xmit, skb_do_redirect).

When a virtual device such as netkit is associated with a PSP device in
another network namespace via PSP_CMD_DEV_ASSOC, incoming PSP packets
decapsulated by psp_dev_rcv() lose their SKB_EXT_PSP extension when
scrubbed during redirection into the peer netns. Without SKB_EXT_PSP,
__psp_sk_rx_policy_check() rejects incoming TCP data segments at the
socket layer with SKB_DROP_REASON_PSP_INPUT.

Introduce a static skb_ext_scrub() helper in net/core/skbuff.c to
preserve SKB_EXT_PSP across skb_scrub_packet() while dropping all other
extensions. If SKB_EXT_PSP is not active, skb_ext_scrub() simply calls
skb_ext_reset(). If SKB_EXT_PSP is the only extension in the skb_ext
buffer, nothing needs to be scrubbed. Otherwise, save the PSP metadata,
reset the extension block, and re-add a fresh SKB_EXT_PSP extension.

Fixes: 06c2dce2d0f6 ("psp: add new netlink cmd for dev-assoc and dev-disassoc")
Link: https://lore.kernel.org/r/20260910-bpf-meta-inside-skb-ext-v2-3-0b21e42180b0@cloudflare.com
Reported-by: Daniel Zahka <daniel.zahka@gmail.com>
Closes: https://lore.kernel.org/netdev/DLODSKPY6ON6.1G6SP3CFFLCPJ@gmail.com/
Co-developed-by: Jakub Sitnicki <jakub@cloudflare.com>
Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com>
Signed-off-by: Wei Wang <weiwan@google.com>
Reviewed-by: Daniel Zahka <daniel.zahka@gmail.com>
---
 net/core/skbuff.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 93c2f440b9ec..ddbf0f3342ca 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -6369,6 +6369,31 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff *from,
 }
 EXPORT_SYMBOL(skb_try_coalesce);
 
+static void skb_ext_scrub(struct sk_buff *skb)
+{
+#if IS_ENABLED(CONFIG_INET_PSP)
+	struct psp_skb_ext *pse = skb_ext_find(skb, SKB_EXT_PSP);
+
+	if (pse) {
+		struct skb_ext *ext = skb->extensions;
+		struct psp_skb_ext tmp;
+
+		/* Nothing else to scrub if PSP is the only extension in ext. */
+		if (ext->chunks == SKB_EXT_CHUNKSIZEOF(*ext) +
+				   skb_ext_type_len[SKB_EXT_PSP])
+			return;
+
+		tmp = *pse;
+		skb_ext_reset(skb);
+		pse = skb_ext_add(skb, SKB_EXT_PSP);
+		if (pse)
+			memcpy(pse, &tmp, sizeof(*pse));
+		return;
+	}
+#endif
+	skb_ext_reset(skb);
+}
+
 /**
  * skb_scrub_packet - scrub an skb
  *
@@ -6380,7 +6405,8 @@ EXPORT_SYMBOL(skb_try_coalesce);
  * operations.
  * skb_scrub_packet can also be used to clean a skb before injecting it in
  * another namespace (@xnet == true). We have to clear all information in the
- * skb that could impact namespace isolation.
+ * skb that could impact namespace isolation. Note that PSP skb extension is
+ * preserved across packet scrubbing by design.
  */
 void skb_scrub_packet(struct sk_buff *skb, bool xnet)
 {
@@ -6388,7 +6414,7 @@ void skb_scrub_packet(struct sk_buff *skb, bool xnet)
 	skb->skb_iif = 0;
 	skb->ignore_df = 0;
 	skb_dst_drop(skb);
-	skb_ext_reset(skb);
+	skb_ext_scrub(skb);
 	nf_reset_ct(skb);
 	nf_reset_trace(skb);
 
-- 
2.56.0.455.g6c5a503b38-goog


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH net v3 2/2] selftests/net: psp: test bidirectional data transfer
  2026-11-06 18:20 [PATCH net v3 0/2] net: psp: preserve SKB_EXT_PSP across packet scrubbing Wei Wang
  2026-11-06 18:20 ` [PATCH net v3 1/2] net: Make PSP skb extension survive " Wei Wang
@ 2026-11-06 18:20 ` Wei Wang
  2026-11-06 18:34   ` Daniel Zahka
  2026-11-07 18:23   ` netdev-bot+sashiko
  1 sibling, 2 replies; 7+ messages in thread
From: Wei Wang @ 2026-11-06 18:20 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni, Andrew Lunn, Shuah Khan
  Cc: Simon Horman, Jakub Sitnicki, Daniel Zahka, Willem de Bruijn,
	netdev, linux-kselftest, linux-kernel, bpf, Wei Wang

_data_basic_send() and _data_basic_send_netkit_psp_assoc() currently
only verify unidirectional data transmission from the test socket to
psp_responder. As a result, they do not exercise the receive path for
PSP-encrypted TCP data segments arriving at the test socket (such as
packets redirected across a netkit interface associated via dev-assoc,
which relies on SKB_EXT_PSP surviving packet scrubbing).

Use the existing "data echo" command in psp_responder to verify that the
test socket can also receive PSP-encrypted data from the peer in both
_data_basic_send() and _data_basic_send_netkit_psp_assoc().

Signed-off-by: Wei Wang <weiwan@google.com>
---
 tools/testing/selftests/drivers/net/psp.py | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/tools/testing/selftests/drivers/net/psp.py b/tools/testing/selftests/drivers/net/psp.py
index 473500901879..c114d2245135 100755
--- a/tools/testing/selftests/drivers/net/psp.py
+++ b/tools/testing/selftests/drivers/net/psp.py
@@ -120,6 +120,26 @@ def _check_data_rx(cfg, exp_len):
     ksft_eq(read_len, exp_len)
 
 
+def _recv_careful(s, target, timeout=2):
+    """Read exactly target bytes, tolerating short reads"""
+    data = b''
+    end = time.monotonic() + timeout
+    while time.monotonic() < end:
+        try:
+            data += s.recv(target - len(data), socket.MSG_DONTWAIT)
+            if len(data) == target:
+                return data
+        except BlockingIOError:
+            time.sleep(0.001)
+    raise KsftFailEx(f"short read, got {len(data)} of {target} bytes")
+
+
+def _req_echo(cfg, s):
+    """Ask the peer to echo, and check the reply arrives intact"""
+    _send_with_ack(cfg, b'data echo\0')
+    ksft_eq(_recv_careful(s, 5), b'echo\0')
+
+
 def _check_data_outq(s, exp_len, force_wait=False):
     outq = 0
     for _ in range(10):
@@ -478,6 +498,7 @@ def _data_basic_send(cfg, version, ipver):
 
     data_len = _send_careful(cfg, s, 100)
     _check_data_rx(cfg, data_len)
+    _req_echo(cfg, s)
     _close_psp_conn(cfg, s)
 
 
@@ -726,6 +747,7 @@ def _data_basic_send_netkit_psp_assoc(cfg, version, ipver):
 
         data_len = _send_careful(cfg, sock, 100)
         _check_data_rx(cfg, data_len)
+        _req_echo(cfg, sock)
         _close_psp_conn(cfg, sock)
 
 
-- 
2.56.0.455.g6c5a503b38-goog


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net v3 2/2] selftests/net: psp: test bidirectional data transfer
  2026-11-06 18:20 ` [PATCH net v3 2/2] selftests/net: psp: test bidirectional data transfer Wei Wang
@ 2026-11-06 18:34   ` Daniel Zahka
  2026-11-06 18:57     ` Daniel Zahka
  2026-11-07 18:23   ` netdev-bot+sashiko
  1 sibling, 1 reply; 7+ messages in thread
From: Daniel Zahka @ 2026-11-06 18:34 UTC (permalink / raw)
  To: Wei Wang, David S . Miller, Jakub Kicinski, Paolo Abeni,
	Andrew Lunn, Shuah Khan
  Cc: Simon Horman, Jakub Sitnicki, Daniel Zahka, Willem de Bruijn,
	netdev, linux-kselftest, linux-kernel, bpf

On Fri Nov 6, 2026 at 1:20 PM EST, Wei Wang wrote:
> _data_basic_send() and _data_basic_send_netkit_psp_assoc() currently
> only verify unidirectional data transmission from the test socket to
> psp_responder. As a result, they do not exercise the receive path for
> PSP-encrypted TCP data segments arriving at the test socket (such as
> packets redirected across a netkit interface associated via dev-assoc,
> which relies on SKB_EXT_PSP surviving packet scrubbing).
>
> Use the existing "data echo" command in psp_responder to verify that the
> test socket can also receive PSP-encrypted data from the peer in both
> _data_basic_send() and _data_basic_send_netkit_psp_assoc().
>
> Signed-off-by: Wei Wang <weiwan@google.com>

Reviewed-by: Daniel Zahka <daniel.zahka@gmail.com>

> ---
>  tools/testing/selftests/drivers/net/psp.py | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/tools/testing/selftests/drivers/net/psp.py b/tools/testing/selftests/drivers/net/psp.py
> index 473500901879..c114d2245135 100755
> --- a/tools/testing/selftests/drivers/net/psp.py
> +++ b/tools/testing/selftests/drivers/net/psp.py
> @@ -120,6 +120,26 @@ def _check_data_rx(cfg, exp_len):
>      ksft_eq(read_len, exp_len)
>  
>  
> +def _recv_careful(s, target, timeout=2):
> +    """Read exactly target bytes, tolerating short reads"""
> +    data = b''
> +    end = time.monotonic() + timeout
> +    while time.monotonic() < end:
> +        try:
> +            data += s.recv(target - len(data), socket.MSG_DONTWAIT)
> +            if len(data) == target:
> +                return data
> +        except BlockingIOError:
> +            time.sleep(0.001)
> +    raise KsftFailEx(f"short read, got {len(data)} of {target} bytes")
> +
> +
> +def _req_echo(cfg, s):
> +    """Ask the peer to echo, and check the reply arrives intact"""
> +    _send_with_ack(cfg, b'data echo\0')
> +    ksft_eq(_recv_careful(s, 5), b'echo\0')
> +
> +
>  def _check_data_outq(s, exp_len, force_wait=False):
>      outq = 0
>      for _ in range(10):
> @@ -478,6 +498,7 @@ def _data_basic_send(cfg, version, ipver):
>  
>      data_len = _send_careful(cfg, s, 100)
>      _check_data_rx(cfg, data_len)
> +    _req_echo(cfg, s)
>      _close_psp_conn(cfg, s)
>  
>  
> @@ -726,6 +747,7 @@ def _data_basic_send_netkit_psp_assoc(cfg, version, ipver):
>  
>          data_len = _send_careful(cfg, sock, 100)
>          _check_data_rx(cfg, data_len)
> +        _req_echo(cfg, sock)
>          _close_psp_conn(cfg, sock)
>  
>  


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net v3 2/2] selftests/net: psp: test bidirectional data transfer
  2026-11-06 18:34   ` Daniel Zahka
@ 2026-11-06 18:57     ` Daniel Zahka
  0 siblings, 0 replies; 7+ messages in thread
From: Daniel Zahka @ 2026-11-06 18:57 UTC (permalink / raw)
  To: Daniel Zahka, Wei Wang, David S . Miller, Jakub Kicinski,
	Paolo Abeni, Andrew Lunn, Shuah Khan
  Cc: Simon Horman, Jakub Sitnicki, Willem de Bruijn, netdev,
	linux-kselftest, linux-kernel, bpf

On Fri Nov 6, 2026 at 1:34 PM EST, Daniel Zahka wrote:
> On Fri Nov 6, 2026 at 1:20 PM EST, Wei Wang wrote:
>> _data_basic_send() and _data_basic_send_netkit_psp_assoc() currently
>> only verify unidirectional data transmission from the test socket to
>> psp_responder. As a result, they do not exercise the receive path for
>> PSP-encrypted TCP data segments arriving at the test socket (such as
>> packets redirected across a netkit interface associated via dev-assoc,
>> which relies on SKB_EXT_PSP surviving packet scrubbing).
>>
>> Use the existing "data echo" command in psp_responder to verify that the
>> test socket can also receive PSP-encrypted data from the peer in both
>> _data_basic_send() and _data_basic_send_netkit_psp_assoc().
>>
>> Signed-off-by: Wei Wang <weiwan@google.com>
>
> Reviewed-by: Daniel Zahka <daniel.zahka@gmail.com>
>
>> ---
>>  tools/testing/selftests/drivers/net/psp.py | 22 ++++++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>>
>> diff --git a/tools/testing/selftests/drivers/net/psp.py b/tools/testing/selftests/drivers/net/psp.py
>> index 473500901879..c114d2245135 100755
>> --- a/tools/testing/selftests/drivers/net/psp.py
>> +++ b/tools/testing/selftests/drivers/net/psp.py
>> @@ -120,6 +120,26 @@ def _check_data_rx(cfg, exp_len):
>>      ksft_eq(read_len, exp_len)
>>  
>>  
>> +def _recv_careful(s, target, timeout=2):
>> +    """Read exactly target bytes, tolerating short reads"""
>> +    data = b''
>> +    end = time.monotonic() + timeout
>> +    while time.monotonic() < end:
>> +        try:
>> +            data += s.recv(target - len(data), socket.MSG_DONTWAIT)
>> +            if len(data) == target:
>> +                return data
>> +        except BlockingIOError:
>> +            time.sleep(0.001)
>> +    raise KsftFailEx(f"short read, got {len(data)} of {target} bytes")
>> +
>> +
>> +def _req_echo(cfg, s):
>> +    """Ask the peer to echo, and check the reply arrives intact"""
>> +    _send_with_ack(cfg, b'data echo\0')
>> +    ksft_eq(_recv_careful(s, 5), b'echo\0')
>> +
>> +
>>  def _check_data_outq(s, exp_len, force_wait=False):
>>      outq = 0
>>      for _ in range(10):
>> @@ -478,6 +498,7 @@ def _data_basic_send(cfg, version, ipver):
>>  
>>      data_len = _send_careful(cfg, s, 100)
>>      _check_data_rx(cfg, data_len)
>> +    _req_echo(cfg, s)
>>      _close_psp_conn(cfg, s)
>>  
>>  
>> @@ -726,6 +747,7 @@ def _data_basic_send_netkit_psp_assoc(cfg, version, ipver):
>>  
>>          data_len = _send_careful(cfg, sock, 100)
>>          _check_data_rx(cfg, data_len)
>> +        _req_echo(cfg, sock)
>>          _close_psp_conn(cfg, sock)
>>  
>>  

It occurs to me now that we could have just changed the key exchange
protocol used here to have a psp encrypted "start of psp" message that
each side sends and blocks waiting to receive like psp.rst suggests. Not
worth respinning, but maybe a future cleanup worth doing.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net v3 1/2] net: Make PSP skb extension survive packet scrubbing
  2026-11-06 18:20 ` [PATCH net v3 1/2] net: Make PSP skb extension survive " Wei Wang
@ 2026-11-07 18:23   ` netdev-bot+sashiko
  0 siblings, 0 replies; 7+ messages in thread
From: netdev-bot+sashiko @ 2026-11-07 18:23 UTC (permalink / raw)
  To: weiwan
  Cc: davem, kuba, pabeni, andrew+netdev, shuah, horms, jakub,
	daniel.zahka, willemb, netdev, linux-kselftest, linux-kernel,
	bpf

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] skb_ext_scrub() keeps SKB_EXT_PSP for every caller of
  skb_scrub_packet().
- [Low] On the slow path, skb_ext_scrub() frees the whole extension block
  with skb_ext_reset() before it re-creates SKB_EXT_PSP with…

--

net: Make PSP skb extension survive packet scrubbing

This patch adds a static skb_ext_scrub() helper, called from
skb_scrub_packet(). It keeps SKB_EXT_PSP and drops every other skb
extension. The goal is to keep the PSP metadata on packets redirected into
a netns associated with a PSP device via PSP_CMD_DEV_ASSOC, so that
__psp_sk_rx_policy_check() stops rejecting them.

Link: https://lore.kernel.org/r/20260910-bpf-meta-inside-skb-ext-v2-3-0b21e42180b0@cloudflare.com

> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 8da53d5b76fc2..3f8dc08b434fa 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -6379,6 +6379,31 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff *from,
>  }
>  EXPORT_SYMBOL(skb_try_coalesce);
>  
> +static void skb_ext_scrub(struct sk_buff *skb)
> +{
> +#if IS_ENABLED(CONFIG_INET_PSP)
> +	struct psp_skb_ext *pse = skb_ext_find(skb, SKB_EXT_PSP);
> +
> +	if (pse) {

[Severity: Medium]

Is SKB_EXT_PSP meant to be kept for every caller of skb_scrub_packet()?
The commit message only argues for one case: a netkit redirect into a
netns associated through PSP_CMD_DEV_ASSOC.

skb_scrub_packet() is also called from:

  - same-netns tunnel decap: ip_tunnel_rcv(), __iptunnel_pull_header()
    for vxlan/geneve, __ip6_tnl_rcv()
  - tunnel encap on TX: iptunnel_xmit(), vxlan_xmit_one(),
    geneve_build_skb()
  - dev_forward_skb(), veth and ipvlan, into any netns
  - skb_do_redirect()

Nothing here checks xnet, the direction, or whether the target netns or
device is associated with pse->dev_id.

psp_dev_rcv() copies the next header into the outer IP header without
checking it:

net/psp/psp_main.c:psp_dev_rcv() {
    ...
	iph->protocol = psph->nexthdr;
    ...
}

So the decrypted payload can be IPIP, GRE or UDP/VXLAN. Once a tunnel
driver decapsulates it, the inner packet now inherits the outer packet's
PSP metadata.

__psp_sk_rx_policy_check() decides based on whether the extension is
present:

include/net/psp/functions.h:__psp_sk_rx_policy_check() {
    ...
	if (!pas)
		return pse ? SKB_DROP_REASON_PSP_INPUT : 0;
    ...
}

Doesn't this change two outcomes?

First, inner or forwarded traffic to a non-PSP socket was accepted before.
That applies both to a non-associated netns and to traffic behind a
tunnel. It would now be dropped with SKB_DROP_REASON_PSP_INPUT.

Second, inner tunnelled TCP segments to a PSP socket were rejected before.
They would now be accepted when spi, dev_id, generation and version match.

The skb_scrub_packet() kernel-doc still says that for xnet we "have to
clear all information in the skb that could impact namespace isolation".
The new note only says the PSP skb extension is "preserved across packet
scrubbing by design".

Should preservation be limited to the associated-netns case? If not,
could the reasoning for the tunnel decap and non-associated netns cases
be documented?

> +		struct skb_ext *ext = skb->extensions;
> +		struct psp_skb_ext tmp;
> +
> +		/* Nothing else to scrub if PSP is the only extension in ext. */
> +		if (ext->chunks == SKB_EXT_CHUNKSIZEOF(*ext) +
> +				   skb_ext_type_len[SKB_EXT_PSP])
> +			return;
> +
> +		tmp = *pse;
> +		skb_ext_reset(skb);
> +		pse = skb_ext_add(skb, SKB_EXT_PSP);
> +		if (pse)
> +			memcpy(pse, &tmp, sizeof(*pse));
> +		return;

[Severity: Low]

What happens to the PSP metadata if skb_ext_add() fails here?

skb_ext_reset() frees the whole extension block first. With
active_extensions now 0, skb_ext_add() goes through
__skb_ext_alloc(GFP_ATOMIC), which can fail under memory pressure. If it
does, the PSP extension is lost without any signal and the packet keeps
going.

That seems to contradict the new kernel-doc line "PSP skb extension is
preserved across packet scrubbing by design".

It also differs from psp_dev_rcv(), which treats the same failure as
fatal:

net/psp/psp_main.c:psp_dev_rcv() {
    ...
	pse = skb_ext_add(skb, SKB_EXT_PSP);
	if (!pse)
		return -EINVAL;
    ...
}

Its callers, mlx5e_psp_offload_handle_rx_skb() and nsim_do_psp(), then
drop the packet.

If the allocation fails here, a data segment for a PSP socket would later
be dropped in __psp_sk_rx_policy_check() with SKB_DROP_REASON_PSP_INPUT.
That is the symptom this patch is meant to fix, and the drop reason would
be misleading.

Could the replacement be allocated before the old block is released? Or
should the new comment be reworded so it does not promise unconditional
preservation?

> +	}
> +#endif
> +	skb_ext_reset(skb);
> +}

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261106182007.374660-1-weiwan%40google.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net v3 2/2] selftests/net: psp: test bidirectional data transfer
  2026-11-06 18:20 ` [PATCH net v3 2/2] selftests/net: psp: test bidirectional data transfer Wei Wang
  2026-11-06 18:34   ` Daniel Zahka
@ 2026-11-07 18:23   ` netdev-bot+sashiko
  1 sibling, 0 replies; 7+ messages in thread
From: netdev-bot+sashiko @ 2026-11-07 18:23 UTC (permalink / raw)
  To: weiwan
  Cc: davem, kuba, pabeni, andrew+netdev, shuah, horms, jakub,
	daniel.zahka, willemb, netdev, linux-kselftest, linux-kernel,
	bpf

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] _recv_careful() in tools/testing/selftests/drivers/net/psp.py,
  added by this patch, does not check for end-of-stream.

--

selftests/net: psp: test bidirectional data transfer

This patch makes the PSP selftests check data in both directions. The
test socket now asks psp_responder for a "data echo" and checks that the
PSP-encrypted reply arrives. The check is added to _data_basic_send() and
_data_basic_send_netkit_psp_assoc().

> diff --git a/tools/testing/selftests/drivers/net/psp.py b/tools/testing/selftests/drivers/net/psp.py
> index 473500901879b..c114d2245135b 100755
> --- a/tools/testing/selftests/drivers/net/psp.py
> +++ b/tools/testing/selftests/drivers/net/psp.py
> @@ -120,6 +120,26 @@ def _check_data_rx(cfg, exp_len):
>      ksft_eq(read_len, exp_len)
>  
>  
> +def _recv_careful(s, target, timeout=2):
> +    """Read exactly target bytes, tolerating short reads"""
> +    data = b''
> +    end = time.monotonic() + timeout
> +    while time.monotonic() < end:
> +        try:
> +            data += s.recv(target - len(data), socket.MSG_DONTWAIT)
> +            if len(data) == target:
> +                return data
> +        except BlockingIOError:
> +            time.sleep(0.001)
> +    raise KsftFailEx(f"short read, got {len(data)} of {target} bytes")

[Severity: Low]
What happens in _recv_careful() if the peer has already sent a FIN on the
data socket?

In that case s.recv(..., socket.MSG_DONTWAIT) returns b'' and does not
raise. data stays the same, the len(data) == target check stays false, and
the loop never reaches time.sleep(0.001), because the sleep is only in the
BlockingIOError branch.

So the loop calls recv() with no delay for the full 2 second timeout. It
then reports a generic "short read, got N of 5 bytes" instead of saying the
peer closed the connection.

This looks reachable. run_session() in psp_responder.c acks "data echo"
even when it has no data socket:

    if (cmd("data echo")) {
        if (data_sock >= 0)
            send(data_sock, "echo", 5, MSG_WAITALL);
        else
            fprintf(stderr, "WARN: echo but no data sock\n");
        send_ack(comm_sock);
    }

The responder can also close an established data socket on its "new data
sock but old one still here" accept path, which sends a FIN to the test
socket.

Would it make sense to check the chunk returned by recv() and raise
KsftFailEx right away, with a message like "peer closed data socket", when
it is empty?

> +
> +
> +def _req_echo(cfg, s):
> +    """Ask the peer to echo, and check the reply arrives intact"""
> +    _send_with_ack(cfg, b'data echo\0')
> +    ksft_eq(_recv_careful(s, 5), b'echo\0')

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261106182007.374660-1-weiwan%40google.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-11-07 18:23 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-06 18:20 [PATCH net v3 0/2] net: psp: preserve SKB_EXT_PSP across packet scrubbing Wei Wang
2026-11-06 18:20 ` [PATCH net v3 1/2] net: Make PSP skb extension survive " Wei Wang
2026-11-07 18:23   ` netdev-bot+sashiko
2026-11-06 18:20 ` [PATCH net v3 2/2] selftests/net: psp: test bidirectional data transfer Wei Wang
2026-11-06 18:34   ` Daniel Zahka
2026-11-06 18:57     ` Daniel Zahka
2026-11-07 18:23   ` netdev-bot+sashiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®