From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id BC6C342478; Wed, 25 Jan 2023 05:12:57 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5AE5C40223; Wed, 25 Jan 2023 05:12:57 +0100 (CET) Received: from mail-vs1-f47.google.com (mail-vs1-f47.google.com [209.85.217.47]) by mails.dpdk.org (Postfix) with ESMTP id CEDC840150 for ; Wed, 25 Jan 2023 05:12:55 +0100 (CET) Received: by mail-vs1-f47.google.com with SMTP id p1so18624050vsr.5 for ; Tue, 24 Jan 2023 20:12:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=34dCFKBfpt+GncWo/GEnrWaAl0rh3Ujjavc4n2cUtYs=; b=PGa60iadzwC9uNHbhKe1MjthAfrVTqIwzsY6q3hos6LzOjM875xfJRyPrvlml7HpEu 8fyvf/nrWajWFpDl2UavJGX9a34XV3RPNNMpyO4pJtLps3jFTE5edQtMPo4vo6ZaXtCl I2UEym/sMdfIgBWAxvXPNkDz+ECDniwXDUIMyqzNkpSJaSWZsx89T2xoVC16kY0O9Rh8 llBKwuwQZ+djolk2kVP/YPygHmMOzxCIbVyToT2I1w8tzZJwRBjuAR+frj5qKA3W/o++ vCM9eY85j/yWQkTgWm20atAGchjZkk8OnQZDTqqbUQuE/VxC+MNMypgM3sHNxF7XCKSL YYaA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=34dCFKBfpt+GncWo/GEnrWaAl0rh3Ujjavc4n2cUtYs=; b=ldyuaO2nwJA6oEafFhKupouvoajwcP+1Rp4oBL2VNBHyHaVFwEzCew0IrMJoSO7L/f DT1A3uupNdUHieODf8PprQ9GXWup7ylfLWhRZZBFb6a4JMGSfxLAnnkL7Pcq4BDMbXM+ Uh2J6ht0Xn7sOIJVJHGGeZn20vdSOXjrCrz5PtNbAPTH+O++5TjwZZVnl0NzWa1uPVOK CIdV5DNp987NqSNTLUzrxczpMlIUsmswFaR6xjZcuun+UHARfaPAJCgOVDXM35Wujylt AZuwMEDTubCOodxmomon05ij/jRd8Kb2r53p9bbV6jCjr/O1ZGmLj029twq3JZKXjVtY yqxA== X-Gm-Message-State: AFqh2kq5I1yk+OWm/RWJx1xCgvMrC0pjTFGSgd+7qu/YNeAFFJSEEFF0 5HYNxArbrJTfWyAUBXYRCDAKEo2KBSEy2ZGf3MxomktYN+U= X-Google-Smtp-Source: AMrXdXtnX0KR7kvRABTyPXxlPu2T+kj62HIeislDJpUW9ZZTA5x2iwQlL7ds7uAXX8rwQ6wUWw4pniPwrR3G0F5/zjw= X-Received: by 2002:a05:6102:5124:b0:3d3:c7fb:d602 with SMTP id bm36-20020a056102512400b003d3c7fbd602mr4236396vsb.31.1674619975114; Tue, 24 Jan 2023 20:12:55 -0800 (PST) MIME-Version: 1.0 References: <20230107161852.3708690-1-s.v.naga.harish.k@intel.com> <20230123180458.486189-1-s.v.naga.harish.k@intel.com> In-Reply-To: From: Jerin Jacob Date: Wed, 25 Jan 2023 09:42:28 +0530 Message-ID: Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params set/get APIs To: "Naga Harish K, S V" Cc: "jerinj@marvell.com" , "Carrillo, Erik G" , "Gujjar, Abhinandan S" , "dev@dpdk.org" , "Jayatheerthan, Jay" Content-Type: text/plain; charset="UTF-8" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Tue, Jan 24, 2023 at 6:37 PM Naga Harish K, S V wrote: > > Hi Jerin, > > > -----Original Message----- > > From: Jerin Jacob > > Sent: Tuesday, January 24, 2023 10:00 AM > > To: Naga Harish K, S V > > Cc: jerinj@marvell.com; Carrillo, Erik G ; Gujjar, > > Abhinandan S ; dev@dpdk.org; Jayatheerthan, > > Jay > > Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params set/get APIs > > > > On Mon, Jan 23, 2023 at 11:35 PM Naga Harish K S V > > wrote: > > > > > > The adapter configuration parameters defined in the ``struct > > > rte_event_eth_rx_adapter_runtime_params`` can be configured and > > > retrieved using ``rte_event_eth_rx_adapter_runtime_params_set`` and > > > ``rte_event_eth_tx_adapter_runtime_params_get`` respectively. > > > > > > Signed-off-by: Naga Harish K S V > > > > > diff --git a/doc/guides/prog_guide/event_ethernet_rx_adapter.rst > > > b/doc/guides/prog_guide/event_ethernet_rx_adapter.rst > > > index 461eca566f..2207d6ffc3 100644 > > > --- a/doc/guides/prog_guide/event_ethernet_rx_adapter.rst > > > +++ b/doc/guides/prog_guide/event_ethernet_rx_adapter.rst > > > @@ -185,6 +185,26 @@ flags for handling received packets, event queue > > > identifier, scheduler type, event priority, polling frequency of the > > > receive queue and flow identifier in struct > > ``rte_event_eth_rx_adapter_queue_conf``. > > > > > > +Set/Get adapter runtime configuration parameters > > > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > + > > > +The runtime configuration parameters of adapter can be set/read using > > > +``rte_event_eth_rx_adapter_runtime_params_set()`` and > > > +``rte_event_eth_rx_adapter_runtime_params_get()`` respectively. The > > > +parameters that can be set/read are defined in ``struct > > rte_event_eth_rx_adapter_runtime_params``. > > > > Good. > > > > > + > > > +``rte_event_eth_rx_adapter_create()`` or > > > +``rte_event_eth_rx_adapter_create_with_params()`` configures the > > > +adapter with default value for maximum packets processed per request to > > 128. > > > +``rte_event_eth_rx_adapter_runtime_params_set()`` function allows to > > > +reconfigure maximum number of packets processed by adapter per > > > +service request. This is alternative to configuring the maximum > > > +packets processed per request by adapter by using > > > +``rte_event_eth_rx_adapter_create_ext()`` with parameter > > ``rte_event_eth_rx_adapter_conf::max_nb_rx``. > > > > This paragraph is not needed IMO. As it is specific to a driver, and we can keep > > Doxygen comment only. > > > > > > > + > > > +``rte_event_eth_rx_adapter_runtime_parmas_get()`` function retrieves > > > +the configuration parameters that are defined in ``struct > > rte_event_eth_rx_adapter_runtime_params``. > > > > Good. > > > > > + > > > Getting and resetting Adapter queue stats > > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > > > diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c > > > b/lib/eventdev/rte_event_eth_rx_adapter.c > > > index 34aa87379e..d8f3e750b7 100644 > > > --- a/lib/eventdev/rte_event_eth_rx_adapter.c > > > +++ b/lib/eventdev/rte_event_eth_rx_adapter.c > > > @@ -35,6 +35,8 @@ > > > #define MAX_VECTOR_NS 1E9 > > > #define MIN_VECTOR_NS 1E5 > > > > > > +#define RXA_NB_RX_WORK_DEFAULT 128 > > > + > > > #define ETH_RX_ADAPTER_SERVICE_NAME_LEN 32 > > > #define ETH_RX_ADAPTER_MEM_NAME_LEN 32 > > > > > > @@ -1554,7 +1556,7 @@ rxa_default_conf_cb(uint8_t id, uint8_t dev_id, > > > } > > > > > > conf->event_port_id = port_id; > > > - conf->max_nb_rx = 128; > > > + conf->max_nb_rx = RXA_NB_RX_WORK_DEFAULT; > > > if (started) > > > ret = rte_event_dev_start(dev_id); > > > rx_adapter->default_cb_arg = 1; @@ -3436,6 +3438,90 @@ > > > rte_event_eth_rx_adapter_instance_get(uint16_t eth_dev_id, > > > return -EINVAL; > > > } > > > > > + > > > +int > > > +rte_event_eth_rx_adapter_runtime_params_set(uint8_t id, > > > + struct rte_event_eth_rx_adapter_runtime_params > > > +*params) { > > > + struct event_eth_rx_adapter *rxa; > > > + int ret; > > > + > > > + if (params == NULL) > > > + return -EINVAL; > > > + > > > + if (rxa_memzone_lookup()) > > > + return -ENOMEM; > > > > Introduce an adapter callback and move SW adapter related logic under callback > > handler. > > > > > Do you mean introducing eventdev PMD callback for HW implementation? Yes. > There are no adapter callback currently for service based SW Implementation. > > > > + > > > + rxa = rxa_id_to_adapter(id); > > > + if (rxa == NULL) > > > + return -EINVAL; > > > + > > > + ret = rxa_caps_check(rxa); > > > + if (ret) > > > + return ret; > > > + > > > + rte_spinlock_lock(&rxa->rx_lock); > > > + rxa->max_nb_rx = params->max_nb_rx; > > > + rte_spinlock_unlock(&rxa->rx_lock); > > > + > > > + return 0; > > > +} > > > + > > > +int > > > +rte_event_eth_rx_adapter_runtime_params_get(uint8_t id, > > > + struct rte_event_eth_rx_adapter_runtime_params > > > +*params) { > > > + struct event_eth_rx_adapter *rxa; > > > + int ret; > > > + > > > + if (params == NULL) > > > + return -EINVAL; > > > > > > Introduce an adapter callback and move SW adapter related logic under callback > > handler. > > > > > > Same as above > > > > + > > > + if (rxa_memzone_lookup()) > > > + return -ENOMEM; > > + > > > + rxa = rxa_id_to_adapter(id); > > > + if (rxa == NULL) > > > + return -EINVAL; > > > + > > > + ret = rxa_caps_check(rxa); > > > + if (ret) > > > + return ret; > > > + > > > + params->max_nb_rx = rxa->max_nb_rx; > > > + > > > + return 0; > > > +} > > > + > > > +/* RX-adapter telemetry callbacks */ > > > #define RXA_ADD_DICT(stats, s) rte_tel_data_add_dict_u64(d, #s, > > > stats.s) > > > > > > static int > > > diff --git a/lib/eventdev/rte_event_eth_rx_adapter.h > > > b/lib/eventdev/rte_event_eth_rx_adapter.h > > > index f4652f40e8..214ffd018c 100644 > > > --- a/lib/eventdev/rte_event_eth_rx_adapter.h > > > +++ b/lib/eventdev/rte_event_eth_rx_adapter.h > > > @@ -39,10 +39,21 @@ > > > * - rte_event_eth_rx_adapter_queue_stats_reset() > > > * - rte_event_eth_rx_adapter_event_port_get() > > > * - rte_event_eth_rx_adapter_instance_get() > > > + * - rte_event_eth_rx_adapter_runtime_params_get() > > > + * - rte_event_eth_rx_adapter_runtime_params_set() > > > * > > > * The application creates an ethernet to event adapter using > > > * rte_event_eth_rx_adapter_create_ext() or > > rte_event_eth_rx_adapter_create() > > > * or rte_event_eth_rx_adapter_create_with_params() functions. > > > + * > > > + * rte_event_eth_rx_adapter_create() or > > > + rte_event_eth_adapter_create_with_params() > > > + * configures the adapter with default value of maximum packets > > > + processed per > > > + * iteration to RXA_NB_RX_WORK_DEFAULT(128). > > > + * rte_event_eth_rx_adapter_runtime_params_set() allows to > > > + re-configure maximum > > > + * packets processed per iteration. This is alternative to using > > > + * rte_event_eth_rx_adapter_create_ext() with parameter > > > + * rte_event_eth_rx_adapter_conf::max_nb_rx > > > > Move this to Doxygen comment against max_nb_rx > > > > > + * > > > * The adapter needs to know which ethernet rx queues to poll for mbufs as > > well > > > * as event device parameters such as the event queue identifier, event > > > * priority and scheduling type that the adapter should use when > > > constructing @@ -299,6 +310,19 @@ struct > > rte_event_eth_rx_adapter_params { > > > /**< flag to indicate that event buffer is separate for each > > > queue */ }; > > > > > > +/** > > > + * Adapter configuration parameters > > > + */ > > > +struct rte_event_eth_rx_adapter_runtime_params { > > > + uint32_t max_nb_rx; > > > + /**< The adapter can return early if it has processed at least > > > + * max_nb_rx mbufs. This isn't treated as a requirement; batching may > > > + * cause the adapter to process more than max_nb_rx mbufs. > > > > Also tell it is valid only for INTERNAL PORT capablity is set. > > > > Do you mean, it is valid only for INTERNAL PORT capability is 'not' set? Yes. > > > > + */ > > > + uint32_t rsvd[15]; > > > + /**< Reserved fields for future use */ > > > > Introduce rte_event_eth_rx_adapter_runtime_params_init() to make sure rsvd is > > zero. > > > > The reserved fields are not used by the adapter or application. Not sure Is it necessary to Introduce > a new API to clear reserved fields. When adapter starts using new fileds(when we add new fieds in future), the old applicaiton which is not using rte_event_eth_rx_adapter_runtime_params_init() may have junk value and then adapter implementation will behave bad. > > > > +}; > > > + > > > /** > > > * > > > * Callback function invoked by the SW adapter before it continues @@ > > > -377,7 +401,7 @@ int rte_event_eth_rx_adapter_create_ext(uint8_t id, > > uint8_t dev_id, > > > * Create a new ethernet Rx event adapter with the specified identifier. > > > * This function uses an internal configuration function that creates an event > > > * port. This default function reconfigures the event device with an > > > - * additional event port and setups up the event port using the > > > port_config > > > + * additional event port and setup the event port using the > > > + port_config > > > * parameter passed into this function. In case the application needs more > > > * control in configuration of the service, it should use the > > > * rte_event_eth_rx_adapter_create_ext() version. > > > @@ -743,6 +767,50 @@ rte_event_eth_rx_adapter_instance_get(uint16_t > > eth_dev_id, > > > uint16_t rx_queue_id, > > > uint8_t *rxa_inst_id); > > >