[PATCH net-next] netfilter: bridge: add nft_bridge_pvid to tag the default pvid for non-tagged packet

[PATCH net-next] netfilter: bridge: add nft_bridge_pvid to tag the default pvid for non-tagged packet

[wenxu]

From: wenxu <wenxu@ucloud.cn>

bridge vlan add dev veth1 vid 200 pvid untagged
bridge vlan add dev veth2 vid 200 pvid untagged

nft add table bridge firewall
nft add chain bridge firewall zones { type filter hook prerouting priority - 300 \; }
nft add rule bridge firewall zones counter ct zone set vlan id map { 100 : 1, 200 : 2 }

As above set the bridge port with pvid, the received packet don't contain
the vlan tag which means the packet should belong to vlan 200 through pvid.
With this pacth user can set the pvid in the prerouting hook before set zone id and
conntrack. So the conntrack can only base on vlan id and map the vlan id to zone id
in the prerouting hook.

Signed-off-by: wenxu <wenxu@ucloud.cn>
---
 net/bridge/netfilter/Kconfig           |  6 ++++
 net/bridge/netfilter/Makefile          |  1 +
 net/bridge/netfilter/nft_bridge_pvid.c | 63 ++++++++++++++++++++++++++++++++++
 3 files changed, 70 insertions(+)
 create mode 100644 net/bridge/netfilter/nft_bridge_pvid.c

diff --git a/net/bridge/netfilter/Kconfig b/net/bridge/netfilter/Kconfig
index f4fb0b9..61f2a31 100644
--- a/net/bridge/netfilter/Kconfig
+++ b/net/bridge/netfilter/Kconfig
@@ -33,6 +33,12 @@ config NF_CONNTRACK_BRIDGE
 
 	  To compile it as a module, choose M here.  If unsure, say N.
 
+config NFT_BRIDGE_PVID
+	tristate "Netfilter nf_tables bridge pvid support"
+	depends on BRIDGE_VLAN_FILTERING
+	help
+	  Add support to add vlan-pvid tag for non-tagged packets.
+
 endif # NF_TABLES_BRIDGE
 
 menuconfig BRIDGE_NF_EBTABLES
diff --git a/net/bridge/netfilter/Makefile b/net/bridge/netfilter/Makefile
index 9d77673..e0d6c59 100644
--- a/net/bridge/netfilter/Makefile
+++ b/net/bridge/netfilter/Makefile
@@ -4,6 +4,7 @@
 #
 
 obj-$(CONFIG_NFT_BRIDGE_REJECT)  += nft_reject_bridge.o
+obj-$(CONFIG_NFT_BRIDGE_PVID)  += nft_bridge_pvid.o
 
 # connection tracking
 obj-$(CONFIG_NF_CONNTRACK_BRIDGE) += nf_conntrack_bridge.o
