netdev - Re: [PATCH net-next 13/27] bridge: use __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: <ca5bb9f1-16ef-c442-b6d7-eb6ae39dae48@lab.ntt.co.jp>
Date:   Wed, 14 Dec 2016 09:40:57 +0900
From:   Toshiaki Makita <makita.toshiaki@....ntt.co.jp>
To:     Michał Mirosław <mirq-linux@...e.qmqm.pl>,
        Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
Cc:     netdev@...r.kernel.org,
        "Pablo Neira Ayuso (supporter:NETFILTER ({IP ,IP6,ARP,EB,NF}TABLES))" 
        <pablo@...filter.org>,
        "Patrick McHardy (supporter:NETFILTER ({IP,IP6,ARP ,EB,NF}TABLES))" 
        <kaber@...sh.net>,
        "Jozsef Kadlecsik (supporter:NETFILTER ({IP,IP6,ARP,EB,NF}TABLES))" 
        <kadlec@...ckhole.kfki.hu>,
        "Stephen Hemminger (maintainer:ETHERNET BRIDGE)" 
        <stephen@...workplumber.org>,
        "open list:NETFILTER ({IP,IP6,ARP,EB ,NF}TABLES)" 
        <netfilter-devel@...r.kernel.org>,
        "moderated list:ETHERNET BRIDGE" <bridge@...ts.linux-foundation.org>
Subject: Re: [PATCH net-next 13/27] bridge: use __vlan_hwaccel helpers

On 2016/12/14 0:11, Michał Mirosław wrote:
> On Tue, Dec 13, 2016 at 03:59:46PM +0300, Sergei Shtylyov wrote:
>> Hello!
>>
>> On 12/13/2016 3:12 AM, Michał Mirosław wrote:
>>
>>> This removes assumption than vlan_tci != 0 when tag is present.
>>>
>>> Signed-off-by: Michał Mirosław <mirq-linux@...e.qmqm.pl>
>>> ---
>>>  net/bridge/br_netfilter_hooks.c | 14 ++++++++------
>>>  net/bridge/br_private.h         |  2 +-
>>>  net/bridge/br_vlan.c            |  6 +++---
>>>  3 files changed, 12 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
>>> index b12501a..2cc0747 100644
>>> --- a/net/bridge/br_netfilter_hooks.c
>>> +++ b/net/bridge/br_netfilter_hooks.c
>> [...]
>>> @@ -749,8 +747,12 @@ static int br_nf_dev_queue_xmit(struct net *net, struct sock *sk, struct sk_buff
>>>
>>>  		data = this_cpu_ptr(&brnf_frag_data_storage);
>>>
>>> -		data->vlan_tci = skb->vlan_tci;
>>> -		data->vlan_proto = skb->vlan_proto;
>>> +		if (skb_vlan_tag_present(skb)) {
>>> +			data->vlan_tci = skb->vlan_tci;
>>> +			data->vlan_proto = skb->vlan_proto;
>>> +		} else
>>> +			data->vlan_proto = 0;
>>
>>    CodingStyle: should use {} in all branches of *if* if at least one branch
>> has {}.
>>
>> [...]
>>> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
>>> index b6de4f4..ef94664 100644
>>> --- a/net/bridge/br_vlan.c
>>> +++ b/net/bridge/br_vlan.c
>>
>>> @@ -444,8 +444,8 @@ static bool __allowed_ingress(const struct net_bridge *br,
>>>  			__vlan_hwaccel_put_tag(skb, br->vlan_proto, pvid);
>>>  		else
>>>  			/* Priority-tagged Frame.
>>> -			 * At this point, We know that skb->vlan_tci had
>>> -			 * VLAN_TAG_PRESENT bit and its VID field was 0x000.
>>> +			 * At this point, We know that skb->vlan_tci VID
>>
>>    s/We/we/.
>>
>>> +			 * field was 0x000.
>>
>>    Simply 0, maybe?

I originally wrote it like this because we are playing with bit field here.
I meant that all of 12 bits are 0 thus we can safely perform bitwise-OR
to update the VID field.

Thanks,
Toshiaki Makita