diff mbox series

[net-next,12/16] idpf: implement XDP_SETUP_PROG in ndo_bpf for splitq

Message ID 20250305162132.1106080-13-aleksander.lobakin@intel.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series idpf: add XDP support | expand

Checks

Context Check Description
netdev/series_format fail Series longer than 15 patches
netdev/tree_selection success Clearly marked for net-next, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 13 of 13 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 191 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 107 this patch: 107
netdev/source_inline success Was 0 now: 0

Commit Message

Alexander Lobakin March 5, 2025, 4:21 p.m. UTC 
From: Michal Kubiak <michal.kubiak@intel.com>

Implement loading/removing XDP program using .ndo_bpf callback
in the split queue mode. Reconfigure and restart the queues if needed
(!!old_prog != !!new_prog), otherwise, just update the pointers.

Signed-off-by: Michal Kubiak <michal.kubiak@intel.com>
Signed-off-by: Alexander Lobakin <aleksander.lobakin@intel.com>
---
 drivers/net/ethernet/intel/idpf/idpf_txrx.h |   4 +-
 drivers/net/ethernet/intel/idpf/xdp.h       |   7 ++
 drivers/net/ethernet/intel/idpf/idpf_lib.c  |   1 +
 drivers/net/ethernet/intel/idpf/idpf_txrx.c |   4 +
 drivers/net/ethernet/intel/idpf/xdp.c       | 114 ++++++++++++++++++++
 5 files changed, 129 insertions(+), 1 deletion(-)

Comments

Maciej Fijalkowski March 7, 2025, 2:16 p.m. UTC | #1 
On Wed, Mar 05, 2025 at 05:21:28PM +0100, Alexander Lobakin wrote:
> From: Michal Kubiak <michal.kubiak@intel.com>
> 
> Implement loading/removing XDP program using .ndo_bpf callback
> in the split queue mode. Reconfigure and restart the queues if needed
> (!!old_prog != !!new_prog), otherwise, just update the pointers.
> 
> Signed-off-by: Michal Kubiak <michal.kubiak@intel.com>
> Signed-off-by: Alexander Lobakin <aleksander.lobakin@intel.com>
> ---
>  drivers/net/ethernet/intel/idpf/idpf_txrx.h |   4 +-
>  drivers/net/ethernet/intel/idpf/xdp.h       |   7 ++
>  drivers/net/ethernet/intel/idpf/idpf_lib.c  |   1 +
>  drivers/net/ethernet/intel/idpf/idpf_txrx.c |   4 +
>  drivers/net/ethernet/intel/idpf/xdp.c       | 114 ++++++++++++++++++++
>  5 files changed, 129 insertions(+), 1 deletion(-)
> 

(...)

