Hi Shibin As I've already explained in my previous email, they are constant ATM, however they will become configurable. I am implementing the Operator and it will make a lot of these "fixed" params configurable. My recommendation is not to try to generate the path in DPDK - as it's likely to be different in different k8s environments. As I've also mentioned, the current patch means that I don't need to come back in 2 months and update DPDK to support n paths for the UDS (aka future-proofing). Additionally - this is a side discussion as far as this patch goes. *The point of this patch is to fix the broken UDS behavior *and it has been tested in a full deployment scenario. @Shibin if you strongly feel that there's a better approach, then please go ahead, implement it, test it in a full deployment scenario and push it for review. In general, allowing the AF_XDP params to be configurable rather than fixing/hardcoding anything in DPDK decouples the AF_XDP DP from DPDK so we don't have to keep coming back to make changes. BR Maryam On 05/12/2023 10:29, Koikkara Reeny, Shibin wrote: > > Hi Maryam, > > Apologies for making it confusing. > > As per the afxdp-plugins-for-kubernetes code it looks like the > udsPodPath or udsSockDir[1] and udsPodSock[1] are constant and not > changing. Only the interface name is changing and we are already > passing the interface name through the command line. So I was > suggesting we can write a logic to create sock path from these logic. > > If I am wrong please correct me, Isn’t that the logic > afxdp-plugins-for-kubernetes doing? [2] > > This is only a suggestions. > > [1] > https://github.com/intel/afxdp-plugins-for-kubernetes/blob/main/constants/constants.go#L84 > > [2] > https://github.com/intel/afxdp-plugins-for-kubernetes/blob/main/internal/deviceplugin/poolManager_test.go#L99 > > I had added 2 small comment earlier in the code. > > Regards, > > Shibin > > *From:*Maryam Tahhan > *Sent:* Monday, December 4, 2023 6:41 PM > *To:* Koikkara Reeny, Shibin ; > ferruh.yigit@amd.com; stephen@networkplumber.org; > lihuisong@huawei.com; fengchengwen@huawei.com; liuyonglong@huawei.com > *Cc:* dev@dpdk.org > *Subject:* Re: [v2] net/af_xdp: enable a sock path alongside use_cni > > Hi Shibin > > I'm not really sure what you are suggesting, is to make an assumption > on the path part where the socket resides (aka hard code it) and then > try to build the full UDS path in DPDK? > > Yes the plugin is using constants ATM for certain parts of the UDS > path, but that's not say that it's something that won't become > configurable later on. Someone may not want to use "/tmp/afxdp_dp/" as > the base directory. Then we'd have to change DPDK's implementation > again. These are not really things that are configured by hand and are > generated by initialization scripts (typically). I would rather build > this with the idea that things can change in the future without having > to change the DPDK implementation again. > > BR > Maryam > > On 04/12/2023 17:18, Koikkara Reeny, Shibin wrote: > > Hi Maryam, > > Apologies for asking this question bit late. > > The UDS sock name will be afxdp.sock only and addition director is created between the sock name and the uds filepath (/tmp/afxdp_dp//afxdp.sock). > > As per the command " --vdev net_af_xdp0,iface=,use_cni=1,uds_path=/tmp/afxdp_dp//afxdp.sock" > > We are already passing the interface name(iface= . So can't we create the uds_path inside the program uds_path="/tmp/afxdp_dp/"+ iface + "afxdp.sock" > > If you check the code afxdp-plugins-for-kubernetes constants.go [1] they still have the constants and also they are using these constants to create the path [2] > > [1]https://github.com/intel/afxdp-plugins-for-kubernetes/blob/main/constants/constants.go#L84 > > [2]https://github.com/intel/afxdp-plugins-for-kubernetes/blob/main/internal/deviceplugin/poolManager_test.go#L78 > > If we are able to create path in the program then user won't have to pass along argument value. > > Regards, > > Shibin > > -----Original Message----- > > From: Maryam Tahhan > > Sent: Monday, December 4, 2023 10:31 AM > > To:ferruh.yigit@amd.com;stephen@networkplumber.org; > > lihuisong@huawei.com;fengchengwen@huawei.com; > > liuyonglong@huawei.com; Koikkara Reeny, Shibin > > > > Cc:dev@dpdk.org; Tahhan, Maryam > > Subject: [v2] net/af_xdp: enable a sock path alongside use_cni > > With the original 'use_cni' implementation, (using a hardcoded socket rather > > than a configurable one), if a single 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 alongside > > the 'use_cni' param. > > Tested with the AF_XDP DP CNI PR 81. > > 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 > > --- > > doc/guides/howto/af_xdp_cni.rst     | 24 ++++++----- > > drivers/net/af_xdp/rte_eth_af_xdp.c | 62 ++++++++++++++++++----------- > > 2 files changed, 54 insertions(+), 32 deletions(-) > > diff --git a/doc/guides/howto/af_xdp_cni.rst > > b/doc/guides/howto/af_xdp_cni.rst index a1a6d5b99c..7829526b40 100644 > > --- a/doc/guides/howto/af_xdp_cni.rst > > +++ b/doc/guides/howto/af_xdp_cni.rst > > @@ -38,9 +38,10 @@ The XSKMAP is a BPF map of AF_XDP sockets (XSK). > > The client can then proceed with creating an AF_XDP socket  and inserting > > that socket into the XSKMAP pointed to by the descriptor. > > -The EAL vdev argument ``use_cni`` is used to indicate that the user wishes - > > to run the PMD in unprivileged mode and to receive the XSKMAP file > > descriptor -from the CNI. > > +The EAL vdev arguments ``use_cni`` and ``uds_path`` are used to > > +indicate that the user wishes to run the PMD in unprivileged mode and > > +to receive the XSKMAP file descriptor from the CNI. > > + > > When this flag is set, > > the ``XSK_LIBBPF_FLAGS__INHIBIT_PROG_LOAD`` libbpf flag  should be > > used when creating the socket @@ -49,7 +50,7 @@ Instead the loading is > > handled by the CNI. > > .. note:: > > -   The Unix Domain Socket file path appear in the end user is > > "/tmp/afxdp.sock". > > +   The Unix Domain Socket file path appears to the end user at > > "/tmp/afxdp_dp//afxdp.sock". > > Prerequisites > > @@ -223,8 +224,7 @@ Howto run dpdk-testpmd with CNI plugin: > >           securityContext: > >            capabilities: > >               add: > > -               - CAP_NET_RAW > > -               - CAP_BPF > > +               - NET_RAW > > Need to update the 1.3. Prerequisites. > >           resources: > >             requests: > >               hugepages-2Mi: 2Gi > > @@ -239,14 +239,20 @@ Howto run dpdk-testpmd with CNI plugin: > >    .. _pod.yaml:https://github.com/intel/afxdp-plugins-for- > > kubernetes/blob/v0.0.2/test/e2e/pod-1c1d.yaml > > +.. note:: > > + > > +   For Kernel versions older than 5.19 `CAP_BPF` is also required in > > +   the container capabilities stanza. > > + > > * Run DPDK with a command like the following: > >    .. code-block:: console > >       kubectl exec -i --container -- \ > > -           //dpdk-testpmd -l 0,1 --no-pci \ > > -           --vdev=net_af_xdp0,use_cni=1,iface= \ > > -           -- --no-mlockall --in-memory > > +           //dpdk-testpmd -l 0-2 --no-pci --main-lcore=2 \ > > +           --vdev net_af_xdp0,iface= > name>,use_cni=1,uds_path=/tmp/afxdp_dp//afxdp.sock > > \ > > +           --vdev net_af_xdp1,iface=e > name>,use_cni=1,uds_path=/tmp/afxdp_dp//afxdp.sock > > There is a typo " iface=e > \ > > +           -- -i --a --nb-cores=2 --rxq=1 --txq=1 > > + --forward-mode=macswap; > > For further reference please use the `e2e`_ test case in `AF_XDP Plugin for > > Kubernetes`_ > > diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c > > b/drivers/net/af_xdp/rte_eth_af_xdp.c > > index 353c8688ec..505ed6cf1e 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" > > @@ -171,6 +170,7 @@ struct pmd_internals { > > 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; > > @@ -191,6 +191,7 @@ struct pmd_process_private { > > #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_CNI_UDS_PATH_ARG "uds_path" > > static const char * const valid_arguments[] = { > > ETH_AF_XDP_IFACE_ARG, > > @@ -201,6 +202,7 @@ static const char * const valid_arguments[] = { > > ETH_AF_XDP_BUDGET_ARG, > > ETH_AF_XDP_FORCE_COPY_ARG, > > ETH_AF_XDP_USE_CNI_ARG, > > + ETH_AF_XDP_USE_CNI_UDS_PATH_ARG, > > NULL > > }; > > @@ -1351,7 +1353,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 +1364,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 +1384,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 +1395,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)); > > @@ -1471,7 +1473,7 @@ 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) > > +         int *req_fd, char *response, int *out_fd, const char > > *uds_path) > > { > > int rval; > > @@ -1483,7 +1485,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 +1509,7 @@ check_response(char *response, char *exp_resp, long > > size)  } > > static int > > -get_cni_fd(char *if_name) > > +get_cni_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 +1522,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 */ > > 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_cni(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 +1543,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_cni(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 +1551,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_cni(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 +1573,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_cni(sock, &server, request, NULL, response, > > &out_fd, > > +uds_path) < 0) { > >          AF_XDP_LOG(ERR, "Error in processing cmd [%s]\n", > > request); > >          goto err_close; > > } > > @@ -1695,17 +1697,16 @@ xsk_configure(struct pmd_internals *internals, > > struct pkt_rx_queue *rxq, > > } > > if (internals->use_cni) { > > -         int err, fd, map_fd; > > +         int err, map_fd; > >          /* get socket fd from CNI plugin */ > > -         map_fd = get_cni_fd(internals->if_name); > > +         map_fd = get_cni_fd(internals->if_name, internals- > > uds_path); > >          if (map_fd < 0) { > >                  AF_XDP_LOG(ERR, "Failed to receive CNI 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; > > @@ -2023,7 +2024,8 @@ 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, int *use_cni, > > +         char *uds_path) > > { > > int ret; > > @@ -2069,6 +2071,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_USE_CNI_UDS_PATH_ARG, > > +                        &parse_prog_arg, uds_path); > > + if (ret < 0) > > +         goto free_kvlist; > > + > > free_kvlist: > > rte_kvargs_free(kvlist); > > return ret; > > @@ -2108,7 +2115,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) > > +            int use_cni, const char *uds_path) > > { > > const char *name = rte_vdev_device_name(dev); > > const unsigned int numa_node = dev->device.numa_node; @@ - > > 2138,6 +2145,7 @@ init_internals(struct rte_vdev_device *dev, const char > > *if_name, > > 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)) { @@ - > > 2328,6 +2336,7 @@ rte_pmd_af_xdp_probe(struct rte_vdev_device *dev) > > 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,7 +2379,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, &force_copy, &use_cni) < 0) { > > +                        &busy_budget, &force_copy, &use_cni, > > uds_path) < 0) { > >          AF_XDP_LOG(ERR, "Invalid kvargs value\n"); > >          return -EINVAL; > > } > > @@ -2387,6 +2396,12 @@ rte_pmd_af_xdp_probe(struct rte_vdev_device > > *dev) > >                  return -EINVAL; > > } > > + if (use_cni && !strnlen(uds_path, PATH_MAX)) { > > +         AF_XDP_LOG(ERR, "When '%s' parameter is used, '%s' must > > also be provided\n", > > +                 ETH_AF_XDP_USE_CNI_ARG, > > ETH_AF_XDP_USE_CNI_UDS_PATH_ARG); > > +                 return -EINVAL; > > + } > > + > > if (strlen(if_name) == 0) { > >          AF_XDP_LOG(ERR, "Network interface must be > > specified\n"); > >          return -EINVAL; > > @@ -2410,7 +2425,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, use_cni, > > uds_path); > > if (eth_dev == NULL) { > >          AF_XDP_LOG(ERR, "Failed to init internals\n"); > >          return -1; > > @@ -2471,4 +2486,5 @@ > > RTE_PMD_REGISTER_PARAM_STRING(net_af_xdp, > >                        "xdp_prog= " > >                        "busy_budget= " > >                        "force_copy= " > > -                       "use_cni= "); > > +                       "use_cni= " > > +                       "uds_path= "); > > -- > > 2.41.0 >