[dpdk-dev] [PATCH 1/2] ethdev: announce change to action modify data

[dpdk-dev] [PATCH 1/2] ethdev: announce change to action modify data

[Matan Azrad]

From: Ori Kam <orika@nvidia.com>

In the current implementation,
the action rte_flow_action_modify_field is not well defined
for fields larger than 64 bits (for example IPv6 source)
In addtion, the byte order is also not well defined.

Both of those issue should be fixed.

Signed-off-by: Ori Kam <orika@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 doc/guides/rel_notes/deprecation.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index d9c0e65921..b530616281 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -158,3 +158,7 @@ Deprecation Notices
 * security: The functions ``rte_security_set_pkt_metadata`` and
   ``rte_security_get_userdata`` will be made inline functions and additional
   flags will be added in structure ``rte_security_ctx`` in DPDK 21.11.
+
+* ethdev: The struct ``rte_flow_action_modify_data`` will be modified
+  to support modifying larger fields than 64 bits.
+  In addition, documentation will be updated to clarify byte order.
-- 
2.25.1

[dpdk-dev] [PATCH 2/2] ethdev: announce moving to general modify function

[Matan Azrad]

From: Ori Kam <orika@nvidia.com>

Currently there is a dedicated modify function for each
field that the application wants to change.
For example:
rte_flow_action_type_set_tp_port to modify destination port of UDP/TCP.
rte_flow_action_type_set_ipv4_dst to modify destination of IPv4.

A new function rte_flow_action_modif_field DPDK added the ability
to use the same function to modify any field, in addtion to be able to
modify the value based on different field and not just immediate value.

Signed-off-by: Ori Kam <orika@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 doc/guides/rel_notes/deprecation.rst | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index b530616281..77491c322f 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -162,3 +162,6 @@ Deprecation Notices
 * ethdev: The struct ``rte_flow_action_modify_data`` will be modified
   to support modifying larger fields than 64 bits.
   In addition, documentation will be updated to clarify byte order.
+
+* ethdev: Announce moving from dedicated modify function for each field,
+  to using the general ``rte_flow_modify_field`` action.
-- 
2.25.1

[dpdk-dev] [PATCH v2 1/2] ethdev: announce change to action modify data

[Ori Kam]

In the current implementation,
the action rte_flow_action_modify_field is not well defined
for fields larger than 64 bits (for example IPv6 source)
In addition, the byte order is also not well defined.

Both of those issue should be fixed.

Signed-off-by: Ori Kam <orika@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
V2:
  Fix typo.
---
 doc/guides/rel_notes/deprecation.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index d9c0e65921..b530616281 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -158,3 +158,7 @@ Deprecation Notices
 * security: The functions ``rte_security_set_pkt_metadata`` and
   ``rte_security_get_userdata`` will be made inline functions and additional
   flags will be added in structure ``rte_security_ctx`` in DPDK 21.11.
+
+* ethdev: The struct ``rte_flow_action_modify_data`` will be modified
+  to support modifying larger fields than 64 bits.
+  In addition, documentation will be updated to clarify byte order.
-- 
2.25.1

[dpdk-dev] [PATCH v2 2/2] ethdev: announce moving to general modify function

[Ori Kam]

Currently there is a dedicated modify function for each
field that the application wants to change.
For example:
rte_flow_action_type_set_tp_port to modify destination port of UDP/TCP.
rte_flow_action_type_set_ipv4_dst to modify destination of IPv4.

A new function rte_flow_action_modify_field DPDK added the ability
to use the same function to modify any field, in addition to be able to
modify the value based on different field and not just immediate value.

Signed-off-by: Ori Kam <orika@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
V2:
  Fix typo.
---
 doc/guides/rel_notes/deprecation.rst | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index b530616281..77491c322f 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -162,3 +162,6 @@ Deprecation Notices
 * ethdev: The struct ``rte_flow_action_modify_data`` will be modified
   to support modifying larger fields than 64 bits.
   In addition, documentation will be updated to clarify byte order.
+
+* ethdev: Announce moving from dedicated modify function for each field,
+  to using the general ``rte_flow_modify_field`` action.
-- 
2.25.1

