[PATCH net v3 0/4] psp: fix transport mode with ipv6 extension headers

[PATCH net v3 0/4] psp: fix transport mode with ipv6 extension headers

[Todd Freed]

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 fixes an mlx5e bug in tso checksum calculation for psp
with ipv6 routing headers.

The second patch fixes the receive path when ipv6 extension headers are
present.

The third patch fixes the send path when ipv6 extension headers are
present.

The fourth patch adds selftests for these cases.

Note that no test case is added for ipv6 fragment headers. As far as
I can tell it's not possible to tx fragments from an ipv6 tcp socket.

All testing was done on connectx-7.

v3:
 - new patch 1 to fix tso checksum seed calculation in mlx5e (Daniel)
 - accept routing headers and reject ah (Daniel)
 - split the psp_dev_rcv() change into its own patch (Daniel)
 - return -EINVAL instead of -1 (Daniel)
 - selftest: one variant per extension header type (Daniel)

v2: https://lore.kernel.org/netdev/20261022223014.556388-1-toddfreed@meta.com
 - walk the extension headers instead of dropping (Daniel)
 - reject fragment headers (Bjorn, Daniel)
 - reject routing headers
 - psp_dev_rcv() handle the rx case as well

v1: https://lore.kernel.org/netdev/20261021210200.788191-1-toddfreed@meta.com

Todd Freed (4):
  net/mlx5e: psp: tso: replace only the length in checksum seed
  psp: support ipv6 extension headers in psp_dev_rcv() decap
  psp: support ipv6 extension headers in psp_dev_encapsulate()
  selftests: drv-net: psp: test ipv6 tcp socket with extension headers

 .../mellanox/mlx5/core/en_accel/psp_rxtx.c    |  21 ++-
 net/psp/psp_main.c                            |  79 +++++++++-
 tools/testing/selftests/drivers/net/config    |   1 +
 tools/testing/selftests/drivers/net/psp.py    | 137 +++++++++++++++++-
 4 files changed, 215 insertions(+), 23 deletions(-)

-- 
2.53.0-Meta

[PATCH net v3 1/4] net/mlx5e: psp: tso: replace only the length in checksum seed

[Todd Freed]

for psp tcp with tso,

 - __tcp_transmit_skb() -> tcp_v6_send_check() or tcp_v4_send_check()
   computes the checksum seed using the addresses on the socket and the
   total length, then

 - mlx5e_psp_handle_tx_skb() recomputes the checksum seed using
   addresses from the packet's l3 header, and the length of one segment

the length difference is expected by the device. however, the
difference in addresses is incorrect in cases where the l3 header daddr
is an intermediate hop (e.g. IPV6_RTHDR). this causes every segment of
the tso packet to go out with a bad tcp checksum, in this case.

the fix is to replace only the length in the seed which tcp had already
computed. this is also what tcp_gso_segment() does.

tested on connectx-7 with the rest of this series applied, with a psp
tcp socket whose segment routing header's first hop isn't the final
destination, and the peer checking tcp checksums in software.

Fixes: e5a1861a298e ("net/mlx5e: Implement PSP Tx data path")
Signed-off-by: Todd Freed <toddfreed@meta.com>
---
 .../mellanox/mlx5/core/en_accel/psp_rxtx.c    | 21 ++++++++-----------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp_rxtx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp_rxtx.c
index 348fd7a96261..d9c2742b3c24 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp_rxtx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp_rxtx.c
@@ -177,18 +177,15 @@ bool mlx5e_psp_handle_tx_skb(struct net_device *netdev,
 		return false;
 	}
 	if (skb_is_gso(skb)) {
-		int len = skb_shinfo(skb)->gso_size + inner_tcp_hdrlen(skb);
-		struct tcphdr *th = inner_tcp_hdr(skb);
-
-		if (skb->protocol == htons(ETH_P_IP)) {
-			const struct iphdr *ip = ip_hdr(skb);
-
-			th->check = ~tcp_v4_check(len, ip->saddr, ip->daddr, 0);
-		} else {
-			const struct ipv6hdr *ip6 = ipv6_hdr(skb);
-
-			th->check = ~tcp_v6_check(len, &ip6->saddr, &ip6->daddr, 0);
-		}
+		u32 seg_len = skb_shinfo(skb)->gso_size + inner_tcp_hdrlen(skb);
+		u32 len = skb->len - skb_inner_transport_offset(skb);
+
+		/* update just the length component of the checksum seed
+		 * tcp computed the seed with the total length, but mlx5
+		 * devices are expecting the segment length
+		 */
+		inet_proto_csum_replace4(&inner_tcp_hdr(skb)->check, skb,
+					 htonl(len), htonl(seg_len), true);
 	}
 
 	return true;
