-----Original Message----- From: Andrew Rybchenko <arybche...@solarflare.com> Sent: Monday, October 14, 2019 12:38 PM To: Kiran Kumar Kokkilagadda <kirankum...@marvell.com>; Adrien Mazarguil <adrien.mazarg...@6wind.com>; John McNamara <john.mcnam...@intel.com>; Marko Kovacevic <marko.kovace...@intel.com>; Thomas Monjalon <tho...@monjalon.net>; Ferruh Yigit <ferruh.yi...@intel.com> Cc: dev@dpdk.org; ajit.khapa...@broadcom.com Subject: [EXT] Re: [dpdk-dev] [PATCH] ethdev: add HIGIG2 key field to flow API
External Email ---------------------------------------------------------------------- On 10/14/19 7:29 AM, kirankum...@marvell.com wrote: > From: Kiran Kumar K <kirankum...@marvell.com> > > Add new rte_flow_item_higig2_hdr in order to match higig2 header. > It is a layer 2.5 protocol and used in broadcom switches. > Header format is based on the following document. > https://urldefense.proofpoint.com/v2/url?u=http-3A__read.pudn.com_downloads558_doc_comm_2301468_HiGig-5Fprotocol.pdf&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=owEKckYY4FTmil1Z6oBURwkTThyuRbLAY9LdfiaT6HA&m=E--ycNQdWbQ_5bNBEDLQHwLS3axpOnsTIAtUe290BPU&s=IDtfzPnjppslgp0Wrkv_TGxl1wIqDspU6jNzC3LKciM&e= > > > Signed-off-by: Kiran Kumar K <kirankum...@marvell.com> > --- > doc/guides/prog_guide/rte_flow.rst | 8 ++++ > lib/librte_ethdev/rte_flow.c | 1 + > lib/librte_ethdev/rte_flow.h | 77 ++++++++++++++++++++++++++++++ As far as I remember support in testpmd is a must requirement to add new RTE flow API feature. >> Will add support to parse this item in testpmd, Will be adding basic fields/ >> non bitmap fields to parse, like classification and vid. > 3 files changed, 86 insertions(+) > > diff --git a/doc/guides/prog_guide/rte_flow.rst > b/doc/guides/prog_guide/rte_flow.rst > index 1c837ff13..71365b159 100644 > --- a/doc/guides/prog_guide/rte_flow.rst > +++ b/doc/guides/prog_guide/rte_flow.rst > @@ -1290,6 +1290,14 @@ Matches a IP Authentication Header (RFC 4302). > - Default ``mask`` matches spi. > > > +Item: ``HIGIG2`` > +^^^^^^^^^^^^^^^^^ > + > +Matches a HIGIG2 header field. It is layer 2.5 protocol and used in > +broadcom switches. > + > +- Default ``mask`` matches classification and vlan. > + Right now there is one empty line between items and two between the last item and actions. It should be preserved. >> Ack, Will add this change. > Actions > ~~~~~~~ > > diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c > index 81a85b995..ca0f68016 100644 > --- a/lib/librte_ethdev/rte_flow.c > +++ b/lib/librte_ethdev/rte_flow.c > @@ -83,6 +83,7 @@ static const struct rte_flow_desc_data rte_flow_desc_item[] > = { > MK_FLOW_ITEM(NSH, sizeof(struct rte_flow_item_nsh)), > MK_FLOW_ITEM(IGMP, sizeof(struct rte_flow_item_igmp)), > MK_FLOW_ITEM(AH, sizeof(struct rte_flow_item_ah)), > + MK_FLOW_ITEM(HIGIG2, sizeof(struct rte_flow_item_higig2_hdr)), > }; > > /** Generate flow_action[] entry. */ > diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h > index bcfc06cdc..59e37f714 100644 > --- a/lib/librte_ethdev/rte_flow.h > +++ b/lib/librte_ethdev/rte_flow.h > @@ -491,6 +491,12 @@ enum rte_flow_item_type { > * > */ > RTE_FLOW_ITEM_TYPE_AH, > + > + /** > + * Matches a HIGIG header. > + * see struct rte_flow_item_higig2_hdr. > + */ > + RTE_FLOW_ITEM_TYPE_HIGIG2, > }; > > /** > @@ -515,6 +521,77 @@ static const struct rte_flow_item_any > rte_flow_item_any_mask = { > }; > #endif > > +/** > + * RTE_FLOW_ITEM_TYPE_HIGIG2 > + * Matches higig2 header. > + */ > +struct rte_higig2_frc { > + uint32_t ksop:8; > + uint32_t resv:3; > + uint32_t mcst:1; > + uint32_t tc:4; > + uint32_t dst_modid:8; > + uint32_t dst_pid:8; > + uint32_t src_modid:8; > + uint32_t src_pid:8; > + uint32_t lbid:8; > + uint32_t dp:2; > + uint32_t resv1:3; > + uint32_t ppd_type:3; > +} __attribute__((packed)); > + > +struct rte_higig2_ppt_type0 { > + uint32_t dst_t:1; > + uint32_t dst_tgid:3; > + uint32_t ingress_tagged:1; > + uint32_t mirror_only:1; > + uint32_t mirror_done:1; > + uint32_t mirror:1; > + uint32_t res:2; > + uint32_t l3:1; > + uint32_t label_present:1; > + uint32_t vc_label2:4; > + uint32_t vc_label1:8; > + uint32_t vc_label0:8; > + uint32_t vid_high:8; > + uint32_t vid_low:8; > + uint32_t pfm:2; > + uint32_t src_t:1; > + uint32_t res1:2; > + uint32_t opcode:3; > + uint32_t hdr_ext_len:3; > + uint32_t res2:5; > +} __attribute__((packed)); > + > +struct rte_higig2_ppt_type1 { > + uint32_t classification:16; > + uint32_t resv:16; > + uint32_t vid:16; > + uint32_t pfm:2; > + uint32_t src_t:1; > + uint32_t resv1:2; > + uint32_t opcode:3; > + uint32_t hdr_ext_len:3; > + uint32_t resv2:5; > +} __attribute__((packed)); > + Usage of bit fields in network headers require big/little endian handling. See lib/librte_net/rte_gre.h Also other network protocol headers are defined in lib/librte_net. >> Will move the header to new file lib/librte_net/rte_higig.h > +RTE_STD_C11 > +struct rte_flow_item_higig2_hdr { > + struct rte_higig2_frc fcr; > + union { > + struct rte_higig2_ppt_type0 ppt0; > + struct rte_higig2_ppt_type1 ppt1; > + }; > +} __attribute__((packed)); > + > +/** Default mask for RTE_FLOW_ITEM_TYPE_HIGIG2. */ > +#ifndef __cplusplus > +static const struct rte_flow_item_higig2_hdr rte_flow_item_higig2_hdr_mask = > { > + .ppt1.classification = 0xffff, > + .ppt1.vid = 0xfff, > +}; It would be useful to see default mask choice motivation. >> Most of the use case scenarios we have, the traffic will be scattered based >> on classification and vid fields, so added these 2 fields as default mask. > +#endif > + > /** > * RTE_FLOW_ITEM_TYPE_VF > *