Re: [dpdk-dev] [PATCH v2 2/2] ethdev: announce moving to general modify function

[Ajit Khaparde]

[-- Attachment #1: Type: text/plain, Size: 1421 bytes --]

On Tue, Aug 3, 2021 at 1:58 AM Ori Kam <orika@nvidia.com> wrote:
>
> Currently there is a dedicated modify function for each
> field that the application wants to change.
> For example:
> rte_flow_action_type_set_tp_port to modify destination port of UDP/TCP.
> rte_flow_action_type_set_ipv4_dst to modify destination of IPv4.
>
> A new function rte_flow_action_modify_field DPDK added the ability
> to use the same function to modify any field, in addition to be able to
> modify the value based on different field and not just immediate value.
>
> Signed-off-by: Ori Kam <orika@nvidia.com>
> Acked-by: Matan Azrad <matan@nvidia.com>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

> ---
> V2:
>   Fix typo.
> ---
>  doc/guides/rel_notes/deprecation.rst | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index b530616281..77491c322f 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -162,3 +162,6 @@ Deprecation Notices
>  * ethdev: The struct ``rte_flow_action_modify_data`` will be modified
>    to support modifying larger fields than 64 bits.
>    In addition, documentation will be updated to clarify byte order.
> +
> +* ethdev: Announce moving from dedicated modify function for each field,
> +  to using the general ``rte_flow_modify_field`` action.
> --
> 2.25.1
>

Re: [dpdk-dev] [PATCH v2 1/2] ethdev: announce change to action modify data

[Ajit Khaparde]

[-- Attachment #1: Type: text/plain, Size: 1279 bytes --]

On Tue, Aug 3, 2021 at 1:58 AM Ori Kam <orika@nvidia.com> wrote:
>
> In the current implementation,
> the action rte_flow_action_modify_field is not well defined
> for fields larger than 64 bits (for example IPv6 source)
> In addition, the byte order is also not well defined.
>
> Both of those issue should be fixed.
>
> Signed-off-by: Ori Kam <orika@nvidia.com>
> Acked-by: Matan Azrad <matan@nvidia.com>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

> ---
> V2:
>   Fix typo.
> ---
>  doc/guides/rel_notes/deprecation.rst | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index d9c0e65921..b530616281 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -158,3 +158,7 @@ Deprecation Notices
>  * security: The functions ``rte_security_set_pkt_metadata`` and
>    ``rte_security_get_userdata`` will be made inline functions and additional
>    flags will be added in structure ``rte_security_ctx`` in DPDK 21.11.
> +
> +* ethdev: The struct ``rte_flow_action_modify_data`` will be modified
> +  to support modifying larger fields than 64 bits.
> +  In addition, documentation will be updated to clarify byte order.
> --
> 2.25.1
>

Re: [dpdk-dev] [PATCH v2 1/2] ethdev: announce change to action modify data

[Andrew Rybchenko]

On 8/3/21 9:10 PM, Ajit Khaparde wrote:
> On Tue, Aug 3, 2021 at 1:58 AM Ori Kam <orika@nvidia.com> wrote:
>>
>> In the current implementation,
>> the action rte_flow_action_modify_field is not well defined
>> for fields larger than 64 bits (for example IPv6 source)
>> In addition, the byte order is also not well defined.
>>
>> Both of those issue should be fixed.
>>
>> Signed-off-by: Ori Kam <orika@nvidia.com>
>> Acked-by: Matan Azrad <matan@nvidia.com>
> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

Re: [dpdk-dev] [PATCH v2 1/2] ethdev: announce change to action modify data

[Jerin Jacob]

On Wed, Aug 4, 2021 at 5:40 PM Andrew Rybchenko
<andrew.rybchenko@oktetlabs.ru> wrote:
>
> On 8/3/21 9:10 PM, Ajit Khaparde wrote:
> > On Tue, Aug 3, 2021 at 1:58 AM Ori Kam <orika@nvidia.com> wrote:
> >>
> >> In the current implementation,
> >> the action rte_flow_action_modify_field is not well defined
> >> for fields larger than 64 bits (for example IPv6 source)
> >> In addition, the byte order is also not well defined.
> >>
> >> Both of those issue should be fixed.
> >>
> >> Signed-off-by: Ori Kam <orika@nvidia.com>
> >> Acked-by: Matan Azrad <matan@nvidia.com>
> > Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>
> Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

Acked-by: Jerin Jacob <jerinj@marvell.com>



>

Re: [dpdk-dev] [PATCH v2 2/2] ethdev: announce moving to general modify function

[Jerin Jacob]

On Tue, Aug 3, 2021 at 11:35 PM Ajit Khaparde
<ajit.khaparde@broadcom.com> wrote:
>
> On Tue, Aug 3, 2021 at 1:58 AM Ori Kam <orika@nvidia.com> wrote:
> >
> > Currently there is a dedicated modify function for each
> > field that the application wants to change.
> > For example:
> > rte_flow_action_type_set_tp_port to modify destination port of UDP/TCP.
> > rte_flow_action_type_set_ipv4_dst to modify destination of IPv4.
> >
> > A new function rte_flow_action_modify_field DPDK added the ability
> > to use the same function to modify any field, in addition to be able to
> > modify the value based on different field and not just immediate value.
> >
> > Signed-off-by: Ori Kam <orika@nvidia.com>
> > Acked-by: Matan Azrad <matan@nvidia.com>
> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

Acked-by: Jerin Jacob <jerinj@marvell.com>


>
> > ---
> > V2:
> >   Fix typo.
> > ---
> >  doc/guides/rel_notes/deprecation.rst | 3 +++
> >  1 file chanAcked-by: Jerin Jacob <jerinj@marvell.com>
ed, 3 insertions(+)
> >
> > diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> > index b530616281..77491c322f 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -162,3 +162,6 @@ Deprecation Notices
> >  * ethdev: The struct ``rte_flow_action_modify_data`` will be modified
> >    to support modifying larger fields than 64 bits.
> >    In addition, documentation will be updated to clarify byte order.
> > +
> > +* ethdev: Announce moving from dedicated modify function for each field,
> > +  to using the general ``rte_flow_modify_field`` action.
> > --
> > 2.25.1
> >

Re: [dpdk-dev] [PATCH v2 1/2] ethdev: announce change to action modify data

[Thomas Monjalon]

> > >> In the current implementation,
> > >> the action rte_flow_action_modify_field is not well defined
> > >> for fields larger than 64 bits (for example IPv6 source)
> > >> In addition, the byte order is also not well defined.
> > >>
> > >> Both of those issue should be fixed.
> > >>
> > >> Signed-off-by: Ori Kam <orika@nvidia.com>
> > >> Acked-by: Matan Azrad <matan@nvidia.com>
> > > Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> > Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Acked-by: Jerin Jacob <jerinj@marvell.com>

Applied, thanks.

Re: [dpdk-dev] [PATCH v2 2/2] ethdev: announce moving to general modify function

[Thomas Monjalon]

> > > Currently there is a dedicated modify function for each
> > > field that the application wants to change.
> > > For example:
> > > rte_flow_action_type_set_tp_port to modify destination port of UDP/TCP.
> > > rte_flow_action_type_set_ipv4_dst to modify destination of IPv4.
> > >
> > > A new function rte_flow_action_modify_field DPDK added the ability
> > > to use the same function to modify any field, in addition to be able to
> > > modify the value based on different field and not just immediate value.
> > >
> > > Signed-off-by: Ori Kam <orika@nvidia.com>
> > > Acked-by: Matan Azrad <matan@nvidia.com>
> > Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Acked-by: Jerin Jacob <jerinj@marvell.com>

> > > +* ethdev: Announce moving from dedicated modify function for each field,
> > > +  to using the general ``rte_flow_modify_field`` action.

This is a very vague announce.
OK we can replace a lot of actions with rte_flow_modify_field,
but it doesn't say when and which functions will be removed.
I think we should make a more precise announce for removal of some actions
in DPDK 22.11.

In the meantime, let's introduce this hint that something is changing
as it is acked.

Applied.