Hi Ophir,
Saturday, November 3, 2018 7:40 PM, Ophir Munk
> Subject: [PATCH v4] net/mlx5: set RSS key to NULL to indicate default RSS
>
> Applications which add RSS rules must supply an RSS key and length.
> If an application is only interested in default RSS operation it should not
> 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 set it
> to 0
> and the PMD will use the default type (ETH_RSS_IP).
>
> Signed-off-by: Ophir Munk <ophi...@mellanox.com>
> ---
> v1:
> Initial release
>
> v2, v3:
> Rebase + following code review
>
> v4:
> Avoid sgementation faulut by not allowing in cation validation key=NULL ane
> key_len!=0 See https://patches.dpdk.org/patch/47645/
>
>
> doc/guides/nics/mlx5.rst | 1 +
> drivers/net/mlx5/mlx5_flow.c | 8 +++++++-
> drivers/net/mlx5/mlx5_flow_dv.c | 7 +++++--
> drivers/net/mlx5/mlx5_flow_verbs.c | 8 ++++++--
> drivers/net/mlx5/mlx5_rxq.c | 18 ++++--------------
> 5 files changed, 23 insertions(+), 19 deletions(-)
>
> diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst index
> 1dc3282..0303152 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.
> +- Default RSS operation with no hash key specification.
> - 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 107a4f0..be2cc6b 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,
>
> 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. */
> + if (rss->key_len == 0 && rss->key != 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)
> return rte_flow_error_set(error, ENOTSUP,
>
> 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 c11ecd4..cdf3377 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -1758,8 +1758,11 @@
> memcpy((*flow->queue), rss->queue,
> rss->queue_num * sizeof(uint16_t));
> flow->rss.queue_num = rss->queue_num;
> - memcpy(flow->key, rss->key, MLX5_RSS_HASH_KEY_LEN);
> - flow->rss.types = rss->types;
> + /* NULL RSS key indicates default RSS key. */
> + rss_key = !rss->key ? rss_hash_default_key : rss->key;
Getting compilation error:
error: 'rss_key' undeclared (first use in this function)
rss_key = !rss->key ? rss_hash_default_key : rss->key;
please address.
Other than that, no more comments. You can put my ack when on the next version.
> + memcpy(flow->key, rss_key, MLX5_RSS_HASH_KEY_LEN);
> + /* RSS type 0 indicates default RSS type ETH_RSS_IP. */
> + flow->rss.types = !rss->types ? ETH_RSS_IP : rss->types;
> flow->rss.level = rss->level;
> /* Added to array only in apply since we need the QP */
> flow->actions |= MLX5_FLOW_ACTION_RSS; diff --git
> a/drivers/net/mlx5/mlx5_flow_verbs.c
> b/drivers/net/mlx5/mlx5_flow_verbs.c
> index 2e506b9..54ac620 100644
> --- a/drivers/net/mlx5/mlx5_flow_verbs.c
> +++ b/drivers/net/mlx5/mlx5_flow_verbs.c
> @@ -925,14 +925,18 @@
> struct mlx5_flow *dev_flow)
> {
> const struct rte_flow_action_rss *rss = action->conf;
> + const uint8_t *rss_key;
> struct rte_flow *flow = dev_flow->flow;
>
> if (flow->queue)
> memcpy((*flow->queue), rss->queue,
> rss->queue_num * sizeof(uint16_t));
> flow->rss.queue_num = rss->queue_num;
> - memcpy(flow->key, rss->key, MLX5_RSS_HASH_KEY_LEN);
> - flow->rss.types = rss->types;
> + /* NULL RSS key indicates default RSS key. */
> + rss_key = !rss->key ? rss_hash_default_key : rss->key;
> + memcpy(flow->key, rss_key, MLX5_RSS_HASH_KEY_LEN);
> + /* RSS type 0 indicates default RSS type (ETH_RSS_IP). */
> + flow->rss.types = !rss->types ? ETH_RSS_IP : rss->types;
> flow->rss.level = rss->level;
> *action_flags |= MLX5_FLOW_ACTION_RSS; } diff --git
> a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c index
> 6df8997..eef4850 100644
> --- a/drivers/net/mlx5/mlx5_rxq.c
> +++ b/drivers/net/mlx5/mlx5_rxq.c
> @@ -1794,10 +1794,6 @@ struct mlx5_hrxq *
> rte_errno = ENOMEM;
> return NULL;
> }
> - if (!rss_key_len) {
> - rss_key_len = MLX5_RSS_HASH_KEY_LEN;
> - rss_key = rss_hash_default_key;
> - }
> #ifdef HAVE_IBV_DEVICE_TUNNEL_SUPPORT
> if (tunnel) {
> qp_init_attr.comp_mask =
> @@ -1823,11 +1819,8 @@ struct mlx5_hrxq *
> IBV_QP_INIT_ATTR_RX_HASH,
> .rx_hash_conf = (struct ibv_rx_hash_conf){
> .rx_hash_function =
> IBV_RX_HASH_FUNC_TOEPLITZ,
> - .rx_hash_key_len = rss_key_len ?
> rss_key_len :
> - MLX5_RSS_HASH_KEY_LEN,
> - .rx_hash_key = rss_key ?
> - (void *)(uintptr_t)rss_key :
> - rss_hash_default_key,
> + .rx_hash_key_len = rss_key_len,
> + .rx_hash_key = (void *)(uintptr_t)rss_key,
> .rx_hash_fields_mask = hash_fields,
> },
> .rwq_ind_tbl = ind_tbl->ind_table,
> @@ -1845,11 +1838,8 @@ struct mlx5_hrxq *
> IBV_QP_INIT_ATTR_RX_HASH,
> .rx_hash_conf = (struct ibv_rx_hash_conf){
> .rx_hash_function =
> IBV_RX_HASH_FUNC_TOEPLITZ,
> - .rx_hash_key_len = rss_key_len ?
> rss_key_len :
> - MLX5_RSS_HASH_KEY_LEN,
> - .rx_hash_key = rss_key ?
> - (void *)(uintptr_t)rss_key :
> - rss_hash_default_key,
> + .rx_hash_key_len = rss_key_len,
> + .rx_hash_key = (void *)(uintptr_t)rss_key,
> .rx_hash_fields_mask = hash_fields,
> },
> .rwq_ind_tbl = ind_tbl->ind_table,
> --
> 1.8.3.1
