From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR01-VE1-obe.outbound.protection.outlook.com (mail-ve1eur01on0081.outbound.protection.outlook.com [104.47.1.81]) by dpdk.org (Postfix) with ESMTP id DF379559A for ; Thu, 1 Nov 2018 15:00:47 +0100 (CET) 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=bh9CogpWuxg0HHYmbro+ywTOXbCH3vGSbfbB9/I2UEc=; b=m0tBEfldx4Mm1DJiNhNP9QxxQMfS4QQz0esdl3m9qtqKIERcPzeYJ4ruo9fogg1adplNUb2ZQzAEhWbGBAc07zeHa2LT5Rjw7VZD/vM8Qfl0RtxVUFBouFxHwywWC9LE+9ZejU/586lr/jw2TtYlbS1JHfLXsKktEW1Pd0OKtAE= Received: from DB7PR05MB4426.eurprd05.prod.outlook.com (52.134.109.15) by DB7PR05MB4410.eurprd05.prod.outlook.com (52.134.109.11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1294.21; Thu, 1 Nov 2018 14:00:45 +0000 Received: from DB7PR05MB4426.eurprd05.prod.outlook.com ([fe80::80e:e6b:baf2:d973]) by DB7PR05MB4426.eurprd05.prod.outlook.com ([fe80::80e:e6b:baf2:d973%3]) with mapi id 15.20.1273.027; Thu, 1 Nov 2018 14:00:45 +0000 From: Shahaf Shuler To: Ophir Munk , "dev@dpdk.org" , Yongseok Koh CC: Thomas Monjalon , Olga Shern , Asaf Penso Thread-Topic: [PATCH v2] net/mlx5: set RSS key to NULL to indicate default RSS Thread-Index: AQHUccEoGKz1zQ7W60C7O6HB0ApP3KU67SfQ Date: Thu, 1 Nov 2018 14:00:45 +0000 Message-ID: References: <1538588246-21436-1-git-send-email-ophirmu@mellanox.com> <1541062744-25491-1-git-send-email-ophirmu@mellanox.com> In-Reply-To: <1541062744-25491-1-git-send-email-ophirmu@mellanox.com> 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=shahafs@mellanox.com; x-originating-ip: [193.47.165.251] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; DB7PR05MB4410; 6:MZWPDSJ7NTC2G61GM3+vlDbNVvkV+iXwbo4qRlwRL9KxuK2nUOircxcGFTYjX48luDiCRQf8/pPHKaIvhM7DV7zNYf28vemUmgJOQh648Sm8UdPFN4z7iLKn5rsY4cLdq2SHn73eCqrOUtXzsJqBLycspnRJSzTyehAtOGsMr9y/nM6mXqm69TK/TSJsjwpyIdHUrrd7LnTsm231d8IiciH4kLA00wB/o6FHQMaGrlSkyGm4cTuf+h+n8QuyTbj/AQjaMC3KP2cxwIqLIqTQZG7b30J7aX7zO0yepVnSxTbiun5oggp3zWpBBkRrR3E3ry91UU2r5IJ80rIJ1VfjW4z6q7DOFe6tRI65ZZDEFyaX+v8OP0tunkRW5el8QvdOiIOLuJImVLS8+urbCenSIm5FN0WWvvKKluXExeEUQvOHWf7VkFAEm6wVoiGZYndEBa0oU0czJmcivu91Xkx2dg==; 5:86WEIxFPMV7dxXggnacvyjN11o8MCmvsMOGDO2F66/rmeJ8WCvhOq/PJHZ4zJKvsDqOkNki7r3s5y8c8E/D4USUPAt42lj2+CjSPaemJz2xKl2HroUs6c5Gu9rNjfjUuAJubEC6Qf1fFL2cr4c9k5l9XG123eS6oE1pEI7t7kWQ=; 7:ddHW9LgCw0KC2UTwtDEqdYf054B/pM5bs6NbYZ7fy8OnN6KZwsK1dooagCI3hHzGSHNM8Zc90bHd1GDygSnDlfwH8DNpEz+5pexHZKmv3kFTOkhTT2r3nvmOV6voLJ0cAwvMvtMtDgwBY5ghAbx6nQ== x-ms-exchange-antispam-srfa-diagnostics: SOS; x-ms-office365-filtering-correlation-id: 7d9738e8-fcab-4b8b-c62a-08d640026b60 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0; PCL:0; RULEID:(7020095)(4652040)(8989299)(5600074)(711020)(4618075)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(2017052603328)(7153060)(7193020); SRVR:DB7PR05MB4410; x-ms-traffictypediagnostic: DB7PR05MB4410: x-ld-processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(278428928389397)(17755550239193); x-ms-exchange-senderadcheck: 1 x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(6040522)(2401047)(8121501046)(5005006)(10201501046)(3002001)(93006095)(93001095)(3231382)(944501410)(52105095)(6055026)(148016)(149066)(150057)(6041310)(20161123560045)(20161123564045)(20161123558120)(20161123562045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(201708071742011)(7699051)(76991095); SRVR:DB7PR05MB4410; BCL:0; PCL:0; RULEID:; SRVR:DB7PR05MB4410; x-forefront-prvs: 0843C17679 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(366004)(376002)(346002)(39860400002)(396003)(136003)(189003)(199004)(86362001)(74316002)(8936002)(102836004)(68736007)(99286004)(81156014)(81166006)(6436002)(76176011)(7696005)(6246003)(107886003)(2906002)(7736002)(9686003)(105586002)(305945005)(8676002)(6506007)(575784001)(316002)(110136005)(26005)(54906003)(53936002)(3846002)(106356001)(6116002)(186003)(55016002)(66066001)(6636002)(476003)(446003)(11346002)(14454004)(256004)(4326008)(229853002)(71190400001)(97736004)(25786009)(486006)(2900100001)(33656002)(5250100002)(5660300001)(2501003)(71200400001)(14444005)(478600001); DIR:OUT; SFP:1101; SCL:1; SRVR:DB7PR05MB4410; H:DB7PR05MB4426.eurprd05.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; MX:1; A:1; received-spf: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: hMUryNNkdrxNSenVlNXhnz0nkIus2TDrXEeKvggaCWB7OTMpYpWGomYNay3GWepyI9x/pAA/UWydgFBD9CCb4ldjVKtITlXUNd1+fM6lNnzKyZmiPzRMv0maGwrDCakISLnRWst2qqrfAdZO17gxNsXFEbwjc3joBfaLRoQ/BBHuFf+Y+zZP5jRTRp12WW2iv+6Qy+Re40Jk9GZRDXTCTinlgdatz3VDvfnXISIOr1eqClwjKWVS4dQkbdNamD977LTy51aI3yiKosdsfGj1h2+cbst7Hps1S7CiC5mpeovyuCmhFjdmAof5GQtQWnS7J3AwqR4o4rsbD43lomIdcB+tg1smLrgecVcKB4/cfRs= 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: 7d9738e8-fcab-4b8b-c62a-08d640026b60 X-MS-Exchange-CrossTenant-originalarrivaltime: 01 Nov 2018 14:00:45.3219 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB7PR05MB4410 Subject: Re: [dpdk-dev] [PATCH v2] net/mlx5: set RSS key to NULL to indicate default RSS 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: Thu, 01 Nov 2018 14:00:48 -0000 Hi Ophir,=20 Thursday, November 1, 2018 10:59 AM, Ophir Munk: > Subject: [PATCH v2] net/mlx5: set RSS key to NULL to indicate default RSS >=20 > Applications which add RSS rules must supply an RSS key and length. > If an application is only interested in default RSS operation it should n= ot care > about the exact RSS key. > By setting the key to NULL - the PMD will use the default RSS key. > In addition if the application does not care about the RSS type it can se= t it to 0 > and the PMD will use the default type (ETH_RSS_IP). >=20 > Signed-off-by: Ophir Munk > --- > doc/guides/nics/mlx5.rst | 1 + > drivers/net/mlx5/mlx5_flow.c | 8 +++++++- > drivers/net/mlx5/mlx5_flow_dv.c | 8 ++++++-- > drivers/net/mlx5/mlx5_flow_verbs.c | 9 +++++++-- > drivers/net/mlx5/mlx5_rxq.c | 14 ++++++-------- > lib/librte_ethdev/rte_flow.h | 2 +- > 6 files changed, 28 insertions(+), 14 deletions(-) >=20 > diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst index > 6769628..de8f15b 100644 > --- a/doc/guides/nics/mlx5.rst > +++ b/doc/guides/nics/mlx5.rst > @@ -54,6 +54,7 @@ Features > - Support for scattered TX and RX frames. > - IPv4, IPv6, TCPv4, TCPv6, UDPv4 and UDPv6 RSS on any number of queues. > - Several RSS hash keys, one for each flow type. > +- NULL RSS key indicates default RSS key I don't think it is a feature, it should be an API implementation.=20 > - Configurable RETA table. > - Support for multiple MAC addresses. > - VLAN filtering. > diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c > index 280af0a..5742989 100644 > --- a/drivers/net/mlx5/mlx5_flow.c > +++ b/drivers/net/mlx5/mlx5_flow.c > @@ -912,7 +912,13 @@ uint32_t mlx5_flow_adjust_priority(struct > rte_eth_dev *dev, int32_t priority, >=20 > RTE_FLOW_ERROR_TYPE_ACTION_CONF, > &rss->level, > "tunnel RSS is not supported"); > - if (rss->key_len < MLX5_RSS_HASH_KEY_LEN) > + /* allow RSS key_len 0 in case of NULL (default) RSS key */ Missing period.=20 > + if (rss->key_len =3D=3D 0 && rss->key !=3D NULL) > + return rte_flow_error_set(error, ENOTSUP, > + > RTE_FLOW_ERROR_TYPE_ACTION_CONF, > + &rss->key_len, > + "RSS hash key length 0"); > + if (rss->key_len > 0 && rss->key_len < MLX5_RSS_HASH_KEY_LEN) Can't you simplify using a single condition:=20 If (rss->key && rss->key_len < MLX5_RSS_HASH_KEY_LEN) Same rss->key addition should be for the case the key_len is too large.=20 > return rte_flow_error_set(error, ENOTSUP, >=20 > RTE_FLOW_ERROR_TYPE_ACTION_CONF, > &rss->key_len, > diff --git a/drivers/net/mlx5/mlx5_flow_dv.c > b/drivers/net/mlx5/mlx5_flow_dv.c index 8f729f4..fd52bf4 100644 > --- a/drivers/net/mlx5/mlx5_flow_dv.c > +++ b/drivers/net/mlx5/mlx5_flow_dv.c > @@ -1056,6 +1056,7 @@ > { > const struct rte_flow_action_queue *queue; > const struct rte_flow_action_rss *rss; > + uint8_t *rss_key; Why no declare it inside the RSS case?=20 > int actions_n =3D dev_flow->dv.actions_n; > struct rte_flow *flow =3D dev_flow->flow; >=20 > @@ -1094,8 +1095,11 @@ > memcpy((*flow->queue), rss->queue, > rss->queue_num * sizeof(uint16_t)); > flow->rss.queue_num =3D rss->queue_num; > - memcpy(flow->key, rss->key, MLX5_RSS_HASH_KEY_LEN); > - flow->rss.types =3D rss->types; > + /* NULL RSS key indicates default RSS key */ Missing period.=20 > + rss_key =3D (!rss->key) ? rss_hash_default_key : rss->key; > + memcpy(flow->key, rss_key, MLX5_RSS_HASH_KEY_LEN); For consistency you should also set the key_len on the flow structure to ML= X5_RSS_HASH_KEY_LEN. > + /* RSS type 0 indicates default RSS type ETH_RSS_IP */ Period.=20 > + flow->rss.types =3D !rss->types ? ETH_RSS_IP : rss->types; > flow->rss.level =3D rss->level; > /* Added to array only in apply since we need the QP */ > flow->actions |=3D MLX5_FLOW_ACTION_RSS; diff --git > a/drivers/net/mlx5/mlx5_flow_verbs.c > b/drivers/net/mlx5/mlx5_flow_verbs.c > index 81bc39f..ae9042f 100644 > --- a/drivers/net/mlx5/mlx5_flow_verbs.c > +++ b/drivers/net/mlx5/mlx5_flow_verbs.c > @@ -925,14 +925,19 @@ > struct mlx5_flow *dev_flow) > { > const struct rte_flow_action_rss *rss =3D action->conf; > + const uint8_t *rss_key; > struct rte_flow *flow =3D dev_flow->flow; >=20 > if (flow->queue) > memcpy((*flow->queue), rss->queue, > rss->queue_num * sizeof(uint16_t)); > flow->rss.queue_num =3D rss->queue_num; > - memcpy(flow->key, rss->key, MLX5_RSS_HASH_KEY_LEN); > - flow->rss.types =3D rss->types; > + > + /* NULL RSS key indicates default RSS key */ period > + rss_key =3D !rss->key ? rss_hash_default_key : rss->key; For consistency you should also set the key_len on the flow structure to ML= X5_RSS_HASH_KEY_LEN.=20 > + memcpy(flow->key, rss_key, MLX5_RSS_HASH_KEY_LEN); > + /* RSS type 0 indicates default RSS type (ETH_RSS_IP) */ period > + flow->rss.types =3D !rss->types ? ETH_RSS_IP : rss->types; > flow->rss.level =3D rss->level; > *action_flags |=3D MLX5_FLOW_ACTION_RSS; } diff --git > a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c index > ed993ea..36fc202 100644 > --- a/drivers/net/mlx5/mlx5_rxq.c > +++ b/drivers/net/mlx5/mlx5_rxq.c > @@ -1783,10 +1783,6 @@ struct mlx5_hrxq * > rte_errno =3D ENOMEM; > return NULL; > } > - if (!rss_key_len) { > - rss_key_len =3D MLX5_RSS_HASH_KEY_LEN; > - rss_key =3D rss_hash_default_key; > - } > #ifdef HAVE_IBV_DEVICE_TUNNEL_SUPPORT > qp =3D mlx5_glue->dv_create_qp > (priv->ctx, > @@ -1798,8 +1794,7 @@ struct mlx5_hrxq * > IBV_QP_INIT_ATTR_RX_HASH, > .rx_hash_conf =3D (struct ibv_rx_hash_conf){ > .rx_hash_function =3D > IBV_RX_HASH_FUNC_TOEPLITZ, > - .rx_hash_key_len =3D rss_key_len ? > rss_key_len : > - MLX5_RSS_HASH_KEY_LEN, > + .rx_hash_key_len =3D rss_key_len, > .rx_hash_key =3D rss_key ? > (void *)(uintptr_t)rss_key : > rss_hash_default_key, > @@ -1824,8 +1819,7 @@ struct mlx5_hrxq * > IBV_QP_INIT_ATTR_RX_HASH, > .rx_hash_conf =3D (struct ibv_rx_hash_conf){ > .rx_hash_function =3D > IBV_RX_HASH_FUNC_TOEPLITZ, > - .rx_hash_key_len =3D rss_key_len ? > rss_key_len : > - MLX5_RSS_HASH_KEY_LEN, > + .rx_hash_key_len =3D rss_key_len, > .rx_hash_key =3D rss_key ? > (void *)(uintptr_t)rss_key : > rss_hash_default_key, > @@ -1888,8 +1882,12 @@ struct mlx5_hrxq * > LIST_FOREACH(hrxq, &priv->hrxqs, next) { > struct mlx5_ind_table_ibv *ind_tbl; >=20 > + if (!rss_key_len) > + rss_key_len =3D MLX5_RSS_HASH_KEY_LEN; In the mlx5_hrxq_new above you trust the rss_key_len to be the correct one,= and it is ok since you validate it on the rss validate function. Why it is not the case here as well?=20 > if (hrxq->rss_key_len !=3D rss_key_len) > continue; > + if (!rss_key) > + rss_key =3D rss_hash_default_key; Same question. You already copied to correct key in flow_verbs_translate_ac= tion_rss or on the flow_dv_create_action funciton.=20 > if (memcmp(hrxq->rss_key, rss_key, rss_key_len)) > continue; > if (hrxq->hash_fields !=3D hash_fields) diff --git > a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h index > c0fe879..6cc72ab 100644 > --- a/lib/librte_ethdev/rte_flow.h > +++ b/lib/librte_ethdev/rte_flow.h > @@ -1785,7 +1785,7 @@ struct rte_flow_action_rss { > uint64_t types; /**< Specific RSS hash types (see ETH_RSS_*). */ > uint32_t key_len; /**< Hash key length in bytes. */ > uint32_t queue_num; /**< Number of entries in @p queue. */ > - const uint8_t *key; /**< Hash key. */ > + const uint8_t *key; /**< Hash key. NULL should indicate default key This change should be on a separate commit in a separate series.=20 It is better to put the doc update above the function and not inline on the= field so that you can elaborate more.=20 Also you are missing the same doc update for the types field.=20 Note: your patch for mlx5 can be accepted also w/o it, but it is better to = align all vendors and I think it is aligned w/ the spirit of the rte_flow m= aintainer.=20 > */ > const uint16_t *queue; /**< Queue indices to use. */ }; >=20 > -- > 1.8.3.1