DPDK patches and discussions
 help / color / mirror / Atom feed
From: Khadem Ullah <14pwcse1224@uetpeshawar.edu.pk>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: dev@dpdk.org, Thomas Monjalon <thomas@monjalon.net>,
	Ferruh Yigit <ferruh.yigit@amd.com>,
	 Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
	Anatoly Burakov <anatoly.burakov@intel.com>
Subject: Re: [RFC v2 01/12] ethdev: allow start/stop from secondary process
Date: Wed, 9 Jul 2025 22:47:05 +0500	[thread overview]
Message-ID: <CA++2-x43vKdvZ0KnJtzAPtYdOcP9JsMQYc1QJVmBC3fFjnQ=TQ@mail.gmail.com> (raw)
In-Reply-To: <20250709173611.6390-2-stephen@networkplumber.org>

[-- Attachment #1: Type: text/plain, Size: 12530 bytes --]

Does that means that the RSS of the secondary application can be also
applied to the primary application, or in case of multiple instances on the
same interface ?

Best,
Khadem

On Wed, Jul 9, 2025, 22:36 Stephen Hemminger <stephen@networkplumber.org>
wrote:

> Before this patch if secondary process called start/stop
> it would only impact the secondary process, the ethdev on the
> primary process was not started.
>
> With this patch, when start/stop is called from secondary,
> it calls the primary and does the operation there. The design
> is generic, and we can later add queue and other operations
> as needed.
>
> Bugzilla ID: 73
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/ethdev/ethdev_driver.c  | 11 ++++-
>  lib/ethdev/ethdev_private.c | 93 +++++++++++++++++++++++++++++++++++++
>  lib/ethdev/ethdev_private.h | 26 +++++++++++
>  lib/ethdev/rte_ethdev.c     | 84 +++++++++++++++++++--------------
>  4 files changed, 177 insertions(+), 37 deletions(-)
>
> diff --git a/lib/ethdev/ethdev_driver.c b/lib/ethdev/ethdev_driver.c
> index ec0c1e1176..66ba7dafc9 100644
> --- a/lib/ethdev/ethdev_driver.c
> +++ b/lib/ethdev/ethdev_driver.c
> @@ -114,6 +114,14 @@ rte_eth_dev_allocate(const char *name)
>                 goto unlock;
>         }
>
> +       if (eth_dev_shared_data->allocated_ports == 0 &&
> +           rte_mp_action_register(ETHDEV_MP, ethdev_server) &&
> +           rte_errno != ENOTSUP) {
> +               RTE_ETHDEV_LOG_LINE(ERR,
> +                        "Could not start %s service", ETHDEV_MP);
> +               goto unlock;
> +       }
> +
>         eth_dev = eth_dev_get(port_id);
>         eth_dev->flow_fp_ops = &rte_flow_fp_default_ops;
>         strlcpy(eth_dev->data->name, name, sizeof(eth_dev->data->name));
> @@ -282,7 +290,8 @@ rte_eth_dev_release_port(struct rte_eth_dev *eth_dev)
>                 memset(eth_dev->data, 0, sizeof(struct rte_eth_dev_data));
>                 eth_dev->data = NULL;
>
> -               eth_dev_shared_data->allocated_ports--;
> +               if (--eth_dev_shared_data->allocated_ports == 0)
> +                       rte_mp_action_unregister(ETHDEV_MP);
>                 eth_dev_shared_data_release();
>         }
>
> diff --git a/lib/ethdev/ethdev_private.c b/lib/ethdev/ethdev_private.c
> index 285d377d91..184cf33f99 100644
> --- a/lib/ethdev/ethdev_private.c
> +++ b/lib/ethdev/ethdev_private.c
> @@ -2,6 +2,8 @@
>   * Copyright(c) 2018 Gaëtan Rivet
>   */
>
> +#include <assert.h>
> +
>  #include <eal_export.h>
>  #include <rte_debug.h>
>
> @@ -477,3 +479,94 @@ eth_dev_tx_queue_config(struct rte_eth_dev *dev,
> uint16_t nb_queues)
>         dev->data->nb_tx_queues = nb_queues;
>         return 0;
>  }
> +
> +static int
> +ethdev_handle_request(const struct ethdev_mp_request *req)
> +{
> +       switch (req->operation) {
> +       case ETH_REQ_START:
> +               return rte_eth_dev_start(req->port_id);
> +
> +       case ETH_REQ_STOP:
> +               return rte_eth_dev_stop(req->port_id);
> +
> +       default:
> +               return -EINVAL;
> +       }
> +}
> +
> +static_assert(sizeof(struct ethdev_mp_request) <= RTE_MP_MAX_PARAM_LEN,
> +       "ethdev MP request bigger than available param space");
> +
> +static_assert(sizeof(struct ethdev_mp_response) <= RTE_MP_MAX_PARAM_LEN,
> +       "ethdev MP response bigger than available param space");
> +
> +int
> +ethdev_server(const struct rte_mp_msg *mp_msg, const void *peer)
> +{
> +       const struct ethdev_mp_request *req
> +               = (const struct ethdev_mp_request *)mp_msg->param;
> +
> +       struct rte_mp_msg mp_resp = {
> +               .name = ETHDEV_MP,
> +       };
> +       struct ethdev_mp_response *resp;
> +
> +       resp = (struct ethdev_mp_response *)mp_resp.param;
> +       mp_resp.len_param = sizeof(*resp);
> +       resp->res_op = req->operation;
> +
> +       /* recv client requests */
> +       if (mp_msg->len_param != sizeof(*req))
> +               resp->err_value = -EINVAL;
> +       else
> +               resp->err_value = ethdev_handle_request(req);
> +
> +       return rte_mp_reply(&mp_resp, peer);
> +}
> +
> +int
> +ethdev_request(uint16_t port_id, enum ethdev_mp_operation operation,
> +              const void *buf, size_t buf_len)
> +{
> +       struct rte_mp_msg mp_req = { };
> +       struct rte_mp_reply mp_reply;
> +       struct ethdev_mp_request *req;
> +       struct timespec ts = {.tv_sec = 5, .tv_nsec = 0};
> +       int ret;
> +
> +       if (sizeof(*req) + buf_len > RTE_MP_MAX_PARAM_LEN) {
> +               RTE_ETHDEV_LOG_LINE(ERR,
> +                                   "request %u port %u invalid len %zu",
> +                                   operation, port_id, buf_len);
> +               return -EINVAL;
> +       }
> +
> +       strlcpy(mp_req.name, ETHDEV_MP, RTE_MP_MAX_NAME_LEN);
> +       mp_req.len_param = sizeof(*req) + buf_len;
> +
> +       req = (struct ethdev_mp_request *)mp_req.param;
> +       req->operation = operation;
> +       req->port_id = port_id;
> +
> +       if (buf_len > 0)
> +               memcpy(req->config, buf, buf_len);
> +
> +       ret = rte_mp_request_sync(&mp_req, &mp_reply, &ts);
> +       if (ret == 0) {
> +               const struct rte_mp_msg *mp_rep = &mp_reply.msgs[0];
> +               const struct ethdev_mp_response *resp
> +                       = (const struct ethdev_mp_response *)mp_rep->param;
> +
> +               if (resp->err_value == 0)
> +                       ret = 0;
> +               else
> +                       rte_errno = -resp->err_value;
> +               free(mp_reply.msgs);
> +       }
> +
> +       if (ret < 0)
> +               RTE_ETHDEV_LOG_LINE(ERR,
> +                      "port %up ethdev op %u failed", port_id, operation);
> +       return ret;
> +}
> diff --git a/lib/ethdev/ethdev_private.h b/lib/ethdev/ethdev_private.h
> index b07b1b4c42..f58f161871 100644
> --- a/lib/ethdev/ethdev_private.h
> +++ b/lib/ethdev/ethdev_private.h
> @@ -79,4 +79,30 @@ void eth_dev_txq_release(struct rte_eth_dev *dev,
> uint16_t qid);
>  int eth_dev_rx_queue_config(struct rte_eth_dev *dev, uint16_t nb_queues);
>  int eth_dev_tx_queue_config(struct rte_eth_dev *dev, uint16_t nb_queues);
>
> +/* Used to allow start/stop from secondary */
> +#define ETHDEV_MP      "mp_ethdev"
> +
> +enum ethdev_mp_operation {
> +       ETH_REQ_START,
> +       ETH_REQ_STOP,
> +};
> +
> +struct ethdev_mp_request {
> +       uint16_t operation;
> +       uint16_t port_id;
> +       uint32_t reserved;
> +
> +       uint8_t config[]; /* operation specific */
> +};
> +
> +struct ethdev_mp_response {
> +       uint16_t res_op;
> +       uint16_t port_id;
> +       int32_t err_value;
> +};
> +
> +int ethdev_server(const struct rte_mp_msg *mp_msg, const void *peer);
> +int ethdev_request(uint16_t port_id, enum ethdev_mp_operation op,
> +                  const void *buf, size_t buf_len);
> +
>  #endif /* _ETH_PRIVATE_H_ */
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index dd7c00bc94..41363af2c3 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -1800,54 +1800,61 @@ rte_eth_dev_start(uint16_t port_id)
>
>         if (dev->data->dev_configured == 0) {
>                 RTE_ETHDEV_LOG_LINE(INFO,
> -                       "Device with port_id=%"PRIu16" is not configured.",
> -                       port_id);
> +                                   "Device with port_id=%"PRIu16" is not
> configured.",
> +                                   port_id);
>                 return -EINVAL;
>         }
>
>         if (dev->data->dev_started != 0) {
>                 RTE_ETHDEV_LOG_LINE(INFO,
> -                       "Device with port_id=%"PRIu16" already started",
> -                       port_id);
> +                                   "Device with port_id=%"PRIu16" already
> started",
> +                                   port_id);
>                 return 0;
>         }
>
> -       ret = rte_eth_dev_info_get(port_id, &dev_info);
> -       if (ret != 0)
> -               return ret;
> +       if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> +               ret = rte_eth_dev_info_get(port_id, &dev_info);
> +               if (ret != 0)
> +                       return ret;
>
> -       restore_flags = rte_eth_get_restore_flags(dev, RTE_ETH_START);
> +               restore_flags = rte_eth_get_restore_flags(dev,
> RTE_ETH_START);
>
> -       /* Lets restore MAC now if device does not support live change */
> -       if ((*dev_info.dev_flags & RTE_ETH_DEV_NOLIVE_MAC_ADDR) &&
> -           (restore_flags & RTE_ETH_RESTORE_MAC_ADDR))
> -               eth_dev_mac_restore(dev, &dev_info);
> +               /* Restore MAC now if device does not support live change
> */
> +               if ((*dev_info.dev_flags & RTE_ETH_DEV_NOLIVE_MAC_ADDR) &&
> +                   (restore_flags & RTE_ETH_RESTORE_MAC_ADDR))
> +                       eth_dev_mac_restore(dev, &dev_info);
>
> -       diag = dev->dev_ops->dev_start(dev);
> -       if (diag == 0)
> -               dev->data->dev_started = 1;
> -       else
> -               return eth_err(port_id, diag);
> +               diag = dev->dev_ops->dev_start(dev);
> +               if (diag == 0)
> +                       dev->data->dev_started = 1;
> +               else
> +                       return eth_err(port_id, diag);
>
> -       ret = eth_dev_config_restore(dev, &dev_info, restore_flags,
> port_id);
> -       if (ret != 0) {
> -               RTE_ETHDEV_LOG_LINE(ERR,
> -                       "Error during restoring configuration for device
> (port %u): %s",
> -                       port_id, rte_strerror(-ret));
> -               ret_stop = rte_eth_dev_stop(port_id);
> -               if (ret_stop != 0) {
> +               ret = eth_dev_config_restore(dev, &dev_info,
> restore_flags, port_id);
> +               if (ret != 0) {
>                         RTE_ETHDEV_LOG_LINE(ERR,
> -                               "Failed to stop device (port %u): %s",
> -                               port_id, rte_strerror(-ret_stop));
> -               }
> +                               "Error during restoring configuration for
> device (port %u): %s",
> +                                port_id, rte_strerror(-ret));
> +                       ret_stop = rte_eth_dev_stop(port_id);
> +                       if (ret_stop != 0) {
> +                               RTE_ETHDEV_LOG_LINE(ERR,
> +                                       "Failed to stop device (port %u):
> %s",
> +                                        port_id, rte_strerror(-ret_stop));
> +                       }
>
> -               return ret;
> -       }
> +                       return ret;
> +               }
>
> -       if (dev->data->dev_conf.intr_conf.lsc == 0) {
> -               if (dev->dev_ops->link_update == NULL)
> -                       return -ENOTSUP;
> -               dev->dev_ops->link_update(dev, 0);
> +               if (dev->data->dev_conf.intr_conf.lsc == 0) {
> +                       if (dev->dev_ops->link_update == NULL)
> +                               return -ENOTSUP;
> +                       dev->dev_ops->link_update(dev, 0);
> +               }
> +       } else {
> +               /* in secondary, proxy to primary */
> +               ret = ethdev_request(port_id, ETH_REQ_START, NULL, 0);
> +               if (ret != 0)
> +                       return ret;
>         }
>
>         /* expose selection of PMD fast-path functions */
> @@ -1880,9 +1887,14 @@ rte_eth_dev_stop(uint16_t port_id)
>         /* point fast-path functions to dummy ones */
>         eth_dev_fp_ops_reset(rte_eth_fp_ops + port_id);
>
> -       ret = dev->dev_ops->dev_stop(dev);
> -       if (ret == 0)
> -               dev->data->dev_started = 0;
> +       if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> +               ret = dev->dev_ops->dev_stop(dev);
> +               if (ret == 0)
> +                       dev->data->dev_started = 0;
> +       } else {
> +               ret = ethdev_request(port_id, ETH_REQ_STOP, NULL, 0);
> +       }
> +
>         rte_ethdev_trace_stop(port_id, ret);
>
>         return ret;
> --
> 2.47.2
>
>

