[PATCH bpf-next 1/2] net: xdp: refactor XDP_QUERY_PROG{,_HW} to netdev

["Björn Töpel" <bjorn.topel@gmail.com>]

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

All XDP capable drivers need to implement the XDP_QUERY_PROG{,_HW}
command of ndo_bpf. The query code is fairly generic. This commit
refactors the query code up from the drivers to the netdev level.

The struct net_device has gained four new members tracking the XDP
program, the corresponding program flags, and which programs
(SKB_MODE, DRV_MODE or HW_MODE) that are activated.

The xdp_prog member, previously only used for SKB_MODE, is shared with
DRV_MODE, since they are mutually exclusive.

The program query operations is all done under the rtnl lock. However,
the xdp_prog member is __rcu annotated, and used in a lock-less manner
for the SKB_MODE. This is because the xdp_prog member is shared from a
query program perspective (again, SKB and DRV are mutual exclusive),
RCU read and assignments functions are still used when altering
xdp_prog, even when only for queries in DRV_MODE. This is for
sparse/lockdep correctness.

A minor behavioral change was done during this refactorization; When
passing a negative file descriptor to a netdev to disable XDP, the
same program flag as the running program is required. An example.

The eth0 is DRV_DRV capable. Previously, this was OK, but confusing:

  # ip link set dev eth0 xdp obj foo.o sec main
  # ip link set dev eth0 xdpgeneric off

Now, the same commands give:

  # ip link set dev eth0 xdp obj foo.o sec main
  # ip link set dev eth0 xdpgeneric off
  Error: native and generic XDP can't be active at the same time.

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
 include/linux/netdevice.h |  13 +++--
 net/core/dev.c            | 120 ++++++++++++++++++++------------------
 net/core/rtnetlink.c      |  33 ++---------
 3 files changed, 76 insertions(+), 90 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 44b47e9df94a..bdcb20a3946c 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1940,6 +1940,11 @@ struct net_device {
 #endif
 	struct hlist_node	index_hlist;
 
+	struct bpf_prog		*xdp_prog_hw;
+	unsigned int		xdp_flags;
+	u32			xdp_prog_flags;
+	u32			xdp_prog_hw_flags;
+
 /*
  * Cache lines mostly used on transmit path
  */
@@ -2045,9 +2050,8 @@ struct net_device {
 
 static inline bool netif_elide_gro(const struct net_device *dev)
 {
-	if (!(dev->features & NETIF_F_GRO) || dev->xdp_prog)
-		return true;
-	return false;
+	return !(dev->features & NETIF_F_GRO) ||
+		dev->xdp_flags & XDP_FLAGS_SKB_MODE;
 }
 
 #define	NETDEV_ALIGN		32
@@ -3684,8 +3688,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 flags);
 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 b6b8505cfb3e..ead16c3f955c 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -8005,31 +8005,31 @@ 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 flags)
 {
-	struct netdev_bpf xdp;
+	struct bpf_prog *prog = NULL;
 
-	if (!bpf_op)
+	if (hweight32(flags) != 1)
 		return 0;
 
-	memset(&xdp, 0, sizeof(xdp));
-	xdp.command = cmd;
-
-	/* Query must always succeed. */
-	WARN_ON(bpf_op(dev, &xdp) < 0 && cmd == XDP_QUERY_PROG);
+	if (flags & (XDP_FLAGS_SKB_MODE | XDP_FLAGS_DRV_MODE))
+		prog = rtnl_dereference(dev->xdp_prog);
+	else if (flags & XDP_FLAGS_HW_MODE)
+		prog = dev->xdp_prog_hw;
 
-	return xdp.prog_id;
+	return prog ? prog->aux->id : 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)
 {
+	bpf_op_t bpf_op = dev->netdev_ops->ndo_bpf;
 	struct netdev_bpf xdp;
+	int err;
 
 	memset(&xdp, 0, sizeof(xdp));
-	if (flags & XDP_FLAGS_HW_MODE)
+	if (target == XDP_FLAGS_HW_MODE)
 		xdp.command = XDP_SETUP_PROG_HW;
 	else
 		xdp.command = XDP_SETUP_PROG;
@@ -8037,35 +8037,41 @@ 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);
+	err = (target == XDP_FLAGS_SKB_MODE) ?
+	      generic_xdp_install(dev, &xdp) :
+	      bpf_op(dev, &xdp);
+	if (!err) {
+		if (prog)
+			dev->xdp_flags |= target;
+		else
+			dev->xdp_flags &= ~target;
+
+		if (target == XDP_FLAGS_HW_MODE) {
+			dev->xdp_prog_hw = prog;
+			dev->xdp_prog_hw_flags = flags;
+		} else if (target == XDP_FLAGS_DRV_MODE) {
+			rcu_assign_pointer(dev->xdp_prog, prog);
+			dev->xdp_prog_flags = flags;
+		}
+	}
+
+	return err;
 }
 
 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 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));
