netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC 0/3] Add support for meta keys, bridge family specific
@ 2014-03-27  9:19 Tomasz Bursztyka
  2014-03-27  9:19 ` [RFC 1/3] netfilter: nf_tables: Make public core function of META expression Tomasz Bursztyka
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Tomasz Bursztyka @ 2014-03-27  9:19 UTC (permalink / raw)
  To: pablo, kaber; +Cc: netfilter-devel, 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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [RFC 1/3] netfilter: nf_tables: Make public core function of META expression
  2014-03-27  9:19 [RFC 0/3] Add support for meta keys, bridge family specific Tomasz Bursztyka
@ 2014-03-27  9:19 ` Tomasz Bursztyka
  2014-03-27  9:19 ` [RFC 2/3] netfilter: nf_tables: Add meta expression key for bridge interface name Tomasz Bursztyka
  2014-03-27  9:19 ` [RFC 3/3] netfilter: nftables: Return preferably given family expression if any Tomasz Bursztyka
  2 siblings, 0 replies; 10+ messages in thread
From: Tomasz Bursztyka @ 2014-03-27  9:19 UTC (permalink / raw)
  To: pablo, kaber; +Cc: netfilter-devel, 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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [RFC 2/3] netfilter: nf_tables: Add meta expression key for bridge interface name
  2014-03-27  9:19 [RFC 0/3] Add support for meta keys, bridge family specific Tomasz Bursztyka
  2014-03-27  9:19 ` [RFC 1/3] netfilter: nf_tables: Make public core function of META expression Tomasz Bursztyka
@ 2014-03-27  9:19 ` Tomasz Bursztyka
  2014-04-08  8:06   ` Pablo Neira Ayuso
  2014-03-27  9:19 ` [RFC 3/3] netfilter: nftables: Return preferably given family expression if any Tomasz Bursztyka
  2 siblings, 1 reply; 10+ messages in thread
From: Tomasz Bursztyka @ 2014-03-27  9:19 UTC (permalink / raw)
  To: pablo, kaber; +Cc: netfilter-devel, 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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [RFC 3/3] netfilter: nftables: Return preferably given family expression if any
  2014-03-27  9:19 [RFC 0/3] Add support for meta keys, bridge family specific Tomasz Bursztyka
  2014-03-27  9:19 ` [RFC 1/3] netfilter: nf_tables: Make public core function of META expression Tomasz Bursztyka
  2014-03-27  9:19 ` [RFC 2/3] netfilter: nf_tables: Add meta expression key for bridge interface name Tomasz Bursztyka
@ 2014-03-27  9:19 ` Tomasz Bursztyka
  2014-03-27  9:26   ` Patrick McHardy
  2 siblings, 1 reply; 10+ messages in thread
From: Tomasz Bursztyka @ 2014-03-27  9:19 UTC (permalink / raw)
  To: pablo, kaber; +Cc: netfilter-devel, 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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [RFC 3/3] netfilter: nftables: Return preferably given family expression if any
  2014-03-27  9:19 ` [RFC 3/3] netfilter: nftables: Return preferably given family expression if any Tomasz Bursztyka
@ 2014-03-27  9:26   ` Patrick McHardy
  2014-03-27 11:00     ` Tomasz Bursztyka
  0 siblings, 1 reply; 10+ messages in thread
From: Patrick McHardy @ 2014-03-27  9:26 UTC (permalink / raw)
  To: Tomasz Bursztyka; +Cc: pablo, netfilter-devel

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC 3/3] netfilter: nftables: Return preferably given family expression if any
  2014-03-27  9:26   ` Patrick McHardy
@ 2014-03-27 11:00     ` Tomasz Bursztyka
  0 siblings, 0 replies; 10+ messages in thread
From: Tomasz Bursztyka @ 2014-03-27 11:00 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: pablo, netfilter-devel

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC 2/3] netfilter: nf_tables: Add meta expression key for bridge interface name
  2014-03-27  9:19 ` [RFC 2/3] netfilter: nf_tables: Add meta expression key for bridge interface name Tomasz Bursztyka
@ 2014-04-08  8:06   ` Pablo Neira Ayuso
  2014-04-08  8:20     ` Tomasz Bursztyka
  0 siblings, 1 reply; 10+ messages in thread
From: Pablo Neira Ayuso @ 2014-04-08  8:06 UTC (permalink / raw)
  To: Tomasz Bursztyka; +Cc: kaber, netfilter-devel

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC 2/3] netfilter: nf_tables: Add meta expression key for bridge interface name
  2014-04-08  8:06   ` Pablo Neira Ayuso
@ 2014-04-08  8:20     ` Tomasz Bursztyka
  2014-04-08  8:34       ` Pablo Neira Ayuso
  0 siblings, 1 reply; 10+ messages in thread
From: Tomasz Bursztyka @ 2014-04-08  8:20 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: kaber, netfilter-devel

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC 2/3] netfilter: nf_tables: Add meta expression key for bridge interface name
  2014-04-08  8:20     ` Tomasz Bursztyka
@ 2014-04-08  8:34       ` Pablo Neira Ayuso
  2014-04-08  9:04         ` Tomasz Bursztyka
  0 siblings, 1 reply; 10+ messages in thread
From: Pablo Neira Ayuso @ 2014-04-08  8:34 UTC (permalink / raw)
  To: Tomasz Bursztyka; +Cc: kaber, netfilter-devel

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?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC 2/3] netfilter: nf_tables: Add meta expression key for bridge interface name
  2014-04-08  8:34       ` Pablo Neira Ayuso
@ 2014-04-08  9:04         ` Tomasz Bursztyka
  0 siblings, 0 replies; 10+ messages in thread
From: Tomasz Bursztyka @ 2014-04-08  9:04 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: kaber, netfilter-devel


> +
> +       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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2014-04-08  9:04 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-27  9:19 [RFC 0/3] Add support for meta keys, bridge family specific Tomasz Bursztyka
2014-03-27  9:19 ` [RFC 1/3] netfilter: nf_tables: Make public core function of META expression Tomasz Bursztyka
2014-03-27  9:19 ` [RFC 2/3] netfilter: nf_tables: Add meta expression key for bridge interface name Tomasz Bursztyka
2014-04-08  8:06   ` Pablo Neira Ayuso
2014-04-08  8:20     ` Tomasz Bursztyka
2014-04-08  8:34       ` Pablo Neira Ayuso
2014-04-08  9:04         ` Tomasz Bursztyka
2014-03-27  9:19 ` [RFC 3/3] netfilter: nftables: Return preferably given family expression if any Tomasz Bursztyka
2014-03-27  9:26   ` Patrick McHardy
2014-03-27 11:00     ` Tomasz Bursztyka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).