[-- Attachment #2: Type: text/html, Size: 15564 bytes --]

  reply	other threads:[~2025-07-09 17:47 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <0250411234927.114568-1-stephen@networkplumber.org>
2025-07-09 17:33 ` [RFC v2 00/12] Packet capture using port mirroring Stephen Hemminger
2025-07-09 17:33   ` [RFC v2 01/12] ethdev: allow start/stop from secondary process Stephen Hemminger
2025-07-09 17:47     ` Khadem Ullah [this message]
2025-07-09 17:33   ` [RFC v2 02/12] test: add test for hotplug and secondary process operations Stephen Hemminger
2025-07-09 17:33   ` [RFC v2 03/12] net/ring: allow lockfree transmit if ring supports it Stephen Hemminger
2025-07-09 17:33   ` [RFC v2 04/12] net/ring: add new devargs for dumpcap use Stephen Hemminger
2025-07-09 17:33   ` [RFC v2 05/12] mbuf: add dynamic flag for use by port mirroring Stephen Hemminger
2025-07-09 17:33   ` [RFC v2 06/12] ethdev: add port mirroring feature Stephen Hemminger
2025-07-09 17:33   ` [RFC v2 07/12] pcapng: split packet copy from header insertion Stephen Hemminger
2025-07-09 17:33   ` [RFC v2 08/12] pcapng: make queue optional Stephen Hemminger
2025-07-09 17:33   ` [RFC v2 09/12] test: add tests for ethdev mirror Stephen Hemminger
2025-07-09 17:33   ` [RFC v2 10/12] app/testpmd: support for port mirroring Stephen Hemminger
2025-07-09 17:33   ` [RFC v2 11/12] app/dumpcap: use port mirror instead of pdump Stephen Hemminger
2025-07-09 17:33   ` [RFC v2 12/12] pdump: mark API and application as deprecated Stephen Hemminger

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='CA++2-x43vKdvZ0KnJtzAPtYdOcP9JsMQYc1QJVmBC3fFjnQ=TQ@mail.gmail.com' \
    --to=14pwcse1224@uetpeshawar.edu.pk \
    --cc=anatoly.burakov@intel.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@amd.com \
    --cc=stephen@networkplumber.org \
    --cc=thomas@monjalon.net \
    /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).