On 1/31/21 12:55 AM, Alexander Lobakin wrote:
> From: Dongseok Yi <dseok.yi@xxxxxxxxxxx>
> Date: Sat, 30 Jan 2021 08:13:27 +0900
>
> > +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.
As you can see in __udpv4_gso_segment_list_csum, we compare
ports and addrs. We should use *struct ipv6hdr* to compare the values
for IPv6 but I am not sure the struct could be under net/ipv4.
The initial idea was to support both IPv4 and IPv6. Thanks, that's a
good point. But the supporting IPv6 would be a new feature. I want to
fix IPv4 first, so the title is restricted to ipv4.
>
> > }
> >
> > 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
