Netdev List
 help / color / mirror / Atom feed
* [PATCH nf-next,v3 6/7] netfilter: nf_tables: flow offload expression
From: Pablo Neira Ayuso @ 2017-12-22 19:27 UTC (permalink / raw)
  To: netfilter-devel
  Cc: netdev, f.fainelli, simon.horman, ronye, jiri, nbd, john,
	kubakici, fw
In-Reply-To: <20171222192732.13188-1-pablo@netfilter.org>

Add new instruction for the nf_tables VM that allows us to specify what
flows are offloaded into a given flow table via name. This new
instruction creates the flow entry and adds it to the flow table.

Only established flows, ie. we have seen traffic in both directions, are
added to the flow table. You can still decide to offload entries at a
later stage via packet counting or checking the ct status in case you
want to offload assured conntracks.

This new extension depends on the conntrack subsystem.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/uapi/linux/netfilter/nf_tables.h |  11 ++
 net/netfilter/Kconfig                    |   7 +
 net/netfilter/Makefile                   |   1 +
 net/netfilter/nft_flow_offload.c         | 268 +++++++++++++++++++++++++++++++
 4 files changed, 287 insertions(+)
 create mode 100644 net/netfilter/nft_flow_offload.c

diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 9ba0f4c13de6..528d832fefb4 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -954,6 +954,17 @@ enum nft_ct_attributes {
 };
 #define NFTA_CT_MAX		(__NFTA_CT_MAX - 1)
 
