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 87046A0555; Thu, 9 Jun 2022 14:19:00 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 74B2C4281A; Thu, 9 Jun 2022 14:19:00 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id EFE1640220 for ; Thu, 9 Jun 2022 14:18:58 +0200 (CEST) Received: from [192.168.1.40] (unknown [188.170.81.145]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id 81F562D5; Thu, 9 Jun 2022 15:18:57 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 81F562D5 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1654777138; bh=gQl5yIgLy05VtYGxGyT4iz9PPts5PPForZgwUYhcGRM=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=s2W8VavnDi5qgZh46xVqlrkExw6F9xfWpcgar2c2Tw1TjcOWPJpqUmvn7A68tdM0Y i8840zbPLB9XESEzu5NI3tbQg+b7cx7A+HmGY4W4Zljw/oEQE0pFQfZP6pEdKxGjSc 8phxxhqvrXpowxqrWXTivMrBYEVq0bUQtmZkbtjU= Message-ID: <0298a215-0f4a-45c7-3998-3b572754499a@oktetlabs.ru> Date: Thu, 9 Jun 2022 15:18:56 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 Subject: Re: [PATCH v3 3/7] ethdev: get meter profile/policy objects Content-Language: en-US To: Alexander Kozyrev , dev@dpdk.org Cc: cristian.dumitrescu@intel.com, jerinjacobk@gmail.com, orika@nvidia.com, thomas@monjalon.net, ivan.malov@oktetlabs.ru, ferruh.yigit@xilinx.com, mohammad.abdul.awal@intel.com, qi.z.zhang@intel.com, jerinj@marvell.com, ajit.khaparde@broadcom.com, bruce.richardson@intel.com References: <20220522105102.1692526-1-akozyrev@nvidia.com> <20220601034408.2579943-1-akozyrev@nvidia.com> <20220601034408.2579943-4-akozyrev@nvidia.com> From: Andrew Rybchenko In-Reply-To: <20220601034408.2579943-4-akozyrev@nvidia.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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 6/1/22 06:44, Alexander Kozyrev wrote: > Introduce a new Meter API to retrieve a Meter profile and policy > objects using the profile/policy ID previously created with > meter_profile_add() and meter_policy_create() functions. > That allows to save the pointer and avoid any lookups in the > corresponding lists for quick access during a flow rule creation. > Also, it eliminates the need for CIR, CBS and EBS calculations > and conversion to a PMD-specific format when the profile is used. > Pointers are destroyed and cannot be used after thecorresponding missing space after "the" above > meter_profile_delete() or meter_policy_delete() are called. > > Signed-off-by: Alexander Kozyrev > --- > .../traffic_metering_and_policing.rst | 7 ++++ > doc/guides/rel_notes/release_22_07.rst | 1 + > lib/ethdev/rte_flow.h | 7 ++++ > lib/ethdev/rte_mtr.c | 41 +++++++++++++++++++ > lib/ethdev/rte_mtr.h | 40 ++++++++++++++++++ > lib/ethdev/rte_mtr_driver.h | 19 +++++++++ > lib/ethdev/version.map | 2 + > 7 files changed, 117 insertions(+) > > diff --git a/doc/guides/prog_guide/traffic_metering_and_policing.rst b/doc/guides/prog_guide/traffic_metering_and_policing.rst > index d1958a023d..2ce3236ad8 100644 > --- a/doc/guides/prog_guide/traffic_metering_and_policing.rst > +++ b/doc/guides/prog_guide/traffic_metering_and_policing.rst > @@ -107,6 +107,13 @@ traffic meter and policing library. > to the list of meter actions (``struct rte_mtr_meter_policy_params::actions``) > specified per color as show in :numref:`figure_rte_mtr_chaining`. > > +#. The ``rte_mtr_meter_profile_get()`` and ``rte_mtr_meter_policy_get()`` > + API functions are available for getting the object pointers directly. > + These pointers allow quick access to profile/policy objects and are > + required by the ``RTE_FLOW_ACTION_TYPE_METER_MARK`` action. > + This action may omit the policy definition to providei flexibility > + to match a color later with the ``RTE_FLOW_ITEM_TYPE_METER_COLOR`` item. > + > Protocol based input color selection > ------------------------------------ > > diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst > index 451ff8d460..6d030bead5 100644 > --- a/doc/guides/rel_notes/release_22_07.rst > +++ b/doc/guides/rel_notes/release_22_07.rst > @@ -73,6 +73,7 @@ New Features > > * Added METER_COLOR item to match Color Marker set by a Meter. > * Added ability to set Color Marker via modify_field Flow API. > + * Added Meter API to get a pointer to profile/policy by their ID. > > * **Updated Intel iavf driver.** > > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h > index 68af109554..9754f6630a 100644 > --- a/lib/ethdev/rte_flow.h > +++ b/lib/ethdev/rte_flow.h > @@ -3827,6 +3827,13 @@ struct rte_flow_action { > */ > struct rte_flow; > > +/** > + * Opaque type for Meter profile object returned by MTR API. > + * > + * This handle can be used to create Meter actions instead of profile ID. > + */ > +struct rte_flow_meter_profile; > + > /** > * @warning > * @b EXPERIMENTAL: this structure may change without prior notice > diff --git a/lib/ethdev/rte_mtr.c b/lib/ethdev/rte_mtr.c > index 441ea1dca9..90fd277040 100644 > --- a/lib/ethdev/rte_mtr.c > +++ b/lib/ethdev/rte_mtr.c > @@ -57,6 +57,25 @@ rte_mtr_ops_get(uint16_t port_id, struct rte_mtr_error *error) > ops->func; \ > }) > > +#define RTE_MTR_HNDL_FUNC(port_id, func) \ > +({ \ > + const struct rte_mtr_ops *ops = \ > + rte_mtr_ops_get(port_id, error); \ > + if (ops == NULL) \ > + return NULL; \ > + \ > + if (ops->func == NULL) { \ > + rte_mtr_error_set(error, \ > + ENOSYS, \ > + RTE_MTR_ERROR_TYPE_UNSPECIFIED, \ > + NULL, \ > + rte_strerror(ENOSYS)); \ > + return NULL; \ > + } \ > + \ > + ops->func; \ > +}) Personally I strongly dislike such macros. It is too tricky to realize what it does taking into account how it is used below. IMHO, code duplication is less evil. > + > /* MTR capabilities get */ > int > rte_mtr_capabilities_get(uint16_t port_id, > @@ -91,6 +110,17 @@ rte_mtr_meter_profile_delete(uint16_t port_id, > meter_profile_id, error); > } > > +/** MTR meter profile get */ > +struct rte_flow_meter_profile * > +rte_mtr_meter_profile_get(uint16_t port_id, > + uint32_t meter_profile_id, > + struct rte_mtr_error *error) > +{ > + struct rte_eth_dev *dev = &rte_eth_devices[port_id]; It is dangerous to get dev pointer without port_id range checking first. I realize that it is checked later, but my point still stands. > + return RTE_MTR_HNDL_FUNC(port_id, meter_profile_get)(dev, > + meter_profile_id, error); > +} > + > /* MTR meter policy validate */ > int > rte_mtr_meter_policy_validate(uint16_t port_id, > @@ -125,6 +155,17 @@ rte_mtr_meter_policy_delete(uint16_t port_id, > policy_id, error); > } > > +/** MTR meter policy get */ > +struct rte_flow_meter_policy * > +rte_mtr_meter_policy_get(uint16_t port_id, > + uint32_t policy_id, > + struct rte_mtr_error *error) > +{ > + struct rte_eth_dev *dev = &rte_eth_devices[port_id]; > + return RTE_MTR_HNDL_FUNC(port_id, meter_policy_get)(dev, > + policy_id, error); > +} > + > /** MTR object create */ > int > rte_mtr_create(uint16_t port_id, > diff --git a/lib/ethdev/rte_mtr.h b/lib/ethdev/rte_mtr.h > index 008bc84f0d..58f0d26215 100644 > --- a/lib/ethdev/rte_mtr.h > +++ b/lib/ethdev/rte_mtr.h > @@ -623,6 +623,26 @@ rte_mtr_meter_profile_delete(uint16_t port_id, > uint32_t meter_profile_id, > struct rte_mtr_error *error); > > +/** Experimental note missing > + * Meter profile object get > + * > + * Get meter profile object for a given meter profile ID. > + * > + * @param[in] port_id > + * The port identifier of the Ethernet device. > + * @param[in] meter_profile_id > + * Meter profile ID. Needs to be the valid. > + * @param[out] error > + * Error details. Filled in only on error, when not NULL. > + * @return > + * A valid handle in case of success, NULL otherwise. > + */ > +__rte_experimental > +struct rte_flow_meter_profile * > +rte_mtr_meter_profile_get(uint16_t port_id, > + uint32_t meter_profile_id, > + struct rte_mtr_error *error); > + > /** > * Check whether a meter policy can be created on a given port. > * > @@ -679,6 +699,26 @@ rte_mtr_meter_policy_add(uint16_t port_id, > struct rte_mtr_meter_policy_params *policy, > struct rte_mtr_error *error); > > +/** Experimental note missing > + * Meter policy object get > + * > + * Get meter policy object for a given meter policy ID. > + * > + * @param[in] port_id > + * The port identifier of the Ethernet device. > + * @param[in] policy_id > + * Meter policy ID. Needs to be the valid. > + * @param[out] error > + * Error details. Filled in only on error, when not NULL. > + * @return > + * A valid handle in case of success, NULL otherwise. > + */ > +__rte_experimental > +struct rte_flow_meter_policy * > +rte_mtr_meter_policy_get(uint16_t port_id, > + uint32_t policy_id, > + struct rte_mtr_error *error); > + > /** > * Define meter policy action list: > * GREEN - GREEN, YELLOW - YELLOW, RED - RED > diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map > index daca7851f2..2609f2e709 100644 > --- a/lib/ethdev/version.map > +++ b/lib/ethdev/version.map > @@ -285,6 +285,8 @@ EXPERIMENTAL { > rte_mtr_color_in_protocol_priority_get; > rte_mtr_color_in_protocol_set; > rte_mtr_meter_vlan_table_update; > + rte_mtr_meter_profile_get; > + rte_mtr_meter_policy_get; if I'm not mistake it should be alphabetically sorted in the group. > }; > > INTERNAL {