[PATCH bpf-next v2 0/2] net: xdp: refactor the XDP_QUERY_PROG and XDP_QUERY_PROG_HW code

Here's yet another attempt tomove the XDP_QUERY_PROG{,_HW} code out
from the drivers to generic netdev code.

The first patch in the series move the XDP query functionality, and
the second remove XDP_QUERY_PROG{,_HW} from all drivers.

Please refer to the individual commit messages for more details.

The patch passes test_offload.py from selftests. Thanks to Jakub for
pointing this out.

I, hopefully, addressed all comments from Jakub and Saeed, except one;
I did not move the XDP struct net_device into a struct of its own.


Thanks,
Björn


Björn Töpel (2):
  net: xdp: refactor XDP_QUERY_PROG{,_HW} to netdev
  net: xdp: remove XDP_QUERY_PROG{,_HW}

 drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c |   4 -
 .../net/ethernet/cavium/thunder/nicvf_main.c  |   3 -
 .../net/ethernet/freescale/dpaa2/dpaa2-eth.c  |   3 -
 drivers/net/ethernet/intel/i40e/i40e_main.c   |   3 -
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   4 -
 .../net/ethernet/intel/ixgbevf/ixgbevf_main.c |   4 -
 .../net/ethernet/mellanox/mlx4/en_netdev.c    |  24 ---
 .../net/ethernet/mellanox/mlx5/core/en_main.c |  18 ---
 .../ethernet/netronome/nfp/nfp_net_common.c   |   4 -
 .../net/ethernet/qlogic/qede/qede_filter.c    |   3 -
 drivers/net/netdevsim/bpf.c                   |   4 -
 drivers/net/netdevsim/netdevsim.h             |   2 +-
 drivers/net/tun.c                             |  15 --
 drivers/net/veth.c                            |  15 --
 drivers/net/virtio_net.c                      |  17 ---
 include/linux/netdevice.h                     |  23 ++-
 include/net/xdp.h                             |   6 +-
 net/core/dev.c                                | 143 +++++++++++-------
 net/core/rtnetlink.c                          |  53 ++++---
 net/core/xdp.c                                |  20 ++-
 20 files changed, 137 insertions(+), 231 deletions(-)

-- 
2.20.1

From: Björn Töpel <redacted>

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 two new members: xdp_prog_hw and
xdp_flags. The former is the offloaded XDP program, if any, and the
latter tracks the flags that the supplied when attaching the XDP
program. The flags only apply to SKB_MODE or DRV_MODE, not HW_MODE.

The xdp_prog member, previously only used for SKB_MODE, is shared with
DRV_MODE. This is OK, due to the fact that SKB_MODE and DRV_MODE are
mutually exclusive. To differentiate between the two modes, a new
internal flag is introduced as well.

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. Now that xdp_prog member is shared from a query
program perspective, 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.

Signed-off-by: Björn Töpel <redacted>
---
 include/linux/netdevice.h |  15 +++--
 include/net/xdp.h         |   4 ++
 net/core/dev.c            | 138 ++++++++++++++++++++++++--------------
 net/core/rtnetlink.c      |  53 +++++++--------
 net/core/xdp.c            |  17 +++--
 5 files changed, 139 insertions(+), 88 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 44b47e9df94a..f3a875a52c6c 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h

@@ -1940,6 +1940,9 @@ struct net_device {
 #endif
 	struct hlist_node	index_hlist;
 
+	struct bpf_prog		*xdp_prog_hw;
+	u32			xdp_flags;
+
 /*
  * Cache lines mostly used on transmit path
  */

@@ -2043,11 +2046,14 @@ struct net_device {
 };
 #define to_net_dev(d) container_of(d, struct net_device, dev)
 
+/* Reusing the XDP flags space for kernel internal flag */
+#define XDP_FLAGS_KERN_GENERIC (1U << 4)
+static_assert(!(XDP_FLAGS_KERN_GENERIC & XDP_FLAGS_MASK));
+
 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_KERN_GENERIC;
 }
 
 #define	NETDEV_ALIGN		32

@@ -3684,8 +3690,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 mode);
 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/include/net/xdp.h b/include/net/xdp.h
index 0f25b3675c5c..3691280c8fc1 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h

@@ -51,6 +51,7 @@ struct xdp_mem_info {
 };
 
 struct page_pool;
+struct netlink_ext_ack;
 
 struct zero_copy_allocator {
 	void (*free)(struct zero_copy_allocator *zca, unsigned long handle);

@@ -166,4 +167,7 @@ bool xdp_attachment_flags_ok(struct xdp_attachment_info *info,
 void xdp_attachment_setup(struct xdp_attachment_info *info,
 			  struct netdev_bpf *bpf);
 
+bool xdp_prog_flags_ok(u32 old_flags, u32 new_flags,
+		       struct netlink_ext_ack *extack);
+
 #endif /* __LINUX_NET_XDP_H__ */

diff --git a/net/core/dev.c b/net/core/dev.c
index b6b8505cfb3e..1a9da508149a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c

@@ -8005,21 +8005,43 @@ 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)
+static u32 dev_xdp_query_generic(struct net_device *dev)
 {
-	struct netdev_bpf xdp;
+	struct bpf_prog *prog = rtnl_dereference(dev->xdp_prog);
 
-	if (!bpf_op)
-		return 0;
+	return prog && dev->xdp_flags & XDP_FLAGS_KERN_GENERIC ?
+		prog->aux->id : 0;
+}
 
-	memset(&xdp, 0, sizeof(xdp));
-	xdp.command = cmd;
+static u32 dev_xdp_query_drv(struct net_device *dev)
+{
+	struct bpf_prog *prog = rtnl_dereference(dev->xdp_prog);
+
+	return prog && !(dev->xdp_flags & XDP_FLAGS_KERN_GENERIC) ?
+		prog->aux->id : 0;
+}
+
+static u32 dev_xdp_current_mode(struct net_device *dev)
+{
+	struct bpf_prog *prog = rtnl_dereference(dev->xdp_prog);
 
-	/* Query must always succeed. */
-	WARN_ON(bpf_op(dev, &xdp) < 0 && cmd == XDP_QUERY_PROG);
+	if (prog)
+		return dev_xdp_query_generic(dev) ? XDP_FLAGS_SKB_MODE :
+			XDP_FLAGS_DRV_MODE;
+	return 0;
+}
 
-	return xdp.prog_id;
+u32 dev_xdp_query(struct net_device *dev, unsigned int mode)
+{
+	switch (mode) {
+	case XDP_FLAGS_DRV_MODE:
+		return dev_xdp_query_drv(dev);
+	case XDP_FLAGS_SKB_MODE:
+		return dev_xdp_query_generic(dev);
+	case XDP_FLAGS_HW_MODE:
+		return dev->xdp_prog_hw ? dev->xdp_prog_hw->aux->id : 0;
+	}
+	return 0;
 }
 
 static int dev_xdp_install(struct net_device *dev, bpf_op_t bpf_op,

@@ -8027,45 +8049,47 @@ static int dev_xdp_install(struct net_device *dev, bpf_op_t bpf_op,
 			   struct bpf_prog *prog)
 {
 	struct netdev_bpf xdp;
+	int err;
 
 	memset(&xdp, 0, sizeof(xdp));
-	if (flags & XDP_FLAGS_HW_MODE)
+	if (flags & XDP_FLAGS_HW_MODE) {
 		xdp.command = XDP_SETUP_PROG_HW;
-	else
+		xdp.flags = XDP_FLAGS_HW_MODE;
+	} else {
 		xdp.command = XDP_SETUP_PROG;
+		xdp.flags = flags;
+	}
 	xdp.extack = extack;
-	xdp.flags = flags;
 	xdp.prog = prog;
 
-	return bpf_op(dev, &xdp);
+	err = bpf_op(dev, &xdp);
+	if (err)
+		return err;
+
+	if (flags & XDP_FLAGS_HW_MODE) {
+		dev->xdp_prog_hw = prog;
+		return 0;
+	}
+
+	rcu_assign_pointer(dev->xdp_prog, prog);
+	dev->xdp_flags = prog ? flags : 0;
+	return 0;
 }
 
 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));
+	struct bpf_prog *prog = rtnl_dereference(dev->xdp_prog);
+	bpf_op_t bpf_op;
 
-	/* 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,
+	if (prog) {
+		bpf_op = dev_xdp_query_generic(dev) ?
+			 generic_xdp_install : dev->netdev_ops->ndo_bpf;
+		WARN_ON(dev_xdp_install(dev, bpf_op, NULL, dev->xdp_flags,
 					NULL));
+	}
+	if (dev_xdp_query(dev, XDP_FLAGS_HW_MODE))
+		WARN_ON(dev_xdp_install(dev, dev->netdev_ops->ndo_bpf,
+					NULL, XDP_FLAGS_HW_MODE, NULL));
 }
 
 /**

@@ -8080,41 +8104,49 @@ 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;
 	struct bpf_prog *prog = NULL;
-	bpf_op_t bpf_op, bpf_chk;
+	u32 mode, curr_mode;
+	bpf_op_t bpf_op;
 	bool offload;
 	int err;
 
 	ASSERT_RTNL();
 
 	offload = flags & XDP_FLAGS_HW_MODE;
-	query = offload ? XDP_QUERY_PROG_HW : XDP_QUERY_PROG;
+	mode = offload ? XDP_FLAGS_HW_MODE : XDP_FLAGS_DRV_MODE;
 
-	bpf_op = bpf_chk = ops->ndo_bpf;
+	bpf_op = dev->netdev_ops->ndo_bpf;
 	if (!bpf_op && (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 (fd >= 0) {
-		if (!offload && __dev_xdp_query(dev, bpf_chk, XDP_QUERY_PROG)) {
+	if (!bpf_op || flags & XDP_FLAGS_SKB_MODE)
+		mode = XDP_FLAGS_SKB_MODE;
+
+	curr_mode = dev_xdp_current_mode(dev);
+
+	if (!offload && curr_mode && (mode ^ curr_mode) &
+	    (XDP_FLAGS_DRV_MODE | XDP_FLAGS_SKB_MODE)) {
+		if (fd >= 0) {
 			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)) {
+		return 0;
+	}
+
+	if (!offload && dev_xdp_query(dev, mode) &&
+	    !xdp_prog_flags_ok(dev->xdp_flags, flags, extack))
+		return -EBUSY;
+
+	if (fd >= 0) {
+		if (flags & XDP_FLAGS_UPDATE_IF_NOEXIST &&
+		    dev_xdp_query(dev, mode)) {
 			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, !!bpf_op);
 		if (IS_ERR(prog))
 			return PTR_ERR(prog);
 

@@ -8125,6 +8157,10 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
 		}
 	}
 
+	if (mode == XDP_FLAGS_SKB_MODE) {
+		bpf_op = generic_xdp_install;
+		flags |= XDP_FLAGS_KERN_GENERIC;
+	}
 	err = dev_xdp_install(dev, bpf_op, 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..5e396fd01d8b 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c

@@ -1360,42 +1360,44 @@ 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)
+static unsigned int rtnl_xdp_mode_to_flag(u8 tgt_mode)
 {
-	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);
+	switch (tgt_mode) {
+	case XDP_ATTACHED_DRV:
+		return XDP_FLAGS_DRV_MODE;
+	case XDP_ATTACHED_SKB:
+		return XDP_FLAGS_SKB_MODE;
+	case XDP_ATTACHED_HW:
+		return XDP_FLAGS_HW_MODE;
+	}
+	return 0;
 }
 
-static u32 rtnl_xdp_prog_hw(struct net_device *dev)
+static u32 rtnl_xdp_mode_to_attr(u8 tgt_mode)
 {
-	return __dev_xdp_query(dev, dev->netdev_ops->ndo_bpf,
-			       XDP_QUERY_PROG_HW);
+	switch (tgt_mode) {
+	case XDP_ATTACHED_DRV:
+		return IFLA_XDP_DRV_PROG_ID;
+	case XDP_ATTACHED_SKB:
+		return IFLA_XDP_SKB_PROG_ID;
+	case XDP_ATTACHED_HW:
+		return IFLA_XDP_HW_PROG_ID;
+	}
+	return 0;
 }
 
 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))
+			       u32 *prog_id, u8 *mode, u8 tgt_mode)
 {
 	u32 curr_id;
 	int err;
 
-	curr_id = get_prog_id(dev);
+	curr_id = dev_xdp_query(dev, rtnl_xdp_mode_to_flag(tgt_mode));
 	if (!curr_id)
 		return 0;
 
 	*prog_id = curr_id;
-	err = nla_put_u32(skb, attr, curr_id);
+	err = nla_put_u32(skb, rtnl_xdp_mode_to_attr(tgt_mode), curr_id);
 	if (err)
 		return err;
 

@@ -1420,16 +1422,13 @@ 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);
+	err = rtnl_xdp_report_one(skb, dev, &prog_id, &mode, XDP_ATTACHED_SKB);
 	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);
+	err = rtnl_xdp_report_one(skb, dev, &prog_id, &mode, XDP_ATTACHED_DRV);
 	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);
+	err = rtnl_xdp_report_one(skb, dev, &prog_id, &mode, XDP_ATTACHED_HW);
 	if (err)
 		goto err_cancel;
 

diff --git a/net/core/xdp.c b/net/core/xdp.c
index 4b2b194f4f1f..af92c04a58d8 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c

@@ -388,16 +388,23 @@ int xdp_attachment_query(struct xdp_attachment_info *info,
 }
 EXPORT_SYMBOL_GPL(xdp_attachment_query);
 
-bool xdp_attachment_flags_ok(struct xdp_attachment_info *info,
-			     struct netdev_bpf *bpf)
+bool xdp_prog_flags_ok(u32 old_flags, u32 new_flags,
+		       struct netlink_ext_ack *extack)
 {
-	if (info->prog && (bpf->flags ^ info->flags) & XDP_FLAGS_MODES) {
-		NL_SET_ERR_MSG(bpf->extack,
-			       "program loaded with different flags");
+	if ((new_flags ^ old_flags) & XDP_FLAGS_MODES) {
+		NL_SET_ERR_MSG(extack, "program loaded with different flags");
 		return false;
 	}
 	return true;
 }
+
+bool xdp_attachment_flags_ok(struct xdp_attachment_info *info,
+			     struct netdev_bpf *bpf)
+{
+	if (info->prog)
+		return xdp_prog_flags_ok(bpf->flags, info->flags, bpf->extack);
+	return true;
+}
 EXPORT_SYMBOL_GPL(xdp_attachment_flags_ok);
 
 void xdp_attachment_setup(struct xdp_attachment_info *info,

-- 
2.20.1

From: Björn Töpel <redacted>

Remove all use of XDP_QUERY_PROG{,_HW}, since it was moved to the
generic code path.

Signed-off-by: Björn Töpel <redacted>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c |  4 ----
 .../net/ethernet/cavium/thunder/nicvf_main.c  |  3 ---
 .../net/ethernet/freescale/dpaa2/dpaa2-eth.c  |  3 ---
 drivers/net/ethernet/intel/i40e/i40e_main.c   |  3 ---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  4 ----
 .../net/ethernet/intel/ixgbevf/ixgbevf_main.c |  4 ----
 .../net/ethernet/mellanox/mlx4/en_netdev.c    | 24 -------------------
 .../net/ethernet/mellanox/mlx5/core/en_main.c | 18 --------------
 .../ethernet/netronome/nfp/nfp_net_common.c   |  4 ----
 .../net/ethernet/qlogic/qede/qede_filter.c    |  3 ---
 drivers/net/netdevsim/bpf.c                   |  4 ----
 drivers/net/netdevsim/netdevsim.h             |  2 +-
 drivers/net/tun.c                             | 15 ------------
 drivers/net/veth.c                            | 15 ------------
 drivers/net/virtio_net.c                      | 17 -------------
 include/linux/netdevice.h                     |  8 -------
 include/net/xdp.h                             |  2 --
 net/core/dev.c                                |  5 ----
 net/core/xdp.c                                |  9 -------
 19 files changed, 1 insertion(+), 146 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
index 0184ef6f05a7..8b1e77522e18 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c

@@ -217,10 +217,6 @@ int bnxt_xdp(struct net_device *dev, struct netdev_bpf *xdp)
 	case XDP_SETUP_PROG:
 		rc = bnxt_xdp_set(bp, xdp->prog);
 		break;
-	case XDP_QUERY_PROG:
-		xdp->prog_id = bp->xdp_prog ? bp->xdp_prog->aux->id : 0;
-		rc = 0;
-		break;
 	default:
 		rc = -EINVAL;
 		break;

diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index c032bef1b776..14c079538cb5 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c

@@ -1914,9 +1914,6 @@ static int nicvf_xdp(struct net_device *netdev, struct netdev_bpf *xdp)
 	switch (xdp->command) {
 	case XDP_SETUP_PROG:
 		return nicvf_xdp_setup(nic, xdp->prog);
-	case XDP_QUERY_PROG:
-		xdp->prog_id = nic->xdp_prog ? nic->xdp_prog->aux->id : 0;
-		return 0;
 	default:
 		return -EINVAL;
 	}

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
index 63b1ecc18c26..a00cf674b8c8 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c

@@ -1763,9 +1763,6 @@ static int dpaa2_eth_xdp(struct net_device *dev, struct netdev_bpf *xdp)
 	switch (xdp->command) {
 	case XDP_SETUP_PROG:
 		return setup_xdp(dev, xdp->prog);
-	case XDP_QUERY_PROG:
-		xdp->prog_id = priv->xdp_prog ? priv->xdp_prog->aux->id : 0;
-		break;
 	default:
 		return -EINVAL;
 	}

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 320562b39686..3dd591068591 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c

@@ -12302,9 +12302,6 @@ static int i40e_xdp(struct net_device *dev,
 	switch (xdp->command) {
 	case XDP_SETUP_PROG:
 		return i40e_xdp_setup(vsi, xdp->prog);
-	case XDP_QUERY_PROG:
-		xdp->prog_id = vsi->xdp_prog ? vsi->xdp_prog->aux->id : 0;
-		return 0;
 	case XDP_SETUP_XSK_UMEM:
 		return i40e_xsk_umem_setup(vsi, xdp->xsk.umem,
 					   xdp->xsk.queue_id);

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 57fd9ee6de66..59dc82c71f9c 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c

@@ -10284,10 +10284,6 @@ static int ixgbe_xdp(struct net_device *dev, struct netdev_bpf *xdp)
 	switch (xdp->command) {
 	case XDP_SETUP_PROG:
 		return ixgbe_xdp_setup(dev, xdp->prog);
-	case XDP_QUERY_PROG:
-		xdp->prog_id = adapter->xdp_prog ?
-			adapter->xdp_prog->aux->id : 0;
-		return 0;
 	case XDP_SETUP_XSK_UMEM:
 		return ixgbe_xsk_umem_setup(adapter, xdp->xsk.umem,
 					    xdp->xsk.queue_id);

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index d189ed247665..80a22d638734 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c

@@ -4485,10 +4485,6 @@ static int ixgbevf_xdp(struct net_device *dev, struct netdev_bpf *xdp)
 	switch (xdp->command) {
 	case XDP_SETUP_PROG:
 		return ixgbevf_xdp_setup(dev, xdp->prog);
-	case XDP_QUERY_PROG:
-		xdp->prog_id = adapter->xdp_prog ?
-			       adapter->xdp_prog->aux->id : 0;
-		return 0;
 	default:
 		return -EINVAL;
 	}

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index c1438ae52a11..8850fc35510a 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c

@@ -2883,35 +2883,11 @@ static int mlx4_xdp_set(struct net_device *dev, struct bpf_prog *prog)
 	return err;
 }
 
-static u32 mlx4_xdp_query(struct net_device *dev)
-{
-	struct mlx4_en_priv *priv = netdev_priv(dev);
-	struct mlx4_en_dev *mdev = priv->mdev;
-	const struct bpf_prog *xdp_prog;
-	u32 prog_id = 0;
-
-	if (!priv->tx_ring_num[TX_XDP])
-		return prog_id;
-
-	mutex_lock(&mdev->state_lock);
-	xdp_prog = rcu_dereference_protected(
-		priv->rx_ring[0]->xdp_prog,
-		lockdep_is_held(&mdev->state_lock));
-	if (xdp_prog)
-		prog_id = xdp_prog->aux->id;
-	mutex_unlock(&mdev->state_lock);
-
-	return prog_id;
-}
-
 static int mlx4_xdp(struct net_device *dev, struct netdev_bpf *xdp)
 {
 	switch (xdp->command) {
 	case XDP_SETUP_PROG:
 		return mlx4_xdp_set(dev, xdp->prog);
-	case XDP_QUERY_PROG:
-		xdp->prog_id = mlx4_xdp_query(dev);
-		return 0;
 	default:
 		return -EINVAL;
 	}

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 457cc39423f2..f1b80c1d9ca2 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c

@@ -4237,29 +4237,11 @@ static int mlx5e_xdp_set(struct net_device *netdev, struct bpf_prog *prog)
 	return err;
 }
 
-static u32 mlx5e_xdp_query(struct net_device *dev)
-{
-	struct mlx5e_priv *priv = netdev_priv(dev);
-	const struct bpf_prog *xdp_prog;
-	u32 prog_id = 0;
-
-	mutex_lock(&priv->state_lock);
-	xdp_prog = priv->channels.params.xdp_prog;
-	if (xdp_prog)
-		prog_id = xdp_prog->aux->id;
-	mutex_unlock(&priv->state_lock);
-
-	return prog_id;
-}
-
 static int mlx5e_xdp(struct net_device *dev, struct netdev_bpf *xdp)
 {
 	switch (xdp->command) {
 	case XDP_SETUP_PROG:
 		return mlx5e_xdp_set(dev, xdp->prog);
-	case XDP_QUERY_PROG:
-		xdp->prog_id = mlx5e_xdp_query(dev);
-		return 0;
 	default:
 		return -EINVAL;
 	}

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index b82b684f52ce..2a9683db54e5 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c

@@ -3548,10 +3548,6 @@ static int nfp_net_xdp(struct net_device *netdev, struct netdev_bpf *xdp)
 		return nfp_net_xdp_setup_drv(nn, xdp);
 	case XDP_SETUP_PROG_HW:
 		return nfp_net_xdp_setup_hw(nn, xdp);
-	case XDP_QUERY_PROG:
-		return xdp_attachment_query(&nn->xdp, xdp);
-	case XDP_QUERY_PROG_HW:
-		return xdp_attachment_query(&nn->xdp_hw, xdp);
 	default:
 		return nfp_app_bpf(nn->app, nn, xdp);
 	}

diff --git a/drivers/net/ethernet/qlogic/qede/qede_filter.c b/drivers/net/ethernet/qlogic/qede/qede_filter.c
index add922b93d2c..69f4e7d37d01 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_filter.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_filter.c

@@ -1118,9 +1118,6 @@ int qede_xdp(struct net_device *dev, struct netdev_bpf *xdp)
 	switch (xdp->command) {
 	case XDP_SETUP_PROG:
 		return qede_xdp_set(edev, xdp->prog);
-	case XDP_QUERY_PROG:
-		xdp->prog_id = edev->xdp_prog ? edev->xdp_prog->aux->id : 0;
-		return 0;
 	default:
 		return -EINVAL;
 	}

diff --git a/drivers/net/netdevsim/bpf.c b/drivers/net/netdevsim/bpf.c
index 2b74425822ab..d03d31721e38 100644
--- a/drivers/net/netdevsim/bpf.c
+++ b/drivers/net/netdevsim/bpf.c

@@ -549,10 +549,6 @@ int nsim_bpf(struct net_device *dev, struct netdev_bpf *bpf)
 	ASSERT_RTNL();
 
 	switch (bpf->command) {
-	case XDP_QUERY_PROG:
-		return xdp_attachment_query(&ns->xdp, bpf);
-	case XDP_QUERY_PROG_HW:
-		return xdp_attachment_query(&ns->xdp_hw, bpf);
 	case XDP_SETUP_PROG:
 		err = nsim_setup_prog_checks(ns, bpf);
 		if (err)

diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 3f398797c2bc..3f75f50b3250 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h

@@ -108,7 +108,7 @@ static inline void nsim_bpf_uninit(struct netdevsim *ns)
 
 static inline int nsim_bpf(struct net_device *dev, struct netdev_bpf *bpf)
 {
-	return bpf->command == XDP_QUERY_PROG ? 0 : -EOPNOTSUPP;
+	return -EOPNOTSUPP;
 }
 
 static inline int nsim_bpf_disable_tc(struct netdevsim *ns)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index abae165dcca5..dbf115aa5c11 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c

@@ -1236,26 +1236,11 @@ static int tun_xdp_set(struct net_device *dev, struct bpf_prog *prog,
 	return 0;
 }
 
-static u32 tun_xdp_query(struct net_device *dev)
-{
-	struct tun_struct *tun = netdev_priv(dev);
-	const struct bpf_prog *xdp_prog;
-
-	xdp_prog = rtnl_dereference(tun->xdp_prog);
-	if (xdp_prog)
-		return xdp_prog->aux->id;
-
-	return 0;
-}
-
 static int tun_xdp(struct net_device *dev, struct netdev_bpf *xdp)
 {
 	switch (xdp->command) {
 	case XDP_SETUP_PROG:
 		return tun_xdp_set(dev, xdp->prog, xdp->extack);
-	case XDP_QUERY_PROG:
-		xdp->prog_id = tun_xdp_query(dev);
-		return 0;
 	default:
 		return -EINVAL;
 	}

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 52110e54e621..27b93e8b844d 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c

@@ -1097,26 +1097,11 @@ static int veth_xdp_set(struct net_device *dev, struct bpf_prog *prog,
 	return err;
 }
 
-static u32 veth_xdp_query(struct net_device *dev)
-{
-	struct veth_priv *priv = netdev_priv(dev);
-	const struct bpf_prog *xdp_prog;
-
-	xdp_prog = priv->_xdp_prog;
-	if (xdp_prog)
-		return xdp_prog->aux->id;
-
-	return 0;
-}
-
 static int veth_xdp(struct net_device *dev, struct netdev_bpf *xdp)
 {
 	switch (xdp->command) {
 	case XDP_SETUP_PROG:
 		return veth_xdp_set(dev, xdp->prog, xdp->extack);
-	case XDP_QUERY_PROG:
-		xdp->prog_id = veth_xdp_query(dev);
-		return 0;
 	default:
 		return -EINVAL;
 	}

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 0d4115c9e20b..2517bd5c74b4 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c

@@ -2513,28 +2513,11 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
 	return err;
 }
 
-static u32 virtnet_xdp_query(struct net_device *dev)
-{
-	struct virtnet_info *vi = netdev_priv(dev);
-	const struct bpf_prog *xdp_prog;
-	int i;
-
-	for (i = 0; i < vi->max_queue_pairs; i++) {
-		xdp_prog = rtnl_dereference(vi->rq[i].xdp_prog);
-		if (xdp_prog)
-			return xdp_prog->aux->id;
-	}
-	return 0;
-}
-
 static int virtnet_xdp(struct net_device *dev, struct netdev_bpf *xdp)
 {
 	switch (xdp->command) {
 	case XDP_SETUP_PROG:
 		return virtnet_xdp_set(dev, xdp->prog, xdp->extack);
-	case XDP_QUERY_PROG:
-		xdp->prog_id = virtnet_xdp_query(dev);
-		return 0;
 	default:
 		return -EINVAL;
 	}

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index f3a875a52c6c..83236af25590 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h

@@ -864,8 +864,6 @@ enum bpf_netdev_command {
 	 */
 	XDP_SETUP_PROG,
 	XDP_SETUP_PROG_HW,
-	XDP_QUERY_PROG,
-	XDP_QUERY_PROG_HW,
 	/* BPF program for offload callbacks, invoked at program load time. */
 	BPF_OFFLOAD_MAP_ALLOC,
 	BPF_OFFLOAD_MAP_FREE,

@@ -885,12 +883,6 @@ struct netdev_bpf {
 			struct bpf_prog *prog;
 			struct netlink_ext_ack *extack;
 		};
-		/* XDP_QUERY_PROG, XDP_QUERY_PROG_HW */
-		struct {
-			u32 prog_id;
-			/* flags with which program was installed */
-			u32 prog_flags;
-		};
 		/* BPF_OFFLOAD_MAP_ALLOC, BPF_OFFLOAD_MAP_FREE */
 		struct {
 			struct bpf_offloaded_map *offmap;

diff --git a/include/net/xdp.h b/include/net/xdp.h
index 3691280c8fc1..abcd86a31c0f 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h

@@ -160,8 +160,6 @@ struct xdp_attachment_info {
 };
 
 struct netdev_bpf;
-int xdp_attachment_query(struct xdp_attachment_info *info,
-			 struct netdev_bpf *bpf);
 bool xdp_attachment_flags_ok(struct xdp_attachment_info *info,
 			     struct netdev_bpf *bpf);
 void xdp_attachment_setup(struct xdp_attachment_info *info,

diff --git a/net/core/dev.c b/net/core/dev.c
index 1a9da508149a..4e7c81a76400 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c

@@ -5156,11 +5156,6 @@ static int generic_xdp_install(struct net_device *dev, struct netdev_bpf *xdp)
 			dev_disable_gro_hw(dev);
 		}
 		break;
-
-	case XDP_QUERY_PROG:
-		xdp->prog_id = old ? old->aux->id : 0;
-		break;
-
 	default:
 		ret = -EINVAL;
 		break;

diff --git a/net/core/xdp.c b/net/core/xdp.c
index af92c04a58d8..295b4d4fd259 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c

@@ -379,15 +379,6 @@ void xdp_return_buff(struct xdp_buff *xdp)
 }
 EXPORT_SYMBOL_GPL(xdp_return_buff);
 
-int xdp_attachment_query(struct xdp_attachment_info *info,
-			 struct netdev_bpf *bpf)
-{
-	bpf->prog_id = info->prog ? info->prog->aux->id : 0;
-	bpf->prog_flags = info->prog ? info->flags : 0;
-	return 0;
-}
-EXPORT_SYMBOL_GPL(xdp_attachment_query);
-
 bool xdp_prog_flags_ok(u32 old_flags, u32 new_flags,
 		       struct netlink_ext_ack *extack)
 {

-- 
2.20.1

On Fri, 31 May 2019 at 11:42, Björn Töpel [off-list ref] wrote:

[...]

I, hopefully, addressed all comments from Jakub and Saeed, except one;
I did not move the XDP struct net_device into a struct of its own.

Uhm, the last sentence was weird.

What I meant was: I did not move the newly introduced XDP members
(flags/hw prog) into a struct of its own.

On Fri, 2019-05-31 at 11:42 +0200, Björn Töpel wrote:

From: Björn Töpel <redacted>

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 two new members: xdp_prog_hw and
xdp_flags. The former is the offloaded XDP program, if any, and the
latter tracks the flags that the supplied when attaching the XDP
program. The flags only apply to SKB_MODE or DRV_MODE, not HW_MODE.

The xdp_prog member, previously only used for SKB_MODE, is shared
with
DRV_MODE. This is OK, due to the fact that SKB_MODE and DRV_MODE are
mutually exclusive. To differentiate between the two modes, a new
internal flag is introduced as well.

Just thinking out loud, why can't we allow any combination of
HW/DRV/SKB modes? they are totally different attach points in a totally
different checkpoints in a frame life cycle.

Down the road i think we will utilize this fact and start introducing
SKB helpers for SKB mode and driver helpers for DRV mode..

if i am reading this correctly this is equivalent to :

if (!offload && (curre_mode != mode)) 
offlad is false then curr_mode and mode must be DRV or GENERIC .. 

better if you keep bitwise operations for actual bitmasks, mode and
curr_mode are not bitmask, they can hold one value each .. according to
your logic.. 

Hi "Björn,

I love your patch! Perhaps something to improve:

[auto build test WARNING on bpf-next/master]

url:    https://github.com/0day-ci/linux/commits/Bj-rn-T-pel/net-xdp-refactor-XDP_QUERY_PROG-_HW-to-netdev/20190601-053952
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
reproduce: make htmldocs

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <redacted>

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:344: warning: Function parameter or member 'range' not described in 'amdgpu_mn_invalidate_range_end'
   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:344: warning: Excess function parameter 'mm' description in 'amdgpu_mn_invalidate_range_end'
   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:344: warning: Excess function parameter 'start' description in 'amdgpu_mn_invalidate_range_end'
   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:344: warning: Excess function parameter 'end' description in 'amdgpu_mn_invalidate_range_end'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:347: warning: cannot understand function prototype: 'struct amdgpu_vm_pt_cursor '
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:348: warning: cannot understand function prototype: 'struct amdgpu_vm_pt_cursor '
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:494: warning: Function parameter or member 'start' not described in 'amdgpu_vm_pt_first_dfs'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:546: warning: Function parameter or member 'adev' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:546: warning: Function parameter or member 'vm' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:546: warning: Function parameter or member 'start' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:546: warning: Function parameter or member 'cursor' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:546: warning: Function parameter or member 'entry' not described in 'for_each_amdgpu_vm_pt_dfs_safe'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:823: warning: Function parameter or member 'level' not described in 'amdgpu_vm_bo_param'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'params' not described in 'amdgpu_vm_update_flags'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'bo' not described in 'amdgpu_vm_update_flags'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'level' not described in 'amdgpu_vm_update_flags'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'pe' not described in 'amdgpu_vm_update_flags'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'addr' not described in 'amdgpu_vm_update_flags'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'count' not described in 'amdgpu_vm_update_flags'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'incr' not described in 'amdgpu_vm_update_flags'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:1285: warning: Function parameter or member 'flags' not described in 'amdgpu_vm_update_flags'
   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:2812: warning: Function parameter or member 'pasid' not described in 'amdgpu_vm_make_compute'
   drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c:375: warning: Excess function parameter 'entry' description in 'amdgpu_irq_dispatch'
   drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c:376: warning: Function parameter or member 'ih' not described in 'amdgpu_irq_dispatch'
   drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c:376: warning: Excess function parameter 'entry' description in 'amdgpu_irq_dispatch'
   drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:1: warning: no structured comments found
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:128: warning: Incorrect use of kernel-doc format: Documentation Makefile include scripts source @atomic_obj
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:210: warning: Function parameter or member 'atomic_obj' not described in 'amdgpu_display_manager'
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:210: warning: Function parameter or member 'backlight_link' not described in 'amdgpu_display_manager'
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:210: warning: Function parameter or member 'backlight_caps' not described in 'amdgpu_display_manager'
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:210: warning: Function parameter or member 'freesync_module' not described in 'amdgpu_display_manager'
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:210: warning: Function parameter or member 'fw_dmcu' not described in 'amdgpu_display_manager'
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:210: warning: Function parameter or member 'dmcu_fw_version' not described in 'amdgpu_display_manager'
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c:1: warning: no structured comments found
   include/drm/drm_drv.h:722: warning: Function parameter or member 'gem_prime_pin' not described in 'drm_driver'
   include/drm/drm_drv.h:722: warning: Function parameter or member 'gem_prime_unpin' not described in 'drm_driver'
   include/drm/drm_drv.h:722: warning: Function parameter or member 'gem_prime_res_obj' not described in 'drm_driver'
   include/drm/drm_drv.h:722: warning: Function parameter or member 'gem_prime_get_sg_table' not described in 'drm_driver'
   include/drm/drm_drv.h:722: warning: Function parameter or member 'gem_prime_import_sg_table' not described in 'drm_driver'
   include/drm/drm_drv.h:722: warning: Function parameter or member 'gem_prime_vmap' not described in 'drm_driver'
   include/drm/drm_drv.h:722: warning: Function parameter or member 'gem_prime_vunmap' not described in 'drm_driver'
   include/drm/drm_drv.h:722: warning: Function parameter or member 'gem_prime_mmap' not described in 'drm_driver'
   include/drm/drm_modeset_helper_vtables.h:1004: warning: Function parameter or member 'prepare_writeback_job' not described in 'drm_connector_helper_funcs'
   include/drm/drm_modeset_helper_vtables.h:1004: warning: Function parameter or member 'cleanup_writeback_job' not described in 'drm_connector_helper_funcs'
   include/drm/drm_atomic_state_helper.h:1: warning: no structured comments found
   drivers/gpu/drm/scheduler/sched_main.c:419: warning: Function parameter or member 'full_recovery' not described in 'drm_sched_start'
   drivers/gpu/drm/i915/i915_vma.h:50: warning: cannot understand function prototype: 'struct i915_vma '
   drivers/gpu/drm/i915/i915_vma.h:1: warning: no structured comments found
   drivers/gpu/drm/i915/intel_guc_fwif.h:536: warning: cannot understand function prototype: 'struct guc_log_buffer_state '
   drivers/gpu/drm/i915/i915_trace.h:1: warning: no structured comments found
   drivers/gpu/drm/i915/i915_reg.h:156: warning: bad line:
   include/linux/interconnect.h:1: warning: no structured comments found
   include/linux/skbuff.h:897: warning: Function parameter or member 'dev_scratch' not described in 'sk_buff'
   include/linux/skbuff.h:897: warning: Function parameter or member 'list' not described in 'sk_buff'
   include/linux/skbuff.h:897: warning: Function parameter or member 'ip_defrag_offset' not described in 'sk_buff'
   include/linux/skbuff.h:897: warning: Function parameter or member 'skb_mstamp_ns' not described in 'sk_buff'
   include/linux/skbuff.h:897: warning: Function parameter or member '__cloned_offset' not described in 'sk_buff'
   include/linux/skbuff.h:897: warning: Function parameter or member 'head_frag' not described in 'sk_buff'
   include/linux/skbuff.h:897: warning: Function parameter or member '__pkt_type_offset' not described in 'sk_buff'
   include/linux/skbuff.h:897: warning: Function parameter or member 'encapsulation' not described in 'sk_buff'
   include/linux/skbuff.h:897: warning: Function parameter or member 'encap_hdr_csum' not described in 'sk_buff'
   include/linux/skbuff.h:897: warning: Function parameter or member 'csum_valid' not described in 'sk_buff'
   include/linux/skbuff.h:897: warning: Function parameter or member '__pkt_vlan_present_offset' not described in 'sk_buff'
   include/linux/skbuff.h:897: warning: Function parameter or member 'vlan_present' not described in 'sk_buff'
   include/linux/skbuff.h:897: warning: Function parameter or member 'csum_complete_sw' not described in 'sk_buff'
   include/linux/skbuff.h:897: warning: Function parameter or member 'csum_level' not described in 'sk_buff'
   include/linux/skbuff.h:897: warning: Function parameter or member 'inner_protocol_type' not described in 'sk_buff'
   include/linux/skbuff.h:897: warning: Function parameter or member 'remcsum_offload' not described in 'sk_buff'
   include/linux/skbuff.h:897: warning: Function parameter or member 'sender_cpu' not described in 'sk_buff'
   include/linux/skbuff.h:897: warning: Function parameter or member 'reserved_tailroom' not described in 'sk_buff'
   include/linux/skbuff.h:897: warning: Function parameter or member 'inner_ipproto' not described in 'sk_buff'
   include/net/sock.h:238: warning: Function parameter or member 'skc_addrpair' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_portpair' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_ipv6only' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_net_refcnt' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_v6_daddr' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_v6_rcv_saddr' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_cookie' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_listener' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_tw_dr' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_rcv_wnd' not described in 'sock_common'
   include/net/sock.h:238: warning: Function parameter or member 'skc_tw_rcv_nxt' not described in 'sock_common'
   include/net/sock.h:520: warning: Function parameter or member 'sk_rx_skb_cache' not described in 'sock'
   include/net/sock.h:520: warning: Function parameter or member 'sk_wq_raw' not described in 'sock'
   include/net/sock.h:520: warning: Function parameter or member 'tcp_rtx_queue' not described in 'sock'
   include/net/sock.h:520: warning: Function parameter or member 'sk_tx_skb_cache' not described in 'sock'
   include/net/sock.h:520: warning: Function parameter or member 'sk_route_forced_caps' not described in 'sock'
   include/net/sock.h:520: warning: Function parameter or member 'sk_txtime_report_errors' not described in 'sock'
   include/net/sock.h:520: warning: Function parameter or member 'sk_validate_xmit_skb' not described in 'sock'
   include/net/sock.h:520: warning: Function parameter or member 'sk_bpf_storage' not described in 'sock'
   include/linux/netdevice.h:2047: warning: Function parameter or member 'gso_partial_features' not described in 'net_device'
   include/linux/netdevice.h:2047: warning: Function parameter or member 'l3mdev_ops' not described in 'net_device'
   include/linux/netdevice.h:2047: warning: Function parameter or member 'xfrmdev_ops' not described in 'net_device'
   include/linux/netdevice.h:2047: warning: Function parameter or member 'tlsdev_ops' not described in 'net_device'
   include/linux/netdevice.h:2047: warning: Function parameter or member 'name_assign_type' not described in 'net_device'
   include/linux/netdevice.h:2047: warning: Function parameter or member 'ieee802154_ptr' not described in 'net_device'
   include/linux/netdevice.h:2047: warning: Function parameter or member 'mpls_ptr' not described in 'net_device'
   include/linux/netdevice.h:2047: warning: Function parameter or member 'xdp_prog' not described in 'net_device'
   include/linux/netdevice.h:2047: warning: Function parameter or member 'gro_flush_timeout' not described in 'net_device'
   include/linux/netdevice.h:2047: warning: Function parameter or member 'nf_hooks_ingress' not described in 'net_device'

quoted

include/linux/netdevice.h:2047: warning: Function parameter or member 'xdp_prog_hw' not described in 'net_device'
include/linux/netdevice.h:2047: warning: Function parameter or member 'xdp_flags' not described in 'net_device'

   include/linux/netdevice.h:2047: warning: Function parameter or member '____cacheline_aligned_in_smp' not described in 'net_device'
   include/linux/netdevice.h:2047: warning: Function parameter or member 'qdisc_hash' not described in 'net_device'
   include/linux/netdevice.h:2047: warning: Function parameter or member 'xps_cpus_map' not described in 'net_device'
   include/linux/netdevice.h:2047: warning: Function parameter or member 'xps_rxqs_map' not described in 'net_device'
   include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(advertising' not described in 'phylink_link_state'
   include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising' not described in 'phylink_link_state'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'quotactl' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'quota_on' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'sb_free_mnt_opts' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'sb_eat_lsm_opts' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'sb_kern_mount' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'sb_show_options' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'sb_add_mnt_opt' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'd_instantiate' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'getprocattr' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1811: warning: Function parameter or member 'setprocattr' not described in 'security_list_options'
   Documentation/admin-guide/mm/numaperf.rst:24: ERROR: Unexpected indentation.
   Documentation/admin-guide/mm/numaperf.rst:24: WARNING: Inline substitution_reference start-string without end-string.
   Documentation/admin-guide/mm/numaperf.rst:25: WARNING: Block quote ends without a blank line; unexpected unindent.
   Documentation/bpf/btf.rst:154: ERROR: Unexpected indentation.
   Documentation/bpf/btf.rst:163: ERROR: Unexpected indentation.
   lib/list_sort.c:128: WARNING: Definition list ends without a blank line; unexpected unindent.
   lib/list_sort.c:161: ERROR: Unexpected indentation.
   lib/list_sort.c:162: WARNING: Block quote ends without a blank line; unexpected unindent.
   include/linux/xarray.h:232: ERROR: Unexpected indentation.
   kernel/time/hrtimer.c:1120: WARNING: Block quote ends without a blank line; unexpected unindent.
   kernel/signal.c:349: WARNING: Inline literal start-string without end-string.
   include/uapi/linux/firewire-cdev.h:312: WARNING: Inline literal start-string without end-string.
   Documentation/driver-api/gpio/driver.rst:419: ERROR: Unknown target name: "devm".
   include/linux/i2c.h:510: WARNING: Inline strong start-string without end-string.
   drivers/ata/libata-core.c:5944: ERROR: Unknown target name: "hw".
   drivers/message/fusion/mptbase.c:5057: WARNING: Definition list ends without a blank line; unexpected unindent.
   drivers/tty/serial/serial_core.c:1959: WARNING: Definition list ends without a blank line; unexpected unindent.
   include/linux/regulator/driver.h:289: ERROR: Unknown target name: "regulator_regmap_x_voltage".
   Documentation/driver-api/soundwire/locking.rst:50: ERROR: Inconsistent literal block quoting.
   Documentation/driver-api/soundwire/locking.rst:51: WARNING: Line block ends without a blank line.
   Documentation/driver-api/soundwire/locking.rst:55: WARNING: Inline substitution_reference start-string without end-string.
   Documentation/driver-api/soundwire/locking.rst:56: WARNING: Line block ends without a blank line.
   include/linux/spi/spi.h:380: ERROR: Unexpected indentation.
   fs/posix_acl.c:636: WARNING: Inline emphasis start-string without end-string.
   fs/debugfs/inode.c:385: WARNING: Inline literal start-string without end-string.
   fs/debugfs/inode.c:464: WARNING: Inline literal start-string without end-string.
   fs/debugfs/inode.c:496: WARNING: Inline literal start-string without end-string.
   fs/debugfs/inode.c:583: WARNING: Inline literal start-string without end-string.
   fs/debugfs/file.c:394: WARNING: Inline literal start-string without end-string.
   fs/debugfs/file.c:400: WARNING: Inline literal start-string without end-string.
   fs/debugfs/file.c:439: WARNING: Inline literal start-string without end-string.
   fs/debugfs/file.c:445: WARNING: Inline literal start-string without end-string.
   fs/debugfs/file.c:484: WARNING: Inline literal start-string without end-string.
   fs/debugfs/file.c:490: WARNING: Inline literal start-string without end-string.
   fs/debugfs/file.c:530: WARNING: Inline literal start-string without end-string.
   fs/debugfs/file.c:536: WARNING: Inline literal start-string without end-string.
   fs/debugfs/file.c:578: WARNING: Inline literal start-string without end-string.
   fs/debugfs/file.c:584: WARNING: Inline literal start-string without end-string.
   fs/debugfs/file.c:845: WARNING: Inline literal start-string without end-string.
   fs/debugfs/file.c:851: WARNING: Inline literal start-string without end-string.
   fs/debugfs/file.c:898: WARNING: Inline literal start-string without end-string.
   fs/debugfs/file.c:904: WARNING: Inline literal start-string without end-string.
   fs/debugfs/file.c:1001: WARNING: Inline literal start-string without end-string.
   fs/debugfs/file.c:1001: WARNING: Inline literal start-string without end-string.
   fs/debugfs/file.c:1096: WARNING: Inline literal start-string without end-string.
   fs/debugfs/file.c:1102: WARNING: Inline literal start-string without end-string.
   Documentation/firmware-guide/acpi/enumeration.rst:430: ERROR: Unexpected indentation.
   Documentation/firmware-guide/acpi/enumeration.rst:432: ERROR: Unexpected indentation.
   Documentation/firmware-guide/acpi/enumeration.rst:434: ERROR: Unexpected indentation.
   Documentation/firmware-guide/acpi/enumeration.rst:435: WARNING: Block quote ends without a blank line; unexpected unindent.
   Documentation/firmware-guide/acpi/enumeration.rst:436: WARNING: Block quote ends without a blank line; unexpected unindent.
   Documentation/firmware-guide/acpi/enumeration.rst:439: ERROR: Unexpected indentation.
   Documentation/firmware-guide/acpi/enumeration.rst:441: ERROR: Unexpected indentation.
   Documentation/firmware-guide/acpi/enumeration.rst:445: WARNING: Block quote ends without a blank line; unexpected unindent.
   Documentation/firmware-guide/acpi/enumeration.rst:447: WARNING: Block quote ends without a blank line; unexpected unindent.
   Documentation/firmware-guide/acpi/enumeration.rst:448: WARNING: Block quote ends without a blank line; unexpected unindent.
   drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:2024: WARNING: Inline emphasis start-string without end-string.
   drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:2026: WARNING: Inline emphasis start-string without end-string.
   Documentation/networking/af_xdp.rst:319: WARNING: Literal block expected; none found.
   Documentation/networking/af_xdp.rst:326: WARNING: Literal block expected; none found.
   Documentation/networking/device_drivers/freescale/dpaa2/dpio-driver.rst:43: WARNING: Definition list ends without a blank line; unexpected unindent.
   Documentation/networking/device_drivers/freescale/dpaa2/dpio-driver.rst:63: ERROR: Unexpected indentation.
   Documentation/networking/dsa/sja1105.rst:91: WARNING: Block quote ends without a blank line; unexpected unindent.
   Documentation/networking/dsa/sja1105.rst:91: WARNING: Block quote ends without a blank line; unexpected unindent.
   include/linux/netdevice.h:3492: WARNING: Inline emphasis start-string without end-string.
   include/linux/netdevice.h:3492: WARNING: Inline emphasis start-string without end-string.
   net/core/dev.c:4999: ERROR: Unknown target name: "page_is".
   Documentation/powerpc/isa-versions.rst:15: ERROR: Malformed table.
   Column span alignment problem in table line 10.

vim +2047 include/linux/netdevice.h

^1da177e4 Linus Torvalds        2005-04-16  2014  
43cb76d91 Greg Kroah-Hartman    2002-04-09  2015  	struct device		dev;
0c509a6c9 Eric W. Biederman     2009-10-29  2016  	const struct attribute_group *sysfs_groups[4];
a953be53c Michael Dalton        2014-01-16  2017  	const struct attribute_group *sysfs_rx_queue_group;
38f7b870d Patrick McHardy       2007-06-13  2018  
38f7b870d Patrick McHardy       2007-06-13  2019  	const struct rtnl_link_ops *rtnl_link_ops;
f25f4e448 Peter P Waskiewicz Jr 2007-07-06  2020  
82cc1a7a5 Peter P Waskiewicz Jr 2008-03-21  2021  	/* for setting kernel sock attribute on TCP connection setup */
82cc1a7a5 Peter P Waskiewicz Jr 2008-03-21  2022  #define GSO_MAX_SIZE		65536
82cc1a7a5 Peter P Waskiewicz Jr 2008-03-21  2023  	unsigned int		gso_max_size;
30b678d84 Ben Hutchings         2012-07-30  2024  #define GSO_MAX_SEGS		65535
30b678d84 Ben Hutchings         2012-07-30  2025  	u16			gso_max_segs;
743b03a83 Eric Dumazet          2016-04-09  2026  
7a6b6f515 Jeff Kirsher          2008-11-25  2027  #ifdef CONFIG_DCB
329535432 Stephen Hemminger     2009-10-05  2028  	const struct dcbnl_rtnl_ops *dcbnl_ops;
2f90b8657 Alexander Duyck       2008-11-20  2029  #endif
ffcfe25bb Alexander Duyck       2018-07-09  2030  	s16			num_tc;
4f57c087d John Fastabend        2011-01-17  2031  	struct netdev_tc_txq	tc_to_txq[TC_MAX_QUEUE];
4f57c087d John Fastabend        2011-01-17  2032  	u8			prio_tc_map[TC_BITMASK + 1];
2f90b8657 Alexander Duyck       2008-11-20  2033  
d11ead756 Ben Hutchings         2011-11-25  2034  #if IS_ENABLED(CONFIG_FCOE)
4d288d576 Yi Zou                2009-02-27  2035  	unsigned int		fcoe_ddp_xid;
4d288d576 Yi Zou                2009-02-27  2036  #endif
86f8515f9 Daniel Borkmann       2013-12-29  2037  #if IS_ENABLED(CONFIG_CGROUP_NET_PRIO)
5bc1421e3 Neil Horman           2011-11-22  2038  	struct netprio_map __rcu *priomap;
5bc1421e3 Neil Horman           2011-11-22  2039  #endif
c1f19b51d Richard Cochran       2010-07-17  2040  	struct phy_device	*phydev;
e679c9c1d Russell King          2018-03-28  2041  	struct sfp_bus		*sfp_bus;
23d3b8bfb Eric Dumazet          2012-09-05  2042  	struct lock_class_key	*qdisc_tx_busylock;
f9eb8aea2 Eric Dumazet          2016-06-06  2043  	struct lock_class_key	*qdisc_running_key;
d746d707a Anuradha Karuppiah    2015-07-14  2044  	bool			proto_down;
619411432 Heiner Kallweit       2018-09-24  2045  	unsigned		wol_enabled:1;
^1da177e4 Linus Torvalds        2005-04-16  2046  };
43cb76d91 Greg Kroah-Hartman    2002-04-09 @2047  #define to_net_dev(d) container_of(d, struct net_device, dev)
^1da177e4 Linus Torvalds        2005-04-16  2048  

:::::: The code at line 2047 was first introduced by commit
:::::: 43cb76d91ee85f579a69d42bc8efc08bac560278 Network: convert network devices to use struct device instead of class_device

:::::: TO: Greg Kroah-Hartman [off-list ref]
:::::: CC: Greg Kroah-Hartman [off-list ref]

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

On 31 May 2019, at 2:42, Björn Töpel wrote:

From: Björn Töpel <redacted>

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 two new members: xdp_prog_hw and
xdp_flags. The former is the offloaded XDP program, if any, and the
latter tracks the flags that the supplied when attaching the XDP
program. The flags only apply to SKB_MODE or DRV_MODE, not HW_MODE.

The xdp_prog member, previously only used for SKB_MODE, is shared with
DRV_MODE. This is OK, due to the fact that SKB_MODE and DRV_MODE are
mutually exclusive. To differentiate between the two modes, a new
internal flag is introduced as well.

I'm not entirely clear why this new flag is needed - GENERIC seems to
be an alias for SKB_MODE, so why just use SKB_MODE directly?

If the user does not explicitly specify a type (skb|drv|hw), then the
command should choose the correct type and then behave as if this type
was specified.

The logic in dev_change_xdp_fd() is too complicated.  It disallows
setting (drv|skb), but allows (hw|skb), which I'm not sure is 
intentional.

It should be clearer as to which combinations are allowed.
-- 
Jonathan

	return xdp->flags & XDP_FLAGS_MODE;

On Fri, 31 May 2019 19:18:17 +0000, Saeed Mahameed wrote:

On Fri, 2019-05-31 at 11:42 +0200, Björn Töpel wrote:

quoted

From: Björn Töpel <redacted>

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 two new members: xdp_prog_hw and
xdp_flags. The former is the offloaded XDP program, if any, and the
latter tracks the flags that the supplied when attaching the XDP
program. The flags only apply to SKB_MODE or DRV_MODE, not HW_MODE.

The xdp_prog member, previously only used for SKB_MODE, is shared
with
DRV_MODE. This is OK, due to the fact that SKB_MODE and DRV_MODE are
mutually exclusive. To differentiate between the two modes, a new
internal flag is introduced as well.  

Just thinking out loud, why can't we allow any combination of
HW/DRV/SKB modes? they are totally different attach points in a totally
different checkpoints in a frame life cycle.

FWIW see Message-ID: [off-list ref]

Down the road i think we will utilize this fact and start introducing
SKB helpers for SKB mode and driver helpers for DRV mode..

Any reason why we would want the extra complexity?  There is cls_bpf
if someone wants skb features after all..

On Fri, 31 May 2019 19:18:17 +0000, Saeed Mahameed wrote:

quoted

+	if (!bpf_op || flags & XDP_FLAGS_SKB_MODE)
+		mode = XDP_FLAGS_SKB_MODE;
+
+	curr_mode = dev_xdp_current_mode(dev);
+
+	if (!offload && curr_mode && (mode ^ curr_mode) &
+	    (XDP_FLAGS_DRV_MODE | XDP_FLAGS_SKB_MODE)) {  

if i am reading this correctly this is equivalent to :

if (!offload && (curre_mode != mode)) 
offlad is false then curr_mode and mode must be DRV or GENERIC .. 

Naw, if curr_mode is not set, i.e. nothing installed now, we don't care
about the diff.

better if you keep bitwise operations for actual bitmasks, mode and
curr_mode are not bitmask, they can hold one value each .. according to
your logic.. 

Well, they hold one bit each, whether one bit is a bitmap perhaps is
disputable? :)

I think the logic is fine.

What happened to my request to move the change in behaviour for
disabling to a separate patch, tho, Bjorn? :)

On Fri, 31 May 2019 11:42:14 +0200, Björn Töpel wrote:

IDK if we should pay the cost of this pointer for every netdev on the
system just for the single production driver out there that implements
HW offload :(  I'm on the fence about this..

+	u32			xdp_flags;
+
 /*
  * Cache lines mostly used on transmit path
  */

So you remove all the attr and flag params just to add a conversion
helpers to get them based on mode?  Why?  Seems like unnecessary churn,
and questionable change :S

Otherwise looks good to me!

On Sat, 1 Jun 2019 at 20:12, Jonathan Lemon [off-list ref] wrote:

On 31 May 2019, at 2:42, Björn Töpel wrote:

quoted

From: Björn Töpel <redacted>

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 two new members: xdp_prog_hw and
xdp_flags. The former is the offloaded XDP program, if any, and the
latter tracks the flags that the supplied when attaching the XDP
program. The flags only apply to SKB_MODE or DRV_MODE, not HW_MODE.

The xdp_prog member, previously only used for SKB_MODE, is shared with
DRV_MODE. This is OK, due to the fact that SKB_MODE and DRV_MODE are
mutually exclusive. To differentiate between the two modes, a new
internal flag is introduced as well.

I'm not entirely clear why this new flag is needed - GENERIC seems to
be an alias for SKB_MODE, so why just use SKB_MODE directly?

If the user does not explicitly specify a type (skb|drv|hw), then the
command should choose the correct type and then behave as if this type
was specified.

Yes, this is kind of hairy.

SKB and DRV are mutually exclusive, but HW is not. IOW, valid options are:
SKB, DRV, HW, SKB+HW DRV+HW.

What complicates things further, is that SKB and DRV can be implicitly
(auto/no flags) or explicitly enabled (flags).

If a user doesn't pass any flags, the "best supported mode" should be
selected. If this "auto mode" is used, it should be seen as a third
mode. E.g.

ip link set dev eth0 xdp on -- OK
ip link set dev eth0 xdp off -- OK

ip link set dev eth0 xdp on -- OK # generic auto selected
ip link set dev eth0 xdpgeneric off -- NOK, bad flags

ip link set dev eth0 xdp on -- OK # drv auto selected
ip link set dev eth0 xdpdrv off -- NOK, bad flags

...and so on. The idea is that a user should use the same set of flags always.

The internal "GENERIC" flag is only to determine if the xdp_prog
represents a DRV version or SKB version. Maybe it would be clearer
just to add an additional xdp_prog_drv to the net_device, instead?

The logic in dev_change_xdp_fd() is too complicated.  It disallows
setting (drv|skb), but allows (hw|skb), which I'm not sure is
intentional.

It should be clearer as to which combinations are allowed.

Fair point. I'll try to clean it up further.


Björn

--
Jonathan

quoted

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. Now that xdp_prog member is shared from a query
program perspective, 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.

Signed-off-by: Björn Töpel <redacted>
---
 include/linux/netdevice.h |  15 +++--
 include/net/xdp.h         |   4 ++
 net/core/dev.c            | 138
++++++++++++++++++++++++--------------
 net/core/rtnetlink.c      |  53 +++++++--------
 net/core/xdp.c            |  17 +++--
 5 files changed, 139 insertions(+), 88 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 44b47e9df94a..f3a875a52c6c 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h

@@ -1940,6 +1940,9 @@ struct net_device {
 #endif
      struct hlist_node       index_hlist;

+     struct bpf_prog         *xdp_prog_hw;
+     u32                     xdp_flags;
+
 /*
  * Cache lines mostly used on transmit path
  */

@@ -2043,11 +2046,14 @@ struct net_device {
 };
 #define to_net_dev(d) container_of(d, struct net_device, dev)

+/* Reusing the XDP flags space for kernel internal flag */
+#define XDP_FLAGS_KERN_GENERIC (1U << 4)
+static_assert(!(XDP_FLAGS_KERN_GENERIC & XDP_FLAGS_MASK));
+
 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_KERN_GENERIC;
 }

 #define      NETDEV_ALIGN            32

@@ -3684,8 +3690,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 mode);
 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/include/net/xdp.h b/include/net/xdp.h
index 0f25b3675c5c..3691280c8fc1 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h

@@ -51,6 +51,7 @@ struct xdp_mem_info {
 };

 struct page_pool;
+struct netlink_ext_ack;

 struct zero_copy_allocator {
      void (*free)(struct zero_copy_allocator *zca, unsigned long handle);

@@ -166,4 +167,7 @@ bool xdp_attachment_flags_ok(struct

xdp_attachment_info *info,
 void xdp_attachment_setup(struct xdp_attachment_info *info,
                        struct netdev_bpf *bpf);

+bool xdp_prog_flags_ok(u32 old_flags, u32 new_flags,
+                    struct netlink_ext_ack *extack);
+
 #endif /* __LINUX_NET_XDP_H__ */

diff --git a/net/core/dev.c b/net/core/dev.c
index b6b8505cfb3e..1a9da508149a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c

@@ -8005,21 +8005,43 @@ 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)
+static u32 dev_xdp_query_generic(struct net_device *dev)
 {
-     struct netdev_bpf xdp;
+     struct bpf_prog *prog = rtnl_dereference(dev->xdp_prog);

-     if (!bpf_op)
-             return 0;
+     return prog && dev->xdp_flags & XDP_FLAGS_KERN_GENERIC ?
+             prog->aux->id : 0;
+}

-     memset(&xdp, 0, sizeof(xdp));
-     xdp.command = cmd;
+static u32 dev_xdp_query_drv(struct net_device *dev)
+{
+     struct bpf_prog *prog = rtnl_dereference(dev->xdp_prog);
+
+     return prog && !(dev->xdp_flags & XDP_FLAGS_KERN_GENERIC) ?
+             prog->aux->id : 0;
+}
+
+static u32 dev_xdp_current_mode(struct net_device *dev)
+{
+     struct bpf_prog *prog = rtnl_dereference(dev->xdp_prog);

-     /* Query must always succeed. */
-     WARN_ON(bpf_op(dev, &xdp) < 0 && cmd == XDP_QUERY_PROG);
+     if (prog)
+             return dev_xdp_query_generic(dev) ? XDP_FLAGS_SKB_MODE :
+                     XDP_FLAGS_DRV_MODE;

        return xdp->flags & XDP_FLAGS_MODE;

quoted

+     return 0;
+}

-     return xdp.prog_id;
+u32 dev_xdp_query(struct net_device *dev, unsigned int mode)
+{
+     switch (mode) {
+     case XDP_FLAGS_DRV_MODE:
+             return dev_xdp_query_drv(dev);
+     case XDP_FLAGS_SKB_MODE:
+             return dev_xdp_query_generic(dev);
+     case XDP_FLAGS_HW_MODE:
+             return dev->xdp_prog_hw ? dev->xdp_prog_hw->aux->id : 0;
+     }
+     return 0;
 }

 static int dev_xdp_install(struct net_device *dev, bpf_op_t bpf_op,

@@ -8027,45 +8049,47 @@ static int dev_xdp_install(struct net_device

*dev, bpf_op_t bpf_op,
                         struct bpf_prog *prog)
 {
      struct netdev_bpf xdp;
+     int err;

      memset(&xdp, 0, sizeof(xdp));
-     if (flags & XDP_FLAGS_HW_MODE)
+     if (flags & XDP_FLAGS_HW_MODE) {
              xdp.command = XDP_SETUP_PROG_HW;
-     else
+             xdp.flags = XDP_FLAGS_HW_MODE;
+     } else {
              xdp.command = XDP_SETUP_PROG;
+             xdp.flags = flags;
+     }
      xdp.extack = extack;
-     xdp.flags = flags;
      xdp.prog = prog;

-     return bpf_op(dev, &xdp);
+     err = bpf_op(dev, &xdp);
+     if (err)
+             return err;
+
+     if (flags & XDP_FLAGS_HW_MODE) {
+             dev->xdp_prog_hw = prog;
+             return 0;
+     }
+
+     rcu_assign_pointer(dev->xdp_prog, prog);
+     dev->xdp_flags = prog ? flags : 0;
+     return 0;
 }

 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));
+     struct bpf_prog *prog = rtnl_dereference(dev->xdp_prog);
+     bpf_op_t bpf_op;

-     /* 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,
+     if (prog) {
+             bpf_op = dev_xdp_query_generic(dev) ?
+                      generic_xdp_install : dev->netdev_ops->ndo_bpf;
+             WARN_ON(dev_xdp_install(dev, bpf_op, NULL, dev->xdp_flags,
                                      NULL));
+     }
+     if (dev_xdp_query(dev, XDP_FLAGS_HW_MODE))
+             WARN_ON(dev_xdp_install(dev, dev->netdev_ops->ndo_bpf,
+                                     NULL, XDP_FLAGS_HW_MODE, NULL));
 }

 /**

@@ -8080,41 +8104,49 @@ 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;
      struct bpf_prog *prog = NULL;
-     bpf_op_t bpf_op, bpf_chk;
+     u32 mode, curr_mode;
+     bpf_op_t bpf_op;
      bool offload;
      int err;

      ASSERT_RTNL();

      offload = flags & XDP_FLAGS_HW_MODE;
-     query = offload ? XDP_QUERY_PROG_HW : XDP_QUERY_PROG;
+     mode = offload ? XDP_FLAGS_HW_MODE : XDP_FLAGS_DRV_MODE;

-     bpf_op = bpf_chk = ops->ndo_bpf;
+     bpf_op = dev->netdev_ops->ndo_bpf;
      if (!bpf_op && (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 (fd >= 0) {
-             if (!offload && __dev_xdp_query(dev, bpf_chk, XDP_QUERY_PROG)) {
+     if (!bpf_op || flags & XDP_FLAGS_SKB_MODE)
+             mode = XDP_FLAGS_SKB_MODE;
+
+     curr_mode = dev_xdp_current_mode(dev);
+
+     if (!offload && curr_mode && (mode ^ curr_mode) &
+         (XDP_FLAGS_DRV_MODE | XDP_FLAGS_SKB_MODE)) {
+             if (fd >= 0) {
                      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)) {
+             return 0;
+     }
+
+     if (!offload && dev_xdp_query(dev, mode) &&
+         !xdp_prog_flags_ok(dev->xdp_flags, flags, extack))
+             return -EBUSY;
+
+     if (fd >= 0) {
+             if (flags & XDP_FLAGS_UPDATE_IF_NOEXIST &&
+                 dev_xdp_query(dev, mode)) {
                      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, !!bpf_op);
              if (IS_ERR(prog))
                      return PTR_ERR(prog);

@@ -8125,6 +8157,10 @@ int dev_change_xdp_fd(struct net_device *dev,

struct netlink_ext_ack *extack,
              }
      }

+     if (mode == XDP_FLAGS_SKB_MODE) {
+             bpf_op = generic_xdp_install;
+             flags |= XDP_FLAGS_KERN_GENERIC;
+     }
      err = dev_xdp_install(dev, bpf_op, 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..5e396fd01d8b 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c

@@ -1360,42 +1360,44 @@ 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)
+static unsigned int rtnl_xdp_mode_to_flag(u8 tgt_mode)
 {
-     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);
+     switch (tgt_mode) {
+     case XDP_ATTACHED_DRV:
+             return XDP_FLAGS_DRV_MODE;
+     case XDP_ATTACHED_SKB:
+             return XDP_FLAGS_SKB_MODE;
+     case XDP_ATTACHED_HW:
+             return XDP_FLAGS_HW_MODE;
+     }
+     return 0;
 }

-static u32 rtnl_xdp_prog_hw(struct net_device *dev)
+static u32 rtnl_xdp_mode_to_attr(u8 tgt_mode)
 {
-     return __dev_xdp_query(dev, dev->netdev_ops->ndo_bpf,
-                            XDP_QUERY_PROG_HW);
+     switch (tgt_mode) {
+     case XDP_ATTACHED_DRV:
+             return IFLA_XDP_DRV_PROG_ID;
+     case XDP_ATTACHED_SKB:
+             return IFLA_XDP_SKB_PROG_ID;
+     case XDP_ATTACHED_HW:
+             return IFLA_XDP_HW_PROG_ID;
+     }
+     return 0;
 }

 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))
+                            u32 *prog_id, u8 *mode, u8 tgt_mode)
 {
      u32 curr_id;
      int err;

-     curr_id = get_prog_id(dev);
+     curr_id = dev_xdp_query(dev, rtnl_xdp_mode_to_flag(tgt_mode));
      if (!curr_id)
              return 0;

      *prog_id = curr_id;
-     err = nla_put_u32(skb, attr, curr_id);
+     err = nla_put_u32(skb, rtnl_xdp_mode_to_attr(tgt_mode), curr_id);
      if (err)
              return err;

@@ -1420,16 +1422,13 @@ 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);
+     err = rtnl_xdp_report_one(skb, dev, &prog_id, &mode,
XDP_ATTACHED_SKB);
      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);
+     err = rtnl_xdp_report_one(skb, dev, &prog_id, &mode,
XDP_ATTACHED_DRV);
      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);
+     err = rtnl_xdp_report_one(skb, dev, &prog_id, &mode,
XDP_ATTACHED_HW);
      if (err)
              goto err_cancel;

diff --git a/net/core/xdp.c b/net/core/xdp.c
index 4b2b194f4f1f..af92c04a58d8 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c

@@ -388,16 +388,23 @@ int xdp_attachment_query(struct

xdp_attachment_info *info,
 }
 EXPORT_SYMBOL_GPL(xdp_attachment_query);

-bool xdp_attachment_flags_ok(struct xdp_attachment_info *info,
-                          struct netdev_bpf *bpf)
+bool xdp_prog_flags_ok(u32 old_flags, u32 new_flags,
+                    struct netlink_ext_ack *extack)
 {
-     if (info->prog && (bpf->flags ^ info->flags) & XDP_FLAGS_MODES) {
-             NL_SET_ERR_MSG(bpf->extack,
-                            "program loaded with different flags");
+     if ((new_flags ^ old_flags) & XDP_FLAGS_MODES) {
+             NL_SET_ERR_MSG(extack, "program loaded with different flags");
              return false;
      }
      return true;
 }
+
+bool xdp_attachment_flags_ok(struct xdp_attachment_info *info,
+                          struct netdev_bpf *bpf)
+{
+     if (info->prog)
+             return xdp_prog_flags_ok(bpf->flags, info->flags, bpf->extack);
+     return true;
+}
 EXPORT_SYMBOL_GPL(xdp_attachment_flags_ok);

 void xdp_attachment_setup(struct xdp_attachment_info *info,
--
2.20.1

On Sat, 1 Jun 2019 at 21:42, Jakub Kicinski
[off-list ref] wrote:

On Fri, 31 May 2019 19:18:17 +0000, Saeed Mahameed wrote:

quoted

On Fri, 2019-05-31 at 11:42 +0200, Björn Töpel wrote:

quoted

From: Björn Töpel <redacted>

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 two new members: xdp_prog_hw and
xdp_flags. The former is the offloaded XDP program, if any, and the
latter tracks the flags that the supplied when attaching the XDP
program. The flags only apply to SKB_MODE or DRV_MODE, not HW_MODE.

The xdp_prog member, previously only used for SKB_MODE, is shared
with
DRV_MODE. This is OK, due to the fact that SKB_MODE and DRV_MODE are
mutually exclusive. To differentiate between the two modes, a new
internal flag is introduced as well.

Just thinking out loud, why can't we allow any combination of
HW/DRV/SKB modes? they are totally different attach points in a totally
different checkpoints in a frame life cycle.

FWIW see Message-ID: [off-list ref]

I've always seen the SKB-mode as something that will eventually be removed.

Clickable link:
https://lore.kernel.org/netdev/20190201080236.446d84d4@redhat.com/ :-P

quoted

Down the road i think we will utilize this fact and start introducing
SKB helpers for SKB mode and driver helpers for DRV mode..

Any reason why we would want the extra complexity?  There is cls_bpf
if someone wants skb features after all..

On Sat, 1 Jun 2019 at 21:57, Jakub Kicinski
[off-list ref] wrote:

On Fri, 31 May 2019 19:18:17 +0000, Saeed Mahameed wrote:

quoted

quoted

+   if (!bpf_op || flags & XDP_FLAGS_SKB_MODE)
+           mode = XDP_FLAGS_SKB_MODE;
+
+   curr_mode = dev_xdp_current_mode(dev);
+
+   if (!offload && curr_mode && (mode ^ curr_mode) &
+       (XDP_FLAGS_DRV_MODE | XDP_FLAGS_SKB_MODE)) {

if i am reading this correctly this is equivalent to :

if (!offload && (curre_mode != mode))
offlad is false then curr_mode and mode must be DRV or GENERIC ..

Naw, if curr_mode is not set, i.e. nothing installed now, we don't care
about the diff.

quoted

better if you keep bitwise operations for actual bitmasks, mode and
curr_mode are not bitmask, they can hold one value each .. according to
your logic..

Well, they hold one bit each, whether one bit is a bitmap perhaps is
disputable? :)

I think the logic is fine.

Hmm, but changing to:

       if (!offload && curr_mode && mode != curr_mode)

is equal, and to Saeed's point, clearer. I'll go that route in a v3.

What happened to my request to move the change in behaviour for
disabling to a separate patch, tho, Bjorn? :)

Actually, I left that out completely. This patch doesn't change the
behavior. After I realized how the flags *should* be used, I don't
think my v1 change makes sense anymore. My v1 patch was to give an
error if you tried to disable, say generic if drv was enabled via
"auto detect/no flags". But this is catched by looking at the flags.

What I did, however, was moving the flags check into change_fd so that
the driver doesn't have to do the check. E.g. the Intel drivers didn't
do correct checking of flags.

On Sat, 1 Jun 2019 at 22:02, Jakub Kicinski
[off-list ref] wrote:

On Fri, 31 May 2019 11:42:14 +0200, Björn Töpel wrote:

quoted

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 44b47e9df94a..f3a875a52c6c 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h

@@ -1940,6 +1940,9 @@ struct net_device {
 #endif
      struct hlist_node       index_hlist;

+     struct bpf_prog         *xdp_prog_hw;

IDK if we should pay the cost of this pointer for every netdev on the
system just for the single production driver out there that implements
HW offload :(  I'm on the fence about this..

Hmm. Adding a config option? Keep the QUERY_PROG_HW?

quoted

+     u32                     xdp_flags;
+
 /*
  * Cache lines mostly used on transmit path
  */

quoted

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index adcc045952c2..5e396fd01d8b 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c

@@ -1360,42 +1360,44 @@ 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)
+static unsigned int rtnl_xdp_mode_to_flag(u8 tgt_mode)
 {
-     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);
+     switch (tgt_mode) {
+     case XDP_ATTACHED_DRV:
+             return XDP_FLAGS_DRV_MODE;
+     case XDP_ATTACHED_SKB:
+             return XDP_FLAGS_SKB_MODE;
+     case XDP_ATTACHED_HW:
+             return XDP_FLAGS_HW_MODE;
+     }
+     return 0;
 }

-static u32 rtnl_xdp_prog_hw(struct net_device *dev)
+static u32 rtnl_xdp_mode_to_attr(u8 tgt_mode)
 {
-     return __dev_xdp_query(dev, dev->netdev_ops->ndo_bpf,
-                            XDP_QUERY_PROG_HW);
+     switch (tgt_mode) {
+     case XDP_ATTACHED_DRV:
+             return IFLA_XDP_DRV_PROG_ID;
+     case XDP_ATTACHED_SKB:
+             return IFLA_XDP_SKB_PROG_ID;
+     case XDP_ATTACHED_HW:
+             return IFLA_XDP_HW_PROG_ID;
+     }
+     return 0;
 }

 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))
+                            u32 *prog_id, u8 *mode, u8 tgt_mode)
 {
      u32 curr_id;
      int err;

-     curr_id = get_prog_id(dev);
+     curr_id = dev_xdp_query(dev, rtnl_xdp_mode_to_flag(tgt_mode));
      if (!curr_id)
              return 0;

      *prog_id = curr_id;
-     err = nla_put_u32(skb, attr, curr_id);
+     err = nla_put_u32(skb, rtnl_xdp_mode_to_attr(tgt_mode), curr_id);
      if (err)
              return err;

@@ -1420,16 +1422,13 @@ 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);
+     err = rtnl_xdp_report_one(skb, dev, &prog_id, &mode, XDP_ATTACHED_SKB);
      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);
+     err = rtnl_xdp_report_one(skb, dev, &prog_id, &mode, XDP_ATTACHED_DRV);
      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);
+     err = rtnl_xdp_report_one(skb, dev, &prog_id, &mode, XDP_ATTACHED_HW);
      if (err)
              goto err_cancel;

So you remove all the attr and flag params just to add a conversion
helpers to get them based on mode?  Why?  Seems like unnecessary churn,
and questionable change :S