-- 
2.53.0-Meta

[PATCH net v3 2/4] psp: support ipv6 extension headers in psp_dev_rcv() decap

[Todd Freed]

psp_dev_rcv() expects the udp header to immediately follow the fixed
ipv6 header, so packets with ipv6 extension headers are dropped by the
is_udp check. additionally, on decap it writes the inner nexthdr into
the fixed header instead of into the last extension header.

fix by skipping the extension headers to find the udp header, and on
decap, setting the inner nexthdr on the last one.

fragment and authentication headers (ah) are not supported in psp
transport mode, so such packets are dropped.

Fixes: 0eddb8023cee ("psp: provide decapsulation and receive helper for drivers")
Signed-off-by: Todd Freed <toddfreed@meta.com>
---
 net/psp/psp_main.c | 62 +++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 59 insertions(+), 3 deletions(-)

diff --git a/net/psp/psp_main.c b/net/psp/psp_main.c
index c9c1a8826b7f..946047440ed3 100644
--- a/net/psp/psp_main.c
+++ b/net/psp/psp_main.c
@@ -4,6 +4,7 @@
 #include <linux/list.h>
 #include <linux/netdevice.h>
 #include <linux/xarray.h>
+#include <net/ipv6.h>
 #include <net/net_namespace.h>
 #include <net/psp.h>
 #include <net/udp.h>
@@ -185,6 +186,39 @@ unsigned int psp_key_size(u32 version)
 }
 EXPORT_SYMBOL(psp_key_size);
 
+/**
+ * psp_ipv6_nexthdr_off() - find the offset to the l4 proto nexthdr
+ * @hdr:	ipv6 fixed header
+ * @len:	l3 header total length
+ *
+ * Return: offset from @hdr, or a negative value to drop the packet
+ */
+static int psp_ipv6_nexthdr_off(const u8 *hdr, unsigned int len)
+{
+	unsigned int nh = offsetof(struct ipv6hdr, nexthdr);
+	unsigned int off = sizeof(struct ipv6hdr);
+	const struct ipv6_opt_hdr *eh;
+	unsigned int rem;
+	u8 type;
+
+	while (off < len) {
+		eh = (const void *)(hdr + off);
+		type = hdr[nh];
+
+		if (!ipv6_ext_hdr(type) || type == NEXTHDR_NONE ||
+		    type == NEXTHDR_FRAGMENT || type == NEXTHDR_AUTH)
+			return -EINVAL;
+		rem = len - off;
+		if (rem < sizeof(*eh))
+			return -EINVAL;
+
+		nh = off + offsetof(struct ipv6_opt_hdr, nexthdr);
+		off += ipv6_optlen(eh);
+	}
+
+	return off == len ? nh : -EINVAL;
+}
+
 static void psp_write_headers(struct net *net, struct sk_buff *skb, __be32 spi,
 			      u8 ver, unsigned int udp_len, __be16 sport)
 {
@@ -303,6 +337,7 @@ int psp_dev_rcv(struct sk_buff *skb, u16 dev_id, u8 generation, bool strip_icv)
 	struct psphdr *psph;
 	struct ethhdr *eth;
 	struct udphdr *uh;
+	int nh_off = 0;
 	__be16 proto;
 	bool is_udp;
 
@@ -331,8 +366,28 @@ int psp_dev_rcv(struct sk_buff *skb, u16 dev_id, u8 generation, bool strip_icv)
 			return -EINVAL;
 	} else {
 		struct ipv6hdr *ipv6h = (struct ipv6hdr *)(skb->data + l2_hlen);
+		u8 nexthdr = ipv6h->nexthdr;
+		__be16 frag_off;
+		int off;
+
+		/* UDP + PSP come after any extension headers */
+		off = ipv6_skip_exthdr(skb, l2_hlen + sizeof(*ipv6h), &nexthdr,
+				       &frag_off);
+		if (unlikely(off < 0))
+			return -EINVAL;
+
+		is_udp = nexthdr == IPPROTO_UDP;
+		l3_hlen = off - l2_hlen;
+		if (l3_hlen != sizeof(struct ipv6hdr) &&
+		    !pskb_may_pull(skb, l2_hlen + l3_hlen + PSP_ENCAP_HLEN))
+			return -EINVAL;
 
-		is_udp = ipv6h->nexthdr == IPPROTO_UDP;
+		/* fragment and authentication headers are not supported in
+		 * transport mode
+		 */
+		nh_off = psp_ipv6_nexthdr_off(skb->data + l2_hlen, l3_hlen);
+		if (unlikely(nh_off < 0))
+			return -EINVAL;
 	}
 
 	if (unlikely(!is_udp))
