Thanks Ciara, I missed the af_xdp.rst :(

Will update and push a v5 shortly


On 15/12/2023 09:55, Loftus, Ciara wrote:
Thanks for the latest patch Maryam. I have one minor suggestion inline.
Also, there are still some references to "use_cni" in af_xdp.rst which should 
be removed/replaced with uds_path.
Once that's done I think the patch should be good to go. Perhaps also consider 
adding a note to the release notes mentioning the new functionality.

Thanks,
Ciara

With the original 'use_cni' implementation, (using a
hardcoded socket rather than a configurable one),
if a DPDK pod is requesting multiple net devices
and these devices are from different pools, then
the container attempts to mount all the netdev UDSes
in the pod as /tmp/afxdp.sock. Which means that at best
only 1 netdev will handshake correctly with the AF_XDP
DP. This patch addresses this by making the socket
parameter configurable using a new vdev param called
'uds_path' and removing the previous 'use_cni' param.
This patch also fixes incorrect references to the
AF_XDP DP as CNI and updates the documentation with a
working example. This change has been tested with the
AF_XDP DP PR 81, with both single and multiple interfaces.

v4:
* Rename af_xdp_cni.rst to af_xdp_dp.rst
* Removed all incorrect references to CNI throughout af_xdp
   PMD file.
* Fixed Typos in af_xdp_dp.rst

v3:
* Remove `use_cni` vdev argument as it's no longer needed.
* Update incorrect CNI references for the AF_XDP DP in the
   documentation.
* Update the documentation to run a simple example with the
   AF_XDP DP plugin in K8s.

v2:
* Rename sock_path to uds_path.
* Update documentation to reflect when CAP_BPF is needed.
* Fix testpmd arguments in the provided example for Pods.
* Use AF_XDP API to update the xskmap entry.

Signed-off-by: Maryam Tahhan <mtah...@redhat.com>
---
  doc/guides/howto/af_xdp_cni.rst     | 253 -------------------------
  doc/guides/howto/af_xdp_dp.rst      | 278
++++++++++++++++++++++++++++
  doc/guides/howto/index.rst          |   2 +-
  drivers/net/af_xdp/rte_eth_af_xdp.c | 100 +++++-----
  4 files changed, 328 insertions(+), 305 deletions(-)
  delete mode 100644 doc/guides/howto/af_xdp_cni.rst
  create mode 100644 doc/guides/howto/af_xdp_dp.rst

<snip>

diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c
b/drivers/net/af_xdp/rte_eth_af_xdp.c
index 353c8688ec..6caad58e60 100644
--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
@@ -88,7 +88,6 @@ RTE_LOG_REGISTER_DEFAULT(af_xdp_logtype, NOTICE);
  #define UDS_MAX_CMD_LEN                       64
  #define UDS_MAX_CMD_RESP              128
  #define UDS_XSK_MAP_FD_MSG            "/xsk_map_fd"
-#define UDS_SOCK                       "/tmp/afxdp.sock"
  #define UDS_CONNECT_MSG                       "/connect"
  #define UDS_HOST_OK_MSG                       "/host_ok"
  #define UDS_HOST_NAK_MSG              "/host_nak"
@@ -170,7 +169,7 @@ struct pmd_internals {
        char prog_path[PATH_MAX];
        bool custom_prog_configured;
        bool force_copy;
-       bool use_cni;
+       char uds_path[PATH_MAX];
        struct bpf_map *map;

        struct rte_ether_addr eth_addr;
@@ -190,7 +189,7 @@ struct pmd_process_private {
  #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"
-#define ETH_AF_XDP_USE_CNI_ARG                 "use_cni"
+#define ETH_AF_XDP_USE_DP_UDS_PATH_ARG "uds_path"
Use the same alignment for "uds_path" as the strings above it.

  static const char * const valid_arguments[] = {
        ETH_AF_XDP_IFACE_ARG,
@@ -200,7 +199,7 @@ static const char * const valid_arguments[] = {
        ETH_AF_XDP_PROG_ARG,
        ETH_AF_XDP_BUDGET_ARG,
        ETH_AF_XDP_FORCE_COPY_ARG,
-       ETH_AF_XDP_USE_CNI_ARG,
+       ETH_AF_XDP_USE_DP_UDS_PATH_ARG,
        NULL
  };

@@ -1351,7 +1350,7 @@ configure_preferred_busy_poll(struct
pkt_rx_queue *rxq)
  }

  static int
-init_uds_sock(struct sockaddr_un *server)
+init_uds_sock(struct sockaddr_un *server, const char *uds_path)
  {
        int sock;

@@ -1362,7 +1361,7 @@ init_uds_sock(struct sockaddr_un *server)
        }

        server->sun_family = AF_UNIX;
-       strlcpy(server->sun_path, UDS_SOCK, sizeof(server->sun_path));
+       strlcpy(server->sun_path, uds_path, sizeof(server->sun_path));

        if (connect(sock, (struct sockaddr *)server, sizeof(struct
sockaddr_un)) < 0) {
                close(sock);
@@ -1382,7 +1381,7 @@ struct msg_internal {
  };

  static int
-send_msg(int sock, char *request, int *fd)
+send_msg(int sock, char *request, int *fd, const char *uds_path)
  {
        int snd;
        struct iovec iov;
@@ -1393,7 +1392,7 @@ send_msg(int sock, char *request, int *fd)

        memset(&dst, 0, sizeof(dst));
        dst.sun_family = AF_UNIX;
-       strlcpy(dst.sun_path, UDS_SOCK, sizeof(dst.sun_path));
+       strlcpy(dst.sun_path, uds_path, sizeof(dst.sun_path));

        /* Initialize message header structure */
        memset(&msgh, 0, sizeof(msgh));
@@ -1470,8 +1469,8 @@ read_msg(int sock, char *response, struct
sockaddr_un *s, int *fd)
  }

  static int
-make_request_cni(int sock, struct sockaddr_un *server, char *request,
-                int *req_fd, char *response, int *out_fd)
+make_request_dp(int sock, struct sockaddr_un *server, char *request,
+                int *req_fd, char *response, int *out_fd, const char
*uds_path)
  {
        int rval;

@@ -1483,7 +1482,7 @@ make_request_cni(int sock, struct sockaddr_un
*server, char *request,
        if (req_fd == NULL)
                rval = write(sock, request, strlen(request));
        else
-               rval = send_msg(sock, request, req_fd);
+               rval = send_msg(sock, request, req_fd, uds_path);

        if (rval < 0) {
                AF_XDP_LOG(ERR, "Write error %s\n", strerror(errno));
@@ -1507,7 +1506,7 @@ check_response(char *response, char *exp_resp,
long size)
  }

  static int
-get_cni_fd(char *if_name)
+get_xskmap_fd(char *if_name, const char *uds_path)
  {
        char request[UDS_MAX_CMD_LEN],
response[UDS_MAX_CMD_RESP];
        char hostname[MAX_LONG_OPT_SZ],
exp_resp[UDS_MAX_CMD_RESP];
@@ -1520,14 +1519,14 @@ get_cni_fd(char *if_name)
                return -1;

        memset(&server, 0, sizeof(server));
-       sock = init_uds_sock(&server);
+       sock = init_uds_sock(&server, uds_path);
        if (sock < 0)
                return -1;

-       /* Initiates handshake to CNI send: /connect,hostname */
+       /* Initiates handshake to AF_XDP Device Plugin send:
/connect,hostname */
        snprintf(request, sizeof(request), "%s,%s", UDS_CONNECT_MSG,
hostname);
        memset(response, 0, sizeof(response));
-       if (make_request_cni(sock, &server, request, NULL, response,
&out_fd) < 0) {
+       if (make_request_dp(sock, &server, request, NULL, response,
&out_fd, uds_path) < 0) {
                AF_XDP_LOG(ERR, "Error in processing cmd [%s]\n", request);
                goto err_close;
        }
@@ -1541,7 +1540,7 @@ get_cni_fd(char *if_name)
        /* Request for "/version" */
        strlcpy(request, UDS_VERSION_MSG, UDS_MAX_CMD_LEN);
        memset(response, 0, sizeof(response));
-       if (make_request_cni(sock, &server, request, NULL, response,
&out_fd) < 0) {
+       if (make_request_dp(sock, &server, request, NULL, response,
&out_fd, uds_path) < 0) {
                AF_XDP_LOG(ERR, "Error in processing cmd [%s]\n", request);
                goto err_close;
        }
@@ -1549,7 +1548,7 @@ get_cni_fd(char *if_name)
        /* Request for file descriptor for netdev name*/
        snprintf(request, sizeof(request), "%s,%s", UDS_XSK_MAP_FD_MSG,
if_name);
        memset(response, 0, sizeof(response));
-       if (make_request_cni(sock, &server, request, NULL, response,
&out_fd) < 0) {
+       if (make_request_dp(sock, &server, request, NULL, response,
&out_fd, uds_path) < 0) {
                AF_XDP_LOG(ERR, "Error in processing cmd [%s]\n", request);
                goto err_close;
        }
@@ -1571,7 +1570,7 @@ get_cni_fd(char *if_name)
        /* Initiate close connection */
        strlcpy(request, UDS_FIN_MSG, UDS_MAX_CMD_LEN);
        memset(response, 0, sizeof(response));
-       if (make_request_cni(sock, &server, request, NULL, response,
&out_fd) < 0) {
+       if (make_request_dp(sock, &server, request, NULL, response,
&out_fd, uds_path) < 0) {
                AF_XDP_LOG(ERR, "Error in processing cmd [%s]\n", request);
                goto err_close;
        }
@@ -1640,7 +1639,7 @@ xsk_configure(struct pmd_internals *internals,
struct pkt_rx_queue *rxq,
  #endif

        /* Disable libbpf from loading XDP program */
-       if (internals->use_cni)
+       if (strnlen(internals->uds_path, PATH_MAX))
                cfg.libbpf_flags |= XSK_LIBBPF_FLAGS__INHIBIT_PROG_LOAD;

        if (strnlen(internals->prog_path, PATH_MAX)) {
@@ -1694,18 +1693,17 @@ xsk_configure(struct pmd_internals *internals,
struct pkt_rx_queue *rxq,
                }
        }

-       if (internals->use_cni) {
-               int err, fd, map_fd;
+       if (strnlen(internals->uds_path, PATH_MAX)) {
+               int err, map_fd;

-               /* get socket fd from CNI plugin */
-               map_fd = get_cni_fd(internals->if_name);
+               /* get socket fd from AF_XDP Device plugin */
+               map_fd = get_xskmap_fd(internals->if_name, internals-
uds_path);
                if (map_fd < 0) {
-                       AF_XDP_LOG(ERR, "Failed to receive CNI plugin fd\n");
+                       AF_XDP_LOG(ERR, "Failed to receive AF_XDP Device
plugin fd\n");
                        goto out_xsk;
                }
-               /* get socket fd */
-               fd = xsk_socket__fd(rxq->xsk);
-               err = bpf_map_update_elem(map_fd, &rxq->xsk_queue_idx,
&fd, 0);
+
+               err = xsk_socket__update_xskmap(rxq->xsk, map_fd);
                if (err) {
                        AF_XDP_LOG(ERR, "Failed to insert unprivileged xsk in
map.\n");
                        goto out_xsk;
@@ -1881,13 +1879,13 @@ static const struct eth_dev_ops ops = {
        .get_monitor_addr = eth_get_monitor_addr,
  };

-/* CNI option works in unprivileged container environment
- * and ethernet device functionality will be reduced. So
- * additional customiszed eth_dev_ops struct is needed
- * for cni. Promiscuous enable and disable functionality
- * is removed.
- **/
-static const struct eth_dev_ops ops_cni = {
+/* AF_XDP Device Plugin option works in unprivileged
+ * container environment and ethernet device functionality
+ * will be reduced. So additional customized eth_dev_ops
+ * struct is needed for the AF_XDP Device Plugin. Promiscuous
+ * enable and disable functionality is removed.
+ */
+static const struct eth_dev_ops ops_afxdp_dp = {
        .dev_start = eth_dev_start,
        .dev_stop = eth_dev_stop,
        .dev_close = eth_dev_close,
@@ -1957,7 +1955,7 @@ parse_name_arg(const char *key __rte_unused,

  /** parse xdp prog argument */
  static int
-parse_prog_arg(const char *key __rte_unused,
+parse_path_arg(const char *key __rte_unused,
               const char *value, void *extra_args)
  {
        char *path = extra_args;
@@ -2023,7 +2021,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 *force_copy, int *use_cni)
+                int *busy_budget, int *force_copy, char *uds_path)
  {
        int ret;

@@ -2050,7 +2048,7 @@ parse_parameters(struct rte_kvargs *kvlist, char
*if_name, int *start_queue,
                goto free_kvlist;

        ret = rte_kvargs_process(kvlist, ETH_AF_XDP_PROG_ARG,
-                                &parse_prog_arg, prog_path);
+                                &parse_path_arg, prog_path);
        if (ret < 0)
                goto free_kvlist;

@@ -2064,8 +2062,8 @@ 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_USE_CNI_ARG,
-                                &parse_integer_arg, use_cni);
+       ret = rte_kvargs_process(kvlist,
ETH_AF_XDP_USE_DP_UDS_PATH_ARG,
+                                &parse_path_arg, uds_path);
        if (ret < 0)
                goto free_kvlist;

@@ -2108,7 +2106,7 @@ 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, int force_copy,
-              int use_cni)
+                  const char *uds_path)
  {
        const char *name = rte_vdev_device_name(dev);
        const unsigned int numa_node = dev->device.numa_node;
@@ -2137,7 +2135,7 @@ init_internals(struct rte_vdev_device *dev, const
char *if_name,
  #endif
        internals->shared_umem = shared_umem;
        internals->force_copy = force_copy;
-       internals->use_cni = use_cni;
+       strlcpy(internals->uds_path, uds_path, PATH_MAX);

        if (xdp_get_channels_info(if_name, &internals->max_queue_cnt,
                                  &internals->combined_queue_cnt)) {
@@ -2196,10 +2194,10 @@ init_internals(struct rte_vdev_device *dev, const
char *if_name,
        eth_dev->data->dev_link = pmd_link;
        eth_dev->data->mac_addrs = &internals->eth_addr;
        eth_dev->data->dev_flags |=
RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
-       if (!internals->use_cni)
+       if (!strnlen(internals->uds_path, PATH_MAX))
                eth_dev->dev_ops = &ops;
        else
-               eth_dev->dev_ops = &ops_cni;
+               eth_dev->dev_ops = &ops_afxdp_dp;

        eth_dev->rx_pkt_burst = eth_af_xdp_rx;
        eth_dev->tx_pkt_burst = eth_af_xdp_tx;
@@ -2327,7 +2325,7 @@ rte_pmd_af_xdp_probe(struct rte_vdev_device
*dev)
        char prog_path[PATH_MAX] = {'\0'};
        int busy_budget = -1, ret;
        int force_copy = 0;
-       int use_cni = 0;
+       char uds_path[PATH_MAX] = {'\0'};
        struct rte_eth_dev *eth_dev = NULL;
        const char *name = rte_vdev_device_name(dev);

@@ -2370,20 +2368,20 @@ 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, &force_copy, &use_cni) < 0) {
+                                &busy_budget, &force_copy, uds_path) < 0) {
                AF_XDP_LOG(ERR, "Invalid kvargs value\n");
                return -EINVAL;
        }

-       if (use_cni && busy_budget > 0) {
+       if (strnlen(uds_path, PATH_MAX) && busy_budget > 0) {
                AF_XDP_LOG(ERR, "When '%s' parameter is used, '%s'
parameter is not valid\n",
-                       ETH_AF_XDP_USE_CNI_ARG,
ETH_AF_XDP_BUDGET_ARG);
+                       ETH_AF_XDP_USE_DP_UDS_PATH_ARG,
ETH_AF_XDP_BUDGET_ARG);
                return -EINVAL;
        }

-       if (use_cni && strnlen(prog_path, PATH_MAX)) {
+       if (strnlen(uds_path, PATH_MAX) && strnlen(prog_path, PATH_MAX))
{
                AF_XDP_LOG(ERR, "When '%s' parameter is used, '%s'
parameter is not valid\n",
-                       ETH_AF_XDP_USE_CNI_ARG,
ETH_AF_XDP_PROG_ARG);
+                       ETH_AF_XDP_USE_DP_UDS_PATH_ARG,
ETH_AF_XDP_PROG_ARG);
                        return -EINVAL;
        }

@@ -2410,7 +2408,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, force_copy, use_cni);
+                                busy_budget, force_copy, uds_path);
        if (eth_dev == NULL) {
                AF_XDP_LOG(ERR, "Failed to init internals\n");
                return -1;
@@ -2471,4 +2469,4 @@ RTE_PMD_REGISTER_PARAM_STRING(net_af_xdp,
                              "xdp_prog=<string> "
                              "busy_budget=<int> "
                              "force_copy=<int> "
-                             "use_cni=<int> ");
+                             "uds_path=<string> ");
--
2.41.0


Reply via email to