netdev - Re: issues with vxlan RX checksum offload under OVS datapath

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 21 Jan 2014 09:37:50 -0800
From:	Jesse Gross <jesse@...ira.com>
To:	Pravin Shelar <pshelar@...ira.com>
Cc:	Or Gerlitz <ogerlitz@...lanox.com>,
	Joseph Gasparakis <joseph.gasparakis@...el.com>,
	netdev <netdev@...r.kernel.org>
Subject: Re: issues with vxlan RX checksum offload under OVS datapath

On Tue, Jan 21, 2014 at 9:30 AM, Pravin Shelar <pshelar@...ira.com> wrote:
> On Sun, Jan 19, 2014 at 2:05 PM, Or Gerlitz <ogerlitz@...lanox.com> wrote:
>> Hi Pravin,
>>
>> While testing the gro udp patches over a setup with openvswitch I noted that
>> the RX checksum offload support introduced by Joseph's commit 0afb166
>> "vxlan: Add capability of Rx checksum offload for inner packet" works fine
>> when you use a setup made of
>>
>> NIC --> IP stack --> vxlan device --> bridge --> tap
>>
>> but not when its
>>
>> NIC --> IP stack --> ovs vxlan port --> OVS DP --> tap
>>
>> I narrowed it down to the fact the when going the OVS pathskb->encapsulation
>> remains true also after the decap is done. Basically, this is the original
>> hunk
>>
>>
>>> @@ -607,7 +607,17 @@ static int vxlan_udp_encap_recv(struct sock *sk,
>>> struct sk_buff *skb)
>>>
>>>         __skb_tunnel_rx(skb, vxlan->dev);
>>>         skb_reset_network_header(skb);
>>> -       skb->ip_summed = CHECKSUM_NONE;
>>> +
>>> +       /* If the NIC driver gave us an encapsulated packet with
>>> +        * CHECKSUM_UNNECESSARY and Rx checksum feature is enabled,
>>> +        * leave the CHECKSUM_UNNECESSARY, the device checksummed it
>>> +        * for us. Otherwise force the upper layers to verify it.
>>> +        */
>>> +       if (skb->ip_summed != CHECKSUM_UNNECESSARY || !skb->encapsulation
>>> ||
>>> +           !(vxlan->dev->features & NETIF_F_RXCSUM))
>>> +               skb->ip_summed = CHECKSUM_NONE;
>>> +
>>> +       skb->encapsulation = 0;
>>
>>
>> which was moved by your commits
>>
>> 5cfccc5 vxlan: Add vxlan recv demux.
>> 7ce0475 vxlan: Restructure vxlan receive.
>>
>> to a code which isn't shared by the vxlan driver and ovs vxlan datapath
>> code, and I was thinking lets just move it back to vxlan_udp_encap_recv.
>> However, we can't really make the "vxlan->dev->features & NETIF_F_RXCSUM"
>> check - since there's no vxlan device for ovs datapath, any idea how to
>> overcome this.
>>
>> Moving this to shared code (while removing the check for
>> vxlan->dev->features) made things to work on my setup, but this misses one
>> of the original conditions, ideas?
>>
> I kept csum check in vxlan-device recv path for same reason. As of now
> there is no efficient way to get ovs-dev features.
> May be we can cache device features in struct datapath from datapath-netdev.\

I don't think that getting the features from the internal device makes
sense - a packet could be destined to a VM instead.

I think that controlling RX checksum on the device that physically
received it should be enough to be able to check for broken devices.
It's not clear what the benefit to turning it off at the VXLAN layer
is - if you really don't want it for some reason, you can always throw
away the checksum later.
--
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