netdev - Re: [PATCH 1/3] vlan: Do not support clearing VLAN_FLAG_REORDER

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 23 May 2011 21:36:11 +0200
From:	Nicolas de Pesloüan 
	<nicolas.2p.debian@...il.com>
To:	Ben Greear <greearb@...delatech.com>
CC:	"Eric W. Biederman" <ebiederm@...ssion.com>,
	David Miller <davem@...emloft.net>,
	Jiri Pirko <jpirko@...hat.com>,
	Changli Gao <xiaosuo@...il.com>, netdev@...r.kernel.org,
	shemminger@...ux-foundation.org, kaber@...sh.net, fubar@...ibm.com,
	eric.dumazet@...il.com, andy@...yhouse.net,
	Jesse Gross <jesse@...ira.com>
Subject: Re: [PATCH 1/3] vlan: Do not support clearing VLAN_FLAG_REORDER_HDR

Le 23/05/2011 18:33, Ben Greear a écrit :
> On 05/23/2011 02:00 AM, Eric W. Biederman wrote:
>> Ben Greear<greearb@...delatech.com> writes:
>>
>>> I believe we have been getting tagged VLAN packets properly
>>> in our test cases. We would not be creating any VLAN devices
>>> in this case, so perhaps the NIC isn't doing any stripping.
>>>
>>> To me, it seems like we should get the fully tagged packet
>>> without having to go muck with aux-data, though it would
>>> be fine if it were *also* in aux-data.
>>
>> Given that pf_packet is a portable interface that works on multiple OS's
>> I tend to agree. Certainly my users would be happier if they don't
>> have to change their code and not having to change tcpdump would
>> also be nice.
>>
>> I'm not certain exactly where in the code it makes sense to put the
>> vlan header back on for pf_packet sockets. The simplest thing would
>> be just before we run the socket filter. If we don't do the simplest
>> thing this raises the question how do we avoid breaking socket filters
>> that look at the packet data and know there is going to be a vlan
>> header there.
>
> That is going to be tricky, since the VLAN header would adjust
> offsets and users could be using some filter that uses offsets
> with no actual mention of VLANs (but expecting it to take
> the VLAN header into account).
>
>> Still the current situation is better than seeing vlan 0 tagged packets
>> twice.
>>
>> My gut feel says if we can cheaply get the socket filters to act like it
>> sees the vlan tag (where the vlan tag belongs) we should not actually
>> put the vlan tag back on until we copy the packet to userspace.
>
> Maybe keep a count of how many sockets with filters and/or pf_packet
> sockets are open, and how many things are registered in
> the 'ptype_all' chain, and only re-add (or never remove) the header if that is > 0?
>
> (And, let the bridging and other kernel logic deal with vlans
> via auxillary methods as well as checking in-line headers.)

Well, this doesn't sound very different from my previous proposal: if a protocol handler is 
registered at parent interface level, can't we simply assume this protocol handler expect the raw 
packet?

Also, I think the main problem is that ptype_all and ptype_base deliveries are different in 
__netif_receive_skb, for no good reason (at least, to my knowledge).

In http://patchwork.ozlabs.org/patch/85578/, I proposed to process protocol handlers that are 
registered on a particular interface inside the loop, then process protocol handlers registered to 
NULL upon exit from the loop.

That way, we would have the opportunity to deliver different skb (tagged/untagged) to different 
level of interface, while we cross the interface stack.

I know there is a possible performance penalty in this implementation, but this can be fixed by 
properly ordering the ptype_all/ptype_base list.

	Nicolas.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html