> +
> +/**
> + * idpf_xdp_setup_prog - handle XDP program install/remove requests
> + * @vport: vport to configure
> + * @xdp: request data (program, extack)
> + *
> + * Return: 0 on success, -errno on failure.
> + */
> +static int
> +idpf_xdp_setup_prog(struct idpf_vport *vport, const struct netdev_bpf *xdp)
> +{
> +	const struct idpf_netdev_priv *np = netdev_priv(vport->netdev);
> +	struct bpf_prog *old, *prog = xdp->prog;
> +	struct idpf_vport_config *cfg;
> +	int ret;
> +
> +	cfg = vport->adapter->vport_config[vport->idx];
> +	if (!vport->num_xdp_txq && vport->num_txq == cfg->max_q.max_txq) {
> +		NL_SET_ERR_MSG_MOD(xdp->extack,
> +				   "No Tx queues available for XDP, please decrease the number of regular SQs");
> +		return -ENOSPC;
> +	}
> +
> +	if (test_bit(IDPF_REMOVE_IN_PROG, vport->adapter->flags) ||

IN_PROG is a bit unfortunate here as it mixes with 'prog' :P

> +	    !!vport->xdp_prog == !!prog) {
> +		if (np->state == __IDPF_VPORT_UP)
> +			idpf_copy_xdp_prog_to_qs(vport, prog);
> +
> +		old = xchg(&vport->xdp_prog, prog);
> +		if (old)
> +			bpf_prog_put(old);
> +
> +		cfg->user_config.xdp_prog = prog;
> +
> +		return 0;
> +	}
> +
> +	old = cfg->user_config.xdp_prog;
> +	cfg->user_config.xdp_prog = prog;
> +
> +	ret = idpf_initiate_soft_reset(vport, IDPF_SR_Q_CHANGE);
> +	if (ret) {
> +		NL_SET_ERR_MSG_MOD(xdp->extack,
> +				   "Could not reopen the vport after XDP setup");
> +
> +		if (prog)
> +			bpf_prog_put(prog);

aren't you missing this for prog->NULL conversion? you have this for
hot-swap case (prog->prog).

> +
> +		cfg->user_config.xdp_prog = old;
> +	}
> +
> +	return ret;
> +}
> +
> +/**
> + * idpf_xdp - handle XDP-related requests
> + * @dev: network device to configure
> + * @xdp: request data (program, extack)
> + *
> + * Return: 0 on success, -errno on failure.
> + */
> +int idpf_xdp(struct net_device *dev, struct netdev_bpf *xdp)
> +{
> +	struct idpf_vport *vport;
> +	int ret;
> +
> +	idpf_vport_ctrl_lock(dev);
> +	vport = idpf_netdev_to_vport(dev);
> +
> +	if (!idpf_is_queue_model_split(vport->txq_model))
> +		goto notsupp;
> +
> +	switch (xdp->command) {
> +	case XDP_SETUP_PROG:
> +		ret = idpf_xdp_setup_prog(vport, xdp);
> +		break;
> +	default:
> +notsupp:
> +		ret = -EOPNOTSUPP;
> +		break;
> +	}
> +
> +	idpf_vport_ctrl_unlock(dev);
> +
> +	return ret;
> +}
> -- 
> 2.48.1
>
Alexander Lobakin March 17, 2025, 2:58 p.m. UTC | #2 
From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Date: Fri, 7 Mar 2025 15:16:48 +0100

