[PATCH v6 1/3] ethdev: rename action modify field data structure

[Suanming Mou]

Hi Thomas,

> -----Original Message-----
> From: Thomas Monjalon <thomas at monjalon.net>
> Sent: Monday, February 5, 2024 7:23 PM
> To: Suanming Mou <suanmingm at nvidia.com>
> Cc: ferruh.yigit at amd.com; Ori Kam <orika at nvidia.com>; Aman Singh
> <aman.deep.singh at intel.com>; Yuying Zhang <yuying.zhang at intel.com>; Dariusz
> Sosnowski <dsosnowski at nvidia.com>; Slava Ovsiienko
> <viacheslavo at nvidia.com>; Matan Azrad <matan at nvidia.com>; Andrew
> Rybchenko <andrew.rybchenko at oktetlabs.ru>; dev at dpdk.org
> Subject: Re: [PATCH v6 1/3] ethdev: rename action modify field data structure
> 
> 02/02/2024 01:42, Suanming Mou:
> > --- a/doc/guides/rel_notes/release_24_03.rst
> > +++ b/doc/guides/rel_notes/release_24_03.rst
> > @@ -124,6 +124,8 @@ ABI Changes
> >
> >  * No ABI change that would break compatibility with 23.11.
> >
> > +* ethdev: Rename the experimental ``struct
> > +rte_flow_action_modify_data`` to be ``struct rte_flow_field_data``
> 
> It should be in API change section.
> Please us past tense as recommened in comments in the file.
OK.

> 
> > --- a/lib/ethdev/rte_flow.h
> > +++ b/lib/ethdev/rte_flow.h
> > @@ -3894,6 +3894,7 @@ struct rte_flow_action_ethdev {
> >
> >  /**
> >   * Field IDs for MODIFY_FIELD action.
> > + * e.g. the packet field IDs used in RTE_FLOW_ACTION_TYPE_MODIFY_FIELD.
> 
> Better to give the full name in the first line, so no need to add a second line of
> comment.

So maybe " Field IDs for packet field, used by RTE_FLOW_ACTION_TYPE_MODIFY_FIELD."?
But when COMPARE item to be added. It will be " Field IDs for packet field, used by RTE_FLOW_ACTION_TYPE_MODIFY_FIELD and RTE_FLOW_ITEM_TYPE_COMPARE."  And I assume that will still need a second line since it is too long.

> 
> [...]
> > - * Field description for MODIFY_FIELD action.
> > + * Field description for packet field.
> > + * e.g. the packet fields used in RTE_FLOW_ACTION_TYPE_MODIFY_FIELD.
> 
> Same here, can be one simple line with full name.
> 
>