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-li...@rere.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
