Pablo Neira Ayuso
2019-Jul-03 12:40 UTC
[Bridge] [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
Use br_vlan_enabled() and br_vlan_get_pvid() helpers as Nikolay suggests. Rename NFT_META_BRI_PVID to NFT_META_BRI_IIFPVID before this patch hits the 5.3 release cycle, to leave room for matching for the output bridge port in the future. Reported-by: Nikolay Aleksandrov <nikolay at cumulusnetworks.com> Fixes: da4f10a4265b ("netfilter: nft_meta: add NFT_META_BRI_PVID support") Signed-off-by: Pablo Neira Ayuso <pablo at netfilter.org> --- include/uapi/linux/netfilter/nf_tables.h | 4 ++-- net/netfilter/nft_meta.c | 17 ++++++++++------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h index 8859535031e2..8a1bd0b1ec8c 100644 --- a/include/uapi/linux/netfilter/nf_tables.h +++ b/include/uapi/linux/netfilter/nf_tables.h @@ -795,7 +795,7 @@ enum nft_exthdr_attributes { * @NFT_META_SECPATH: boolean, secpath_exists (!!skb->sp) * @NFT_META_IIFKIND: packet input interface kind name (dev->rtnl_link_ops->kind) * @NFT_META_OIFKIND: packet output interface kind name (dev->rtnl_link_ops->kind) - * @NFT_META_BRI_PVID: packet input bridge port pvid + * @NFT_META_BRI_IIFPVID: packet input bridge port pvid */ enum nft_meta_keys { NFT_META_LEN, @@ -826,7 +826,7 @@ enum nft_meta_keys { NFT_META_SECPATH, NFT_META_IIFKIND, NFT_META_OIFKIND, - NFT_META_BRI_PVID, + NFT_META_BRI_IIFPVID, }; /** diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c index 4f8116de70f8..b8d8adc0852e 100644 --- a/net/netfilter/nft_meta.c +++ b/net/netfilter/nft_meta.c @@ -240,14 +240,17 @@ void nft_meta_get_eval(const struct nft_expr *expr, goto err; strncpy((char *)dest, p->br->dev->name, IFNAMSIZ); return; - case NFT_META_BRI_PVID: + case NFT_META_BRI_IIFPVID: { + u16 p_pvid; + if (in == NULL || (p = br_port_get_rtnl_rcu(in)) == NULL) goto err; - if (br_opt_get(p->br, BROPT_VLAN_ENABLED)) { - nft_reg_store16(dest, br_get_pvid(nbp_vlan_group_rcu(p))); - return; - } - goto err; + if (!br_vlan_enabled(in)) + goto err; + br_vlan_get_pvid(in, &p_pvid); + nft_reg_store16(dest, p_pvid); + return; + } #endif case NFT_META_IIFKIND: if (in == NULL || in->rtnl_link_ops == NULL) @@ -375,7 +378,7 @@ static int nft_meta_get_init(const struct nft_ctx *ctx, return -EOPNOTSUPP; len = IFNAMSIZ; break; - case NFT_META_BRI_PVID: + case NFT_META_BRI_IIFPVID: if (ctx->family != NFPROTO_BRIDGE) return -EOPNOTSUPP; len = sizeof(u16); -- 2.11.0
Nikolay Aleksandrov
2019-Jul-03 12:42 UTC
[Bridge] [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
On 03/07/2019 15:40, Pablo Neira Ayuso wrote:> Use br_vlan_enabled() and br_vlan_get_pvid() helpers as Nikolay > suggests. > > Rename NFT_META_BRI_PVID to NFT_META_BRI_IIFPVID before this patch hits > the 5.3 release cycle, to leave room for matching for the output bridge > port in the future. > > Reported-by: Nikolay Aleksandrov <nikolay at cumulusnetworks.com> > Fixes: da4f10a4265b ("netfilter: nft_meta: add NFT_META_BRI_PVID support") > Signed-off-by: Pablo Neira Ayuso <pablo at netfilter.org> > --- > include/uapi/linux/netfilter/nf_tables.h | 4 ++-- > net/netfilter/nft_meta.c | 17 ++++++++++------- > 2 files changed, 12 insertions(+), 9 deletions(-) >Awesome, thanks! Acked-by: Nikolay Aleksandrov <nikolay at cumulusnetworks.com>
Nikolay Aleksandrov
2019-Jul-03 12:48 UTC
[Bridge] [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
On 03/07/2019 15:40, Pablo Neira Ayuso wrote:> Use br_vlan_enabled() and br_vlan_get_pvid() helpers as Nikolay > suggests. > > Rename NFT_META_BRI_PVID to NFT_META_BRI_IIFPVID before this patch hits > the 5.3 release cycle, to leave room for matching for the output bridge > port in the future. > > Reported-by: Nikolay Aleksandrov <nikolay at cumulusnetworks.com> > Fixes: da4f10a4265b ("netfilter: nft_meta: add NFT_META_BRI_PVID support") > Signed-off-by: Pablo Neira Ayuso <pablo at netfilter.org> > --- > include/uapi/linux/netfilter/nf_tables.h | 4 ++-- > net/netfilter/nft_meta.c | 17 ++++++++++------- > 2 files changed, 12 insertions(+), 9 deletions(-) >Ah actually I might've hurried too much with the suggestion, we don't have a helper to check through a port if vlan filtering has been enabled. The helper assumes that the device is a bridge. So below you'll have to use p->br->dev with br_vlan_enabled(). And in fact the proper way would be to get the "upper" device via netdev_master_upper_dev_get() and then to use that in order to avoid direct dereferencing.> diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h > index 8859535031e2..8a1bd0b1ec8c 100644 > --- a/include/uapi/linux/netfilter/nf_tables.h > +++ b/include/uapi/linux/netfilter/nf_tables.h > @@ -795,7 +795,7 @@ enum nft_exthdr_attributes { > * @NFT_META_SECPATH: boolean, secpath_exists (!!skb->sp) > * @NFT_META_IIFKIND: packet input interface kind name (dev->rtnl_link_ops->kind) > * @NFT_META_OIFKIND: packet output interface kind name (dev->rtnl_link_ops->kind) > - * @NFT_META_BRI_PVID: packet input bridge port pvid > + * @NFT_META_BRI_IIFPVID: packet input bridge port pvid > */ > enum nft_meta_keys { > NFT_META_LEN, > @@ -826,7 +826,7 @@ enum nft_meta_keys { > NFT_META_SECPATH, > NFT_META_IIFKIND, > NFT_META_OIFKIND, > - NFT_META_BRI_PVID, > + NFT_META_BRI_IIFPVID, > }; > > /** > diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c > index 4f8116de70f8..b8d8adc0852e 100644 > --- a/net/netfilter/nft_meta.c > +++ b/net/netfilter/nft_meta.c > @@ -240,14 +240,17 @@ void nft_meta_get_eval(const struct nft_expr *expr, > goto err; > strncpy((char *)dest, p->br->dev->name, IFNAMSIZ); > return; > - case NFT_META_BRI_PVID: > + case NFT_META_BRI_IIFPVID: { > + u16 p_pvid; > + > if (in == NULL || (p = br_port_get_rtnl_rcu(in)) == NULL) > goto err; > - if (br_opt_get(p->br, BROPT_VLAN_ENABLED)) { > - nft_reg_store16(dest, br_get_pvid(nbp_vlan_group_rcu(p))); > - return; > - } > - goto err; > + if (!br_vlan_enabled(in)) > + goto err; > + br_vlan_get_pvid(in, &p_pvid); > + nft_reg_store16(dest, p_pvid); > + return; > + } > #endif > case NFT_META_IIFKIND: > if (in == NULL || in->rtnl_link_ops == NULL) > @@ -375,7 +378,7 @@ static int nft_meta_get_init(const struct nft_ctx *ctx, > return -EOPNOTSUPP; > len = IFNAMSIZ; > break; > - case NFT_META_BRI_PVID: > + case NFT_META_BRI_IIFPVID: > if (ctx->family != NFPROTO_BRIDGE) > return -EOPNOTSUPP; > len = sizeof(u16); >
wenxu
2019-Jul-03 13:53 UTC
[Bridge] [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
? 2019/7/3 20:40, Pablo Neira Ayuso ??:> Use br_vlan_enabled() and br_vlan_get_pvid() helpers as Nikolay > suggests. > > Rename NFT_META_BRI_PVID to NFT_META_BRI_IIFPVID before this patch hits > the 5.3 release cycle, to leave room for matching for the output bridge > port in the future. > > Reported-by: Nikolay Aleksandrov <nikolay at cumulusnetworks.com> > Fixes: da4f10a4265b ("netfilter: nft_meta: add NFT_META_BRI_PVID support") > Signed-off-by: Pablo Neira Ayuso <pablo at netfilter.org> > --- > include/uapi/linux/netfilter/nf_tables.h | 4 ++-- > net/netfilter/nft_meta.c | 17 ++++++++++------- > 2 files changed, 12 insertions(+), 9 deletions(-) > > diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h > index 8859535031e2..8a1bd0b1ec8c 100644 > --- a/include/uapi/linux/netfilter/nf_tables.h > +++ b/include/uapi/linux/netfilter/nf_tables.h > @@ -795,7 +795,7 @@ enum nft_exthdr_attributes { > * @NFT_META_SECPATH: boolean, secpath_exists (!!skb->sp) > * @NFT_META_IIFKIND: packet input interface kind name (dev->rtnl_link_ops->kind) > * @NFT_META_OIFKIND: packet output interface kind name (dev->rtnl_link_ops->kind) > - * @NFT_META_BRI_PVID: packet input bridge port pvid > + * @NFT_META_BRI_IIFPVID: packet input bridge port pvid > */ > enum nft_meta_keys { > NFT_META_LEN, > @@ -826,7 +826,7 @@ enum nft_meta_keys { > NFT_META_SECPATH, > NFT_META_IIFKIND, > NFT_META_OIFKIND, > - NFT_META_BRI_PVID, > + NFT_META_BRI_IIFPVID, > }; > > /** > diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c > index 4f8116de70f8..b8d8adc0852e 100644 > --- a/net/netfilter/nft_meta.c > +++ b/net/netfilter/nft_meta.c > @@ -240,14 +240,17 @@ void nft_meta_get_eval(const struct nft_expr *expr, > goto err; > strncpy((char *)dest, p->br->dev->name, IFNAMSIZ); > return; > - case NFT_META_BRI_PVID: > + case NFT_META_BRI_IIFPVID: { > + u16 p_pvid; > + > if (in == NULL || (p = br_port_get_rtnl_rcu(in)) == NULL) > goto err; > - if (br_opt_get(p->br, BROPT_VLAN_ENABLED)) { > - nft_reg_store16(dest, br_get_pvid(nbp_vlan_group_rcu(p))); > - return; > - } > - goto err; > + if (!br_vlan_enabled(in)) > + goto err; > + br_vlan_get_pvid(in, &p_pvid);I find the function br_vlan_get_pvid is ASSERT_RTNL();. But in this senario is under packet path.> + nft_reg_store16(dest, p_pvid); > + return; > + } > #endif > case NFT_META_IIFKIND: > if (in == NULL || in->rtnl_link_ops == NULL) > @@ -375,7 +378,7 @@ static int nft_meta_get_init(const struct nft_ctx *ctx, > return -EOPNOTSUPP; > len = IFNAMSIZ; > break; > - case NFT_META_BRI_PVID: > + case NFT_META_BRI_IIFPVID: > if (ctx->family != NFPROTO_BRIDGE) > return -EOPNOTSUPP; > len = sizeof(u16);