[dpdk-dev] [PATCH v5 01/11] ethdev: add extensions attributes to IPv6 item
Dekel Peled
dekelp at nvidia.com
Tue Oct 13 10:22:44 CEST 2020
Thanks, PSB.
> -----Original Message-----
> From: Thomas Monjalon <thomas at monjalon.net>
> Sent: Monday, October 12, 2020 11:42 PM
> To: Ori Kam <orika at nvidia.com>; Dekel Peled <dekelp at nvidia.com>
> Cc: ferruh.yigit at intel.com; arybchenko at solarflare.com;
> konstantin.ananyev at intel.com; olivier.matz at 6wind.com;
> wenzhuo.lu at intel.com; beilei.xing at intel.com;
> bernard.iremonger at intel.com; Matan Azrad <matan at nvidia.com>; Shahaf
> Shuler <shahafs at nvidia.com>; Slava Ovsiienko <viacheslavo at nvidia.com>;
> dev at dpdk.org; Asaf Penso <asafp at nvidia.com>
> Subject: Re: [dpdk-dev] [PATCH v5 01/11] ethdev: add extensions attributes
> to IPv6 item
>
> 12/10/2020 12:43, Dekel Peled:
> > - * Note: IPv6 options are handled by dedicated pattern items, see
> > - * RTE_FLOW_ITEM_TYPE_IPV6_EXT.
> > + * Dedicated flags indicate existence of specific extension headers.
> > + * Every type of extension header can use a dedicated pattern item,
> > + or
> > + * the generic item RTE_FLOW_ITEM_TYPE_IPV6_EXT.
>
> I don't understand this last sentence.
I'll rephrase.
>
> > */
> > struct rte_flow_item_ipv6 {
> > struct rte_ipv6_hdr hdr; /**< IPv6 header definition. */
> > + uint32_t hop_ext_exist:1;
> > + /**< Hop-by-Hop Options extension header exists. */
> > + uint32_t rout_ext_exist:1;
>
> "rout" looks weird. Would be "route" appropriate?
I'll change to "route".
>
> > + /**< Routing extension header exists. */
> > + uint32_t frag_ext_exist:1;
> > + /**< Fragment extension header exists. */
> > + uint32_t auth_ext_exist:1;
> > + /**< Authentication extension header exists. */
> > + uint32_t esp_ext_exist:1;
> > + /**< Encapsulation Security Payload extension header exists. */
> > + uint32_t dest_ext_exist:1;
> > + /**< Destination Options extension header exists. */
> > + uint32_t mobil_ext_exist:1;
> > + /**< Mobility extension header exists. */
> > + uint32_t hip_ext_exist:1;
> > + /**< Host Identity Protocol extension header exists. */
> > + uint32_t shim6_ext_exist:1;
> > + /**< Shim6 Protocol extension header exists. */
>
> About the field names, the "_exist" suffix is pretty clear, but without being
> able to say why, I feel it is a strange name.
> I was thinking about renaming the fields with a "has_" prefix.
> Does it look better?
I'm afraid a "has_" prefix doesn't look appropriate IMHO.
I still prefer the "_exist" suffix.
>
More information about the dev
mailing list