From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 36838A04B5; Thu, 1 Oct 2020 10:04:34 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 8C3C61D6FE; Thu, 1 Oct 2020 10:04:32 +0200 (CEST) Received: from nat-hk.nvidia.com (nat-hk.nvidia.com [203.18.50.4]) by dpdk.org (Postfix) with ESMTP id 05EB51D6F7 for ; Thu, 1 Oct 2020 10:04:30 +0200 (CEST) Received: from HKMAIL101.nvidia.com (Not Verified[10.18.92.100]) by nat-hk.nvidia.com (using TLS: TLSv1.2, AES256-SHA) id ; Thu, 01 Oct 2020 16:04:28 +0800 Received: from HKMAIL102.nvidia.com (10.18.16.11) by HKMAIL101.nvidia.com (10.18.16.10) with Microsoft SMTP Server (TLS) id 15.0.1473.3; Thu, 1 Oct 2020 08:04:28 +0000 Received: from NAM10-DM6-obe.outbound.protection.outlook.com (104.47.58.109) by HKMAIL102.nvidia.com (10.18.16.11) with Microsoft SMTP Server (TLS) id 15.0.1473.3 via Frontend Transport; Thu, 1 Oct 2020 08:04:28 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=PNcTueWEfzWNaCvC00HiT8/gFQgeOLj5uRXWg5W/hiZf2PWfUWznnCa6Jiscx+j9w2elnGXmglL07qpxN1mg2CNCVFKC6jLac0ffUadQhtMJ3WoE90nhux9dTgPQ6wycYs+S4Ii3GMpjChThFa1TcDdWaK4nOPaYXJAJ38fIgsRIygEdTmCVow4XnPgKIu1ZhxCs466j5sbHd2sY8CrdzuGU7DBpdfLZ0LtQHZ0+Vqh5fbMAom97WHjbAzC6wXk6TtiJZ7bifBPGr9yra1DJjcIBYDXnLkaBf2RSRM2naVhlzyZ5hKn3n1r3kvBCT83ud/GhUgx0hdQrbUOAbMg/Tg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=K67IdTLmKmUimUom0J6+DjATwft8Gia/Yn2770qNk3M=; b=Xg0+EIKd9GzesomJ0d2utToQchSPkTdIm4k3C+0vAfmr17kqTPUO8msndz7ybCTCfxN3UiV6ucvorOpIQUDP9a4sbo7j9c8k50VFwWMJZl/rcqH+dMfZNRvzcIt3sidXkJ8n1F9uf5phFPEKb1zV5obiFrTNirJEFjcGu8cJkjsNBJ0/hpTm6kjMWcm9d/bqc7e9lP/H9eBWcVAQHFupQHgbnHCHQNT7irzFi8MjmGGiiqhLvLqidvZDi9FQZEYskWPbDZ7zo7CH4zj8yAAwTE4qacfrhSL5848vNRHQm1jmxvVl0J9/km85PVag4mijR4/2TsrCyWmyhUBNa/RkAA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nvidia.com; dmarc=pass action=none header.from=nvidia.com; dkim=pass header.d=nvidia.com; arc=none Received: from MN2PR12MB4286.namprd12.prod.outlook.com (2603:10b6:208:199::22) by MN2PR12MB3711.namprd12.prod.outlook.com (2603:10b6:208:161::24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3412.20; Thu, 1 Oct 2020 08:04:26 +0000 Received: from MN2PR12MB4286.namprd12.prod.outlook.com ([fe80::61fd:a36e:cf4f:2d3f]) by MN2PR12MB4286.namprd12.prod.outlook.com ([fe80::61fd:a36e:cf4f:2d3f%8]) with mapi id 15.20.3433.035; Thu, 1 Oct 2020 08:04:26 +0000 From: Ori Kam To: Li Zhang , Dekel Peled , "Slava Ovsiienko" , Matan Azrad CC: "dev@dpdk.org" , NBU-Contact-Thomas Monjalon , Raslan Darawsheh Thread-Topic: [PATCH v2 1/1] net/mlx5: support match ICMP identifier fields Thread-Index: AQHWlUjWhs4WcyaHEUCExA7zgUYPJKmBN0QwgAAYkACAAAYFgIAA+6lg Date: Thu, 1 Oct 2020 08:04:25 +0000 Message-ID: References: <20200923023522.21135-1-lizh@nvidia.com> <20200928033813.22112-1-lizh@nvidia.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: nvidia.com; dkim=none (message not signed) header.d=none;nvidia.com; dmarc=none action=none header.from=nvidia.com; x-originating-ip: [147.236.152.129] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 2f94060e-b92c-4864-573e-08d865e09d5f x-ms-traffictypediagnostic: MN2PR12MB3711: x-ld-processed: 43083d15-7273-40c1-b7db-39efd9ccc17a,ExtAddr x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:7691; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: 0vMhMAvwgbgViHrtktw6EYPJDuoFQjZA6VuPX7HmB0wA+Bc7oGTYp5UWS6ixALMfHmMWFpFdVmaqLZCvUILgujdbNzk4SWXIVhVXqTpJ78nEkM4vtnGlou0DdxErH5RldDUoyg1RYG1JYe3sS6gGA1ig9JQMvZ0UCLjLf1KLNrCvGrLd6fwSe0Rt5sQF1u8FIdmrrvM9w7lvESF2WMWdirgrwWfAAGrHDR6B7Gccjc/I7de7sZEf9U+LLzvnkj8gJxPAUhAswLKnkv9F+Cju0R4JNYfAa9l51hmA1+gQI/50DNLXAd3vCGoHPSd5+aXgPF2D3SLYvXYaQ0wOg6fCfxKoY7VJhSlgMHYXNy3jmoi1AUnVZKj9pWVJO3FQQCeK x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MN2PR12MB4286.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(136003)(396003)(346002)(376002)(39860400002)(366004)(5660300002)(33656002)(71200400001)(66446008)(64756008)(86362001)(8936002)(66476007)(8676002)(76116006)(2906002)(316002)(83380400001)(66556008)(110136005)(4326008)(52536014)(7696005)(6506007)(66946007)(54906003)(186003)(26005)(6636002)(478600001)(107886003)(55016002)(9686003)(53546011); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata: MNXTBe1rRHue0jLENKjocricGGzKq0O1hrBJ5puBST5TJTNwNn58c3UvG3+yLkMW4jvNWBqQXe7yqWrMWa9lSX0dMggKqkcuTsjCi0VwarPd39RDI48DRWbOmJPmyqdguScP4R+rhznpjwkUG9fbsmX0oYA9bb5KVgyK12D5lX7UHjMX1BmtkJrKjrCLZkEXI76iwox6YZOAbEz47BgY+Rh27Qs+t++/m/vq5rDEs0M3mkVvI4Q85AJhTAYwfGSlrm8SJoxksq25XgCUT79PRYjoRA3hlGkFVP+LC4g7ncfjAWlhyLo+squjUvr882LKPUk6ZtaHNYwmMsBge7gwxYjXnoQTzt1+J4GaDCylyWAm9euRSGdbhSyK58vKIAt851MiOkNTj5QgJRVjy/vT9mBHXs0e3q4akPhEXYYcnbt5WfJNHjynTBh945mbcz1o+soj0O/ofev+WzKH8Wk5d+QenM5fqcFhxl1zTeduQbuklJvpGNkFveYGMEyK6MHNdjOKJvgMiAyEAWGwSDsbZT3z1UVOIsA8XxWiV1FwINflScj4LT8iCPGgcg9Epjgt3cz9Mer+G50uBdXnn68xag5w/1Aa1gF2Sv8X7R2v9em7QPrAfeGWkcIgJMTyGzinNG+OU+X0QqqCfWhZ+659VQ== Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: MN2PR12MB4286.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 2f94060e-b92c-4864-573e-08d865e09d5f X-MS-Exchange-CrossTenant-originalarrivaltime: 01 Oct 2020 08:04:25.8444 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 43083d15-7273-40c1-b7db-39efd9ccc17a X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: ZiVwwCCJC7jvRukCY603XkYrD1PLiVQCxLAx2GznN1PT5Cy5t536IDdDickGwpoAzDis6Yqg+9MpqkYa+9PHNQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR12MB3711 X-OriginatorOrg: Nvidia.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1; t=1601539468; bh=K67IdTLmKmUimUom0J6+DjATwft8Gia/Yn2770qNk3M=; h=ARC-Seal:ARC-Message-Signature:ARC-Authentication-Results:From:To: CC:Subject:Thread-Topic:Thread-Index:Date:Message-ID:References: In-Reply-To:Accept-Language:Content-Language:X-MS-Has-Attach: X-MS-TNEF-Correlator:authentication-results:x-originating-ip: x-ms-publictraffictype:x-ms-office365-filtering-correlation-id: x-ms-traffictypediagnostic:x-ld-processed: x-ms-exchange-transport-forked:x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers:x-ms-exchange-senderadcheck: x-microsoft-antispam:x-microsoft-antispam-message-info: x-forefront-antispam-report:x-ms-exchange-antispam-messagedata: Content-Type:Content-Transfer-Encoding:MIME-Version: X-MS-Exchange-CrossTenant-AuthAs: X-MS-Exchange-CrossTenant-AuthSource: X-MS-Exchange-CrossTenant-Network-Message-Id: X-MS-Exchange-CrossTenant-originalarrivaltime: X-MS-Exchange-CrossTenant-fromentityheader: X-MS-Exchange-CrossTenant-id:X-MS-Exchange-CrossTenant-mailboxtype: X-MS-Exchange-CrossTenant-userprincipalname: X-MS-Exchange-Transport-CrossTenantHeadersStamped:X-OriginatorOrg; b=KItdCFV4jjI6ekASlDNzqxGBjVt8O86d9DZKVlYKPVYi3Ciz0EcDP/qYOQ84BMq95 GWjrmTxC1xejFFZvPFaXNEOBi1RkqZMN3BdybgAzutoU5CNVZtQ8l6BvQF3L8SldWa pIJWf9Zklpyqdz2VD36FsbVRVFmjdkHkKn0YRTycwcmK5E0RNjFHf2x6qkwn9ZxBQM Og+z9oCX6Qo5LTqWd0Z31IWMYjc6j7jFYmOtiQ96rtYOhMnSJOueifPbqWLhls1aLy vWUhLfR2ZAvxxa4TJUdxKIxgYR0WyzjcVQxS/84BX36C6jQRdUNmSva73iQHdA3GjT gkCwFiio6/TuQ== Subject: Re: [dpdk-dev] [PATCH v2 1/1] net/mlx5: support match ICMP identifier fields X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi,=20 PSB I also addressed other response in this mail to avoid threading of this= mail. Best, Ori > -----Original Message----- > From: Li Zhang > Sent: Wednesday, September 30, 2020 6:42 PM > Subject: RE: [PATCH v2 1/1] net/mlx5: support match ICMP identifier field= s >=20 > Hi Ori, >=20 > Update my reply about split mlx5_flow_validate_item_icmp function inline >=20 > Regards, > Li Zhang > > -----Original Message----- > > From: Li Zhang > > Sent: Wednesday, September 30, 2020 11:20 PM > > Subject: RE: [PATCH v2 1/1] net/mlx5: support match ICMP identifier fie= lds > > > > Hi Ori, > > > > Thanks for your comments, please take a look my reply inline. > > > > Regards, > > Li Zhang > > > -----Original Message----- > > > From: Ori Kam > > > Sent: Wednesday, September 30, 2020 10:46 PM > > > To: Li Zhang ; Dekel Peled ; Slav= a > > > Ovsiienko ; Matan Azrad > > > Cc: dev@dpdk.org; NBU-Contact-Thomas Monjalon > > ; > > > Raslan Darawsheh > > > Subject: RE: [PATCH v2 1/1] net/mlx5: support match ICMP identifier > > > fields > > > > > > Hi Li, > > > > > > PSB, > > > > > > Thanks, > > > Ori > > > > > > > -----Original Message----- > > > > From: Li Zhang > > > > Sent: Monday, September 28, 2020 6:38 AM > > > > Subject: [PATCH v2 1/1] net/mlx5: support match ICMP identifier > > > > fields > > > > > > > > PRM expose fields "Icmp_header_data" in 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 s= et. > > > > > > > Is it relevant only for ipv4 ICMP? > > Yes it is only for ipv4 ICMP. For Ipv6 ICMP, it is icmpv6_header_data. Thanks. > > > > > > > Signed-off-by: Li Zhang > > > > --- > > > > doc/guides/nics/mlx5.rst | 4 ++-- > > > > doc/guides/rel_notes/release_20_11.rst | 2 +- > > > > drivers/net/mlx5/mlx5_flow.c | 9 +++++++-- > > > > drivers/net/mlx5/mlx5_flow_dv.c | 16 +++++++++++++++- > > > > 4 files changed, 25 insertions(+), 6 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 rem= oved. > > > > > > > > -- 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=3D1 > > > > > > > > -- enable ICMP/ICMP6 code/type fields matching:: > > > > +- enable ICMP(code/type/identifier/sequence number) / > > > > +ICMP6(code/type) > > > > fields matching:: > > > > > > > > FLEX_PARSER_PROFILE_ENABLE=3D2 > > > > > > > > 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=3DN`` > > > > 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..7bd5c5da94 100644 > > > > --- a/drivers/net/mlx5/mlx5_flow.c > > > > +++ b/drivers/net/mlx5/mlx5_flow.c > > > > @@ -1303,6 +1303,7 @@ mlx5_flow_validate_item_icmp(const struct > > > > rte_flow_item *item, > > > > struct rte_flow_error *error) > > > > > > This function is shared between the dv and the verbs. > > > I think we can support this only in dv, so we need to split this func= tion. > > Thanks, I will split this function in v3 patch. > The mlx5_flow_validate_item_icmp is only called by dv now, also it does n= ot > need split it since verbs not use it. Strange so why is it in shared place and not DV but never mind. > > > > > > > { > > > > const struct rte_flow_item_icmp *mask =3D item->mask; > > > > + struct rte_flow_item_icmp default_mask; > > > > > > I think the correct name is nic_mask. > > Thanks, I will change in v3 patch. > > > > > > > const int tunnel =3D !!(item_flags & MLX5_FLOW_LAYER_TUNNEL); > > > > const uint64_t l3m =3D tunnel ? MLX5_FLOW_LAYER_INNER_L3_IPV4 : > > > > MLX5_FLOW_LAYER_OUTER_L3_IPV4; > > > @@ -1324,11 +1325,15 @@ > > > > mlx5_flow_validate_item_icmp(const struct rte_flow_item *item, > > > > return rte_flow_error_set(error, EINVAL, > > > > RTE_FLOW_ERROR_TYPE_ITEM, > > > > item, > > > > "multiple L4 layers not supported"); > > > > + memcpy(&default_mask, &rte_flow_item_icmp_mask, > > > > + sizeof(struct rte_flow_item_icmp)); > > > > + default_mask.hdr.icmp_ident =3D RTE_BE16(0xFFFF); > > > > + default_mask.hdr.icmp_seq_nb =3D RTE_BE16(0xFFFF); > > > > > > You don't need to mem copy just init this structure when you declare = it. > > Thanks, I will change in v3 patch. > > > > > > > if (!mask) > > > > - mask =3D &rte_flow_item_icmp_mask; > > > > + mask =3D &default_mask; > > > > ret =3D mlx5_flow_item_acceptable > > > > (item, (const uint8_t *)mask, > > > > - (const uint8_t *)&rte_flow_item_icmp_mask, > > > > + (const uint8_t *)&default_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..b5d6455067 > > > 100644 > > > > --- a/drivers/net/mlx5/mlx5_flow_dv.c > > > > +++ b/drivers/net/mlx5/mlx5_flow_dv.c > > > > @@ -7378,6 +7378,8 @@ flow_dv_translate_item_icmp(void *matcher, > > > void > > > > *key, { > > > > const struct rte_flow_item_icmp *icmp_m =3D item->mask; > > > > const struct rte_flow_item_icmp *icmp_v =3D item->spec; > > > > + uint32_t icmp_header_data_m =3D 0; > > > > + uint32_t icmp_header_data_v =3D 0; > > > > void *headers_m; > > > > void *headers_v; > > > > void *misc3_m =3D MLX5_ADDR_OF(fte_match_param, matcher, @@ - > > > 7396,8 > > > > +7398,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) > > > > + if (!icmp_m) { > > > > icmp_m =3D &rte_flow_item_icmp_mask; > > > > + icmp_header_data_m =3D RTE_BE32(UINT32_MAX); > > > > > > Why are you setting the data as default mask? > > > > > Because icmp_code and icmp_type still need the default mask. > > We do not need to change the old one. > > We just add the new one for our new match fields. Yes but there is no need to add new fields to the mask, it will break exist= ing applications. So please remove it. > > > > + } else { > > > > + icmp_header_data_m =3D rte_cpu_to_be_16(icmp_m- > > > > >hdr.icmp_seq_nb); > > > > > > Isn't the icmp_seq_nb already in BE? > > No, it is not BE and need use rte_cpu_to_be_16 > > > > > > > + icmp_header_data_m |=3D > > > > + rte_cpu_to_be_16(icmp_m->hdr.icmp_ident) << 16; > > > > > > Again the icmp_ident is already in BE > > > > > It is not BE and need use rte_cpu_to_be_16 Why is it not BE? From the structure definition: struct rte_icmp_hdr { uint8_t icmp_type; /* ICMP packet type. */ uint8_t icmp_code; /* ICMP packet code. */ rte_be16_t icmp_cksum; /* ICMP packet checksum. */ rte_be16_t icmp_ident; /* ICMP packet identifier. */ rte_be16_t icmp_seq_nb; /* ICMP packet sequence number. */ } __rte_packed; Also you are setting cpu value with BE. Maybe you want to rte_be_to_cpu? > > > > + } > > > > /* > > > > * 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 +7420,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 =3D rte_cpu_to_be_16(icmp_v- > > > >hdr.icmp_seq_nb); > > > > + icmp_header_data_v |=3D rte_cpu_to_be_16(icmp_v- > > >hdr.icmp_ident) > > > > << 16; > > > > > > The BE issue again, > > It is not BE and need use rte_cpu_to_be_16 See above comment. > > > > + 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