[dpdk-dev] [PATCH v3 2/8] app/testpmd: add GENEVE option item support

[Slava Ovsiienko]

Hi,

> -----Original Message-----
> From: Suanming Mou <suanmingm at nvidia.com>
> Sent: Friday, January 8, 2021 14:11
> To: Shiri Kuzin <shirik at nvidia.com>; dev at dpdk.org
> Cc: Slava Ovsiienko <viacheslavo at nvidia.com>; NBU-Contact-Adrien Mazarguil
> <adrien.mazarguil at 6wind.com>; Ori Kam <orika at nvidia.com>;
> ferruh.yigit at intel.com; NBU-Contact-Thomas Monjalon
> <thomas at monjalon.net>; Raslan Darawsheh <rasland at nvidia.com>
> Subject: RE: [dpdk-dev] [PATCH v3 2/8] app/testpmd: add GENEVE option item
> support
> 
> Hi,
> 
> > -----Original Message-----
> > From: dev <dev-bounces at dpdk.org> On Behalf Of Shiri Kuzin
> > Sent: Thursday, January 7, 2021 4:39 PM
> > To: dev at dpdk.org
> > Cc: Slava Ovsiienko <viacheslavo at nvidia.com>; NBU-Contact-Adrien
> > Mazarguil <adrien.mazarguil at 6wind.com>; Ori Kam <orika at nvidia.com>;
> > ferruh.yigit at intel.com; NBU-Contact-Thomas Monjalon
> > <thomas at monjalon.net>; Raslan Darawsheh <rasland at nvidia.com>
> > Subject: [dpdk-dev] [PATCH v3 2/8] app/testpmd: add GENEVE option item
> > support
> >
> > From: Viacheslav Ovsiienko <viacheslavo at nvidia.com>
> >
> > The patch adds the GENEVE option rte flow item support to command line
> > interpreter. The flow command with GENEVE option items looks like:
> >
> >   flow create 0 ingress pattern eth / ipv4 / udp / geneve vni is 100 /
> >        geneve-opt class is 99 length is 1 type is 0 data is 0x669988 /
> >        end actions drop / end
> >
> > The option length should be specified in 32-bit words, this value
> > specifies the length of the data pattern/mask arrays (should be
> > multiplied by sizeof(uint32_t) to be expressed in bytes. If match on
> > the length itself is not needed the mask should be set to zero, in
> > this case length is used to specify the pattern/mask array lengths only.
> >
> > Signed-off-by: Viacheslav Ovsiienko <viacheslavo at nvidia.com>
> > ---
> >  app/test-pmd/cmdline_flow.c                 | 102 ++++++++++++++++++--
> >  doc/guides/testpmd_app_ug/testpmd_funcs.rst |   8 ++
> >  2 files changed, 104 insertions(+), 6 deletions(-)
> >
> 
> snip
> 
> >  	/* Output buffer is not necessarily NUL-terminated. */ @@ -7486,6
> > +7558,9 @@ flow_item_default_mask(const struct rte_flow_item *item)
> >  	case RTE_FLOW_ITEM_TYPE_GENEVE:
> >  		mask = &rte_flow_item_geneve_mask;
> >  		break;
> > +	case RTE_FLOW_ITEM_TYPE_GENEVE_OPT:
> > +		mask = &rte_flow_item_geneve_opt_mask;
> > +		break;
> >  	case RTE_FLOW_ITEM_TYPE_PPPOE_PROTO_ID:
> >  		mask = &rte_flow_item_pppoe_proto_id_mask;
> >  		break;
> > @@ -7598,6 +7673,8 @@ cmd_set_raw_parsed(const struct buffer *in)
> >  	/* process hdr from upper layer to low layer (L3/L4 -> L2). */
> >  	data_tail = data + ACTION_RAW_ENCAP_MAX_DATA;
> >  	for (i = n - 1 ; i >= 0; --i) {
> > +		const struct rte_flow_item_geneve_opt *opt;
> > +
> >  		item = in->args.vc.pattern + i;
> >  		if (item->spec == NULL)
> >  			item->spec = flow_item_default_mask(item); @@ -
> > 7650,6 +7727,19 @@ cmd_set_raw_parsed(const struct buffer *in)
> >  		case RTE_FLOW_ITEM_TYPE_GENEVE:
> >  			size = sizeof(struct rte_geneve_hdr);
> >  			break;
> > +		case RTE_FLOW_ITEM_TYPE_GENEVE_OPT:
> > +			opt = (const struct rte_flow_item_geneve_opt *)
> > +								item->spec;
> > +			size = sizeof(struct rte_flow_item_geneve_opt) -
> > +			       sizeof(opt->data);
> 
> If I understand correctly, the potential here is to calculate the geneve option
> size, right?
> Have we confirmed the struct is well aligned for all arch, and will not be aligned
> to other size?
> Or is it better to use "offsetof" here?

Yes, you are right, "offsetof" would be more relevant, thank you the catching,

With best regards,
Slava

> 
> > +			if (opt->option_len && opt->data) {
> > +				*total_size += opt->option_len *
> > +					       sizeof(uint32_t);
> > +				rte_memcpy(data_tail - (*total_size),
> > +					   opt->data,
> > +					   opt->option_len * sizeof(uint32_t));
> > +			}
> > +			break;
> >  		case RTE_FLOW_ITEM_TYPE_L2TPV3OIP:
> >  			size = sizeof(rte_be32_t);
> >  			proto = 0x73;
> > diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > index 9be450066e..37278d31d6 100644
> > --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > @@ -3680,6 +3680,14 @@ This section lists supported pattern items and
> > their attributes, if any.
> >    - ``vni {unsigned}``: virtual network identifier.
> >    - ``protocol {unsigned}``: protocol type.
> >
> > +- ``geneve-opt``: match GENEVE header option.
> > +
> > +  - ``class {unsigned}``: GENEVE option class.
> > +  - ``type {unsigned}``: GENEVE option type.
> > +  - ``length {unsigned}``: GENEVE option length in 32-bit words.
> > +  - ``data {hex string}``: GENEVE option data, the legnt is defined by
> > +    ``length`` field
> > +
> >  - ``vxlan-gpe``: match VXLAN-GPE header.
> >
> >    - ``vni {unsigned}``: VXLAN-GPE identifier.
> > --
> > 2.21.0