From: Dongseok Yi <dseok.yi@xxxxxxxxxxx> Date: Sat, 30 Jan 2021 08:13:27 +0900 > UDP/IP header of UDP GROed frag_skbs are not updated even after NAT > forwarding. Only the header of head_skb from ip_finish_output_gso -> > skb_gso_segment is updated but following frag_skbs are not updated. > > A call path skb_mac_gso_segment -> inet_gso_segment -> > udp4_ufo_fragment -> __udp_gso_segment -> __udp_gso_segment_list > does not try to update UDP/IP header of the segment list but copy > only the MAC header. > > Update port, addr and check of each skb of the segment list in > __udp_gso_segment_list. It covers both SNAT and DNAT. > > Fixes: 9fd1ff5d2ac7 (udp: Support UDP fraglist GRO/GSO.) > Signed-off-by: Dongseok Yi <dseok.yi@xxxxxxxxxxx> > Acked-by: Steffen Klassert <steffen.klassert@xxxxxxxxxxx> > --- > v1: > Steffen Klassert said, there could be 2 options. > https://lore.kernel.org/patchwork/patch/1362257/ > I was trying to write a quick fix, but it was not easy to forward > segmented list. Currently, assuming DNAT only. > > v2: > Per Steffen Klassert request, moved the procedure from > udp4_ufo_fragment to __udp_gso_segment_list and support SNAT. > > v3: > Per Steffen Klassert request, applied fast return by comparing seg > and seg->next at the beginning of __udpv4_gso_segment_list_csum. > > Fixed uh->dest = *newport and iph->daddr = *newip to > *oldport = *newport and *oldip = *newip. > > v4: > Clear "Changes Requested" mark in > https://patchwork.kernel.org/project/netdevbpf > > Simplified the return statement in __udp_gso_segment_list. > > include/net/udp.h | 2 +- > net/ipv4/udp_offload.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++---- > net/ipv6/udp_offload.c | 2 +- > 3 files changed, 66 insertions(+), 7 deletions(-) > > diff --git a/include/net/udp.h b/include/net/udp.h > index 877832b..01351ba 100644 > --- a/include/net/udp.h > +++ b/include/net/udp.h > @@ -178,7 +178,7 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb, > int udp_gro_complete(struct sk_buff *skb, int nhoff, udp_lookup_t lookup); > > struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb, > - netdev_features_t features); > + netdev_features_t features, bool is_ipv6); > > static inline struct udphdr *udp_gro_udphdr(struct sk_buff *skb) > { > diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c > index ff39e94..cfc8726 100644 > --- a/net/ipv4/udp_offload.c > +++ b/net/ipv4/udp_offload.c > @@ -187,8 +187,67 @@ struct sk_buff *skb_udp_tunnel_segment(struct sk_buff *skb, > } > EXPORT_SYMBOL(skb_udp_tunnel_segment); > > +static void __udpv4_gso_segment_csum(struct sk_buff *seg, > + __be32 *oldip, __be32 *newip, > + __be16 *oldport, __be16 *newport) > +{ > + struct udphdr *uh; > + struct iphdr *iph; > + > + if (*oldip == *newip && *oldport == *newport) > + return; > + > + uh = udp_hdr(seg); > + iph = ip_hdr(seg); > + > + if (uh->check) { > + inet_proto_csum_replace4(&uh->check, seg, *oldip, *newip, > + true); > + inet_proto_csum_replace2(&uh->check, seg, *oldport, *newport, > + false); > + if (!uh->check) > + uh->check = CSUM_MANGLED_0; > + } > + *oldport = *newport; > + > + csum_replace4(&iph->check, *oldip, *newip); > + *oldip = *newip; > +} > + > +static struct sk_buff *__udpv4_gso_segment_list_csum(struct sk_buff *segs) > +{ > + struct sk_buff *seg; > + struct udphdr *uh, *uh2; > + struct iphdr *iph, *iph2; > + > + seg = segs; > + uh = udp_hdr(seg); > + iph = ip_hdr(seg); > + > + if ((udp_hdr(seg)->dest == udp_hdr(seg->next)->dest) && > + (udp_hdr(seg)->source == udp_hdr(seg->next)->source) && > + (ip_hdr(seg)->daddr == ip_hdr(seg->next)->daddr) && > + (ip_hdr(seg)->saddr == ip_hdr(seg->next)->saddr)) > + return segs; > + > + while ((seg = seg->next)) { > + uh2 = udp_hdr(seg); > + iph2 = ip_hdr(seg); > + > + __udpv4_gso_segment_csum(seg, > + &iph2->saddr, &iph->saddr, > + &uh2->source, &uh->source); > + __udpv4_gso_segment_csum(seg, > + &iph2->daddr, &iph->daddr, > + &uh2->dest, &uh->dest); > + } > + > + return segs; > +} > + > static struct sk_buff *__udp_gso_segment_list(struct sk_buff *skb, > - netdev_features_t features) > + netdev_features_t features, > + bool is_ipv6) > { > unsigned int mss = skb_shinfo(skb)->gso_size; > > @@ -198,11 +257,11 @@ static struct sk_buff *__udp_gso_segment_list(struct sk_buff *skb, > > udp_hdr(skb)->len = htons(sizeof(struct udphdr) + mss); > > - return skb; > + return is_ipv6 ? skb : __udpv4_gso_segment_list_csum(skb); I don't think it's okay to fix checksums only for IPv4. IPv6 checksum mangling doesn't depend on any code from net/ipv6. Just use inet_proto_csum_replace16() for v6 addresses (see nf_nat_proto.c for reference). You can guard the path for IPv6 with IS_ENABLED(CONFIG_IPV6) to optimize IPv4-only systems a bit. > } > > struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb, > - netdev_features_t features) > + netdev_features_t features, bool is_ipv6) > { > struct sock *sk = gso_skb->sk; > unsigned int sum_truesize = 0; > @@ -214,7 +273,7 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb, > __be16 newlen; > > if (skb_shinfo(gso_skb)->gso_type & SKB_GSO_FRAGLIST) > - return __udp_gso_segment_list(gso_skb, features); > + return __udp_gso_segment_list(gso_skb, features, is_ipv6); > > mss = skb_shinfo(gso_skb)->gso_size; > if (gso_skb->len <= sizeof(*uh) + mss) > @@ -328,7 +387,7 @@ static struct sk_buff *udp4_ufo_fragment(struct sk_buff *skb, > goto out; > > if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4) > - return __udp_gso_segment(skb, features); > + return __udp_gso_segment(skb, features, false); > > mss = skb_shinfo(skb)->gso_size; > if (unlikely(skb->len <= mss)) > diff --git a/net/ipv6/udp_offload.c b/net/ipv6/udp_offload.c > index c7bd7b1..faa823c 100644 > --- a/net/ipv6/udp_offload.c > +++ b/net/ipv6/udp_offload.c > @@ -42,7 +42,7 @@ static struct sk_buff *udp6_ufo_fragment(struct sk_buff *skb, > goto out; > > if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4) > - return __udp_gso_segment(skb, features); > + return __udp_gso_segment(skb, features, true); > > mss = skb_shinfo(skb)->gso_size; > if (unlikely(skb->len <= mss)) > -- > 2.7.4 Thanks, Al
