Re: [dpdk-dev] [PATCH v3 1/5] ethdev: add API to negotiate delivery of Rx meta data

[Ori Kam <orika@nvidia.com>]

Hi Andrew,

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Tuesday, October 5, 2021 11:39 AM
> Subject: Re: [PATCH v3 1/5] ethdev: add API to negotiate delivery of Rx meta
> data
> 
> Hi Ori,
> 
> On 10/5/21 11:17 AM, Ori Kam wrote:
> > Hi Andrew,
> >
> >> -----Original Message-----
> >> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >> Sent: Tuesday, October 5, 2021 10:27 AM
> >> Subject: Re: [PATCH v3 1/5] ethdev: add API to negotiate delivery of
> >> Rx meta data
> >>
> >> On 10/5/21 9:30 AM, Ori Kam wrote:
> >>> Hi Andrew,
> >>>
> >>>> -----Original Message-----
> >>>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >>>> Sent: Monday, October 4, 2021 4:53 PM
> >>>> Subject: Re: [PATCH v3 1/5] ethdev: add API to negotiate delivery
> >>>> of Rx meta data
> >>>>
> >>>> On 10/4/21 2:39 PM, Ivan Malov wrote:
> >>>>> On 04/10/2021 09:56, Ori Kam wrote:
> >>>>>>> On 04/10/2021 00:04, Ori Kam wrote:
> >>>>>>>> I understand that you are only talking about enabling the
> >>>>>>>> action, meaning to let the PMD know that at some point there
> >>>>>>>> will be a rule that will use the mark action for example.
> >>>>>>>> Is my understanding correct?
> >>>>>>> Not really. The causal relationships are as follows. The
> >>>>>>> application comes to realise that it will need to use, say,
> >>>>>>> action MARK in flows.
> >>>>>>> This, in turn, means that, in order to be able to actually see
> >>>>>>> the mark in received packets, the application needs to ensure
> >>>>>>> that a) the NIC will be able to deliver the mark to the PMD and
> >>>>>>> b) that the PMD will be able to deliver the mark to the
> >>>>>>> application. In particular, in the case of Rx mark,
> >>>>>>> (b) doesn't
> >>>>>>> need to be negotiated = field "mark" is anyway provisioned in
> >>>>>>> the mbuf structure, so no need to enable it. But (a) needs to be
> negotiated.
> >>>>>>> Hence this
> >>>>>>> API.
> >>>>>>>
> >>>>>> Please see my above comment I think we both agree.
> >>>>> Agree to have the 4-th flag in the new API to cover this "custom /
> >>>>> raw metdata" delivery? Personally, I tend to agree, but maybe
> >>>>> Andrew can express his opinion, too.
> >>>> Of course, it could be added, but we're not going to support it in
> >>>> net/sfc. So, I think the flag should be added when a PMD will going
> >>>> to
> >> support it (e.g.
> >>>> net/mlx5).
> >>> I think it should be added now, and more I think that this patch
> >>> should add the missing function to all PMDs 😊
> >>
> >> Sorry, but I disagree. Could you point out to DPDK documentation
> >> where it is written? Should all new API be supported in all PMDs by the API
> contributor?
> >>
> > This changes existing PMD beavior, until now there was no need to
> > register the MARK now you require it, it is just like change the shared counter
> you needed to fix different drivers.
> > This is not critical to me like I said in other thread as long is it
> > is clear that if PMD doesn't support the new function it doesn't mean the the
> PMD has issue with the request.
> 
> I see your point. Hopefully the function description in v4 is clear that it is not
> the case. If callback is not supported by a driver, application should try to use
> all required metadata.
> So, there is no breakage in accordance with defined API contract.
> 

Agree les pretty but works.

> Many thanks for your review notes. The review really makes the API clearer
> and better documented.
> 

Trying to do my best.

> > One more thing, I think this flag should be added now since you need
> > it, I think you should report that you don't support it.
> > since just like we talked there is no real difference between metadata and
> MARK.
> > What do you think?
> 
> It sounds like a trick :) Negative support is *not* a support in fact. DPDK policy
> requires support of a feature in a PMD and in-tree application. Of course, it is
> not a problem to add meta. It is really easy to do. I just don't want to add it in
> v5 to be deleted in v6 because of my above concerns.
> 
This was not a trick. I understand what you are saying.
if we say that metadata is the same as mark, (I think we all agree on it) and that
application need to notify pmd about such operations, I assume it will try to see how to
request the metadata.

I'm O.K. with adding it later and in any case I promise you that if you add it
it will stay.

> @Thomas, what do you think?
> 
> Andrew.

Ori