diff --git a/net/bridge/netfilter/nft_bridge_pvid.c b/net/bridge/netfilter/nft_bridge_pvid.c
new file mode 100644
index 0000000..93a4d38
--- /dev/null
+++ b/net/bridge/netfilter/nft_bridge_pvid.c
@@ -0,0 +1,63 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/netfilter_bridge.h>
+#include <net/netfilter/nf_tables.h>
+#include "../br_private.h"
+
+static void nft_bridge_pvid_eval(const struct nft_expr *expr,
+				 struct nft_regs *regs,
+				 const struct nft_pktinfo *pkt)
+{
+	struct sk_buff *skb = pkt->skb;
+	struct net_bridge_port *p;
+
+	p = br_port_get_rtnl_rcu(skb->dev);
+
+	if (p && br_opt_get(p->br, BROPT_VLAN_ENABLED) &&
+	    !skb_vlan_tag_present(skb)) {
+		u16 pvid = br_get_pvid(nbp_vlan_group_rcu(p));
+
+		if (pvid)
+			__vlan_hwaccel_put_tag(skb, p->br->vlan_proto, pvid);
+	}
+}
+
+static int nft_bridge_pvid_validate(const struct nft_ctx *ctx,
+				    const struct nft_expr *expr,
+				    const struct nft_data **data)
+{
+	return nft_chain_validate_hooks(ctx->chain, 1 << NF_BR_PRE_ROUTING);
+}
+
+static struct nft_expr_type nft_bridge_pvid_type;
+static const struct nft_expr_ops nft_bridge_pvid_ops = {
+	.type		= &nft_bridge_pvid_type,
+	.size		= NFT_EXPR_SIZE(0),
+	.eval		= nft_bridge_pvid_eval,
+	.validate	= nft_bridge_pvid_validate,
+};
+
+static struct nft_expr_type nft_bridge_pvid_type __read_mostly = {
+	.family		= NFPROTO_BRIDGE,
+	.name		= "pvid",
+	.ops		= &nft_bridge_pvid_ops,
+	.owner		= THIS_MODULE,
+};
+
+static int __init nft_bridge_pvid_module_init(void)
+{
+	return nft_register_expr(&nft_bridge_pvid_type);
+}
+
+static void __exit nft_bridge_pvid_module_exit(void)
+{
+	nft_unregister_expr(&nft_bridge_pvid_type);
+}
+
+module_init(nft_bridge_pvid_module_init);
+module_exit(nft_bridge_pvid_module_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_ALIAS_NFT_AF_EXPR(AF_BRIDGE, "pvid");
-- 
1.8.3.1

Re: [PATCH net-next] netfilter: bridge: add nft_bridge_pvid to tag the default pvid for non-tagged packet

[Pablo Neira Ayuso]

On Sat, Jun 15, 2019 at 08:14:21PM +0800, wenxu@ucloud.cn wrote:
[...]
> +static void nft_bridge_pvid_eval(const struct nft_expr *expr,
> +				 struct nft_regs *regs,
> +				 const struct nft_pktinfo *pkt)
> +{
> +	struct sk_buff *skb = pkt->skb;
> +	struct net_bridge_port *p;
> +
> +	p = br_port_get_rtnl_rcu(skb->dev);
> +
> +	if (p && br_opt_get(p->br, BROPT_VLAN_ENABLED) &&
> +	    !skb_vlan_tag_present(skb)) {
> +		u16 pvid = br_get_pvid(nbp_vlan_group_rcu(p));
> +
> +		if (pvid)
> +			__vlan_hwaccel_put_tag(skb, p->br->vlan_proto, pvid);

I see two things here:

#1 Extend new NFT_META_BRIDGE_PVID nft_meta to fetch of 'pvid',
   probably add net/bridge/netfilter/nft_meta_bridge.c for this.

#2 Extend nft_meta to allow to set the vlan tag via
   __vlan_hwaccel_put_tag().

If these two changes are in place, then it should be possible to set
skbuff vlan id based on the pvid, if this is what you need.

This would allow for:

        vlan id set bridge pvid

Re: [PATCH net-next] netfilter: bridge: add nft_bridge_pvid to tag the default pvid for non-tagged packet

[wenxu]

On 6/19/2019 12:40 AM, Pablo Neira Ayuso wrote:
> On Sat, Jun 15, 2019 at 08:14:21PM +0800, wenxu@ucloud.cn wrote:
> [...]
>> +static void nft_bridge_pvid_eval(const struct nft_expr *expr,
>> +				 struct nft_regs *regs,
>> +				 const struct nft_pktinfo *pkt)
>> +{
>> +	struct sk_buff *skb = pkt->skb;
>> +	struct net_bridge_port *p;
>> +
>> +	p = br_port_get_rtnl_rcu(skb->dev);
>> +
>> +	if (p && br_opt_get(p->br, BROPT_VLAN_ENABLED) &&
>> +	    !skb_vlan_tag_present(skb)) {
>> +		u16 pvid = br_get_pvid(nbp_vlan_group_rcu(p));
>> +
>> +		if (pvid)
>> +			__vlan_hwaccel_put_tag(skb, p->br->vlan_proto, pvid);
> I see two things here:
>
> #1 Extend new NFT_META_BRIDGE_PVID nft_meta to fetch of 'pvid',
>    probably add net/bridge/netfilter/nft_meta_bridge.c for this.
I can get this, it provide a bridge pvid (get meta). But why put it in

nft_meta_bridge.c but not nft_meta.c?

>
> #2 Extend nft_meta to allow to set the vlan tag via
>    __vlan_hwaccel_put_tag().

why there is also extend nft_meta?  So it's a set meta. Is "vlan id set"

 not base on nft_payload ?

>
> If these two changes are in place, then it should be possible to set
> skbuff vlan id based on the pvid, if this is what you need.
>
> This would allow for:
>
>         vlan id set bridge pvid
>