On Wed, Jun 26, 2019 at 3:17 PM Jason Baron <jba...@akamai.com> wrote: > > > > On 6/14/19 4:53 PM, Jason Baron wrote: > > > > > > On 6/13/19 5:20 PM, Willem de Bruijn wrote: > >>>>> @@ -237,6 +237,7 @@ static inline int find_next_netdev_feature(u64 > >>>>> feature, unsigned long start) > >>>>> NETIF_F_GSO_GRE_CSUM | > >>>>> \ > >>>>> NETIF_F_GSO_IPXIP4 | > >>>>> \ > >>>>> NETIF_F_GSO_IPXIP6 | > >>>>> \ > >>>>> + NETIF_F_GSO_UDP_L4 | > >>>>> \ > >>>>> NETIF_F_GSO_UDP_TUNNEL | > >>>>> \ > >>>>> NETIF_F_GSO_UDP_TUNNEL_CSUM) > >>>> > >>>> Are you adding this to NETIF_F_GSO_ENCAP_ALL? Wouldn't it make more > >>>> sense to add it to NETIF_F_GSO_SOFTWARE? > >>>> > >>> > >>> Yes, I'm adding to NETIF_F_GSO_ENCAP_ALL (not very clear from the > >>> context). I will fix the commit log. > >>> > >>> In: 83aa025 udp: add gso support to virtual devices, the support was > >>> also added to NETIF_F_GSO_ENCAP_ALL (although subsequently reverted due > >>> to UDP GRO not being in place), so I wonder what the reason was for that? > >> > >> That was probably just a bad choice on my part. > >> > >> It worked in practice, but if NETIF_F_GSO_SOFTWARE works the same > >> without unexpected side effects, then I agree that it is the better choice. > >> > >> That choice does appear to change behavior when sending over tunnel > >> devices. Might it send tunneled GSO packets over loopback? > >> > >> > > > > I set up a test case using fou tunneling through a bridge device using > > the udpgso_bench_tx test where packets are not received correctly if > > NETIF_F_GSO_UDP_L4 is added to NETIF_F_GSO_SOFTWARE. If I have it added > > to NETIF_F_GSO_ENCAP_ALL, it does work correctly. So there are more > > fixes required to include it in NETIF_F_GSO_SOFTWARE. > > > > The use-case I have only requires it to be in NETIF_F_GSO_ENCAP_ALL, but > > if it needs to go in NETIF_F_GSO_SOFTWARE, I can look at what's required > > more next week. > > > > Hi, > > I haven't had a chance to investigate what goes wrong with including > NETIF_F_GSO_UDP_L4 in NETIF_F_GSO_SOFTWARE - but I was just wondering if > people are ok with NETIF_F_GSO_UDP_L4 being added to > NETIF_F_GSO_ENCAP_ALL and not NETIF_F_GSO_SOFTWARE (ie the original > patch as posted)? > > As I mentioned that is sufficient for my use-case, and its how Willem > originally proposed this.
Indeed, based on the previous discussion this sounds fine to me.
