On 26/04/2016 16:38, "Flavio Leitner" <f...@sysclose.org> wrote:

>On Fri, Apr 15, 2016 at 05:02:45PM -0700, Daniele Di Proietto wrote:
>> The system userspace testsuite uses the userspace datapath with
>> netdev-linux devices, connected to veth pairs with the AF_PACKET socket:
>> 
>>              (veth pair)     (AF_PACKET)
>> TCP stack -> p0 ---> ovs-p0  -------------> netdev-linux (userspace OVS)
>> 
>> Unfortunately this configuration has some problems with offloads: a
>> packet generated by the TCP stack maybe sent to p0 without being
>> checksummed or segmented. The AF_PACKET socket, by default, ignores the
>> offloads and just transmits the data of the packets to userspace, but:
>> 
>> 1. The packet may need GSO, so the data will be too big to be received
>>    by the userspace datapath
>> 2. The packet might have incomplete checksums, so it will likely be
>>    discarded by the receiver.
>> 
>> Problem 1 causes TCP connections to see a congestion window smaller than
>> the MTU, which hurts performance but doesn't prevent communication.
>> 
>> Problem 2 was hidden in the testsuite by a Linux kernel bug, fixed by
>> commit ce8c839b74e3("veth: don’t modify ip_summed; doing so treats
>> packets with bad checksums as good").  In the kernels that include the
>> fix, the userspace datapath is able to process pings, but not tcp or udp
>> data.
>> 
>> Unfortunately I couldn't find a way to ask the AF_PACKET to perform
>> offloads in kernel.  A possible fix would be to use the PACKET_VNET_HDR
>> sockopt and perform the offloads in userspace.
>> 
>> Until a proper fix is worked out for netdev-linux, this commit disables
>> offloads on the non-OVS side of the veth pair, as a workaround.
>> 
>> Signed-off-by: Daniele Di Proietto <diproiet...@vmware.com>
>> ---
>
>I understand why you need this but I don't think it should be part of
>this patchset.
>
>We should start noting somewhere things that we need to revisit in
>the future.
>
>Acked-by: Flavio Leitner <f...@sysclose.org>

You're right, I'll push that separately.  I've only included it here
because the system tests in the last patch need this.

I can add a line in TODO.md for that

Thanks for the review!

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to