+/**
+ * enum nft_flow_attributes - ct offload expression attributes
+ * @NFTA_FLOW_TABLE_NAME: flow table name (NLA_STRING)
+ */
+enum nft_offload_attributes {
+	NFTA_FLOW_UNSPEC,
+	NFTA_FLOW_TABLE_NAME,
+	__NFTA_FLOW_MAX,
+};
+#define NFTA_FLOW_MAX		(__NFTA_FLOW_MAX - 1)
+
 enum nft_limit_type {
 	NFT_LIMIT_PKTS,
 	NFT_LIMIT_PKT_BYTES
diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index 0c6256db5a6c..1ada46345f3c 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -497,6 +497,13 @@ config NFT_CT
 	  This option adds the "ct" expression that you can use to match
 	  connection tracking information such as the flow state.
 
+config NFT_FLOW_OFFLOAD
+	depends on NF_CONNTRACK
+	tristate "Netfilter nf_tables hardware flow offload module"
+	help
+	  This option adds the "flow_offload" expression that you can use to
+	  choose what flows are placed into the hardware.
+
 config NFT_SET_RBTREE
 	tristate "Netfilter nf_tables rbtree set module"
 	help
diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
index 1f7d92bd571a..2c1b8de922f2 100644
--- a/net/netfilter/Makefile
+++ b/net/netfilter/Makefile
@@ -83,6 +83,7 @@ obj-$(CONFIG_NFT_META)		+= nft_meta.o
 obj-$(CONFIG_NFT_RT)		+= nft_rt.o
 obj-$(CONFIG_NFT_NUMGEN)	+= nft_numgen.o
 obj-$(CONFIG_NFT_CT)		+= nft_ct.o
+obj-$(CONFIG_NFT_FLOW_OFFLOAD)	+= nft_flow_offload.o
 obj-$(CONFIG_NFT_LIMIT)		+= nft_limit.o
 obj-$(CONFIG_NFT_NAT)		+= nft_nat.o
 obj-$(CONFIG_NFT_OBJREF)	+= nft_objref.o
diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
new file mode 100644
index 000000000000..4f16c37acaa3
--- /dev/null
+++ b/net/netfilter/nft_flow_offload.c
@@ -0,0 +1,268 @@
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/netlink.h>
+#include <linux/netfilter.h>
+#include <linux/workqueue.h>
+#include <linux/spinlock.h>
+#include <linux/netfilter/nf_tables.h>
+#include <net/ip.h> /* for ipv4 options. */
+#include <net/netfilter/nf_tables.h>
+#include <net/netfilter/nf_tables_core.h>
+#include <net/netfilter/nf_conntrack_core.h>
+#include <linux/netfilter/nf_conntrack_common.h>
+#include <net/netfilter/nf_flow_table.h>
+
+struct nft_flow_offload {
+	struct nft_flowtable	*flowtable;
+};
+
+static int nft_flow_route(const struct nft_pktinfo *pkt,
+			  const struct nf_conn *ct,
+			  struct nf_flow_route *route,
+			  enum ip_conntrack_dir dir)
+{
+	struct dst_entry *this_dst = skb_dst(pkt->skb);
+	struct dst_entry *other_dst;
+	const struct nf_afinfo *ai;
+	struct flowi fl;
+
+	memset(&fl, 0, sizeof(fl));
+	switch (nft_pf(pkt)) {
+	case NFPROTO_IPV4:
+		fl.u.ip4.daddr = ct->tuplehash[!dir].tuple.dst.u3.ip;
+		break;
+	case NFPROTO_IPV6:
+		fl.u.ip6.daddr = ct->tuplehash[!dir].tuple.dst.u3.in6;
+		break;
+	}
+
+	ai = nf_get_afinfo(nft_pf(pkt));
+	if (ai) {
+		ai->route(nft_net(pkt), &other_dst, &fl, false);
+		if (!other_dst)
+			return -ENOENT;
+	}
+
+	route->tuple[dir].dst		= this_dst;
+	route->tuple[dir].ifindex	= nft_in(pkt)->ifindex;
+	route->tuple[!dir].dst		= other_dst;
+	route->tuple[!dir].ifindex	= nft_out(pkt)->ifindex;
+
+	return 0;
+}
+
+static bool nft_flow_offload_skip(struct sk_buff *skb)
+{
+	struct ip_options *opt  = &(IPCB(skb)->opt);
+
+	if (unlikely(opt->optlen))
+		return true;
+	if (skb_sec_path(skb))
+		return true;
+
+	return false;
+}
+
+static void nft_flow_offload_eval(const struct nft_expr *expr,
+				  struct nft_regs *regs,
+				  const struct nft_pktinfo *pkt)
+{
+	struct nft_flow_offload *priv = nft_expr_priv(expr);
+	struct nf_flowtable *flowtable = &priv->flowtable->data;
+	enum ip_conntrack_info ctinfo;
+	struct nf_flow_route route;
+	struct flow_offload *flow;
+	enum ip_conntrack_dir dir;
+	struct nf_conn *ct;
+	int ret;
+
+	if (nft_flow_offload_skip(pkt->skb))
+		goto out;
+
+	ct = nf_ct_get(pkt->skb, &ctinfo);
+	if (!ct)
+		goto out;
+
+	switch (ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.protonum) {
+	case IPPROTO_TCP:
+	case IPPROTO_UDP:
+		break;
+	default:
+		goto out;
+	}
+
+	if (test_bit(IPS_HELPER_BIT, &ct->status))
+		goto out;
+
+	if (ctinfo == IP_CT_NEW ||
+	    ctinfo == IP_CT_RELATED)
+		goto out;
+
+	if (test_and_set_bit(IPS_OFFLOAD_BIT, &ct->status))
+		goto out;
+
+	dir = CTINFO2DIR(ctinfo);
+	if (nft_flow_route(pkt, ct, &route, dir) < 0)
+		goto err_flow_route;
+
+	flow = flow_offload_alloc(ct, &route);
+	if (!flow)
+		goto err_flow_alloc;
+
+	ret = flow_offload_add(flowtable, flow);
+	if (ret < 0)
+		goto err_flow_add;
+
+	return;
+
+err_flow_add:
+	flow_offload_free(flow);
+err_flow_alloc:
+	dst_release(route.tuple[!dir].dst);
+err_flow_route:
+	clear_bit(IPS_OFFLOAD_BIT, &ct->status);
+out:
+	regs->verdict.code = NFT_BREAK;
+}
+
+static int nft_flow_offload_validate(const struct nft_ctx *ctx,
+				     const struct nft_expr *expr,
+				     const struct nft_data **data)
+{
+	unsigned int hook_mask = (1 << NF_INET_FORWARD);
+
+	return nft_chain_validate_hooks(ctx->chain, hook_mask);
+}
+
+static int nft_flow_offload_init(const struct nft_ctx *ctx,
+				 const struct nft_expr *expr,
+				 const struct nlattr * const tb[])
+{
+	struct nft_flow_offload *priv = nft_expr_priv(expr);
+	u8 genmask = nft_genmask_next(ctx->net);
+	struct nft_flowtable *flowtable;
+
+	if (!tb[NFTA_FLOW_TABLE_NAME])
+		return -EINVAL;
+
+	flowtable = nf_tables_flowtable_lookup(ctx->table,
+					       tb[NFTA_FLOW_TABLE_NAME],
+					       genmask);
+	if (IS_ERR(flowtable))
+		return PTR_ERR(flowtable);
+
+	priv->flowtable = flowtable;
+	flowtable->use++;
+
+	return nf_ct_netns_get(ctx->net, ctx->afi->family);
+}
+
+static void nft_flow_offload_destroy(const struct nft_ctx *ctx,
+				     const struct nft_expr *expr)
+{
+	struct nft_flow_offload *priv = nft_expr_priv(expr);
+
+	priv->flowtable->use--;
+	nf_ct_netns_put(ctx->net, ctx->afi->family);
+}
+
+static int nft_flow_offload_dump(struct sk_buff *skb, const struct nft_expr *expr)
+{
+	struct nft_flow_offload *priv = nft_expr_priv(expr);
+
+	if (nla_put_string(skb, NFTA_FLOW_TABLE_NAME, priv->flowtable->name))
+		goto nla_put_failure;
+
+	return 0;
+
+nla_put_failure:
+	return -1;
+}
+
+struct nft_expr_type nft_flow_offload_type;
+static const struct nft_expr_ops nft_flow_offload_ops = {
+	.type		= &nft_flow_offload_type,
+	.size		= NFT_EXPR_SIZE(sizeof(struct nft_flow_offload)),
+	.eval		= nft_flow_offload_eval,
+	.init		= nft_flow_offload_init,
+	.destroy	= nft_flow_offload_destroy,
+	.validate	= nft_flow_offload_validate,
+	.dump		= nft_flow_offload_dump,
+};
+
+struct nft_expr_type nft_flow_offload_type __read_mostly = {
+	.name		= "flow_offload",
+	.ops		= &nft_flow_offload_ops,
+	.maxattr	= NFTA_FLOW_MAX,
+	.owner		= THIS_MODULE,
+};
+
+static void flow_offload_iterate_cleanup(struct flow_offload *flow, void *data)
+{
+	struct net_device *dev = data;
+
+	if (dev && flow->tuplehash[0].tuple.iifidx != dev->ifindex)
+		return;
+
+	flow_offload_dead(flow);
+}
+
+static void nft_flow_offload_iterate_cleanup(struct nf_flowtable *flowtable,
+					     void *data)
+{
+	nf_flow_table_iterate(flowtable, flow_offload_iterate_cleanup, data);
+}
+
+static int flow_offload_netdev_event(struct notifier_block *this,
+				     unsigned long event, void *ptr)
+{
+	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
+
+	if (event != NETDEV_DOWN)
+		return NOTIFY_DONE;
+
+	nft_flow_table_iterate(dev_net(dev), nft_flow_offload_iterate_cleanup, dev);
+
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block flow_offload_netdev_notifier = {
+	.notifier_call	= flow_offload_netdev_event,
+};
+
+static int __init nft_flow_offload_module_init(void)
+{
+	int err;
+
+	register_netdevice_notifier(&flow_offload_netdev_notifier);
+
+	err = nft_register_expr(&nft_flow_offload_type);
+	if (err < 0)
+		goto register_expr;
+
+	return 0;
+
+register_expr:
+	unregister_netdevice_notifier(&flow_offload_netdev_notifier);
+	return err;
+}
+
+static void __exit nft_flow_offload_module_exit(void)
+{
+	struct net *net;
+
+	nft_unregister_expr(&nft_flow_offload_type);
+	unregister_netdevice_notifier(&flow_offload_netdev_notifier);
+	rtnl_lock();
+	for_each_net(net)
+		nft_flow_table_iterate(net, nft_flow_offload_iterate_cleanup, NULL);
+	rtnl_unlock();
+}
+
+module_init(nft_flow_offload_module_init);
+module_exit(nft_flow_offload_module_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Pablo Neira Ayuso <pablo@netfilter.org>");
+MODULE_ALIAS_NFT_EXPR("flow_offload");
-- 
2.11.0


^ permalink raw reply related

* [PATCH nf-next,v3 5/7] netfilter: flow table support for the mixed IPv4/IPv6 family
From: Pablo Neira Ayuso @ 2017-12-22 19:27 UTC (permalink / raw)
  To: netfilter-devel
  Cc: netdev, f.fainelli, simon.horman, ronye, jiri, nbd, john,
	kubakici, fw
In-Reply-To: <20171222192732.13188-1-pablo@netfilter.org>

This patch adds the IPv6 flow table type, that implements the datapath
flow table to forward IPv6 traffic.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_flow_table.h   |  5 ++++
 net/ipv4/netfilter/nf_flow_table_ipv4.c |  3 ++-
 net/ipv6/netfilter/nf_flow_table_ipv6.c |  3 ++-
 net/netfilter/Kconfig                   |  8 ++++++
 net/netfilter/nf_flow_table_inet.c      | 48 +++++++++++++++++++++++++++++++++
 5 files changed, 65 insertions(+), 2 deletions(-)
 create mode 100644 net/netfilter/nf_flow_table_inet.c

diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h
index 161f71ca78a0..b22b22082733 100644
--- a/include/net/netfilter/nf_flow_table.h
+++ b/include/net/netfilter/nf_flow_table.h
@@ -111,6 +111,11 @@ struct flow_ports {
 	__be16 source, dest;
 };
 
+unsigned int nf_flow_offload_ip_hook(void *priv, struct sk_buff *skb,
+				     const struct nf_hook_state *state);
+unsigned int nf_flow_offload_ipv6_hook(void *priv, struct sk_buff *skb,
+				       const struct nf_hook_state *state);
+
 #define MODULE_ALIAS_NF_FLOWTABLE(family)	\
 	MODULE_ALIAS("nf-flowtable-" __stringify(family))
 
diff --git a/net/ipv4/netfilter/nf_flow_table_ipv4.c b/net/ipv4/netfilter/nf_flow_table_ipv4.c
index ac56c0f0492a..b2d01eb25f2c 100644
--- a/net/ipv4/netfilter/nf_flow_table_ipv4.c
+++ b/net/ipv4/netfilter/nf_flow_table_ipv4.c
@@ -202,7 +202,7 @@ static bool nf_flow_exceeds_mtu(struct sk_buff *skb, const struct rtable *rt)
 	return false;
 }
 
-static unsigned int
+unsigned int
 nf_flow_offload_ip_hook(void *priv, struct sk_buff *skb,
 			const struct nf_hook_state *state)
 {
@@ -254,6 +254,7 @@ nf_flow_offload_ip_hook(void *priv, struct sk_buff *skb,
 
 	return NF_STOLEN;
 }
+EXPORT_SYMBOL_GPL(nf_flow_offload_ip_hook);
 
 static struct nf_flowtable_type flowtable_ipv4 = {
 	.family		= NFPROTO_IPV4,
diff --git a/net/ipv6/netfilter/nf_flow_table_ipv6.c b/net/ipv6/netfilter/nf_flow_table_ipv6.c
index ab78703154d8..021209be0c3c 100644
--- a/net/ipv6/netfilter/nf_flow_table_ipv6.c
+++ b/net/ipv6/netfilter/nf_flow_table_ipv6.c
@@ -196,7 +196,7 @@ static bool nf_flow_exceeds_mtu(struct sk_buff *skb, const struct rt6_info *rt)
 	return false;
 }
 
-static unsigned int
+unsigned int
 nf_flow_ipv6_offload_hook(void *priv, struct sk_buff *skb,
 			 const struct nf_hook_state *state)
 {
@@ -248,6 +248,7 @@ nf_flow_ipv6_offload_hook(void *priv, struct sk_buff *skb,
 
 	return NF_STOLEN;
 }
+EXPORT_SYMBOL_GPL(nf_flow_ipv6_offload_hook);
 
 static struct nf_flowtable_type flowtable_ipv6 = {
 	.family		= NFPROTO_IPV6,
diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index af0f58322515..0c6256db5a6c 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -649,6 +649,14 @@ endif # NF_TABLES_NETDEV
 
 endif # NF_TABLES
 
+config NF_FLOW_TABLE_INET
+	select NF_FLOW_TABLE
+	tristate "Netfilter flow table mixed IPv4/IPv6 module"
+	help
+          This option adds the flow table mixed IPv4/IPv6 support.
+
+	  To compile it as a module, choose M here.
+
 config NF_FLOW_TABLE
 	tristate "Netfilter flow table module"
 	help
diff --git a/net/netfilter/nf_flow_table_inet.c b/net/netfilter/nf_flow_table_inet.c
new file mode 100644
index 000000000000..281209aeba8f
--- /dev/null
+++ b/net/netfilter/nf_flow_table_inet.c
@@ -0,0 +1,48 @@
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/netfilter.h>
+#include <linux/rhashtable.h>
+#include <net/netfilter/nf_flow_table.h>
+#include <net/netfilter/nf_tables.h>
+
+static unsigned int
+nf_flow_offload_inet_hook(void *priv, struct sk_buff *skb,
+			  const struct nf_hook_state *state)
+{
+	switch (skb->protocol) {
+	case htons(ETH_P_IP):
+		return nf_flow_offload_ip_hook(priv, skb, state);
+	case htons(ETH_P_IPV6):
+		return nf_flow_offload_ipv6_hook(priv, skb, state);
+	}
+
+	return NF_ACCEPT;
+}
+
+static struct nf_flowtable_type flowtable_inet = {
+	.family		= NFPROTO_INET,
+	.params		= &nf_flow_offload_rhash_params,
+	.gc		= nf_flow_offload_work_gc,
+	.hook		= nf_flow_offload_inet_hook,
+	.owner		= THIS_MODULE,
+};
+
+static int __init nf_flow_inet_module_init(void)
+{
+	nft_register_flowtable_type(&flowtable_inet);
+
+	return 0;
+}
+
+static void __exit nf_flow_inet_module_exit(void)
+{
+	nft_unregister_flowtable_type(&flowtable_inet);
+}
+
+module_init(nf_flow_inet_module_init);
+module_exit(nf_flow_inet_module_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Pablo Neira Ayuso <pablo@netfilter.org>");
+MODULE_ALIAS_NF_FLOWTABLE(1); /* NFPROTO_INET */
-- 
2.11.0


^ permalink raw reply related

* [PATCH nf-next,v3 4/7] netfilter: flow table support for IPv6
From: Pablo Neira Ayuso @ 2017-12-22 19:27 UTC (permalink / raw)
  To: netfilter-devel
  Cc: netdev, f.fainelli, simon.horman, ronye, jiri, nbd, john,
	kubakici, fw
In-Reply-To: <20171222192732.13188-1-pablo@netfilter.org>

This patch adds the IPv6 flow table type, that implements the datapath
flow table to forward IPv6 traffic.

This patch exports ip6_dst_mtu_forward() that is required to check for
mtu to pass up packets that need PMTUD handling to the classic
forwarding path.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/ipv6.h                      |   2 +
 net/ipv6/ip6_output.c                   |   3 +-
 net/ipv6/netfilter/Kconfig              |   8 +
 net/ipv6/netfilter/Makefile             |   3 +
 net/ipv6/netfilter/nf_flow_table_ipv6.c | 277 ++++++++++++++++++++++++++++++++
 5 files changed, 292 insertions(+), 1 deletion(-)
 create mode 100644 net/ipv6/netfilter/nf_flow_table_ipv6.c

diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index 6eac5cf8f1e6..ff069a8e0cde 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -912,6 +912,8 @@ static inline struct sk_buff *ip6_finish_skb(struct sock *sk)
 			      &inet6_sk(sk)->cork);
 }
 
+unsigned int ip6_dst_mtu_forward(const struct dst_entry *dst);
+
 int ip6_dst_lookup(struct net *net, struct sock *sk, struct dst_entry **dst,
 		   struct flowi6 *fl6);
 struct dst_entry *ip6_dst_lookup_flow(const struct sock *sk, struct flowi6 *fl6,
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 43ca864327c7..5ccd082ce182 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -362,7 +362,7 @@ static inline int ip6_forward_finish(struct net *net, struct sock *sk,
 	return dst_output(net, sk, skb);
 }
 
-static unsigned int ip6_dst_mtu_forward(const struct dst_entry *dst)
+unsigned int ip6_dst_mtu_forward(const struct dst_entry *dst)
 {
 	unsigned int mtu;
 	struct inet6_dev *idev;
@@ -382,6 +382,7 @@ static unsigned int ip6_dst_mtu_forward(const struct dst_entry *dst)
 
 	return mtu;
 }
+EXPORT_SYMBOL_GPL(ip6_dst_mtu_forward);
 
 static bool ip6_pkt_too_big(const struct sk_buff *skb, unsigned int mtu)
 {
diff --git a/net/ipv6/netfilter/Kconfig b/net/ipv6/netfilter/Kconfig
index 6acb2eecd986..806e95375ec8 100644
--- a/net/ipv6/netfilter/Kconfig
+++ b/net/ipv6/netfilter/Kconfig
@@ -71,6 +71,14 @@ config NFT_FIB_IPV6
 endif # NF_TABLES_IPV6
 endif # NF_TABLES
 
+config NF_FLOW_TABLE_IPV6
+	select NF_FLOW_TABLE
+	tristate "Netfilter flow table IPv6 module"
+	help
+	  This option adds the flow table IPv6 support.
+
+	  To compile it as a module, choose M here.
+
 config NF_DUP_IPV6
 	tristate "Netfilter IPv6 packet duplication to alternate destination"
 	depends on !NF_CONNTRACK || NF_CONNTRACK
diff --git a/net/ipv6/netfilter/Makefile b/net/ipv6/netfilter/Makefile
index fe180c96040e..7dceadbb9eea 100644
--- a/net/ipv6/netfilter/Makefile
+++ b/net/ipv6/netfilter/Makefile
@@ -44,6 +44,9 @@ obj-$(CONFIG_NFT_REDIR_IPV6) += nft_redir_ipv6.o
 obj-$(CONFIG_NFT_DUP_IPV6) += nft_dup_ipv6.o
 obj-$(CONFIG_NFT_FIB_IPV6) += nft_fib_ipv6.o
 
+# flow table support
+obj-$(CONFIG_NF_FLOW_TABLE_IPV6) += nf_flow_table_ipv6.o
+
 # matches
 obj-$(CONFIG_IP6_NF_MATCH_AH) += ip6t_ah.o
 obj-$(CONFIG_IP6_NF_MATCH_EUI64) += ip6t_eui64.o
diff --git a/net/ipv6/netfilter/nf_flow_table_ipv6.c b/net/ipv6/netfilter/nf_flow_table_ipv6.c
new file mode 100644
index 000000000000..ab78703154d8
--- /dev/null
+++ b/net/ipv6/netfilter/nf_flow_table_ipv6.c
@@ -0,0 +1,277 @@
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/netfilter.h>
+#include <linux/rhashtable.h>
+#include <linux/ipv6.h>
+#include <linux/netdevice.h>
+#include <linux/ipv6.h>
+#include <net/ipv6.h>
+#include <net/ip6_route.h>
+#include <net/neighbour.h>
+#include <net/netfilter/nf_flow_table.h>
+#include <net/netfilter/nf_tables.h>
+/* For layer 4 checksum field offset. */
+#include <linux/tcp.h>
+#include <linux/udp.h>
+
+static int nf_flow_nat_ipv6_tcp(struct sk_buff *skb, unsigned int thoff,
+				struct in6_addr *addr,
+				struct in6_addr *new_addr)
+{
+	struct tcphdr *tcph;
+
+	if (!pskb_may_pull(skb, thoff + sizeof(*tcph)) ||
+	    skb_try_make_writable(skb, thoff + sizeof(*tcph)))
+		return -1;
+
+	tcph = (void *)(skb_network_header(skb) + thoff);
+	inet_proto_csum_replace16(&tcph->check, skb, addr->s6_addr32,
+				  new_addr->s6_addr32, true);
+
+	return 0;
+}
+
+static int nf_flow_nat_ipv6_udp(struct sk_buff *skb, unsigned int thoff,
+				struct in6_addr *addr,
+				struct in6_addr *new_addr)
+{
+	struct udphdr *udph;
+
+	if (!pskb_may_pull(skb, thoff + sizeof(*udph)) ||
+	    skb_try_make_writable(skb, thoff + sizeof(*udph)))
+		return -1;
+
+	udph = (void *)(skb_network_header(skb) + thoff);
+	if (udph->check || skb->ip_summed == CHECKSUM_PARTIAL) {
+		inet_proto_csum_replace16(&udph->check, skb, addr->s6_addr32,
+					  new_addr->s6_addr32, true);
+		if (!udph->check)
+			udph->check = CSUM_MANGLED_0;
+	}
+
+	return 0;
+}
+
+static int nf_flow_nat_ipv6_l4proto(struct sk_buff *skb, struct ipv6hdr *ip6h,
+				    unsigned int thoff, struct in6_addr *addr,
+				    struct in6_addr *new_addr)
+{
+	switch (ip6h->nexthdr) {
+	case IPPROTO_TCP:
+		if (nf_flow_nat_ipv6_tcp(skb, thoff, addr, new_addr) < 0)
+			return NF_DROP;
+		break;
+	case IPPROTO_UDP:
+		if (nf_flow_nat_ipv6_udp(skb, thoff, addr, new_addr) < 0)
+			return NF_DROP;
+		break;
+	}
+
+	return 0;
+}
+
+static int nf_flow_snat_ipv6(const struct flow_offload *flow,
+			     struct sk_buff *skb, struct ipv6hdr *ip6h,
+			     unsigned int thoff,
+			     enum flow_offload_tuple_dir dir)
+{
+	struct in6_addr addr, new_addr;
+
+	switch (dir) {
+	case FLOW_OFFLOAD_DIR_ORIGINAL:
+		addr = ip6h->saddr;
+		new_addr = flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple.dst_v6;
+		ip6h->saddr = new_addr;
+		break;
+	case FLOW_OFFLOAD_DIR_REPLY:
+		addr = ip6h->daddr;
+		new_addr = flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.src_v6;
+		ip6h->daddr = new_addr;
+		break;
+	default:
+		return -1;
+	}
+
+	return nf_flow_nat_ipv6_l4proto(skb, ip6h, thoff, &addr, &new_addr);
+}
+
+static int nf_flow_dnat_ipv6(const struct flow_offload *flow,
+			     struct sk_buff *skb, struct ipv6hdr *ip6h,
+			     unsigned int thoff,
+			     enum flow_offload_tuple_dir dir)
+{
+	struct in6_addr addr, new_addr;
+
+	switch (dir) {
+	case FLOW_OFFLOAD_DIR_ORIGINAL:
+		addr = ip6h->daddr;
+		new_addr = flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple.src_v6;
+		ip6h->daddr = new_addr;
+		break;
+	case FLOW_OFFLOAD_DIR_REPLY:
+		addr = ip6h->saddr;
+		new_addr = flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.dst_v6;
+		ip6h->saddr = new_addr;
+		break;
+	default:
+		return -1;
+	}
+
+	return nf_flow_nat_ipv6_l4proto(skb, ip6h, thoff, &addr, &new_addr);
+}
+
+static int nf_flow_nat_ipv6(const struct flow_offload *flow,
+			    struct sk_buff *skb,
+			    enum flow_offload_tuple_dir dir)
+{
+	struct ipv6hdr *ip6h = ipv6_hdr(skb);
+	unsigned int thoff = sizeof(*ip6h);
+
+	if (flow->flags & FLOW_OFFLOAD_SNAT &&
+	    (nf_flow_snat_port(flow, skb, thoff, ip6h->nexthdr, dir) < 0 ||
+	     nf_flow_snat_ipv6(flow, skb, ip6h, thoff, dir) < 0))
+		return -1;
+	if (flow->flags & FLOW_OFFLOAD_DNAT &&
+	    (nf_flow_dnat_port(flow, skb, thoff, ip6h->nexthdr, dir) < 0 ||
+	     nf_flow_dnat_ipv6(flow, skb, ip6h, thoff, dir) < 0))
+		return -1;
+
+	return 0;
+}
+
+static int nf_flow_tuple_ipv6(struct sk_buff *skb, const struct net_device *dev,
+			      struct flow_offload_tuple *tuple)
+{
+	struct flow_ports *ports;
+	struct ipv6hdr *ip6h;
+	unsigned int thoff;
+
+	if (!pskb_may_pull(skb, sizeof(*ip6h)))
+		return -1;
+
+	ip6h = ipv6_hdr(skb);
+
+	if (ip6h->nexthdr != IPPROTO_TCP &&
+	    ip6h->nexthdr != IPPROTO_UDP)
+		return -1;
+
+	thoff = sizeof(*ip6h);
+	if (!pskb_may_pull(skb, thoff + sizeof(*ports)))
+		return -1;
+
+	ports = (struct flow_ports *)(skb_network_header(skb) + thoff);
+
+	tuple->src_v6		= ip6h->saddr;
+	tuple->dst_v6		= ip6h->daddr;
+	tuple->src_port		= ports->source;
+	tuple->dst_port		= ports->dest;
+	tuple->l3proto		= AF_INET6;
+	tuple->l4proto		= ip6h->nexthdr;
+	tuple->iifidx		= dev->ifindex;
+
+	return 0;
+}
+
+/* Based on ip_exceeds_mtu(). */
+static bool __nf_flow_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)
+{
+	if (skb->len <= mtu)
+		return false;
+
+	if (skb_is_gso(skb) && skb_gso_validate_mtu(skb, mtu))
+		return false;
+
+	return true;
+}
+
+static bool nf_flow_exceeds_mtu(struct sk_buff *skb, const struct rt6_info *rt)
+{
+	u32 mtu;
+
+	mtu = ip6_dst_mtu_forward(&rt->dst);
+	if (__nf_flow_exceeds_mtu(skb, mtu))
+		return true;
+
+	return false;
+}
+
+static unsigned int
+nf_flow_ipv6_offload_hook(void *priv, struct sk_buff *skb,
+			 const struct nf_hook_state *state)
+{
+	struct flow_offload_tuple_rhash *tuplehash;
+	struct nf_flowtable *flow_table = priv;
+	struct flow_offload_tuple tuple = {};
+	enum flow_offload_tuple_dir dir;
+	struct flow_offload *flow;
+	struct net_device *outdev;
+	struct in6_addr *nexthop;
+	struct ipv6hdr *ip6h;
+	struct rt6_info *rt;
+
+	if (skb->protocol != htons(ETH_P_IPV6))
+		return NF_ACCEPT;
+
+	if (nf_flow_tuple_ipv6(skb, state->in, &tuple) < 0)
+		return NF_ACCEPT;
+
+	tuplehash = flow_offload_lookup(flow_table, &tuple);
+	if (tuplehash == NULL)
+		return NF_ACCEPT;
+
+	outdev = dev_get_by_index_rcu(state->net, tuplehash->tuple.oifidx);
+	if (!outdev)
+		return NF_ACCEPT;
+
+	dir = tuplehash->tuple.dir;
+	flow = container_of(tuplehash, struct flow_offload, tuplehash[dir]);
+
+	rt = (struct rt6_info *)flow->tuplehash[dir].tuple.dst_cache;
+	if (unlikely(nf_flow_exceeds_mtu(skb, rt)))
+		return NF_ACCEPT;
+
+	if (skb_try_make_writable(skb, sizeof(*ip6h)))
+		return NF_DROP;
+
+	if (flow->flags & (FLOW_OFFLOAD_SNAT | FLOW_OFFLOAD_DNAT) &&
+	    nf_flow_nat_ipv6(flow, skb, dir) < 0)
+		return NF_DROP;
+
+	flow->timeout = (u32)jiffies + NF_FLOW_TIMEOUT;
+	ip6h = ipv6_hdr(skb);
+	ip6h->hop_limit--;
+
+	skb->dev = outdev;
+	nexthop = rt6_nexthop(rt, &flow->tuplehash[!dir].tuple.src_v6);
+	neigh_xmit(NEIGH_ND_TABLE, outdev, &nexthop, skb);
+
+	return NF_STOLEN;
+}
+
+static struct nf_flowtable_type flowtable_ipv6 = {
+	.family		= NFPROTO_IPV6,
+	.params		= &nf_flow_offload_rhash_params,
+	.gc		= nf_flow_offload_work_gc,
+	.hook		= nf_flow_ipv6_offload_hook,
+	.owner		= THIS_MODULE,
+};
+
+static int __init nf_flow_ipv6_module_init(void)
+{
+	nft_register_flowtable_type(&flowtable_ipv6);
+
+	return 0;
+}
+
+static void __exit nf_flow_ipv6_module_exit(void)
+{
+	nft_unregister_flowtable_type(&flowtable_ipv6);
+}
+
+module_init(nf_flow_ipv6_module_init);
+module_exit(nf_flow_ipv6_module_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Pablo Neira Ayuso <pablo@netfilter.org>");
+MODULE_ALIAS_NF_FLOWTABLE(AF_INET6);
-- 
2.11.0


^ permalink raw reply related

* [PATCH nf-next,v3 0/7] Flow offload infrastructure
From: Pablo Neira Ayuso @ 2017-12-22 19:27 UTC (permalink / raw)
  To: netfilter-devel
  Cc: netdev, f.fainelli, simon.horman, ronye, jiri, nbd, john,
	kubakici, fw

Hi,

This is a new round of the patchset to add the flow offload
infrastructure [1][2].

This round comes with IPv6 and mixed IPv4/IPv6 support, hardware offload
support in a separated nf_flow_table_hw module, port translation, net
namespace support and several bugfixes.

Patch 7/7 has been tagged as RFC, I will keep this one back until
there's an initial driver that introduces flow table offload support,
likely in a branch in nf-next.git once this new infrastructure gets
merged upstream.

Comments welcome, thanks.

[1] https://lwn.net/Articles/738214/
[2] https://marc.info/?l=netfilter-devel&m=151266258119014&w=2

Pablo Neira Ayuso (7):
  netfilter: nf_tables: add flow table netlink frontend
  netfilter: add generic flow table infrastructure
  netfilter: flow table support for IPv4
  netfilter: flow table support for IPv6
  netfilter: flow table support for the mixed IPv4/IPv6 family
  netfilter: nf_tables: flow offload expression
  netfilter: nf_flow_table: add hardware offload support

 include/linux/netdevice.h                |   9 +
 include/net/ipv6.h                       |   2 +
 include/net/netfilter/nf_flow_table.h    | 128 ++++++
 include/net/netfilter/nf_tables.h        |  48 ++
 include/uapi/linux/netfilter/nf_tables.h |  64 +++
 net/ipv4/netfilter/Kconfig               |   8 +
 net/ipv4/netfilter/Makefile              |   3 +
 net/ipv4/netfilter/nf_flow_table_ipv4.c  | 284 ++++++++++++
 net/ipv6/ip6_output.c                    |   3 +-
 net/ipv6/netfilter/Kconfig               |   8 +
 net/ipv6/netfilter/Makefile              |   3 +
 net/ipv6/netfilter/nf_flow_table_ipv6.c  | 278 ++++++++++++
 net/netfilter/Kconfig                    |  31 ++
 net/netfilter/Makefile                   |   5 +
 net/netfilter/nf_flow_table.c            | 447 ++++++++++++++++++
 net/netfilter/nf_flow_table_hw.c         | 127 ++++++
 net/netfilter/nf_flow_table_inet.c       |  48 ++
 net/netfilter/nf_tables_api.c            | 749 ++++++++++++++++++++++++++++++-
 net/netfilter/nft_flow_offload.c         | 272 +++++++++++
 19 files changed, 2515 insertions(+), 2 deletions(-)
 create mode 100644 include/net/netfilter/nf_flow_table.h
 create mode 100644 net/ipv4/netfilter/nf_flow_table_ipv4.c
 create mode 100644 net/ipv6/netfilter/nf_flow_table_ipv6.c
 create mode 100644 net/netfilter/nf_flow_table.c
 create mode 100644 net/netfilter/nf_flow_table_hw.c
 create mode 100644 net/netfilter/nf_flow_table_inet.c
 create mode 100644 net/netfilter/nft_flow_offload.c

-- 
2.11.0


^ permalink raw reply

* Re: [PATCH net] rtnetlink: fix struct net reference leak
From: Craig Gallek @ 2017-12-22 19:14 UTC (permalink / raw)
  To: Nicolas Dichtel; +Cc: David Miller, Jiri Benc, netdev, Jason A . Donenfeld
In-Reply-To: <CAEfhGiztjNZcoeOC7SGCYqf8abUAsg9kgLTTYTQSPOWcGnSeWw@mail.gmail.com>

On Fri, Dec 22, 2017 at 8:59 AM, Craig Gallek <kraigatgoog@gmail.com> wrote:
> On Fri, Dec 22, 2017 at 3:11 AM, Nicolas Dichtel
> <nicolas.dichtel@6wind.com> wrote:
>> Le 21/12/2017 à 23:18, Craig Gallek a écrit :
>>> From: Craig Gallek <kraig@google.com>
>>>
>>> The below referenced commit extended the RTM_GETLINK interface to
>>> allow querying by netns id.  The netnsid property was previously
>>> defined as a signed integer, but this patch assumes that the user
>>> always passes a positive integer.  syzkaller discovered this problem
>>> by setting a negative netnsid and then calling the get-link path
>>> in a tight loop.  This surprisingly quickly overflows the reference
>>> count on the associated struct net, potentially destroying it.  When the
>>> default namespace is used, the machine crashes in strange and interesting
>>> ways.
>>>
>>> Unfortunately, this is not easy to reproduce with just the ip tool
>>> as it enforces unsigned integer parsing despite the interface interpeting
>>> the NETNSID attribute as signed.
>>>
>>> I'm not sure why this attribute is signed in the first place, but
>>> the first commit that introduced it (6621dd29eb9b) is in v4.15-rc4,
>>> so I assume it's too late to change.
>> A valid (assigned) nsid is always >= 0.
>>
>>>
>>> This patch removes the positive netns id assumption, but adds another
>>> assumption that the netns id 0 is always the 'self' identifying id (for
>>> which an additional struct net reference is not necessary).
>> We cannot make this assumption, this is wrong. nsids may be automatically
>> allocated by the kernel, and it starts by 0.
>> The current netns can be identify by NETNSA_NSID_NOT_ASSIGNED, ie -1.
> Thank you, I'll respin this with NETNSA_NSID_NOT_ASSIGNED as the sentinel value.

Looking at the netns id code more closely, there are several places
that assume ids will never be zero (short of the sentinel).  I think
the only simple fix here is to update the netlink interfaces to not
accept negative values as input.  I'm going to send that patch
instead...

^ permalink raw reply

* Re: [PATCH] bpf: selftest for late caller stack size increase
From: Alexei Starovoitov @ 2017-12-22 19:11 UTC (permalink / raw)
  To: Jann Horn; +Cc: Alexei Starovoitov, Daniel Borkmann, netdev, linux-kernel
In-Reply-To: <20171222181235.158636-1-jannh@google.com>

On Fri, Dec 22, 2017 at 07:12:35PM +0100, Jann Horn wrote:
> This checks that it is not possible to bypass the total stack size check in
> update_stack_depth() by calling a function that uses a large amount of
> stack memory *before* using a large amount of stack memory in the caller.
> 
> Currently, the first added testcase causes a rejection as expected, but
> the second testcase is (AFAICS incorrectly) accepted:
> 
> [...]
> #483/p calls: stack overflow using two frames (post-call access) FAIL
> Unexpected success to load!
> 0: (85) call pc+2
> caller:
>  R10=fp0,call_-1
> callee:
>  frame1: R1=ctx(id=0,off=0,imm=0) R10=fp0,call_0
> 3: (72) *(u8 *)(r10 -300) = 0
> 4: (b7) r0 = 0
> 5: (95) exit
> returning from callee:
>  frame1: R0_w=inv0 R1=ctx(id=0,off=0,imm=0) R10=fp0,call_0
> to caller at 1:
>  R0_w=inv0 R10=fp0,call_-1
> 
> from 5 to 1: R0=inv0 R10=fp0,call_-1
> 1: (72) *(u8 *)(r10 -300) = 0
> 2: (95) exit
> processed 6 insns, stack depth 300+300

got it. thanks for the test!
working on a fix.

^ permalink raw reply

* Re: INFO: task hung in bpf_exit_net
From: Marcelo Ricardo Leitner @ 2017-12-22 19:04 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: David Ahern, syzbot, LKML, Ingo Molnar, Peter Zijlstra,
	syzkaller-bugs, David Miller, Florian Westphal, Daniel Borkmann,
	Xin Long, jakub.kicinski, mschiffer, Vladislav Yasevich,
	Jiri Benc, netdev, Neil Horman, linux-sctp
In-Reply-To: <20171222182807.GC32765@localhost.localdomain>

On Fri, Dec 22, 2017 at 04:28:07PM -0200, Marcelo Ricardo Leitner wrote:
> On Fri, Dec 22, 2017 at 11:58:08AM +0100, Dmitry Vyukov wrote:
> ...
> > > Same with this one, perhaps related to / fixed by:
> > >     http://patchwork.ozlabs.org/patch/850957/
> > >
> > 
> > 
> > 
> > Looking at the log, this one seems to be an infinite loop in SCTP code
> > with console output in it. Kernel is busy printing gazilion of:
> > 
> > [  176.491099] sctp: sctp_transport_update_pmtu: Reported pmtu 508 too
> > low, using default minimum of 512
> > ** 110 printk messages dropped **
> > [  176.503409] sctp: sctp_transport_update_pmtu: Reported pmtu 508 too
> > low, using default minimum of 512
> > ** 103 printk messages dropped **
> > ...
> > [  246.742374] sctp: sctp_transport_update_pmtu: Reported pmtu 508 too
> > low, using default minimum of 512
> > [  246.742484] sctp: sctp_transport_update_pmtu: Reported pmtu 508 too
> > low, using default minimum of 512
> > [  246.742590] sctp: sctp_transport_update_pmtu: Reported pmtu 508 too
> > low, using default minimum of 512
> > 
> > Looks like a different issue.
> > 
> 
> Oh. I guess this is caused by the interface having a MTU smaller than
> SCTP_DEFAULT_MINSEGMENT (512), as the icmp frag needed handler
> (sctp_icmp_frag_needed) will trigger an instant retransmission.
> But as the MTU is smaller, SCTP won't update it, but will issue the
> retransmission anyway.
> 
> I will test this soon. Should be fairly easy to trigger it.

Reproduced it.

netns A veth0(1500) - veth1(1500) B veth2(508) - veth3(508) C

When A sends a sctp packet bigger than 508, it triggers the issue as B
will reply a icmp frag needed with a size that sctp won't accept but
will retransmit anyway.

  Marcelo

^ permalink raw reply

* Re: [PATCH v2 bpf-next 2/2] tools/bpftool: fix bpftool build with bintutils >= 2.8
From: Quentin Monnet @ 2017-12-22 18:50 UTC (permalink / raw)
  To: Roman Gushchin, netdev
  Cc: linux-kernel, kernel-team, Jakub Kicinski, Alexei Starovoitov,
	Daniel Borkmann
In-Reply-To: <20171222161152.24715-2-guro@fb.com>

Hi Roman,

2017-12-22 16:11 UTC+0000 ~ Roman Gushchin <guro@fb.com>
> Bpftool build is broken with binutils version 2.28 and later.

Could you check the binutils version? I believe it changed in 2.29
instead of 2.28. Could you update your commit log and subject
accordingly, please?

> The cause is commit 003ca0fd2286 ("Refactor disassembler selection")
> in the binutils repo, which changed the disassembler() function
> signature.
> 
> Fix this by adding a new "feature" to the tools/build/features
> infrastructure and make it responsible for decision which
> disassembler() function signature to use.
> 
> Signed-off-by: Roman Gushchin <guro@fb.com>
> Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> ---
>  tools/bpf/Makefile                                | 29 +++++++++++++++++++++++
>  tools/bpf/bpf_jit_disasm.c                        |  7 ++++++
>  tools/bpf/bpftool/Makefile                        | 24 +++++++++++++++++++
>  tools/bpf/bpftool/jit_disasm.c                    |  7 ++++++
>  tools/build/feature/Makefile                      |  4 ++++
>  tools/build/feature/test-disassembler-four-args.c | 15 ++++++++++++
>  6 files changed, 86 insertions(+)
>  create mode 100644 tools/build/feature/test-disassembler-four-args.c
> 
> diff --git a/tools/bpf/Makefile b/tools/bpf/Makefile
> index 07a6697466ef..c8ec0ae16bf0 100644
> --- a/tools/bpf/Makefile
> +++ b/tools/bpf/Makefile
> @@ -9,6 +9,35 @@ MAKE = make
>  CFLAGS += -Wall -O2
>  CFLAGS += -D__EXPORTED_HEADERS__ -I../../include/uapi -I../../include
>  
> +ifeq ($(srctree),)
> +srctree := $(patsubst %/,%,$(dir $(CURDIR)))
> +srctree := $(patsubst %/,%,$(dir $(srctree)))
> +endif
> +
> +FEATURE_USER = .bpf
> +FEATURE_TESTS = libbfd disassembler-four-args
> +FEATURE_DISPLAY = libbfd disassembler-four-args

Thanks for adding libbfd as I requested. However, you do not use it in
the Makefile to prevent compilation if the feature is not detected (see
"bpfdep" or "elfdep" in tools/lib/bpf/Makefile. Sorry, I should have
pointed it in my previous review.

But actually, I have another issue related to the libbfd feature: since
commit 280e7c48c3b8 ("perf tools: fix BFD detection on opensuse") it
requires libiberty so that libbfd is correctly detected, but libiberty
is not needed on all distros (at least Ubuntu can have libbfd without
libiberty). Typically, detection fails on my setup, although I do have
libbfd installed. So forcing libbfd feature here may eventually force
users to install libraries they do not need to compile bpftool, which is
not what we want.

I do not have a clean work around to suggest. Maybe have one
"libbfd-something" feature that tries to compile without libiberty, then
another one that tries with it, and compile the tools if at least one of
them succeeds. But it's probably for another patch series. In the
meantime, would you please simply remove libbfd detection here and
accept my apologies for suggesting to add it in the previous review?

> +
> +check_feat := 1
> +NON_CHECK_FEAT_TARGETS := clean bpftool_clean
> +ifdef MAKECMDGOALS
> +ifeq ($(filter-out $(NON_CHECK_FEAT_TARGETS),$(MAKECMDGOALS)),)
> +  check_feat := 0
> +endif
> +endif
> +
> +ifeq ($(check_feat),1)
> +ifeq ($(FEATURES_DUMP),)
> +include $(srctree)/tools/build/Makefile.feature
> +else
> +include $(FEATURES_DUMP)
> +endif
> +endif
> +
> +ifeq ($(feature-disassembler-four-args), 1)
> +CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE
> +endif
> +
>  %.yacc.c: %.y
>  	$(YACC) -o $@ -d $<
>  
> diff --git a/tools/bpf/bpf_jit_disasm.c b/tools/bpf/bpf_jit_disasm.c
> index 75bf526a0168..30044bc4f389 100644
> --- a/tools/bpf/bpf_jit_disasm.c
> +++ b/tools/bpf/bpf_jit_disasm.c
> @@ -72,7 +72,14 @@ static void get_asm_insns(uint8_t *image, size_t len, int opcodes)
>  
>  	disassemble_init_for_target(&info);
>  
> +#ifdef DISASM_FOUR_ARGS_SIGNATURE
> +	disassemble = disassembler(info.arch,
> +				   bfd_big_endian(bfdf),
> +				   info.mach,
> +				   bfdf);
> +#else
>  	disassemble = disassembler(bfdf);
> +#endif
>  	assert(disassemble);
>  
>  	do {
> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> index f8f31a8d9269..2237bc43f71c 100644
> --- a/tools/bpf/bpftool/Makefile
> +++ b/tools/bpf/bpftool/Makefile
> @@ -46,6 +46,30 @@ LIBS = -lelf -lbfd -lopcodes $(LIBBPF)
>  INSTALL ?= install
>  RM ?= rm -f
>  
> +FEATURE_USER = .bpftool
> +FEATURE_TESTS = libbfd disassembler-four-args
> +FEATURE_DISPLAY = libbfd disassembler-four-args
> +
> +check_feat := 1
> +NON_CHECK_FEAT_TARGETS := clean uninstall doc doc-clean doc-install doc-uninstall

Nit: exclude "install" as well? I know libbpf does not exclude it, but
if the user runs `make` then `make install` we do not need to check
again the features for binary installation.

> +ifdef MAKECMDGOALS
> +ifeq ($(filter-out $(NON_CHECK_FEAT_TARGETS),$(MAKECMDGOALS)),)
> +  check_feat := 0
> +endif
> +endif
> +
> +ifeq ($(check_feat),1)
> +ifeq ($(FEATURES_DUMP),)
> +include $(srctree)/tools/build/Makefile.feature
> +else
> +include $(FEATURES_DUMP)
> +endif
> +endif
> +
> +ifeq ($(feature-disassembler-four-args), 1)
> +CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE
> +endif
> +
>  include $(wildcard *.d)
>  
>  all: $(OUTPUT)bpftool
> diff --git a/tools/bpf/bpftool/jit_disasm.c b/tools/bpf/bpftool/jit_disasm.c
> index 1551d3918d4c..57d32e8a1391 100644
> --- a/tools/bpf/bpftool/jit_disasm.c
> +++ b/tools/bpf/bpftool/jit_disasm.c
> @@ -107,7 +107,14 @@ void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes)
>  
>  	disassemble_init_for_target(&info);
>  
> +#ifdef DISASM_FOUR_ARGS_SIGNATURE
> +	disassemble = disassembler(info.arch,
> +				   bfd_big_endian(bfdf),
> +				   info.mach,
> +				   bfdf);
> +#else
>  	disassemble = disassembler(bfdf);
> +#endif
>  	assert(disassemble);
>  
>  	if (json_output)
> diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
> index 96982640fbf8..17f2c73fff8b 100644
> --- a/tools/build/feature/Makefile
> +++ b/tools/build/feature/Makefile
> @@ -13,6 +13,7 @@ FILES=                                          \
>           test-hello.bin                         \
>           test-libaudit.bin                      \
>           test-libbfd.bin                        \
> +         test-disassembler-four-args.bin        \
>           test-liberty.bin                       \
>           test-liberty-z.bin                     \
>           test-cplus-demangle.bin                \
> @@ -188,6 +189,9 @@ $(OUTPUT)test-libpython-version.bin:
>  $(OUTPUT)test-libbfd.bin:
>  	$(BUILD) -DPACKAGE='"perf"' -lbfd -lz -liberty -ldl
>  
> +$(OUTPUT)test-disassembler-four-args.bin:
> +	$(BUILD) -lbfd -lopcodes
> +
>  $(OUTPUT)test-liberty.bin:
>  	$(CC) $(CFLAGS) -Wall -Werror -o $@ test-libbfd.c -DPACKAGE='"perf"' $(LDFLAGS) -lbfd -ldl -liberty
>  
> diff --git a/tools/build/feature/test-disassembler-four-args.c b/tools/build/feature/test-disassembler-four-args.c
> new file mode 100644
> index 000000000000..45ce65cfddf0
> --- /dev/null
> +++ b/tools/build/feature/test-disassembler-four-args.c
> @@ -0,0 +1,15 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <bfd.h>
> +#include <dis-asm.h>
> +
> +int main(void)
> +{
> +	bfd *abfd = bfd_openr(NULL, NULL);
> +
> +	disassembler(bfd_get_arch(abfd),
> +		     bfd_big_endian(abfd),
> +		     bfd_get_mach(abfd),
> +		     abfd);
> +
> +	return 0;
> +}
> 

The rest of the patch looks fine to me, thanks for addressing all my
comments!

Quentin

^ permalink raw reply

* Re: INFO: task hung in bpf_exit_net
From: Marcelo Ricardo Leitner @ 2017-12-22 18:28 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: David Ahern, syzbot, LKML, Ingo Molnar, Peter Zijlstra,
	syzkaller-bugs, David Miller, Florian Westphal, Daniel Borkmann,
	Xin Long, jakub.kicinski, mschiffer, Vladislav Yasevich,
	Jiri Benc, netdev, Neil Horman, linux-sctp
In-Reply-To: <CACT4Y+aaSX4S3KHJjqkiQhhOZAtEN_fMD1m_Ve3rz4u4x9KSWg@mail.gmail.com>

On Fri, Dec 22, 2017 at 11:58:08AM +0100, Dmitry Vyukov wrote:
...
> > Same with this one, perhaps related to / fixed by:
> >     http://patchwork.ozlabs.org/patch/850957/
> >
> 
> 
> 
> Looking at the log, this one seems to be an infinite loop in SCTP code
> with console output in it. Kernel is busy printing gazilion of:
> 
> [  176.491099] sctp: sctp_transport_update_pmtu: Reported pmtu 508 too
> low, using default minimum of 512
> ** 110 printk messages dropped **
> [  176.503409] sctp: sctp_transport_update_pmtu: Reported pmtu 508 too
> low, using default minimum of 512
> ** 103 printk messages dropped **
> ...
> [  246.742374] sctp: sctp_transport_update_pmtu: Reported pmtu 508 too
> low, using default minimum of 512
> [  246.742484] sctp: sctp_transport_update_pmtu: Reported pmtu 508 too
> low, using default minimum of 512
> [  246.742590] sctp: sctp_transport_update_pmtu: Reported pmtu 508 too
> low, using default minimum of 512
> 
> Looks like a different issue.
> 

Oh. I guess this is caused by the interface having a MTU smaller than
SCTP_DEFAULT_MINSEGMENT (512), as the icmp frag needed handler
(sctp_icmp_frag_needed) will trigger an instant retransmission.
But as the MTU is smaller, SCTP won't update it, but will issue the
retransmission anyway.

I will test this soon. Should be fairly easy to trigger it.

  Marcelo

^ permalink raw reply

* Re: [PATCH V2 net-next 3/3] rds: tcp: cleanup if kmem_cache_alloc fails in rds_tcp_conn_alloc()
From: Santosh Shilimkar @ 2017-12-22 18:17 UTC (permalink / raw)
  To: Sowmini Varadhan, netdev; +Cc: davem, rds-devel
In-Reply-To: <8356144d4426672ea85d0f671fa1f7f6a0b7f768.1513962765.git.sowmini.varadhan@oracle.com>

On 12/22/2017 9:39 AM, Sowmini Varadhan wrote:
> If kmem_cache_alloc() fails in the middle of the for() loop,
> cleanup anything that might have been allocated so far.
> 
> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
> ---
> v2: target net-next, not net
> 
Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>

^ permalink raw reply

* [PATCH v2] sctp: Replace use of sockets_allocated with specified macro.
From: Tonghao Zhang @ 2017-12-22 18:15 UTC (permalink / raw)
  To: netdev; +Cc: eric.dumazet, davem, Tonghao Zhang

The patch(180d8cd942ce) replaces all uses of struct sock fields'
memory_pressure, memory_allocated, sockets_allocated, and sysctl_mem
to accessor macros. But the sockets_allocated field of sctp sock is
not replaced at all. Then replace it now for unifying the code.

Fixes: 180d8cd942ce ("foundations of per-cgroup memory pressure controlling.")
Cc: Glauber Costa <glommer@parallels.com>
Signed-off-by: Tonghao Zhang <zhangtonghao@didichuxing.com>
---
fix typo.
---
 net/sctp/socket.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index aadcd4244d9b..a5e2150ab013 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -4569,7 +4569,7 @@ static int sctp_init_sock(struct sock *sk)
 	SCTP_DBG_OBJCNT_INC(sock);
 
 	local_bh_disable();
-	percpu_counter_inc(&sctp_sockets_allocated);
+	sk_sockets_allocated_inc(sk);
 	sock_prot_inuse_add(net, sk->sk_prot, 1);
 
 	/* Nothing can fail after this block, otherwise
@@ -4613,7 +4613,7 @@ static void sctp_destroy_sock(struct sock *sk)
 	}
 	sctp_endpoint_free(sp->ep);
 	local_bh_disable();
-	percpu_counter_dec(&sctp_sockets_allocated);
+	sk_sockets_allocated_dec(sk);
 	sock_prot_inuse_add(sock_net(sk), sk->sk_prot, -1);
 	local_bh_enable();
 }
-- 
2.13.6

^ permalink raw reply related

* Re: [PATCH V2 net-next 2/3] rds: tcp: initialize t_tcp_detached to false
From: Santosh Shilimkar @ 2017-12-22 18:15 UTC (permalink / raw)
  To: Sowmini Varadhan, netdev; +Cc: davem, rds-devel
In-Reply-To: <0500544d8e065679e6e31c4ad2793758d2160aba.1513962765.git.sowmini.varadhan@oracle.com>

On 12/22/2017 9:39 AM, Sowmini Varadhan wrote:
> Commit f10b4cff98c6 ("rds: tcp: atomically purge entries from
> rds_tcp_conn_list during netns delete") adds the field t_tcp_detached,
> but this needs to be initialized explicitly to false.
> 
> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
> ---
> v2: target net-next, not net
> 
Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>

^ permalink raw reply

* Re: correctness of BPF stack size checking logic for multi-function programs?
From: Jann Horn @ 2017-12-22 18:15 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Alexei Starovoitov, Daniel Borkmann, kernel list,
	Network Development
In-Reply-To: <20171222033744.ahdcgq6m56jbrgth@ast-mbp>

On Fri, Dec 22, 2017 at 4:37 AM, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> On Fri, Dec 22, 2017 at 02:14:45AM +0100, Jann Horn wrote:
>> Hi!
>>
>> I saw the recently-added support for multiple functions in a single
>> program in BPF. I've stumbled over something that looks like it might
>> be a bug; I haven't verified it yet, but I thought I should give you a
>> heads-up before this lands in a release in case I'm right. If I'm
>> wrong, it might be worth adding a comment to stacksafe() that explains
>> why.
[...]
> but I will rewrite a test case for it unless you beat me to it :)

I just sent a failing test case for the case I'm talking about, subject
"[PATCH] bpf: selftest for late caller stack size increase".

^ permalink raw reply

* Re: [PATCH V2 net-next 1/3] rds; Reset rs->rs_bound_addr in rds_add_bound() failure path
From: Santosh Shilimkar @ 2017-12-22 18:15 UTC (permalink / raw)
  To: Sowmini Varadhan, netdev; +Cc: davem, rds-devel
In-Reply-To: <c1267c64f578141e16a544e6f1d792acbb1829d8.1513962765.git.sowmini.varadhan@oracle.com>

On 12/22/2017 9:38 AM, Sowmini Varadhan wrote:
> If the rds_sock is not added to the bind_hash_table, we must
> reset rs_bound_addr so that rds_remove_bound will not trip on
> this rds_sock.
> 
> rds_add_bound() does a rds_sock_put() in this failure path, so
> failing to reset rs_bound_addr will result in a socket refcount
> bug, and will trigger a WARN_ON with the stack shown below when
> the application subsequently tries to close the PF_RDS socket.
> 
>       WARNING: CPU: 20 PID: 19499 at net/rds/af_rds.c:496 \
> 		rds_sock_destruct+0x15/0x30 [rds]
>         :
>       __sk_destruct+0x21/0x190
>       rds_remove_bound.part.13+0xb6/0x140 [rds]
>       rds_release+0x71/0x120 [rds]
>       sock_release+0x1a/0x70
>       sock_close+0xe/0x20
>       __fput+0xd5/0x210
>       task_work_run+0x82/0xa0
>       do_exit+0x2ce/0xb30
>       ? syscall_trace_enter+0x1cc/0x2b0
>       do_group_exit+0x39/0xa0
>       SyS_exit_group+0x10/0x10
>       do_syscall_64+0x61/0x1a0
> 
> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
> ---
> v2: target net-next, not net
> 
Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>

^ permalink raw reply

* Re: [PATCH net-next v5 0/5] Introduce NETIF_F_GRO_HW
From: Alexander Duyck @ 2017-12-22 18:14 UTC (permalink / raw)
  To: Sabrina Dubroca; +Cc: Michael Chan, David Miller, Netdev, Andrew Gospodarek
In-Reply-To: <20171222145758.GA685@bistromath.localdomain>

On Fri, Dec 22, 2017 at 6:57 AM, Sabrina Dubroca <sd@queasysnail.net> wrote:
> Hello,
>
> Sorry for commenting late.
>
> 2017-12-16, 03:09:39 -0500, Michael Chan wrote:
>> Introduce NETIF_F_GRO_HW feature flag and convert drivers that support
>> hardware GRO to use the new flag.
>>
>> v5:
>> - Documentation changes requested by Alexander Duyck.
>> - bnx2x changes requested by Manish Chopra to enable LRO by default, and
>> disable GRO_HW if disable_tpa module parameter is set.
>>
>> v4:
>> - more changes requested by Alexander Duyck:
>> - check GRO_HW/GRO dependency in drivers's ndo_fix_features().
>> - Reverse the order of RXCSUM and GRO_HW dependency check in
>> netdev_fix_features().
>> - No propagation in netdev_disable_gro_hw().
>
> IIUC, with the patches that were applied, each driver can define
> whether GRO_HW depends on GRO? From a user's perspective, this
> inconsistent behavior is going to be quite confusing.
>
> Worse than inconsistent behavior, it looks like a driver deciding that
> GRO_HW doesn't depend on GRO is going to introduce a change of
> behavior.  Previously, when GRO was disabled, there wouldn't be any
> packet over MTU handed to the network stack.  Now, even if GRO is
> disabled, GRO_HW might still be enabled, so we might get over-MTU
> packets because of hardware GRO.

This isn't actually true. LRO was still handling packets larger than
MTU over even when GRO was disabled.

> I don't think drivers should be allowed to say "GRO_HW doesn't depend
> on GRO".

Why not, it doesn't. In my mind GRO_HW is closer to LRO than it is to
GRO. The only ugly bit as I see it is that these devices were exposing
the feature via the GRO flag in the first place. So for the sake of
legacy they might want to carry around the dependency.

> I think it's reasonable to be able to disable software GRO even if
> hardware GRO is enabled. Thus, I would propose:
> - keep the current GRO flag
> - add a GRO_HW flag, depending on GRO, enforced by the core as in
>   earlier versions of these patches
> - add a GRO_SW flag, also depending on GRO

This seems like a bunch of extra overhead for not much gain. Do we
really need to fork GRO into 3 bits? I would argue that GRO_HW really
should have been branded something like FORWARDABLE_LRO, but nobody
wanted to touch the name LRO since it apparently has some negative
stigma to it. If we had used a name like that we probably wouldn't be
going through all these extra hoops. The only real reason why this is
even being associated with GRO in the first place is that is how this
feature was hidden by the drivers so they got around having to deal
with the LRO being disabled for routing/forwarding issue. Those are
the parts that want to keep it associated with GRO since that is how
they exposed it in their devices originally.

- Alex

^ permalink raw reply

* [PATCH] bpf: selftest for late caller stack size increase
From: Jann Horn @ 2017-12-22 18:12 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann; +Cc: netdev, linux-kernel

This checks that it is not possible to bypass the total stack size check in
update_stack_depth() by calling a function that uses a large amount of
stack memory *before* using a large amount of stack memory in the caller.

Currently, the first added testcase causes a rejection as expected, but
the second testcase is (AFAICS incorrectly) accepted:

[...]
#483/p calls: stack overflow using two frames (post-call access) FAIL
Unexpected success to load!
0: (85) call pc+2
caller:
 R10=fp0,call_-1
callee:
 frame1: R1=ctx(id=0,off=0,imm=0) R10=fp0,call_0
3: (72) *(u8 *)(r10 -300) = 0
4: (b7) r0 = 0
5: (95) exit
returning from callee:
 frame1: R0_w=inv0 R1=ctx(id=0,off=0,imm=0) R10=fp0,call_0
to caller at 1:
 R0_w=inv0 R10=fp0,call_-1

from 5 to 1: R0=inv0 R10=fp0,call_-1
1: (72) *(u8 *)(r10 -300) = 0
2: (95) exit
processed 6 insns, stack depth 300+300
[...]
Summary: 704 PASSED, 1 FAILED

AFAICS the JIT-generated code for the second testcase shows that this
really causes the stack pointer to be decremented by 300+300:

first function:
00000000  55                push rbp
00000001  4889E5            mov rbp,rsp
00000004  4881EC58010000    sub rsp,0x158
0000000B  4883ED28          sub rbp,byte +0x28
[...]
00000025  E89AB3AFE5        call 0xffffffffe5afb3c4
0000002A  C685D4FEFFFF00    mov byte [rbp-0x12c],0x0
[...]
00000041  4883C528          add rbp,byte +0x28
00000045  C9                leave
00000046  C3                ret

second function:
00000000  55                push rbp
00000001  4889E5            mov rbp,rsp
00000004  4881EC58010000    sub rsp,0x158
0000000B  4883ED28          sub rbp,byte +0x28
[...]
00000025  C685D4FEFFFF00    mov byte [rbp-0x12c],0x0
[...]
0000003E  4883C528          add rbp,byte +0x28
00000042  C9                leave
00000043  C3                ret

Signed-off-by: Jann Horn <jannh@google.com>
---
 tools/testing/selftests/bpf/test_verifier.c | 34 +++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 3bacff0d6f91..71fb0be81b78 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -8729,6 +8729,40 @@ static struct bpf_test tests[] = {
 		.prog_type = BPF_PROG_TYPE_XDP,
 		.result = ACCEPT,
 	},
+	{
+		"calls: stack overflow using two frames (pre-call access)",
+		.insns = {
+			/* prog 1 */
+			BPF_ST_MEM(BPF_B, BPF_REG_10, -300, 0),
+			BPF_RAW_INSN(BPF_JMP|BPF_CALL, 0, 1, 0, 1),
+			BPF_EXIT_INSN(),
+
+			/* prog 2 */
+			BPF_ST_MEM(BPF_B, BPF_REG_10, -300, 0),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+		},
+		.prog_type = BPF_PROG_TYPE_XDP,
+		.errstr = "combined stack size",
+		.result = REJECT,
+	},
+	{
+		"calls: stack overflow using two frames (post-call access)",
+		.insns = {
+			/* prog 1 */
+			BPF_RAW_INSN(BPF_JMP|BPF_CALL, 0, 1, 0, 2),
+			BPF_ST_MEM(BPF_B, BPF_REG_10, -300, 0),
+			BPF_EXIT_INSN(),
+
+			/* prog 2 */
+			BPF_ST_MEM(BPF_B, BPF_REG_10, -300, 0),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+		},
+		.prog_type = BPF_PROG_TYPE_XDP,
+		.errstr = "combined stack size",
+		.result = REJECT,
+	},
 	{
 		"calls: spill into caller stack frame",
 		.insns = {
-- 
2.15.1.620.gb9897f4670-goog

^ permalink raw reply related

* [PATCH V2 net-next 3/3] rds: tcp: cleanup if kmem_cache_alloc fails in rds_tcp_conn_alloc()
From: Sowmini Varadhan @ 2017-12-22 17:39 UTC (permalink / raw)
  To: netdev; +Cc: davem, rds-devel, sowmini.varadhan, santosh.shilimkar
In-Reply-To: <cover.1513962765.git.sowmini.varadhan@oracle.com>

If kmem_cache_alloc() fails in the middle of the for() loop,
cleanup anything that might have been allocated so far.

Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
---
v2: target net-next, not net

 net/rds/tcp.c |   46 ++++++++++++++++++++++++++--------------------
 1 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/net/rds/tcp.c b/net/rds/tcp.c
index a61a498..2e554ef 100644
--- a/net/rds/tcp.c
+++ b/net/rds/tcp.c
@@ -270,16 +270,33 @@ static int rds_tcp_laddr_check(struct net *net, __be32 addr)
 	return -EADDRNOTAVAIL;
 }
 
+static void rds_tcp_conn_free(void *arg)
+{
+	struct rds_tcp_connection *tc = arg;
+	unsigned long flags;
+
+	rdsdebug("freeing tc %p\n", tc);
+
+	spin_lock_irqsave(&rds_tcp_conn_lock, flags);
+	if (!tc->t_tcp_node_detached)
+		list_del(&tc->t_tcp_node);
+	spin_unlock_irqrestore(&rds_tcp_conn_lock, flags);
+
+	kmem_cache_free(rds_tcp_conn_slab, tc);
+}
+
 static int rds_tcp_conn_alloc(struct rds_connection *conn, gfp_t gfp)
 {
 	struct rds_tcp_connection *tc;
-	int i;
+	int i, j;
+	int ret = 0;
 
 	for (i = 0; i < RDS_MPATH_WORKERS; i++) {
 		tc = kmem_cache_alloc(rds_tcp_conn_slab, gfp);
-		if (!tc)
-			return -ENOMEM;
-
+		if (!tc) {
+			ret = -ENOMEM;
+			break;
+		}
 		mutex_init(&tc->t_conn_path_lock);
 		tc->t_sock = NULL;
 		tc->t_tinc = NULL;
@@ -296,22 +313,11 @@ static int rds_tcp_conn_alloc(struct rds_connection *conn, gfp_t gfp)
 		rdsdebug("rds_conn_path [%d] tc %p\n", i,
 			 conn->c_path[i].cp_transport_data);
 	}
-
-	return 0;
-}
-
-static void rds_tcp_conn_free(void *arg)
-{
-	struct rds_tcp_connection *tc = arg;
-	unsigned long flags;
-	rdsdebug("freeing tc %p\n", tc);
-
-	spin_lock_irqsave(&rds_tcp_conn_lock, flags);
-	if (!tc->t_tcp_node_detached)
-		list_del(&tc->t_tcp_node);
-	spin_unlock_irqrestore(&rds_tcp_conn_lock, flags);
-
-	kmem_cache_free(rds_tcp_conn_slab, tc);
+	if (ret) {
+		for (j = 0; j < i; j++)
+			rds_tcp_conn_free(conn->c_path[j].cp_transport_data);
+	}
+	return ret;
 }
 
 static bool list_has_conn(struct list_head *list, struct rds_connection *conn)
-- 
1.7.1

^ permalink raw reply related

* [PATCH V2 net-next 1/3] rds; Reset rs->rs_bound_addr in rds_add_bound() failure path
From: Sowmini Varadhan @ 2017-12-22 17:38 UTC (permalink / raw)
  To: netdev; +Cc: davem, rds-devel, sowmini.varadhan, santosh.shilimkar
In-Reply-To: <cover.1513962765.git.sowmini.varadhan@oracle.com>

If the rds_sock is not added to the bind_hash_table, we must
reset rs_bound_addr so that rds_remove_bound will not trip on
this rds_sock.

rds_add_bound() does a rds_sock_put() in this failure path, so
failing to reset rs_bound_addr will result in a socket refcount
bug, and will trigger a WARN_ON with the stack shown below when
the application subsequently tries to close the PF_RDS socket.

     WARNING: CPU: 20 PID: 19499 at net/rds/af_rds.c:496 \
		rds_sock_destruct+0x15/0x30 [rds]
       :
     __sk_destruct+0x21/0x190
     rds_remove_bound.part.13+0xb6/0x140 [rds]
     rds_release+0x71/0x120 [rds]
     sock_release+0x1a/0x70
     sock_close+0xe/0x20
     __fput+0xd5/0x210
     task_work_run+0x82/0xa0
     do_exit+0x2ce/0xb30
     ? syscall_trace_enter+0x1cc/0x2b0
     do_group_exit+0x39/0xa0
     SyS_exit_group+0x10/0x10
     do_syscall_64+0x61/0x1a0

Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
---
v2: target net-next, not net

 net/rds/bind.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/net/rds/bind.c b/net/rds/bind.c
index 75d43dc..5aa3a64 100644
--- a/net/rds/bind.c
+++ b/net/rds/bind.c
@@ -114,6 +114,7 @@ static int rds_add_bound(struct rds_sock *rs, __be32 addr, __be16 *port)
 			  rs, &addr, (int)ntohs(*port));
 			break;
 		} else {
+			rs->rs_bound_addr = 0;
 			rds_sock_put(rs);
 			ret = -ENOMEM;
 			break;
-- 
1.7.1

^ permalink raw reply related

* [PATCH V2 net-next 2/3] rds: tcp: initialize t_tcp_detached to false
From: Sowmini Varadhan @ 2017-12-22 17:39 UTC (permalink / raw)
  To: netdev; +Cc: davem, rds-devel, sowmini.varadhan, santosh.shilimkar
In-Reply-To: <cover.1513962765.git.sowmini.varadhan@oracle.com>

Commit f10b4cff98c6 ("rds: tcp: atomically purge entries from
rds_tcp_conn_list during netns delete") adds the field t_tcp_detached,
but this needs to be initialized explicitly to false.

Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
---
v2: target net-next, not net

 net/rds/tcp.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/net/rds/tcp.c b/net/rds/tcp.c
index 39f502d..a61a498 100644
--- a/net/rds/tcp.c
+++ b/net/rds/tcp.c
@@ -290,6 +290,7 @@ static int rds_tcp_conn_alloc(struct rds_connection *conn, gfp_t gfp)
 		tc->t_cpath = &conn->c_path[i];
 
 		spin_lock_irq(&rds_tcp_conn_lock);
+		tc->t_tcp_node_detached = false;
 		list_add_tail(&tc->t_tcp_node, &rds_tcp_conn_list);
 		spin_unlock_irq(&rds_tcp_conn_lock);
 		rdsdebug("rds_conn_path [%d] tc %p\n", i,
-- 
1.7.1

^ permalink raw reply related

* [PATCH V2 net-next 0/3] rds bug fixes
From: Sowmini Varadhan @ 2017-12-22 17:38 UTC (permalink / raw)
  To: netdev; +Cc: davem, rds-devel, sowmini.varadhan, santosh.shilimkar

Ran into pre-existing bugs when working on the fix for
   https://www.spinics.net/lists/netdev/msg472849.html

The bugs fixed in this patchset are unrelated to the syzbot 
failure (which I'm still testing and trying to reproduce) but 
meanwhile, let's get these fixes out of the way.

V2: target net-next (rds:tcp patches have a dependancy on 
changes that are in net-next, but not yet in net)

Sowmini Varadhan (3):
  rds; Reset rs->rs_bound_addr in rds_add_bound() failure path
  rds: tcp: initialized t_tcp_detached to false
  rds: tcp: cleanup if kmem_cache_alloc fails in rds_tcp_conn_alloc()

 net/rds/bind.c |    1 +
 net/rds/tcp.c  |   47 +++++++++++++++++++++++++++--------------------
 2 files changed, 28 insertions(+), 20 deletions(-)

^ permalink raw reply

* Re: [PATCH v3 1/4] security: Add support for SCTP security hooks
From: Marcelo Ricardo Leitner @ 2017-12-22 17:45 UTC (permalink / raw)
  To: Casey Schaufler
  Cc: selinux, netdev, linux-sctp, linux-security-module, paul,
	vyasevich, nhorman, sds, eparis, richard_c_haines
In-Reply-To: <50a04eaf-6130-74d0-43e4-b09bd4de2180@schaufler-ca.com>

On Fri, Dec 22, 2017 at 09:20:45AM -0800, Casey Schaufler wrote:
> On 12/22/2017 5:05 AM, Marcelo Ricardo Leitner wrote:
> > From: Richard Haines <richard_c_haines@btinternet.com>
> >
> > The SCTP security hooks are explained in:
> > Documentation/security/LSM-sctp.rst

Thanks Casey for your comments. However, I'm not that acquainted with
these area of codes and I cannot work on them. I'll just wait for
Richard then.

  Marcelo

^ permalink raw reply

* Re: [PATCH net 2/3] rds: tcp: initialize t_tcp_detached to false
From: Sowmini Varadhan @ 2017-12-22 17:26 UTC (permalink / raw)
  To: netdev, davem; +Cc: rds-devel, santosh.shilimkar
In-Reply-To: <0500544d8e065679e6e31c4ad2793758d2160aba.1513946398.git.sowmini.varadhan@oracle.com>

On (12/22/17 08:14), Sowmini Varadhan wrote:
> Commit f10b4cff98c6 ("rds: tcp: atomically purge entries from
> rds_tcp_conn_list during netns delete") adds the field t_tcp_detached,
> but this needs to be initialized explicitly to false.

I just realized that t_tcp_detached (and the above commit) has not 
made its way to net yet, so patch 2 should apply to net-next (not net).

Please ignore this patch series,  I'll submit a V2.
Apologies for the confusion.

--Sowmini

^ permalink raw reply

* Re: [PATCH v3 1/4] security: Add support for SCTP security hooks
From: Casey Schaufler @ 2017-12-22 17:20 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner, selinux, netdev, linux-sctp,
	linux-security-module
  Cc: paul, vyasevich, nhorman, sds, eparis, richard_c_haines
In-Reply-To: <80e6a4fb06ec6b8f81577abd11827d75dd6689ce.1513940757.git.marcelo.leitner@gmail.com>

On 12/22/2017 5:05 AM, Marcelo Ricardo Leitner wrote:
> From: Richard Haines <richard_c_haines@btinternet.com>
>
> The SCTP security hooks are explained in:
> Documentation/security/LSM-sctp.rst
>
> Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
> Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
>  Documentation/security/LSM-sctp.rst | 194 ++++++++++++++++++++++++++++++++++++
>  include/linux/lsm_hooks.h           |  35 +++++++
>  include/linux/security.h            |  25 +++++
>  security/security.c                 |  22 ++++
>  4 files changed, 276 insertions(+)
>  create mode 100644 Documentation/security/LSM-sctp.rst
>
> diff --git a/Documentation/security/LSM-sctp.rst b/Documentation/security/LSM-sctp.rst
> new file mode 100644
> index 0000000000000000000000000000000000000000..61373672ce9f63bbd52d953500f44cdf3427c3f0
> --- /dev/null
> +++ b/Documentation/security/LSM-sctp.rst
> @@ -0,0 +1,194 @@
> +SCTP LSM Support
> +================
> +
> +For security module support, three sctp specific hooks have been implemented::
> +
> +    security_sctp_assoc_request()
> +    security_sctp_bind_connect()
> +    security_sctp_sk_clone()
> +
> +Also the following security hook has been utilised::
> +
> +    security_inet_conn_established()
> +
> +The usage of these hooks are described below with the SELinux implementation
> +described in ``Documentation/security/SELinux-sctp.rst``
> +
> +
> +security_sctp_assoc_request()
> +-----------------------------
> +This new hook passes the ``@ep`` and ``@chunk->skb`` (the association INIT
> +packet) to the security module. Returns 0 on success, error on failure.
> +::
> +
> +    @ep - pointer to sctp endpoint structure.
> +    @skb - pointer to skbuff of association packet.
> +
> +The security module performs the following operations:
> +     IF this is the first association on ``@ep->base.sk``, then set the peer
> +     sid to that in ``@skb``. This will ensure there is only one peer sid
> +     assigned to ``@ep->base.sk`` that may support multiple associations.
> +
> +     ELSE validate the ``@ep->base.sk peer_sid`` against the ``@skb peer sid``
> +     to determine whether the association should be allowed or denied.
> +
> +     Set the sctp ``@ep sid`` to socket's sid (from ``ep->base.sk``) with
> +     MLS portion taken from ``@skb peer sid``. This will be used by SCTP
> +     TCP style sockets and peeled off connections as they cause a new socket
> +     to be generated.
> +
> +     If IP security options are configured (CIPSO/CALIPSO), then the ip
> +     options are set on the socket.

Please! Basing the documentation for the infrastructure behavior
on a specific security module implementation makes it *really rough*
to adopt it to a different module. It makes it doubly difficult to
define how it will work with multiple modules. Take the SELinux specifics
out of the documentation for the hooks. Describe the general intention,
not how SELinux uses it.

> +
> +
> +security_sctp_bind_connect()
> +-----------------------------
> +This new hook passes one or more ipv4/ipv6 addresses to the security module
> +for validation based on the ``@optname`` that will result in either a bind or
> +connect service as shown in the permission check tables below.
> +Returns 0 on success, error on failure.
> +::
> +
> +    @sk      - Pointer to sock structure.
> +    @optname - Name of the option to validate.
> +    @address - One or more ipv4 / ipv6 addresses.
> +    @addrlen - The total length of address(s). This is calculated on each
> +               ipv4 or ipv6 address using sizeof(struct sockaddr_in) or
> +               sizeof(struct sockaddr_in6).
> +
> +  ------------------------------------------------------------------
> +  |                     BIND Type Checks                           |
> +  |       @optname             |         @address contains         |
> +  |----------------------------|-----------------------------------|
> +  | SCTP_SOCKOPT_BINDX_ADD     | One or more ipv4 / ipv6 addresses |
> +  | SCTP_PRIMARY_ADDR          | Single ipv4 or ipv6 address       |
> +  | SCTP_SET_PEER_PRIMARY_ADDR | Single ipv4 or ipv6 address       |
> +  ------------------------------------------------------------------
> +
> +  ------------------------------------------------------------------
> +  |                   CONNECT Type Checks                          |
> +  |       @optname             |         @address contains         |
> +  |----------------------------|-----------------------------------|
> +  | SCTP_SOCKOPT_CONNECTX      | One or more ipv4 / ipv6 addresses |
> +  | SCTP_PARAM_ADD_IP          | One or more ipv4 / ipv6 addresses |
> +  | SCTP_SENDMSG_CONNECT       | Single ipv4 or ipv6 address       |
> +  | SCTP_PARAM_SET_PRIMARY     | Single ipv4 or ipv6 address       |
> +  ------------------------------------------------------------------
> +
> +A summary of the ``@optname`` entries is as follows::
> +
> +    SCTP_SOCKOPT_BINDX_ADD - Allows additional bind addresses to be
> +                             associated after (optionally) calling
> +                             bind(3).
> +                             sctp_bindx(3) adds a set of bind
> +                             addresses on a socket.
> +
> +    SCTP_SOCKOPT_CONNECTX - Allows the allocation of multiple
> +                            addresses for reaching a peer
> +                            (multi-homed).
> +                            sctp_connectx(3) initiates a connection
> +                            on an SCTP socket using multiple
> +                            destination addresses.
> +
> +    SCTP_SENDMSG_CONNECT  - Initiate a connection that is generated by a
> +                            sendmsg(2) or sctp_sendmsg(3) on a new asociation.
> +
> +    SCTP_PRIMARY_ADDR     - Set local primary address.
> +
> +    SCTP_SET_PEER_PRIMARY_ADDR - Request peer sets address as
> +                                 association primary.
> +
> +    SCTP_PARAM_ADD_IP          - These are used when Dynamic Address
> +    SCTP_PARAM_SET_PRIMARY     - Reconfiguration is enabled as explained below.
> +
> +
> +To support Dynamic Address Reconfiguration the following parameters must be
> +enabled on both endpoints (or use the appropriate **setsockopt**\(2))::
> +
> +    /proc/sys/net/sctp/addip_enable
> +    /proc/sys/net/sctp/addip_noauth_enable
> +
> +then the following *_PARAM_*'s are sent to the peer in an
> +ASCONF chunk when the corresponding ``@optname``'s are present::
> +
> +          @optname                      ASCONF Parameter
> +         ----------                    ------------------
> +    SCTP_SOCKOPT_BINDX_ADD     ->   SCTP_PARAM_ADD_IP
> +    SCTP_SET_PEER_PRIMARY_ADDR ->   SCTP_PARAM_SET_PRIMARY
> +
> +
> +security_sctp_sk_clone()
> +-------------------------
> +This new hook is called whenever a new socket is created by **accept**\(2)
> +(i.e. a TCP style socket) or when a socket is 'peeled off' e.g userspace
> +calls **sctp_peeloff**\(3). ``security_sctp_sk_clone()`` will set the new
> +sockets sid and peer sid to that contained in the ``@ep sid`` and
> +``@ep peer sid`` respectively.
> +::
> +
> +    @ep - pointer to old sctp endpoint structure.
> +    @sk - pointer to old sock structure.
> +    @sk - pointer to new sock structure.

Again, SELinux may set the sids in this way, but another
module may treat the hook differently. Don't put SELinux
specifics into the infrastructure documentation.

> +
> +
> +security_inet_conn_established()
> +---------------------------------
> +This hook has been added to the receive COOKIE ACK processing where it sets
> +the connection's peer sid to that in ``@skb``::
> +
> +    @sk  - pointer to sock structure.
> +    @skb - pointer to skbuff of the COOKIE ACK packet.
> +
> +
> +Security Hooks used for Association Establishment
> +=================================================
> +The following diagram shows the use of ``security_sctp_connect_bind()``,
> +``security_sctp_assoc_request()``, ``security_inet_conn_established()`` when
> +establishing an association.
> +::
> +
> +      SCTP endpoint "A"                                SCTP endpoint "Z"
> +      =================                                =================
> +    sctp_sf_do_prm_asoc()
> + Association setup can be initiated
> + by a connect(2), sctp_connectx(3),
> + sendmsg(2) or sctp_sendmsg(3).
> + These will result in a call to
> + security_sctp_bind_connect() to
> + initiate an association to
> + SCTP peer endpoint "Z".
> +         INIT --------------------------------------------->
> +                                                   sctp_sf_do_5_1B_init()
> +                                                 Respond to an INIT chunk.
> +                                             SCTP peer endpoint "A" is
> +                                             asking for an association. Call
> +                                             security_sctp_assoc_request()
> +                                             to set the peer label if first
> +                                             association.
> +                                             If not first association, check
> +                                             whether allowed, IF so send:
> +          <----------------------------------------------- INIT ACK
> +          |                                  ELSE audit event and silently
> +          |                                       discard the packet.
> +          |
> +    COOKIE ECHO ------------------------------------------>
> +                                                          |
> +                                                          |
> +                                                          |
> +          <------------------------------------------- COOKIE ACK
> +          |                                               |
> +    sctp_sf_do_5_1E_ca                                    |
> + Call security_inet_conn_established()                    |
> + to set the correct peer sid.                             |
> +          |                                               |
> +          |                               If SCTP_SOCKET_TCP or peeled off
> +          |                               socket security_sctp_sk_clone() is
> +          |                               called to clone the new socket.
> +          |                                               |
> +      ESTABLISHED                                    ESTABLISHED
> +          |                                               |
> +    ------------------------------------------------------------------
> +    |                     Association Established                    |
> +    ------------------------------------------------------------------
> +
> +
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index c9258124e41757187cdb8b2f83c5901966345902..92ee9c6c604212ce38590bd2e5fcba55617b9c04 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -906,6 +906,32 @@
>   *	associated with the TUN device's security structure.
>   *	@security pointer to the TUN devices's security structure.
>   *
> + * Security hooks for SCTP
> + *
> + * @sctp_assoc_request:
> + *	If first association, then set the peer sid to that in @skb. If
> + *	@sctp_cid is from an INIT chunk, then set the sctp endpoint sid to
> + *	socket's sid (ep->base.sk) with MLS portion taken from peer sid.

Don't direct the module hook to do something. If the SCTP
code knows what every module should do, why not have the
SCTP code do it? Do not describe the hook in terms of the
SELinux behavior.

> + *	@ep pointer to sctp endpoint structure.
> + *	@skb pointer to skbuff of association packet.
> + *	Return 0 on success, error on failure.
> + * @sctp_bind_connect:
> + *	Validiate permissions required for each address associated with sock
> + *	@sk. Depending on @optname, the addresses will be treated as either
> + *	for a connect or bind service. The @addrlen is calculated on each
> + *	ipv4 and ipv6 address using sizeof(struct sockaddr_in) or
> + *	sizeof(struct sockaddr_in6).
> + *	@sk pointer to sock structure.
> + *	@optname name of the option to validate.
> + *	@address list containing one or more ipv4/ipv6 addresses.
> + *	@addrlen total length of address(s).
> + *	Return 0 on success, error on failure.
> + * @sctp_sk_clone:
> + *	Sets the new child socket's sid to the old endpoint sid.

... and again.

> + *	@ep pointer to old sctp endpoint structure.
> + *	@sk pointer to old sock structure.
> + *	@sk pointer to new sock structure.
> + *
>   * Security hooks for Infiniband
>   *
>   * @ib_pkey_access:
> @@ -1631,6 +1657,12 @@ union security_list_options {
>  	int (*tun_dev_attach_queue)(void *security);
>  	int (*tun_dev_attach)(struct sock *sk, void *security);
>  	int (*tun_dev_open)(void *security);
> +	int (*sctp_assoc_request)(struct sctp_endpoint *ep,
> +				  struct sk_buff *skb);
> +	int (*sctp_bind_connect)(struct sock *sk, int optname,
> +				 struct sockaddr *address, int addrlen);
> +	void (*sctp_sk_clone)(struct sctp_endpoint *ep, struct sock *sk,
> +			      struct sock *newsk);
>  #endif	/* CONFIG_SECURITY_NETWORK */
>  
>  #ifdef CONFIG_SECURITY_INFINIBAND
> @@ -1869,6 +1901,9 @@ struct security_hook_heads {
>  	struct list_head tun_dev_attach_queue;
>  	struct list_head tun_dev_attach;
>  	struct list_head tun_dev_open;
> +	struct list_head sctp_assoc_request;
> +	struct list_head sctp_bind_connect;
> +	struct list_head sctp_sk_clone;
>  #endif	/* CONFIG_SECURITY_NETWORK */
>  #ifdef CONFIG_SECURITY_INFINIBAND
>  	struct list_head ib_pkey_access;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index ce6265960d6c430a90e1ad3c3749d0a438ecaca9..51f6cc2417f278674dfbd434587af805cb0c03d3 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -115,6 +115,7 @@ struct xfrm_policy;
>  struct xfrm_state;
>  struct xfrm_user_sec_ctx;
>  struct seq_file;
> +struct sctp_endpoint;
>  
>  #ifdef CONFIG_MMU
>  extern unsigned long mmap_min_addr;
> @@ -1229,6 +1230,11 @@ int security_tun_dev_create(void);
>  int security_tun_dev_attach_queue(void *security);
>  int security_tun_dev_attach(struct sock *sk, void *security);
>  int security_tun_dev_open(void *security);
> +int security_sctp_assoc_request(struct sctp_endpoint *ep, struct sk_buff *skb);
> +int security_sctp_bind_connect(struct sock *sk, int optname,
> +			       struct sockaddr *address, int addrlen);
> +void security_sctp_sk_clone(struct sctp_endpoint *ep, struct sock *sk,
> +			    struct sock *newsk);
>  
>  #else	/* CONFIG_SECURITY_NETWORK */
>  static inline int security_unix_stream_connect(struct sock *sock,
> @@ -1421,6 +1427,25 @@ static inline int security_tun_dev_open(void *security)
>  {
>  	return 0;
>  }
> +
> +static inline int security_sctp_assoc_request(struct sctp_endpoint *ep,
> +					      struct sk_buff *skb)
> +{
> +	return 0;
> +}
> +
> +static inline int security_sctp_bind_connect(struct sock *sk, int optname,
> +					     struct sockaddr *address,
> +					     int addrlen)
> +{
> +	return 0;
> +}
> +
> +static inline void security_sctp_sk_clone(struct sctp_endpoint *ep,
> +					  struct sock *sk,
> +					  struct sock *newsk)
> +{
> +}
>  #endif	/* CONFIG_SECURITY_NETWORK */
>  
>  #ifdef CONFIG_SECURITY_INFINIBAND
> diff --git a/security/security.c b/security/security.c
> index 4bf0f571b4ef94df1d3c44b7fed6b7b651c1924f..1400678f6b72b36123f2fa2b909f35d257a62cd4 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1472,6 +1472,7 @@ void security_inet_conn_established(struct sock *sk,
>  {
>  	call_void_hook(inet_conn_established, sk, skb);
>  }
> +EXPORT_SYMBOL(security_inet_conn_established);
>  
>  int security_secmark_relabel_packet(u32 secid)
>  {
> @@ -1527,6 +1528,27 @@ int security_tun_dev_open(void *security)
>  }
>  EXPORT_SYMBOL(security_tun_dev_open);
>  
> +int security_sctp_assoc_request(struct sctp_endpoint *ep, struct sk_buff *skb)
> +{
> +	return call_int_hook(sctp_assoc_request, 0, ep, skb);
> +}
> +EXPORT_SYMBOL(security_sctp_assoc_request);
> +
> +int security_sctp_bind_connect(struct sock *sk, int optname,
> +			       struct sockaddr *address, int addrlen)
> +{
> +	return call_int_hook(sctp_bind_connect, 0, sk, optname,
> +			     address, addrlen);
> +}
> +EXPORT_SYMBOL(security_sctp_bind_connect);
> +
> +void security_sctp_sk_clone(struct sctp_endpoint *ep, struct sock *sk,
> +			    struct sock *newsk)
> +{
> +	call_void_hook(sctp_sk_clone, ep, sk, newsk);
> +}
> +EXPORT_SYMBOL(security_sctp_sk_clone);
> +
>  #endif	/* CONFIG_SECURITY_NETWORK */
>  
>  #ifdef CONFIG_SECURITY_INFINIBAND


^ permalink raw reply

* [bpf-next V2 PATCH 14/14] samples/bpf: program demonstrating access to xdp_rxq_info
From: Jesper Dangaard Brouer @ 2017-12-22 17:12 UTC (permalink / raw)
  To: Daniel Borkmann, Alexei Starovoitov
  Cc: netdev, Jesper Dangaard Brouer, dsahern, gospo, bjorn.topel,
	michael.chan
In-Reply-To: <151396262289.20006.1429172971820409456.stgit@firesoul>

This sample program can be used for monitoring and reporting how many
packets per sec (pps) are received per NIC RX queue index and which
CPU processed the packet. In itself it is a useful tool for quickly
identifying RSS imbalance issues, see below.

The default XDP action is XDP_PASS in-order to provide a monitor
mode. For benchmarking purposes it is possible to specify other XDP
actions on the cmdline --action.

Output below shows an imbalance RSS case where most RXQ's deliver to
CPU-0 while CPU-2 only get packets from a single RXQ.  Looking at
things from a CPU level the two CPUs are processing approx the same
amount, BUT looking at the rx_queue_index levels it is clear that
RXQ-2 receive much better service, than other RXQs which all share CPU-0.

Running XDP on dev:i40e1 (ifindex:3) action:XDP_PASS
XDP stats       CPU     pps         issue-pps
XDP-RX CPU      0       900,473     0
XDP-RX CPU      2       906,921     0
XDP-RX CPU      total   1,807,395

RXQ stats       RXQ:CPU pps         issue-pps
rx_queue_index    0:0   180,098     0
rx_queue_index    0:sum 180,098
rx_queue_index    1:0   180,098     0
rx_queue_index    1:sum 180,098
rx_queue_index    2:2   906,921     0
rx_queue_index    2:sum 906,921
rx_queue_index    3:0   180,098     0
rx_queue_index    3:sum 180,098
rx_queue_index    4:0   180,082     0
rx_queue_index    4:sum 180,082
rx_queue_index    5:0   180,093     0
rx_queue_index    5:sum 180,093

Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 samples/bpf/Makefile            |    4 
 samples/bpf/xdp_rxq_info_kern.c |   96 +++++++
 samples/bpf/xdp_rxq_info_user.c |  532 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 632 insertions(+)
 create mode 100644 samples/bpf/xdp_rxq_info_kern.c
 create mode 100644 samples/bpf/xdp_rxq_info_user.c

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 4fb944a7ecf8..3ff7a05bea9a 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -41,6 +41,7 @@ hostprogs-y += xdp_redirect
 hostprogs-y += xdp_redirect_map
 hostprogs-y += xdp_redirect_cpu
 hostprogs-y += xdp_monitor
+hostprogs-y += xdp_rxq_info
 hostprogs-y += syscall_tp
 
 # Libbpf dependencies
@@ -90,6 +91,7 @@ xdp_redirect-objs := bpf_load.o $(LIBBPF) xdp_redirect_user.o
 xdp_redirect_map-objs := bpf_load.o $(LIBBPF) xdp_redirect_map_user.o
 xdp_redirect_cpu-objs := bpf_load.o $(LIBBPF) xdp_redirect_cpu_user.o
 xdp_monitor-objs := bpf_load.o $(LIBBPF) xdp_monitor_user.o
+xdp_rxq_info-objs := bpf_load.o $(LIBBPF) xdp_rxq_info_user.o
 syscall_tp-objs := bpf_load.o $(LIBBPF) syscall_tp_user.o
 
 # Tell kbuild to always build the programs
@@ -139,6 +141,7 @@ always += xdp_redirect_kern.o
 always += xdp_redirect_map_kern.o
 always += xdp_redirect_cpu_kern.o
 always += xdp_monitor_kern.o
+always += xdp_rxq_info_kern.o
 always += syscall_tp_kern.o
 
 HOSTCFLAGS += -I$(objtree)/usr/include
@@ -182,6 +185,7 @@ HOSTLOADLIBES_xdp_redirect += -lelf
 HOSTLOADLIBES_xdp_redirect_map += -lelf
 HOSTLOADLIBES_xdp_redirect_cpu += -lelf
 HOSTLOADLIBES_xdp_monitor += -lelf
+HOSTLOADLIBES_xdp_rxq_info += -lelf
 HOSTLOADLIBES_syscall_tp += -lelf
 
 # Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on cmdline:
diff --git a/samples/bpf/xdp_rxq_info_kern.c b/samples/bpf/xdp_rxq_info_kern.c
new file mode 100644
index 000000000000..19cbd662202c
--- /dev/null
+++ b/samples/bpf/xdp_rxq_info_kern.c
@@ -0,0 +1,96 @@
+/* Example howto extract XDP RX-queue info
+ *
+ * Copyright (c) 2017 Jesper Dangaard Brouer, Red Hat Inc.
+ * Released under terms in GPL version 2.  See COPYING.
+ */
+#include <uapi/linux/bpf.h>
+#include "bpf_helpers.h"
+
+/* Config setup from with userspace
+ *
+ * User-side setup ifindex in config_map, to verify that
+ * ctx->ingress_ifindex is correct (against configured ifindex)
+ */
+struct config {
+	__u32 action;
+	int ifindex;
+};
+struct bpf_map_def SEC("maps") config_map = {
+	.type		= BPF_MAP_TYPE_ARRAY,
+	.key_size	= sizeof(int),
+	.value_size	= sizeof(struct config),
+	.max_entries	= 1,
+};
+
+/* Common stats data record (shared with userspace) */
+struct datarec {
+	__u64 processed;
+	__u64 issue;
+};
+
+struct bpf_map_def SEC("maps") stats_global_map = {
+	.type		= BPF_MAP_TYPE_PERCPU_ARRAY,
+	.key_size	= sizeof(u32),
+	.value_size	= sizeof(struct datarec),
+	.max_entries	= 1,
+};
+
+#define MAX_RXQs 64
+
+/* Stats per rx_queue_index (per CPU) */
+struct bpf_map_def SEC("maps") rx_queue_index_map = {
+	.type		= BPF_MAP_TYPE_PERCPU_ARRAY,
+	.key_size	= sizeof(u32),
+	.value_size	= sizeof(struct datarec),
+	.max_entries	= MAX_RXQs + 1,
+};
+
+SEC("xdp_prog0")
+int  xdp_prognum0(struct xdp_md *ctx)
+{
+	void *data_end = (void *)(long)ctx->data_end;
+	void *data     = (void *)(long)ctx->data;
+	struct datarec *rec, *rxq_rec;
+	int ingress_ifindex;
+	struct config *config;
+	u32 key = 0;
+
+	/* Global stats record */
+	rec = bpf_map_lookup_elem(&stats_global_map, &key);
+	if (!rec)
+		return XDP_ABORTED;
+	rec->processed++;
+
+	/* Accessing ctx->ingress_ifindex, cause BPF to rewrite BPF
+	 * instructions inside kernel to access xdp_rxq->dev->ifindex
+	 */
+	ingress_ifindex = ctx->ingress_ifindex;
+
+	config = bpf_map_lookup_elem(&config_map, &key);
+	if (!config)
+		return XDP_ABORTED;
+
+	/* Simple test: check ctx provided ifindex is as expected */
+	if (ingress_ifindex != config->ifindex) {
+		/* count this error case */
+		rec->issue++;
+		return XDP_ABORTED;
+	}
+
+	/* Update stats per rx_queue_index. Handle if rx_queue_index
+	 * is larger than stats map can contain info for.
+	 */
+	key = ctx->rx_queue_index;
+	if (key >= MAX_RXQs)
+		key = MAX_RXQs;
+	rxq_rec = bpf_map_lookup_elem(&rx_queue_index_map, &key);
+	if (!rxq_rec)
+		return XDP_ABORTED;
+	rxq_rec->processed++;
+	if (key == MAX_RXQs)
+		rxq_rec->issue++;
+
+	return config->action;
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/samples/bpf/xdp_rxq_info_user.c b/samples/bpf/xdp_rxq_info_user.c
new file mode 100644
index 000000000000..d062b0e09924
--- /dev/null
+++ b/samples/bpf/xdp_rxq_info_user.c
@@ -0,0 +1,532 @@
+/*
+ * Copyright (c) 2017 Jesper Dangaard Brouer, Red Hat Inc.
+ * Released under terms in GPL version 2.  See COPYING.
+ */
+static const char *__doc__ = " XDP RX-queue info extract example\n\n"
+	"Monitor how many packets per sec (pps) are received\n"
+	"per NIC RX queue index and which CPU processed the packet\n"
+	;
+
+#include <errno.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <string.h>
+#include <unistd.h>
+#include <locale.h>
+#include <sys/resource.h>
+#include <getopt.h>
+#include <net/if.h>
+#include <time.h>
+
+#include <arpa/inet.h>
+#include <linux/if_link.h>
+
+#include "libbpf.h"
+#include "bpf_load.h"
+#include "bpf_util.h"
+
+static int ifindex = -1;
+static char ifname_buf[IF_NAMESIZE];
+static char *ifname;
+
+static __u32 xdp_flags;
+
+/* Exit return codes */
+#define EXIT_OK		0
+#define EXIT_FAIL		1
+#define EXIT_FAIL_OPTION	2
+#define EXIT_FAIL_XDP		3
+#define EXIT_FAIL_BPF		4
+#define EXIT_FAIL_MEM		5
+
+static const struct option long_options[] = {
+	{"help",	no_argument,		NULL, 'h' },
+	{"dev",		required_argument,	NULL, 'd' },
+	{"skb-mode",	no_argument,		NULL, 'S' },
+	{"sec",		required_argument,	NULL, 's' },
+	{"no-separators", no_argument,		NULL, 'z' },
+	{"action",	required_argument,	NULL, 'a' },
+	{0, 0, NULL,  0 }
+};
+
+static void int_exit(int sig)
+{
+	fprintf(stderr,
+		"Interrupted: Removing XDP program on ifindex:%d device:%s\n",
+		ifindex, ifname);
+	if (ifindex > -1)
+		set_link_xdp_fd(ifindex, -1, xdp_flags);
+	exit(EXIT_OK);
+}
+
+struct config {
+	__u32 action;
+	int ifindex;
+};
+#define XDP_ACTION_MAX (XDP_TX + 1)
+#define XDP_ACTION_MAX_STRLEN 11
+static const char *xdp_action_names[XDP_ACTION_MAX] = {
+	[XDP_ABORTED]	= "XDP_ABORTED",
+	[XDP_DROP]	= "XDP_DROP",
+	[XDP_PASS]	= "XDP_PASS",
+	[XDP_TX]	= "XDP_TX",
+};
+
+static const char *action2str(int action)
+{
+	if (action < XDP_ACTION_MAX)
+		return xdp_action_names[action];
+	return NULL;
+}
+
+static int parse_xdp_action(char *action_str)
+{
+	size_t maxlen;
+	__u64 action = -1;
+	int i;
+
+	for (i = 0; i < XDP_ACTION_MAX; i++) {
+		maxlen = XDP_ACTION_MAX_STRLEN;
+		if (strncmp(xdp_action_names[i], action_str, maxlen) == 0) {
+			action = i;
+			break;
+		}
+	}
+	return action;
+}
+
+static void list_xdp_actions(void)
+{
+	int i;
+
+	printf("Available XDP --action <options>\n");
+	for (i = 0; i < XDP_ACTION_MAX; i++)
+		printf("\t%s\n", xdp_action_names[i]);
+	printf("\n");
+}
+
+static void usage(char *argv[])
+{
+	int i;
+
+	printf("\nDOCUMENTATION:\n%s\n", __doc__);
+	printf(" Usage: %s (options-see-below)\n", argv[0]);
+	printf(" Listing options:\n");
+	for (i = 0; long_options[i].name != 0; i++) {
+		printf(" --%-12s", long_options[i].name);
+		if (long_options[i].flag != NULL)
+			printf(" flag (internal value:%d)",
+				*long_options[i].flag);
+		else
+			printf(" short-option: -%c",
+				long_options[i].val);
+		printf("\n");
+	}
+	printf("\n");
+	list_xdp_actions();
+}
+
+#define NANOSEC_PER_SEC 1000000000 /* 10^9 */
+static __u64 gettime(void)
+{
+	struct timespec t;
+	int res;
+
+	res = clock_gettime(CLOCK_MONOTONIC, &t);
+	if (res < 0) {
+		fprintf(stderr, "Error with gettimeofday! (%i)\n", res);
+		exit(EXIT_FAIL);
+	}
+	return (__u64) t.tv_sec * NANOSEC_PER_SEC + t.tv_nsec;
+}
+
+/* Common stats data record shared with _kern.c */
+struct datarec {
+	__u64 processed;
+	__u64 issue;
+};
+struct record {
+	__u64 timestamp;
+	struct datarec total;
+	struct datarec *cpu;
+};
+struct stats_record {
+	struct record stats;
+	struct record *rxq;
+};
+
+static struct datarec *alloc_record_per_cpu(void)
+{
+	unsigned int nr_cpus = bpf_num_possible_cpus();
+	struct datarec *array;
+	size_t size;
+
+	size = sizeof(struct datarec) * nr_cpus;
+	array = malloc(size);
+	memset(array, 0, size);
+	if (!array) {
+		fprintf(stderr, "Mem alloc error (nr_cpus:%u)\n", nr_cpus);
+		exit(EXIT_FAIL_MEM);
+	}
+	return array;
+}
+
+static struct record *alloc_record_per_rxq(void)
+{
+	unsigned int nr_rxqs = map_data[2].def.max_entries;
+	struct record *array;
+	size_t size;
+
+	size = sizeof(struct record) * nr_rxqs;
+	array = malloc(size);
+	memset(array, 0, size);
+	if (!array) {
+		fprintf(stderr, "Mem alloc error (nr_rxqs:%u)\n", nr_rxqs);
+		exit(EXIT_FAIL_MEM);
+	}
+	return array;
+}
+
+static struct stats_record *alloc_stats_record(void)
+{
+	unsigned int nr_rxqs = map_data[2].def.max_entries;
+	struct stats_record *rec;
+	int i;
+
+	rec = malloc(sizeof(*rec));
+	memset(rec, 0, sizeof(*rec));
+	if (!rec) {
+		fprintf(stderr, "Mem alloc error\n");
+		exit(EXIT_FAIL_MEM);
+	}
+	rec->rxq = alloc_record_per_rxq();
+	for (i = 0; i < nr_rxqs; i++)
+		rec->rxq[i].cpu = alloc_record_per_cpu();
+
+	rec->stats.cpu = alloc_record_per_cpu();
+	return rec;
+}
+
+static void free_stats_record(struct stats_record *r)
+{
+	unsigned int nr_rxqs = map_data[2].def.max_entries;
+	int i;
+
+	for (i = 0; i < nr_rxqs; i++)
+		free(r->rxq[i].cpu);
+
+	free(r->rxq);
+	free(r->stats.cpu);
+	free(r);
+}
+
+static bool map_collect_percpu(int fd, __u32 key, struct record *rec)
+{
+	/* For percpu maps, userspace gets a value per possible CPU */
+	unsigned int nr_cpus = bpf_num_possible_cpus();
+	struct datarec values[nr_cpus];
+	__u64 sum_processed = 0;
+	__u64 sum_issue = 0;
+	int i;
+
+	if ((bpf_map_lookup_elem(fd, &key, values)) != 0) {
+		fprintf(stderr,
+			"ERR: bpf_map_lookup_elem failed key:0x%X\n", key);
+		return false;
+	}
+	/* Get time as close as possible to reading map contents */
+	rec->timestamp = gettime();
+
+	/* Record and sum values from each CPU */
+	for (i = 0; i < nr_cpus; i++) {
+		rec->cpu[i].processed = values[i].processed;
+		sum_processed        += values[i].processed;
+		rec->cpu[i].issue = values[i].issue;
+		sum_issue        += values[i].issue;
+	}
+	rec->total.processed = sum_processed;
+	rec->total.issue     = sum_issue;
+	return true;
+}
+
+static void stats_collect(struct stats_record *rec)
+{
+	int fd, i, max_rxqs;
+
+	fd = map_data[1].fd; /* map: stats_global_map */
+	map_collect_percpu(fd, 0, &rec->stats);
+
+	fd = map_data[2].fd; /* map: rx_queue_index_map */
+	max_rxqs = map_data[2].def.max_entries;
+	for (i = 0; i < max_rxqs; i++)
+		map_collect_percpu(fd, i, &rec->rxq[i]);
+}
+
+static double calc_period(struct record *r, struct record *p)
+{
+	double period_ = 0;
+	__u64 period = 0;
+
+	period = r->timestamp - p->timestamp;
+	if (period > 0)
+		period_ = ((double) period / NANOSEC_PER_SEC);
+
+	return period_;
+}
+
+static __u64 calc_pps(struct datarec *r, struct datarec *p, double period_)
+{
+	__u64 packets = 0;
+	__u64 pps = 0;
+
+	if (period_ > 0) {
+		packets = r->processed - p->processed;
+		pps = packets / period_;
+	}
+	return pps;
+}
+
+static __u64 calc_errs_pps(struct datarec *r,
+			    struct datarec *p, double period_)
+{
+	__u64 packets = 0;
+	__u64 pps = 0;
+
+	if (period_ > 0) {
+		packets = r->issue - p->issue;
+		pps = packets / period_;
+	}
+	return pps;
+}
+
+static void stats_print(struct stats_record *stats_rec,
+			struct stats_record *stats_prev,
+			int action)
+{
+	unsigned int nr_cpus = bpf_num_possible_cpus();
+	unsigned int nr_rxqs = map_data[2].def.max_entries;
+	double pps = 0, err = 0;
+	struct record *rec, *prev;
+	double t;
+	int rxq;
+	int i;
+
+	/* Header */
+	printf("\nRunning XDP on dev:%s (ifindex:%d) action:%s\n",
+	       ifname, ifindex, action2str(action));
+
+	/* stats_global_map */
+	{
+		char *fmt_rx = "%-15s %-7d %'-11.0f %'-10.0f %s\n";
+		char *fm2_rx = "%-15s %-7s %'-11.0f\n";
+		char *errstr = "";
+
+		printf("%-15s %-7s %-11s %-11s\n",
+		       "XDP stats", "CPU", "pps", "issue-pps");
+
+		rec  =  &stats_rec->stats;
+		prev = &stats_prev->stats;
+		t = calc_period(rec, prev);
+		for (i = 0; i < nr_cpus; i++) {
+			struct datarec *r = &rec->cpu[i];
+			struct datarec *p = &prev->cpu[i];
+
+			pps = calc_pps     (r, p, t);
+			err = calc_errs_pps(r, p, t);
+			if (err > 0)
+				errstr = "invalid-ifindex";
+			if (pps > 0)
+				printf(fmt_rx, "XDP-RX CPU",
+					i, pps, err, errstr);
+		}
+		pps  = calc_pps     (&rec->total, &prev->total, t);
+		err  = calc_errs_pps(&rec->total, &prev->total, t);
+		printf(fm2_rx, "XDP-RX CPU", "total", pps, err);
+	}
+
+	/* rx_queue_index_map */
+	printf("\n%-15s %-7s %-11s %-11s\n",
+	       "RXQ stats", "RXQ:CPU", "pps", "issue-pps");
+
+	for (rxq = 0; rxq < nr_rxqs; rxq++) {
+		char *fmt_rx = "%-15s %3d:%-3d %'-11.0f %'-10.0f %s\n";
+		char *fm2_rx = "%-15s %3d:%-3s %'-11.0f\n";
+		char *errstr = "";
+		int rxq_ = rxq;
+
+		/* Last RXQ in map catch overflows */
+		if (rxq_ == nr_rxqs - 1)
+			rxq_ = -1;
+
+		rec  =  &stats_rec->rxq[rxq];
+		prev = &stats_prev->rxq[rxq];
+		t = calc_period(rec, prev);
+		for (i = 0; i < nr_cpus; i++) {
+			struct datarec *r = &rec->cpu[i];
+			struct datarec *p = &prev->cpu[i];
+
+			pps = calc_pps     (r, p, t);
+			err = calc_errs_pps(r, p, t);
+			if (err > 0) {
+				if (rxq_ == -1)
+					errstr = "map-overflow-RXQ";
+				else
+					errstr = "err";
+			}
+			if (pps > 0)
+				printf(fmt_rx, "rx_queue_index",
+				       rxq_, i, pps, err, errstr);
+		}
+		pps  = calc_pps     (&rec->total, &prev->total, t);
+		err  = calc_errs_pps(&rec->total, &prev->total, t);
+		if (pps || err)
+			printf(fm2_rx, "rx_queue_index", rxq_, "sum", pps, err);
+	}
+}
+
+
+/* Pointer swap trick */
+static inline void swap(struct stats_record **a, struct stats_record **b)
+{
+	struct stats_record *tmp;
+
+	tmp = *a;
+	*a = *b;
+	*b = tmp;
+}
+
+static void stats_poll(int interval, int action)
+{
+	struct stats_record *record, *prev;
+
+	record = alloc_stats_record();
+	prev   = alloc_stats_record();
+	stats_collect(record);
+
+	while (1) {
+		swap(&prev, &record);
+		stats_collect(record);
+		stats_print(record, prev, action);
+		sleep(interval);
+	}
+
+	free_stats_record(record);
+	free_stats_record(prev);
+}
+
+
+int main(int argc, char **argv)
+{
+	struct rlimit r = {10 * 1024 * 1024, RLIM_INFINITY};
+	bool use_separators = true;
+	struct config cfg = { 0 };
+	char filename[256];
+	int longindex = 0;
+	int interval = 2;
+	__u32 key = 0;
+	int opt, err;
+
+	char action_str_buf[XDP_ACTION_MAX_STRLEN + 1 /* for \0 */] = { 0 };
+	int action = XDP_PASS; /* Default action */
+	char *action_str = NULL;
+
+	snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
+
+	if (setrlimit(RLIMIT_MEMLOCK, &r)) {
+		perror("setrlimit(RLIMIT_MEMLOCK)");
+		return 1;
+	}
+
+	if (load_bpf_file(filename)) {
+		fprintf(stderr, "ERR in load_bpf_file(): %s", bpf_log_buf);
+		return EXIT_FAIL;
+	}
+
+	if (!prog_fd[0]) {
+		fprintf(stderr, "ERR: load_bpf_file: %s\n", strerror(errno));
+		return EXIT_FAIL;
+	}
+
+	/* Parse commands line args */
+	while ((opt = getopt_long(argc, argv, "hSd:",
+				  long_options, &longindex)) != -1) {
+		switch (opt) {
+		case 'd':
+			if (strlen(optarg) >= IF_NAMESIZE) {
+				fprintf(stderr, "ERR: --dev name too long\n");
+				goto error;
+			}
+			ifname = (char *)&ifname_buf;
+			strncpy(ifname, optarg, IF_NAMESIZE);
+			ifindex = if_nametoindex(ifname);
+			if (ifindex == 0) {
+				fprintf(stderr,
+					"ERR: --dev name unknown err(%d):%s\n",
+					errno, strerror(errno));
+				goto error;
+			}
+			break;
+		case 's':
+			interval = atoi(optarg);
+			break;
+		case 'S':
+			xdp_flags |= XDP_FLAGS_SKB_MODE;
+			break;
+		case 'z':
+			use_separators = false;
+			break;
+		case 'a':
+			action_str = (char *)&action_str_buf;
+			strncpy(action_str, optarg, XDP_ACTION_MAX_STRLEN);
+			break;
+		case 'h':
+		error:
+		default:
+			usage(argv);
+			return EXIT_FAIL_OPTION;
+		}
+	}
+	/* Required option */
+	if (ifindex == -1) {
+		fprintf(stderr, "ERR: required option --dev missing\n");
+		usage(argv);
+		return EXIT_FAIL_OPTION;
+	}
+	cfg.ifindex = ifindex;
+
+	/* Parse action string */
+	if (action_str) {
+		action = parse_xdp_action(action_str);
+		if (action < 0) {
+			fprintf(stderr, "ERR: Invalid XDP --action: %s\n",
+				action_str);
+			list_xdp_actions();
+			return EXIT_FAIL_OPTION;
+		}
+	}
+	cfg.action = action;
+
+	/* Trick to pretty printf with thousands separators use %' */
+	if (use_separators)
+		setlocale(LC_NUMERIC, "en_US");
+
+	/* User-side setup ifindex in config_map */
+	err = bpf_map_update_elem(map_fd[0], &key, &cfg, 0);
+	if (err) {
+		fprintf(stderr, "Store config failed (err:%d)\n", err);
+		exit(EXIT_FAIL_BPF);
+	}
+
+	/* Remove XDP program when program is interrupted */
+	signal(SIGINT, int_exit);
+
+	if (set_link_xdp_fd(ifindex, prog_fd[0], xdp_flags) < 0) {
+		fprintf(stderr, "link set xdp fd failed\n");
+		return EXIT_FAIL_XDP;
+	}
+
+	stats_poll(interval, action);
+	return EXIT_OK;
+}

^ permalink raw reply related

* [bpf-next V2 PATCH 13/14] bpf: finally expose xdp_rxq_info to XDP bpf-programs
From: Jesper Dangaard Brouer @ 2017-12-22 17:12 UTC (permalink / raw)
  To: Daniel Borkmann, Alexei Starovoitov
  Cc: netdev, Jesper Dangaard Brouer, dsahern, gospo, bjorn.topel,
	michael.chan
In-Reply-To: <151396262289.20006.1429172971820409456.stgit@firesoul>

Now all XDP driver have been updated to setup xdp_rxq_info and assign
this to xdp_buff->rxq.  Thus, it is now safe to enable access to some
of the xdp_rxq_info struct members.

This patch extend xdp_md and expose UAPI to userspace for
ingress_ifindex and rx_queue_index.  Access happens via bpf
instruction rewrite, that load data directly from struct xdp_rxq_info.

* ingress_ifindex map to xdp_rxq_info->dev->ifindex
* rx_queue_index  map to xdp_rxq_info->queue_index

Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 include/uapi/linux/bpf.h |    3 +++
 net/core/filter.c        |   19 +++++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 69eabfcb9bdb..a6000a95d40e 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -899,6 +899,9 @@ struct xdp_md {
 	__u32 data;
 	__u32 data_end;
 	__u32 data_meta;
+	/* Below access go though struct xdp_rxq_info */
+	__u32 ingress_ifindex; /* rxq->dev->ifindex */
+	__u32 rx_queue_index;  /* rxq->queue_index  */
 };
 
 enum sk_action {
diff --git a/net/core/filter.c b/net/core/filter.c
index 130b842c3a15..acdb94c0e97f 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4304,6 +4304,25 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
 				      si->dst_reg, si->src_reg,
 				      offsetof(struct xdp_buff, data_end));
 		break;
+	case offsetof(struct xdp_md, ingress_ifindex):
+		*insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(struct xdp_buff, rxq),
+				      si->dst_reg, si->src_reg,
+				      offsetof(struct xdp_buff, rxq));
+		*insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(struct xdp_rxq_info, dev),
+				      si->dst_reg, si->dst_reg,
+				      offsetof(struct xdp_rxq_info, dev));
+		*insn++ = BPF_LDX_MEM(BPF_W, si->dst_reg, si->dst_reg,
+				      bpf_target_off(struct net_device,
+						     ifindex, 4, target_size));
+		break;
+	case offsetof(struct xdp_md, rx_queue_index):
+		*insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(struct xdp_buff, rxq),
+				      si->dst_reg, si->src_reg,
+				      offsetof(struct xdp_buff, rxq));
+		*insn++ = BPF_LDX_MEM(BPF_W, si->dst_reg, si->dst_reg,
+				      bpf_target_off(struct xdp_rxq_info,
+						queue_index, 4, target_size));
+		break;
 	}
 
 	return insn - insn_buf;

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox