* [PATCH net 0/2] psp: drop packets with extension headers in transport mode
@ 2026-10-21 21:01 Todd Freed
2026-10-21 21:01 ` [PATCH net 1/2] psp: drop transport mode packets with extension headers Todd Freed
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Todd Freed @ 2026-10-21 21:01 UTC (permalink / raw)
To: netdev, toddfreed
Cc: Daniel Zahka, Jakub Kicinski, Willem de Bruijn, David S. Miller,
Eric Dumazet, Paolo Abeni, Simon Horman, Andrew Lunn, Shuah Khan,
Rahul Rameshbabu, Raed Salem, Cosmin Ratiu, linux-kselftest
While working on adding support for socket-less psp associations, I
noticed that this case was not being handled correctly. It will be more
relevant with that series, because it adds udp support, for which the
psp nexthdr will be taken from the inner packet.
Since the tx association encodes intent to send encrypted packets, but
because of other configuration (socket options, routes) the packets are
sent unencrypted, sending this change as a fix.
The first patch drops the packets, the second patch adds selftests for
that case.
Todd Freed (2):
psp: drop transport mode packets with extension headers
selftests: drv-net: psp: test ipv6 tcp socket with extension headers
net/psp/psp_main.c | 7 +-
tools/testing/selftests/drivers/net/config | 1 +
tools/testing/selftests/drivers/net/psp.py | 120 ++++++++++++++++++++-
3 files changed, 125 insertions(+), 3 deletions(-)
--
2.53.0-Meta
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH net 1/2] psp: drop transport mode packets with extension headers 2026-10-21 21:01 [PATCH net 0/2] psp: drop packets with extension headers in transport mode Todd Freed @ 2026-10-21 21:01 ` Todd Freed 2026-10-25 21:30 ` netdev-bot+sashiko 2026-10-21 21:02 ` [PATCH net 2/2] selftests: drv-net: psp: test ipv6 tcp socket " Todd Freed ` (2 subsequent siblings) 3 siblings, 1 reply; 13+ messages in thread From: Todd Freed @ 2026-10-21 21:01 UTC (permalink / raw) To: netdev, toddfreed Cc: Daniel Zahka, Jakub Kicinski, Willem de Bruijn, David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman, Andrew Lunn, Shuah Khan, Rahul Rameshbabu, Raed Salem, Cosmin Ratiu, linux-kselftest psp_dev_encapsulate() inserts the udp and psp headers and rewrites the l3 next header to udp. if the packet is an ipv6 packet with extension headers, the transport header comes after them, so rewriting the next header of the ipv6 fixed header is wrong. a psp tcp socket emits such packets if certain socket options are set on it (e.g. IPV6_DSTOPTS). with mlx5, these packets also go out unencrypted. drop such packets on egress rather than them send malformed. note that a tcp socket which has such options set stops making progress. its packets are dropped (SKB_DROP_REASON_PSP_OUTPUT) until the connection times out. refusing the combination on the control path, when the options or the tx association are set, would be helpful but is not implemented here. extension headers can also come from a route, so the datapath check is needed either way. Fixes: fc724515741a ("psp: provide encapsulation helper for drivers") Signed-off-by: Todd Freed <toddfreed@meta.com> --- net/psp/psp_main.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/net/psp/psp_main.c b/net/psp/psp_main.c index c9c1a8826b7f..0d276e2aceff 100644 --- a/net/psp/psp_main.c +++ b/net/psp/psp_main.c @@ -243,7 +243,7 @@ static void psp_write_headers(struct net *net, struct sk_buff *skb, __be32 spi, } /* Encapsulate a TCP packet with PSP by adding the UDP+PSP headers and filling - * them in. + * them in. IPv6 packets with extension headers are rejected. */ bool psp_dev_encapsulate(struct net *net, struct sk_buff *skb, __be32 spi, u8 ver, __be16 sport) @@ -256,6 +256,11 @@ bool psp_dev_encapsulate(struct net *net, struct sk_buff *skb, __be32 spi, skb->protocol != htons(ETH_P_IPV6)) return false; + /* extension headers not supported */ + if (skb->protocol == htons(ETH_P_IPV6) && + network_len != sizeof(struct ipv6hdr)) + return false; + if (skb_cow_head(skb, PSP_ENCAP_HLEN)) return false; -- 2.53.0-Meta ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH net 1/2] psp: drop transport mode packets with extension headers 2026-10-21 21:01 ` [PATCH net 1/2] psp: drop transport mode packets with extension headers Todd Freed @ 2026-10-25 21:30 ` netdev-bot+sashiko 0 siblings, 0 replies; 13+ messages in thread From: netdev-bot+sashiko @ 2026-10-25 21:30 UTC (permalink / raw) To: toddfreed Cc: netdev, daniel.zahka, kuba, willemdebruijn.kernel, davem, edumazet, pabeni, horms, andrew+netdev, shuah, rrameshbabu, raeds, cratiu, linux-kselftest Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider. Critical: 0 · High: 1 · Medium: 0 · Low: 0 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). - [High] The new check in psp_dev_encapsulate() (net/psp/psp_main.c:260-262) only looks for IPv6 extension headers… -- psp: drop transport mode packets with extension headers This adds a check to psp_dev_encapsulate() that rejects IPv6 packets whose network header length is not sizeof(struct ipv6hdr). Without it, the nexthdr of the fixed IPv6 header gets rewritten to UDP even when extension headers sit between it and the TCP header. That produced malformed packets, and on mlx5 unencrypted ones. > diff --git a/net/psp/psp_main.c b/net/psp/psp_main.c > index c9c1a8826b7f9..0d276e2aceff4 100644 > --- a/net/psp/psp_main.c > +++ b/net/psp/psp_main.c [ ... ] > @@ -256,6 +256,11 @@ bool psp_dev_encapsulate(struct net *net, struct sk_buff *skb, __be32 spi, > skb->protocol != htons(ETH_P_IPV6)) > return false; > > + /* extension headers not supported */ > + if (skb->protocol == htons(ETH_P_IPV6) && > + network_len != sizeof(struct ipv6hdr)) > + return false; > + [Severity: High] Does this check catch the case where a route inserts the extension headers? The commit message gives that case as the reason for a datapath check: "extension headers can also come from a route, so the datapath check is needed either way." network_len comes from skb_network_header_len(), which is only transport_header - network_header. The lwtunnel encapsulators that add IPv6 extension headers on egress insert the extension header and then set the transport header to the offset right after the fixed header: net/ipv6/seg6_iptunnel.c:seg6_do_srh() { ... skb_set_transport_header(skb, sizeof(struct ipv6hdr)); nf_reset_ct(skb); ... } rpl_do_srh_inline() in net/ipv6/rpl_iptunnel.c does the same. So do the ioam6 inline and encap helpers in net/ipv6/ioam6_iptunnel.c, which call skb_set_transport_header(skb, sizeof(*hdr)). Take a PSP TCP flow that leaves the PSP netdev through a seg6, rpl or ioam6 lwtunnel route. Its network_len is exactly sizeof(struct ipv6hdr), even though a Routing or Hop-by-Hop header sits right after the fixed header: seg6_output_core() seg6_do_srh() skb_set_transport_header(skb, sizeof(struct ipv6hdr)) dev_queue_xmit() psp_validate_xmit() mlx5e_psp_handle_tx_skb() (or nsim_do_psp()) psp_dev_encapsulate() network_len == sizeof(struct ipv6hdr), so the new check passes After that, psp_dev_encapsulate() still does: ipv6_hdr(skb)->nexthdr = IPPROTO_UDP; This overwrites NEXTHDR_ROUTING or NEXTHDR_HOP. The UDP and PSP headers end up in front of the extension header. psp_write_headers() also sets psph->nexthdr = IPPROTO_TCP, but the next bytes are an SRH or HBH header. Wouldn't this produce the same kind of malformed packet that this patch is meant to drop? The follow-up selftest patch in the series doesn't touch psp_main.c, so this path still seems reachable at the end of the series. Could the check look at the header chain itself instead? For example, rejecting the packet when ipv6_hdr(skb)->nexthdr != IPPROTO_TCP would cover both the socket option case and the route case. > if (skb_cow_head(skb, PSP_ENCAP_HLEN)) > return false; > -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261021210200.788191-1-toddfreed%40meta.com ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net 2/2] selftests: drv-net: psp: test ipv6 tcp socket with extension headers 2026-10-21 21:01 [PATCH net 0/2] psp: drop packets with extension headers in transport mode Todd Freed 2026-10-21 21:01 ` [PATCH net 1/2] psp: drop transport mode packets with extension headers Todd Freed @ 2026-10-21 21:02 ` Todd Freed 2026-10-25 21:30 ` netdev-bot+sashiko 2026-10-21 22:07 ` [PATCH net 0/2] psp: drop packets with extension headers in transport mode Daniel Zahka 2026-10-22 15:19 ` Willem de Bruijn 3 siblings, 1 reply; 13+ messages in thread From: Todd Freed @ 2026-10-21 21:02 UTC (permalink / raw) To: netdev, toddfreed Cc: Daniel Zahka, Jakub Kicinski, Willem de Bruijn, David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman, Andrew Lunn, Shuah Khan, Rahul Rameshbabu, Raed Salem, Cosmin Ratiu, linux-kselftest send on a psp tcp socket in three phases, checking the device's tx-packets and what the peer received after each 1/ no socket options - the data is sent and counted. 2/ an extension header set on the socket - assert that the packets are dropped, that tx-packets is unchanged, and that nothing reaches the peer. the data stays queued. 3/ option removed - the queued data and new data are sent and counted, and the peer gets all of it. phases 2 and 3 run twice, with a destination options header (IPV6_DSTOPTS) and with a hop-by-hop options header (IPV6_HOPOPTS), each holding one padn option. in phase 2, a software filter on the peer also counts udp packets from the socket's address to the peer's. the well-formed psp packets reach the peer as decrypted tcp packets, as expected. the malformed packets show up as udp. if the peer can't take the filter, that check is skipped. tx-packets is read after it has quiesced, because some devices refresh counter periodically (mlx5 takes about a second). without the fix in the preceding patch: - with netdevsim, the packets of phase 2 are encapsulated and counted in tx-packets, and the test fails on that. - with mlx5 (connectx-7), the packets go out unencrypted and aren't counted in tx-packets. the test fails on the udp packets which reach the peer. capturing on the peer, tcpdump -i <dev> -nn -A 'ip6 and src host <sender> and not tcp' shows them as udp, with the test's payload readable Signed-off-by: Todd Freed <toddfreed@meta.com> --- tools/testing/selftests/drivers/net/config | 1 + tools/testing/selftests/drivers/net/psp.py | 120 ++++++++++++++++++++- 2 files changed, 119 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/drivers/net/config b/tools/testing/selftests/drivers/net/config index 4838adf27fa1..409f9a96a102 100644 --- a/tools/testing/selftests/drivers/net/config +++ b/tools/testing/selftests/drivers/net/config @@ -4,6 +4,7 @@ CONFIG_DEBUG_INFO_BTF_MODULES=n CONFIG_INET_PSP=y CONFIG_IPV6=y CONFIG_MACSEC=m +CONFIG_NET_ACT_GACT=m CONFIG_NET_ACT_SKBEDIT=m CONFIG_NET_CLS_ACT=y CONFIG_NET_CLS_BPF=y diff --git a/tools/testing/selftests/drivers/net/psp.py b/tools/testing/selftests/drivers/net/psp.py index a5b1e14f120f..2a109ef61468 100755 --- a/tools/testing/selftests/drivers/net/psp.py +++ b/tools/testing/selftests/drivers/net/psp.py @@ -16,12 +16,12 @@ from lib.py import ksft_run, ksft_exit, ksft_pr from lib.py import ksft_true, ksft_eq, ksft_ne, ksft_gt, ksft_raises from lib.py import ksft_not_none from lib.py import ksft_variants, KsftNamedVariant -from lib.py import KsftSkipEx, KsftFailEx +from lib.py import KsftSkipEx, KsftFailEx, CmdExitFailure from lib.py import NetDrvEpEnv, NetDrvContEnv from lib.py import Netlink, NlError, PSPFamily, RtnlFamily from lib.py import NetNSEnter from lib.py import bkg, rand_port, wait_port_listen -from lib.py import ip +from lib.py import ip, tc TCP_ULP = 31 @@ -120,6 +120,57 @@ def _check_data_outq(s, exp_len, force_wait=False): def _get_stat(cfg, key): return cfg.pspnl.get_stats({'dev-id': cfg.psp_dev_id})[key] + +def _get_quiesced_stat(cfg, key, timeout=10): + """ + Like _get_stat(), but wait till the value stops changing. Note + that some devices refresh flow counters periodically, e.g. once + per second with mlx5. + """ + val = _get_stat(cfg, key) + end = time.monotonic() + timeout + while True: + time.sleep(1.5) + new = _get_stat(cfg, key) + if new == val: + return val + val = new + if time.monotonic() >= end: + ksft_pr(f"{key} still changing after {timeout}s: {val}") + return val + + +def _remote_udp_from(cfg, src, dst): + """ + Count what reaches the remote from @src to @dst as UDP, with a software + filter on its ingress (offloaded filter counters are refreshed + periodically). Returns a function reading the count, or None if the + filter can't be installed. + """ + flt = (f"dev {cfg.remote_ifname} ingress protocol ipv6 pref 1 " + f"handle 0x5053 flower") + try: + qdiscs = tc(f"qdisc show dev {cfg.remote_ifname}", json=True, + host=cfg.remote) + if not any(q['kind'] == 'clsact' for q in qdiscs): + tc(f"qdisc add dev {cfg.remote_ifname} clsact", host=cfg.remote) + defer(tc, f"qdisc del dev {cfg.remote_ifname} clsact", + host=cfg.remote) + tc(f"filter add {flt} skip_hw src_ip {src} dst_ip {dst} " + f"ip_proto udp action drop", host=cfg.remote) + except CmdExitFailure as e: + ksft_pr(f"Can't count UDP on the remote: {e}") + return None + defer(tc, f"filter del {flt}", host=cfg.remote) + + def count(): + filters = tc(f"-s filter get {flt}", json=True, host=cfg.remote) + for f in filters: + for act in f.get("options", {}).get("actions", []): + return act["stats"]["packets"] + raise KsftFailEx("no action on the remote filter") + return count + # # Test case boiler plate # @@ -624,6 +675,71 @@ def removal_device_bi(cfg): _close_conn(cfg, s) +def data_send_ipv6_exthdr(cfg): + """ Test that IPv6 packets with extension headers are dropped """ + cfg.require_ipver("6") + _init_psp_dev(cfg) + + s = _make_psp_conn(cfg, 0, "6") + + rx_assoc = cfg.pspnl.rx_assoc({"version": 0, + "dev-id": cfg.psp_dev_id, + "sock-fd": s.fileno()}) + tx = _spi_xchg(s, rx_assoc['rx-key']) + cfg.pspnl.tx_assoc({"dev-id": cfg.psp_dev_id, + "version": 0, + "tx-key": tx, + "sock-fd": s.fileno()}) + _check_data_outq(s, 0) + + def wait_outq_empty(): + for _ in range(100): + if _get_outq(s) == 0: + break + time.sleep(0.5) + _check_data_outq(s, 0) + + # 1. No options: sent and counted + tx_pkts = _get_quiesced_stat(cfg, 'tx-packets') + total = _send_careful(cfg, s, 20) + wait_outq_empty() + ksft_gt(_get_quiesced_stat(cfg, 'tx-packets'), tx_pkts) + _check_data_rx(cfg, total) + + # In happy path, PSP traffic arrives at the remote decrypted as TCP + remote_udp = _remote_udp_from(cfg, s.getsockname()[0], + s.getpeername()[0]) + + # An extension header with one PadN option, as destination options and + # as hop-by-hop options + for name, opt in (("IPV6_DSTOPTS", socket.IPV6_DSTOPTS), + ("IPV6_HOPOPTS", socket.IPV6_HOPOPTS)): + # 2. With the option: dropped, not counted, and nothing reaches + # the peer. The data stays queued. + s.setsockopt(socket.IPPROTO_IPV6, opt, + bytes([0, 0, 1, 4, 0, 0, 0, 0])) + tx_pkts = _get_quiesced_stat(cfg, 'tx-packets') + udp = remote_udp() if remote_udp else 0 + queued = _send_careful(cfg, s, 20) + _check_data_outq(s, queued, force_wait=True) + ksft_eq(_get_quiesced_stat(cfg, 'tx-packets'), tx_pkts, comment=name) + if remote_udp: + ksft_eq(remote_udp(), udp, comment=f"{name}: UDP at the remote") + _check_data_rx(cfg, total) + total += queued + + # 3. Option removed: the queued data, then new data, sent and + # counted + tx_pkts = _get_quiesced_stat(cfg, 'tx-packets') + s.setsockopt(socket.IPPROTO_IPV6, opt, b'') + wait_outq_empty() + total += _send_careful(cfg, s, 20) + wait_outq_empty() + ksft_gt(_get_quiesced_stat(cfg, 'tx-packets'), tx_pkts, comment=name) + _check_data_rx(cfg, total) + _close_psp_conn(cfg, s) + + def _get_psp_ver_ip_variants(): for ver in range(4): for ipv in ("4", "6"): -- 2.53.0-Meta ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH net 2/2] selftests: drv-net: psp: test ipv6 tcp socket with extension headers 2026-10-21 21:02 ` [PATCH net 2/2] selftests: drv-net: psp: test ipv6 tcp socket " Todd Freed @ 2026-10-25 21:30 ` netdev-bot+sashiko 0 siblings, 0 replies; 13+ messages in thread From: netdev-bot+sashiko @ 2026-10-25 21:30 UTC (permalink / raw) To: toddfreed Cc: netdev, daniel.zahka, kuba, willemdebruijn.kernel, davem, edumazet, pabeni, horms, andrew+netdev, shuah, rrameshbabu, raeds, cratiu, linux-kselftest Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider. Critical: 0 · High: 0 · Medium: 1 · Low: 2 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] On hardware PSP devices such as mlx5, data_send_ipv6_exthdr() in tools/testing/selftests/drivers/net/psp.py passes whether or not… - [Low] The docstring of _remote_udp_from() in tools/testing/selftests/drivers/net/psp.py describes the filter as… - [Low] data_send_ipv6_exthdr() in tools/testing/selftests/drivers/net/psp.py opens the PSP connection at… -- selftests: drv-net: psp: test ipv6 tcp socket with extension headers This adds data_send_ipv6_exthdr() to the psp.py selftest. It sends on an IPv6 PSP TCP socket without options, then with IPV6_DSTOPTS or IPV6_HOPOPTS set, then with the option removed. After each phase it checks tx-packets and what the peer received. It also adds the _get_quiesced_stat() and _remote_udp_from() helpers and enables CONFIG_NET_ACT_GACT in the config fragment. > diff --git a/tools/testing/selftests/drivers/net/psp.py b/tools/testing/selftests/drivers/net/psp.py > index a5b1e14f120f0..2a109ef61468a 100755 > --- a/tools/testing/selftests/drivers/net/psp.py > +++ b/tools/testing/selftests/drivers/net/psp.py [ ... ] > @@ -120,6 +120,57 @@ def _check_data_outq(s, exp_len, force_wait=False): [ ... ] > +def _remote_udp_from(cfg, src, dst): > + """ > + Count what reaches the remote from @src to @dst as UDP, with a software > + filter on its ingress (offloaded filter counters are refreshed > + periodically). Returns a function reading the count, or None if the > + filter can't be installed. > + """ > + flt = (f"dev {cfg.remote_ifname} ingress protocol ipv6 pref 1 " > + f"handle 0x5053 flower") > + try: > + qdiscs = tc(f"qdisc show dev {cfg.remote_ifname}", json=True, > + host=cfg.remote) > + if not any(q['kind'] == 'clsact' for q in qdiscs): > + tc(f"qdisc add dev {cfg.remote_ifname} clsact", host=cfg.remote) > + defer(tc, f"qdisc del dev {cfg.remote_ifname} clsact", > + host=cfg.remote) > + tc(f"filter add {flt} skip_hw src_ip {src} dst_ip {dst} " > + f"ip_proto udp action drop", host=cfg.remote) [Severity: Low] This isn't a bug, but the docstring says the filter counts UDP from @src to @dst. The filter is installed with "action drop", though, so matching packets are also dropped while it is in place. Could the docstring mention the drop? Alternatively, "action pass" would keep the filter a pure counter. > + except CmdExitFailure as e: > + ksft_pr(f"Can't count UDP on the remote: {e}") > + return None [ ... ] > @@ -624,6 +675,71 @@ def removal_device_bi(cfg): > _close_conn(cfg, s) > > > +def data_send_ipv6_exthdr(cfg): > + """ Test that IPv6 packets with extension headers are dropped """ > + cfg.require_ipver("6") > + _init_psp_dev(cfg) > + > + s = _make_psp_conn(cfg, 0, "6") [ ... ] > + for name, opt in (("IPV6_DSTOPTS", socket.IPV6_DSTOPTS), > + ("IPV6_HOPOPTS", socket.IPV6_HOPOPTS)): > + # 2. With the option: dropped, not counted, and nothing reaches > + # the peer. The data stays queued. > + s.setsockopt(socket.IPPROTO_IPV6, opt, > + bytes([0, 0, 1, 4, 0, 0, 0, 0])) > + tx_pkts = _get_quiesced_stat(cfg, 'tx-packets') > + udp = remote_udp() if remote_udp else 0 > + queued = _send_careful(cfg, s, 20) > + _check_data_outq(s, queued, force_wait=True) > + ksft_eq(_get_quiesced_stat(cfg, 'tx-packets'), tx_pkts, comment=name) > + if remote_udp: > + ksft_eq(remote_udp(), udp, comment=f"{name}: UDP at the remote") [Severity: Medium] On mlx5, is this the only check that can tell a fixed kernel from an unfixed one? The commit message says that without the fix, mlx5 sends the phase 2 packets unencrypted and doesn't count them in tx-packets. The driver takes tx_packets from a hardware counter: drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c:mlx5e_psp_get_stats() { ... stats->tx_packets = nstats.psp_tx_pkts; stats->tx_bytes = nstats.psp_tx_bytes; stats->tx_error = atomic_read(&priv->psp->tx_drop); ... } So the ksft_eq() on tx-packets holds with or without the fix. The leaked packets reach the peer as UDP, not TCP, so _check_data_rx() also passes. Phase 3 retransmits without the option, so it passes too. _remote_udp_from() returns None from its except block on any CmdExitFailure. Examples: no tc, flower or gact on the remote, no permission, or an existing filter at pref 1 / handle 0x5053. In that case udp is set to 0, this check is skipped, and the test reports PASS. The commit message does say "if the peer can't take the filter, that check is skipped". Even so, could the test raise KsftXfailEx or KsftSkipEx when remote_udp is None on a device other than netdevsim? That way CI could see that the case wasn't covered. Another option is a check that doesn't depend on the remote, such as tx-error. mlx5 reports it from priv->psp->tx_drop, which increments when psp_dev_encapsulate() fails. netdevsim doesn't report tx-error, so this check would have to be per device. > + _check_data_rx(cfg, total) > + total += queued [ ... ] > + _check_data_rx(cfg, total) > + _close_psp_conn(cfg, s) [Severity: Low] Should the code from _make_psp_conn() up to here be wrapped in try/finally? _data_mss_adjust(), data_stale_key(), removal_device_rx() and removal_device_bi() all close the connection from a finally block. ksft_eq() and ksft_gt() don't raise, but several calls in between can: - _send_careful() raises RuntimeError after 10 EAGAIN retries. This is more likely in phase 2, where nothing gets acked. - The count() closure raises KsftFailEx. Its "tc -s filter get" call can also raise CmdExitFailure, since it is outside the try/except in _remote_udp_from(). - s.setsockopt() can raise OSError. - The rx_assoc and tx_assoc calls can raise NlError. If any of these raises, "data close" is never sent. The responder keeps its data socket until the next accept, where it prints "WARN: new data sock but old one still here". The local socket is only closed once it's garbage collected. If the exception happens in phase 2, the socket still has the extension header option and queued data. It then keeps retransmitting packets that psp_dev_encapsulate() drops, until it times out. -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261021210200.788191-1-toddfreed%40meta.com ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net 0/2] psp: drop packets with extension headers in transport mode 2026-10-21 21:01 [PATCH net 0/2] psp: drop packets with extension headers in transport mode Todd Freed 2026-10-21 21:01 ` [PATCH net 1/2] psp: drop transport mode packets with extension headers Todd Freed 2026-10-21 21:02 ` [PATCH net 2/2] selftests: drv-net: psp: test ipv6 tcp socket " Todd Freed @ 2026-10-21 22:07 ` Daniel Zahka 2026-10-21 23:16 ` Todd Bert Freed 2026-10-22 15:19 ` Willem de Bruijn 3 siblings, 1 reply; 13+ messages in thread From: Daniel Zahka @ 2026-10-21 22:07 UTC (permalink / raw) To: Todd Freed, netdev Cc: Daniel Zahka, Jakub Kicinski, Willem de Bruijn, David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman, Andrew Lunn, Shuah Khan, Rahul Rameshbabu, Raed Salem, Cosmin Ratiu, linux-kselftest On Wed Oct 21, 2026 at 5:01 PM EDT, Todd Freed wrote: > While working on adding support for socket-less psp associations, I > noticed that this case was not being handled correctly. It will be more > relevant with that series, because it adds udp support, for which the > psp nexthdr will be taken from the inner packet. > > Since the tx association encodes intent to send encrypted packets, but > because of other configuration (socket options, routes) the packets are > sent unencrypted, sending this change as a fix. > > The first patch drops the packets, the second patch adds selftests for > that case. > Why not just fix psp_dev_encapsulate() to put the psp headers in the right position? With the self test we are going to make PSP + ipv6 ext header == drop part of the uapi? > Todd Freed (2): > psp: drop transport mode packets with extension headers > selftests: drv-net: psp: test ipv6 tcp socket with extension headers > > net/psp/psp_main.c | 7 +- > tools/testing/selftests/drivers/net/config | 1 + > tools/testing/selftests/drivers/net/psp.py | 120 ++++++++++++++++++++- > 3 files changed, 125 insertions(+), 3 deletions(-) ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net 0/2] psp: drop packets with extension headers in transport mode 2026-10-21 22:07 ` [PATCH net 0/2] psp: drop packets with extension headers in transport mode Daniel Zahka @ 2026-10-21 23:16 ` Todd Bert Freed 2026-10-22 11:59 ` Björn Töpel 2026-10-22 12:06 ` Daniel Zahka 0 siblings, 2 replies; 13+ messages in thread From: Todd Bert Freed @ 2026-10-21 23:16 UTC (permalink / raw) To: Daniel Zahka Cc: netdev, Jakub Kicinski, Willem de Bruijn, David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman, Andrew Lunn, Shuah Khan, Rahul Rameshbabu, Raed Salem, Cosmin Ratiu, linux-kselftest On Wed, Oct 21, 2026 at 3:07 PM Daniel Zahka <daniel.zahka@gmail.com> wrote: > > > > The first patch drops the packets, the second patch adds selftests for > > that case. > > > > Why not just fix psp_dev_encapsulate() to put the psp headers in the > right position? With the self test we are going to make PSP + ipv6 ext > header == drop part of the uapi? If the original ipv6 packet has extension headers, then the nexthdr on the fixed ipv6 header identifies the first extension header, and the L4 proto is in the nexthdr on the last one in the chain. psp_dev_encapsulate() places the udp+psp headers after all the extension headers in this case. The issue is that it rewrites the fixed ipv6 header nexthdr to be UDP, ipv6_hdr(skb)->nexthdr = IPPROTO_UDP; It would need to walk the chain, and set the nexthdr on the final extension header to UDP, instead, if the correct policy were for udp+psp to come after the extension headers in all cases. I'm not sure that's the correct policy though ; for some headers (e.g. the fragment header) it might make sense to leave them on the inner packet. Note that the psp spec does not describe this. Perhaps the user should express the desired policy? Explicitly dropping ipv6 packets with fragments is consistent with the current behavior (it already doesn't work). -Todd ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net 0/2] psp: drop packets with extension headers in transport mode 2026-10-21 23:16 ` Todd Bert Freed @ 2026-10-22 11:59 ` Björn Töpel 2026-10-22 12:06 ` Daniel Zahka 1 sibling, 0 replies; 13+ messages in thread From: Björn Töpel @ 2026-10-22 11:59 UTC (permalink / raw) To: Todd Bert Freed, Daniel Zahka Cc: netdev, Jakub Kicinski, Willem de Bruijn, David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman, Andrew Lunn, Shuah Khan, Rahul Rameshbabu, Raed Salem, Cosmin Ratiu, linux-kselftest Todd Bert Freed <toddfreed@meta.com> writes: > On Wed, Oct 21, 2026 at 3:07 PM Daniel Zahka <daniel.zahka@gmail.com> wrote: >> > >> > The first patch drops the packets, the second patch adds selftests for >> > that case. >> > >> >> Why not just fix psp_dev_encapsulate() to put the psp headers in the >> right position? With the self test we are going to make PSP + ipv6 ext >> header == drop part of the uapi? > > If the original ipv6 packet has extension headers, then the nexthdr on the > fixed ipv6 header identifies the first extension header, and the L4 proto is > in the nexthdr on the last one in the chain. > > psp_dev_encapsulate() places the udp+psp headers after all the extension > headers in this case. > > The issue is that it rewrites the fixed ipv6 header nexthdr to be UDP, > > ipv6_hdr(skb)->nexthdr = IPPROTO_UDP; > > It would need to walk the chain, and set the nexthdr on the final extension > header to UDP, instead, if the correct policy were for udp+psp to come > after the extension headers in all cases. > > I'm not sure that's the correct policy though ; for some headers (e.g. the > fragment header) it might make sense to leave them on the inner packet. > Note that the psp spec does not describe this. Perhaps the user should > express the desired policy? Fragment extension header would be weird to keep in transport mode... I'd say drop is the right call. Björn ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net 0/2] psp: drop packets with extension headers in transport mode 2026-10-21 23:16 ` Todd Bert Freed 2026-10-22 11:59 ` Björn Töpel @ 2026-10-22 12:06 ` Daniel Zahka 2026-10-22 22:43 ` Todd Bert Freed 1 sibling, 1 reply; 13+ messages in thread From: Daniel Zahka @ 2026-10-22 12:06 UTC (permalink / raw) To: Todd Bert Freed, Daniel Zahka Cc: netdev, Jakub Kicinski, Willem de Bruijn, David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman, Andrew Lunn, Shuah Khan, Rahul Rameshbabu, Raed Salem, Cosmin Ratiu, linux-kselftest On Wed Oct 21, 2026 at 7:16 PM EDT, Todd Bert Freed wrote: > On Wed, Oct 21, 2026 at 3:07 PM Daniel Zahka <daniel.zahka@gmail.com> wrote: >> > >> > The first patch drops the packets, the second patch adds selftests for >> > that case. >> > >> >> Why not just fix psp_dev_encapsulate() to put the psp headers in the >> right position? With the self test we are going to make PSP + ipv6 ext >> header == drop part of the uapi? > > If the original ipv6 packet has extension headers, then the nexthdr on the > fixed ipv6 header identifies the first extension header, and the L4 proto is > in the nexthdr on the last one in the chain. > > psp_dev_encapsulate() places the udp+psp headers after all the extension > headers in this case. > > The issue is that it rewrites the fixed ipv6 header nexthdr to be UDP, > > ipv6_hdr(skb)->nexthdr = IPPROTO_UDP; > > It would need to walk the chain, and set the nexthdr on the final extension > header to UDP, instead, if the correct policy were for udp+psp to come > after the extension headers in all cases. > > I'm not sure that's the correct policy though ; for some headers (e.g. the > fragment header) it might make sense to leave them on the inner packet. > Note that the psp spec does not describe this. Perhaps the user should > express the desired policy? > > Explicitly dropping ipv6 packets with fragments is consistent with the > current behavior (it already doesn't work). > > -Todd I think walking the extension headers and inserting before the transport header is better than dropping. I agree with Bjorn that rejecting fragment header makes sense because the receiver can't decrypt the fragmented PSP payload. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net 0/2] psp: drop packets with extension headers in transport mode 2026-10-22 12:06 ` Daniel Zahka @ 2026-10-22 22:43 ` Todd Bert Freed 0 siblings, 0 replies; 13+ messages in thread From: Todd Bert Freed @ 2026-10-22 22:43 UTC (permalink / raw) To: Daniel Zahka Cc: netdev, Jakub Kicinski, Willem de Bruijn, David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman, Andrew Lunn, Shuah Khan, Rahul Rameshbabu, Raed Salem, Cosmin Ratiu, linux-kselftest > > Explicitly dropping ipv6 packets with fragments is consistent with the > > current behavior (it already doesn't work). > > > > -Todd > > I think walking the extension headers and inserting before the transport > header is better than dropping. I agree with Bjorn that rejecting > fragment header makes sense because the receiver can't decrypt the > fragmented PSP payload. Sounds good. Please see v2, where I have implemented this. After doing more research today, I discovered reasons why routing headers will also not cleanly work, so dropping those as well. -Todd ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net 0/2] psp: drop packets with extension headers in transport mode 2026-10-21 21:01 [PATCH net 0/2] psp: drop packets with extension headers in transport mode Todd Freed ` (2 preceding siblings ...) 2026-10-21 22:07 ` [PATCH net 0/2] psp: drop packets with extension headers in transport mode Daniel Zahka @ 2026-10-22 15:19 ` Willem de Bruijn 2026-10-22 16:58 ` Todd Bert Freed 3 siblings, 1 reply; 13+ messages in thread From: Willem de Bruijn @ 2026-10-22 15:19 UTC (permalink / raw) To: Todd Freed, netdev, toddfreed Cc: Daniel Zahka, Jakub Kicinski, Willem de Bruijn, David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman, Andrew Lunn, Shuah Khan, Rahul Rameshbabu, Raed Salem, Cosmin Ratiu, linux-kselftest Todd Freed wrote: > While working on adding support for socket-less psp associations, I > noticed that this case was not being handled correctly. It will be more > relevant with that series, because it adds udp support, for which the > psp nexthdr will be taken from the inner packet. Just curious: what is the plan for replay protection for UDP? As PSP depends on the inner L4 for replay protection today. FWIW agreed on the ext_hdr conclusion in this thread. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net 0/2] psp: drop packets with extension headers in transport mode 2026-10-22 15:19 ` Willem de Bruijn @ 2026-10-22 16:58 ` Todd Bert Freed 2026-10-22 20:35 ` Willem de Bruijn 0 siblings, 1 reply; 13+ messages in thread From: Todd Bert Freed @ 2026-10-22 16:58 UTC (permalink / raw) To: Willem de Bruijn Cc: netdev, Daniel Zahka, Jakub Kicinski, David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman, Andrew Lunn, Shuah Khan, Rahul Rameshbabu, Raed Salem, Cosmin Ratiu, linux-kselftest > Todd Freed wrote: > > While working on adding support for socket-less psp associations, I > > noticed that this case was not being handled correctly. It will be more > > relevant with that series, because it adds udp support, for which the > > psp nexthdr will be taken from the inner packet. > > Just curious: what is the plan for replay protection for UDP? > > As PSP depends on the inner L4 for replay protection today. > > FWIW agreed on the ext_hdr conclusion in this thread. To clarify, socket-less psp associations will apply tunnel-mode psp encap. So it is in this sense that UDP will be supported (not e.g. adding support for udp sockets with transport mode psp encap). That being said, UDP which is PSP encrypted (transport or tunnel mode) would be susceptible to replay attacks, just like regular UDP. Except in the sense that after enough time has passed, the packet would not decrypt if received again (operator has rotated the keys, etc). Not sure if you had some other kind of replay protection in mind. -Todd ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net 0/2] psp: drop packets with extension headers in transport mode 2026-10-22 16:58 ` Todd Bert Freed @ 2026-10-22 20:35 ` Willem de Bruijn 0 siblings, 0 replies; 13+ messages in thread From: Willem de Bruijn @ 2026-10-22 20:35 UTC (permalink / raw) To: Todd Bert Freed, Willem de Bruijn Cc: netdev, Daniel Zahka, Jakub Kicinski, David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman, Andrew Lunn, Shuah Khan, Rahul Rameshbabu, Raed Salem, Cosmin Ratiu, linux-kselftest Todd Bert Freed wrote: > > Todd Freed wrote: > > > While working on adding support for socket-less psp associations, I > > > noticed that this case was not being handled correctly. It will be more > > > relevant with that series, because it adds udp support, for which the > > > psp nexthdr will be taken from the inner packet. > > > > Just curious: what is the plan for replay protection for UDP? > > > > As PSP depends on the inner L4 for replay protection today. > > > > FWIW agreed on the ext_hdr conclusion in this thread. > > To clarify, socket-less psp associations will apply tunnel-mode psp encap. > So it is in this sense that UDP will be supported (not e.g. adding support > for udp sockets with transport mode psp encap). > > That being said, UDP which is PSP encrypted (transport or tunnel mode) > would be susceptible to replay attacks, just like regular UDP. Except in > the sense that after enough time has passed, the packet would not > decrypt if received again (operator has rotated the keys, etc). > > Not sure if you had some other kind of replay protection in mind. Got it, thanks. No. I'm just slightly concerned that casual users of such a feature might assume that the security protocol handles replay protection. ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-10-25 21:30 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-10-21 21:01 [PATCH net 0/2] psp: drop packets with extension headers in transport mode Todd Freed 2026-10-21 21:01 ` [PATCH net 1/2] psp: drop transport mode packets with extension headers Todd Freed 2026-10-25 21:30 ` netdev-bot+sashiko 2026-10-21 21:02 ` [PATCH net 2/2] selftests: drv-net: psp: test ipv6 tcp socket " Todd Freed 2026-10-25 21:30 ` netdev-bot+sashiko 2026-10-21 22:07 ` [PATCH net 0/2] psp: drop packets with extension headers in transport mode Daniel Zahka 2026-10-21 23:16 ` Todd Bert Freed 2026-10-22 11:59 ` Björn Töpel 2026-10-22 12:06 ` Daniel Zahka 2026-10-22 22:43 ` Todd Bert Freed 2026-10-22 15:19 ` Willem de Bruijn 2026-10-22 16:58 ` Todd Bert Freed 2026-10-22 20:35 ` Willem de Bruijn
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox