On 1/31/21 12:55 AM, Alexander Lobakin wrote:
> From: Dongseok Yi <dseok...@samsung.com>
> 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


Reply via email to