Date:	Mon, 29 Sep 2014 22:33:23 +0300
From:	Or Gerlitz <gerlitz.or@...il.com>
To:	Tom Herbert <therbert@...gle.com>
Cc:	Or Gerlitz <or.gerlitz@...il.com>,
	David Miller <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH v3 5/7] net: Add GSO support for UDP tunnels with checksum

On Mon, Sep 29, 2014 at 7:11 AM, Tom Herbert <therbert@...gle.com> wrote:
> On Sat, Sep 27, 2014 at 1:39 PM, Or Gerlitz <or.gerlitz@...il.com> wrote:
>> On Sat, Sep 27, 2014 at 12:57 AM, Tom Herbert <therbert@...gle.com> wrote:
>>> On Fri, Sep 26, 2014 at 2:05 PM, Or Gerlitz <or.gerlitz@...il.com> wrote:
>>>> On Thu, Jun 5, 2014 at 3:20 AM, Tom Herbert <therbert@...gle.com> wrote:
>>
>>>>> Added a new netif feature for GSO_UDP_TUNNEL_CSUM. This indicates
>>>>> that a device is capable of computing the UDP checksum in the
>>>>> encapsulating header of a UDP tunnel.
>>
>>>> Do we have upstream driver that supports GSO_UDP_TUNNEL_CSUM? did you
>>>> had such driver/patch while doing this patches? when a driver
>>>> advertizes that bit, should they look over the xmit path on the new
>>>> encap_hdr_csum bit?
>>
>>> No, no, and encap_hdr_csum should only be set with
>>> SKB_GSO_UDP_TUNNEL_CSUM or SKB_GSO_GRE_CSUM.
>>
>> I'm still trying to dig the bigger picture w.r.t checksum of the outer
>> UDP packet from the patches -- if I got it right, once these patches
>> were picked upstream, there's a scheme where the kernel either
>> computes this checksum or let the device do that - when they advertize
>> NETIF_F_GSO_YYY_CSUM  and in that case
>> (skb->encap_hdr_csum == true) should be interpreted as a directive to
>> do that, right?
>
> NETIF_F_GSO_UDP_CSUM does indicate that device is capable of setting
> outer UDP checksum. gso_type in an skbuff indicates to driver that the
> outer checksum needs to be computed (e.g. SKB_GSO_UDP_TUNNEL_CSUM).
> skb->encap_hdr_csum is only used in software GSO, drivers should not
> care about this.
>
>>
>> So what happens when the device isn't capable to compute that checksum
>> (e.g they don't set _GSO_UDP_TUNNEL_CSUM) but they do advertize the
>> GSO_UDP_TUNNEL bit?
>>
> Then for tunnels configured to use outer checksum software GSO would be used.

I see. Can you point on the code that does this job, i.e goes to SW
GSO unless the driver advertizes both bits

Also, I saw some weird behavior recently (which I haven't fully nailed
down, so didn't rush to post that) when I tested vxlan between
node/NIC that supported offloads  to another node/NIC which doesn't.
That is packets in one direction where flying w. udp csum = 0 and in
the other direction with udp csum != 0, did you run such test? does it
work for you?

>> I was worried that we can run into this scheme - the stack computes
>> the outer checksum for the giant 64K UDP chunck that encapsulate a 64K
>> TCP segment, but when the NIC will issue the segmentation, they will
>> likely to just repeat ~40 times (64K/1500) the original udp checksum
>> for the packets they send, which will be treated as bad checksum on
>> the receiving end, bad.
>>
> This shouldn't happen since the driver would not advertise
> SKB_GSO_UDP_TUNNEL_CSUM.
>
> We can add some comments in skbuff.h to clarify some of the GSO types.

that will be very helpful

>>> find . -name '*.[ch]' -exec fgrep -l GSO_UDP_TUNNEL {} \;
>>> returns
>>> ./ethernet/intel/i40e/i40e_main.c
>>> ./ethernet/broadcom/bnx2x/bnx2x_main.c
>>> ./ethernet/qlogic/qlcnic/qlcnic_main.c
>>> ./ethernet/mellanox/mlx4/en_netdev.c
>>> ./ethernet/emulex/benet/be_main.c

>> cool, I see here four 40Gbs NICs that support GSO offloading of VXLAN
>> traffic, each of them can serve you for testing new developments you
>> do in that area.

any thoughts here?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists