* [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
@ 2019-07-03 12:40 ` Pablo Neira Ayuso
0 siblings, 0 replies; 18+ messages in thread
From: Pablo Neira Ayuso @ 2019-07-03 12:40 UTC (permalink / raw)
To: netfilter-devel; +Cc: wenxu, nikolay, bridge
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@cumulusnetworks.com>
Fixes: da4f10a4265b ("netfilter: nft_meta: add NFT_META_BRI_PVID support")
Signed-off-by: Pablo Neira Ayuso <pablo@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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Bridge] [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
@ 2019-07-03 12:40 ` Pablo Neira Ayuso
0 siblings, 0 replies; 18+ messages in thread
From: Pablo Neira Ayuso @ 2019-07-03 12:40 UTC (permalink / raw)
To: netfilter-devel; +Cc: nikolay, wenxu, bridge
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@cumulusnetworks.com>
Fixes: da4f10a4265b ("netfilter: nft_meta: add NFT_META_BRI_PVID support")
Signed-off-by: Pablo Neira Ayuso <pablo@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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
2019-07-03 12:40 ` [Bridge] " Pablo Neira Ayuso
@ 2019-07-03 12:42 ` Nikolay Aleksandrov
-1 siblings, 0 replies; 18+ messages in thread
From: Nikolay Aleksandrov @ 2019-07-03 12:42 UTC (permalink / raw)
To: Pablo Neira Ayuso, netfilter-devel; +Cc: wenxu, bridge
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@cumulusnetworks.com>
> Fixes: da4f10a4265b ("netfilter: nft_meta: add NFT_META_BRI_PVID support")
> Signed-off-by: Pablo Neira Ayuso <pablo@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@cumulusnetworks.com>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Bridge] [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
@ 2019-07-03 12:42 ` Nikolay Aleksandrov
0 siblings, 0 replies; 18+ messages in thread
From: Nikolay Aleksandrov @ 2019-07-03 12:42 UTC (permalink / raw)
To: Pablo Neira Ayuso, netfilter-devel; +Cc: wenxu, bridge
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@cumulusnetworks.com>
> Fixes: da4f10a4265b ("netfilter: nft_meta: add NFT_META_BRI_PVID support")
> Signed-off-by: Pablo Neira Ayuso <pablo@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@cumulusnetworks.com>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
2019-07-03 12:40 ` [Bridge] " Pablo Neira Ayuso
@ 2019-07-03 12:48 ` Nikolay Aleksandrov
-1 siblings, 0 replies; 18+ messages in thread
From: Nikolay Aleksandrov @ 2019-07-03 12:48 UTC (permalink / raw)
To: Pablo Neira Ayuso, netfilter-devel; +Cc: wenxu, bridge
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@cumulusnetworks.com>
> Fixes: da4f10a4265b ("netfilter: nft_meta: add NFT_META_BRI_PVID support")
> Signed-off-by: Pablo Neira Ayuso <pablo@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);
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Bridge] [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
@ 2019-07-03 12:48 ` Nikolay Aleksandrov
0 siblings, 0 replies; 18+ messages in thread
From: Nikolay Aleksandrov @ 2019-07-03 12:48 UTC (permalink / raw)
To: Pablo Neira Ayuso, netfilter-devel; +Cc: wenxu, bridge
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@cumulusnetworks.com>
> Fixes: da4f10a4265b ("netfilter: nft_meta: add NFT_META_BRI_PVID support")
> Signed-off-by: Pablo Neira Ayuso <pablo@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);
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
2019-07-03 12:48 ` [Bridge] " Nikolay Aleksandrov
@ 2019-07-03 12:53 ` Nikolay Aleksandrov
-1 siblings, 0 replies; 18+ messages in thread
From: Nikolay Aleksandrov @ 2019-07-03 12:53 UTC (permalink / raw)
To: Pablo Neira Ayuso, netfilter-devel; +Cc: wenxu, bridge
On 03/07/2019 15:48, Nikolay Aleksandrov wrote:
> 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@cumulusnetworks.com>
>> Fixes: da4f10a4265b ("netfilter: nft_meta: add NFT_META_BRI_PVID support")
>> Signed-off-by: Pablo Neira Ayuso <pablo@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.
>
Sorry for the multiple replies, I just noticed this is running from fast-path so you'll
have to use the _rcu variant and check for null since they are unlinked before the port
flag is removed in del_nbp() (bridge/bf_if.c).
>> 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);
>>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Bridge] [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
@ 2019-07-03 12:53 ` Nikolay Aleksandrov
0 siblings, 0 replies; 18+ messages in thread
From: Nikolay Aleksandrov @ 2019-07-03 12:53 UTC (permalink / raw)
To: Pablo Neira Ayuso, netfilter-devel; +Cc: wenxu, bridge
On 03/07/2019 15:48, Nikolay Aleksandrov wrote:
> 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@cumulusnetworks.com>
>> Fixes: da4f10a4265b ("netfilter: nft_meta: add NFT_META_BRI_PVID support")
>> Signed-off-by: Pablo Neira Ayuso <pablo@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.
>
Sorry for the multiple replies, I just noticed this is running from fast-path so you'll
have to use the _rcu variant and check for null since they are unlinked before the port
flag is removed in del_nbp() (bridge/bf_if.c).
>> 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);
>>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
2019-07-03 12:53 ` [Bridge] " Nikolay Aleksandrov
@ 2019-07-03 12:58 ` Nikolay Aleksandrov
-1 siblings, 0 replies; 18+ messages in thread
From: Nikolay Aleksandrov @ 2019-07-03 12:58 UTC (permalink / raw)
To: Pablo Neira Ayuso, netfilter-devel; +Cc: wenxu, bridge
On 03/07/2019 15:53, Nikolay Aleksandrov wrote:
> On 03/07/2019 15:48, Nikolay Aleksandrov wrote:
>> 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@cumulusnetworks.com>
>>> Fixes: da4f10a4265b ("netfilter: nft_meta: add NFT_META_BRI_PVID support")
>>> Signed-off-by: Pablo Neira Ayuso <pablo@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.
>>
>
> Sorry for the multiple replies, I just noticed this is running from fast-path so you'll
> have to use the _rcu variant and check for null since they are unlinked before the port
> flag is removed in del_nbp() (bridge/bf_if.c).
>
And then just scratch all of that and use p->br->dev directly, I saw it's already dereferenced before. :)
Apologies for the noise.
>>> 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);
>>>
>>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Bridge] [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
@ 2019-07-03 12:58 ` Nikolay Aleksandrov
0 siblings, 0 replies; 18+ messages in thread
From: Nikolay Aleksandrov @ 2019-07-03 12:58 UTC (permalink / raw)
To: Pablo Neira Ayuso, netfilter-devel; +Cc: wenxu, bridge
On 03/07/2019 15:53, Nikolay Aleksandrov wrote:
> On 03/07/2019 15:48, Nikolay Aleksandrov wrote:
>> 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@cumulusnetworks.com>
>>> Fixes: da4f10a4265b ("netfilter: nft_meta: add NFT_META_BRI_PVID support")
>>> Signed-off-by: Pablo Neira Ayuso <pablo@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.
>>
>
> Sorry for the multiple replies, I just noticed this is running from fast-path so you'll
> have to use the _rcu variant and check for null since they are unlinked before the port
> flag is removed in del_nbp() (bridge/bf_if.c).
>
And then just scratch all of that and use p->br->dev directly, I saw it's already dereferenced before. :)
Apologies for the noise.
>>> 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);
>>>
>>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
2019-07-03 12:40 ` [Bridge] " Pablo Neira Ayuso
@ 2019-07-03 13:53 ` wenxu
-1 siblings, 0 replies; 18+ messages in thread
From: wenxu @ 2019-07-03 13:53 UTC (permalink / raw)
To: Pablo Neira Ayuso, netfilter-devel; +Cc: nikolay, bridge
在 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@cumulusnetworks.com>
> Fixes: da4f10a4265b ("netfilter: nft_meta: add NFT_META_BRI_PVID support")
> Signed-off-by: Pablo Neira Ayuso <pablo@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);
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Bridge] [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
@ 2019-07-03 13:53 ` wenxu
0 siblings, 0 replies; 18+ messages in thread
From: wenxu @ 2019-07-03 13:53 UTC (permalink / raw)
To: Pablo Neira Ayuso, netfilter-devel; +Cc: nikolay, bridge
在 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@cumulusnetworks.com>
> Fixes: da4f10a4265b ("netfilter: nft_meta: add NFT_META_BRI_PVID support")
> Signed-off-by: Pablo Neira Ayuso <pablo@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);
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
2019-07-03 13:53 ` [Bridge] " wenxu
@ 2019-07-03 14:15 ` Pablo Neira Ayuso
-1 siblings, 0 replies; 18+ messages in thread
From: Pablo Neira Ayuso @ 2019-07-03 14:15 UTC (permalink / raw)
To: wenxu; +Cc: netfilter-devel, nikolay, bridge
[-- Attachment #1: Type: text/plain, Size: 650 bytes --]
Hi,
I'm planning to revert from nf-next
da4f10a4265b netfilter: nft_meta: add NFT_META_BRI_PVID support
because:
* Nikolay wants us to use the helpers, however, through the existing
approach this creates a dependency between nft_meta and the bridge
module. I think I suggested this already, but it seems there is a
need for nft_meta_bridge, otherwise nft_meta pulls in the bridge
modules as a dependency.
* NFT_META_BRI_PVID needs to be rename to NFT_META_BRI_IIFPVID.
* We need new helpers to access this information from rcu path, I'm
attaching a patch for such helper for review.
so we take the time to get this right :-)
[-- Attachment #2: 0001-bridge-add-br_vlan_get_pvid_rcu.patch --]
[-- Type: text/x-diff, Size: 2717 bytes --]
From 260cb904228b23d62fddad0e1898c82218a69c57 Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Wed, 3 Jul 2019 16:03:12 +0200
Subject: [PATCH] bridge: add br_vlan_get_pvid_rcu()
This new function allows you to fetch bridge pvid from packet path.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
include/linux/if_bridge.h | 6 ++++++
net/bridge/br_vlan.c | 19 +++++++++++++++----
2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
index f3fab5d0ea97..950db1dad830 100644
--- a/include/linux/if_bridge.h
+++ b/include/linux/if_bridge.h
@@ -88,6 +88,7 @@ static inline bool br_multicast_router(const struct net_device *dev)
#if IS_ENABLED(CONFIG_BRIDGE) && IS_ENABLED(CONFIG_BRIDGE_VLAN_FILTERING)
bool br_vlan_enabled(const struct net_device *dev);
int br_vlan_get_pvid(const struct net_device *dev, u16 *p_pvid);
+int br_vlan_get_pvid_rcu(const struct net_device *dev, u16 *p_pvid);
int br_vlan_get_info(const struct net_device *dev, u16 vid,
struct bridge_vlan_info *p_vinfo);
#else
@@ -101,6 +102,11 @@ static inline int br_vlan_get_pvid(const struct net_device *dev, u16 *p_pvid)
return -EINVAL;
}
+static inline int br_vlan_get_pvid_rcu(const struct net_device *dev, u16 *p_pvid)
+{
+ return -EINVAL;
+}
+
static inline int br_vlan_get_info(const struct net_device *dev, u16 vid,
struct bridge_vlan_info *p_vinfo)
{
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index f47f526b4f19..8d97b91ad503 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -1227,13 +1227,11 @@ void br_vlan_get_stats(const struct net_bridge_vlan *v,
}
}
-int br_vlan_get_pvid(const struct net_device *dev, u16 *p_pvid)
+static int __br_vlan_get_pvid(const struct net_device *dev,
+ struct net_bridge_port *p, u16 *p_pvid)
{
struct net_bridge_vlan_group *vg;
- struct net_bridge_port *p;
- ASSERT_RTNL();
- p = br_port_get_check_rtnl(dev);
if (p)
vg = nbp_vlan_group(p);
else if (netif_is_bridge_master(dev))
@@ -1244,8 +1242,21 @@ int br_vlan_get_pvid(const struct net_device *dev, u16 *p_pvid)
*p_pvid = br_get_pvid(vg);
return 0;
}
+
+int br_vlan_get_pvid(const struct net_device *dev, u16 *p_pvid)
+{
+ ASSERT_RTNL();
+
+ return __br_vlan_get_pvid(dev, br_port_get_check_rtnl(dev), p_pvid);
+}
EXPORT_SYMBOL_GPL(br_vlan_get_pvid);
+int br_vlan_get_pvid_rcu(const struct net_device *dev, u16 *p_pvid)
+{
+ return __br_vlan_get_pvid(dev, br_port_get_check_rcu(dev), p_pvid);
+}
+EXPORT_SYMBOL_GPL(br_vlan_get_pvid_rcu);
+
int br_vlan_get_info(const struct net_device *dev, u16 vid,
struct bridge_vlan_info *p_vinfo)
{
--
2.11.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [Bridge] [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
@ 2019-07-03 14:15 ` Pablo Neira Ayuso
0 siblings, 0 replies; 18+ messages in thread
From: Pablo Neira Ayuso @ 2019-07-03 14:15 UTC (permalink / raw)
To: wenxu; +Cc: nikolay, bridge, netfilter-devel
[-- Attachment #1: Type: text/plain, Size: 650 bytes --]
Hi,
I'm planning to revert from nf-next
da4f10a4265b netfilter: nft_meta: add NFT_META_BRI_PVID support
because:
* Nikolay wants us to use the helpers, however, through the existing
approach this creates a dependency between nft_meta and the bridge
module. I think I suggested this already, but it seems there is a
need for nft_meta_bridge, otherwise nft_meta pulls in the bridge
modules as a dependency.
* NFT_META_BRI_PVID needs to be rename to NFT_META_BRI_IIFPVID.
* We need new helpers to access this information from rcu path, I'm
attaching a patch for such helper for review.
so we take the time to get this right :-)
[-- Attachment #2: 0001-bridge-add-br_vlan_get_pvid_rcu.patch --]
[-- Type: text/x-diff, Size: 2717 bytes --]
From 260cb904228b23d62fddad0e1898c82218a69c57 Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Wed, 3 Jul 2019 16:03:12 +0200
Subject: [PATCH] bridge: add br_vlan_get_pvid_rcu()
This new function allows you to fetch bridge pvid from packet path.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
include/linux/if_bridge.h | 6 ++++++
net/bridge/br_vlan.c | 19 +++++++++++++++----
2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
index f3fab5d0ea97..950db1dad830 100644
--- a/include/linux/if_bridge.h
+++ b/include/linux/if_bridge.h
@@ -88,6 +88,7 @@ static inline bool br_multicast_router(const struct net_device *dev)
#if IS_ENABLED(CONFIG_BRIDGE) && IS_ENABLED(CONFIG_BRIDGE_VLAN_FILTERING)
bool br_vlan_enabled(const struct net_device *dev);
int br_vlan_get_pvid(const struct net_device *dev, u16 *p_pvid);
+int br_vlan_get_pvid_rcu(const struct net_device *dev, u16 *p_pvid);
int br_vlan_get_info(const struct net_device *dev, u16 vid,
struct bridge_vlan_info *p_vinfo);
#else
@@ -101,6 +102,11 @@ static inline int br_vlan_get_pvid(const struct net_device *dev, u16 *p_pvid)
return -EINVAL;
}
+static inline int br_vlan_get_pvid_rcu(const struct net_device *dev, u16 *p_pvid)
+{
+ return -EINVAL;
+}
+
static inline int br_vlan_get_info(const struct net_device *dev, u16 vid,
struct bridge_vlan_info *p_vinfo)
{
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index f47f526b4f19..8d97b91ad503 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -1227,13 +1227,11 @@ void br_vlan_get_stats(const struct net_bridge_vlan *v,
}
}
-int br_vlan_get_pvid(const struct net_device *dev, u16 *p_pvid)
+static int __br_vlan_get_pvid(const struct net_device *dev,
+ struct net_bridge_port *p, u16 *p_pvid)
{
struct net_bridge_vlan_group *vg;
- struct net_bridge_port *p;
- ASSERT_RTNL();
- p = br_port_get_check_rtnl(dev);
if (p)
vg = nbp_vlan_group(p);
else if (netif_is_bridge_master(dev))
@@ -1244,8 +1242,21 @@ int br_vlan_get_pvid(const struct net_device *dev, u16 *p_pvid)
*p_pvid = br_get_pvid(vg);
return 0;
}
+
+int br_vlan_get_pvid(const struct net_device *dev, u16 *p_pvid)
+{
+ ASSERT_RTNL();
+
+ return __br_vlan_get_pvid(dev, br_port_get_check_rtnl(dev), p_pvid);
+}
EXPORT_SYMBOL_GPL(br_vlan_get_pvid);
+int br_vlan_get_pvid_rcu(const struct net_device *dev, u16 *p_pvid)
+{
+ return __br_vlan_get_pvid(dev, br_port_get_check_rcu(dev), p_pvid);
+}
+EXPORT_SYMBOL_GPL(br_vlan_get_pvid_rcu);
+
int br_vlan_get_info(const struct net_device *dev, u16 vid,
struct bridge_vlan_info *p_vinfo)
{
--
2.11.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
2019-07-03 14:15 ` [Bridge] " Pablo Neira Ayuso
@ 2019-07-03 14:52 ` Nikolay Aleksandrov
-1 siblings, 0 replies; 18+ messages in thread
From: Nikolay Aleksandrov @ 2019-07-03 14:52 UTC (permalink / raw)
To: Pablo Neira Ayuso, wenxu; +Cc: netfilter-devel, bridge
On 03/07/2019 17:15, Pablo Neira Ayuso wrote:
> Hi,
>
> I'm planning to revert from nf-next
>
> da4f10a4265b netfilter: nft_meta: add NFT_META_BRI_PVID support
>
> because:
>
> * Nikolay wants us to use the helpers, however, through the existing
> approach this creates a dependency between nft_meta and the bridge
> module. I think I suggested this already, but it seems there is a
> need for nft_meta_bridge, otherwise nft_meta pulls in the bridge
> modules as a dependency.
>
> * NFT_META_BRI_PVID needs to be rename to NFT_META_BRI_IIFPVID.
>
> * We need new helpers to access this information from rcu path, I'm
> attaching a patch for such helper for review.
>
> so we take the time to get this right :-)
>
Hi,
The plan sounds good to me. I also went over the patch and it looks good.
I think it'd be nice if we can get rid of the br_private.h include and
make nft_meta (or meta_bridge) use linux/if_bridge.h instead. Having
a clear distinction between what is supposed to be exported and what
remains internal would be great. I will help out with that.
Thanks,
Nik
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Bridge] [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
@ 2019-07-03 14:52 ` Nikolay Aleksandrov
0 siblings, 0 replies; 18+ messages in thread
From: Nikolay Aleksandrov @ 2019-07-03 14:52 UTC (permalink / raw)
To: Pablo Neira Ayuso, wenxu; +Cc: bridge, netfilter-devel
On 03/07/2019 17:15, Pablo Neira Ayuso wrote:
> Hi,
>
> I'm planning to revert from nf-next
>
> da4f10a4265b netfilter: nft_meta: add NFT_META_BRI_PVID support
>
> because:
>
> * Nikolay wants us to use the helpers, however, through the existing
> approach this creates a dependency between nft_meta and the bridge
> module. I think I suggested this already, but it seems there is a
> need for nft_meta_bridge, otherwise nft_meta pulls in the bridge
> modules as a dependency.
>
> * NFT_META_BRI_PVID needs to be rename to NFT_META_BRI_IIFPVID.
>
> * We need new helpers to access this information from rcu path, I'm
> attaching a patch for such helper for review.
>
> so we take the time to get this right :-)
>
Hi,
The plan sounds good to me. I also went over the patch and it looks good.
I think it'd be nice if we can get rid of the br_private.h include and
make nft_meta (or meta_bridge) use linux/if_bridge.h instead. Having
a clear distinction between what is supposed to be exported and what
remains internal would be great. I will help out with that.
Thanks,
Nik
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
2019-07-03 14:15 ` [Bridge] " Pablo Neira Ayuso
@ 2019-07-03 14:59 ` wenxu
-1 siblings, 0 replies; 18+ messages in thread
From: wenxu @ 2019-07-03 14:59 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, nikolay, bridge
Agree with you, After add the rcu patch I will resent the series fo nft_meta_bridge!.
在 2019/7/3 22:15, Pablo Neira Ayuso 写道:
> Hi,
>
> I'm planning to revert from nf-next
>
> da4f10a4265b netfilter: nft_meta: add NFT_META_BRI_PVID support
>
> because:
>
> * Nikolay wants us to use the helpers, however, through the existing
> approach this creates a dependency between nft_meta and the bridge
> module. I think I suggested this already, but it seems there is a
> need for nft_meta_bridge, otherwise nft_meta pulls in the bridge
> modules as a dependency.
>
> * NFT_META_BRI_PVID needs to be rename to NFT_META_BRI_IIFPVID.
>
> * We need new helpers to access this information from rcu path, I'm
> attaching a patch for such helper for review.
>
> so we take the time to get this right :-)
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Bridge] [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
@ 2019-07-03 14:59 ` wenxu
0 siblings, 0 replies; 18+ messages in thread
From: wenxu @ 2019-07-03 14:59 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: nikolay, bridge, netfilter-devel
Agree with you, After add the rcu patch I will resent the series fo nft_meta_bridge!.
在 2019/7/3 22:15, Pablo Neira Ayuso 写道:
> Hi,
>
> I'm planning to revert from nf-next
>
> da4f10a4265b netfilter: nft_meta: add NFT_META_BRI_PVID support
>
> because:
>
> * Nikolay wants us to use the helpers, however, through the existing
> approach this creates a dependency between nft_meta and the bridge
> module. I think I suggested this already, but it seems there is a
> need for nft_meta_bridge, otherwise nft_meta pulls in the bridge
> modules as a dependency.
>
> * NFT_META_BRI_PVID needs to be rename to NFT_META_BRI_IIFPVID.
>
> * We need new helpers to access this information from rcu path, I'm
> attaching a patch for such helper for review.
>
> so we take the time to get this right :-)
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2019-07-03 15:00 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-03 12:40 [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector Pablo Neira Ayuso
2019-07-03 12:40 ` [Bridge] " Pablo Neira Ayuso
2019-07-03 12:42 ` Nikolay Aleksandrov
2019-07-03 12:42 ` [Bridge] " Nikolay Aleksandrov
2019-07-03 12:48 ` Nikolay Aleksandrov
2019-07-03 12:48 ` [Bridge] " Nikolay Aleksandrov
2019-07-03 12:53 ` Nikolay Aleksandrov
2019-07-03 12:53 ` [Bridge] " Nikolay Aleksandrov
2019-07-03 12:58 ` Nikolay Aleksandrov
2019-07-03 12:58 ` [Bridge] " Nikolay Aleksandrov
2019-07-03 13:53 ` wenxu
2019-07-03 13:53 ` [Bridge] " wenxu
2019-07-03 14:15 ` Pablo Neira Ayuso
2019-07-03 14:15 ` [Bridge] " Pablo Neira Ayuso
2019-07-03 14:52 ` Nikolay Aleksandrov
2019-07-03 14:52 ` [Bridge] " Nikolay Aleksandrov
2019-07-03 14:59 ` wenxu
2019-07-03 14:59 ` [Bridge] " wenxu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.