netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jiri Benc <jbenc@redhat.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: netdev@vger.kernel.org, Jamal Hadi Salim <jhs@mojatatu.com>,
	Cong Wang <xiyou.wangcong@gmail.com>,
	Jiri Pirko <jiri@resnulli.us>
Subject: Re: [PATCH iproute2 2/2] tc: m_tunnel_key: add csum/nocsum option
Date: Thu, 15 Jun 2017 10:21:10 +0200	[thread overview]
Message-ID: <20170615102110.7e740c3e@griffin> (raw)
In-Reply-To: <20170614133807.2320f98b@xeon-e3>

On Wed, 14 Jun 2017 13:38:07 -0700, Stephen Hemminger wrote:
> Does this change the default? Before your patches what was the checksum
> setting for the new tunnel.

Yes, it does. See the kernel patches.

I realize it's a user visible change; however, given that the real
reason act_tunnel_key was introduced is offloading of openvswitch and
openvswitch doesn't use it yet and I very much doubt there are other
users, I'm still proposing the change. It makes things aligned between
the non-lwt tunnels, tc and even the current openvswitch (which
includes the csum by default as well). When someone someday starts
using lwtunnels with tc for other uses, this will make it much less
surprising and will prevent user errors. Not mentioning performance
which is in general going to be better with the csum.

Furthermore, the only use case for nocsum is a compatibility with
broken VTEPs from hw vendors. This is not a setup act_tunnel_key is
going to be used in: in such case, you pretty much want the VXLAN
internal control plane, not an external one.

We're also not breaking existing Linux setups, even if there were any.
Even with udp6zerocsumrx, the non-zero csumed packets are still
accepted.

I was notified about the current confusing behavior by Red Hat QA when
they were trying to test the tunnel_key action and couldn't make it
work for IPv6. When those guys needed a help of the kernel developer
and the kernel developer in question had to insert a few printks to
realize what's going on, no regular user would be able to use it ;-)
(Of course, that's "fixable" by documentation.)

 Jiri

  reply	other threads:[~2017-06-15  8:21 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-14 19:19 [PATCH net-next 0/2] net: sched: act_tunnel_key: UDP checksums Jiri Benc
2017-06-14 19:19 ` [PATCH net-next 1/2] net: sched: act_tunnel_key: request UDP checksum by default Jiri Benc
2017-06-14 19:19 ` [PATCH net-next 2/2] net: sched: act_tunnel_key: make UDP checksum configurable Jiri Benc
2017-06-14 19:29 ` [PATCH iproute2 1/2] tc: m_tunnel_key: reformat the usage text Jiri Benc
2017-06-14 19:30 ` [PATCH iproute2 2/2] tc: m_tunnel_key: add csum/nocsum option Jiri Benc
2017-06-14 20:38   ` Stephen Hemminger
2017-06-15  8:21     ` Jiri Benc [this message]
2017-06-16 16:12   ` Stephen Hemminger
2017-06-15 18:21 ` [PATCH net-next 0/2] net: sched: act_tunnel_key: UDP checksums David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170615102110.7e740c3e@griffin \
    --to=jbenc@redhat.com \
    --cc=jhs@mojatatu.com \
    --cc=jiri@resnulli.us \
    --cc=netdev@vger.kernel.org \
    --cc=stephen@networkplumber.org \
    --cc=xiyou.wangcong@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).