* [Bridge] [PATCH 7/7 nf-next] netfilter:nft_meta: add NFT_META_VLAN support
From: wenxu @ 2019-07-04 7:22 UTC (permalink / raw)
To: pablo, nikolay; +Cc: bridge, netfilter-devel
In-Reply-To: <1562224955-3979-1-git-send-email-wenxu@ucloud.cn>
From: wenxu <wenxu@ucloud.cn>
This patch provide a meta vlan to set the vlan tag of the packet.
for q-in-q outer vlan id 20:
meta vlan set 0x88a8:20
set the default 0x8100 vlan type with vlan id 20
meta vlan set 20
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
include/net/netfilter/nft_meta.h | 5 ++++-
include/uapi/linux/netfilter/nf_tables.h | 4 ++++
net/netfilter/nft_meta.c | 22 ++++++++++++++++++++++
3 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/include/net/netfilter/nft_meta.h b/include/net/netfilter/nft_meta.h
index 5c69e9b..cb0f1e8 100644
--- a/include/net/netfilter/nft_meta.h
+++ b/include/net/netfilter/nft_meta.h
@@ -6,7 +6,10 @@ struct nft_meta {
enum nft_meta_keys key:8;
union {
enum nft_registers dreg:8;
- enum nft_registers sreg:8;
+ struct {
+ enum nft_registers sreg:8;
+ enum nft_registers sreg2:8;
+ };
};
};
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index a0d1dbd..699524a 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -797,6 +797,7 @@ enum nft_exthdr_attributes {
* @NFT_META_OIFKIND: packet output interface kind name (dev->rtnl_link_ops->kind)
* @NFT_META_BRI_IIFPVID: packet input bridge port pvid
* @NFT_META_BRI_IIFVPROTO: packet input bridge vlan proto
+ * @NFT_META_VLAN: packet vlan metadata
*/
enum nft_meta_keys {
NFT_META_LEN,
@@ -829,6 +830,7 @@ enum nft_meta_keys {
NFT_META_OIFKIND,
NFT_META_BRI_IIFPVID,
NFT_META_BRI_IIFVPROTO,
+ NFT_META_VLAN,
};
/**
@@ -895,12 +897,14 @@ enum nft_hash_attributes {
* @NFTA_META_DREG: destination register (NLA_U32)
* @NFTA_META_KEY: meta data item to load (NLA_U32: nft_meta_keys)
* @NFTA_META_SREG: source register (NLA_U32)
+ * @NFTA_META_SREG2: source register (NLA_U32)
*/
enum nft_meta_attributes {
NFTA_META_UNSPEC,
NFTA_META_DREG,
NFTA_META_KEY,
NFTA_META_SREG,
+ NFTA_META_SREG2,
__NFTA_META_MAX
};
#define NFTA_META_MAX (__NFTA_META_MAX - 1)
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index 18a848b..9303de3 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -271,6 +271,17 @@ void nft_meta_set_eval(const struct nft_expr *expr,
skb->secmark = value;
break;
#endif
+ case NFT_META_VLAN: {
+ u32 *sreg2 = ®s->data[meta->sreg2];
+ __be16 vlan_proto;
+ u16 vlan_tci;
+
+ vlan_tci = nft_reg_load16(sreg);
+ vlan_proto = nft_reg_load16(sreg2);
+
+ __vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
+ break;
+ }
default:
WARN_ON(1);
}
@@ -281,6 +292,7 @@ void nft_meta_set_eval(const struct nft_expr *expr,
[NFTA_META_DREG] = { .type = NLA_U32 },
[NFTA_META_KEY] = { .type = NLA_U32 },
[NFTA_META_SREG] = { .type = NLA_U32 },
+ [NFTA_META_SREG2] = { .type = NLA_U32 },
};
EXPORT_SYMBOL_GPL(nft_meta_policy);
@@ -432,6 +444,13 @@ int nft_meta_set_init(const struct nft_ctx *ctx,
case NFT_META_PKTTYPE:
len = sizeof(u8);
break;
+ case NFT_META_VLAN:
+ len = sizeof(u16);
+ priv->sreg2 = nft_parse_register(tb[NFTA_META_SREG2]);
+ err = nft_validate_register_load(priv->sreg2, len);
+ if (err < 0)
+ return err;
+ break;
default:
return -EOPNOTSUPP;
}
@@ -457,6 +476,9 @@ int nft_meta_get_dump(struct sk_buff *skb,
goto nla_put_failure;
if (nft_dump_register(skb, NFTA_META_DREG, priv->dreg))
goto nla_put_failure;
+ if (priv->key == NFT_META_VLAN &&
+ nft_dump_register(skb, NFTA_META_SREG2, priv->sreg2))
+ goto nla_put_failure;
return 0;
nla_put_failure:
--
1.8.3.1
^ permalink raw reply related
* [Bridge] [PATCH 6/7 nf-next] netfilter: nft_meta_bridge: Add NFT_META_BRI_IIFVPROTO support
From: wenxu @ 2019-07-04 7:22 UTC (permalink / raw)
To: pablo, nikolay; +Cc: bridge, netfilter-devel
In-Reply-To: <1562224955-3979-1-git-send-email-wenxu@ucloud.cn>
From: wenxu <wenxu@ucloud.cn>
This patch provide a meta to get the bridge vlan proto
nft add rule bridge firewall zones counter meta br_vlan_proto 0x8100
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
include/uapi/linux/netfilter/nf_tables.h | 2 ++
net/bridge/netfilter/nft_meta_bridge.c | 12 ++++++++++++
2 files changed, 14 insertions(+)
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 8a1bd0b..a0d1dbd 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -796,6 +796,7 @@ enum nft_exthdr_attributes {
* @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_IIFPVID: packet input bridge port pvid
+ * @NFT_META_BRI_IIFVPROTO: packet input bridge vlan proto
*/
enum nft_meta_keys {
NFT_META_LEN,
@@ -827,6 +828,7 @@ enum nft_meta_keys {
NFT_META_IIFKIND,
NFT_META_OIFKIND,
NFT_META_BRI_IIFPVID,
+ NFT_META_BRI_IIFVPROTO,
};
/**
diff --git a/net/bridge/netfilter/nft_meta_bridge.c b/net/bridge/netfilter/nft_meta_bridge.c
index 9487d42..2cd145a 100644
--- a/net/bridge/netfilter/nft_meta_bridge.c
+++ b/net/bridge/netfilter/nft_meta_bridge.c
@@ -49,6 +49,17 @@ static void nft_meta_bridge_get_eval(const struct nft_expr *expr,
nft_reg_store16(dest, p_pvid);
return;
}
+ case NFT_META_BRI_IIFVPROTO: {
+ u16 p_proto;
+
+ br_dev = nft_meta_get_bridge(in);
+ if (!br_dev || !br_vlan_enabled(br_dev))
+ goto err;
+
+ br_vlan_get_proto(in, &p_proto);
+ nft_reg_store16(dest, p_proto);
+ return;
+ }
default:
goto out;
}
@@ -75,6 +86,7 @@ static int nft_meta_bridge_get_init(const struct nft_ctx *ctx,
len = IFNAMSIZ;
break;
case NFT_META_BRI_IIFPVID:
+ case NFT_META_BRI_IIFVPROTO:
len = sizeof(u16);
break;
default:
--
1.8.3.1
^ permalink raw reply related
* [Bridge] [PATCH 5/7 nf-next] bridge: add br_vlan_get_proto()
From: wenxu @ 2019-07-04 7:22 UTC (permalink / raw)
To: pablo, nikolay; +Cc: bridge, netfilter-devel
In-Reply-To: <1562224955-3979-1-git-send-email-wenxu@ucloud.cn>
From: wenxu <wenxu@ucloud.cn>
This new function allows you to fetch bridge vlan proto.
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
include/linux/if_bridge.h | 6 ++++++
net/bridge/br_vlan.c | 10 ++++++++++
2 files changed, 16 insertions(+)
diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
index 950db1d..9e57c44 100644
--- a/include/linux/if_bridge.h
+++ b/include/linux/if_bridge.h
@@ -89,6 +89,7 @@ static inline bool br_multicast_router(const struct net_device *dev)
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_proto(const struct net_device *dev, u16 *p_proto);
int br_vlan_get_info(const struct net_device *dev, u16 vid,
struct bridge_vlan_info *p_vinfo);
#else
@@ -102,6 +103,11 @@ static inline int br_vlan_get_pvid(const struct net_device *dev, u16 *p_pvid)
return -EINVAL;
}
+static inline int br_vlan_get_proto(const struct net_device *dev, u16 *p_proto)
+{
+ return -EINVAL;
+}
+
static inline int br_vlan_get_pvid_rcu(const struct net_device *dev, u16 *p_pvid)
{
return -EINVAL;
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 8d97b91..021cc9f66 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -797,6 +797,16 @@ bool br_vlan_enabled(const struct net_device *dev)
}
EXPORT_SYMBOL_GPL(br_vlan_enabled);
+int br_vlan_get_proto(const struct net_device *dev, u16 *p_proto)
+{
+ struct net_bridge *br = netdev_priv(dev);
+
+ *p_proto = ntohs(br->vlan_proto);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(br_vlan_get_proto);
+
int __br_vlan_set_proto(struct net_bridge *br, __be16 proto)
{
int err = 0;
--
1.8.3.1
^ permalink raw reply related
* [Bridge] [PATCH 4/7 nf-next] netfilter: nft_meta_bridge: add NFT_META_BRI_IIFPVID support
From: wenxu @ 2019-07-04 7:22 UTC (permalink / raw)
To: pablo, nikolay; +Cc: bridge, netfilter-devel
In-Reply-To: <1562224955-3979-1-git-send-email-wenxu@ucloud.cn>
From: wenxu <wenxu@ucloud.cn>
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 get the pvid of bridge ports.
So add the following rule for as the first rule in the chain of zones.
nft add rule bridge firewall zones counter meta vlan set meta briifpvid
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
include/uapi/linux/netfilter/nf_tables.h | 2 ++
net/bridge/netfilter/nft_meta_bridge.c | 15 +++++++++++++++
2 files changed, 17 insertions(+)
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index c6c8ec5..8a1bd0b 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -795,6 +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_IIFPVID: packet input bridge port pvid
*/
enum nft_meta_keys {
NFT_META_LEN,
@@ -825,6 +826,7 @@ enum nft_meta_keys {
NFT_META_SECPATH,
NFT_META_IIFKIND,
NFT_META_OIFKIND,
+ NFT_META_BRI_IIFPVID,
};
/**
diff --git a/net/bridge/netfilter/nft_meta_bridge.c b/net/bridge/netfilter/nft_meta_bridge.c
index 2ea8acb..9487d42 100644
--- a/net/bridge/netfilter/nft_meta_bridge.c
+++ b/net/bridge/netfilter/nft_meta_bridge.c
@@ -7,6 +7,7 @@
#include <linux/netfilter/nf_tables.h>
#include <net/netfilter/nf_tables.h>
#include <net/netfilter/nft_meta.h>
+#include <linux/if_bridge.h>
static const struct net_device *
nft_meta_get_bridge(const struct net_device *dev)
@@ -37,6 +38,17 @@ static void nft_meta_bridge_get_eval(const struct nft_expr *expr,
if (!br_dev)
goto err;
break;
+ case NFT_META_BRI_IIFPVID: {
+ u16 p_pvid;
+
+ br_dev = nft_meta_get_bridge(in);
+ if (!br_dev || !br_vlan_enabled(br_dev))
+ goto err;
+
+ br_vlan_get_pvid_rcu(in, &p_pvid);
+ nft_reg_store16(dest, p_pvid);
+ return;
+ }
default:
goto out;
}
@@ -62,6 +74,9 @@ static int nft_meta_bridge_get_init(const struct nft_ctx *ctx,
case NFT_META_BRI_OIFNAME:
len = IFNAMSIZ;
break;
+ case NFT_META_BRI_IIFPVID:
+ len = sizeof(u16);
+ break;
default:
return nft_meta_get_init(ctx, expr, tb);
}
--
1.8.3.1
^ permalink raw reply related
* [Bridge] [PATCH 3/7 nf-next] bridge: add br_vlan_get_pvid_rcu()
From: wenxu @ 2019-07-04 7:22 UTC (permalink / raw)
To: pablo, nikolay; +Cc: bridge, netfilter-devel
In-Reply-To: <1562224955-3979-1-git-send-email-wenxu@ucloud.cn>
From: wenxu <wenxu@ucloud.cn>
This new function allows you to fetch bridge pvid from packet path.
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
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 f3fab5d..950db1d 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 f47f526..8d97b91 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)
{
--
1.8.3.1
^ permalink raw reply related
* [Bridge] [PATCH 2/7 nf-next] netfilter: nft_meta_bridge: Remove the br_private.h header
From: wenxu @ 2019-07-04 7:22 UTC (permalink / raw)
To: pablo, nikolay; +Cc: bridge, netfilter-devel
In-Reply-To: <1562224955-3979-1-git-send-email-wenxu@ucloud.cn>
From: wenxu <wenxu@ucloud.cn>
Mkae the nft_bridge_meta can't direct access the bridge
internal API.
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
net/bridge/netfilter/nft_meta_bridge.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/net/bridge/netfilter/nft_meta_bridge.c b/net/bridge/netfilter/nft_meta_bridge.c
index dde8651..2ea8acb 100644
--- a/net/bridge/netfilter/nft_meta_bridge.c
+++ b/net/bridge/netfilter/nft_meta_bridge.c
@@ -8,7 +8,14 @@
#include <net/netfilter/nf_tables.h>
#include <net/netfilter/nft_meta.h>
-#include "../br_private.h"
+static const struct net_device *
+nft_meta_get_bridge(const struct net_device *dev)
+{
+ if (dev && netif_is_bridge_port(dev))
+ return netdev_master_upper_dev_get_rcu((struct net_device *)dev);
+
+ return NULL;
+}
static void nft_meta_bridge_get_eval(const struct nft_expr *expr,
struct nft_regs *regs,
@@ -17,22 +24,24 @@ static void nft_meta_bridge_get_eval(const struct nft_expr *expr,
const struct nft_meta *priv = nft_expr_priv(expr);
const struct net_device *in = nft_in(pkt), *out = nft_out(pkt);
u32 *dest = ®s->data[priv->dreg];
- const struct net_bridge_port *p;
+ const struct net_device *br_dev;
switch (priv->key) {
case NFT_META_BRI_IIFNAME:
- if (in == NULL || (p = br_port_get_rcu(in)) == NULL)
+ br_dev = nft_meta_get_bridge(in);
+ if (!br_dev)
goto err;
break;
case NFT_META_BRI_OIFNAME:
- if (out == NULL || (p = br_port_get_rcu(out)) == NULL)
+ br_dev = nft_meta_get_bridge(out);
+ if (!br_dev)
goto err;
break;
default:
goto out;
}
- strncpy((char *)dest, p->br->dev->name, IFNAMSIZ);
+ strncpy((char *)dest, br_dev->name, IFNAMSIZ);
return;
out:
return nft_meta_get_eval(expr, regs, pkt);
--
1.8.3.1
^ permalink raw reply related
* [Bridge] [PATCH 1/7 nf-next] netfilter: separate bridge meta key from nft_meta into meta_bridge
From: wenxu @ 2019-07-04 7:22 UTC (permalink / raw)
To: pablo, nikolay; +Cc: bridge, netfilter-devel
From: wenxu <wenxu@ucloud.cn>
Separate bridge meta key from nft_meta to meta_bridge for other key
support. So there is n dependency between nft_meta and the bridge
module
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
include/net/netfilter/nft_meta.h | 44 ++++++++++++
net/bridge/netfilter/Kconfig | 6 ++
net/bridge/netfilter/Makefile | 1 +
net/bridge/netfilter/nft_meta_bridge.c | 127 +++++++++++++++++++++++++++++++++
net/netfilter/nf_tables_core.c | 1 +
net/netfilter/nft_meta.c | 81 ++++++++-------------
6 files changed, 207 insertions(+), 53 deletions(-)
create mode 100644 include/net/netfilter/nft_meta.h
create mode 100644 net/bridge/netfilter/nft_meta_bridge.c
diff --git a/include/net/netfilter/nft_meta.h b/include/net/netfilter/nft_meta.h
new file mode 100644
index 0000000..5c69e9b
--- /dev/null
+++ b/include/net/netfilter/nft_meta.h
@@ -0,0 +1,44 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _NFT_META_H_
+#define _NFT_META_H_
+
+struct nft_meta {
+ enum nft_meta_keys key:8;
+ union {
+ enum nft_registers dreg:8;
+ enum nft_registers sreg:8;
+ };
+};
+
+extern const struct nla_policy nft_meta_policy[];
+
+int nft_meta_get_init(const struct nft_ctx *ctx,
+ const struct nft_expr *expr,
+ const struct nlattr * const tb[]);
+
+int nft_meta_set_init(const struct nft_ctx *ctx,
+ const struct nft_expr *expr,
+ const struct nlattr * const tb[]);
+
+int nft_meta_get_dump(struct sk_buff *skb,
+ const struct nft_expr *expr);
+
+int nft_meta_set_dump(struct sk_buff *skb,
+ const struct nft_expr *expr);
+
+void nft_meta_get_eval(const struct nft_expr *expr,
+ struct nft_regs *regs,
+ const struct nft_pktinfo *pkt);
+
+void nft_meta_set_eval(const struct nft_expr *expr,
+ struct nft_regs *regs,
+ const struct nft_pktinfo *pkt);
+
+void nft_meta_set_destroy(const struct nft_ctx *ctx,
+ const struct nft_expr *expr);
+
+int nft_meta_set_validate(const struct nft_ctx *ctx,
+ const struct nft_expr *expr,
+ const struct nft_data **data);
+
+#endif
diff --git a/net/bridge/netfilter/Kconfig b/net/bridge/netfilter/Kconfig
index f4fb0b9..fbc7085 100644
--- a/net/bridge/netfilter/Kconfig
+++ b/net/bridge/netfilter/Kconfig
@@ -9,6 +9,12 @@ menuconfig NF_TABLES_BRIDGE
bool "Ethernet Bridge nf_tables support"
if NF_TABLES_BRIDGE
+
+config NFT_BRIDGE_META
+ tristate "Netfilter nf_table bridge meta support"
+ help
+ Add support for bridge dedicated meta key.
+
config NFT_BRIDGE_REJECT
tristate "Netfilter nf_tables bridge reject support"
depends on NFT_REJECT && NFT_REJECT_IPV4 && NFT_REJECT_IPV6
diff --git a/net/bridge/netfilter/Makefile b/net/bridge/netfilter/Makefile
index 9d77673..8e2c575 100644
--- a/net/bridge/netfilter/Makefile
+++ b/net/bridge/netfilter/Makefile
@@ -3,6 +3,7 @@
# Makefile for the netfilter modules for Link Layer filtering on a bridge.
#
+obj-$(CONFIG_NFT_BRIDGE_META) += nft_meta_bridge.o
obj-$(CONFIG_NFT_BRIDGE_REJECT) += nft_reject_bridge.o
# connection tracking
diff --git a/net/bridge/netfilter/nft_meta_bridge.c b/net/bridge/netfilter/nft_meta_bridge.c
new file mode 100644
index 0000000..dde8651
--- /dev/null
+++ b/net/bridge/netfilter/nft_meta_bridge.c
@@ -0,0 +1,127 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/netlink.h>
+#include <linux/netfilter.h>
+#include <linux/netfilter/nf_tables.h>
+#include <net/netfilter/nf_tables.h>
+#include <net/netfilter/nft_meta.h>
+
+#include "../br_private.h"
+
+static void nft_meta_bridge_get_eval(const struct nft_expr *expr,
+ struct nft_regs *regs,
+ const struct nft_pktinfo *pkt)
+{
+ const struct nft_meta *priv = nft_expr_priv(expr);
+ const struct net_device *in = nft_in(pkt), *out = nft_out(pkt);
+ u32 *dest = ®s->data[priv->dreg];
+ const struct net_bridge_port *p;
+
+ switch (priv->key) {
+ case NFT_META_BRI_IIFNAME:
+ if (in == NULL || (p = br_port_get_rcu(in)) == NULL)
+ goto err;
+ break;
+ case NFT_META_BRI_OIFNAME:
+ if (out == NULL || (p = br_port_get_rcu(out)) == NULL)
+ goto err;
+ break;
+ default:
+ goto out;
+ }
+
+ strncpy((char *)dest, p->br->dev->name, IFNAMSIZ);
+ return;
+out:
+ return nft_meta_get_eval(expr, regs, pkt);
+err:
+ regs->verdict.code = NFT_BREAK;
+}
+
+static int nft_meta_bridge_get_init(const struct nft_ctx *ctx,
+ const struct nft_expr *expr,
+ const struct nlattr * const tb[])
+{
+ struct nft_meta *priv = nft_expr_priv(expr);
+ unsigned int len;
+
+ priv->key = ntohl(nla_get_be32(tb[NFTA_META_KEY]));
+ switch (priv->key) {
+ case NFT_META_BRI_IIFNAME:
+ case NFT_META_BRI_OIFNAME:
+ len = IFNAMSIZ;
+ break;
+ default:
+ return nft_meta_get_init(ctx, expr, tb);
+ }
+
+ priv->dreg = nft_parse_register(tb[NFTA_META_DREG]);
+ return nft_validate_register_store(ctx, priv->dreg, NULL,
+ NFT_DATA_VALUE, len);
+}
+
+static struct nft_expr_type nft_meta_bridge_type;
+static const struct nft_expr_ops nft_meta_bridge_get_ops = {
+ .type = &nft_meta_bridge_type,
+ .size = NFT_EXPR_SIZE(sizeof(struct nft_meta)),
+ .eval = nft_meta_bridge_get_eval,
+ .init = nft_meta_bridge_get_init,
+ .dump = nft_meta_get_dump,
+};
+
+static const struct nft_expr_ops nft_meta_bridge_set_ops = {
+ .type = &nft_meta_bridge_type,
+ .size = NFT_EXPR_SIZE(sizeof(struct nft_meta)),
+ .eval = nft_meta_set_eval,
+ .init = nft_meta_set_init,
+ .destroy = nft_meta_set_destroy,
+ .dump = nft_meta_set_dump,
+ .validate = nft_meta_set_validate,
+};
+
+static const struct nft_expr_ops *
+nft_meta_bridge_select_ops(const struct nft_ctx *ctx,
+ const struct nlattr * const tb[])
+{
+ if (tb[NFTA_META_KEY] == NULL)
+ return ERR_PTR(-EINVAL);
+
+ if (tb[NFTA_META_DREG] && tb[NFTA_META_SREG])
+ return ERR_PTR(-EINVAL);
+
+ if (tb[NFTA_META_DREG])
+ return &nft_meta_bridge_get_ops;
+
+ if (tb[NFTA_META_SREG])
+ return &nft_meta_bridge_set_ops;
+
+ return ERR_PTR(-EINVAL);
+}
+
+static struct nft_expr_type nft_meta_bridge_type __read_mostly = {
+ .family = NFPROTO_BRIDGE,
+ .name = "meta",
+ .select_ops = nft_meta_bridge_select_ops,
+ .policy = nft_meta_policy,
+ .maxattr = NFTA_META_MAX,
+ .owner = THIS_MODULE,
+};
+
+static int __init nft_meta_bridge_module_init(void)
+{
+ return nft_register_expr(&nft_meta_bridge_type);
+}
+
+static void __exit nft_meta_bridge_module_exit(void)
+{
+ nft_unregister_expr(&nft_meta_bridge_type);
+}
+
+module_init(nft_meta_bridge_module_init);
+module_exit(nft_meta_bridge_module_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("wenxu <wenxu@ucloud.cn>");
+MODULE_ALIAS_NFT_AF_EXPR(AF_BRIDGE, "meta");
diff --git a/net/netfilter/nf_tables_core.c b/net/netfilter/nf_tables_core.c
index b950cd3..96c74c4 100644
--- a/net/netfilter/nf_tables_core.c
+++ b/net/netfilter/nf_tables_core.c
@@ -19,6 +19,7 @@
#include <net/netfilter/nf_tables_core.h>
#include <net/netfilter/nf_tables.h>
#include <net/netfilter/nf_log.h>
+#include <net/netfilter/nft_meta.h>
static noinline void __nft_trace_packet(struct nft_traceinfo *info,
const struct nft_chain *chain,
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index a54329b863..18a848b 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -21,23 +21,12 @@
#include <net/tcp_states.h> /* for TCP_TIME_WAIT */
#include <net/netfilter/nf_tables.h>
#include <net/netfilter/nf_tables_core.h>
+#include <net/netfilter/nft_meta.h>
#include <uapi/linux/netfilter_bridge.h> /* NF_BR_PRE_ROUTING */
-struct nft_meta {
- enum nft_meta_keys key:8;
- union {
- enum nft_registers dreg:8;
- enum nft_registers sreg:8;
- };
-};
-
static DEFINE_PER_CPU(struct rnd_state, nft_prandom_state);
-#ifdef CONFIG_NF_TABLES_BRIDGE
-#include "../bridge/br_private.h"
-#endif
-
void nft_meta_get_eval(const struct nft_expr *expr,
struct nft_regs *regs,
const struct nft_pktinfo *pkt)
@@ -47,9 +36,6 @@ void nft_meta_get_eval(const struct nft_expr *expr,
const struct net_device *in = nft_in(pkt), *out = nft_out(pkt);
struct sock *sk;
u32 *dest = ®s->data[priv->dreg];
-#ifdef CONFIG_NF_TABLES_BRIDGE
- const struct net_bridge_port *p;
-#endif
switch (priv->key) {
case NFT_META_LEN:
@@ -229,18 +215,6 @@ void nft_meta_get_eval(const struct nft_expr *expr,
nft_reg_store8(dest, secpath_exists(skb));
break;
#endif
-#ifdef CONFIG_NF_TABLES_BRIDGE
- case NFT_META_BRI_IIFNAME:
- if (in == NULL || (p = br_port_get_rcu(in)) == NULL)
- goto err;
- strncpy((char *)dest, p->br->dev->name, IFNAMSIZ);
- return;
- case NFT_META_BRI_OIFNAME:
- if (out == NULL || (p = br_port_get_rcu(out)) == NULL)
- goto err;
- strncpy((char *)dest, p->br->dev->name, IFNAMSIZ);
- return;
-#endif
case NFT_META_IIFKIND:
if (in == NULL || in->rtnl_link_ops == NULL)
goto err;
@@ -260,10 +234,11 @@ void nft_meta_get_eval(const struct nft_expr *expr,
err:
regs->verdict.code = NFT_BREAK;
}
+EXPORT_SYMBOL_GPL(nft_meta_get_eval);
-static void nft_meta_set_eval(const struct nft_expr *expr,
- struct nft_regs *regs,
- const struct nft_pktinfo *pkt)
+void nft_meta_set_eval(const struct nft_expr *expr,
+ struct nft_regs *regs,
+ const struct nft_pktinfo *pkt)
{
const struct nft_meta *meta = nft_expr_priv(expr);
struct sk_buff *skb = pkt->skb;
@@ -300,16 +275,18 @@ static void nft_meta_set_eval(const struct nft_expr *expr,
WARN_ON(1);
}
}
+EXPORT_SYMBOL_GPL(nft_meta_set_eval);
-static const struct nla_policy nft_meta_policy[NFTA_META_MAX + 1] = {
+const struct nla_policy nft_meta_policy[NFTA_META_MAX + 1] = {
[NFTA_META_DREG] = { .type = NLA_U32 },
[NFTA_META_KEY] = { .type = NLA_U32 },
[NFTA_META_SREG] = { .type = NLA_U32 },
};
+EXPORT_SYMBOL_GPL(nft_meta_policy);
-static int nft_meta_get_init(const struct nft_ctx *ctx,
- const struct nft_expr *expr,
- const struct nlattr * const tb[])
+int nft_meta_get_init(const struct nft_ctx *ctx,
+ const struct nft_expr *expr,
+ const struct nlattr * const tb[])
{
struct nft_meta *priv = nft_expr_priv(expr);
unsigned int len;
@@ -360,14 +337,6 @@ static int nft_meta_get_init(const struct nft_ctx *ctx,
len = sizeof(u8);
break;
#endif
-#ifdef CONFIG_NF_TABLES_BRIDGE
- case NFT_META_BRI_IIFNAME:
- case NFT_META_BRI_OIFNAME:
- if (ctx->family != NFPROTO_BRIDGE)
- return -EOPNOTSUPP;
- len = IFNAMSIZ;
- break;
-#endif
default:
return -EOPNOTSUPP;
}
@@ -376,6 +345,7 @@ static int nft_meta_get_init(const struct nft_ctx *ctx,
return nft_validate_register_store(ctx, priv->dreg, NULL,
NFT_DATA_VALUE, len);
}
+EXPORT_SYMBOL_GPL(nft_meta_get_init);
static int nft_meta_get_validate(const struct nft_ctx *ctx,
const struct nft_expr *expr,
@@ -409,9 +379,9 @@ static int nft_meta_get_validate(const struct nft_ctx *ctx,
#endif
}
-static int nft_meta_set_validate(const struct nft_ctx *ctx,
- const struct nft_expr *expr,
- const struct nft_data **data)
+int nft_meta_set_validate(const struct nft_ctx *ctx,
+ const struct nft_expr *expr,
+ const struct nft_data **data)
{
struct nft_meta *priv = nft_expr_priv(expr);
unsigned int hooks;
@@ -437,10 +407,11 @@ static int nft_meta_set_validate(const struct nft_ctx *ctx,
return nft_chain_validate_hooks(ctx->chain, hooks);
}
+EXPORT_SYMBOL_GPL(nft_meta_set_validate);
-static int nft_meta_set_init(const struct nft_ctx *ctx,
- const struct nft_expr *expr,
- const struct nlattr * const tb[])
+int nft_meta_set_init(const struct nft_ctx *ctx,
+ const struct nft_expr *expr,
+ const struct nlattr * const tb[])
{
struct nft_meta *priv = nft_expr_priv(expr);
unsigned int len;
@@ -475,9 +446,10 @@ static int nft_meta_set_init(const struct nft_ctx *ctx,
return 0;
}
+EXPORT_SYMBOL_GPL(nft_meta_set_init);
-static int nft_meta_get_dump(struct sk_buff *skb,
- const struct nft_expr *expr)
+int nft_meta_get_dump(struct sk_buff *skb,
+ const struct nft_expr *expr)
{
const struct nft_meta *priv = nft_expr_priv(expr);
@@ -490,8 +462,9 @@ static int nft_meta_get_dump(struct sk_buff *skb,
nla_put_failure:
return -1;
}
+EXPORT_SYMBOL_GPL(nft_meta_get_dump);
-static int nft_meta_set_dump(struct sk_buff *skb, const struct nft_expr *expr)
+int nft_meta_set_dump(struct sk_buff *skb, const struct nft_expr *expr)
{
const struct nft_meta *priv = nft_expr_priv(expr);
@@ -505,15 +478,17 @@ static int nft_meta_set_dump(struct sk_buff *skb, const struct nft_expr *expr)
nla_put_failure:
return -1;
}
+EXPORT_SYMBOL_GPL(nft_meta_set_dump);
-static void nft_meta_set_destroy(const struct nft_ctx *ctx,
- const struct nft_expr *expr)
+void nft_meta_set_destroy(const struct nft_ctx *ctx,
+ const struct nft_expr *expr)
{
const struct nft_meta *priv = nft_expr_priv(expr);
if (priv->key == NFT_META_NFTRACE)
static_branch_dec(&nft_trace_enabled);
}
+EXPORT_SYMBOL_GPL(nft_meta_set_destroy);
static const struct nft_expr_ops nft_meta_get_ops = {
.type = &nft_meta_type,
--
1.8.3.1
^ permalink raw reply related
* Re: [Bridge] [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
From: wenxu @ 2019-07-03 14:59 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: nikolay, bridge, netfilter-devel
In-Reply-To: <20190703141507.mnhzqapu4iaan5d7@salvia>
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
* Re: [Bridge] [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
From: Nikolay Aleksandrov @ 2019-07-03 14:52 UTC (permalink / raw)
To: Pablo Neira Ayuso, wenxu; +Cc: bridge, netfilter-devel
In-Reply-To: <20190703141507.mnhzqapu4iaan5d7@salvia>
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
* Re: [Bridge] [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
From: Pablo Neira Ayuso @ 2019-07-03 14:15 UTC (permalink / raw)
To: wenxu; +Cc: nikolay, bridge, netfilter-devel
In-Reply-To: <ecb6d9e8-7923-07ba-8940-c69fc251f4c3@ucloud.cn>
[-- 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
* Re: [Bridge] [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
From: wenxu @ 2019-07-03 13:53 UTC (permalink / raw)
To: Pablo Neira Ayuso, netfilter-devel; +Cc: nikolay, bridge
In-Reply-To: <20190703124040.19279-1-pablo@netfilter.org>
在 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
* Re: [Bridge] [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
From: Nikolay Aleksandrov @ 2019-07-03 12:58 UTC (permalink / raw)
To: Pablo Neira Ayuso, netfilter-devel; +Cc: wenxu, bridge
In-Reply-To: <36eb3c84-0874-ff18-dfb8-02a907aaccdb@cumulusnetworks.com>
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
* Re: [Bridge] [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
From: Nikolay Aleksandrov @ 2019-07-03 12:53 UTC (permalink / raw)
To: Pablo Neira Ayuso, netfilter-devel; +Cc: wenxu, bridge
In-Reply-To: <a5c31aff-669f-072e-b0f9-499edf6361a8@cumulusnetworks.com>
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
* Re: [Bridge] [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
From: Nikolay Aleksandrov @ 2019-07-03 12:48 UTC (permalink / raw)
To: Pablo Neira Ayuso, netfilter-devel; +Cc: wenxu, bridge
In-Reply-To: <20190703124040.19279-1-pablo@netfilter.org>
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
* Re: [Bridge] [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
From: Nikolay Aleksandrov @ 2019-07-03 12:42 UTC (permalink / raw)
To: Pablo Neira Ayuso, netfilter-devel; +Cc: wenxu, bridge
In-Reply-To: <20190703124040.19279-1-pablo@netfilter.org>
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
* [Bridge] [PATCH] netfilter: nft_meta: fix bridge port vlan ID selector
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
* Re: [Bridge] [RFC net-next] net: dsa: add support for MC_DISABLED attribute
From: Linus Lüssing @ 2019-07-02 23:13 UTC (permalink / raw)
To: Ido Schimmel
Cc: Florian Fainelli, Jiri Pirko, b.a.t.m.a.n, nikolay,
netdev@vger.kernel.org, bridge, Russell King - ARM Linux admin,
Vivien Didelot, Ido Schimmel, davem@davemloft.net
In-Reply-To: <20190702171158.GA7182@splinter>
Hi Ido,
> Do you differentiate between IPv4 and IPv6 in batman-adv?
For most things, yes: The querier state is kept separately for
IPv4 and IPv6. And we do have something like a "router node"
flag to signalize that a node needs all multicast traffic, which
is split into IPv4 and IPv6.
The "MDB" equivalent in batman-adv (multicast entries in our "TT",
translation table) are on MAC address base right now, not on an IP
address base yet, so that sounds similar to what you do in mlxsw?
Regarding querier state, we periodically ask the
bridge via "br_multicast_has_querier_anywhere(dev, ETH_P_IP)"
and "br_multicast_has_querier_anywhere(dev, ETH_P_IPV6)".
(Something more event based with handler functions would probably
be nicer, but this was the easier thing to start with.)
> 1. All the IPv6 MDB entries need to be removed from the device. At least
> in mlxsw, we do not have a way to ignore only IPv6 entries. From the
> device's perspective, an MDB entry is just a multicast DMAC with a
> bitmap of ports packets should be replicated to.
Ah, I see, yes. We had a similar "issue". Initially we just always
added any multicast entry into our translation table offered by
the IP stack or bridge, no matter what a querier state or "router
node" state said. Which would have led to a node receiving two
copies of a multicast packet if it were both a querier or router
and were also having a listener announced via IGMP/MLD.
So there we also just recently changed that, to filter out
IPv6 multicast TT entries if this node were also announcing itself as
an MLD querier or IPv6 router. And same, independently for
IPv4/IGMP.
> 2. We need to split the flood tables used for IPv4 and IPv6 unregistered
> multicast packets. For IPv4, packets should only be flooded to mrouter
> ports whereas for IPv6 packets should be flooded to all the member
> ports.
This one I do not fully understand yet. Why don't you apply the
"flood to all ports" (in the no IGMP querier present case)
for IPv4, too?
Sure, for IPv4 nothing "essential" will break, as IPv4 unicast
does not rely on multicast (contrary to IPv6, due to NDP, as you
mentioned). But still there would be potential multicast packet loss
for a 239.x.x.x listener on the local link, for instance, wouldn't
there?
If I'm not mistaken, we do not apply differing behaviour for IPv4
vs. IPv6 in the bridge either and would flood on all ports for IPv4
with no querier present, too.
Regards, Linus
^ permalink raw reply
* Re: [Bridge] Validation of forward_delay seems wrong...
From: Andrew Lunn @ 2019-07-02 21:35 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: netdev, Roopa Prabhu, bridge
In-Reply-To: <55f24bfb-4239-dda8-24f8-26b6b2fa9f9e@cumulusnetworks.com>
> Hi Andrew,
> The man page is wrong, these have been in USER_HZ scaled clock_t format from the beginning.
> TBH a lot of the time/delay bridge config options are messed up like that.
Hi Nikola
Yes, that is a mess.
arch/alpha/include/asm/param.h:# define USER_HZ 1024
arch/ia64/include/asm/param.h:# define USER_HZ HZ
include/asm-generic/param.h:# define USER_HZ 100
And ia64 does
# define HZ CONFIG_HZ
So it seems pretty hard for user space to get this right in a generic
fashion.
Andrew
^ permalink raw reply
* Re: [Bridge] Validation of forward_delay seems wrong...
From: Nikolay Aleksandrov @ 2019-07-02 21:21 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, Roopa Prabhu, bridge
In-Reply-To: <55f24bfb-4239-dda8-24f8-26b6b2fa9f9e@cumulusnetworks.com>
On 03/07/2019 00:19, Nikolay Aleksandrov wrote:
> On 02/07/2019 23:47, Andrew Lunn wrote:
>> Hi Nikolay
>>
>> The man page says that the bridge forward_delay is in units of
>> seconds, and should be between 2 and 30.
>>
>> I've tested on a couple of different kernel versions, and this appears
>> to be not working correctly:
>>
>> ip link set br0 type bridge forward_delay 2
>> RTNETLINK answers: Numerical result out of range
>>
>> ip link set br0 type bridge forward_delay 199
>> RTNETLINK answers: Numerical result out of range
>>
>> ip link set br0 type bridge forward_delay 200
>> #
>>
>> ip link set br0 type bridge forward_delay 3000
>> #
>>
>> ip link set br0 type bridge forward_delay 3001
>> RTNETLINK answers: Numerical result out of range
>>
>> I've not checked what delay is actually being used here, but clearly
>> something is mixed up.
>>
>> grep HZ .config
>> CONFIG_HZ_PERIODIC=y
>> # CONFIG_NO_HZ_IDLE is not set
>> # CONFIG_NO_HZ_FULL is not set
>> # CONFIG_NO_HZ is not set
>> CONFIG_HZ_FIXED=0
>> CONFIG_HZ_100=y
>> # CONFIG_HZ_200 is not set
>> # CONFIG_HZ_250 is not set
>> # CONFIG_HZ_300 is not set
>> # CONFIG_HZ_500 is not set
>> # CONFIG_HZ_1000 is not set
>> CONFIG_HZ=100
>>
>> Thanks
>> Andrew
>>
>
> Hi Andrew,
> The man page is wrong, these have been in USER_HZ scaled clock_t format from the beginning.
> TBH a lot of the time/delay bridge config options are messed up like that.
> We've been discussing adding special _ms versions in iproute2 to make them
> more user-friendly and understandable. Will cook a patch for the man page.
>
> Cheers,
> Nik
>
>
Err, I meant it is seconds just scaled, if it wasn't clear.
^ permalink raw reply
* Re: [Bridge] Validation of forward_delay seems wrong...
From: Nikolay Aleksandrov @ 2019-07-02 21:19 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, Roopa Prabhu, bridge
In-Reply-To: <20190702204705.GC28471@lunn.ch>
On 02/07/2019 23:47, Andrew Lunn wrote:
> Hi Nikolay
>
> The man page says that the bridge forward_delay is in units of
> seconds, and should be between 2 and 30.
>
> I've tested on a couple of different kernel versions, and this appears
> to be not working correctly:
>
> ip link set br0 type bridge forward_delay 2
> RTNETLINK answers: Numerical result out of range
>
> ip link set br0 type bridge forward_delay 199
> RTNETLINK answers: Numerical result out of range
>
> ip link set br0 type bridge forward_delay 200
> #
>
> ip link set br0 type bridge forward_delay 3000
> #
>
> ip link set br0 type bridge forward_delay 3001
> RTNETLINK answers: Numerical result out of range
>
> I've not checked what delay is actually being used here, but clearly
> something is mixed up.
>
> grep HZ .config
> CONFIG_HZ_PERIODIC=y
> # CONFIG_NO_HZ_IDLE is not set
> # CONFIG_NO_HZ_FULL is not set
> # CONFIG_NO_HZ is not set
> CONFIG_HZ_FIXED=0
> CONFIG_HZ_100=y
> # CONFIG_HZ_200 is not set
> # CONFIG_HZ_250 is not set
> # CONFIG_HZ_300 is not set
> # CONFIG_HZ_500 is not set
> # CONFIG_HZ_1000 is not set
> CONFIG_HZ=100
>
> Thanks
> Andrew
>
Hi Andrew,
The man page is wrong, these have been in USER_HZ scaled clock_t format from the beginning.
TBH a lot of the time/delay bridge config options are messed up like that.
We've been discussing adding special _ms versions in iproute2 to make them
more user-friendly and understandable. Will cook a patch for the man page.
Cheers,
Nik
^ permalink raw reply
* [Bridge] Validation of forward_delay seems wrong...
From: Andrew Lunn @ 2019-07-02 20:47 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: netdev, Roopa Prabhu, bridge
Hi Nikolay
The man page says that the bridge forward_delay is in units of
seconds, and should be between 2 and 30.
I've tested on a couple of different kernel versions, and this appears
to be not working correctly:
ip link set br0 type bridge forward_delay 2
RTNETLINK answers: Numerical result out of range
ip link set br0 type bridge forward_delay 199
RTNETLINK answers: Numerical result out of range
ip link set br0 type bridge forward_delay 200
#
ip link set br0 type bridge forward_delay 3000
#
ip link set br0 type bridge forward_delay 3001
RTNETLINK answers: Numerical result out of range
I've not checked what delay is actually being used here, but clearly
something is mixed up.
grep HZ .config
CONFIG_HZ_PERIODIC=y
# CONFIG_NO_HZ_IDLE is not set
# CONFIG_NO_HZ_FULL is not set
# CONFIG_NO_HZ is not set
CONFIG_HZ_FIXED=0
CONFIG_HZ_100=y
# CONFIG_HZ_200 is not set
# CONFIG_HZ_250 is not set
# CONFIG_HZ_300 is not set
# CONFIG_HZ_500 is not set
# CONFIG_HZ_1000 is not set
CONFIG_HZ=100
Thanks
Andrew
^ permalink raw reply
* Re: [Bridge] [PATCH net 0/4] net: bridge: fix possible stale skb pointers
From: David Miller @ 2019-07-02 18:54 UTC (permalink / raw)
To: nikolay; +Cc: martin, netdev, roopa, bridge, yoshfuji
In-Reply-To: <20190702120021.13096-1-nikolay@cumulusnetworks.com>
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Date: Tue, 2 Jul 2019 15:00:17 +0300
> In the bridge driver we have a couple of places which call pskb_may_pull
> but we've cached skb pointers before that and use them after which can
> lead to out-of-bounds/stale pointer use. I've had these in my "to fix"
> list for some time and now we got a report (patch 01) so here they are.
> Patches 02-04 are fixes based on code inspection. Also patch 01 was
> tested by Martin Weinelt, Martin if you don't mind please add your
> tested-by tag to it by replying with Tested-by: name <email>.
> I've also briefly tested the set by trying to exercise those code paths.
Series applied, thanks.
^ permalink raw reply
* Re: [Bridge] [RFC net-next] net: dsa: add support for MC_DISABLED attribute
From: Ido Schimmel @ 2019-07-02 17:11 UTC (permalink / raw)
To: Linus Lüssing
Cc: andrew@lunn.ch, Florian Fainelli, Jiri Pirko, b.a.t.m.a.n,
nikolay, netdev@vger.kernel.org, bridge,
Russell King - ARM Linux admin, davem@davemloft.net, Ido Schimmel,
Vivien Didelot
In-Reply-To: <20190630165601.GC2500@otheros>
On Sun, Jun 30, 2019 at 06:56:01PM +0200, Linus Lüssing wrote:
> > On Sun, Jun 23, 2019 at 10:44:27AM +0300, Ido Schimmel wrote:
> > > See commit b00589af3b04 ("bridge: disable snooping if there is no
> > > querier"). I think that's unfortunate behavior that we need because
> > > multicast snooping is enabled by default. If it weren't enabled by
> > > default, then anyone enabling it would also make sure there's a querier
> > > in the network.
>
> I do not quite understand that point. In a way, that's what we
> have right now, isn't it? By default it's disabled, because by
> default there is no querier on the link. So anyone wanting to use
> multicast snooping will need to make sure there's a querier in the
> network.
Hi Linus,
Querier state is not reflected to drivers ATM, so drivers believe the
bridge is multicast aware and unregistered multicast packets are only
flooded to mrouter ports. Hosts that are silent (because there is no
querier) never get the traffic addressed to them (f.e., IPv6 neighbour
solicitation).
> Overall I think the querier (election) mechanism in the standards could
> need an update. While the lowest-address first might have
> worked well back then, in uniform, fully wired networks where the
> position of the querier did not matter, this is not a good
> solution anymore in networks involving wireless, dynamic connections.
> Especially in wireless mesh networks this is a bit of an issue for
> us. Ideally, the querier mechanism were dismissed in favour of simply
> unsolicited, periodic IGMP/MLD reports...
>
> But of course, updating IETF standards is no solution for now.
>
> While more complicated, it would not be impossible to consider the
> querier state, would it? I mean you probably already need to
> consider the case of a user disabling multicast snooping during
> runtime, right?
Sure, this is implemented.
> So similarly, you could react to appearing or disappearing queriers?
Yes, but it's a bit more complicated since we need to differentiate
between IPv4 and IPv6. If the bridge is multicast aware, but there is
only IPv4 querier on the link, then:
1. All the IPv6 MDB entries need to be removed from the device. At least
in mlxsw, we do not have a way to ignore only IPv6 entries. From the
device's perspective, an MDB entry is just a multicast DMAC with a
bitmap of ports packets should be replicated to.
2. We need to split the flood tables used for IPv4 and IPv6 unregistered
multicast packets. For IPv4, packets should only be flooded to mrouter
ports whereas for IPv6 packets should be flooded to all the member
ports.
Do you differentiate between IPv4 and IPv6 in batman-adv?
> Cheers, Linus
Thanks for the feedback!
^ permalink raw reply
* Re: [Bridge] [RFC net-next] net: dsa: add support for MC_DISABLED attribute
From: Nikolay Aleksandrov @ 2019-07-02 14:27 UTC (permalink / raw)
To: Linus Lüssing, Ido Schimmel
Cc: andrew@lunn.ch, Florian Fainelli, Jiri Pirko, b.a.t.m.a.n,
netdev@vger.kernel.org, bridge, Russell King - ARM Linux admin,
davem@davemloft.net, Ido Schimmel, Vivien Didelot
In-Reply-To: <20190630165601.GC2500@otheros>
On 30/06/2019 19:56, Linus Lüssing wrote:
> On Sat, Jun 29, 2019 at 07:29:45PM +0300, Ido Schimmel wrote:
>> I would like to avoid having drivers take the querier state into account
>> as it will only complicate things further.
>
> I absolutely share your pain. Initially in the early prototypes of
> multicast awareness in batman-adv we did not consider the querier state.
> And doing so later did indeed complicate the code a good bit in batman-adv
> (together with the IGMP/MLD suppression issues). I would have loved to
> avoid that.
>
>
>> Is there anything we can do about it? Enable the bridge querier if no
>> other querier was detected? Commit c5c23260594c ("bridge: Add
>> multicast_querier toggle and disable queries by default") disabled
>> queries by default, but I'm only suggesting to turn them on if no other
>> querier was detected on the link. Do you think it's still a problem?
>
> As soon as you start becoming the querier, you will not be able to reliably
> detect anymore whether you are the only querier candidate.
>
> If any random Linux host using a bridge device were potentially becoming
> a querier, that would cause quite some trouble when this host is
> behind some bad, bottleneck connection. This host will receive
> all multicast traffic, not just IGMP/MLD reports. And with a
> congested connection and then unreliable IGMP/MLD, multicast would
> become unreliable overall in this domain. So it's important that
> your querier is not running in the "dark, remote, dusty closet" of
> your network (topologically speaking).
>
+1
We definitely don't want random hosts becoming queriers
>> On Sun, Jun 23, 2019 at 10:44:27AM +0300, Ido Schimmel wrote:
>>> See commit b00589af3b04 ("bridge: disable snooping if there is no
>>> querier"). I think that's unfortunate behavior that we need because
>>> multicast snooping is enabled by default. If it weren't enabled by
>>> default, then anyone enabling it would also make sure there's a querier
>>> in the network.
>
> I do not quite understand that point. In a way, that's what we
> have right now, isn't it? By default it's disabled, because by
> default there is no querier on the link. So anyone wanting to use
> multicast snooping will need to make sure there's a querier in the
> network.
>
Indeed, also you could create the bridge with explicit mcast parameters if you need
different behaviour on start. Unfortunately I think you'll have to handle
the querier state.
>
> Overall I think the querier (election) mechanism in the standards could
> need an update. While the lowest-address first might have
> worked well back then, in uniform, fully wired networks where the
> position of the querier did not matter, this is not a good
> solution anymore in networks involving wireless, dynamic connections.
> Especially in wireless mesh networks this is a bit of an issue for
> us. Ideally, the querier mechanism were dismissed in favour of simply
> unsolicited, periodic IGMP/MLD reports...
>
> But of course, updating IETF standards is no solution for now.
>
> While more complicated, it would not be impossible to consider the
> querier state, would it? I mean you probably already need to
> consider the case of a user disabling multicast snooping during
> runtime, right? So similarly, you could react to appearing or
> disappearing queriers?
>
> Cheers, Linus
>
Thanks,
Nik
^ permalink raw reply
* Re: [Bridge] [PATCH net 2/4] net: bridge: mcast: fix stale ipv6 hdr pointer when handling v6 query
From: Martin Weinelt @ 2019-07-02 12:37 UTC (permalink / raw)
To: Nikolay Aleksandrov, netdev; +Cc: yoshfuji, roopa, bridge, davem
In-Reply-To: <20190702120021.13096-3-nikolay@cumulusnetworks.com>
[-- Attachment #1.1: Type: text/plain, Size: 1382 bytes --]
Tested-by: Martin Weinelt <martin@linuxlounge.net>
On 7/2/19 2:00 PM, Nikolay Aleksandrov wrote:
> We get a pointer to the ipv6 hdr in br_ip6_multicast_query but we may
> call pskb_may_pull afterwards and end up using a stale pointer.
> So use the header directly, it's just 1 place where it's needed.
>
> Fixes: 08b202b67264 ("bridge br_multicast: IPv6 MLD support.")
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> ---
> net/bridge/br_multicast.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index f37897e7b97b..3d8deac2353d 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -1279,7 +1279,6 @@ static int br_ip6_multicast_query(struct net_bridge *br,
> u16 vid)
> {
> unsigned int transport_len = ipv6_transport_len(skb);
> - const struct ipv6hdr *ip6h = ipv6_hdr(skb);
> struct mld_msg *mld;
> struct net_bridge_mdb_entry *mp;
> struct mld2_query *mld2q;
> @@ -1323,7 +1322,7 @@ static int br_ip6_multicast_query(struct net_bridge *br,
>
> if (is_general_query) {
> saddr.proto = htons(ETH_P_IPV6);
> - saddr.u.ip6 = ip6h->saddr;
> + saddr.u.ip6 = ipv6_hdr(skb)->saddr;
>
> br_multicast_query_received(br, port, &br->ip6_other_query,
> &saddr, max_delay);
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox