[PATCH bpf-next v3 0/5] net: xdp: refactor XDP program queries

[PATCH bpf-next v3 0/5] net: xdp: refactor XDP program queries

[Björn Töpel]

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

I took a bit different approach with the v3. In this revision I
introduced to a new netdev_xdp structure that tracks the XDP programs,
and instead of sharing the xdp_prog member between DRV and SKB (again,
they mutual exclusive). With this, there's no need for a special
"what-mode-am-I-in" flag for SKB/DRV.

Jakub, what's your thoughts on the special handling of XDP offloading?
Maybe it's just overkill? Just allocate space for the offloaded
program regardless support or not? Also, please review the
dev_xdp_support_offload() addition into the nfp code.

The last two patches move the attach flag checks to generic code, and
removes the flags member from netdev_bpf.

Please refer to the individual commit messages for more details.

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


Thanks,
Björn

Björn Töpel (5):
  net: xdp: refactor XDP_QUERY_PROG{,_HW} to netdev
  nfp, netdevsim: use dev_xdp_support_offload() function
  net: xdp: remove XDP_QUERY_PROG{,_HW}
  net: xdp: refactor XDP flags checking
  net: xdp: remove xdp_attachment_flags_ok() and flags member

 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   |  10 -
 .../net/ethernet/netronome/nfp/nfp_net_main.c |   7 +
 .../net/ethernet/qlogic/qede/qede_filter.c    |   3 -
 drivers/net/netdevsim/bpf.c                   |   7 -
 drivers/net/netdevsim/netdev.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                     |  30 +--
 include/net/xdp.h                             |   5 -
 net/core/dev.c                                | 172 ++++++++++++------
 net/core/rtnetlink.c                          |  31 +---
 net/core/xdp.c                                |  22 ---
 22 files changed, 147 insertions(+), 256 deletions(-)

-- 
2.20.1

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

[Björn Töpel]

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. This commit refactors the query code up from the
drivers to the netdev level.

The xdp_prog member of the struct net_device has evolved into a struct
netdev_xdp. The new structure contains an array of XDP programs, which
are allocated at run time. Each entry represents an XDP program for a
certain mode (SKB, DRV or HW). Only netdevs that has announced support
for XDP offload, using the new dev_xdp_support_offload() function,
will allocate space for HW XDP programs. This means that that
offloading capability can be checked in the generic path as well.

For simplicity, SKB and DRV XDP programs populate different entries,
even though they cannot be enabled simultaneously.

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
 include/linux/netdevice.h |  21 +++--
 net/core/dev.c            | 156 ++++++++++++++++++++++++++------------
 net/core/rtnetlink.c      |  31 +-------
 3 files changed, 126 insertions(+), 82 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 44b47e9df94a..fd2fae39218f 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -903,6 +903,14 @@ struct netdev_bpf {
 	};
 };
 
+int dev_xdp_support_offload(struct net_device *dev);
+
+struct netdev_xdp {
+	struct bpf_prog __rcu **prog;
+	int num_progs;
+	bool explicit_mode;
+};
+
 #ifdef CONFIG_XFRM_OFFLOAD
 struct xfrmdev_ops {
 	int	(*xdo_dev_state_add) (struct xfrm_state *x);
@@ -1921,7 +1929,7 @@ struct net_device {
 	unsigned int		num_rx_queues;
 	unsigned int		real_num_rx_queues;
 
-	struct bpf_prog __rcu	*xdp_prog;
+	struct netdev_xdp	xdp;
 	unsigned long		gro_flush_timeout;
 	rx_handler_func_t __rcu	*rx_handler;
 	void __rcu		*rx_handler_data;
@@ -2045,9 +2053,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.prog && dev->xdp.prog[0]);
 }
 
 #define	NETDEV_ALIGN		32
@@ -3682,10 +3689,12 @@ struct sk_buff *dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
 				    struct netdev_queue *txq, int *ret);
 
 typedef int (*bpf_op_t)(struct net_device *dev, struct netdev_bpf *bpf);
+typedef u32 (*xdp_query_prog_t)(struct net_device *dev);
 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_generic(struct net_device *dev);
+u32 dev_xdp_query_drv(struct net_device *dev);
+u32 dev_xdp_query_hw(struct net_device *dev);
 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 66f7508825bd..d6ea5733986f 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4813,6 +4813,11 @@ static inline int nf_ingress(struct sk_buff *skb, struct packet_type **pt_prev,
 	return 0;
 }
 
+static struct bpf_prog *dev_xdp_get_generic(struct net_device *dev)
+{
+	return dev->xdp.prog ? rcu_dereference(dev->xdp.prog[0]) : NULL;
+}
+
 static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc,
 				    struct packet_type **ppt_prev)
 {
@@ -4845,7 +4850,7 @@ static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc,
 		int ret2;
 
 		preempt_disable();
-		ret2 = do_xdp_generic(rcu_dereference(skb->dev->xdp_prog), skb);
+		ret2 = do_xdp_generic(dev_xdp_get_generic(skb->dev), skb);
 		preempt_enable();
 
 		if (ret2 != XDP_PASS)
@@ -5133,13 +5138,13 @@ static void __netif_receive_skb_list(struct list_head *head)
 
 static int generic_xdp_install(struct net_device *dev, struct netdev_bpf *xdp)
 {
-	struct bpf_prog *old = rtnl_dereference(dev->xdp_prog);
+	struct bpf_prog *old = rtnl_dereference(dev->xdp.prog[0]);
 	struct bpf_prog *new = xdp->prog;
 	int ret = 0;
 
 	switch (xdp->command) {
 	case XDP_SETUP_PROG:
-		rcu_assign_pointer(dev->xdp_prog, new);
+		rcu_assign_pointer(dev->xdp.prog[0], new);
 		if (old)
 			bpf_prog_put(old);
 
@@ -7971,67 +7976,93 @@ 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(struct net_device *dev, int mode)
 {
-	struct netdev_bpf xdp;
+	if (dev->xdp.prog && dev->xdp.num_progs > mode) {
+		struct bpf_prog *prog = rtnl_dereference(dev->xdp.prog[mode]);
 
-	if (!bpf_op)
-		return 0;
+		return prog ? prog->aux->id : 0;
+	}
+	return 0;
+}
 
-	memset(&xdp, 0, sizeof(xdp));
-	xdp.command = cmd;
+u32 dev_xdp_query_generic(struct net_device *dev)
+{
+	return __dev_xdp_query(dev, 0);
+}
 
-	/* Query must always succeed. */
-	WARN_ON(bpf_op(dev, &xdp) < 0 && cmd == XDP_QUERY_PROG);
+u32 dev_xdp_query_drv(struct net_device *dev)
+{
+	return __dev_xdp_query(dev, 1);
+}
 
-	return xdp.prog_id;
+u32 dev_xdp_query_hw(struct net_device *dev)
+{
+	return __dev_xdp_query(dev, 2);
+}
+
+static bool dev_xdp_support_hw(struct net_device *dev)
+{
+	return dev->xdp.num_progs > 2;
 }
 
 static int dev_xdp_install(struct net_device *dev, bpf_op_t bpf_op,
 			   struct netlink_ext_ack *extack, u32 flags,
 			   struct bpf_prog *prog)
 {
-	struct netdev_bpf xdp;
+	struct netdev_bpf xdp = {};
+	struct bpf_prog **entry;
+	int err;
 
-	memset(&xdp, 0, sizeof(xdp));
-	if (flags & XDP_FLAGS_HW_MODE)
+	if (!dev->xdp.prog) {
+		dev->xdp.prog = kcalloc(dev->xdp.num_progs,
+					sizeof(dev->xdp.prog), GFP_KERNEL);
+		if (!dev->xdp.prog)
+			return -ENOMEM;
+	}
+
+	if (flags & XDP_FLAGS_HW_MODE) {
 		xdp.command = XDP_SETUP_PROG_HW;
-	else
+		entry = &dev->xdp.prog[2];
+	} else {
 		xdp.command = XDP_SETUP_PROG;
+		entry = bpf_op == generic_xdp_install ? &dev->xdp.prog[0] :
+			&dev->xdp.prog[1];
+	}
+
 	xdp.extack = extack;
 	xdp.flags = flags;
 	xdp.prog = prog;
 
-	return bpf_op(dev, &xdp);
+	err = bpf_op(dev, &xdp);
+	if (err)
+		return err;
+
+	rcu_assign_pointer(*entry, prog);
+	dev->xdp.explicit_mode =
+		prog ? flags & (XDP_FLAGS_SKB_MODE | XDP_FLAGS_DRV_MODE) : 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));
+	u32 flags;
 
-	/* Remove from the driver */
-	ndo_bpf = dev->netdev_ops->ndo_bpf;
-	if (!ndo_bpf)
-		return;
+	if (dev_xdp_query_generic(dev)) {
+		flags = dev->xdp.explicit_mode ? XDP_FLAGS_SKB_MODE : 0;
+		WARN_ON(dev_xdp_install(dev, generic_xdp_install, NULL,
+					flags, NULL));
+	}
 
-	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));
+	if (dev_xdp_query_drv(dev)) {
+		flags = dev->xdp.explicit_mode ? XDP_FLAGS_DRV_MODE : 0;
+		WARN_ON(dev_xdp_install(dev, dev->netdev_ops->ndo_bpf, NULL,
+					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_query_hw(dev))
+		WARN_ON(dev_xdp_install(dev, dev->netdev_ops->ndo_bpf, NULL,
+					XDP_FLAGS_HW_MODE, NULL));
 }
 
 /**
@@ -8047,34 +8078,41 @@ 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;
+	xdp_query_prog_t query, check;
 	struct bpf_prog *prog = NULL;
-	bpf_op_t bpf_op, bpf_chk;
+	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;
 
-	bpf_op = bpf_chk = ops->ndo_bpf;
+	if (offload && !dev_xdp_support_hw(dev)) {
+		NL_SET_ERR_MSG(extack, "underlying driver does not support XDP offload");
+		return -EOPNOTSUPP;
+	}
+
+	bpf_op = ops->ndo_bpf;
+	query = offload ? dev_xdp_query_hw : dev_xdp_query_drv;
+	check = dev_xdp_query_generic;
 	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))
+	if (!bpf_op || (flags & XDP_FLAGS_SKB_MODE)) {
 		bpf_op = generic_xdp_install;
-	if (bpf_op == bpf_chk)
-		bpf_chk = generic_xdp_install;
+		query = dev_xdp_query_generic;
+	}
+	if (bpf_op == generic_xdp_install)
+		check = dev_xdp_query_drv;
 
 	if (fd >= 0) {
-		if (!offload && __dev_xdp_query(dev, bpf_chk, XDP_QUERY_PROG)) {
+		if (!offload && check(dev)) {
 			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) && query(dev)) {
 			NL_SET_ERR_MSG(extack, "XDP program already attached");
 			return -EBUSY;
 		}
@@ -8098,6 +8136,24 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
 	return err;
 }
 
+/**
+ *	dev_xdp_support_offload	- announce XDP offload support
+ *	@dev: device
+ *
+ *	Announce that the netdev has support for XDP offloading.
+ */
+int dev_xdp_support_offload(struct net_device *dev)
+{
+	if (dev->xdp.prog) {
+		netdev_WARN(dev, "announcing XDP offloading support too late!\n");
+		return -EINVAL;
+	}
+
+	dev->xdp.num_progs = 3;
+	return 0;
+}
+EXPORT_SYMBOL(dev_xdp_support_offload);
+
 /**
  *	dev_new_index	-	allocate an ifindex
  *	@net: the applicable net namespace
@@ -9175,6 +9231,7 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 		dev->ethtool_ops = &default_ethtool_ops;
 
 	nf_hook_ingress_init(dev);
+	dev->xdp.num_progs = 2;
 
 	return dev;
 
@@ -9206,6 +9263,7 @@ void free_netdev(struct net_device *dev)
 	might_sleep();
 	netif_free_tx_queues(dev);
 	netif_free_rx_queues(dev);
+	kfree(dev->xdp.prog);
 
 	kfree(rcu_dereference_protected(dev->ingress_queue, 1));
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index adcc045952c2..aac7c71ca163 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1360,32 +1360,9 @@ 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))
+			       xdp_query_prog_t get_prog_id)
 {
 	u32 curr_id;
 	int err;
@@ -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, dev_xdp_query_generic);
 	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, dev_xdp_query_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);
+				  IFLA_XDP_HW_PROG_ID, dev_xdp_query_hw);
 	if (err)
 		goto err_cancel;
 
-- 
2.20.1

[PATCH bpf-next v3 2/5] nfp, netdevsim: use dev_xdp_support_offload() function

[Björn Töpel]

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

Make all drivers with XDP offloading support use
dev_xdp_support_offload().

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_net_main.c | 7 +++++++
 drivers/net/netdevsim/netdev.c                    | 4 ++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
index 986464d4a206..e320cacc95e6 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
@@ -135,6 +135,13 @@ nfp_net_pf_alloc_vnic(struct nfp_pf *pf, bool needs_netdev,
 			nfp_net_free(nn);
 			return ERR_PTR(err);
 		}
+		if (nn->app && nn->app->type->xdp_offload) {
+			err = dev_xdp_support_offload(nn->dp.netdev);
+			if (err) {
+				nfp_net_free(nn);
+				return ERR_PTR(err);
+			}
+		}
 	}
 
 	pf->num_vnics++;
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index e5c8aa08e1cd..26e4fc12cdbe 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -311,6 +311,10 @@ nsim_create(struct nsim_dev *nsim_dev, struct nsim_dev_port *nsim_dev_port)
 	SET_NETDEV_DEV(dev, &ns->nsim_bus_dev->dev);
 	dev->netdev_ops = &nsim_netdev_ops;
 
+	err = dev_xdp_support_offload(dev);
+	if (err)
+		goto err_free_netdev;
+
 	rtnl_lock();
 	err = nsim_bpf_init(ns);
 	if (err)
-- 
2.20.1

[PATCH bpf-next v3 3/5] net: xdp: remove XDP_QUERY_PROG{,_HW}

[Björn Töpel]

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

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

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
 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 753957ec72be..ef94da2abaf8 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 c65cefd84eda..b3a5346a9ee3 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -4246,29 +4246,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 fd2fae39218f..6b700005288d 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 0f25b3675c5c..4ad4b20fe2c0 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -159,8 +159,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 d6ea5733986f..8e9f5693a7da 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 4b2b194f4f1f..6f76ad995fef 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_attachment_flags_ok(struct xdp_attachment_info *info,
 			     struct netdev_bpf *bpf)
 {
-- 
2.20.1

[PATCH bpf-next v3 4/5] net: xdp: refactor XDP flags checking

[Björn Töpel]

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

Similar to query a driver for a loaded XDP program, each driver need
to check the attachment flags when a program is loaded.

E.g., if an XDP program was loaded without explicit flags (fallback
mode), it must be reloaded/removed without explicit flags.

This commit moves that check to generic code as well.

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
 net/core/dev.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 8e9f5693a7da..bb5fbb395596 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -8075,8 +8075,8 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
 	const struct net_device_ops *ops = dev->netdev_ops;
 	xdp_query_prog_t query, check;
 	struct bpf_prog *prog = NULL;
+	bool offload, expl;
 	bpf_op_t bpf_op;
-	bool offload;
 	int err;
 
 	ASSERT_RTNL();
@@ -8124,6 +8124,14 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
 		}
 	}
 
+	expl = dev->xdp.explicit_mode;
+	if (!offload && query(dev) && !!expl ^ !!(flags & XDP_FLAGS_MODES)) {
+		NL_SET_ERR_MSG(extack, "program loaded with different flags");
+		if (prog)
+			bpf_prog_put(prog);
+		return -EBUSY;
+	}
+
 	err = dev_xdp_install(dev, bpf_op, extack, flags, prog);
 	if (err < 0 && prog)
 		bpf_prog_put(prog);
-- 
2.20.1

[PATCH bpf-next v3 5/5] net: xdp: remove xdp_attachment_flags_ok() and flags member

[Björn Töpel]

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

The attachment flags check is done in the generic netdev code, so
there is no need for this function anymore. Remove it and all uses of
it.

Further; Passing flags from struct netdev_bpf when attaching an XDP
program is no longer necessary, so let us remove that member.

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_net_common.c |  6 ------
 drivers/net/netdevsim/bpf.c                         |  3 ---
 include/linux/netdevice.h                           |  1 -
 include/net/xdp.h                                   |  3 ---
 net/core/dev.c                                      |  1 -
 net/core/xdp.c                                      | 13 -------------
 6 files changed, 27 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index 2a9683db54e5..c164da24c28c 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -3497,9 +3497,6 @@ static int nfp_net_xdp_setup_drv(struct nfp_net *nn, struct netdev_bpf *bpf)
 	struct nfp_net_dp *dp;
 	int err;
 
-	if (!xdp_attachment_flags_ok(&nn->xdp, bpf))
-		return -EBUSY;
-
 	if (!prog == !nn->dp.xdp_prog) {
 		WRITE_ONCE(nn->dp.xdp_prog, prog);
 		xdp_attachment_setup(&nn->xdp, bpf);
@@ -3528,9 +3525,6 @@ static int nfp_net_xdp_setup_hw(struct nfp_net *nn, struct netdev_bpf *bpf)
 {
 	int err;
 
-	if (!xdp_attachment_flags_ok(&nn->xdp_hw, bpf))
-		return -EBUSY;
-
 	err = nfp_app_xdp_offload(nn->app, nn, bpf->prog, bpf->extack);
 	if (err)
 		return err;
diff --git a/drivers/net/netdevsim/bpf.c b/drivers/net/netdevsim/bpf.c
index d03d31721e38..51b2430f1edc 100644
--- a/drivers/net/netdevsim/bpf.c
+++ b/drivers/net/netdevsim/bpf.c
@@ -190,9 +190,6 @@ nsim_xdp_set_prog(struct netdevsim *ns, struct netdev_bpf *bpf,
 {
 	int err;
 
-	if (!xdp_attachment_flags_ok(xdp, bpf))
-		return -EBUSY;
-
 	if (bpf->command == XDP_SETUP_PROG && !ns->bpf_xdpdrv_accept) {
 		NSIM_EA(bpf->extack, "driver XDP disabled in DebugFS");
 		return -EOPNOTSUPP;
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 6b700005288d..d7fa2c9fa031 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -879,7 +879,6 @@ struct netdev_bpf {
 	union {
 		/* XDP_SETUP_PROG */
 		struct {
-			u32 flags;
 			struct bpf_prog *prog;
 			struct netlink_ext_ack *extack;
 		};
diff --git a/include/net/xdp.h b/include/net/xdp.h
index 4ad4b20fe2c0..854267b3b624 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -155,12 +155,9 @@ xdp_data_meta_unsupported(const struct xdp_buff *xdp)
 
 struct xdp_attachment_info {
 	struct bpf_prog *prog;
-	u32 flags;
 };
 
 struct netdev_bpf;
-bool xdp_attachment_flags_ok(struct xdp_attachment_info *info,
-			     struct netdev_bpf *bpf);
 void xdp_attachment_setup(struct xdp_attachment_info *info,
 			  struct netdev_bpf *bpf);
 
diff --git a/net/core/dev.c b/net/core/dev.c
index bb5fbb395596..b0476545fbc8 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -8026,7 +8026,6 @@ static int dev_xdp_install(struct net_device *dev, bpf_op_t bpf_op,
 	}
 
 	xdp.extack = extack;
-	xdp.flags = flags;
 	xdp.prog = prog;
 
 	err = bpf_op(dev, &xdp);
diff --git a/net/core/xdp.c b/net/core/xdp.c
index 6f76ad995fef..b2cdebd0b17d 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -379,25 +379,12 @@ void xdp_return_buff(struct xdp_buff *xdp)
 }
 EXPORT_SYMBOL_GPL(xdp_return_buff);
 
-bool xdp_attachment_flags_ok(struct xdp_attachment_info *info,
-			     struct netdev_bpf *bpf)
-{
-	if (info->prog && (bpf->flags ^ info->flags) & XDP_FLAGS_MODES) {
-		NL_SET_ERR_MSG(bpf->extack,
-			       "program loaded with different flags");
-		return false;
-	}
-	return true;
-}
-EXPORT_SYMBOL_GPL(xdp_attachment_flags_ok);
-
 void xdp_attachment_setup(struct xdp_attachment_info *info,
 			  struct netdev_bpf *bpf)
 {
 	if (info->prog)
 		bpf_prog_put(info->prog);
 	info->prog = bpf->prog;
-	info->flags = bpf->flags;
 }
 EXPORT_SYMBOL_GPL(xdp_attachment_setup);
 
-- 
2.20.1

Re: [PATCH bpf-next v3 0/5] net: xdp: refactor XDP program queries

[Jakub Kicinski]

On Mon, 10 Jun 2019 18:02:29 +0200, Björn Töpel wrote:
> Jakub, what's your thoughts on the special handling of XDP offloading?
> Maybe it's just overkill? Just allocate space for the offloaded
> program regardless support or not? Also, please review the
> dev_xdp_support_offload() addition into the nfp code.

I'm not a huge fan of the new approach - it adds a conditional move,
dereference and a cache line reference to the fast path :(

I think it'd be fine to allocate entries for all 3 types, but the
potential of slowing down DRV may not be a good thing in a refactoring
series.

Re: [PATCH bpf-next v3 0/5] net: xdp: refactor XDP program queries

[Björn Töpel]

On 2019-06-11 00:24, Jakub Kicinski wrote:
> On Mon, 10 Jun 2019 18:02:29 +0200, Björn Töpel wrote:
>> Jakub, what's your thoughts on the special handling of XDP offloading?
>> Maybe it's just overkill? Just allocate space for the offloaded
>> program regardless support or not? Also, please review the
>> dev_xdp_support_offload() addition into the nfp code.
> 
> I'm not a huge fan of the new approach - it adds a conditional move,
> dereference and a cache line reference to the fast path :(
> 
> I think it'd be fine to allocate entries for all 3 types, but the
> potential of slowing down DRV may not be a good thing in a refactoring
> series.
> 

Note, that currently it's "only" the XDP_SKB path that's affected, but
yeah, I agree with out. And going forward, I'd like to use the netdev
xdp_prog from the Intel drivers, instead of spreading/caching it all over.

I'll go back to the drawing board. Any suggestions on a how/where the
program should be stored in the netdev are welcome! :-) ...or maybe just
simply store the netdev_xdp flat (w/o the additional allocation step) in
net_device. Three programs and the boolean (remove the num_progs).


Björn

Re: [PATCH bpf-next v3 0/5] net: xdp: refactor XDP program queries

[Toke Høiland-Jørgensen]

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

> On 2019-06-11 00:24, Jakub Kicinski wrote:
>> On Mon, 10 Jun 2019 18:02:29 +0200, Björn Töpel wrote:
>>> Jakub, what's your thoughts on the special handling of XDP offloading?
>>> Maybe it's just overkill? Just allocate space for the offloaded
>>> program regardless support or not? Also, please review the
>>> dev_xdp_support_offload() addition into the nfp code.
>> 
>> I'm not a huge fan of the new approach - it adds a conditional move,
>> dereference and a cache line reference to the fast path :(
>> 
>> I think it'd be fine to allocate entries for all 3 types, but the
>> potential of slowing down DRV may not be a good thing in a refactoring
>> series.
>> 
>
> Note, that currently it's "only" the XDP_SKB path that's affected, but
> yeah, I agree with out. And going forward, I'd like to use the netdev
> xdp_prog from the Intel drivers, instead of spreading/caching it all over.
>
> I'll go back to the drawing board. Any suggestions on a how/where the
> program should be stored in the netdev are welcome! :-) ...or maybe just
> simply store the netdev_xdp flat (w/o the additional allocation step) in
> net_device. Three programs and the boolean (remove the num_progs).

This seems reasonable to me (and thanks for keeping at this!).

-Toke

Re: [PATCH bpf-next v3 0/5] net: xdp: refactor XDP program queries

[Jakub Kicinski]

On Tue, 11 Jun 2019 09:24:41 +0200, Björn Töpel wrote:
> On 2019-06-11 00:24, Jakub Kicinski wrote:
> > On Mon, 10 Jun 2019 18:02:29 +0200, Björn Töpel wrote:  
> >> Jakub, what's your thoughts on the special handling of XDP offloading?
> >> Maybe it's just overkill? Just allocate space for the offloaded
> >> program regardless support or not? Also, please review the
> >> dev_xdp_support_offload() addition into the nfp code.  
> > 
> > I'm not a huge fan of the new approach - it adds a conditional move,
> > dereference and a cache line reference to the fast path :(
> > 
> > I think it'd be fine to allocate entries for all 3 types, but the
> > potential of slowing down DRV may not be a good thing in a refactoring
> > series.
> >   
> 
> Note, that currently it's "only" the XDP_SKB path that's affected, but
> yeah, I agree with out. And going forward, I'd like to use the netdev
> xdp_prog from the Intel drivers, instead of spreading/caching it all over.
> 
> I'll go back to the drawing board. Any suggestions on a how/where the
> program should be stored in the netdev are welcome! :-) ...or maybe just
> simply store the netdev_xdp flat (w/o the additional allocation step) in
> net_device. Three programs and the boolean (remove the num_progs).

Three progs?  Are we planning to allow SKB and DRV at the same time?
One prog and flags looked fairly reasonable to me.  Flags can even be
a u8.  The offload prog can continue to live in the driver.

Re: [PATCH bpf-next v3 0/5] net: xdp: refactor XDP program queries

[Björn Töpel]

On 2019-06-11 19:22, Jakub Kicinski wrote:
> Three progs?  Are we planning to allow SKB and DRV at the same time?

No! :-)

> One prog and flags looked fairly reasonable to me.  Flags can even be
> a u8.  The offload prog can continue to live in the driver.

Ok, I'll take that route (only adding a u8 flags member to net_device).

Re: [PATCH bpf-next v3 0/5] net: xdp: refactor XDP program queries

[Jonathan Lemon]

On 11 Jun 2019, at 0:24, Björn Töpel wrote:

> On 2019-06-11 00:24, Jakub Kicinski wrote:
>> On Mon, 10 Jun 2019 18:02:29 +0200, Björn Töpel wrote:
>>> Jakub, what's your thoughts on the special handling of XDP offloading?
>>> Maybe it's just overkill? Just allocate space for the offloaded
>>> program regardless support or not? Also, please review the
>>> dev_xdp_support_offload() addition into the nfp code.
>>
>> I'm not a huge fan of the new approach - it adds a conditional move,
>> dereference and a cache line reference to the fast path :(
>>
>> I think it'd be fine to allocate entries for all 3 types, but the
>> potential of slowing down DRV may not be a good thing in a refactoring
>> series.
>>
>
> Note, that currently it's "only" the XDP_SKB path that's affected, but
> yeah, I agree with out. And going forward, I'd like to use the netdev
> xdp_prog from the Intel drivers, instead of spreading/caching it all over.
>
> I'll go back to the drawing board. Any suggestions on a how/where the
> program should be stored in the netdev are welcome! :-) ...or maybe just
> simply store the netdev_xdp flat (w/o the additional allocation step) in
> net_device. Three programs and the boolean (remove the num_progs).

A flat allocation does seem like the best path forward.

Thanks for keeping at this!
-- 
Jonathan