* [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
* 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
* [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 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®