Re: [RFC][net-next-2.6 PATCH 2/4] net: 8021Q consolidate header_ops routines

[Jesse Gross <jesse@nicira.com>]

On Thu, Oct 21, 2010 at 3:10 PM, John Fastabend
<john.r.fastabend@intel.com> wrote:
> The only thing the 8021Q header ops routines are required
> for is the VLAN_FLAG_REORDER_HDR otherwise by the time
> the VLAN tag has been added the packet is already on
> its way down the stack. In this case using the Ethernet
> ops works OK.
>
> At present the VLAN_FLAG_REORDER_HDR flag does not work
> with vlan offloads. As I understand the flag the intent
> is to allow taps on the vlan device and possibly the
> QOS layer to see the vlan tag info.
>
> By inserting the tag in vlan_tci any taps or QOS policies
> should be able to retrieve the vlan info. This allows
> the flag to work the same in both the offload case and
> non-offloaded case. And allows us to use the underlying
> ethernet ops.
>
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>

I noticed that you dropped this patch from your most recent series, so
I went back to take a look at it.  I realized that it probably works
inconsistently since header caching doesn't take into account
skb->vlan_tci, so whether you see the tag depends on the state of the
cache.

It would be really good to have this type of code consolidation, both
for the sake of sanity and to eliminate the inconsistent behavior.  We
could do that by either not using header caching or making it work
with vlan offloading somehow.  However, I'm not sure that there's
really much point in that.  VLAN_FLAG_REORDER_HDR doesn't work with
cards that do vlan offloading, which is a pretty significant number of
them.  It similarly works inconsistently on the rx side.  So it's
broken most of the time and worse, the behavior changes depending on
the NIC (and now the ethtool setting).  Can we just eliminate it?