@@ -387,10 +442,11 @@ int psp_dev_rcv(struct sk_buff *skb, u16 dev_id, u8 generation, bool strip_icv)
 	} else {
 		struct ipv6hdr *ipv6h = (struct ipv6hdr *)(skb->data + l2_hlen);
 
-		if (unlikely(ntohs(ipv6h->payload_len) < encap))
+		if (unlikely(ntohs(ipv6h->payload_len) <
+			     l3_hlen - sizeof(*ipv6h) + encap))
 			return -EINVAL;
 
-		ipv6h->nexthdr = psph->nexthdr;
+		((u8 *)ipv6h)[nh_off] = psph->nexthdr;
 		ipv6h->payload_len = htons(ntohs(ipv6h->payload_len) - encap);
 	}
 
-- 
2.53.0-Meta

[PATCH net v3 3/4] psp: support ipv6 extension headers in psp_dev_encapsulate()

[Todd Freed]

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 produces a malformed packet.

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.

fix by walking the extension header chain to rewrite the nexthdr on the
last one in the chain instead. the walk also rejects packets with
extension headers that aren't supported in transport mode,

- NEXTHDR_FRAGMENT. this could work, but udp+psp would need to come
  before it, which is different than how the other option types are
  handled. for now, keep fragments unsupported for transport mode.

- NEXTHDR_AUTH. changing what follows would break the AH ICV. Also, AH
  adds little incremental value when used with psp, whose ICV already
  authenticates the payload. AH remains unsupported for psp transport
  mode.

note that the stack doesn't fragment ipv6 tcp packets, so a psp socket
won't be able to exercise the tx fragment path.

Suggested-by: Daniel Zahka <daniel.zahka@gmail.com>
Fixes: fc724515741a ("psp: provide encapsulation helper for drivers")
Signed-off-by: Todd Freed <toddfreed@meta.com>
---
 net/psp/psp_main.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/net/psp/psp_main.c b/net/psp/psp_main.c
index 946047440ed3..e6d2136cb9a7 100644
--- a/net/psp/psp_main.c
+++ b/net/psp/psp_main.c
@@ -277,7 +277,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. With IPv6, they go after any extension headers.
  */
 bool psp_dev_encapsulate(struct net *net, struct sk_buff *skb, __be32 spi,
 			 u8 ver, __be16 sport)