> On Wed, Mar 05, 2025 at 05:21:28PM +0100, Alexander Lobakin wrote:
>> From: Michal Kubiak <michal.kubiak@intel.com>
>>
>> Implement loading/removing XDP program using .ndo_bpf callback
>> in the split queue mode. Reconfigure and restart the queues if needed
>> (!!old_prog != !!new_prog), otherwise, just update the pointers.
>>
>> Signed-off-by: Michal Kubiak <michal.kubiak@intel.com>
>> Signed-off-by: Alexander Lobakin <aleksander.lobakin@intel.com>
>> ---
>>  drivers/net/ethernet/intel/idpf/idpf_txrx.h |   4 +-
>>  drivers/net/ethernet/intel/idpf/xdp.h       |   7 ++
>>  drivers/net/ethernet/intel/idpf/idpf_lib.c  |   1 +
>>  drivers/net/ethernet/intel/idpf/idpf_txrx.c |   4 +
>>  drivers/net/ethernet/intel/idpf/xdp.c       | 114 ++++++++++++++++++++
>>  5 files changed, 129 insertions(+), 1 deletion(-)
>>
> 
> (...)
> 
>> +
>> +/**
>> + * idpf_xdp_setup_prog - handle XDP program install/remove requests
>> + * @vport: vport to configure
>> + * @xdp: request data (program, extack)
>> + *
>> + * Return: 0 on success, -errno on failure.
>> + */
>> +static int
>> +idpf_xdp_setup_prog(struct idpf_vport *vport, const struct netdev_bpf *xdp)
>> +{
>> +	const struct idpf_netdev_priv *np = netdev_priv(vport->netdev);
>> +	struct bpf_prog *old, *prog = xdp->prog;
>> +	struct idpf_vport_config *cfg;
>> +	int ret;
>> +
>> +	cfg = vport->adapter->vport_config[vport->idx];
>> +	if (!vport->num_xdp_txq && vport->num_txq == cfg->max_q.max_txq) {
>> +		NL_SET_ERR_MSG_MOD(xdp->extack,
>> +				   "No Tx queues available for XDP, please decrease the number of regular SQs");
>> +		return -ENOSPC;
>> +	}
>> +
>> +	if (test_bit(IDPF_REMOVE_IN_PROG, vport->adapter->flags) ||
> 
> IN_PROG is a bit unfortunate here as it mixes with 'prog' :P

Authentic idpf dictionary ¯\_(ツ)_/¯

> 
>> +	    !!vport->xdp_prog == !!prog) {
>> +		if (np->state == __IDPF_VPORT_UP)
>> +			idpf_copy_xdp_prog_to_qs(vport, prog);
>> +
>> +		old = xchg(&vport->xdp_prog, prog);
>> +		if (old)
>> +			bpf_prog_put(old);
>> +
>> +		cfg->user_config.xdp_prog = prog;
>> +
>> +		return 0;
>> +	}
>> +
>> +	old = cfg->user_config.xdp_prog;
>> +	cfg->user_config.xdp_prog = prog;
>> +
>> +	ret = idpf_initiate_soft_reset(vport, IDPF_SR_Q_CHANGE);
>> +	if (ret) {
>> +		NL_SET_ERR_MSG_MOD(xdp->extack,
>> +				   "Could not reopen the vport after XDP setup");
>> +
>> +		if (prog)
>> +			bpf_prog_put(prog);
> 
> aren't you missing this for prog->NULL conversion? you have this for
> hot-swap case (prog->prog).

This path (soft_reset) handles NULL => prog and prog => NULL. This
branch in particular handles errors during the soft reset, when we need
to restore the original prog and put the new one.

What you probably meant is that I don't have bpf_prog_put(old) in case
everything went well below? Breh =\

> 
>> +
>> +		cfg->user_config.xdp_prog = old;
>> +	}
>> +
>> +	return ret;
>> +}

