Netdev List
 help / color / mirror / Atom feed
From: "Toke Høiland-Jørgensen" <toke@redhat.com>
To: David Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org,
	"Jesper Dangaard Brouer" <brouer@redhat.com>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Alexei Starovoitov" <ast@kernel.org>,
	"Jakub Kicinski" <jakub.kicinski@netronome.com>,
	BjörnTöpel <bjorn.topel@gmail.com>
Subject: [PATCH net-next v4 1/6] net: xdp: refactor XDP attach
Date: Mon, 08 Apr 2019 19:05:56 +0200	[thread overview]
Message-ID: <155474315660.24432.7026778509168675619.stgit@alrua-x1> (raw)
In-Reply-To: <155474315642.24432.6179239576879119104.stgit@alrua-x1>

From: Björn Töpel <bjorn.topel@intel.com>

Generic XDP and driver XDP cannot be enabled at the same time. However,
they don't share any state; let's fix that. Here, dev->xdp_prog, is
used for both driver and generic mode. This removes the need for the
XDP_QUERY_PROG command to ndo_bpf.

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
 include/linux/netdevice.h |    8 +-
 net/core/dev.c            |  148 ++++++++++++++++++++++++++-------------------
 net/core/rtnetlink.c      |   14 +---
 3 files changed, 93 insertions(+), 77 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index eb9f05e0863d..68d4bbc44c63 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1957,7 +1957,7 @@ struct net_device {
 	struct cpu_rmap		*rx_cpu_rmap;
 #endif
 	struct hlist_node	index_hlist;
-
+	unsigned int		xdp_target;
 /*
  * Cache lines mostly used on transmit path
  */
@@ -2063,7 +2063,8 @@ struct net_device {
 
 static inline bool netif_elide_gro(const struct net_device *dev)
 {
-	if (!(dev->features & NETIF_F_GRO) || dev->xdp_prog)
+	if (!(dev->features & NETIF_F_GRO) ||
+	    dev->xdp_target == XDP_FLAGS_SKB_MODE)
 		return true;
 	return false;
 }
@@ -3702,8 +3703,7 @@ struct sk_buff *dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
 typedef int (*bpf_op_t)(struct net_device *dev, struct netdev_bpf *bpf);
 int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
 		      int fd, u32 flags);
-u32 __dev_xdp_query(struct net_device *dev, bpf_op_t xdp_op,
-		    enum bpf_netdev_command cmd);
+u32 __dev_xdp_query(struct net_device *dev, unsigned int target);
 int xdp_umem_query(struct net_device *dev, u16 queue_id);
 
 int __dev_forward_skb(struct net_device *dev, struct sk_buff *skb);
diff --git a/net/core/dev.c b/net/core/dev.c
index d5b1315218d3..feafc3580350 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5121,35 +5121,25 @@ static void __netif_receive_skb_list(struct list_head *head)
 
 static int generic_xdp_install(struct net_device *dev, struct netdev_bpf *xdp)
 {
-	struct bpf_prog *old = rtnl_dereference(dev->xdp_prog);
 	struct bpf_prog *new = xdp->prog;
-	int ret = 0;
-
-	switch (xdp->command) {
-	case XDP_SETUP_PROG:
-		rcu_assign_pointer(dev->xdp_prog, new);
-		if (old)
-			bpf_prog_put(old);
 
-		if (old && !new) {
+	if (xdp->command == XDP_SETUP_PROG) {
+		if (static_key_enabled(&generic_xdp_needed_key) && !new) {
 			static_branch_dec(&generic_xdp_needed_key);
-		} else if (new && !old) {
+		} else if (new &&
+			   !static_key_enabled(&generic_xdp_needed_key)) {
 			static_branch_inc(&generic_xdp_needed_key);
 			dev_disable_lro(dev);
 			dev_disable_gro_hw(dev);
 		}
-		break;
 
-	case XDP_QUERY_PROG:
-		xdp->prog_id = old ? old->aux->id : 0;
-		break;
+		if (new)
+			bpf_prog_put(new);
 
-	default:
-		ret = -EINVAL;
-		break;
+		return 0;
 	}
 
-	return ret;
+	return -EINVAL;
 }
 
 static int netif_receive_skb_internal(struct sk_buff *skb)
@@ -7981,30 +7971,50 @@ int dev_change_proto_down_generic(struct net_device *dev, bool proto_down)
 }
 EXPORT_SYMBOL(dev_change_proto_down_generic);
 
-u32 __dev_xdp_query(struct net_device *dev, bpf_op_t bpf_op,
-		    enum bpf_netdev_command cmd)
+u32 __dev_xdp_query(struct net_device *dev, unsigned int target)
 {
-	struct netdev_bpf xdp;
+	if (target == XDP_FLAGS_DRV_MODE || target == XDP_FLAGS_SKB_MODE) {
+		struct bpf_prog *prog = rtnl_dereference(dev->xdp_prog);
 
-	if (!bpf_op)
-		return 0;
+		return prog && (dev->xdp_target == target) ? prog->aux->id : 0;
+	}
 
-	memset(&xdp, 0, sizeof(xdp));
-	xdp.command = cmd;
+	if (target == XDP_FLAGS_HW_MODE) {
+		struct netdev_bpf xdp = { .command = XDP_QUERY_PROG_HW };
 
-	/* Query must always succeed. */
-	WARN_ON(bpf_op(dev, &xdp) < 0 && cmd == XDP_QUERY_PROG);
+		if (!dev->netdev_ops->ndo_bpf)
+			return 0;
+		dev->netdev_ops->ndo_bpf(dev, &xdp);
+		return xdp.prog_id;
+	}
 
-	return xdp.prog_id;
+	WARN_ON(1);
+	return 0;
 }
 
-static int dev_xdp_install(struct net_device *dev, bpf_op_t bpf_op,
+static int dev_xdp_install(struct net_device *dev, unsigned int target,
 			   struct netlink_ext_ack *extack, u32 flags,
 			   struct bpf_prog *prog)
 {
-	struct netdev_bpf xdp;
+	const struct net_device_ops *ops = dev->netdev_ops;
+	struct bpf_prog *old = NULL;
+	struct netdev_bpf xdp = {};
+	int ret;
+
+	if (target != XDP_FLAGS_HW_MODE) {
+		if (prog) {
+			prog = bpf_prog_inc(prog);
+			if (IS_ERR(prog))
+				return PTR_ERR(prog);
+		}
+
+		old = rtnl_dereference(dev->xdp_prog);
+		if (!old && !prog)
+			return 0;
+		rcu_assign_pointer(dev->xdp_prog, prog);
+		dev->xdp_target = prog ? target : 0;
+	}
 
-	memset(&xdp, 0, sizeof(xdp));
 	if (flags & XDP_FLAGS_HW_MODE)
 		xdp.command = XDP_SETUP_PROG_HW;
 	else
@@ -8013,7 +8023,24 @@ static int dev_xdp_install(struct net_device *dev, bpf_op_t bpf_op,
 	xdp.flags = flags;
 	xdp.prog = prog;
 
-	return bpf_op(dev, &xdp);
+	if (target == XDP_FLAGS_SKB_MODE)
+		ret = generic_xdp_install(dev, &xdp);
+	else
+		ret = ops->ndo_bpf(dev, &xdp);
+
+	if (target != XDP_FLAGS_HW_MODE) {
+		if (ret) {
+			if (prog)
+				bpf_prog_put(prog);
+			rcu_assign_pointer(dev->xdp_prog, old);
+			dev->xdp_target = old ? target : 0;
+		} else {
+			if (old)
+				bpf_prog_put(old);
+		}
+	}
+
+	return ret;
 }
 
 static void dev_xdp_uninstall(struct net_device *dev)
@@ -8021,27 +8048,28 @@ static void dev_xdp_uninstall(struct net_device *dev)
 	struct netdev_bpf xdp;
 	bpf_op_t ndo_bpf;
 
-	/* Remove generic XDP */
-	WARN_ON(dev_xdp_install(dev, generic_xdp_install, NULL, 0, NULL));
+	/* Remove generic/native XDP */
+	WARN_ON(dev_xdp_install(dev, dev->xdp_target, NULL, 0, NULL));
 
 	/* Remove from the driver */
 	ndo_bpf = dev->netdev_ops->ndo_bpf;
 	if (!ndo_bpf)
 		return;
 
-	memset(&xdp, 0, sizeof(xdp));
-	xdp.command = XDP_QUERY_PROG;
-	WARN_ON(ndo_bpf(dev, &xdp));
-	if (xdp.prog_id)
-		WARN_ON(dev_xdp_install(dev, ndo_bpf, NULL, xdp.prog_flags,
-					NULL));
-
-	/* Remove HW offload */
 	memset(&xdp, 0, sizeof(xdp));
 	xdp.command = XDP_QUERY_PROG_HW;
 	if (!ndo_bpf(dev, &xdp) && xdp.prog_id)
-		WARN_ON(dev_xdp_install(dev, ndo_bpf, NULL, xdp.prog_flags,
-					NULL));
+		WARN_ON(dev_xdp_install(dev, XDP_FLAGS_HW_MODE, NULL,
+					xdp.prog_flags, NULL));
+}
+
+static bool dev_validate_active_xdp(struct net_device *dev, unsigned int target)
+{
+	if (target == XDP_FLAGS_DRV_MODE)
+		return dev->xdp_target != XDP_FLAGS_SKB_MODE;
+	if (target == XDP_FLAGS_SKB_MODE)
+		return dev->xdp_target != XDP_FLAGS_DRV_MODE;
+	return true;
 }
 
 /**
@@ -8057,40 +8085,36 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
 		      int fd, u32 flags)
 {
 	const struct net_device_ops *ops = dev->netdev_ops;
-	enum bpf_netdev_command query;
+	bool offload, drv = !!ops->ndo_bpf;
 	struct bpf_prog *prog = NULL;
-	bpf_op_t bpf_op, bpf_chk;
-	bool offload;
+	unsigned int target;
 	int err;
 
 	ASSERT_RTNL();
 
 	offload = flags & XDP_FLAGS_HW_MODE;
-	query = offload ? XDP_QUERY_PROG_HW : XDP_QUERY_PROG;
+	target = offload ? XDP_FLAGS_HW_MODE : XDP_FLAGS_DRV_MODE;
 
-	bpf_op = bpf_chk = ops->ndo_bpf;
-	if (!bpf_op && (flags & (XDP_FLAGS_DRV_MODE | XDP_FLAGS_HW_MODE))) {
+	if (!drv && (flags & (XDP_FLAGS_DRV_MODE | XDP_FLAGS_HW_MODE))) {
 		NL_SET_ERR_MSG(extack, "underlying driver does not support XDP in native mode");
 		return -EOPNOTSUPP;
 	}
-	if (!bpf_op || (flags & XDP_FLAGS_SKB_MODE))
-		bpf_op = generic_xdp_install;
-	if (bpf_op == bpf_chk)
-		bpf_chk = generic_xdp_install;
+	if (!drv || (flags & XDP_FLAGS_SKB_MODE))
+		target = XDP_FLAGS_SKB_MODE;
+
+	if (!dev_validate_active_xdp(dev, target)) {
+		NL_SET_ERR_MSG(extack, "native and generic XDP can't be active at the same time");
+		return -EEXIST;
+	}
 
 	if (fd >= 0) {
-		if (!offload && __dev_xdp_query(dev, bpf_chk, XDP_QUERY_PROG)) {
-			NL_SET_ERR_MSG(extack, "native and generic XDP can't be active at the same time");
-			return -EEXIST;
-		}
 		if ((flags & XDP_FLAGS_UPDATE_IF_NOEXIST) &&
-		    __dev_xdp_query(dev, bpf_op, query)) {
+		    __dev_xdp_query(dev, target)) {
 			NL_SET_ERR_MSG(extack, "XDP program already attached");
 			return -EBUSY;
 		}
 
-		prog = bpf_prog_get_type_dev(fd, BPF_PROG_TYPE_XDP,
-					     bpf_op == ops->ndo_bpf);
+		prog = bpf_prog_get_type_dev(fd, BPF_PROG_TYPE_XDP, drv);
 		if (IS_ERR(prog))
 			return PTR_ERR(prog);
 
@@ -8101,7 +8125,7 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
 		}
 	}
 
-	err = dev_xdp_install(dev, bpf_op, extack, flags, prog);
+	err = dev_xdp_install(dev, target, extack, flags, prog);
 	if (err < 0 && prog)
 		bpf_prog_put(prog);
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index f9b964fd4e4d..d9c7fe34c3a1 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1362,25 +1362,17 @@ static int rtnl_fill_link_ifmap(struct sk_buff *skb, struct net_device *dev)
 
 static u32 rtnl_xdp_prog_skb(struct net_device *dev)
 {
-	const struct bpf_prog *generic_xdp_prog;
-
-	ASSERT_RTNL();
-
-	generic_xdp_prog = rtnl_dereference(dev->xdp_prog);
-	if (!generic_xdp_prog)
-		return 0;
-	return generic_xdp_prog->aux->id;
+	return __dev_xdp_query(dev, XDP_FLAGS_SKB_MODE);
 }
 
 static u32 rtnl_xdp_prog_drv(struct net_device *dev)
 {
-	return __dev_xdp_query(dev, dev->netdev_ops->ndo_bpf, XDP_QUERY_PROG);
+	return __dev_xdp_query(dev, XDP_FLAGS_DRV_MODE);
 }
 
 static u32 rtnl_xdp_prog_hw(struct net_device *dev)
 {
-	return __dev_xdp_query(dev, dev->netdev_ops->ndo_bpf,
-			       XDP_QUERY_PROG_HW);
+	return __dev_xdp_query(dev, XDP_FLAGS_HW_MODE);
 }
 
 static int rtnl_xdp_report_one(struct sk_buff *skb, struct net_device *dev,


  parent reply	other threads:[~2019-04-08 17:06 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-08 17:05 [PATCH net-next v4 0/6] xdp: Use a default map for xdp_redirect helper Toke Høiland-Jørgensen
2019-04-08 17:05 ` [PATCH net-next v4 4/6] xdp: Always use a devmap for XDP_REDIRECT to a device Toke Høiland-Jørgensen
2019-04-09  8:25   ` Jesper Dangaard Brouer
2019-04-09  9:23     ` Toke Høiland-Jørgensen
2019-04-10  7:59   ` [xdp] 9cb54e254c: kernel/bpf/devmap.c:#suspicious_rcu_dereference_check() kernel test robot
2019-04-10 10:00     ` Toke Høiland-Jørgensen
2019-04-08 17:05 ` [PATCH net-next v4 5/6] xdp: Add devmap_idx map type for looking up devices by ifindex Toke Høiland-Jørgensen
2019-04-08 17:05 ` Toke Høiland-Jørgensen [this message]
2019-04-08 20:23   ` [PATCH net-next v4 1/6] net: xdp: refactor XDP attach Daniel Borkmann
2019-05-09 10:40     ` Björn Töpel
2019-04-09 12:54   ` kbuild test robot
2019-04-08 17:05 ` [PATCH net-next v4 3/6] xdp: Refactor devmap code in preparation for subsequent additions Toke Høiland-Jørgensen
2019-04-08 17:05 ` [PATCH net-next v4 2/6] net: xdp: remove XDP_QUERY_PROG Toke Høiland-Jørgensen
2019-04-08 17:05 ` [PATCH net-next v4 6/6] selftests/bpf: Add test for default devmap allocation Toke Høiland-Jørgensen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=155474315660.24432.7026778509168675619.stgit@alrua-x1 \
    --to=toke@redhat.com \
    --cc=ast@kernel.org \
    --cc=bjorn.topel@gmail.com \
    --cc=brouer@redhat.com \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=jakub.kicinski@netronome.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox