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