netdev - Re: [PATCH ipsec 4/4] xfrm: Support crypto offload for outbound IPv4 UDP-encapsulated ESP packet

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAHktDpMAms8-Vx=XT-fMJgd1ZXCvvy0wLp0GFw4q3e+ekHtdvQ@mail.gmail.com>
Date: Tue, 9 Jul 2024 14:43:11 +0800
From: Mike Yu <yumike@...gle.com>
To: Steffen Klassert <steffen.klassert@...unet.com>
Cc: netdev@...r.kernel.org, stanleyjhu@...gle.com, martinwu@...gle.com, 
	chiachangwang@...gle.com
Subject: Re: [PATCH ipsec 4/4] xfrm: Support crypto offload for outbound IPv4
 UDP-encapsulated ESP packet

Fixed the comment style and sent v2 patchset.


Mike

Mike


On Mon, Jul 8, 2024 at 7:30 PM Steffen Klassert
<steffen.klassert@...unet.com> wrote:
>
> On Tue, Jul 02, 2024 at 04:44:51PM +0800, Mike Yu wrote:
> > esp_xmit() is already able to handle UDP encapsulation through the call to
> > esp_output_head(). The missing part in esp_xmit() is to correct the outer
> > IP header.
> >
> > Test: Enabled both dir=in/out IPsec crypto offload, and verified IPv4
> >       UDP-encapsulated ESP packets on both wifi/cellular network
> > Signed-off-by: Mike Yu <yumike@...gle.com>
> > ---
> >  net/ipv4/esp4.c         |  7 ++++++-
> >  net/ipv4/esp4_offload.c | 14 +++++++++++++-
> >  2 files changed, 19 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
> > index 3968d3f98e08..cd4b52e131ce 100644
> > --- a/net/ipv4/esp4.c
> > +++ b/net/ipv4/esp4.c
> > @@ -349,6 +349,7 @@ static struct ip_esp_hdr *esp_output_udp_encap(struct sk_buff *skb,
> >  {
> >       struct udphdr *uh;
> >       unsigned int len;
> > +     struct xfrm_offload *xo = xfrm_offload(skb);
> >
> >       len = skb->len + esp->tailen - skb_transport_offset(skb);
> >       if (len + sizeof(struct iphdr) > IP_MAX_MTU)
> > @@ -360,7 +361,11 @@ static struct ip_esp_hdr *esp_output_udp_encap(struct sk_buff *skb,
> >       uh->len = htons(len);
> >       uh->check = 0;
> >
> > -     *skb_mac_header(skb) = IPPROTO_UDP;
> > +     // For IPv4 ESP with UDP encapsulation, if xo is not null, the skb is in the crypto offload
> > +     // data path, which means that esp_output_udp_encap is called outside of the XFRM stack.
> > +     // In this case, the mac header doesn't point to the IPv4 protocol field, so don't set it.
>
> Please use networking style comments.
>
> > +     if (!xo || encap_type != UDP_ENCAP_ESPINUDP)
> > +             *skb_mac_header(skb) = IPPROTO_UDP;
> >
> >       return (struct ip_esp_hdr *)(uh + 1);
> >  }
> > diff --git a/net/ipv4/esp4_offload.c b/net/ipv4/esp4_offload.c
> > index b3271957ad9a..ccfc466ddf6c 100644
> > --- a/net/ipv4/esp4_offload.c
> > +++ b/net/ipv4/esp4_offload.c
> > @@ -264,6 +264,7 @@ static int esp_xmit(struct xfrm_state *x, struct sk_buff *skb,  netdev_features_
> >       struct esp_info esp;
> >       bool hw_offload = true;
> >       __u32 seq;
> > +     int encap_type = 0;
> >
> >       esp.inplace = true;
> >
> > @@ -296,8 +297,10 @@ static int esp_xmit(struct xfrm_state *x, struct sk_buff *skb,  netdev_features_
> >
> >       esp.esph = ip_esp_hdr(skb);
> >
> > +     if (x->encap)
> > +             encap_type = x->encap->encap_type;
> >
> > -     if (!hw_offload || !skb_is_gso(skb)) {
> > +     if (!hw_offload || !skb_is_gso(skb) || (hw_offload && encap_type == UDP_ENCAP_ESPINUDP)) {
> >               esp.nfrags = esp_output_head(x, skb, &esp);
> >               if (esp.nfrags < 0)
> >                       return esp.nfrags;
> > @@ -324,6 +327,15 @@ static int esp_xmit(struct xfrm_state *x, struct sk_buff *skb,  netdev_features_
> >
> >       esp.seqno = cpu_to_be64(seq + ((u64)xo->seq.hi << 32));
> >
> > +     if (hw_offload && encap_type == UDP_ENCAP_ESPINUDP) {
> > +             // In the XFRM stack, the encapsulation protocol is set to iphdr->protocol by
> > +             // setting *skb_mac_header(skb) (see esp_output_udp_encap()) where skb->mac_header
> > +             // points to iphdr->protocol (see xfrm4_tunnel_encap_add()).
> > +             // However, in esp_xmit(), skb->mac_header doesn't point to iphdr->protocol.
> > +             // Therefore, the protocol field needs to be corrected.
>
> Same here.
>