[PATCH] net/af_xdp: allow using copy mode in XSK

[PATCH] net/af_xdp: allow using copy mode in XSK

[Xiaoyun Li]

DPDK assumes that users only want AF_XDP socket (XSK) into zero copy
mode when the kernel supports it. However, sometimes kernel driver
doesn't support it well and copy mode is more stable and preferred.

This patch allows using devarg "-a xx:xx.x,no_zerocopy=1" to force
the AF_XDP socket into copy mode.

Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
---
 doc/guides/nics/af_xdp.rst          |  2 ++
 drivers/net/af_xdp/rte_eth_af_xdp.c | 25 ++++++++++++++++++++-----
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/doc/guides/nics/af_xdp.rst b/doc/guides/nics/af_xdp.rst
index 56681c8365..5bcdf55d05 100644
--- a/doc/guides/nics/af_xdp.rst
+++ b/doc/guides/nics/af_xdp.rst
@@ -36,6 +36,8 @@ The following options can be provided to set up an af_xdp port in DPDK.
     default 0);
 *   ``xdp_prog`` - path to custom xdp program (optional, default none);
 *   ``busy_budget`` - busy polling budget (optional, default 64);
+*   ``no_zerocopy`` - PMD will force AF_XDP socket into copy mode (optional,
+    default 0);
 
 Prerequisites
 -------------
diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c
index 1e37da6e84..61d7eabc14 100644
--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
@@ -150,6 +150,7 @@ struct pmd_internals {
 	bool shared_umem;
 	char prog_path[PATH_MAX];
 	bool custom_prog_configured;
+	bool no_zerocopy;
 	struct bpf_map *map;
 
 	struct rte_ether_addr eth_addr;
@@ -168,6 +169,7 @@ struct pmd_process_private {
 #define ETH_AF_XDP_SHARED_UMEM_ARG		"shared_umem"
 #define ETH_AF_XDP_PROG_ARG			"xdp_prog"
 #define ETH_AF_XDP_BUDGET_ARG			"busy_budget"
+#define ETH_AF_XDP_NO_ZEROCOPY_ARG              "no_zerocopy"
 
 static const char * const valid_arguments[] = {
 	ETH_AF_XDP_IFACE_ARG,
@@ -176,6 +178,7 @@ static const char * const valid_arguments[] = {
 	ETH_AF_XDP_SHARED_UMEM_ARG,
 	ETH_AF_XDP_PROG_ARG,
 	ETH_AF_XDP_BUDGET_ARG,
+	ETH_AF_XDP_NO_ZEROCOPY_ARG,
 	NULL
 };
 
@@ -1308,6 +1311,10 @@ xsk_configure(struct pmd_internals *internals, struct pkt_rx_queue *rxq,
 	cfg.xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 	cfg.bind_flags = 0;
 
+	/* Force AF_XDP socket into copy mode when users want no zerocopy mode */
+	if (internals->no_zerocopy)
+		cfg.bind_flags |= XDP_COPY;
+
 #if defined(XDP_USE_NEED_WAKEUP)
 	cfg.bind_flags |= XDP_USE_NEED_WAKEUP;
 #endif
@@ -1655,7 +1662,7 @@ xdp_get_channels_info(const char *if_name, int *max_queues,
 static int
 parse_parameters(struct rte_kvargs *kvlist, char *if_name, int *start_queue,
 			int *queue_cnt, int *shared_umem, char *prog_path,
-			int *busy_budget)
+			int *busy_budget, int *no_zerocopy)
 {
 	int ret;
 
@@ -1691,6 +1698,11 @@ parse_parameters(struct rte_kvargs *kvlist, char *if_name, int *start_queue,
 	if (ret < 0)
 		goto free_kvlist;
 
+	ret = rte_kvargs_process(kvlist, ETH_AF_XDP_NO_ZEROCOPY_ARG,
+				&parse_integer_arg, no_zerocopy);
+	if (ret < 0)
+		goto free_kvlist;
+
 free_kvlist:
 	rte_kvargs_free(kvlist);
 	return ret;
@@ -1729,7 +1741,7 @@ get_iface_info(const char *if_name,
 static struct rte_eth_dev *
 init_internals(struct rte_vdev_device *dev, const char *if_name,
 		int start_queue_idx, int queue_cnt, int shared_umem,
-		const char *prog_path, int busy_budget)
+		const char *prog_path, int busy_budget, int no_zerocopy)
 {
 	const char *name = rte_vdev_device_name(dev);
 	const unsigned int numa_node = dev->device.numa_node;
@@ -1757,6 +1769,7 @@ init_internals(struct rte_vdev_device *dev, const char *if_name,
 	}
 #endif
 	internals->shared_umem = shared_umem;
+	internals->no_zerocopy = no_zerocopy;
 
 	if (xdp_get_channels_info(if_name, &internals->max_queue_cnt,
 				  &internals->combined_queue_cnt)) {
@@ -1941,6 +1954,7 @@ rte_pmd_af_xdp_probe(struct rte_vdev_device *dev)
 	int shared_umem = 0;
 	char prog_path[PATH_MAX] = {'\0'};
 	int busy_budget = -1, ret;
+	int no_zerocopy = 0;
 	struct rte_eth_dev *eth_dev = NULL;
 	const char *name = rte_vdev_device_name(dev);
 
@@ -1986,7 +2000,7 @@ rte_pmd_af_xdp_probe(struct rte_vdev_device *dev)
 
 	if (parse_parameters(kvlist, if_name, &xsk_start_queue_idx,
 			     &xsk_queue_cnt, &shared_umem, prog_path,
-			     &busy_budget) < 0) {
+			     &busy_budget, &no_zerocopy) < 0) {
 		AF_XDP_LOG(ERR, "Invalid kvargs value\n");
 		return -EINVAL;
 	}
@@ -2001,7 +2015,7 @@ rte_pmd_af_xdp_probe(struct rte_vdev_device *dev)
 
 	eth_dev = init_internals(dev, if_name, xsk_start_queue_idx,
 					xsk_queue_cnt, shared_umem, prog_path,
-					busy_budget);
+					busy_budget, no_zerocopy);
 	if (eth_dev == NULL) {
 		AF_XDP_LOG(ERR, "Failed to init internals\n");
 		return -1;
@@ -2060,4 +2074,5 @@ RTE_PMD_REGISTER_PARAM_STRING(net_af_xdp,
 			      "queue_count=<int> "
 			      "shared_umem=<int> "
 			      "xdp_prog=<string> "
-			      "busy_budget=<int>");
+			      "busy_budget=<int> "
+			      "no_zerocopy=<int> ");
-- 
2.25.1

RE: [PATCH] net/af_xdp: allow using copy mode in XSK

[Zhang, Qi Z]

> -----Original Message-----
> From: Li, Xiaoyun <xiaoyun.li@intel.com>
> Sent: Monday, June 13, 2022 11:13 PM
> To: Loftus, Ciara <ciara.loftus@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; dev@dpdk.org
> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>
> Subject: [PATCH] net/af_xdp: allow using copy mode in XSK
> 
> DPDK assumes that users only want AF_XDP socket (XSK) into zero copy mode
> when the kernel supports it. However, sometimes kernel driver doesn't
> support it well and copy mode is more stable and preferred.

rename "no_zerocopy" to "force_copy" looks better? 

This aligned with the comment on the macro definition in kernel

#define XDP_COPY        (1 << 1) /* Force copy-mode */

And also make the below code more smooth.

	if (internals->force_copy)
		cfg.bind_flags |= XDP_COPY;

RE: [PATCH] net/af_xdp: allow using copy mode in XSK

[Li, Xiaoyun]

Hi

> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: Tuesday, June 14, 2022 00:38
> To: Li, Xiaoyun <xiaoyun.li@intel.com>; Loftus, Ciara
> <ciara.loftus@intel.com>; dev@dpdk.org
> Subject: RE: [PATCH] net/af_xdp: allow using copy mode in XSK
> 
> 
> 
> > -----Original Message-----
> > From: Li, Xiaoyun <xiaoyun.li@intel.com>
> > Sent: Monday, June 13, 2022 11:13 PM
> > To: Loftus, Ciara <ciara.loftus@intel.com>; Zhang, Qi Z
> > <qi.z.zhang@intel.com>; dev@dpdk.org
> > Cc: Li, Xiaoyun <xiaoyun.li@intel.com>
> > Subject: [PATCH] net/af_xdp: allow using copy mode in XSK
> >
> > DPDK assumes that users only want AF_XDP socket (XSK) into zero copy
> > mode when the kernel supports it. However, sometimes kernel driver
> > doesn't support it well and copy mode is more stable and preferred.
> 
> rename "no_zerocopy" to "force_copy" looks better?
> 
> This aligned with the comment on the macro definition in kernel
> 
> #define XDP_COPY        (1 << 1) /* Force copy-mode */
> 
> And also make the below code more smooth.
> 
> 	if (internals->force_copy)
> 		cfg.bind_flags |= XDP_COPY;

Sounds good. Thanks. Will do in the next version.
> 
> 

[PATCH v2] net/af_xdp: allow using copy mode in XSK

[Xiaoyun Li]

DPDK assumes that users only want AF_XDP socket (XSK) into zero copy
mode when the kernel supports it. However, sometimes kernel driver
doesn't support it well and copy mode is more stable and preferred.

This patch allows using devarg "-a xx:xx.x,force_copy=1" to force the
AF_XDP socket into copy mode.

Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
---
v2:
* Change name from no_zerocopy to force_copy.
---
 doc/guides/nics/af_xdp.rst          |  2 ++
 drivers/net/af_xdp/rte_eth_af_xdp.c | 25 ++++++++++++++++++++-----
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/doc/guides/nics/af_xdp.rst b/doc/guides/nics/af_xdp.rst
index 56681c8365..d42e0f1f79 100644
--- a/doc/guides/nics/af_xdp.rst
+++ b/doc/guides/nics/af_xdp.rst
@@ -36,6 +36,8 @@ The following options can be provided to set up an af_xdp port in DPDK.
     default 0);
 *   ``xdp_prog`` - path to custom xdp program (optional, default none);
 *   ``busy_budget`` - busy polling budget (optional, default 64);
+*   ``force_copy`` - PMD will force AF_XDP socket into copy mode (optional,
+    default 0);
 
 Prerequisites
 -------------
diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c
index 1e37da6e84..fce649c2a1 100644
--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
@@ -150,6 +150,7 @@ struct pmd_internals {
 	bool shared_umem;
 	char prog_path[PATH_MAX];
 	bool custom_prog_configured;
+	bool force_copy;
 	struct bpf_map *map;
 
 	struct rte_ether_addr eth_addr;
@@ -168,6 +169,7 @@ struct pmd_process_private {
 #define ETH_AF_XDP_SHARED_UMEM_ARG		"shared_umem"
 #define ETH_AF_XDP_PROG_ARG			"xdp_prog"
 #define ETH_AF_XDP_BUDGET_ARG			"busy_budget"
+#define ETH_AF_XDP_FORCE_COPY_ARG		"force_copy"
 
 static const char * const valid_arguments[] = {
 	ETH_AF_XDP_IFACE_ARG,
@@ -176,6 +178,7 @@ static const char * const valid_arguments[] = {
 	ETH_AF_XDP_SHARED_UMEM_ARG,
 	ETH_AF_XDP_PROG_ARG,
 	ETH_AF_XDP_BUDGET_ARG,
+	ETH_AF_XDP_FORCE_COPY_ARG,
 	NULL
 };
 
@@ -1308,6 +1311,10 @@ xsk_configure(struct pmd_internals *internals, struct pkt_rx_queue *rxq,
 	cfg.xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 	cfg.bind_flags = 0;
 
+	/* Force AF_XDP socket into copy mode when users want it */
+	if (internals->force_copy)
+		cfg.bind_flags |= XDP_COPY;
+
 #if defined(XDP_USE_NEED_WAKEUP)
 	cfg.bind_flags |= XDP_USE_NEED_WAKEUP;
 #endif
@@ -1655,7 +1662,7 @@ xdp_get_channels_info(const char *if_name, int *max_queues,
 static int
 parse_parameters(struct rte_kvargs *kvlist, char *if_name, int *start_queue,
 			int *queue_cnt, int *shared_umem, char *prog_path,
-			int *busy_budget)
+			int *busy_budget, int *force_copy)
 {
 	int ret;
 
@@ -1691,6 +1698,11 @@ parse_parameters(struct rte_kvargs *kvlist, char *if_name, int *start_queue,
 	if (ret < 0)
 		goto free_kvlist;
 
+	ret = rte_kvargs_process(kvlist, ETH_AF_XDP_FORCE_COPY_ARG,
+				&parse_integer_arg, force_copy);
+	if (ret < 0)
+		goto free_kvlist;
+
 free_kvlist:
 	rte_kvargs_free(kvlist);
 	return ret;
@@ -1729,7 +1741,7 @@ get_iface_info(const char *if_name,
 static struct rte_eth_dev *
 init_internals(struct rte_vdev_device *dev, const char *if_name,
 		int start_queue_idx, int queue_cnt, int shared_umem,
-		const char *prog_path, int busy_budget)
+		const char *prog_path, int busy_budget, int force_copy)
 {
 	const char *name = rte_vdev_device_name(dev);
 	const unsigned int numa_node = dev->device.numa_node;
@@ -1757,6 +1769,7 @@ init_internals(struct rte_vdev_device *dev, const char *if_name,
 	}
 #endif
 	internals->shared_umem = shared_umem;
+	internals->force_copy = force_copy;
 
 	if (xdp_get_channels_info(if_name, &internals->max_queue_cnt,
 				  &internals->combined_queue_cnt)) {
@@ -1941,6 +1954,7 @@ rte_pmd_af_xdp_probe(struct rte_vdev_device *dev)
 	int shared_umem = 0;
 	char prog_path[PATH_MAX] = {'\0'};
 	int busy_budget = -1, ret;
+	int force_copy = 0;
 	struct rte_eth_dev *eth_dev = NULL;
 	const char *name = rte_vdev_device_name(dev);
 
@@ -1986,7 +2000,7 @@ rte_pmd_af_xdp_probe(struct rte_vdev_device *dev)
 
 	if (parse_parameters(kvlist, if_name, &xsk_start_queue_idx,
 			     &xsk_queue_cnt, &shared_umem, prog_path,
-			     &busy_budget) < 0) {
+			     &busy_budget, &force_copy) < 0) {
 		AF_XDP_LOG(ERR, "Invalid kvargs value\n");
 		return -EINVAL;
 	}
@@ -2001,7 +2015,7 @@ rte_pmd_af_xdp_probe(struct rte_vdev_device *dev)
 
 	eth_dev = init_internals(dev, if_name, xsk_start_queue_idx,
 					xsk_queue_cnt, shared_umem, prog_path,
-					busy_budget);
+					busy_budget, force_copy);
 	if (eth_dev == NULL) {
 		AF_XDP_LOG(ERR, "Failed to init internals\n");
 		return -1;
@@ -2060,4 +2074,5 @@ RTE_PMD_REGISTER_PARAM_STRING(net_af_xdp,
 			      "queue_count=<int> "
 			      "shared_umem=<int> "
 			      "xdp_prog=<string> "
-			      "busy_budget=<int>");
+			      "busy_budget=<int> "
+			      "force_copy=<int> ");
-- 
2.25.1

Re: [PATCH v2] net/af_xdp: allow using copy mode in XSK

[Andrew Rybchenko]

On 6/14/22 12:10, Xiaoyun Li wrote:
> DPDK assumes that users only want AF_XDP socket (XSK) into zero copy
> mode when the kernel supports it. However, sometimes kernel driver
> doesn't support it well and copy mode is more stable and preferred.
> 
> This patch allows using devarg "-a xx:xx.x,force_copy=1" to force the
> AF_XDP socket into copy mode.
> 
> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
> ---
> v2:
> * Change name from no_zerocopy to force_copy.
> ---

Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

Applied to dpdk-next-net/main, thanks.