+	if (dev->xdp_flags & XDP_FLAGS_SKB_MODE) {
+		WARN_ON(dev_xdp_install(dev, XDP_FLAGS_SKB_MODE,
+					NULL, 0, NULL));
+	}
+	if (dev->xdp_flags & XDP_FLAGS_DRV_MODE) {
+		WARN_ON(dev_xdp_install(dev, XDP_FLAGS_DRV_MODE,
+					NULL, dev->xdp_prog_flags, NULL));
+	}
+	if (dev->xdp_flags & XDP_FLAGS_HW_MODE) {
+		WARN_ON(dev_xdp_install(dev, XDP_FLAGS_HW_MODE,
+					NULL, dev->xdp_prog_hw_flags, NULL));
+	}
 }
 
 /**
@@ -8080,41 +8086,41 @@ static void dev_xdp_uninstall(struct net_device *dev)
 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_supp = !!dev->netdev_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_supp && (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_supp || (flags & XDP_FLAGS_SKB_MODE))
+		target = XDP_FLAGS_SKB_MODE;
+
+	if ((target == XDP_FLAGS_SKB_MODE &&
+	     dev->xdp_flags & XDP_FLAGS_DRV_MODE) ||
+	    (target == XDP_FLAGS_DRV_MODE &&
+	     dev->xdp_flags & XDP_FLAGS_SKB_MODE)) {
+		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)) {
+		if (flags & XDP_FLAGS_UPDATE_IF_NOEXIST &&
+		    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_supp);
+
 		if (IS_ERR(prog))
 			return PTR_ERR(prog);
 
@@ -8125,7 +8131,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 adcc045952c2..99ca58db297f 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1360,37 +1360,14 @@ static int rtnl_fill_link_ifmap(struct sk_buff *skb, struct net_device *dev)
 	return 0;
 }
 
-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;
-}
-
-static u32 rtnl_xdp_prog_drv(struct net_device *dev)
-{
-	return __dev_xdp_query(dev, dev->netdev_ops->ndo_bpf, XDP_QUERY_PROG);
-}
-
-static u32 rtnl_xdp_prog_hw(struct net_device *dev)
-{
-	return __dev_xdp_query(dev, dev->netdev_ops->ndo_bpf,
-			       XDP_QUERY_PROG_HW);
-}
-
 static int rtnl_xdp_report_one(struct sk_buff *skb, struct net_device *dev,
 			       u32 *prog_id, u8 *mode, u8 tgt_mode, u32 attr,
-			       u32 (*get_prog_id)(struct net_device *dev))
+			       unsigned int tgt_flags)
 {
 	u32 curr_id;
 	int err;
 
-	curr_id = get_prog_id(dev);
+	curr_id = dev_xdp_query(dev, tgt_flags);
 	if (!curr_id)
 		return 0;
 
@@ -1421,15 +1398,15 @@ static int rtnl_xdp_fill(struct sk_buff *skb, struct net_device *dev)
 	prog_id = 0;
 	mode = XDP_ATTACHED_NONE;
 	err = rtnl_xdp_report_one(skb, dev, &prog_id, &mode, XDP_ATTACHED_SKB,
-				  IFLA_XDP_SKB_PROG_ID, rtnl_xdp_prog_skb);
+				  IFLA_XDP_SKB_PROG_ID, XDP_FLAGS_SKB_MODE);
 	if (err)
 		goto err_cancel;
 	err = rtnl_xdp_report_one(skb, dev, &prog_id, &mode, XDP_ATTACHED_DRV,
-				  IFLA_XDP_DRV_PROG_ID, rtnl_xdp_prog_drv);
+				  IFLA_XDP_DRV_PROG_ID, XDP_FLAGS_DRV_MODE);
 	if (err)
 		goto err_cancel;
 	err = rtnl_xdp_report_one(skb, dev, &prog_id, &mode, XDP_ATTACHED_HW,
-				  IFLA_XDP_HW_PROG_ID, rtnl_xdp_prog_hw);
+				  IFLA_XDP_HW_PROG_ID, XDP_FLAGS_HW_MODE);
 	if (err)
 		goto err_cancel;
 
-- 
2.20.1