On 6/26/19 4:41 PM, Willem de Bruijn wrote:
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.
Willem
Are you OK to ACK this? If not, is there something else you'd rather see
here?
Thanks
Josh