Thanks,
Olek
Maciej Fijalkowski March 19, 2025, 4:23 p.m. UTC | #3 
On Mon, Mar 17, 2025 at 03:58:12PM +0100, Alexander Lobakin wrote:
> From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> Date: Fri, 7 Mar 2025 15:16:48 +0100
> 
> > On Wed, Mar 05, 2025 at 05:21:28PM +0100, Alexander Lobakin wrote:
> >> From: Michal Kubiak <michal.kubiak@intel.com>
> >>
> >> Implement loading/removing XDP program using .ndo_bpf callback
> >> in the split queue mode. Reconfigure and restart the queues if needed
> >> (!!old_prog != !!new_prog), otherwise, just update the pointers.
> >>
> >> Signed-off-by: Michal Kubiak <michal.kubiak@intel.com>
> >> Signed-off-by: Alexander Lobakin <aleksander.lobakin@intel.com>
> >> ---
> >>  drivers/net/ethernet/intel/idpf/idpf_txrx.h |   4 +-
> >>  drivers/net/ethernet/intel/idpf/xdp.h       |   7 ++
> >>  drivers/net/ethernet/intel/idpf/idpf_lib.c  |   1 +
> >>  drivers/net/ethernet/intel/idpf/idpf_txrx.c |   4 +
> >>  drivers/net/ethernet/intel/idpf/xdp.c       | 114 ++++++++++++++++++++
> >>  5 files changed, 129 insertions(+), 1 deletion(-)
> >>
> > 
> > (...)
> > 
> >> +
> >> +/**
> >> + * idpf_xdp_setup_prog - handle XDP program install/remove requests
> >> + * @vport: vport to configure
> >> + * @xdp: request data (program, extack)
> >> + *
> >> + * Return: 0 on success, -errno on failure.
> >> + */
> >> +static int
> >> +idpf_xdp_setup_prog(struct idpf_vport *vport, const struct netdev_bpf *xdp)
> >> +{
> >> +	const struct idpf_netdev_priv *np = netdev_priv(vport->netdev);
> >> +	struct bpf_prog *old, *prog = xdp->prog;
> >> +	struct idpf_vport_config *cfg;
> >> +	int ret;
> >> +
> >> +	cfg = vport->adapter->vport_config[vport->idx];
> >> +	if (!vport->num_xdp_txq && vport->num_txq == cfg->max_q.max_txq) {
> >> +		NL_SET_ERR_MSG_MOD(xdp->extack,
> >> +				   "No Tx queues available for XDP, please decrease the number of regular SQs");
> >> +		return -ENOSPC;
> >> +	}
> >> +
> >> +	if (test_bit(IDPF_REMOVE_IN_PROG, vport->adapter->flags) ||
> > 
> > IN_PROG is a bit unfortunate here as it mixes with 'prog' :P
> 
> Authentic idpf dictionary ¯\_(ツ)_/¯
> 
> > 
> >> +	    !!vport->xdp_prog == !!prog) {
> >> +		if (np->state == __IDPF_VPORT_UP)
> >> +			idpf_copy_xdp_prog_to_qs(vport, prog);
> >> +
> >> +		old = xchg(&vport->xdp_prog, prog);
> >> +		if (old)
> >> +			bpf_prog_put(old);
> >> +
> >> +		cfg->user_config.xdp_prog = prog;
> >> +
> >> +		return 0;
> >> +	}
> >> +
> >> +	old = cfg->user_config.xdp_prog;
> >> +	cfg->user_config.xdp_prog = prog;
> >> +
> >> +	ret = idpf_initiate_soft_reset(vport, IDPF_SR_Q_CHANGE);
> >> +	if (ret) {
> >> +		NL_SET_ERR_MSG_MOD(xdp->extack,
> >> +				   "Could not reopen the vport after XDP setup");
> >> +
> >> +		if (prog)
> >> +			bpf_prog_put(prog);
> > 
> > aren't you missing this for prog->NULL conversion? you have this for
> > hot-swap case (prog->prog).
> 
> This path (soft_reset) handles NULL => prog and prog => NULL. This
> branch in particular handles errors during the soft reset, when we need
> to restore the original prog and put the new one.
> 
> What you probably meant is that I don't have bpf_prog_put(old) in case
> everything went well below? Breh =\

yes, best to check with bpftool if there are dangling bpf progs on system
after using few xdp samples, for example.

> 
> > 
> >> +
> >> +		cfg->user_config.xdp_prog = old;
> >> +	}
> >> +
> >> +	return ret;
> >> +}
> 
> Thanks,
> Olek
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.h b/drivers/net/ethernet/intel/idpf/idpf_txrx.h
index 6d9eb6f4ab38..38ef0db08133 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_txrx.h
+++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.h
@@ -485,6 +485,7 @@  struct idpf_txq_stash {
  * @desc_ring: virtual descriptor ring address
  * @bufq_sets: Pointer to the array of buffer queues in splitq mode
  * @napi: NAPI instance corresponding to this queue (splitq)
+ * @xdp_prog: attached XDP program
  * @rx_buf: See struct &libeth_fqe
  * @pp: Page pool pointer in singleq mode
  * @tail: Tail offset. Used for both queue models single and split.
@@ -525,13 +526,14 @@  struct idpf_rx_queue {
 		struct {
 			struct idpf_bufq_set *bufq_sets;
 			struct napi_struct *napi;
+			struct bpf_prog __rcu *xdp_prog;
 		};
 		struct {
 			struct libeth_fqe *rx_buf;
 			struct page_pool *pp;
+			void __iomem *tail;
 		};
 	};
-	void __iomem *tail;
 
 	DECLARE_BITMAP(flags, __IDPF_Q_FLAGS_NBITS);
 	u16 idx;
diff --git a/drivers/net/ethernet/intel/idpf/xdp.h b/drivers/net/ethernet/intel/idpf/xdp.h
index 8ace8384f348..a72a7638a6ea 100644
--- a/drivers/net/ethernet/intel/idpf/xdp.h
+++ b/drivers/net/ethernet/intel/idpf/xdp.h
@@ -6,12 +6,19 @@ 
 
 #include <linux/types.h>
 
+struct bpf_prog;
 struct idpf_vport;
+struct net_device;
+struct netdev_bpf;
 
 int idpf_xdp_rxq_info_init_all(const struct idpf_vport *vport);
 void idpf_xdp_rxq_info_deinit_all(const struct idpf_vport *vport);
+void idpf_copy_xdp_prog_to_qs(const struct idpf_vport *vport,
+			      struct bpf_prog *xdp_prog);
 
 int idpf_vport_xdpq_get(const struct idpf_vport *vport);
 void idpf_vport_xdpq_put(const struct idpf_vport *vport);
 
+int idpf_xdp(struct net_device *dev, struct netdev_bpf *xdp);
+
 #endif /* _IDPF_XDP_H_ */
diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/ethernet/intel/idpf/idpf_lib.c
index 0f4edc9cd1ad..84ca8c08bd56 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
@@ -2368,4 +2368,5 @@  static const struct net_device_ops idpf_netdev_ops = {
 	.ndo_get_stats64 = idpf_get_stats64,
 	.ndo_set_features = idpf_set_features,
 	.ndo_tx_timeout = idpf_tx_timeout,
+	.ndo_bpf = idpf_xdp,
 };
diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
index 97513822d614..e152fbe4ebe3 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
@@ -1102,6 +1102,8 @@  static void idpf_vport_queue_grp_rel_all(struct idpf_vport *vport)
  */
 void idpf_vport_queues_rel(struct idpf_vport *vport)
 {
+	idpf_copy_xdp_prog_to_qs(vport, NULL);
+
 	idpf_tx_desc_rel_all(vport);
 	idpf_rx_desc_rel_all(vport);
 
@@ -1664,6 +1666,8 @@  int idpf_vport_queues_alloc(struct idpf_vport *vport)
 	if (err)
 		goto err_out;
 
+	idpf_copy_xdp_prog_to_qs(vport, vport->xdp_prog);
+
 	return 0;
 
 err_out:
diff --git a/drivers/net/ethernet/intel/idpf/xdp.c b/drivers/net/ethernet/intel/idpf/xdp.c
index 8770249b5abe..c0322fa7bfee 100644
--- a/drivers/net/ethernet/intel/idpf/xdp.c
+++ b/drivers/net/ethernet/intel/idpf/xdp.c
@@ -4,6 +4,7 @@ 
 #include <net/libeth/xdp.h>
 
 #include "idpf.h"
+#include "idpf_virtchnl.h"
 #include "xdp.h"
 
 static int idpf_rxq_for_each(const struct idpf_vport *vport,
@@ -115,6 +116,33 @@  void idpf_xdp_rxq_info_deinit_all(const struct idpf_vport *vport)
 			  (void *)(size_t)vport->rxq_model);
 }
 
+static int idpf_xdp_rxq_assign_prog(struct idpf_rx_queue *rxq, void *arg)
+{
+	struct mutex *lock = &rxq->q_vector->vport->adapter->vport_ctrl_lock;
+	struct bpf_prog *prog = arg;
+	struct bpf_prog *old;
+
+	if (prog)
+		bpf_prog_inc(prog);
+
+	old = rcu_replace_pointer(rxq->xdp_prog, prog, lockdep_is_held(lock));
+	if (old)
+		bpf_prog_put(old);
+
+	return 0;
+}
+
+/**
+ * idpf_copy_xdp_prog_to_qs - set pointers to XDP program for each Rx queue
+ * @vport: vport to setup XDP for
+ * @xdp_prog: XDP program that should be copied to all Rx queues
+ */
+void idpf_copy_xdp_prog_to_qs(const struct idpf_vport *vport,
+			      struct bpf_prog *xdp_prog)
+{
+	idpf_rxq_for_each(vport, idpf_xdp_rxq_assign_prog, xdp_prog);
+}
+
 int idpf_vport_xdpq_get(const struct idpf_vport *vport)
 {
 	struct libeth_xdpsq_timer **timers __free(kvfree) = NULL;
@@ -187,3 +215,89 @@  void idpf_vport_xdpq_put(const struct idpf_vport *vport)
 		idpf_queue_clear(NOIRQ, xdpq);
 	}
 }
