From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR01-DB5-obe.outbound.protection.outlook.com (mail-db5eur01on0070.outbound.protection.outlook.com [104.47.2.70]) by dpdk.org (Postfix) with ESMTP id 6708E2952 for ; Wed, 3 Oct 2018 07:22:02 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=SDTgH1kbbDZLYRAWJb29BTbpDcB2PwcqPV2EjhHrV5I=; b=oLOoLGc74YyXeDqV4LfRVdb3jaO/aNv96+g+Lq6oZ2GWIiWpxCQsJumdNzpYdu/Y6988BEt8pXUW01m9FzFc4dYqdJi9ngfTbEUH+sSA24z24xO4zUqkMp9urHhjTLCLNVDnCTxcoFwmSMoe+vujnMXQOr7Tgmk6boUTzRX2faE= Received: from VI1PR05MB4224.eurprd05.prod.outlook.com (52.133.12.13) by VI1PR05MB3517.eurprd05.prod.outlook.com (10.175.242.139) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1207.18; Wed, 3 Oct 2018 05:22:00 +0000 Received: from VI1PR05MB4224.eurprd05.prod.outlook.com ([fe80::f911:fe8:927d:400b]) by VI1PR05MB4224.eurprd05.prod.outlook.com ([fe80::f911:fe8:927d:400b%3]) with mapi id 15.20.1207.018; Wed, 3 Oct 2018 05:22:00 +0000 From: Dekel Peled To: Yongseok Koh CC: "dev@dpdk.org" , Shahaf Shuler , Ori Kam Thread-Topic: [PATCH v2] net/mlx5: support metadata as flow rule criteria Thread-Index: AQHUV9QZKIjH5dYi806iVa+arUtzIaULyK5A Date: Wed, 3 Oct 2018 05:22:00 +0000 Message-ID: References: <1537105328-9367-1-git-send-email-dekelp@mellanox.com> <1538057935-34663-1-git-send-email-dekelp@mellanox.com> <20180929090906.GA95127@minint-98vp2qg> In-Reply-To: <20180929090906.GA95127@minint-98vp2qg> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=dekelp@mellanox.com; x-originating-ip: [193.47.165.251] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; VI1PR05MB3517; 6:2p5scjABoK/kgWADY5oJXAACYiXLGiBK3qsYse1Zy6zegRgNBxMimK1C20XSlTtM5yqusQgKsZZ1883qJ4AYOZkD0BLLsIN/7QelyBqx35cGZDNWQKZxAUSaE7rOWjtENGAFV2KZql+enLM6Sl49c0OoqIWfL//6JYGFTTM/pYYVLNlhG2xXWSRx/pMVaL3l7fX9ihwaunQZQ3lH0qOK+oesGrIb81bdNdLbz21/J5zQZZBmA0YcCXKppCmzS6gVrKsBcc0+Jb5md33/vnAnEJIpY7cZCDHELjP5RZcHnVZgfs7fq92yibcvh/OHkUZgNKPRD9gS1JALcOCFVMUBXMOXIU8Ij72PkNKusG8NLIsg1M5+0eLTjaTC01RVsX16/GrVdIrLLX9izus+IeNxtLp27njgzRk4JxInATLE4rgTWbipVhJBqdwEPzmCjDkJv9+sLIhrBvQZW9Hoi7cL1w==; 5:yz5YmWiRGGNciBF94mk/Po5J9E4D/rekFYiLE99MImztoUz4blC5yLMh0yFaexcTILDGfzv3CLT5yUTV1bLbLPbFTeoNMZSKuA5EjnNRh+HSuZZk+hPRVe8tvsqh9ImAQS74w6apstblynsxyPwwEVTAMDWj0Eu9cNU44cRedZg=; 7:pz6oQJaQR8c9dQYOCFRWQF55kXtT988NRjbTylCH5lra/gvk1gfzQmCQWs+9FSt++8Geg5Pzz0QiCgZZqTg8JA4DMRb2R1lRvuw9zyJW8ZJNR1W7UUwuQ9zKNOPXjE0ZvGa/dKK0OBUvVKCoRJWHxC+J765+jO8i+vlXoz5uqFrN/Qsr/EQIMM1nw/p58vCw1pkRYrSnfRkTPIE4B+X4goAtNR8dGsXjwRBRZ2kZTSFLuIm8uqGgLtlSLJvpqkSp x-ms-exchange-antispam-srfa-diagnostics: SOS; x-ms-office365-filtering-correlation-id: f56506d4-ff9d-4bb9-a539-08d628f0259d x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0; PCL:0; RULEID:(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600074)(711020)(4618075)(2017052603328)(7153060)(7193020); SRVR:VI1PR05MB3517; x-ms-traffictypediagnostic: VI1PR05MB3517: x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(788757137089); x-ms-exchange-senderadcheck: 1 x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(6040522)(2401047)(5005006)(8121501046)(3231355)(944501410)(4982022)(52105095)(3002001)(10201501046)(93006095)(93001095)(6055026)(149066)(150057)(6041310)(20161123560045)(20161123558120)(20161123562045)(20161123564045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(201708071742011)(7699051); SRVR:VI1PR05MB3517; BCL:0; PCL:0; RULEID:; SRVR:VI1PR05MB3517; x-forefront-prvs: 0814A2C7A3 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(39860400002)(366004)(346002)(376002)(396003)(136003)(199004)(189003)(13464003)(71200400001)(71190400001)(81166006)(106356001)(68736007)(105586002)(3846002)(2906002)(6116002)(561944003)(14454004)(33656002)(478600001)(966005)(25786009)(2900100001)(107886003)(54906003)(5250100002)(305945005)(74316002)(7736002)(97736004)(6862004)(6246003)(9686003)(6306002)(6436002)(55016002)(316002)(53936002)(53946003)(4326008)(229853002)(14444005)(76176011)(53546011)(476003)(5660300001)(6506007)(256004)(11346002)(446003)(7696005)(26005)(81156014)(6636002)(186003)(102836004)(8676002)(486006)(86362001)(66066001)(8936002)(4744004)(99286004)(579004); DIR:OUT; SFP:1101; SCL:1; SRVR:VI1PR05MB3517; H:VI1PR05MB4224.eurprd05.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1; received-spf: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: GQ8Mj2T5dFVBDa/CPlQc+R7U1Nae20dOqQN3V7H2qLX5wxDI6SeLv/gnmDog2Mx+BLKRpxHJ/CmX7Vza5CbDdRCPUp7ITpEH4Nv6Ei+XbjAmhu24T5HKbksujnpxxl21i1pf8I8R/EotoHkwRA3xHzrtBGHNIx6hS5iJkYcgJ0FhXZ59gKdmxHxfGY5wmORKV80r+V0iXgv96kgxmauAXR9GZrZSsxxq8kvNuxq+n2EFEjuuu10mutqwxr527DwfVzGpZSvSakusB1IFuCHI34PR5M4dmrlVNbXVFN2gp/npJlFhjQbAHZX1oBmYCbFd1kFzPauQxzDZVY6msyby3rj+MFCIPXZNDu6x9W+T36Y= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: f56506d4-ff9d-4bb9-a539-08d628f0259d X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Oct 2018 05:22:00.6604 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR05MB3517 Subject: Re: [dpdk-dev] [PATCH v2] net/mlx5: support metadata as flow rule criteria 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: , X-List-Received-Date: Wed, 03 Oct 2018 05:22:02 -0000 Thanks, PSB. > -----Original Message----- > From: Yongseok Koh > Sent: Saturday, September 29, 2018 12:09 PM > To: Dekel Peled > Cc: dev@dpdk.org; Shahaf Shuler ; Ori Kam > > Subject: Re: [PATCH v2] net/mlx5: support metadata as flow rule criteria >=20 > On Thu, Sep 27, 2018 at 05:18:55PM +0300, Dekel Peled wrote: > > As described in series starting at [1], it adds option to set metadata > > value as match pattern when creating a new flow rule. > > > > This patch adds metadata support in mlx5 driver, in two parts: > > - Add the setting of metadata value in matcher when creating > > a new flow rule. > > - Add the passing of metadata value from mbuf to wqe when > > indicated by ol_flag, in different burst functions. > > > > [1] "ethdev: support metadata as flow rule criteria" > > http://mails.dpdk.org/archives/dev/2018-September/113270.html > > > > Signed-off-by: Dekel Peled > > --- > > V2: > > Split the support of egress rules to a different patch. > > --- > > drivers/net/mlx5/mlx5_flow.c | 2 +- > > drivers/net/mlx5/mlx5_flow.h | 8 +++ > > drivers/net/mlx5/mlx5_flow_dv.c | 96 > +++++++++++++++++++++++++++++++++++ > > drivers/net/mlx5/mlx5_prm.h | 2 +- > > drivers/net/mlx5/mlx5_rxtx.c | 35 +++++++++++-- > > drivers/net/mlx5/mlx5_rxtx_vec.c | 31 ++++++++--- > > drivers/net/mlx5/mlx5_rxtx_vec.h | 1 + > > drivers/net/mlx5/mlx5_rxtx_vec_neon.h | 4 +- > > drivers/net/mlx5/mlx5_rxtx_vec_sse.h | 4 +- > > drivers/net/mlx5/mlx5_txq.c | 6 +++ > > 10 files changed, 174 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/net/mlx5/mlx5_flow.c > > b/drivers/net/mlx5/mlx5_flow.c index 8007bf1..9581691 100644 > > --- a/drivers/net/mlx5/mlx5_flow.c > > +++ b/drivers/net/mlx5/mlx5_flow.c > > @@ -417,7 +417,7 @@ uint32_t mlx5_flow_adjust_priority(struct > rte_eth_dev *dev, int32_t priority, > > * @return > > * 0 on success, a negative errno value otherwise and rte_errno is s= et. > > */ > > -static int > > +int > > mlx5_flow_item_acceptable(const struct rte_flow_item *item, > > const uint8_t *mask, > > const uint8_t *nic_mask, > > diff --git a/drivers/net/mlx5/mlx5_flow.h > > b/drivers/net/mlx5/mlx5_flow.h index 10d700a..d91ae17 100644 > > --- a/drivers/net/mlx5/mlx5_flow.h > > +++ b/drivers/net/mlx5/mlx5_flow.h > > @@ -42,6 +42,9 @@ > > #define MLX5_FLOW_LAYER_GRE (1u << 14) #define > MLX5_FLOW_LAYER_MPLS > > (1u << 15) > > > > +/* General pattern items bits. */ > > +#define MLX5_FLOW_ITEM_METADATA (1u << 16) > > + > > /* Outer Masks. */ > > #define MLX5_FLOW_LAYER_OUTER_L3 \ > > (MLX5_FLOW_LAYER_OUTER_L3_IPV4 | > MLX5_FLOW_LAYER_OUTER_L3_IPV6) @@ > > -299,6 +302,11 @@ int mlx5_flow_validate_action_rss(const struct > > rte_flow_action *action, int mlx5_flow_validate_attributes(struct > rte_eth_dev *dev, > > const struct rte_flow_attr *attributes, > > struct rte_flow_error *error); > > +int mlx5_flow_item_acceptable(const struct rte_flow_item *item, > > + const uint8_t *mask, > > + const uint8_t *nic_mask, > > + unsigned int size, > > + struct rte_flow_error *error); > > int mlx5_flow_validate_item_eth(const struct rte_flow_item *item, > > uint64_t item_flags, > > struct rte_flow_error *error); > > diff --git a/drivers/net/mlx5/mlx5_flow_dv.c > > b/drivers/net/mlx5/mlx5_flow_dv.c index cf663cd..2439f5e 100644 > > --- a/drivers/net/mlx5/mlx5_flow_dv.c > > +++ b/drivers/net/mlx5/mlx5_flow_dv.c > > @@ -36,6 +36,55 @@ > > #ifdef HAVE_IBV_FLOW_DV_SUPPORT > > > > /** > > + * Validate META item. > > + * > > + * @param[in] item > > + * Item specification. > > + * @param[in] attr > > + * Attributes of flow that includes this item. > > + * @param[out] error > > + * Pointer to error structure. > > + * > > + * @return > > + * 0 on success, a negative errno value otherwise and rte_errno is s= et. > > + */ > > +static int > > +mlx5_flow_validate_item_meta(const struct rte_flow_item *item, >=20 > Naming rule here is starting from flow_dv_*() for static funcs. For globa= l > funcs, it should start from mlx5_, so it should be mlx5_flow_dv_*(). Renamed function to flow_dv_validate_item_meta. Left it as static in mlx5_flow_dv.c, since it is relevant for DV only. >=20 > Or, you can put it in the mlx5_flow.c as a common helper function althoug= h it > is used only by DV. >=20 > I prefer the latter. >=20 > > + const struct rte_flow_attr *attr, > > + struct rte_flow_error *error) > > +{ > > + const struct rte_flow_item_meta *spec =3D item->spec; > > + const struct rte_flow_item_meta *mask =3D item->mask; > > + > > + const struct rte_flow_item_meta nic_mask =3D { > > + .data =3D RTE_BE32(UINT32_MAX) > > + }; > > + > > + int ret; > > + > > + if (!spec) > > + return rte_flow_error_set(error, EINVAL, > > + > RTE_FLOW_ERROR_TYPE_ITEM_SPEC, > > + item->spec, > > + "data cannot be empty"); > > + if (!mask) > > + mask =3D &rte_flow_item_meta_mask; > > + ret =3D mlx5_flow_item_acceptable(item, (const uint8_t *)mask, > > + (const uint8_t *)&nic_mask, > > + sizeof(struct rte_flow_item_meta), > > + error); > > + if (ret < 0) > > + return ret; > > + >=20 > No blank line is allowed. Blank line removed. >=20 > > + if (attr->ingress) > > + return rte_flow_error_set(error, ENOTSUP, > > + > RTE_FLOW_ERROR_TYPE_ATTR_INGRESS, > > + NULL, > > + "pattern not supported for > ingress"); >=20 > If it is only supported with egress flow, then please group this patch wi= th the > other one - "net/mlx5: allow flow rule with attribute egress", and make i= t > applied later than that by specifying [1/2] and [2/2]. I will update the commit log with the relevant data as done=20 in http://mails.dpdk.org/archives/dev/2018-September/113280.html >=20 > > + return 0; > > +} > > + > > +/** > > * Verify the @p attributes will be correctly understood by the NIC an= d > store > > * them in the @p flow if everything is correct. > > * > > @@ -216,6 +265,12 @@ > > return ret; > > item_flags |=3D MLX5_FLOW_LAYER_MPLS; > > break; > > + case RTE_FLOW_ITEM_TYPE_META: > > + ret =3D mlx5_flow_validate_item_meta(items, attr, > error); > > + if (ret < 0) > > + return ret; > > + item_flags |=3D MLX5_FLOW_ITEM_METADATA; > > + break; > > default: > > return rte_flow_error_set(error, ENOTSUP, > > > RTE_FLOW_ERROR_TYPE_ITEM, > > @@ -853,6 +908,43 @@ > > } > > > > /** > > + * Add META item to matcher > > + * > > + * @param[in, out] matcher > > + * Flow matcher. > > + * @param[in, out] key > > + * Flow matcher value. > > + * @param[in] item > > + * Flow pattern to translate. > > + * @param[in] inner > > + * Item is inner pattern. > > + */ > > +static void > > +flow_dv_translate_item_meta(void *matcher, void *key, > > + const struct rte_flow_item *item) { > > + const struct rte_flow_item_meta *metam; > > + const struct rte_flow_item_meta *metav; >=20 > Ori changed naming like meta_m and meta_v. Renamed. >=20 > > + void *misc2_m =3D > > + MLX5_ADDR_OF(fte_match_param, matcher, > misc_parameters_2); > > + void *misc2_v =3D > > + MLX5_ADDR_OF(fte_match_param, key, > misc_parameters_2); > > + > > + metam =3D (const void *)item->mask; > > + if (!metam) > > + metam =3D &rte_flow_item_meta_mask; > > + >=20 > No blank line is allowed except for the one after variable declaration. P= lease > remove such blank lines in the whole patches. Done. >=20 > > + metav =3D (const void *)item->spec; > > + if (metav) { > > + MLX5_SET(fte_match_set_misc2, misc2_m, > metadata_reg_a, > > + RTE_BE32(metam->data)); > > + MLX5_SET(fte_match_set_misc2, misc2_v, metadata_reg_a, > > + RTE_BE32(metav->data)); > > + } > > +} > > + > > +/** > > * Update the matcher and the value based the selected item. > > * > > * @param[in, out] matcher > > @@ -938,6 +1030,10 @@ > > flow_dv_translate_item_vxlan(tmatcher->mask.buf, key, > item, > > inner); > > break; > > + case RTE_FLOW_ITEM_TYPE_META: > > + flow_dv_translate_item_meta(tmatcher->mask.buf, key, > item); > > + break; > > + > > default: > > break; > > } > > diff --git a/drivers/net/mlx5/mlx5_prm.h b/drivers/net/mlx5/mlx5_prm.h > > index 4e2f9f4..a905397 100644 > > --- a/drivers/net/mlx5/mlx5_prm.h > > +++ b/drivers/net/mlx5/mlx5_prm.h > > @@ -159,7 +159,7 @@ struct mlx5_wqe_eth_seg_small { > > uint8_t cs_flags; > > uint8_t rsvd1; > > uint16_t mss; > > - uint32_t rsvd2; > > + uint32_t flow_table_metadata; > > uint16_t inline_hdr_sz; > > uint8_t inline_hdr[2]; > > } __rte_aligned(MLX5_WQE_DWORD_SIZE); > > diff --git a/drivers/net/mlx5/mlx5_rxtx.c > > b/drivers/net/mlx5/mlx5_rxtx.c index 558e6b6..d596e4e 100644 > > --- a/drivers/net/mlx5/mlx5_rxtx.c > > +++ b/drivers/net/mlx5/mlx5_rxtx.c > > @@ -523,6 +523,7 @@ > > uint8_t tso =3D txq->tso_en && (buf->ol_flags & > PKT_TX_TCP_SEG); > > uint32_t swp_offsets =3D 0; > > uint8_t swp_types =3D 0; > > + uint32_t metadata =3D 0; > > uint16_t tso_segsz =3D 0; > > #ifdef MLX5_PMD_SOFT_COUNTERS > > uint32_t total_length =3D 0; > > @@ -566,6 +567,9 @@ > > cs_flags =3D txq_ol_cksum_to_cs(buf); > > txq_mbuf_to_swp(txq, buf, (uint8_t *)&swp_offsets, > &swp_types); > > raw =3D ((uint8_t *)(uintptr_t)wqe) + 2 * > MLX5_WQE_DWORD_SIZE; > > + /* Copy metadata from mbuf if valid */ > > + if (buf->ol_flags & PKT_TX_METADATA) > > + metadata =3D buf->hash.fdir.hi; >=20 > I saw someone suggested to add a union field to name it properly. I also > agree on the idea. It is quite confusing to get meta data from hash field= . Done, added field tx_metadata in union mbuf.hash. >=20 > > /* Replace the Ethernet type by the VLAN if necessary. */ > > if (buf->ol_flags & PKT_TX_VLAN_PKT) { > > uint32_t vlan =3D rte_cpu_to_be_32(0x81000000 | @@ > -781,7 +785,7 @@ > > swp_offsets, > > cs_flags | (swp_types << 8) | > > (rte_cpu_to_be_16(tso_segsz) << 16), > > - 0, > > + rte_cpu_to_be_32(metadata), > > (ehdr << 16) | > rte_cpu_to_be_16(tso_header_sz), > > }; > > } else { > > @@ -795,7 +799,7 @@ > > wqe->eseg =3D (rte_v128u32_t){ > > swp_offsets, > > cs_flags | (swp_types << 8), > > - 0, > > + rte_cpu_to_be_32(metadata), > > (ehdr << 16) | > rte_cpu_to_be_16(pkt_inline_sz), > > }; > > } > > @@ -861,7 +865,7 @@ > > mpw->wqe->eseg.inline_hdr_sz =3D 0; > > mpw->wqe->eseg.rsvd0 =3D 0; > > mpw->wqe->eseg.rsvd1 =3D 0; > > - mpw->wqe->eseg.rsvd2 =3D 0; > > + mpw->wqe->eseg.flow_table_metadata =3D 0; > > mpw->wqe->ctrl[0] =3D rte_cpu_to_be_32((MLX5_OPC_MOD_MPW > << 24) | > > (txq->wqe_ci << 8) | > > MLX5_OPCODE_TSO); > > @@ -971,6 +975,8 @@ > > if ((mpw.state =3D=3D MLX5_MPW_STATE_OPENED) && > > ((mpw.len !=3D length) || > > (segs_n !=3D 1) || > > + (mpw.wqe->eseg.flow_table_metadata !=3D > > + rte_cpu_to_be_32(buf->hash.fdir.hi)) || > > (mpw.wqe->eseg.cs_flags !=3D cs_flags))) > > mlx5_mpw_close(txq, &mpw); > > if (mpw.state =3D=3D MLX5_MPW_STATE_CLOSED) { @@ -984,6 > +990,8 @@ > > max_wqe -=3D 2; > > mlx5_mpw_new(txq, &mpw, length); > > mpw.wqe->eseg.cs_flags =3D cs_flags; > > + mpw.wqe->eseg.flow_table_metadata =3D > > + rte_cpu_to_be_32(buf->hash.fdir.hi); > > } > > /* Multi-segment packets must be alone in their MPW. */ > > assert((segs_n =3D=3D 1) || (mpw.pkts_n =3D=3D 0)); @@ -1082,7 > +1090,7 @@ > > mpw->wqe->eseg.cs_flags =3D 0; > > mpw->wqe->eseg.rsvd0 =3D 0; > > mpw->wqe->eseg.rsvd1 =3D 0; > > - mpw->wqe->eseg.rsvd2 =3D 0; > > + mpw->wqe->eseg.flow_table_metadata =3D 0; > > inl =3D (struct mlx5_wqe_inl_small *) > > (((uintptr_t)mpw->wqe) + 2 * MLX5_WQE_DWORD_SIZE); > > mpw->data.raw =3D (uint8_t *)&inl->raw; @@ -1199,12 +1207,16 @@ > > if (mpw.state =3D=3D MLX5_MPW_STATE_OPENED) { > > if ((mpw.len !=3D length) || > > (segs_n !=3D 1) || > > + (mpw.wqe->eseg.flow_table_metadata !=3D > > + rte_cpu_to_be_32(buf->hash.fdir.hi)) || > > (mpw.wqe->eseg.cs_flags !=3D cs_flags)) > > mlx5_mpw_close(txq, &mpw); > > } else if (mpw.state =3D=3D MLX5_MPW_INL_STATE_OPENED) { > > if ((mpw.len !=3D length) || > > (segs_n !=3D 1) || > > (length > inline_room) || > > + (mpw.wqe->eseg.flow_table_metadata !=3D > > + rte_cpu_to_be_32(buf->hash.fdir.hi)) || >=20 > Isn't this mbuf field vaild only if there's PKT_TX_METADATA? Without the > flag, it could be a garbage, right? And I think the value in the eseg mig= ht not > be metadata of previous packet if the packet didn't have the flag. It wou= ld be > better to define metadata and get it early in this loop like cs_flags. E.= g. >=20 > metadata =3D buf->ol_flags & PKT_TX_METADATA ? > rte_cpu_to_be_32(buf->hash.fdir.hi) : 0; > cs_flags =3D txq_ol_cksum_to_cs(buf); >=20 > Same for the rest of similar funcs. Done. >=20 > > (mpw.wqe->eseg.cs_flags !=3D cs_flags)) { > > mlx5_mpw_inline_close(txq, &mpw); > > inline_room =3D > > @@ -1224,12 +1236,21 @@ > > max_wqe -=3D 2; > > mlx5_mpw_new(txq, &mpw, length); > > mpw.wqe->eseg.cs_flags =3D cs_flags; > > + /* Copy metadata from mbuf if valid */ > > + if (buf->ol_flags & PKT_TX_METADATA) > > + mpw.wqe- > >eseg.flow_table_metadata =3D > > + rte_cpu_to_be_32(buf- > >hash.fdir.hi); > > + >=20 > Yes, this can be allowed but the following is preferred. OK. > mpw.wqe- > >eseg.flow_table_metadata =3D > rte_cpu_to_be_32 > (buf->hash.fdir.hi); > > } else { > > if (unlikely(max_wqe < wqe_inl_n)) > > break; > > max_wqe -=3D wqe_inl_n; > > mlx5_mpw_inline_new(txq, &mpw, length); > > mpw.wqe->eseg.cs_flags =3D cs_flags; > > + /* Copy metadata from mbuf if valid */ > > + if (buf->ol_flags & PKT_TX_METADATA) > > + mpw.wqe- > >eseg.flow_table_metadata =3D > > + rte_cpu_to_be_32(buf- > >hash.fdir.hi); >=20 > Same here. >=20 > > } > > } > > /* Multi-segment packets must be alone in their MPW. */ > @@ -1482,6 > > +1503,8 @@ > > (length <=3D txq->inline_max_packet_sz && > > inl_pad + sizeof(inl_hdr) + length > > > mpw_room) || > > + (mpw.wqe->eseg.flow_table_metadata !=3D > > + rte_cpu_to_be_32(buf->hash.fdir.hi)) || > > (mpw.wqe->eseg.cs_flags !=3D cs_flags)) > > max_wqe -=3D mlx5_empw_close(txq, &mpw); > > } > > @@ -1505,6 +1528,10 @@ > > sizeof(inl_hdr) + length <=3D mpw_room && > > !txq->mpw_hdr_dseg; > > mpw.wqe->eseg.cs_flags =3D cs_flags; > > + /* Copy metadata from mbuf if valid */ > > + if (buf->ol_flags & PKT_TX_METADATA) > > + mpw.wqe->eseg.flow_table_metadata =3D > > + rte_cpu_to_be_32(buf- > >hash.fdir.hi); > > } else { > > /* Evaluate whether the next packet can be inlined. > > * Inlininig is possible when: > > diff --git a/drivers/net/mlx5/mlx5_rxtx_vec.c > > b/drivers/net/mlx5/mlx5_rxtx_vec.c > > index 0a4aed8..132943a 100644 > > --- a/drivers/net/mlx5/mlx5_rxtx_vec.c > > +++ b/drivers/net/mlx5/mlx5_rxtx_vec.c > > @@ -41,6 +41,8 @@ > > > > /** > > * Count the number of packets having same ol_flags and calculate > cs_flags. > > + * If PKT_TX_METADATA is set in ol_flags, packets must have same > > + metadata > > + * as well. > > * > > * @param pkts > > * Pointer to array of packets. > > @@ -48,25 +50,38 @@ > > * Number of packets. > > * @param cs_flags > > * Pointer of flags to be returned. > > + * @param txq_offloads > > + * Offloads enabled on Tx queue > > * > > * @return > > - * Number of packets having same ol_flags. > > + * Number of packets having same ol_flags and metadata, if relevant. > > */ > > static inline unsigned int > > -txq_calc_offload(struct rte_mbuf **pkts, uint16_t pkts_n, uint8_t > > *cs_flags) > > +txq_calc_offload(struct rte_mbuf **pkts, uint16_t pkts_n, uint8_t > *cs_flags, > > + const uint64_t txq_offloads) > > { > > unsigned int pos; > > const uint64_t ol_mask =3D > > PKT_TX_IP_CKSUM | PKT_TX_TCP_CKSUM | > > PKT_TX_UDP_CKSUM | PKT_TX_TUNNEL_GRE | > > - PKT_TX_TUNNEL_VXLAN | PKT_TX_OUTER_IP_CKSUM; > > + PKT_TX_TUNNEL_VXLAN | PKT_TX_OUTER_IP_CKSUM | > PKT_TX_METADATA; > > > > if (!pkts_n) > > return 0; > > /* Count the number of packets having same ol_flags. */ > > - for (pos =3D 1; pos < pkts_n; ++pos) > > - if ((pkts[pos]->ol_flags ^ pkts[0]->ol_flags) & ol_mask) > > + for (pos =3D 1; pos < pkts_n; ++pos) { > > + if ((txq_offloads & MLX5_VEC_TX_CKSUM_OFFLOAD_CAP) > && > > + ((pkts[pos]->ol_flags ^ pkts[0]->ol_flags) & > ol_mask)) > > break; > > + /* If the metadata ol_flag is set, > > + * metadata must be same in all packets. > > + */ > > + if ((txq_offloads & DEV_TX_OFFLOAD_MATCH_METADATA) > && > > + (pkts[pos]->ol_flags & PKT_TX_METADATA) && > > + pkts[0]->hash.fdir.hi !=3D pkts[pos]->hash.fdir.hi) > > + break; > > + } > > + > > *cs_flags =3D txq_ol_cksum_to_cs(pkts[0]); > > return pos; > > } > > @@ -137,8 +152,10 @@ > > n =3D RTE_MIN((uint16_t)(pkts_n - nb_tx), > MLX5_VPMD_TX_MAX_BURST); > > if (txq->offloads & DEV_TX_OFFLOAD_MULTI_SEGS) > > n =3D txq_count_contig_single_seg(&pkts[nb_tx], n); > > - if (txq->offloads & MLX5_VEC_TX_CKSUM_OFFLOAD_CAP) > > - n =3D txq_calc_offload(&pkts[nb_tx], n, &cs_flags); > > + if (txq->offloads & (MLX5_VEC_TX_CKSUM_OFFLOAD_CAP | > > + DEV_TX_OFFLOAD_MATCH_METADATA)) >=20 > How about writing a separate func - txq_count_contig_same_metadata()? > And it would be better to rename txq_calc_offload() to > txq_count_contig_same_csflag(). > Then, there could be less redundant if-clause in the funcs. It was considered during implementation but decided to handle all related l= ogic In same function. >=20 > > + n =3D txq_calc_offload(&pkts[nb_tx], n, > > + &cs_flags, txq->offloads); > > ret =3D txq_burst_v(txq, &pkts[nb_tx], n, cs_flags); > > nb_tx +=3D ret; > > if (!ret) > > diff --git a/drivers/net/mlx5/mlx5_rxtx_vec.h > > b/drivers/net/mlx5/mlx5_rxtx_vec.h > > index fb884f9..fda7004 100644 > > --- a/drivers/net/mlx5/mlx5_rxtx_vec.h > > +++ b/drivers/net/mlx5/mlx5_rxtx_vec.h > > @@ -22,6 +22,7 @@ > > /* HW offload capabilities of vectorized Tx. */ #define > > MLX5_VEC_TX_OFFLOAD_CAP \ > > (MLX5_VEC_TX_CKSUM_OFFLOAD_CAP | \ > > + DEV_TX_OFFLOAD_MATCH_METADATA | \ > > DEV_TX_OFFLOAD_MULTI_SEGS) > > > > /* > > diff --git a/drivers/net/mlx5/mlx5_rxtx_vec_neon.h > > b/drivers/net/mlx5/mlx5_rxtx_vec_neon.h > > index b37b738..20c9427 100644 > > --- a/drivers/net/mlx5/mlx5_rxtx_vec_neon.h > > +++ b/drivers/net/mlx5/mlx5_rxtx_vec_neon.h > > @@ -237,6 +237,7 @@ > > uint8x16_t *t_wqe; > > uint8_t *dseg; > > uint8x16_t ctrl; > > + uint32_t md; /* metadata */ > > > > /* Make sure all packets can fit into a single WQE. */ > > assert(elts_n > pkts_n); > > @@ -293,10 +294,11 @@ > > ctrl =3D vqtbl1q_u8(ctrl, ctrl_shuf_m); > > vst1q_u8((void *)t_wqe, ctrl); > > /* Fill ESEG in the header. */ > > + md =3D pkts[0]->hash.fdir.hi; > > vst1q_u8((void *)(t_wqe + 1), > > ((uint8x16_t) { 0, 0, 0, 0, > > cs_flags, 0, 0, 0, > > - 0, 0, 0, 0, > > + md >> 24, md >> 16, md >> 8, md, > > 0, 0, 0, 0 })); >=20 > I know compiler would be a good optimization but as it is very performanc= e > critical path, let's optimize it by adding metadata as an argument for > txq_burst_v(). >=20 > static inline uint16_t > txq_burst_v(struct mlx5_txq_data *txq, struct rte_mbuf **pkts, uint16_t > pkts_n, > uint8_t cs_flags, rte_be32_t metadata) { ... > vst1q_u32((void *)(t_wqe + 1), > ((uint32x4_t) { 0, cs_flags, metadata, 0 })); ... > } >=20 > mlx5_tx_burst_raw_vec() should call txq_burst_v(..., 0, 0). As 0 is a bui= ltin > constant and txq_burst_v() is inline, it would get any performance drop. >=20 > In mlx5_tx_burst_vec(), > ret =3D txq_burst_v(txq, &pkts[nb_tx], n, cs_flags, metadata); >=20 > , where metadata is gotten from txq_count_contig_same_metadata() and > should be big-endian. Done. >=20 > > #ifdef MLX5_PMD_SOFT_COUNTERS > > txq->stats.opackets +=3D pkts_n; > > diff --git a/drivers/net/mlx5/mlx5_rxtx_vec_sse.h > > b/drivers/net/mlx5/mlx5_rxtx_vec_sse.h > > index 54b3783..7c8535c 100644 > > --- a/drivers/net/mlx5/mlx5_rxtx_vec_sse.h > > +++ b/drivers/net/mlx5/mlx5_rxtx_vec_sse.h > > @@ -236,6 +236,7 @@ > > 0, 1, 2, 3 /* bswap32 */); > > __m128i *t_wqe, *dseg; > > __m128i ctrl; > > + uint32_t md; /* metadata */ > > > > /* Make sure all packets can fit into a single WQE. */ > > assert(elts_n > pkts_n); > > @@ -292,9 +293,10 @@ > > ctrl =3D _mm_shuffle_epi8(ctrl, shuf_mask_ctrl); > > _mm_store_si128(t_wqe, ctrl); > > /* Fill ESEG in the header. */ > > + md =3D pkts[0]->hash.fdir.hi; > > _mm_store_si128(t_wqe + 1, > > _mm_set_epi8(0, 0, 0, 0, > > - 0, 0, 0, 0, > > + md, md >> 8, md >> 16, md >> 24, > > 0, 0, 0, cs_flags, > > 0, 0, 0, 0)); >=20 > If you take my proposal above, this should be: > _mm_store_si128(t_wqe + 1, _mm_set_epi32(0, metadata, cs_flags, > 0)); Done. >=20 > > #ifdef MLX5_PMD_SOFT_COUNTERS > > diff --git a/drivers/net/mlx5/mlx5_txq.c b/drivers/net/mlx5/mlx5_txq.c > > index f9bc473..7263fb1 100644 > > --- a/drivers/net/mlx5/mlx5_txq.c > > +++ b/drivers/net/mlx5/mlx5_txq.c > > @@ -128,6 +128,12 @@ > > offloads |=3D (DEV_TX_OFFLOAD_VXLAN_TNL_TSO | > > DEV_TX_OFFLOAD_GRE_TNL_TSO); > > } > > + > > +#ifdef HAVE_IBV_FLOW_DV_SUPPORT > > + if (config->dv_flow_en) > > + offloads |=3D DEV_TX_OFFLOAD_MATCH_METADATA; #endif > > + > > return offloads; > > } > > > > -- > > 1.8.3.1 > >