netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jiri Benc <jbenc@redhat.com>
To: Pravin Shelar <pshelar@ovn.org>
Cc: Linux Kernel Network Developers <netdev@vger.kernel.org>,
	Eric Garver <e@erig.me>
Subject: Re: [PATCH net-next v3 0/6] openvswitch: make vlan handling consistent
Date: Mon, 10 Oct 2016 14:46:17 +0200	[thread overview]
Message-ID: <20161010144617.67fd499b@griffin> (raw)
In-Reply-To: <CAOrHB_BTJecjrUsJVzhO4W-edX1V8U7_sn3AHa_Mo+v50n4ycQ@mail.gmail.com>

On Fri, 7 Oct 2016 12:59:08 -0700, Pravin Shelar wrote:
> On Fri, Oct 7, 2016 at 9:07 AM, Jiri Benc <jbenc@redhat.com> wrote:
> > Always keep the first vlan tag "accelerated", i.e. in skb->vlan_tci.
> >
> > Unfortunately, with all the changes since v2, this patchset no longer has
> > the nice deletions > insertions diffstat. I still think it's worth it, as it
> > makes things more consistent overall.
> >
> After looking at the changes, I am not sure about the value. These
> patches are making code bit complicated by processing vlan header
> twice rather than once in current code.

Yes, this is a trade-off. A bit more complexity on packet ingress, less
complexity on packet processing.

My main motivation was L3 packets where the code in packet_length
became more complicated than I'd like to to cover all possible cases.
Normalizing the vlan tags looked as a pretty obvious improvement. But
I'm not that thrilled with what it evolved into. I think it's slightly
better than what we have now but I can understand how you may think
opposite.

I'll rip the fixes (patches 3 and 6) off this patchset and send them
separately.

> As far as patch 6 is concerned I think we could do MTU checks similar
> to the rest of networking stack (for example is_skb_forwardable()).
> That would simplify things here.

Fixing the current code is not that hard. The real problem is the added
complexity with L3 packets. I'll look more into the possible solutions
for the L3 patchset.

 Jiri

      reply	other threads:[~2016-10-10 12:46 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-07 16:07 [PATCH net-next v3 0/6] openvswitch: make vlan handling consistent Jiri Benc
2016-10-07 16:07 ` [PATCH net-next v3 1/6] openvswitch: make skb modifiable in ovs_flow_key_extract* Jiri Benc
2016-10-07 16:07 ` [PATCH net-next v3 2/6] openvswitch: normalize vlan rx path Jiri Benc
2016-10-07 16:07 ` [PATCH net-next v3 3/6] openvswitch: add NETIF_F_HW_VLAN_STAG_TX to internal dev Jiri Benc
2016-10-07 16:07 ` [PATCH net-next v3 4/6] openvswitch: keep vlan tag accelerated on internal device Jiri Benc
2016-10-07 16:07 ` [PATCH net-next v3 5/6] openvswitch: remove unreachable code in vlan parsing Jiri Benc
2016-10-07 16:07 ` [PATCH net-next v3 6/6] openvswitch: fix vlan subtraction from packet length Jiri Benc
2016-10-07 19:59 ` [PATCH net-next v3 0/6] openvswitch: make vlan handling consistent Pravin Shelar
2016-10-10 12:46   ` Jiri Benc [this message]

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=20161010144617.67fd499b@griffin \
    --to=jbenc@redhat.com \
    --cc=e@erig.me \
    --cc=netdev@vger.kernel.org \
    --cc=pshelar@ovn.org \
    /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).