[ovs-dev] [PATCH] ovn-controller: Provide the option to set Encap.options:csum

[Numan Siddique]

On Mon, Jan 16, 2017 at 11:46 PM, Russell Bryant <russell at ovn.org> wrote:

>
>
> On Mon, Jan 16, 2017 at 12:01 PM, Ben Pfaff <blp at ovn.org> wrote:
>
>> On Mon, Jan 16, 2017 at 09:28:28AM -0500, Russell Bryant wrote:
>> > On Sat, Jan 14, 2017 at 11:29 AM, Ben Pfaff <blp at ovn.org> wrote:
>> >
>> > > On Sat, Jan 14, 2017 at 07:37:53PM +0530, Numan Siddique wrote:
>> > > > On Sat, Jan 14, 2017 at 3:18 AM, Ben Pfaff <blp at ovn.org> wrote:
>> > > >
>> > > > > On Tue, Jan 10, 2017 at 11:34:42AM +0530, Numan Siddique wrote:
>> > > > > > ovn-controller by default enables UDP checksums for geneve
>> > > > > > tunnels. With this patch user can set the desired value in
>> > > > > > Open_vSwitch.external_ids:ovn_encap_csum.
>> > > > > >
>> > > > > > Signed-off-by: Numan Siddique <nusiddiq at redhat.com>
>> > > > >
>> > > > > I don't see technical problems with this, but I also don't know
>> why a
>> > > > > user would want to disable checksums.  Can you send a v2 that
>> adds this
>> > > > > rationale to the documentation and to the commit message?
>> > > > >
>> > > >
>> > > > Thanks for the review. Sure I will do that. The reason for this
>> patch is
>> > > -
>> > > > we are seeing significant performance increase (more than 100%) in
>> our
>> > > > testing when tunnel checksum is disabled.​
>> > > >
>> > > > ​The lab servers have nics with geneve offload support (
>> > > > tx-udp_tnl-segmentation
>> > > > ​)​
>> > > > .​
>> > >
>> > > OK, that sounds like a good reason to document.
>> >
>> >
>> > In particular, it looks like the NICs we have, Intel X710, will do TCP
>> > Segmentation Offload (TSO) with geneve or vxlan, but only if udp
>> checksums
>> > are turned off.  Once they're on, TCP throughput gets cut to less than
>> half.
>> >
>> > This is going to be painful to document well if it's hardware dependent.
>> > I'm not sure what the better default is, since checksums should actually
>> > improve performance for NICs without geneve offload support.
>>
>> Well, it's at least helpful to document that performance is the reason,
>> and that offloads factor into the issue.  Otherwise readers will have no
>> idea why they'd want to turn off checksums.  At worst, users can
>> benchmark both cases in their environments.
>>
>
> Totally agreed.  I was just adding some more detail about what we've seen,
> and then reflecting on how we might want to document this new option for
> users.
>
>
​I have submitted v2 of the patch with updated documenation -
https://mail.openvswitch.org/pipermail/ovs-dev/2017-January/327660.html

Thanks​
​Numan
​

> --
> Russell Bryant
>