+
+/**
+ * idpf_xdp_setup_prog - handle XDP program install/remove requests
+ * @vport: vport to configure
+ * @xdp: request data (program, extack)
+ *
+ * Return: 0 on success, -errno on failure.
+ */
+static int
+idpf_xdp_setup_prog(struct idpf_vport *vport, const struct netdev_bpf *xdp)
+{
+	const struct idpf_netdev_priv *np = netdev_priv(vport->netdev);
+	struct bpf_prog *old, *prog = xdp->prog;
+	struct idpf_vport_config *cfg;
+	int ret;
+
+	cfg = vport->adapter->vport_config[vport->idx];
+	if (!vport->num_xdp_txq && vport->num_txq == cfg->max_q.max_txq) {
+		NL_SET_ERR_MSG_MOD(xdp->extack,
+				   "No Tx queues available for XDP, please decrease the number of regular SQs");
+		return -ENOSPC;
+	}
+
+	if (test_bit(IDPF_REMOVE_IN_PROG, vport->adapter->flags) ||
+	    !!vport->xdp_prog == !!prog) {
+		if (np->state == __IDPF_VPORT_UP)
+			idpf_copy_xdp_prog_to_qs(vport, prog);
+
+		old = xchg(&vport->xdp_prog, prog);
+		if (old)
+			bpf_prog_put(old);
+
+		cfg->user_config.xdp_prog = prog;
+
+		return 0;
+	}
+
+	old = cfg->user_config.xdp_prog;
+	cfg->user_config.xdp_prog = prog;
+
+	ret = idpf_initiate_soft_reset(vport, IDPF_SR_Q_CHANGE);
+	if (ret) {
+		NL_SET_ERR_MSG_MOD(xdp->extack,
+				   "Could not reopen the vport after XDP setup");
+
+		if (prog)
+			bpf_prog_put(prog);
+
+		cfg->user_config.xdp_prog = old;
+	}
+
+	return ret;
+}
+
+/**
+ * idpf_xdp - handle XDP-related requests
+ * @dev: network device to configure
+ * @xdp: request data (program, extack)
+ *
+ * Return: 0 on success, -errno on failure.
+ */
+int idpf_xdp(struct net_device *dev, struct netdev_bpf *xdp)
+{
+	struct idpf_vport *vport;
+	int ret;
+
+	idpf_vport_ctrl_lock(dev);
+	vport = idpf_netdev_to_vport(dev);
+
+	if (!idpf_is_queue_model_split(vport->txq_model))
+		goto notsupp;
+
+	switch (xdp->command) {
+	case XDP_SETUP_PROG:
+		ret = idpf_xdp_setup_prog(vport, xdp);
+		break;
+	default:
+notsupp:
+		ret = -EOPNOTSUPP;
+		break;
+	}
+
+	idpf_vport_ctrl_unlock(dev);
+
+	return ret;
+}