[v3,1/2] ethdev: introduce transfer attribute to shared action conf
Checks
Commit Message
In a flow rule, attribute "transfer" means operation level
at which both traffic is matched and actions are conducted.
Add the very same attribute to shared action configuration.
If a driver needs to prepare HW resources in two different
ways, depending on the operation level, in order to set up
an action, then this new attribute will indicate the level.
Also, when handling a flow rule insertion, the driver will
be able to turn down a shared action if its level is unfit.
Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
Acked-by: Ori Kam <orika@nvidia.com>
---
lib/librte_ethdev/rte_flow.h | 8 ++++++++
1 file changed, 8 insertions(+)
Comments
On 11/2/20 2:43 PM, Ivan Malov wrote:
> In a flow rule, attribute "transfer" means operation level
> at which both traffic is matched and actions are conducted.
>
> Add the very same attribute to shared action configuration.
> If a driver needs to prepare HW resources in two different
> ways, depending on the operation level, in order to set up
> an action, then this new attribute will indicate the level.
> Also, when handling a flow rule insertion, the driver will
> be able to turn down a shared action if its level is unfit.
>
> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
> Acked-by: Ori Kam <orika@nvidia.com>
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
On 11/2/2020 11:43 AM, Ivan Malov wrote:
> In a flow rule, attribute "transfer" means operation level
> at which both traffic is matched and actions are conducted.
>
> Add the very same attribute to shared action configuration.
> If a driver needs to prepare HW resources in two different
> ways, depending on the operation level, in order to set up
> an action, then this new attribute will indicate the level.
> Also, when handling a flow rule insertion, the driver will
> be able to turn down a shared action if its level is unfit.
>
> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
> Acked-by: Ori Kam <orika@nvidia.com>
> ---
> lib/librte_ethdev/rte_flow.h | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index a8eac4deb..8b970ba0b 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -3487,6 +3487,14 @@ struct rte_flow_shared_action_conf {
> /**< Action valid for rules applied to ingress traffic. */
> uint32_t egress:1;
> /**< Action valid for rules applied to egress traffic. */
> +
> + /**
> + * When set to 1, indicates that the action is valid for
> + * transfer traffic; otherwise, for non-transfer traffic.
> + *
> + * See struct rte_flow_attr.
> + */
> + uint32_t transfer:1;
Is this require any documentation update?
Also cc'ed Andrey, as he is author of the shared action feature, @Andrey can you
please review this update?
On Mon, Nov 2, 2020 at 5:15 AM Andrew Rybchenko <
andrew.rybchenko@oktetlabs.ru> wrote:
> On 11/2/20 2:43 PM, Ivan Malov wrote:
> > In a flow rule, attribute "transfer" means operation level
> > at which both traffic is matched and actions are conducted.
> >
> > Add the very same attribute to shared action configuration.
> > If a driver needs to prepare HW resources in two different
> > ways, depending on the operation level, in order to set up
> > an action, then this new attribute will indicate the level.
> > Also, when handling a flow rule insertion, the driver will
> > be able to turn down a shared action if its level is unfit.
> >
> > Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
> > Acked-by: Ori Kam <orika@nvidia.com>
>
> Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Hi Ferruh,
On 02/11/2020 21:54, Ferruh Yigit wrote:
> On 11/2/2020 11:43 AM, Ivan Malov wrote:
>> In a flow rule, attribute "transfer" means operation level
>> at which both traffic is matched and actions are conducted.
>>
>> Add the very same attribute to shared action configuration.
>> If a driver needs to prepare HW resources in two different
>> ways, depending on the operation level, in order to set up
>> an action, then this new attribute will indicate the level.
>> Also, when handling a flow rule insertion, the driver will
>> be able to turn down a shared action if its level is unfit.
>>
>> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
>> Acked-by: Ori Kam <orika@nvidia.com>
>> ---
>> lib/librte_ethdev/rte_flow.h | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
>> index a8eac4deb..8b970ba0b 100644
>> --- a/lib/librte_ethdev/rte_flow.h
>> +++ b/lib/librte_ethdev/rte_flow.h
>> @@ -3487,6 +3487,14 @@ struct rte_flow_shared_action_conf {
>> /**< Action valid for rules applied to ingress traffic. */
>> uint32_t egress:1;
>> /**< Action valid for rules applied to egress traffic. */
>> +
>> + /**
>> + * When set to 1, indicates that the action is valid for
>> + * transfer traffic; otherwise, for non-transfer traffic.
>> + *
>> + * See struct rte_flow_attr.
>> + */
>> + uint32_t transfer:1;
>
> Is this require any documentation update?
>
> Also cc'ed Andrey, as he is author of the shared action feature, @Andrey
> can you please review this update?
Many-many thanks to you for reviewing the patch. And thanks for inviting
@Andrey. I should've done that from the very beginning.
What's for documentation update, = I did take a look at
"doc/guides/prog_guide/rte_flow.rst" after Ori had suggested to do so.
As far as I can learn from the file, the convention is to describe the
action structure itself, i.e. not any auxiliary structures.
For example, direct input to action SHARED is "struct
rte_flow_shared_action". And it's already described by an empty table
("table:: SHARED") in the doc file.
On the other hand, documentation of "struct rte_flow_shared_action_conf"
belongs in a place where the API rte_flow_shared_action_create() is
documented. This API is only mentioned by
"doc/guides/prog_guide/rte_flow.rst", and it looks like it's not
documented anywhere but in the header file itself
("lib/librte_ethdev/rte_flow.h").
So, it looks like this particular patch does not need to provide an
update to documentation other that the existing comment before the field
in the structure.
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Monday, November 2, 2020 8:55 PM
> To: Ivan Malov <ivan.malov@oktetlabs.ru>; dev@dpdk.org; Andrey Vesnovaty
> <andreyv@nvidia.com>
> Cc: Xueming(Steven) Li <xuemingl@nvidia.com>; Ori Kam <orika@nvidia.com>;
> NBU-Contact-Thomas Monjalon <thomas@monjalon.net>; Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru>
> Subject: Re: [PATCH v3 1/2] ethdev: introduce transfer attribute to shared action
> conf
>
> On 11/2/2020 11:43 AM, Ivan Malov wrote:
> > In a flow rule, attribute "transfer" means operation level
> > at which both traffic is matched and actions are conducted.
> >
> > Add the very same attribute to shared action configuration.
> > If a driver needs to prepare HW resources in two different
> > ways, depending on the operation level, in order to set up
> > an action, then this new attribute will indicate the level.
> > Also, when handling a flow rule insertion, the driver will
> > be able to turn down a shared action if its level is unfit.
> >
> > Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
> > Acked-by: Ori Kam <orika@nvidia.com>
> > ---
> > lib/librte_ethdev/rte_flow.h | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> > index a8eac4deb..8b970ba0b 100644
> > --- a/lib/librte_ethdev/rte_flow.h
> > +++ b/lib/librte_ethdev/rte_flow.h
> > @@ -3487,6 +3487,14 @@ struct rte_flow_shared_action_conf {
> > /**< Action valid for rules applied to ingress traffic. */
> > uint32_t egress:1;
> > /**< Action valid for rules applied to egress traffic. */
> > +
> > + /**
> > + * When set to 1, indicates that the action is valid for
> > + * transfer traffic; otherwise, for non-transfer traffic.
> > + *
> > + * See struct rte_flow_attr.
> > + */
> > + uint32_t transfer:1;
>
> Is this require any documentation update?
>
> Also cc'ed Andrey, as he is author of the shared action feature, @Andrey can
> you
> please review this update?
Acked-by: Andrey Vesnovaty <andreyv@nvidia.com>
On 11/3/2020 2:10 PM, Ivan Malov wrote:
> Hi Ferruh,
>
> On 02/11/2020 21:54, Ferruh Yigit wrote:
>> On 11/2/2020 11:43 AM, Ivan Malov wrote:
>>> In a flow rule, attribute "transfer" means operation level
>>> at which both traffic is matched and actions are conducted.
>>>
>>> Add the very same attribute to shared action configuration.
>>> If a driver needs to prepare HW resources in two different
>>> ways, depending on the operation level, in order to set up
>>> an action, then this new attribute will indicate the level.
>>> Also, when handling a flow rule insertion, the driver will
>>> be able to turn down a shared action if its level is unfit.
>>>
>>> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
>>> Acked-by: Ori Kam <orika@nvidia.com>
>>> ---
>>> lib/librte_ethdev/rte_flow.h | 8 ++++++++
>>> 1 file changed, 8 insertions(+)
>>>
>>> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
>>> index a8eac4deb..8b970ba0b 100644
>>> --- a/lib/librte_ethdev/rte_flow.h
>>> +++ b/lib/librte_ethdev/rte_flow.h
>>> @@ -3487,6 +3487,14 @@ struct rte_flow_shared_action_conf {
>>> /**< Action valid for rules applied to ingress traffic. */
>>> uint32_t egress:1;
>>> /**< Action valid for rules applied to egress traffic. */
>>> +
>>> + /**
>>> + * When set to 1, indicates that the action is valid for
>>> + * transfer traffic; otherwise, for non-transfer traffic.
>>> + *
>>> + * See struct rte_flow_attr.
>>> + */
>>> + uint32_t transfer:1;
>>
>> Is this require any documentation update?
>>
>> Also cc'ed Andrey, as he is author of the shared action feature, @Andrey can
>> you please review this update?
>
> Many-many thanks to you for reviewing the patch. And thanks for inviting
> @Andrey. I should've done that from the very beginning.
>
> What's for documentation update, = I did take a look at
> "doc/guides/prog_guide/rte_flow.rst" after Ori had suggested to do so. As far as
> I can learn from the file, the convention is to describe the action structure
> itself, i.e. not any auxiliary structures.
>
> For example, direct input to action SHARED is "struct rte_flow_shared_action".
> And it's already described by an empty table ("table:: SHARED") in the doc file.
>
> On the other hand, documentation of "struct rte_flow_shared_action_conf" belongs
> in a place where the API rte_flow_shared_action_create() is documented. This API
> is only mentioned by "doc/guides/prog_guide/rte_flow.rst", and it looks like
> it's not documented anywhere but in the header file itself
> ("lib/librte_ethdev/rte_flow.h").
>
> So, it looks like this particular patch does not need to provide an update to
> documentation other that the existing comment before the field in the structure.
>
I missed that Ori requested same before, and I agree on your documentation
analysis above, there is not clear location to put this other than doxygen
comment, so OK to proceed as it is.
On 11/3/2020 2:20 PM, Andrey Vesnovaty wrote:
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Monday, November 2, 2020 8:55 PM
>> To: Ivan Malov <ivan.malov@oktetlabs.ru>; dev@dpdk.org; Andrey Vesnovaty
>> <andreyv@nvidia.com>
>> Cc: Xueming(Steven) Li <xuemingl@nvidia.com>; Ori Kam <orika@nvidia.com>;
>> NBU-Contact-Thomas Monjalon <thomas@monjalon.net>; Andrew Rybchenko
>> <andrew.rybchenko@oktetlabs.ru>
>> Subject: Re: [PATCH v3 1/2] ethdev: introduce transfer attribute to shared action
>> conf
>>
>> On 11/2/2020 11:43 AM, Ivan Malov wrote:
>>> In a flow rule, attribute "transfer" means operation level
>>> at which both traffic is matched and actions are conducted.
>>>
>>> Add the very same attribute to shared action configuration.
>>> If a driver needs to prepare HW resources in two different
>>> ways, depending on the operation level, in order to set up
>>> an action, then this new attribute will indicate the level.
>>> Also, when handling a flow rule insertion, the driver will
>>> be able to turn down a shared action if its level is unfit.
>>>
>>> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
>>> Acked-by: Ori Kam <orika@nvidia.com>
>>> ---
>>> lib/librte_ethdev/rte_flow.h | 8 ++++++++
>>> 1 file changed, 8 insertions(+)
>>>
>>> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
>>> index a8eac4deb..8b970ba0b 100644
>>> --- a/lib/librte_ethdev/rte_flow.h
>>> +++ b/lib/librte_ethdev/rte_flow.h
>>> @@ -3487,6 +3487,14 @@ struct rte_flow_shared_action_conf {
>>> /**< Action valid for rules applied to ingress traffic. */
>>> uint32_t egress:1;
>>> /**< Action valid for rules applied to egress traffic. */
>>> +
>>> + /**
>>> + * When set to 1, indicates that the action is valid for
>>> + * transfer traffic; otherwise, for non-transfer traffic.
>>> + *
>>> + * See struct rte_flow_attr.
>>> + */
>>> + uint32_t transfer:1;
>>
>> Is this require any documentation update?
>>
>> Also cc'ed Andrey, as he is author of the shared action feature, @Andrey can
>> you
>> please review this update?
>
> Acked-by: Andrey Vesnovaty <andreyv@nvidia.com>
>
Series applied to dpdk-next-net/main, thanks.
@@ -3487,6 +3487,14 @@ struct rte_flow_shared_action_conf {
/**< Action valid for rules applied to ingress traffic. */
uint32_t egress:1;
/**< Action valid for rules applied to egress traffic. */
+
+ /**
+ * When set to 1, indicates that the action is valid for
+ * transfer traffic; otherwise, for non-transfer traffic.
+ *
+ * See struct rte_flow_attr.
+ */
+ uint32_t transfer:1;
};
/**