[RFC 0/3] Add support for meta keys, bridge family specific

[RFC 0/3] Add support for meta keys, bridge family specific

[Tomasz Bursztyka]

Hi Patrick and Pablo,

I need your input on that one, thus I am sending it as an RFC.

Patch 1 and 2 are straight forward so I'll skip. (though there might be 
improvement to be done. For instance, the init code part is 90% same so
it could be factorized maybe)

Now patch 3: without it, the bridge specific module for meta expression
cannot work: bare meta expression module gets in use since it's generic from
family level.
It's again quite trivial, but I would like your opinion about it.

I was thinking also on different solutions like registering a meta expr type
for every family... but it's not really memory wise.

With patch 3, at worst it loops through the entire expression type list
and this case happens for most of expressions, since they are family agnostic.
So it's not great either...

What do you think?

(Btw, I already made the patches for libnftnl and nftables but I skipped those
for this RFC.)

Tomasz

-- 
1.8.3.2

[RFC 1/3] netfilter: nf_tables: Make public core function of META expression

[Tomasz Bursztyka]

This will be useful to create network family dedicated META expression
as for NFPROTO_BRIDGE for instance.

Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
---
 include/net/netfilter/nft_meta.h | 32 ++++++++++++++++++++++++++++++
 net/netfilter/nft_meta.c         | 42 ++++++++++++++++++++--------------------
 2 files changed, 53 insertions(+), 21 deletions(-)
 create mode 100644 include/net/netfilter/nft_meta.h

diff --git a/include/net/netfilter/nft_meta.h b/include/net/netfilter/nft_meta.h
new file mode 100644
index 0000000..2fcb32f
--- /dev/null
+++ b/include/net/netfilter/nft_meta.h
@@ -0,0 +1,32 @@
+#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_init_validate_get(uint32_t key);
+
+int nft_meta_init_validate_set(uint32_t key);
+
+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_data data[NFT_REG_MAX + 1],
+		       const struct nft_pktinfo *pkt);
+
+void nft_meta_set_eval(const struct nft_expr *expr,
+		       struct nft_data data[NFT_REG_MAX + 1],
+		       const struct nft_pktinfo *pkt);
+
+#endif
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index e8254ad..1b1422c 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -18,18 +18,11 @@
 #include <net/sock.h>
 #include <net/tcp_states.h> /* for TCP_TIME_WAIT */
 #include <net/netfilter/nf_tables.h>
+#include <net/netfilter/nft_meta.h>
 
-struct nft_meta {
-	enum nft_meta_keys	key:8;
-	union {
-		enum nft_registers	dreg:8;
-		enum nft_registers	sreg:8;
-	};
-};
-
-static void nft_meta_get_eval(const struct nft_expr *expr,
-			      struct nft_data data[NFT_REG_MAX + 1],
-			      const struct nft_pktinfo *pkt)
+void nft_meta_get_eval(const struct nft_expr *expr,
+		       struct nft_data data[NFT_REG_MAX + 1],
+		       const struct nft_pktinfo *pkt)
 {
 	const struct nft_meta *priv = nft_expr_priv(expr);
 	const struct sk_buff *skb = pkt->skb;
@@ -140,10 +133,11 @@ static void nft_meta_get_eval(const struct nft_expr *expr,
 err:
 	data[NFT_REG_VERDICT].verdict = NFT_BREAK;
 }
+EXPORT_SYMBOL_GPL(nft_meta_get_eval);
 
-static void nft_meta_set_eval(const struct nft_expr *expr,
-			      struct nft_data data[NFT_REG_MAX + 1],
-			      const struct nft_pktinfo *pkt)
+void nft_meta_set_eval(const struct nft_expr *expr,
+		       struct nft_data data[NFT_REG_MAX + 1],
+		       const struct nft_pktinfo *pkt)
 {
 	const struct nft_meta *meta = nft_expr_priv(expr);
 	struct sk_buff *skb = pkt->skb;
@@ -163,14 +157,16 @@ 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_init_validate_set(uint32_t key)
+int nft_meta_init_validate_set(uint32_t key)
 {
 	switch (key) {
 	case NFT_META_MARK:
@@ -181,8 +177,9 @@ static int nft_meta_init_validate_set(uint32_t key)
 		return -EOPNOTSUPP;
 	}
 }
+EXPORT_SYMBOL_GPL(nft_meta_init_validate_set);
 
-static int nft_meta_init_validate_get(uint32_t key)
+int nft_meta_init_validate_get(uint32_t key)
 {
 	switch (key) {
 	case NFT_META_LEN:
@@ -211,6 +208,7 @@ static int nft_meta_init_validate_get(uint32_t key)
 	}
 
 }
+EXPORT_SYMBOL_GPL(nft_meta_init_validate_get);
 
 static int nft_meta_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
 			 const struct nlattr * const tb[])
@@ -246,8 +244,8 @@ static int nft_meta_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
 	return 0;
 }
 
