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

[Russell Bryant]

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.

-- 
Russell Bryant