Re: [dpdk-dev] [PATCH v4 1/5] ethdev: negotiate delivery of packet metadata from HW to PMD

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

On 10/5/21 3:50 PM, Ivan Malov wrote:
> Hi Ori,
> 
> On 05/10/2021 15:03, Ori Kam wrote:
>> Hi Ivan,
>>
>> Just a nit below.
>>
>>> -----Original Message-----
>>> From: Ivan Malov <ivan.malov@oktetlabs.ru>
>>> Sent: Tuesday, October 5, 2021 2:50 AM
>>> Subject: [PATCH v4 1/5] ethdev: negotiate delivery of packet metadata
>>> from
>>> HW to PMD
>>>
>>> Provide an API to let the application control the NIC's ability to
>>> deliver specific
>>> kinds of per-packet metadata to the PMD.
>>>
>>> Checks for the NIC's ability to set these kinds of metadata in the
>>> first place
>>> (support for the flow actions) belong in flow API responsibility
>>> domain (flow
>>> validate mechanism).
>>> This topic is out of scope of the new API in question.
>>>
>>> The PMD's ability to deliver received metadata to the user by virtue
>>> of mbuf
>>> fields should be covered by mbuf library.
>>> It is also out of scope of the new API in question.
>>>
>>
>> +1 very clear.
>>
>>> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
>>> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
>>> Acked-by: Ray Kinsella <mdr@ashroe.eu>
>>> Acked-by: Jerin Jacob <jerinj@marvell.com>
>>> ---
>>
>> [Snip]
>>
>>> --- a/lib/ethdev/rte_ethdev.h
>>> +++ b/lib/ethdev/rte_ethdev.h
>>> @@ -4902,6 +4902,59 @@ __rte_experimental  int
>>> rte_eth_representor_info_get(uint16_t port_id,
>>>                    struct rte_eth_representor_info *info);
>>>
>>> +/** The NIC is able to deliver flag (if set) with packets to the PMD.
>>> +*/ #define RTE_ETH_RX_METADATA_USER_FLAG (UINT64_C(1) << 0)
>>> +
>>> +/** The NIC is able to deliver mark ID with packets to the PMD. */
>>> +#define RTE_ETH_RX_METADATA_USER_MARK (UINT64_C(1) << 1)
>>> +
>>> +/** The NIC is able to deliver tunnel ID with packets to the PMD. */
>>> +#define RTE_ETH_RX_METADATA_TUNNEL_ID (UINT64_C(1) << 2)
>>> +
>>> +/**
>>> + * @warning
>>> + * @b EXPERIMENTAL: this API may change without prior notice
>>> + *
>>> + * Negotiate the NIC's ability to deliver specific kinds of metadata
>>> to the PMD.
>>> + *
>>> + * Invoke this API before the first rte_eth_dev_configure() invocation
>>> + * to let the PMD make preparations that are inconvenient to do later.
>>> + *
>>> + * The negotiation process is as follows:
>>> + *
>>> + * - the application requests features intending to use at least some
>>> +of them;
>>> + * - the PMD responds with the guaranteed subset of the requested
>>> +feature set;
>>> + * - the application can retry negotiation with another set of
>>> +features;
>>> + * - the application can pass zero to clear the negotiation result;
>>> + * - the last negotiated result takes effect upon the ethdev start.
>>
>> Not upon ethdev configure?
> 
> Well, technically, doing "configure()" just closes the negotiation
> window. I guess, "to take effect" is "to be activated", and activation
> of Rx features typically happens on Rx subsystem start.

Yes, i.e. ethdev port start from application point of view

> I know it might seem a bit inconsistent, but in any case the API
> contract says clearly that invocations of "metadata_negotiate()" should
> be done before "configure()".
> 
> Andrew?

Yes, the reason to define order is to simplify implementation.
When configure is invoked, PMD know that Rx metadata are
negotiated and it should treat all other bits of the
configuration with respect to Rx metadata configuration,
of course, if applicable.

So, I think the question is right and correct description
should say: ... upon the ethdev configure and start.

Andrew.