From: "Daniel Zahka" <daniel.zahka@gmail.com>
To: "Todd Freed" <toddfreed@meta.com>, <netdev@vger.kernel.org>
Cc: "Daniel Zahka" <daniel.zahka@gmail.com>,
"Jakub Kicinski" <kuba@kernel.org>,
"Willem de Bruijn" <willemdebruijn.kernel@gmail.com>,
"David S. Miller" <davem@davemloft.net>,
"Eric Dumazet" <edumazet@kernel.org>,
"Paolo Abeni" <pabeni@redhat.com>,
"Simon Horman" <horms@kernel.org>,
"Andrew Lunn" <andrew+netdev@lunn.ch>,
"Shuah Khan" <shuah@kernel.org>, "Raed Salem" <raeds@nvidia.com>,
"Rahul Rameshbabu" <rrameshbabu@nvidia.com>,
"Cosmin Ratiu" <cratiu@nvidia.com>,
linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
"Björn Töpel" <bjorn@kernel.org>
Subject: Re: [PATCH net v2 1/2] psp: fix transport mode encap with ipv6 extension headers
Date: Fri, 23 Oct 2026 07:23:55 -0400 [thread overview]
Message-ID: <DMC6H9OT57RA.30RKA7CO0856R@gmail.com> (raw)
In-Reply-To: <20261022223014.556388-2-toddfreed@meta.com>
On Thu Oct 22, 2026 at 6:30 PM EDT, Todd Freed wrote:
> 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 looks at the extension header
> types only to find their lengths, and to reject these,
>
> - 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_ROUTING. the tcp checksum covers the final destination, but
> drivers seed tso checksums from the daddr in the fixed hdr
> (mlx5e_psp_handle_tx_skb()), so this would produce bad checksums.
> could be supported with more work.
>
The stack should seed the tcp check correctly with the sk dst addr. If
mlx5's lso implementation can't take that into account, I'm not sure a
generic helper should be the one rejecting the skb.
> - NEXTHDR_NONE, and types which ipv6_ext_hdr() doesn't know.
>
> on rx, the inverse process is performed in psp_dev_rcv(). transport mode
> psp fragments are rejected on this path as before.
>
> 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")
> Fixes: 0eddb8023cee ("psp: provide decapsulation and receive helper for drivers")
> Signed-off-by: Todd Freed <toddfreed@meta.com>
> ---
> net/psp/psp_main.c | 78 +++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 70 insertions(+), 8 deletions(-)
>
> diff --git a/net/psp/psp_main.c b/net/psp/psp_main.c
> index c9c1a8826b7f..0c009d7b08c0 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,42 @@ 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 -1 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_ROUTING)
> + return -1;
return an error code instead of -1?
> + rem = len - off;
> + if (rem < sizeof(*eh))
> + return -1;
> +
> + nh = off + offsetof(struct ipv6_opt_hdr, nexthdr);
> + if (type == NEXTHDR_AUTH)
> + off += ipv6_authlen(eh);
I was expecting auth header to be part of the reject list.
> + else
> + off += ipv6_optlen(eh);
> + }
> +
> + return off == len ? nh : -1;
> +}
> +
> static void psp_write_headers(struct net *net, struct sk_buff *skb, __be32 spi,
> u8 ver, unsigned int udp_len, __be16 sport)
> {
> @@ -243,7 +280,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)
> @@ -251,10 +288,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;
> -
> - if (skb->protocol != htons(ETH_P_IP) &&
> - skb->protocol != htons(ETH_P_IPV6))
> + int nh_off = 0;
> +
> + 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;
> @@ -272,7 +316,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);
> }
>
> @@ -331,8 +375,23 @@ int psp_dev_rcv(struct sk_buff *skb, u16 dev_id, u8 generation, bool strip_icv)
> return -EINVAL;
If you send a v3, I think you should put the psp_dev_rcv() in a
serparate patch.
> } else {
> struct ipv6hdr *ipv6h = (struct ipv6hdr *)(skb->data + l2_hlen);
> + u8 nexthdr = ipv6h->nexthdr;
> + __be16 frag_off;
> + int off;
>
> - is_udp = ipv6h->nexthdr == IPPROTO_UDP;
> + /* UDP + PSP come after any extension headers. Fragment and
> + * routing headers are not supported in transport mode.
> + */
> + off = ipv6_skip_exthdr(skb, l2_hlen + sizeof(*ipv6h), &nexthdr,
> + &frag_off);
> + if (unlikely(off < 0 || frag_off))
> + 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;
> }
>
> if (unlikely(!is_udp))
> @@ -386,11 +445,14 @@ int psp_dev_rcv(struct sk_buff *skb, u16 dev_id, u8 generation, bool strip_icv)
> iph->check = ip_fast_csum((u8 *)iph, iph->ihl);
> } else {
> struct ipv6hdr *ipv6h = (struct ipv6hdr *)(skb->data + l2_hlen);
> + int nh_off = psp_ipv6_nexthdr_off((u8 *)ipv6h, l3_hlen);
>
> - if (unlikely(ntohs(ipv6h->payload_len) < encap))
> + if (unlikely(nh_off < 0 ||
> + 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);
> }
>
next prev parent reply other threads:[~2026-10-23 11:23 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-22 22:30 [PATCH net v2 0/2] psp: fix transport mode " Todd Freed
2026-10-22 22:30 ` [PATCH net v2 1/2] psp: fix transport mode encap " Todd Freed
2026-10-23 11:23 ` Daniel Zahka [this message]
2026-10-23 17:20 ` Todd Bert Freed
2026-10-22 22:30 ` [PATCH net v2 2/2] selftests: drv-net: psp: test ipv6 tcp socket with " Todd Freed
2026-10-23 11:44 ` Daniel Zahka
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DMC6H9OT57RA.30RKA7CO0856R@gmail.com \
--to=daniel.zahka@gmail.com \
--cc=andrew+netdev@lunn.ch \
--cc=bjorn@kernel.org \
--cc=cratiu@nvidia.com \
--cc=davem@davemloft.net \
--cc=edumazet@kernel.org \
--cc=horms@kernel.org \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=raeds@nvidia.com \
--cc=rrameshbabu@nvidia.com \
--cc=shuah@kernel.org \
--cc=toddfreed@meta.com \
--cc=willemdebruijn.kernel@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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®