From: "Loftus, Ciara" <ciara.loftus@intel.com>
To: "Tahhan, Maryam" <mtahhan@redhat.com>,
"ferruh.yigit@amd.com" <ferruh.yigit@amd.com>,
"stephen@networkplumber.org" <stephen@networkplumber.org>,
"lihuisong@huawei.com" <lihuisong@huawei.com>,
"fengchengwen@huawei.com" <fengchengwen@huawei.com>,
"liuyonglong@huawei.com" <liuyonglong@huawei.com>,
"Marchand, David" <david.marchand@redhat.com>,
"Koikkara Reeny, Shibin" <shibin.koikkara.reeny@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
"Tahhan, Maryam" <mtahhan@redhat.com>,
"stable@dpdk.org" <stable@dpdk.org>
Subject: RE: [v11 2/3] net/af_xdp: fix multi interface support for K8s
Date: Fri, 1 Mar 2024 15:43:42 +0000 [thread overview]
Message-ID: <MW4PR11MB58728DB7098730D0C5D28BDD8E5E2@MW4PR11MB5872.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20240229132129.656166-3-mtahhan@redhat.com>
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
next prev parent reply other threads:[~2024-03-01 15:43 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20240229132129.656166-1-mtahhan@redhat.com>
2024-02-29 13:21 ` [v11 1/3] docs: AF_XDP Device Plugin Maryam Tahhan
2024-02-29 13:21 ` [v11 2/3] net/af_xdp: fix multi interface support for K8s Maryam Tahhan
2024-03-01 15:43 ` Loftus, Ciara [this message]
2024-03-01 17:04 ` Maryam Tahhan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=MW4PR11MB58728DB7098730D0C5D28BDD8E5E2@MW4PR11MB5872.namprd11.prod.outlook.com \
--to=ciara.loftus@intel.com \
--cc=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=fengchengwen@huawei.com \
--cc=ferruh.yigit@amd.com \
--cc=lihuisong@huawei.com \
--cc=liuyonglong@huawei.com \
--cc=mtahhan@redhat.com \
--cc=shibin.koikkara.reeny@intel.com \
--cc=stable@dpdk.org \
--cc=stephen@networkplumber.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).