| Message ID | e6e09a2f745a652382ac7859770ec16c75d8e149.1481586602.git.mirq-linux@rere.qmqm.pl |
|---|---|
| State | Deferred |
| Delegated to: | Pablo Neira |
| Headers | show |
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@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? [...] MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@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? Thanks, fixed. -- Michał Mirosław -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@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 -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 @@ -682,10 +682,8 @@ static int br_nf_push_frag_xmit(struct net *net, struct sock *sk, struct sk_buff return 0; } - if (data->vlan_tci) { - skb->vlan_tci = data->vlan_tci; - skb->vlan_proto = data->vlan_proto; - } + if (data->vlan_proto) + __vlan_hwaccel_put_tag(skb, data->vlan_proto, data->vlan_tci); skb_copy_to_linear_data_offset(skb, -data->size, data->mac, data->size); __skb_push(skb, data->encap_size); @@ -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; + data->encap_size = nf_bridge_encap_header_len(skb); data->size = ETH_HLEN + data->encap_size; diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index 8ce621e..2efbdaf 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -819,7 +819,7 @@ static inline int br_vlan_get_tag(const struct sk_buff *skb, u16 *vid) int err = 0; if (skb_vlan_tag_present(skb)) { - *vid = skb_vlan_tag_get(skb) & VLAN_VID_MASK; + *vid = skb_vlan_tag_get_id(skb); } else { *vid = 0; err = -EINVAL; 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 @@ -377,7 +377,7 @@ struct sk_buff *br_handle_vlan(struct net_bridge *br, } if (v->flags & BRIDGE_VLAN_INFO_UNTAGGED) - skb->vlan_tci = 0; + __vlan_hwaccel_clear_tag(skb); out: return skb; } @@ -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 + * field was 0x000. * We update only VID field and preserve PCP field. */ skb->vlan_tci |= pvid;
This removes assumption than vlan_tci != 0 when tag is present. Signed-off-by: Michał Mirosław <mirq-linux@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(-)