netdev - Re: [PATCH] filter: introduce SKF_AD_VLAN

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]

Message-ID: <54F8046D.6070807@plumgrid.com>
Date:	Wed, 04 Mar 2015 23:23:25 -0800
From:	Alexei Starovoitov <ast@...mgrid.com>
To:	Jiri Pirko <jpirko@...hat.com>
CC:	Guy Harris <guy@...m.mit.edu>,
	Michal Sekletar <msekleta@...hat.com>, netdev@...r.kernel.org
Subject: Re: [PATCH] filter: introduce SKF_AD_VLAN_PROTO BPF extension

On 3/4/15 10:50 PM, Jiri Pirko wrote:
> Thu, Mar 05, 2015 at 12:47:06AM CET, ast@...mgrid.com wrote:
>> On 3/4/15 1:14 PM, Guy Harris wrote:
>>>
>>> On Mar 4, 2015, at 1:03 PM, Alexei Starovoitov <ast@...mgrid.com> wrote:
>>>
>>>> the patch is correct and looks clean, but I don't understand
>>>> the motivation for the patch.
>>>> There is already SKF_AD_VLAN_TAG_PRESENT. If it is set then only
>>>> two possible values of vlan_proto are ETH_P_8021Q or ETH_P_8021AD.
>>>> If there another vlan header inside the packet, it's AD.
>>>> So you can do the filtering already without adding new bpf extension...
>>>
>>> I presume he's referring to
>>>
>>> 	https://github.com/the-tcpdump-group/libpcap/issues/397
>>>
>>> or
>>>
>>> 	https://github.com/the-tcpdump-group/libpcap/issues/390
>>
>> ok. context is clear.
>> yet, it still sounds like something to fix inside libpcap.
>
> Libpcap need to somehow let kernel now what vlan proto it should filter on.

I don't think that's what bug report talking about. It's looking to
match on vlan id regardless of vlan_proto. Only vlan_tag_present is
needed to make it work.

> Also, it is not true that another vlan header inside packet is AD. You
> can have packet with just AD header (or 2 AD or 2 Q, etc).

yes. Correct, but then again it doesn't seem to be the goal of the bug 
report.
To make myself clear. I think the patch is ok, I'm only asking for
clear justification based on real use case and not invented one.

--
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