DPDK patches and discussions
 help / color / mirror / Atom feed
From: Li Zhang <lizh@nvidia.com>
To: Ori Kam <orika@nvidia.com>, Dekel Peled <dekelp@nvidia.com>,
	"Slava Ovsiienko" <viacheslavo@nvidia.com>,
	Matan Azrad <matan@nvidia.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	NBU-Contact-Thomas Monjalon <thomas@monjalon.net>,
	Raslan Darawsheh <rasland@nvidia.com>
Subject: Re: [dpdk-dev] [PATCH v4 1/1] net/mlx5: support match ICMP identifier	fields
Date: Mon, 5 Oct 2020 09:41:03 +0000
Message-ID: <DM6PR12MB40902BD1797F266B9CD5883CBF0C0@DM6PR12MB4090.namprd12.prod.outlook.com> (raw)
In-Reply-To: <DM6PR12MB409035A2CCCF16A366A5EA30BF0C0@DM6PR12MB4090.namprd12.prod.outlook.com>

Hi Qri,

Please help review the new patch V6
https://patchwork.dpdk.org/patch/79613/

Regards,
Li Zhang
> -----Original Message-----
> From: Li Zhang
> Sent: Monday, October 5, 2020 5:20 PM
> To: Ori Kam <orika@nvidia.com>; Dekel Peled <dekelp@nvidia.com>; Slava
> Ovsiienko <viacheslavo@nvidia.com>; Matan Azrad <matan@nvidia.com>
> Cc: dev@dpdk.org; NBU-Contact-Thomas Monjalon
> <thomas@monjalon.net>; Raslan Darawsheh <rasland@nvidia.com>
> Subject: RE: [dpdk-dev] [PATCH v4 1/1] net/mlx5: support match ICMP
> identifier fields
> 
> Hi Qri,
> 
> Thanks for your comments.
> I will update it in V5 patch.
> Please help review it to make sure I got your point.
> 
> Regards,
> Li Zhang
> 
> > -----Original Message-----
> > From: Ori Kam <orika@nvidia.com>
> > Sent: Sunday, October 4, 2020 7:59 PM
> > To: Li Zhang <lizh@nvidia.com>; Dekel Peled <dekelp@nvidia.com>; Slava
> > Ovsiienko <viacheslavo@nvidia.com>; Matan Azrad <matan@nvidia.com>
> > Cc: dev@dpdk.org; NBU-Contact-Thomas Monjalon
> <thomas@monjalon.net>;
> > Raslan Darawsheh <rasland@nvidia.com>
> > Subject: RE: [dpdk-dev] [PATCH v4 1/1] net/mlx5: support match ICMP
> > identifier fields
> >
> > Hi Li
> >
> > PSB
> >
> > Best,
> > Ori
> > > -----Original Message-----
> > > From: dev <dev-bounces@dpdk.org> On Behalf Of Li Zhang
> > > Sent: Friday, October 2, 2020 11:41 AM
> > > Subject: [dpdk-dev] [PATCH v4 1/1] net/mlx5: support match ICMP
> > > identifier fields
> > >
> > > PRM expose fields "Icmp_header_data" in IPv4 ICMP.
> > > Update ICMP mask parameter with ICMP identifier and sequence number
> > > fields.
> > > ICMP sequence number spec with mask, Icmp_header_data low 16 bits
> > > are
> > set.
> > > ICMP identifier spec with mask, Icmp_header_data high 16 bits are set.
> > >
> > > Signed-off-by: Li Zhang <lizh@nvidia.com>
> > > ---
> > >  doc/guides/nics/mlx5.rst               |  4 ++--
> > >  doc/guides/rel_notes/release_20_11.rst |  2 +-
> > >  drivers/net/mlx5/mlx5_flow.c           | 10 ++++++++--
> > >  drivers/net/mlx5/mlx5_flow_dv.c        | 24 ++++++++++++++++++++++--
> > >  4 files changed, 33 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
> > > index 211c0c5a6c..576dbe5efd 100644
> > > --- a/doc/guides/nics/mlx5.rst
> > > +++ b/doc/guides/nics/mlx5.rst
> > > @@ -288,7 +288,7 @@ Limitations
> > >    - The input buffer, providing the removal size, is not validated.
> > >    - The buffer size must match the length of the headers to be removed.
> > >
> > > -- ICMP/ICMP6 code/type matching, IP-in-IP and MPLS flow matching
> > > are all
> > > +- ICMP(code/type/identifier/sequence number) / ICMP6(code/type)
> > > +matching,
> > > IP-in-IP and MPLS flow matching are all
> > >    mutually exclusive features which cannot be supported together
> > >    (see :ref:`mlx5_firmware_config`).
> > >
> > > @@ -1009,7 +1009,7 @@ Below are some firmware configurations listed.
> > >
> > >      FLEX_PARSER_PROFILE_ENABLE=1
> > >
> > > -- enable ICMP/ICMP6 code/type fields matching::
> > > +- enable ICMP(code/type/identifier/sequence number) /
> > > +ICMP6(code/type)
> > > fields matching::
> > >
> > >      FLEX_PARSER_PROFILE_ENABLE=2
> > >
> > > diff --git a/doc/guides/rel_notes/release_20_11.rst
> > > b/doc/guides/rel_notes/release_20_11.rst
> > > index c6642f5f94..791f133d8f 100644
> > > --- a/doc/guides/rel_notes/release_20_11.rst
> > > +++ b/doc/guides/rel_notes/release_20_11.rst
> > > @@ -73,7 +73,7 @@ New Features
> > >    * Added flag action.
> > >    * Added raw encap/decap actions.
> > >    * Added VXLAN encap/decap actions.
> > > -  * Added ICMP and ICMP6 matching items.
> > > +  * Added ICMP(code/type/identifier/sequence number) and
> > > ICMP6(code/type) matching items.
> > >    * Added option to set port mask for insertion/deletion:
> > >      ``--portmask=N``
> > >      where N represents the hexadecimal bitmask of ports used.
> > > diff --git a/drivers/net/mlx5/mlx5_flow.c
> > > b/drivers/net/mlx5/mlx5_flow.c index 416505f1c8..3cabfd4627 100644
> > > --- a/drivers/net/mlx5/mlx5_flow.c
> > > +++ b/drivers/net/mlx5/mlx5_flow.c
> > > @@ -1303,6 +1303,12 @@ mlx5_flow_validate_item_icmp(const struct
> > > rte_flow_item *item,
> > >  			     struct rte_flow_error *error)  {
> > >  	const struct rte_flow_item_icmp *mask = item->mask;
> > > +	const struct rte_flow_item_icmp nic_mask = {
> > > +		.hdr.icmp_type = 0xff,
> > > +		.hdr.icmp_code = 0xff,
> > > +		.hdr.icmp_ident = RTE_BE16(0xffff),
> > > +		.hdr.icmp_seq_nb = RTE_BE16(0xffff),
> > > +	};
> > >  	const int tunnel = !!(item_flags & MLX5_FLOW_LAYER_TUNNEL);
> > >  	const uint64_t l3m = tunnel ? MLX5_FLOW_LAYER_INNER_L3_IPV4 :
> > >  				      MLX5_FLOW_LAYER_OUTER_L3_IPV4;
> > @@ -1325,10 +1331,10 @@
> > > mlx5_flow_validate_item_icmp(const struct rte_flow_item *item,
> > >  					  RTE_FLOW_ERROR_TYPE_ITEM,
> > > item,
> > >  					  "multiple L4 layers not supported");
> > >  	if (!mask)
> > > -		mask = &rte_flow_item_icmp_mask;
> > > +		mask = &nic_mask;
> > >  	ret = mlx5_flow_item_acceptable
> > >  		(item, (const uint8_t *)mask,
> > > -		 (const uint8_t *)&rte_flow_item_icmp_mask,
> > > +		 (const uint8_t *)&nic_mask,
> > >  		 sizeof(struct rte_flow_item_icmp), error);
> > >  	if (ret < 0)
> > >  		return ret;
> > > diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> > > b/drivers/net/mlx5/mlx5_flow_dv.c index 3819cdb266..a0b78934fd
> > > 100644
> > > --- a/drivers/net/mlx5/mlx5_flow_dv.c
> > > +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> > > @@ -7378,6 +7378,14 @@ flow_dv_translate_item_icmp(void *matcher,
> > void
> > > *key,  {
> > >  	const struct rte_flow_item_icmp *icmp_m = item->mask;
> > >  	const struct rte_flow_item_icmp *icmp_v = item->spec;
> > > +	const struct rte_flow_item_icmp nic_mask = {
> > > +		.hdr.icmp_type = 0xff,
> > > +		.hdr.icmp_code = 0xff,
> > > +		.hdr.icmp_ident = RTE_BE16(0xffff),
> > > +		.hdr.icmp_seq_nb = RTE_BE16(0xffff),
> > > +	};
> >
> > This is incorrect, like before, you are changing existing app behavior.
> > Think about it this way, until now the app didn't place any mask so he
> > just matched on the rte_default now with our new version all the
> > matching will fail. Since you are adding more must fields.
> >
> > > +	uint32_t icmp_header_data_m = 0;
> > > +	uint32_t icmp_header_data_v = 0;
> > >  	void *headers_m;
> > >  	void *headers_v;
> > >  	void *misc3_m = MLX5_ADDR_OF(fte_match_param, matcher, @@ -
> > 7396,8
> > > +7404,14 @@ flow_dv_translate_item_icmp(void *matcher, void *key,
> > >  	MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_protocol,
> > > IPPROTO_ICMP);
> > >  	if (!icmp_v)
> > >  		return;
> > > -	if (!icmp_m)
> > > -		icmp_m = &rte_flow_item_icmp_mask;
> > > +	if (!icmp_m) {
> > > +		icmp_m = &nic_mask;
> > > +		icmp_header_data_m = UINT32_MAX;
> >
> > Please see comment above.
> >
> > > +	} else {
> > > +		icmp_header_data_m = rte_be_to_cpu_16(icmp_m-
> > > >hdr.icmp_seq_nb);
> > > +		icmp_header_data_m |=
> > > +			rte_be_to_cpu_16(icmp_m->hdr.icmp_ident) << 16;
> > > +	}
> > >  	/*
> > >  	 * Force flow only to match the non-fragmented IPv4 ICMP packets.
> > >  	 * If only the protocol is specified, no need to match the frag.
> > > @@ -7412,6 +7426,12 @@ flow_dv_translate_item_icmp(void *matcher,
> > void
> > > *key,
> > >  		 icmp_m->hdr.icmp_code);
> > >  	MLX5_SET(fte_match_set_misc3, misc3_v, icmp_code,
> > >  		 icmp_v->hdr.icmp_code & icmp_m->hdr.icmp_code);
> > > +	icmp_header_data_v = rte_be_to_cpu_16(icmp_v-
> > >hdr.icmp_seq_nb);
> > > +	icmp_header_data_v |= rte_be_to_cpu_16(icmp_v->hdr.icmp_ident)
> > > << 16;
> > > +	MLX5_SET(fte_match_set_misc3, misc3_m, icmp_header_data,
> > > +		 icmp_header_data_m);
> > > +	MLX5_SET(fte_match_set_misc3, misc3_v, icmp_header_data,
> > > +		 icmp_header_data_v & icmp_header_data_m);
> > >  }
> > >
> > >  /**
> > > --
> > > 2.21.0


  reply	other threads:[~2020-10-05  9:41 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20200930164734.23675-1-lizh@nvidia.co>
2020-10-02  8:40 ` Li Zhang
2020-10-04 11:59   ` Ori Kam
2020-10-05  9:20     ` Li Zhang
2020-10-05  9:41       ` Li Zhang [this message]
2020-10-05  9:18   ` [dpdk-dev] [PATCH v5 " Li Zhang
2020-10-05  9:31     ` [dpdk-dev] [PATCH v6 " Li Zhang
2020-10-05 10:33       ` Ori Kam
2020-10-06  5:12         ` Li Zhang
2020-10-06 12:07           ` Ori Kam
2020-10-07  6:08             ` Li Zhang
2020-10-09  6:01       ` [dpdk-dev] [PATCH v7 " Li Zhang
2020-10-09  6:06         ` [dpdk-dev] [PATCH v8 " Li Zhang
2020-10-09  6:11           ` [dpdk-dev] [PATCH v9 " Li Zhang
2020-10-11 10:19             ` Raslan Darawsheh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DM6PR12MB40902BD1797F266B9CD5883CBF0C0@DM6PR12MB4090.namprd12.prod.outlook.com \
    --to=lizh@nvidia.com \
    --cc=dekelp@nvidia.com \
    --cc=dev@dpdk.org \
    --cc=matan@nvidia.com \
    --cc=orika@nvidia.com \
    --cc=rasland@nvidia.com \
    --cc=thomas@monjalon.net \
    --cc=viacheslavo@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git