@@ -285,10 +285,17 @@ bool psp_dev_encapsulate(struct net *net, struct sk_buff *skb, __be32 spi,
 	u32 network_len = skb_network_header_len(skb);
 	u32 ethr_len = skb_mac_header_len(skb);
 	u32 bufflen = ethr_len + network_len;
+	int nh_off = 0;
 
-	if (skb->protocol != htons(ETH_P_IP) &&
-	    skb->protocol != htons(ETH_P_IPV6))
+	if (skb->protocol == htons(ETH_P_IPV6)) {
+		/* UDP + PSP after the extension headers, if any */
+		nh_off = psp_ipv6_nexthdr_off(skb_network_header(skb),
+					      network_len);
+		if (nh_off < 0)
+			return false;
+	} else if (skb->protocol != htons(ETH_P_IP)) {
 		return false;
+	}
 
 	if (skb_cow_head(skb, PSP_ENCAP_HLEN))
 		return false;
@@ -306,7 +313,7 @@ bool psp_dev_encapsulate(struct net *net, struct sk_buff *skb, __be32 spi,
 		ip_hdr(skb)->check =
 			ip_fast_csum((u8 *)ip_hdr(skb), ip_hdr(skb)->ihl);
 	} else {
-		ipv6_hdr(skb)->nexthdr = IPPROTO_UDP;
+		skb_network_header(skb)[nh_off] = IPPROTO_UDP;
 		be16_add_cpu(&ipv6_hdr(skb)->payload_len, PSP_ENCAP_HLEN);
 	}
 
-- 
2.53.0-Meta

[PATCH net v3 4/4] selftests: drv-net: psp: test ipv6 tcp socket with extension headers

[Todd Freed]

send on a psp tcp socket in three phases, and after each, check that the
data was sent, counted in the device's tx-packets, and received by the
peer. phases:

 1/ no socket options.

 2/ an extension header set on the socket.

 3/ option removed.

the test has a variant for each supported extension header type. for
the routing header, the test enables seg6_enabled on the peer, which
otherwise drops packets with a segment routing header.

a software filter on the peer also counts udp packets from the socket's
address to the peer's. the psp packets reach the peer as decrypted tcp
packets, as expected. malformed or unencrypted packets show up as udp,
and the test asserts there are none. if the peer can't take the filter,
the test is skipped.

tx-packets is read after it has quiesced, because some devices refresh
counters periodically (mlx5 takes about a second).

without the fixes in the preceding patches:

- with netdevsim, the peer receives none of the data of phase 2. it
  stays in the send queue, and the test fails after waiting for it to
  drain.
- 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 | 137 ++++++++++++++++++++-
 2 files changed, 135 insertions(+), 3 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..2a56a7bf5d24 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 cmd, 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,85 @@ def removal_device_bi(cfg):
         _close_conn(cfg, s)
 
 
+def _ipv6_exthdr_variants():
+    padn = bytes([0, 0, 1, 4, 0, 0, 0, 0])  # one PadN option
+
+    def srh(peer):
+        """ segment routing header, with @peer as its only segment """
+        return (bytes([0, 2, 4, 0, 0, 0, 0, 0]) +
+                socket.inet_pton(socket.AF_INET6, peer))
+
+    yield KsftNamedVariant("dstopts", socket.IPV6_DSTOPTS, lambda peer: padn)
+    yield KsftNamedVariant("hopopts", socket.IPV6_HOPOPTS, lambda peer: padn)
+    yield KsftNamedVariant("rthdr", socket.IPV6_RTHDR, srh)
+
+
+def _remote_seg6_enable(cfg):
+    """ Make the remote accept segment routing headers """
+    for conf in ("all", cfg.remote_ifname):
+        # slashes, so that sysctl doesn't split a dotted ifname
+        key = f"net/ipv6/conf/{conf}/seg6_enabled"
+        old = cmd(f"sysctl -n {key}", host=cfg.remote).stdout.strip()
+        cmd(f"sysctl -qw {key}=1", host=cfg.remote)
+        defer(cmd, f"sysctl -qw {key}={old}", host=cfg.remote)
+
+
+@ksft_variants(_ipv6_exthdr_variants())
+def data_send_ipv6_exthdr(cfg, opt, val):
+    """ Test PSP on IPv6 packets with an extension header """
+    cfg.require_ipver("6")
+    _init_psp_dev(cfg)
+    if opt == socket.IPV6_RTHDR:
+        _remote_seg6_enable(cfg)
+
+    s = _make_psp_conn(cfg, 0, "6")
+    try:
+        # In happy path, PSP traffic arrives at the remote decrypted as TCP
+        remote_udp = _remote_udp_from(cfg, s.getsockname()[0],
+                                      s.getpeername()[0])
+        if not remote_udp:
+            raise KsftSkipEx("can't count UDP on the remote")
+
+        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)
+
+        total = 0
+
+        def send(phase):
+            """ Sent encrypted, counted, and received. Nothing reaches the
+            peer as plain UDP.
+            """
+            nonlocal total
+            tx_pkts = _get_quiesced_stat(cfg, 'tx-packets')
+            udp = remote_udp()
+            total += _send_careful(cfg, s, 20)
+            for _ in range(100):
+                if _get_outq(s) == 0:
+                    break
+                time.sleep(0.5)
+            _check_data_outq(s, 0)
+            ksft_gt(_get_quiesced_stat(cfg, 'tx-packets'), tx_pkts,
+                    comment=phase)
+            ksft_eq(remote_udp(), udp, comment=f"{phase}: UDP at the remote")
+            _check_data_rx(cfg, total)
+
+        send("no option")
+        # UDP + PSP go after the extension header
+        s.setsockopt(socket.IPPROTO_IPV6, opt, val(s.getpeername()[0]))
+        send("with the option")
+        s.setsockopt(socket.IPPROTO_IPV6, opt, b'')
+        send("option removed")
+    finally:
+        _close_psp_conn(cfg, s)
+
+
 def _get_psp_ver_ip_variants():
     for ver in range(4):
         for ipv in ("4", "6"):
@@ -1019,7 +1149,8 @@ def main() -> None:
                                                           cfg.comm_port),
                                                          timeout=1)
 
-                cases = [data_basic_send, data_mss_adjust]
+                cases = [data_basic_send, data_mss_adjust,
+                         data_send_ipv6_exthdr]
 
                 if has_cont:
                     cases += [
-- 
2.53.0-Meta