[v11 2/3] net/af_xdp: fix multi interface support for K8s

[Loftus, Ciara]

snip

> @@ -1695,17 +1699,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);
> +             /* get socket fd from AF_XDP Device Plugin */
> +             map_fd = uds_get_xskmap_fd(internals->if_name, internals-
> >dp_path);
>               if (map_fd < 0) {
> -                     AF_XDP_LOG(ERR, "Failed to receive CNI plugin fd\n");
> +                     AF_XDP_LOG(ERR, "Failed to receive xskmap fd from
> AF_XDP Device Plugin\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);

Hi Maryam,

I've reviewed the series again. I haven't tested the device-plugin specific functionality as I don't have that environment set up, but outside of that I am happy that the new functionality doesn't break anything else. The doc updates look good to me now, thank you for the fixes.

I have just spotted one issue and I apologise for only catching it now.
Patch 2 introduces a dependency on the xsk_socket__update_xskmap function which is available in:
libbpf >= v0.3.0 and <= v0.6.0
libxdp > v1.2.0

The af_xdp.rst guide states we are compatible with libbpf (on it's own) <= v0.6.0. So users using libbpf < v0.3.0 will get an undefined reference warning for the xsk_socket__update_xskmap function.

Is it possible to implement fallback functionality (or if that's not possible, bail out) if that function is not available? See how this is done for the xsk_socket__create_shared function in meson.build and compat.h.

Thanks,
Ciara

>               if (err) {
>                       AF_XDP_LOG(ERR, "Failed to insert unprivileged xsk in
> map.\n");
>                       goto out_xsk;
> @@ -1881,13 +1884,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.
> +/* AF_XDP Device Plugin option works in unprivileged
> + * container environments and ethernet device functionality
> + * will be reduced. So additional customised eth_dev_ops
> + * struct is needed for the Device Plugin. Promiscuous
> + * enable and disable functionality is removed.
>   **/
> -static const struct eth_dev_ops ops_cni = {
> +static const struct eth_dev_ops ops_afxdp_dp = {
>       .dev_start = eth_dev_start,
>       .dev_stop = eth_dev_stop,
>       .dev_close = eth_dev_close,
> @@ -2023,7 +2026,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 *dp_path)
>  {
>       int ret;
>
> @@ -2069,6 +2073,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_DP_PATH_ARG,
> +                              &parse_prog_arg, dp_path);
> +     if (ret < 0)
> +             goto free_kvlist;
> +
>  free_kvlist:
>       rte_kvargs_free(kvlist);
>       return ret;
> @@ -2108,7 +2117,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 *dp_path)
>  {
>       const char *name = rte_vdev_device_name(dev);
>       const unsigned int numa_node = dev->device.numa_node;
> @@ -2138,6 +2147,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->dp_path, dp_path, PATH_MAX);
>
>       if (xdp_get_channels_info(if_name, &internals->max_queue_cnt,
>                                 &internals->combined_queue_cnt)) {
> @@ -2199,7 +2209,7 @@ init_internals(struct rte_vdev_device *dev, const
> char *if_name,
>       if (!internals->use_cni)
>               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;
> @@ -2328,6 +2338,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 dp_path[PATH_MAX] = {'\0'};
>       struct rte_eth_dev *eth_dev = NULL;
>       const char *name = rte_vdev_device_name(dev);
>
> @@ -2370,7 +2381,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, dp_path) <
> 0) {
>               AF_XDP_LOG(ERR, "Invalid kvargs value\n");
>               return -EINVAL;
>       }
> @@ -2384,7 +2395,19 @@ rte_pmd_af_xdp_probe(struct rte_vdev_device
> *dev)
>       if (use_cni && 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);
> -                     return -EINVAL;
> +             return -EINVAL;
> +     }
> +
> +     if (use_cni && !strnlen(dp_path, PATH_MAX)) {
> +             snprintf(dp_path, sizeof(dp_path), "%s/%s/%s",
> DP_BASE_PATH, if_name, DP_UDS_SOCK);
> +             AF_XDP_LOG(INFO, "'%s' parameter not provided, setting
> value to '%s'\n",
> +                     ETH_AF_XDP_DP_PATH_ARG, dp_path);
> +     }
> +
> +     if (!use_cni && strnlen(dp_path, PATH_MAX)) {
> +             AF_XDP_LOG(ERR, "'%s' parameter is set, but '%s' was not
> enabled\n",
> +                     ETH_AF_XDP_DP_PATH_ARG,
> ETH_AF_XDP_USE_CNI_ARG);
> +             return -EINVAL;
>       }
>
>       if (strlen(if_name) == 0) {
> @@ -2410,7 +2433,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, dp_path);
>       if (eth_dev == NULL) {
>               AF_XDP_LOG(ERR, "Failed to init internals\n");
>               return -1;
> @@ -2471,4 +2494,5 @@ RTE_PMD_REGISTER_PARAM_STRING(net_af_xdp,
>                             "xdp_prog=<string> "
>                             "busy_budget=<int> "
>                             "force_copy=<int> "
> -                           "use_cni=<int> ");
> +                           "use_cni=<int> "
> +                           "dp_path=<string> ");
> --
> 2.41.0