-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);
 
@@ -260,9 +258,10 @@ 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);
 
@@ -276,6 +275,7 @@ static int nft_meta_set_dump(struct sk_buff *skb,
 nla_put_failure:
 	return -1;
 }
+EXPORT_SYMBOL_GPL(nft_meta_set_dump);
 
 static struct nft_expr_type nft_meta_type;
 static const struct nft_expr_ops nft_meta_get_ops = {
-- 
1.8.3.2

[RFC 2/3] netfilter: nf_tables: Add meta expression key for bridge interface name

[Tomasz Bursztyka]

NFT_META_IBRIFNAME to get packet input bridge interface name
NFT_META_OBRIFNAME to get packet output bridge interface name

Such meta key are accessible only through NFPROTO_BRIDGE family, on a
dedicated nft meta module: nft_meta_bridge.

Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
---
 include/uapi/linux/netfilter/nf_tables.h |   4 +
 net/bridge/Makefile                      |   1 +
 net/bridge/netfilter/Kconfig             |  12 ++-
 net/bridge/netfilter/Makefile            |   1 +
 net/bridge/netfilter/nft_meta_bridge.c   | 162 +++++++++++++++++++++++++++++++
 5 files changed, 179 insertions(+), 1 deletion(-)
 create mode 100644 net/bridge/netfilter/nft_meta_bridge.c

diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 83c985a..6b84a2e 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -533,6 +533,8 @@ enum nft_exthdr_attributes {
  * @NFT_META_SECMARK: packet secmark (skb->secmark)
  * @NFT_META_NFPROTO: netfilter protocol
  * @NFT_META_L4PROTO: layer 4 protocol number
+ * @NFT_META_BRI_IIFNAME: packet input bridge interface name
+ * @NFT_META_BRI_OIFNAME: packet output bridge interface name
  */
 enum nft_meta_keys {
 	NFT_META_LEN,
@@ -552,6 +554,8 @@ enum nft_meta_keys {
 	NFT_META_SECMARK,
 	NFT_META_NFPROTO,
 	NFT_META_L4PROTO,
+	NFT_META_BRI_IIFNAME,
+	NFT_META_BRI_OIFNAME,
 };
 
 /**
diff --git a/net/bridge/Makefile b/net/bridge/Makefile
index e85498b2f..58acd82 100644
--- a/net/bridge/Makefile
+++ b/net/bridge/Makefile
@@ -16,4 +16,5 @@ bridge-$(CONFIG_BRIDGE_IGMP_SNOOPING) += br_multicast.o br_mdb.o
 
 bridge-$(CONFIG_BRIDGE_VLAN_FILTERING) += br_vlan.o
 
+obj-$(CONFIG_NF_TABLES_BRIDGE) += netfilter/
 obj-$(CONFIG_BRIDGE_NF_EBTABLES) += netfilter/
diff --git a/net/bridge/netfilter/Kconfig b/net/bridge/netfilter/Kconfig
index 5ca74a0..906783d 100644
--- a/net/bridge/netfilter/Kconfig
+++ b/net/bridge/netfilter/Kconfig
@@ -2,10 +2,20 @@
 # Bridge netfilter configuration
 #
 #
-config NF_TABLES_BRIDGE
+menuconfig NF_TABLES_BRIDGE
 	depends on NF_TABLES
 	tristate "Ethernet Bridge nf_tables support"
 
+if NF_TABLES_BRIDGE
+
+config NFT_BRIDGE_META
+	tristate "Netfilter nf_table bridge meta support"
+	depends on NFT_META
+	help
+	  Add support for bridge dedicated meta key.
+
+endif # NF_TABLES_BRIDGE
+
 menuconfig BRIDGE_NF_EBTABLES
 	tristate "Ethernet Bridge tables (ebtables) support"
 	depends on BRIDGE && NETFILTER
diff --git a/net/bridge/netfilter/Makefile b/net/bridge/netfilter/Makefile
index ea7629f..6f2f394 100644
--- a/net/bridge/netfilter/Makefile
+++ b/net/bridge/netfilter/Makefile
@@ -3,6 +3,7 @@
 #
 
 obj-$(CONFIG_NF_TABLES_BRIDGE) += nf_tables_bridge.o
+obj-$(CONFIG_NFT_BRIDGE_META)  += nft_meta_bridge.o
 
 obj-$(CONFIG_BRIDGE_NF_EBTABLES) += ebtables.o
 
diff --git a/net/bridge/netfilter/nft_meta_bridge.c b/net/bridge/netfilter/nft_meta_bridge.c
new file mode 100644
index 0000000..411a6b5
--- /dev/null
+++ b/net/bridge/netfilter/nft_meta_bridge.c
@@ -0,0 +1,162 @@
+/*
+ * Copyright (c) 2012 Intel Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#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_data data[NFT_REG_MAX + 1],
+				     const struct nft_pktinfo *pkt)
+{
+	const struct nft_meta *priv = nft_expr_priv(expr);
+	const struct net_device *in = pkt->in, *out = pkt->out;
+	struct nft_data *dest = &data[priv->dreg];
+	const struct net_bridge_port *p;
+
+	if (pkt->ops->pf != NFPROTO_BRIDGE)
+		goto out;
+
+	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->data, p->br->dev->name, sizeof(dest->data));
+	return;
+out:
+	return nft_meta_get_eval(expr, data, pkt);
+err:
+	data[NFT_REG_VERDICT].verdict = NFT_BREAK;
+}
+
+static int nft_meta_bridge_init_validate_get(uint32_t key)
+{
+	switch (key) {
+	case NFT_META_BRI_IIFNAME:
+	case NFT_META_BRI_OIFNAME:
+		return 0;
+	default:
+		break;
+	}
+
+	return nft_meta_init_validate_get(key);
+}
+
+static int nft_meta_bridge_init(const struct nft_ctx *ctx,
+				const struct nft_expr *expr,
+				const struct nlattr * const tb[])
+{
+	struct nft_meta *priv = nft_expr_priv(expr);
+	int err;
+
+	priv->key = ntohl(nla_get_be32(tb[NFTA_META_KEY]));
+
+	if (tb[NFTA_META_DREG]) {
+		err = nft_meta_bridge_init_validate_get(priv->key);
+		if (err < 0)
+			return err;
+
+		priv->dreg = ntohl(nla_get_be32(tb[NFTA_META_DREG]));
+		err = nft_validate_output_register(priv->dreg);
+		if (err < 0)
+			return err;
+
+		return nft_validate_data_load(ctx, priv->dreg, NULL,
+					      NFT_DATA_VALUE);
+	}
+
+	err = nft_meta_init_validate_set(priv->key);
+	if (err < 0)
+		return err;
+
+	priv->sreg = ntohl(nla_get_be32(tb[NFTA_META_SREG]));
+	err = nft_validate_input_register(priv->sreg);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
+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_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_bridge_init,
+	.dump		= nft_meta_set_dump,
+};
+
+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("Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>");
+MODULE_ALIAS_NFT_AF_EXPR(AF_BRIDGE, "meta");
-- 
1.8.3.2

Re: [RFC 2/3] netfilter: nf_tables: Add meta expression key for bridge interface name

[Pablo Neira Ayuso]

Hi Tomasz,

On Thu, Mar 27, 2014 at 11:19:31AM +0200, Tomasz Bursztyka wrote:
> NFT_META_IBRIFNAME to get packet input bridge interface name
> NFT_META_OBRIFNAME to get packet output bridge interface name
> 
> Such meta key are accessible only through NFPROTO_BRIDGE family, on a
> dedicated nft meta module: nft_meta_bridge.
> 
> Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org>
> Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
> ---
>  include/uapi/linux/netfilter/nf_tables.h |   4 +
>  net/bridge/Makefile                      |   1 +
>  net/bridge/netfilter/Kconfig             |  12 ++-
>  net/bridge/netfilter/Makefile            |   1 +
>  net/bridge/netfilter/nft_meta_bridge.c   | 162 +++++++++++++++++++++++++++++++
>  5 files changed, 179 insertions(+), 1 deletion(-)
>  create mode 100644 net/bridge/netfilter/nft_meta_bridge.c
> 
> diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
> index 83c985a..6b84a2e 100644
> --- a/include/uapi/linux/netfilter/nf_tables.h
> +++ b/include/uapi/linux/netfilter/nf_tables.h
> @@ -533,6 +533,8 @@ enum nft_exthdr_attributes {
>   * @NFT_META_SECMARK: packet secmark (skb->secmark)
>   * @NFT_META_NFPROTO: netfilter protocol
>   * @NFT_META_L4PROTO: layer 4 protocol number
> + * @NFT_META_BRI_IIFNAME: packet input bridge interface name
> + * @NFT_META_BRI_OIFNAME: packet output bridge interface name
>   */
>  enum nft_meta_keys {
>  	NFT_META_LEN,
> @@ -552,6 +554,8 @@ enum nft_meta_keys {
>  	NFT_META_SECMARK,
>  	NFT_META_NFPROTO,
>  	NFT_META_L4PROTO,
> +	NFT_META_BRI_IIFNAME,
> +	NFT_META_BRI_OIFNAME,
>  };
>  
>  /**
> diff --git a/net/bridge/Makefile b/net/bridge/Makefile
> index e85498b2f..58acd82 100644
> --- a/net/bridge/Makefile
> +++ b/net/bridge/Makefile
> @@ -16,4 +16,5 @@ bridge-$(CONFIG_BRIDGE_IGMP_SNOOPING) += br_multicast.o br_mdb.o
>  
>  bridge-$(CONFIG_BRIDGE_VLAN_FILTERING) += br_vlan.o
>  
> +obj-$(CONFIG_NF_TABLES_BRIDGE) += netfilter/
>  obj-$(CONFIG_BRIDGE_NF_EBTABLES) += netfilter/
> diff --git a/net/bridge/netfilter/Kconfig b/net/bridge/netfilter/Kconfig
> index 5ca74a0..906783d 100644
> --- a/net/bridge/netfilter/Kconfig
> +++ b/net/bridge/netfilter/Kconfig
> @@ -2,10 +2,20 @@
>  # Bridge netfilter configuration
>  #
>  #
> -config NF_TABLES_BRIDGE
> +menuconfig NF_TABLES_BRIDGE
>  	depends on NF_TABLES
>  	tristate "Ethernet Bridge nf_tables support"
>  
> +if NF_TABLES_BRIDGE
> +
> +config NFT_BRIDGE_META
> +	tristate "Netfilter nf_table bridge meta support"
> +	depends on NFT_META
> +	help
> +	  Add support for bridge dedicated meta key.

... like the bridge port name.

> +
> +endif # NF_TABLES_BRIDGE
> +
>  menuconfig BRIDGE_NF_EBTABLES
>  	tristate "Ethernet Bridge tables (ebtables) support"
>  	depends on BRIDGE && NETFILTER
> diff --git a/net/bridge/netfilter/Makefile b/net/bridge/netfilter/Makefile
> index ea7629f..6f2f394 100644
> --- a/net/bridge/netfilter/Makefile
> +++ b/net/bridge/netfilter/Makefile
> @@ -3,6 +3,7 @@
>  #
>  
>  obj-$(CONFIG_NF_TABLES_BRIDGE) += nf_tables_bridge.o
> +obj-$(CONFIG_NFT_BRIDGE_META)  += nft_meta_bridge.o
>  
>  obj-$(CONFIG_BRIDGE_NF_EBTABLES) += ebtables.o
>  
> diff --git a/net/bridge/netfilter/nft_meta_bridge.c b/net/bridge/netfilter/nft_meta_bridge.c
> new file mode 100644
> index 0000000..411a6b5
> --- /dev/null
> +++ b/net/bridge/netfilter/nft_meta_bridge.c
> @@ -0,0 +1,162 @@
> +/*
> + * Copyright (c) 2012 Intel Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#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_data data[NFT_REG_MAX + 1],
> +				     const struct nft_pktinfo *pkt)
> +{
> +	const struct nft_meta *priv = nft_expr_priv(expr);
> +	const struct net_device *in = pkt->in, *out = pkt->out;
> +	struct nft_data *dest = &data[priv->dreg];
> +	const struct net_bridge_port *p;
> +
> +	if (pkt->ops->pf != NFPROTO_BRIDGE)
> +		goto out;

Is this possible or just defensive? I think we only allow the
selection of this expression flavour when the bridge family is used.

> +	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->data, p->br->dev->name, sizeof(dest->data));
> +	return;
> +out:
> +	return nft_meta_get_eval(expr, data, pkt);
> +err:
> +	data[NFT_REG_VERDICT].verdict = NFT_BREAK;
> +}
> +
> +static int nft_meta_bridge_init_validate_get(uint32_t key)
> +{
> +	switch (key) {
> +	case NFT_META_BRI_IIFNAME:
> +	case NFT_META_BRI_OIFNAME:
> +		return 0;
> +	default:
> +		break;
> +	}
> +
> +	return nft_meta_init_validate_get(key);
> +}
> +
> +static int nft_meta_bridge_init(const struct nft_ctx *ctx,
> +				const struct nft_expr *expr,
> +				const struct nlattr * const tb[])
> +{
> +	struct nft_meta *priv = nft_expr_priv(expr);
> +	int err;
> +
> +	priv->key = ntohl(nla_get_be32(tb[NFTA_META_KEY]));
> +
> +	if (tb[NFTA_META_DREG]) {
> +		err = nft_meta_bridge_init_validate_get(priv->key);
> +		if (err < 0)
> +			return err;
> +
> +		priv->dreg = ntohl(nla_get_be32(tb[NFTA_META_DREG]));
> +		err = nft_validate_output_register(priv->dreg);
> +		if (err < 0)
> +			return err;
> +
> +		return nft_validate_data_load(ctx, priv->dreg, NULL,
> +					      NFT_DATA_VALUE);
> +	}
> +
> +	err = nft_meta_init_validate_set(priv->key);
> +	if (err < 0)
> +		return err;
> +
> +	priv->sreg = ntohl(nla_get_be32(tb[NFTA_META_SREG]));
> +	err = nft_validate_input_register(priv->sreg);
> +	if (err < 0)
> +		return err;

Please, also rework this so we have one _init function for the get and
the set variants, ie. nft_meta_bridge_get_init and
nft_meta_bridge_set_init, I'd suggest.

Apart from that, this patch looks fine to me. Thanks.

Re: [RFC 2/3] netfilter: nf_tables: Add meta expression key for bridge interface name

[Tomasz Bursztyka]

Hi Pablo,

> Please, also rework this so we have one _init function for the get and
> the set variants, ie. nft_meta_bridge_get_init and
> nft_meta_bridge_set_init, I'd suggest.
>
> Apart from that, this patch looks fine to me. Thanks.

I fully changed that on the version 2. This RFC is no longer valid, prior to
Patrick's comments and also his changes on nft_meta.c

Tomasz

Re: [RFC 2/3] netfilter: nf_tables: Add meta expression key for bridge interface name

[Pablo Neira Ayuso]

On Tue, Apr 08, 2014 at 11:20:35AM +0300, Tomasz Bursztyka wrote:
> Hi Pablo,
> 
> >Please, also rework this so we have one _init function for the get and
> >the set variants, ie. nft_meta_bridge_get_init and
> >nft_meta_bridge_set_init, I'd suggest.
> >
> >Apart from that, this patch looks fine to me. Thanks.
> 
> I fully changed that on the version 2. This RFC is no longer valid, prior to
> Patrick's comments and also his changes on nft_meta.c

Right, I looked at the wrong patchset, sorry.

In http://patchwork.ozlabs.org/patch/336891/, I can still see there
this chunk though.

+static void nft_meta_bridge_get_eval(const struct nft_expr *expr,
+                                    struct nft_data data[NFT_REG_MAX
+ 1],
+                                    const struct nft_pktinfo *pkt)
+{
+       const struct nft_meta *priv = nft_expr_priv(expr);
+       const struct net_device *in = pkt->in, *out = pkt->out;
+       struct nft_data *dest = &data[priv->dreg];
+       const struct net_bridge_port *p;
+
+       if (pkt->ops->pf != NFPROTO_BRIDGE)

Do you really need this or is it just defensive?

Re: [RFC 2/3] netfilter: nf_tables: Add meta expression key for bridge interface name

[Tomasz Bursztyka]

> +
> +       if (pkt->ops->pf != NFPROTO_BRIDGE)
>
> Do you really need this or is it just defensive?

Defensive, but useless. Will get rid of it.

Tomasz

[RFC 3/3] netfilter: nftables: Return preferably given family expression if any

[Tomasz Bursztyka]

Currently, when looking up for the proper expression type, what comes
first is returned. Which might end up to be a non-family tight type.
Instead, if a specific family type exist, it will be better to return
this one.

Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
---
 net/netfilter/nf_tables_api.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index adce01e..83de6c1 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1117,14 +1117,17 @@ EXPORT_SYMBOL_GPL(nft_unregister_expr);
 static const struct nft_expr_type *__nft_expr_type_get(u8 family,
 						       struct nlattr *nla)
 {
-	const struct nft_expr_type *type;
-
-	list_for_each_entry(type, &nf_tables_expressions, list) {
-		if (!nla_strcmp(nla, type->name) &&
-		    (!type->family || type->family == family))
-			return type;
+	const struct nft_expr_type *test, *type = NULL;
+
+	list_for_each_entry(test, &nf_tables_expressions, list) {
+		if (!nla_strcmp(nla, test->name)) {
+			if (test->family == family)
+				return test;
+			if (!test->family)
+				type = test;
+		}
 	}
-	return NULL;
+	return type;
 }
 
 static const struct nft_expr_type *nft_expr_type_get(u8 family,
-- 
1.8.3.2

Re: [RFC 3/3] netfilter: nftables: Return preferably given family expression if any

[Patrick McHardy]

On Thu, Mar 27, 2014 at 11:19:32AM +0200, Tomasz Bursztyka wrote:
> Currently, when looking up for the proper expression type, what comes
> first is returned. Which might end up to be a non-family tight type.
> Instead, if a specific family type exist, it will be better to return
> this one.

Easier suggestion:

Change nft_register_expr() to appent NFPROTO_UNSPEC to the end of the
list and prepend all others.

> 
> Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
> ---
>  net/netfilter/nf_tables_api.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index adce01e..83de6c1 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -1117,14 +1117,17 @@ EXPORT_SYMBOL_GPL(nft_unregister_expr);
>  static const struct nft_expr_type *__nft_expr_type_get(u8 family,
>  						       struct nlattr *nla)
>  {
> -	const struct nft_expr_type *type;
> -
> -	list_for_each_entry(type, &nf_tables_expressions, list) {
> -		if (!nla_strcmp(nla, type->name) &&
> -		    (!type->family || type->family == family))
> -			return type;
> +	const struct nft_expr_type *test, *type = NULL;
> +
> +	list_for_each_entry(test, &nf_tables_expressions, list) {
> +		if (!nla_strcmp(nla, test->name)) {
> +			if (test->family == family)
> +				return test;
> +			if (!test->family)
> +				type = test;
> +		}
>  	}
> -	return NULL;
> +	return type;
>  }
>  
>  static const struct nft_expr_type *nft_expr_type_get(u8 family,
> -- 
> 1.8.3.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 3/3] netfilter: nftables: Return preferably given family expression if any

[Tomasz Bursztyka]

>> Currently, when looking up for the proper expression type, what comes
>> >first is returned. Which might end up to be a non-family tight type.
>> >Instead, if a specific family type exist, it will be better to return
>> >this one.
> Easier suggestion:
>
> Change nft_register_expr() to appent NFPROTO_UNSPEC to the end of the
> list and prepend all others.

Obvious one which I missed, as usual.

Thanks Patrick

Tomasz