[dpdk-dev,2/2] net/mlx4: refactor RSS conversion functions
Checks
Commit Message
Since commit 97b2217ae5bc ("net/mlx4: advertise supported RSS hash
functions"), this PMD includes two similar-looking functions that convert
RSS hash fields between Verbs and DPDK formats.
This patch refactors them as a single two-way function and gets rid of
redundant helper macros.
Note the loss of the "static" keyword on the out[] (now verbs[]) array
introduced by commit cbd737416c34 ("net/mlx4: avoid constant recreations in
function") is what prevents the reliance on macro definitions for static
initializers at the expense of a few extra instructions. An acceptable
trade-off given this function is not involved in data plane operations.
Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Cc: Ophir Munk <ophirmu@mellanox.com>
---
drivers/net/mlx4/mlx4_ethdev.c | 3 +-
drivers/net/mlx4/mlx4_flow.c | 147 ++++++++++--------------------------
drivers/net/mlx4/mlx4_flow.h | 4 +-
3 files changed, 44 insertions(+), 110 deletions(-)
Comments
Hi Adrien,
I like the idea of having one conversion function from dpdk to verbs and vice versa.
I have some comments inline regarding the implementation.
> -----Original Message-----
> From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> Sent: Tuesday, May 15, 2018 6:51 PM
> To: Shahaf Shuler <shahafs@mellanox.com>
> Cc: dev@dpdk.org; Ophir Munk <ophirmu@mellanox.com>
> Subject: [PATCH 2/2] net/mlx4: refactor RSS conversion functions
>
> Since commit 97b2217ae5bc ("net/mlx4: advertise supported RSS hash
> functions"), this PMD includes two similar-looking functions that convert RSS
> hash fields between Verbs and DPDK formats.
>
> This patch refactors them as a single two-way function and gets rid of
> redundant helper macros.
>
> Note the loss of the "static" keyword on the out[] (now verbs[]) array
> introduced by commit cbd737416c34 ("net/mlx4: avoid constant recreations
> in
> function") is what prevents the reliance on macro definitions for static
> initializers at the expense of a few extra instructions. An acceptable trade-off
> given this function is not involved in data plane operations.
>
> Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> Cc: Ophir Munk <ophirmu@mellanox.com>
> ---
> drivers/net/mlx4/mlx4_ethdev.c | 3 +-
> drivers/net/mlx4/mlx4_flow.c | 147 ++++++++++--------------------------
> drivers/net/mlx4/mlx4_flow.h | 4 +-
> 3 files changed, 44 insertions(+), 110 deletions(-)
>
> diff --git a/drivers/net/mlx4/mlx4_ethdev.c
> b/drivers/net/mlx4/mlx4_ethdev.c index 0a9c2e2f5..30deb3ef0 100644
> --- a/drivers/net/mlx4/mlx4_ethdev.c
> +++ b/drivers/net/mlx4/mlx4_ethdev.c
> @@ -587,8 +587,7 @@ mlx4_dev_infos_get(struct rte_eth_dev *dev, struct
> rte_eth_dev_info *info)
> ETH_LINK_SPEED_20G |
> ETH_LINK_SPEED_40G |
> ETH_LINK_SPEED_56G;
> - info->flow_type_rss_offloads =
> - mlx4_ibv_to_rss_types(priv->hw_rss_sup);
> + info->flow_type_rss_offloads = mlx4_conv_rss_types(priv, 0, 1);
Calling with parameters (priv, 0, 1) parameters in not as intuitive as calling with (priv, DEFAULT_RSS, DPDK_TO_VERBS)
Would you consider using definitions for the 0, 1 parameters to increase readability?
> }
>
> /**
> diff --git a/drivers/net/mlx4/mlx4_flow.c b/drivers/net/mlx4/mlx4_flow.c
> index ebc9eeb8b..90d206b55 100644
> --- a/drivers/net/mlx4/mlx4_flow.c
> +++ b/drivers/net/mlx4/mlx4_flow.c
> @@ -42,41 +42,6 @@
> #include "mlx4_rxtx.h"
> #include "mlx4_utils.h"
>
> -/** IBV supported RSS hash functions combinations */ -#define
> MLX4_IBV_IPV4_HF ( \
> - IBV_RX_HASH_SRC_IPV4 | \
> - IBV_RX_HASH_DST_IPV4)
> -#define MLX4_IBV_IPV6_HF ( \
> - IBV_RX_HASH_SRC_IPV6 | \
> - IBV_RX_HASH_DST_IPV6)
> -#define MLX4_IBV_TCP_HF ( \
> - IBV_RX_HASH_SRC_PORT_TCP | \
> - IBV_RX_HASH_DST_PORT_TCP)
> -#define MLX4_IBV_UDP_HF ( \
> - IBV_RX_HASH_SRC_PORT_UDP | \
> - IBV_RX_HASH_DST_PORT_UDP)
> -
> -/** Supported RSS hash functions combinations */ -#define
> MLX4_RSS_IPV4_HF ( \
> - ETH_RSS_IPV4 | \
> - ETH_RSS_FRAG_IPV4 | \
> - ETH_RSS_NONFRAG_IPV4_OTHER)
> -#define MLX4_RSS_IPV6_HF ( \
> - ETH_RSS_IPV6 | \
> - ETH_RSS_FRAG_IPV6 | \
> - ETH_RSS_NONFRAG_IPV6_OTHER | \
> - ETH_RSS_IPV6_EX)
> -#define MLX4_RSS_IPV4_TCP_HF ( \
> - ETH_RSS_NONFRAG_IPV4_TCP)
> -#define MLX4_RSS_IPV6_TCP_HF ( \
> - ETH_RSS_NONFRAG_IPV6_TCP | \
> - ETH_RSS_IPV6_TCP_EX)
> -#define MLX4_RSS_IPV4_UDP_HF ( \
> - ETH_RSS_NONFRAG_IPV4_UDP)
> -#define MLX4_RSS_IPV6_UDP_HF ( \
> - ETH_RSS_NONFRAG_IPV6_UDP | \
> - ETH_RSS_IPV6_UDP_EX)
> -
> /** Static initializer for a list of subsequent item types. */ #define
> NEXT_ITEM(...) \
> (const enum rte_flow_item_type []){ \
> @@ -111,7 +76,7 @@ struct mlx4_drop {
> };
>
> /**
> - * Convert DPDK RSS hash types to their Verbs equivalent.
> + * Convert supported RSS hash field types between DPDK and Verbs
> formats.
> *
> * This function returns the supported (default) set when @p types has
> * special value 0.
> @@ -119,106 +84,76 @@ struct mlx4_drop {
> * @param priv
> * Pointer to private structure.
> * @param types
> - * Hash types in DPDK format (see struct rte_eth_rss_conf).
> + * Depending on @p verbs_to_dpdk, hash types in either DPDK (see struct
> + * rte_eth_rss_conf) or Verbs format.
> + * @param verbs_to_dpdk
> + * A zero value converts @p types from DPDK to Verbs, a nonzero value
> + * performs the reverse operation.
> *
> * @return
> - * A valid Verbs RSS hash fields mask for mlx4 on success, (uint64_t)-1
> - * otherwise and rte_errno is set.
> + * Converted RSS hash fields on success, (uint64_t)-1 otherwise and
> + * rte_errno is set.
> */
> uint64_t
> -mlx4_conv_rss_types(struct priv *priv, uint64_t types)
> +mlx4_conv_rss_types(struct priv *priv, uint64_t types, int
> +verbs_to_dpdk)
> {
> - enum { IPV4, IPV6, TCP, UDP, };
> - static const uint64_t in[] = {
> + enum {
> + INNER, IPV4, IPV6, TCP, UDP,
> + IPV4_TCP, IPV4_UDP, IPV6_TCP, IPV6_UDP,
> + };
> + static const uint64_t dpdk[] = {
> + [INNER] = 0,
> [IPV4] = (ETH_RSS_IPV4 |
> ETH_RSS_FRAG_IPV4 |
> - ETH_RSS_NONFRAG_IPV4_TCP |
> - ETH_RSS_NONFRAG_IPV4_UDP |
> ETH_RSS_NONFRAG_IPV4_OTHER),
> [IPV6] = (ETH_RSS_IPV6 |
> ETH_RSS_FRAG_IPV6 |
> - ETH_RSS_NONFRAG_IPV6_TCP |
> - ETH_RSS_NONFRAG_IPV6_UDP |
> ETH_RSS_NONFRAG_IPV6_OTHER |
> - ETH_RSS_IPV6_EX |
> - ETH_RSS_IPV6_TCP_EX |
> - ETH_RSS_IPV6_UDP_EX),
> - [TCP] = (ETH_RSS_NONFRAG_IPV4_TCP |
> - ETH_RSS_NONFRAG_IPV6_TCP |
> - ETH_RSS_IPV6_TCP_EX),
> - [UDP] = (ETH_RSS_NONFRAG_IPV4_UDP |
> - ETH_RSS_NONFRAG_IPV6_UDP |
> - ETH_RSS_IPV6_UDP_EX),
> + ETH_RSS_IPV6_EX),
> + [TCP] = 0,
> + [UDP] = 0,
> + [IPV4_TCP] = ETH_RSS_NONFRAG_IPV4_TCP,
> + [IPV4_UDP] = ETH_RSS_NONFRAG_IPV4_UDP,
> + [IPV6_TCP] = (ETH_RSS_NONFRAG_IPV6_TCP |
> + ETH_RSS_IPV6_TCP_EX),
> + [IPV6_UDP] = (ETH_RSS_NONFRAG_IPV6_UDP |
> + ETH_RSS_IPV6_UDP_EX),
> };
> - static const uint64_t out[RTE_DIM(in)] = {
> + const uint64_t verbs[RTE_DIM(dpdk)] = {
> + [INNER] = IBV_RX_HASH_INNER,
> [IPV4] = IBV_RX_HASH_SRC_IPV4 | IBV_RX_HASH_DST_IPV4,
> [IPV6] = IBV_RX_HASH_SRC_IPV6 | IBV_RX_HASH_DST_IPV6,
> [TCP] = IBV_RX_HASH_SRC_PORT_TCP |
> IBV_RX_HASH_DST_PORT_TCP,
> [UDP] = IBV_RX_HASH_SRC_PORT_UDP |
> IBV_RX_HASH_DST_PORT_UDP,
> + [IPV4_TCP] = verbs[IPV4] | verbs[TCP],
> + [IPV4_UDP] = verbs[IPV4] | verbs[UDP],
> + [IPV6_TCP] = verbs[IPV6] | verbs[TCP],
> + [IPV6_UDP] = verbs[IPV6] | verbs[UDP],
> };
> + const uint64_t *in = verbs_to_dpdk ? verbs : dpdk;
> + const uint64_t *out = verbs_to_dpdk ? dpdk : verbs;
> uint64_t seen = 0;
> uint64_t conv = 0;
> unsigned int i;
>
> - if (!types)
> - return priv->hw_rss_sup;
> - for (i = 0; i != RTE_DIM(in); ++i)
> + if (!types) {
> + if (!verbs_to_dpdk)
> + return priv->hw_rss_sup;
> + types = priv->hw_rss_sup;
> + }
> + for (i = 0; i != RTE_DIM(dpdk); ++i)
> if (types & in[i]) {
> seen |= types & in[i];
> conv |= out[i];
> }
> - if ((conv & priv->hw_rss_sup) == conv && !(types & ~seen))
> + if ((verbs_to_dpdk || (conv & priv->hw_rss_sup) == conv) &&
> + !(types & ~seen))
> return conv;
> rte_errno = ENOTSUP;
> return (uint64_t)-1;
> }
>
1. I wonder if [INNER] case is correct. Assuming we are given IBV_RX_HASH_INNER I think we should return with error ENOTSUP.
However in the implementation above we return 0 as a valid DPDK RSS value.
Returning a 0 value is also confusing since it is used in dpdk as "Best effort" RSS HF.
2. Assuming we are given = IBV_RX_HASH_SRC_IPV4 alone (without IBV_RX_HASH_DST_IPV4) I think we should return with error ENOSUP since mlx4 does not support a hash based on SRC IPV4 alone. However in the implementation above we return a valid answer of all the flags from [IPV4], [IPV4_TCP] and [IPV4_UDP]. Can you please explain this logic?
The same question repeats for all other "_DST_" only or "_SRC_" only alone constants.
3. Given "IBV_RX_HASH_SRC_PORT_TCP | IBV_RX_HASH_DST_PORT_TCP" alone we are returning a DPDK reply which includes TCP flags + all IPV4 and IPV6 flags. Can you please explain this logic?
As mentioned I like the idea of one function to transform either direction between verbs and dpdk but please explain the current implementation following the questions above.
> /**
> - * Convert verbs RSS types to their DPDK equivalents.
> - *
> - * This function returns a group of RSS DPDK types given their equivalent
> group
> - * of verbs types.
> - * For example both source IPv4 and destination IPv4 verbs types are
> converted
> - * into their equivalent RSS group types. If each of these verbs types existed
> - * exclusively - no conversion would take place.
> - *
> - * @param types
> - * RSS hash types in verbs format.
> - *
> - * @return
> - * DPDK RSS hash fields supported by mlx4.
> - */
> -uint64_t
> -mlx4_ibv_to_rss_types(uint64_t types)
> -{
> - enum { IPV4, IPV6, IPV4_TCP, IPV6_TCP, IPV4_UDP, IPV6_UDP};
> -
> - static const uint64_t in[] = {
> - [IPV4] = MLX4_IBV_IPV4_HF,
> - [IPV6] = MLX4_IBV_IPV6_HF,
> - [IPV4_TCP] = MLX4_IBV_IPV4_HF | MLX4_IBV_TCP_HF,
> - [IPV6_TCP] = MLX4_IBV_IPV6_HF | MLX4_IBV_TCP_HF,
> - [IPV4_UDP] = MLX4_IBV_IPV4_HF | MLX4_IBV_UDP_HF,
> - [IPV6_UDP] = MLX4_IBV_IPV6_HF | MLX4_IBV_UDP_HF,
> - };
> - static const uint64_t out[RTE_DIM(in)] = {
> - [IPV4] = MLX4_RSS_IPV4_HF,
> - [IPV6] = MLX4_RSS_IPV6_HF,
> - [IPV4_TCP] = MLX4_RSS_IPV4_HF |
> MLX4_RSS_IPV4_TCP_HF,
> - [IPV6_TCP] = MLX4_RSS_IPV6_HF |
> MLX4_RSS_IPV6_TCP_HF,
> - [IPV4_UDP] = MLX4_RSS_IPV4_HF |
> MLX4_RSS_IPV4_UDP_HF,
> - [IPV6_UDP] = MLX4_RSS_IPV6_HF |
> MLX4_RSS_IPV6_UDP_HF,
> - };
> - uint64_t conv = 0;
> - unsigned int i;
> -
> - for (i = 0; i != RTE_DIM(in); ++i)
> - if ((types & in[i]) == in[i])
> - conv |= out[i];
> - return conv;
> -}
> -
> -/**
> * Merge Ethernet pattern item into flow rule handle.
> *
> * Additional mlx4-specific constraints on supported fields:
> @@ -890,7 +825,7 @@ mlx4_flow_prepare(struct priv *priv,
> goto exit_action_not_supported;
> }
> rte_errno = 0;
> - fields = mlx4_conv_rss_types(priv, rss->types);
> + fields = mlx4_conv_rss_types(priv, rss->types, 0);
> if (fields == (uint64_t)-1 && rte_errno) {
> msg = "unsupported RSS hash type
> requested";
> goto exit_action_not_supported;
> diff --git a/drivers/net/mlx4/mlx4_flow.h b/drivers/net/mlx4/mlx4_flow.h
> index 2e82903bd..2917ebe95 100644
> --- a/drivers/net/mlx4/mlx4_flow.h
> +++ b/drivers/net/mlx4/mlx4_flow.h
> @@ -48,8 +48,8 @@ struct rte_flow {
>
> /* mlx4_flow.c */
>
> -uint64_t mlx4_conv_rss_types(struct priv *priv, uint64_t types); -uint64_t
> mlx4_ibv_to_rss_types(uint64_t ibv_rss_types);
> +uint64_t mlx4_conv_rss_types(struct priv *priv, uint64_t types,
> + int verbs_to_dpdk);
> int mlx4_flow_sync(struct priv *priv, struct rte_flow_error *error); void
> mlx4_flow_clean(struct priv *priv); int mlx4_filter_ctrl(struct rte_eth_dev
> *dev,
> --
> 2.11.0
Hi Ophir,
On Thu, May 17, 2018 at 05:46:45PM +0000, Ophir Munk wrote:
> Hi Adrien,
> I like the idea of having one conversion function from dpdk to verbs and vice versa.
> I have some comments inline regarding the implementation.
>
> > -----Original Message-----
> > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> > Sent: Tuesday, May 15, 2018 6:51 PM
> > To: Shahaf Shuler <shahafs@mellanox.com>
> > Cc: dev@dpdk.org; Ophir Munk <ophirmu@mellanox.com>
> > Subject: [PATCH 2/2] net/mlx4: refactor RSS conversion functions
> >
> > Since commit 97b2217ae5bc ("net/mlx4: advertise supported RSS hash
> > functions"), this PMD includes two similar-looking functions that convert RSS
> > hash fields between Verbs and DPDK formats.
> >
> > This patch refactors them as a single two-way function and gets rid of
> > redundant helper macros.
> >
> > Note the loss of the "static" keyword on the out[] (now verbs[]) array
> > introduced by commit cbd737416c34 ("net/mlx4: avoid constant recreations
> > in
> > function") is what prevents the reliance on macro definitions for static
> > initializers at the expense of a few extra instructions. An acceptable trade-off
> > given this function is not involved in data plane operations.
> >
> > Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > Cc: Ophir Munk <ophirmu@mellanox.com>
> > ---
> > drivers/net/mlx4/mlx4_ethdev.c | 3 +-
> > drivers/net/mlx4/mlx4_flow.c | 147 ++++++++++--------------------------
> > drivers/net/mlx4/mlx4_flow.h | 4 +-
> > 3 files changed, 44 insertions(+), 110 deletions(-)
> >
> > diff --git a/drivers/net/mlx4/mlx4_ethdev.c
> > b/drivers/net/mlx4/mlx4_ethdev.c index 0a9c2e2f5..30deb3ef0 100644
> > --- a/drivers/net/mlx4/mlx4_ethdev.c
> > +++ b/drivers/net/mlx4/mlx4_ethdev.c
> > @@ -587,8 +587,7 @@ mlx4_dev_infos_get(struct rte_eth_dev *dev, struct
> > rte_eth_dev_info *info)
> > ETH_LINK_SPEED_20G |
> > ETH_LINK_SPEED_40G |
> > ETH_LINK_SPEED_56G;
> > - info->flow_type_rss_offloads =
> > - mlx4_ibv_to_rss_types(priv->hw_rss_sup);
> > + info->flow_type_rss_offloads = mlx4_conv_rss_types(priv, 0, 1);
>
> Calling with parameters (priv, 0, 1) parameters in not as intuitive as calling with (priv, DEFAULT_RSS, DPDK_TO_VERBS)
> Would you consider using definitions for the 0, 1 parameters to increase readability?
Doing so would require either new macro or enum definitions, which I would
like to avoid for such a minor internal function.
Keep in mind the 0 value standing for "defaults" RSS types is both publicly
documented [1] and documented again in mlx4_conv_rss_types().
As for DPDK_TO_VERBS, this is already the name of the parameter (int
dpdk_to_verbs), currently a boolean value. I would have defined a type if
there were more than two possible states.
[1] http://dpdk.org/doc/guides/prog_guide/rte_flow.html#action-rss
>
> > }
> >
> > /**
> > diff --git a/drivers/net/mlx4/mlx4_flow.c b/drivers/net/mlx4/mlx4_flow.c
> > index ebc9eeb8b..90d206b55 100644
> > --- a/drivers/net/mlx4/mlx4_flow.c
> > +++ b/drivers/net/mlx4/mlx4_flow.c
> > @@ -42,41 +42,6 @@
> > #include "mlx4_rxtx.h"
> > #include "mlx4_utils.h"
> >
> > -/** IBV supported RSS hash functions combinations */ -#define
> > MLX4_IBV_IPV4_HF ( \
> > - IBV_RX_HASH_SRC_IPV4 | \
> > - IBV_RX_HASH_DST_IPV4)
> > -#define MLX4_IBV_IPV6_HF ( \
> > - IBV_RX_HASH_SRC_IPV6 | \
> > - IBV_RX_HASH_DST_IPV6)
> > -#define MLX4_IBV_TCP_HF ( \
> > - IBV_RX_HASH_SRC_PORT_TCP | \
> > - IBV_RX_HASH_DST_PORT_TCP)
> > -#define MLX4_IBV_UDP_HF ( \
> > - IBV_RX_HASH_SRC_PORT_UDP | \
> > - IBV_RX_HASH_DST_PORT_UDP)
> > -
> > -/** Supported RSS hash functions combinations */ -#define
> > MLX4_RSS_IPV4_HF ( \
> > - ETH_RSS_IPV4 | \
> > - ETH_RSS_FRAG_IPV4 | \
> > - ETH_RSS_NONFRAG_IPV4_OTHER)
> > -#define MLX4_RSS_IPV6_HF ( \
> > - ETH_RSS_IPV6 | \
> > - ETH_RSS_FRAG_IPV6 | \
> > - ETH_RSS_NONFRAG_IPV6_OTHER | \
> > - ETH_RSS_IPV6_EX)
> > -#define MLX4_RSS_IPV4_TCP_HF ( \
> > - ETH_RSS_NONFRAG_IPV4_TCP)
> > -#define MLX4_RSS_IPV6_TCP_HF ( \
> > - ETH_RSS_NONFRAG_IPV6_TCP | \
> > - ETH_RSS_IPV6_TCP_EX)
> > -#define MLX4_RSS_IPV4_UDP_HF ( \
> > - ETH_RSS_NONFRAG_IPV4_UDP)
> > -#define MLX4_RSS_IPV6_UDP_HF ( \
> > - ETH_RSS_NONFRAG_IPV6_UDP | \
> > - ETH_RSS_IPV6_UDP_EX)
> > -
> > /** Static initializer for a list of subsequent item types. */ #define
> > NEXT_ITEM(...) \
> > (const enum rte_flow_item_type []){ \
> > @@ -111,7 +76,7 @@ struct mlx4_drop {
> > };
> >
> > /**
> > - * Convert DPDK RSS hash types to their Verbs equivalent.
> > + * Convert supported RSS hash field types between DPDK and Verbs
> > formats.
> > *
> > * This function returns the supported (default) set when @p types has
> > * special value 0.
> > @@ -119,106 +84,76 @@ struct mlx4_drop {
> > * @param priv
> > * Pointer to private structure.
> > * @param types
> > - * Hash types in DPDK format (see struct rte_eth_rss_conf).
> > + * Depending on @p verbs_to_dpdk, hash types in either DPDK (see struct
> > + * rte_eth_rss_conf) or Verbs format.
> > + * @param verbs_to_dpdk
> > + * A zero value converts @p types from DPDK to Verbs, a nonzero value
> > + * performs the reverse operation.
> > *
> > * @return
> > - * A valid Verbs RSS hash fields mask for mlx4 on success, (uint64_t)-1
> > - * otherwise and rte_errno is set.
> > + * Converted RSS hash fields on success, (uint64_t)-1 otherwise and
> > + * rte_errno is set.
> > */
> > uint64_t
> > -mlx4_conv_rss_types(struct priv *priv, uint64_t types)
> > +mlx4_conv_rss_types(struct priv *priv, uint64_t types, int
> > +verbs_to_dpdk)
> > {
> > - enum { IPV4, IPV6, TCP, UDP, };
> > - static const uint64_t in[] = {
> > + enum {
> > + INNER, IPV4, IPV6, TCP, UDP,
> > + IPV4_TCP, IPV4_UDP, IPV6_TCP, IPV6_UDP,
> > + };
> > + static const uint64_t dpdk[] = {
> > + [INNER] = 0,
> > [IPV4] = (ETH_RSS_IPV4 |
> > ETH_RSS_FRAG_IPV4 |
> > - ETH_RSS_NONFRAG_IPV4_TCP |
> > - ETH_RSS_NONFRAG_IPV4_UDP |
> > ETH_RSS_NONFRAG_IPV4_OTHER),
> > [IPV6] = (ETH_RSS_IPV6 |
> > ETH_RSS_FRAG_IPV6 |
> > - ETH_RSS_NONFRAG_IPV6_TCP |
> > - ETH_RSS_NONFRAG_IPV6_UDP |
> > ETH_RSS_NONFRAG_IPV6_OTHER |
> > - ETH_RSS_IPV6_EX |
> > - ETH_RSS_IPV6_TCP_EX |
> > - ETH_RSS_IPV6_UDP_EX),
> > - [TCP] = (ETH_RSS_NONFRAG_IPV4_TCP |
> > - ETH_RSS_NONFRAG_IPV6_TCP |
> > - ETH_RSS_IPV6_TCP_EX),
> > - [UDP] = (ETH_RSS_NONFRAG_IPV4_UDP |
> > - ETH_RSS_NONFRAG_IPV6_UDP |
> > - ETH_RSS_IPV6_UDP_EX),
> > + ETH_RSS_IPV6_EX),
> > + [TCP] = 0,
> > + [UDP] = 0,
> > + [IPV4_TCP] = ETH_RSS_NONFRAG_IPV4_TCP,
> > + [IPV4_UDP] = ETH_RSS_NONFRAG_IPV4_UDP,
> > + [IPV6_TCP] = (ETH_RSS_NONFRAG_IPV6_TCP |
> > + ETH_RSS_IPV6_TCP_EX),
> > + [IPV6_UDP] = (ETH_RSS_NONFRAG_IPV6_UDP |
> > + ETH_RSS_IPV6_UDP_EX),
> > };
> > - static const uint64_t out[RTE_DIM(in)] = {
> > + const uint64_t verbs[RTE_DIM(dpdk)] = {
> > + [INNER] = IBV_RX_HASH_INNER,
> > [IPV4] = IBV_RX_HASH_SRC_IPV4 | IBV_RX_HASH_DST_IPV4,
> > [IPV6] = IBV_RX_HASH_SRC_IPV6 | IBV_RX_HASH_DST_IPV6,
> > [TCP] = IBV_RX_HASH_SRC_PORT_TCP |
> > IBV_RX_HASH_DST_PORT_TCP,
> > [UDP] = IBV_RX_HASH_SRC_PORT_UDP |
> > IBV_RX_HASH_DST_PORT_UDP,
> > + [IPV4_TCP] = verbs[IPV4] | verbs[TCP],
> > + [IPV4_UDP] = verbs[IPV4] | verbs[UDP],
> > + [IPV6_TCP] = verbs[IPV6] | verbs[TCP],
> > + [IPV6_UDP] = verbs[IPV6] | verbs[UDP],
> > };
> > + const uint64_t *in = verbs_to_dpdk ? verbs : dpdk;
> > + const uint64_t *out = verbs_to_dpdk ? dpdk : verbs;
> > uint64_t seen = 0;
> > uint64_t conv = 0;
> > unsigned int i;
> >
> > - if (!types)
> > - return priv->hw_rss_sup;
> > - for (i = 0; i != RTE_DIM(in); ++i)
> > + if (!types) {
> > + if (!verbs_to_dpdk)
> > + return priv->hw_rss_sup;
> > + types = priv->hw_rss_sup;
> > + }
> > + for (i = 0; i != RTE_DIM(dpdk); ++i)
> > if (types & in[i]) {
> > seen |= types & in[i];
> > conv |= out[i];
> > }
> > - if ((conv & priv->hw_rss_sup) == conv && !(types & ~seen))
> > + if ((verbs_to_dpdk || (conv & priv->hw_rss_sup) == conv) &&
> > + !(types & ~seen))
> > return conv;
> > rte_errno = ENOTSUP;
> > return (uint64_t)-1;
> > }
> >
>
> 1. I wonder if [INNER] case is correct. Assuming we are given IBV_RX_HASH_INNER I think we should return with error ENOTSUP.
> However in the implementation above we return 0 as a valid DPDK RSS value.
> Returning a 0 value is also confusing since it is used in dpdk as "Best effort" RSS HF.
Well, this function performs exactly one job: convert recognized DPDK or
Verbs RSS hash types to the other format. Verbs's INNER is supported but has
no ETH_RSS equivalent type, hence is discarded.
> 2. Assuming we are given = IBV_RX_HASH_SRC_IPV4 alone (without IBV_RX_HASH_DST_IPV4) I think we should return with error ENOSUP since mlx4 does not support a hash based on SRC IPV4 alone.
> However in the implementation above we return a valid answer of all the flags from [IPV4], [IPV4_TCP] and [IPV4_UDP]. Can you please explain this logic?
> The same question repeats for all other "_DST_" only or "_SRC_" only alone constants.
This is done like that for the following reasons:
- Original code didn't do it either.
- Doing so would be overkill. Consider that the PMD has a list of known
supported IBV fields but doesn't choose them on its own, the application
does so through a different API without the ability to tell SRC and DST
apart. Hence it's safe to toggle them together when dealing with DPDK
applications.
> 3. Given "IBV_RX_HASH_SRC_PORT_TCP | IBV_RX_HASH_DST_PORT_TCP" alone we are returning a DPDK reply which includes TCP flags + all IPV4 and IPV6 flags. Can you please explain this logic?
Same as above basically. The DPDK API does not have a concept of TCP alone,
it's always on top of something (v4, v6, with and without fragments or
extended headers, see ETH_RSS_TCP). Therefore a pure TCP input on the
Verbs side issues all of them on the DPDK side.
> As mentioned I like the idea of one function to transform either direction between verbs and dpdk but please explain the current implementation following the questions above.
>
> > /**
> > - * Convert verbs RSS types to their DPDK equivalents.
> > - *
> > - * This function returns a group of RSS DPDK types given their equivalent
> > group
> > - * of verbs types.
> > - * For example both source IPv4 and destination IPv4 verbs types are
> > converted
> > - * into their equivalent RSS group types. If each of these verbs types existed
> > - * exclusively - no conversion would take place.
> > - *
> > - * @param types
> > - * RSS hash types in verbs format.
> > - *
> > - * @return
> > - * DPDK RSS hash fields supported by mlx4.
> > - */
> > -uint64_t
> > -mlx4_ibv_to_rss_types(uint64_t types)
> > -{
> > - enum { IPV4, IPV6, IPV4_TCP, IPV6_TCP, IPV4_UDP, IPV6_UDP};
> > -
> > - static const uint64_t in[] = {
> > - [IPV4] = MLX4_IBV_IPV4_HF,
> > - [IPV6] = MLX4_IBV_IPV6_HF,
> > - [IPV4_TCP] = MLX4_IBV_IPV4_HF | MLX4_IBV_TCP_HF,
> > - [IPV6_TCP] = MLX4_IBV_IPV6_HF | MLX4_IBV_TCP_HF,
> > - [IPV4_UDP] = MLX4_IBV_IPV4_HF | MLX4_IBV_UDP_HF,
> > - [IPV6_UDP] = MLX4_IBV_IPV6_HF | MLX4_IBV_UDP_HF,
> > - };
> > - static const uint64_t out[RTE_DIM(in)] = {
> > - [IPV4] = MLX4_RSS_IPV4_HF,
> > - [IPV6] = MLX4_RSS_IPV6_HF,
> > - [IPV4_TCP] = MLX4_RSS_IPV4_HF |
> > MLX4_RSS_IPV4_TCP_HF,
> > - [IPV6_TCP] = MLX4_RSS_IPV6_HF |
> > MLX4_RSS_IPV6_TCP_HF,
> > - [IPV4_UDP] = MLX4_RSS_IPV4_HF |
> > MLX4_RSS_IPV4_UDP_HF,
> > - [IPV6_UDP] = MLX4_RSS_IPV6_HF |
> > MLX4_RSS_IPV6_UDP_HF,
> > - };
> > - uint64_t conv = 0;
> > - unsigned int i;
> > -
> > - for (i = 0; i != RTE_DIM(in); ++i)
> > - if ((types & in[i]) == in[i])
> > - conv |= out[i];
> > - return conv;
> > -}
> > -
> > -/**
> > * Merge Ethernet pattern item into flow rule handle.
> > *
> > * Additional mlx4-specific constraints on supported fields:
> > @@ -890,7 +825,7 @@ mlx4_flow_prepare(struct priv *priv,
> > goto exit_action_not_supported;
> > }
> > rte_errno = 0;
> > - fields = mlx4_conv_rss_types(priv, rss->types);
> > + fields = mlx4_conv_rss_types(priv, rss->types, 0);
> > if (fields == (uint64_t)-1 && rte_errno) {
> > msg = "unsupported RSS hash type
> > requested";
> > goto exit_action_not_supported;
> > diff --git a/drivers/net/mlx4/mlx4_flow.h b/drivers/net/mlx4/mlx4_flow.h
> > index 2e82903bd..2917ebe95 100644
> > --- a/drivers/net/mlx4/mlx4_flow.h
> > +++ b/drivers/net/mlx4/mlx4_flow.h
> > @@ -48,8 +48,8 @@ struct rte_flow {
> >
> > /* mlx4_flow.c */
> >
> > -uint64_t mlx4_conv_rss_types(struct priv *priv, uint64_t types); -uint64_t
> > mlx4_ibv_to_rss_types(uint64_t ibv_rss_types);
> > +uint64_t mlx4_conv_rss_types(struct priv *priv, uint64_t types,
> > + int verbs_to_dpdk);
> > int mlx4_flow_sync(struct priv *priv, struct rte_flow_error *error); void
> > mlx4_flow_clean(struct priv *priv); int mlx4_filter_ctrl(struct rte_eth_dev
> > *dev,
> > --
> > 2.11.0
Hi Adrien,
Please help me walkthrough this logic. Something doesn't end up correctly.
Friday, May 18, 2018 12:49 PM, Adrien Mazarguil:
> Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org
> Subject: Re: [PATCH 2/2] net/mlx4: refactor RSS conversion functions
> > > Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > > Cc: Ophir Munk <ophirmu@mellanox.com>
> > > *
> > > * @return
> > > - * A valid Verbs RSS hash fields mask for mlx4 on success, (uint64_t)-1
> > > - * otherwise and rte_errno is set.
> > > + * Converted RSS hash fields on success, (uint64_t)-1 otherwise and
> > > + * rte_errno is set.
> > > */
> > > uint64_t
> > > -mlx4_conv_rss_types(struct priv *priv, uint64_t types)
> > > +mlx4_conv_rss_types(struct priv *priv, uint64_t types, int
> > > +verbs_to_dpdk)
> > > {
Let's assume:
Types = IBV_RX_HASH_SRC_IPV4 | IBV_RX_HASH_DST_IPV4
And verbs_to_dpdk=1
> > > - enum { IPV4, IPV6, TCP, UDP, };
> > > - static const uint64_t in[] = {
> > > + enum {
> > > + INNER, IPV4, IPV6, TCP, UDP,
> > > + IPV4_TCP, IPV4_UDP, IPV6_TCP, IPV6_UDP,
> > > + };
> > > + static const uint64_t dpdk[] = {
> > > + [INNER] = 0,
> > > [IPV4] = (ETH_RSS_IPV4 |
> > > ETH_RSS_FRAG_IPV4 |
> > > - ETH_RSS_NONFRAG_IPV4_TCP |
> > > - ETH_RSS_NONFRAG_IPV4_UDP |
> > > ETH_RSS_NONFRAG_IPV4_OTHER),
> > > [IPV6] = (ETH_RSS_IPV6 |
> > > ETH_RSS_FRAG_IPV6 |
> > > - ETH_RSS_NONFRAG_IPV6_TCP |
> > > - ETH_RSS_NONFRAG_IPV6_UDP |
> > > ETH_RSS_NONFRAG_IPV6_OTHER |
> > > - ETH_RSS_IPV6_EX |
> > > - ETH_RSS_IPV6_TCP_EX |
> > > - ETH_RSS_IPV6_UDP_EX),
> > > - [TCP] = (ETH_RSS_NONFRAG_IPV4_TCP |
> > > - ETH_RSS_NONFRAG_IPV6_TCP |
> > > - ETH_RSS_IPV6_TCP_EX),
> > > - [UDP] = (ETH_RSS_NONFRAG_IPV4_UDP |
> > > - ETH_RSS_NONFRAG_IPV6_UDP |
> > > - ETH_RSS_IPV6_UDP_EX),
> > > + ETH_RSS_IPV6_EX),
> > > + [TCP] = 0,
> > > + [UDP] = 0,
> > > + [IPV4_TCP] = ETH_RSS_NONFRAG_IPV4_TCP,
> > > + [IPV4_UDP] = ETH_RSS_NONFRAG_IPV4_UDP,
> > > + [IPV6_TCP] = (ETH_RSS_NONFRAG_IPV6_TCP |
> > > + ETH_RSS_IPV6_TCP_EX),
> > > + [IPV6_UDP] = (ETH_RSS_NONFRAG_IPV6_UDP |
> > > + ETH_RSS_IPV6_UDP_EX),
> > > };
> > > - static const uint64_t out[RTE_DIM(in)] = {
> > > + const uint64_t verbs[RTE_DIM(dpdk)] = {
> > > + [INNER] = IBV_RX_HASH_INNER,
> > > [IPV4] = IBV_RX_HASH_SRC_IPV4 |
> IBV_RX_HASH_DST_IPV4,
> > > [IPV6] = IBV_RX_HASH_SRC_IPV6 |
> IBV_RX_HASH_DST_IPV6,
> > > [TCP] = IBV_RX_HASH_SRC_PORT_TCP |
> IBV_RX_HASH_DST_PORT_TCP,
> > > [UDP] = IBV_RX_HASH_SRC_PORT_UDP |
> IBV_RX_HASH_DST_PORT_UDP,
> > > + [IPV4_TCP] = verbs[IPV4] | verbs[TCP],
> > > + [IPV4_UDP] = verbs[IPV4] | verbs[UDP],
> > > + [IPV6_TCP] = verbs[IPV6] | verbs[TCP],
> > > + [IPV6_UDP] = verbs[IPV6] | verbs[UDP],
> > > };
> > > + const uint64_t *in = verbs_to_dpdk ? verbs : dpdk;
> > > + const uint64_t *out = verbs_to_dpdk ? dpdk : verbs;
> > > uint64_t seen = 0;
> > > uint64_t conv = 0;
> > > unsigned int i;
> > >
> > > - if (!types)
Types != 0 , we skip.
> > > - return priv->hw_rss_sup;
> > > - for (i = 0; i != RTE_DIM(in); ++i)
> > > + if (!types) {
> > > + if (!verbs_to_dpdk)
> > > + return priv->hw_rss_sup;
> > > + types = priv->hw_rss_sup;
> > > + }
> > > + for (i = 0; i != RTE_DIM(dpdk); ++i)
> > > if (types & in[i]) {
> > > seen |= types & in[i];
> > > conv |= out[i];
> > > }
After this loop:
seen = (IBV_RX_HASH_SRC_PORT_IPV4 | IBV_RX_HASH_DST_IPV4)
conv = dpdk[IPV4] | dpdk[IPV4_TCP] | dpdk[IPV4_UDP]
> > > - if ((conv & priv->hw_rss_sup) == conv && !(types & ~seen))
> > > + if ((verbs_to_dpdk || (conv & priv->hw_rss_sup) == conv) &&
> > > + !(types & ~seen))
All types were seen && verbs_to_dpdk hence returning that PMD support RSS based on IPV4 | TCPV4 | UDPV4
While according to the reported capabilities form kernel only IPV4 is supported.
> > > return conv;
> > > rte_errno = ENOTSUP;
> > > return (uint64_t)-1;
> > > }
> > >
> >
On Mon, May 21, 2018 at 10:59:36AM +0000, Shahaf Shuler wrote:
> Hi Adrien,
>
> Please help me walkthrough this logic. Something doesn't end up correctly.
>
> Friday, May 18, 2018 12:49 PM, Adrien Mazarguil:
> > Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org
> > Subject: Re: [PATCH 2/2] net/mlx4: refactor RSS conversion functions
> > > > Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > > > Cc: Ophir Munk <ophirmu@mellanox.com>
> > > > *
> > > > * @return
> > > > - * A valid Verbs RSS hash fields mask for mlx4 on success, (uint64_t)-1
> > > > - * otherwise and rte_errno is set.
> > > > + * Converted RSS hash fields on success, (uint64_t)-1 otherwise and
> > > > + * rte_errno is set.
> > > > */
> > > > uint64_t
> > > > -mlx4_conv_rss_types(struct priv *priv, uint64_t types)
> > > > +mlx4_conv_rss_types(struct priv *priv, uint64_t types, int
> > > > +verbs_to_dpdk)
> > > > {
>
>
> Let's assume:
> Types = IBV_RX_HASH_SRC_IPV4 | IBV_RX_HASH_DST_IPV4
> And verbs_to_dpdk=1
>
> > > > - enum { IPV4, IPV6, TCP, UDP, };
> > > > - static const uint64_t in[] = {
> > > > + enum {
> > > > + INNER, IPV4, IPV6, TCP, UDP,
> > > > + IPV4_TCP, IPV4_UDP, IPV6_TCP, IPV6_UDP,
> > > > + };
> > > > + static const uint64_t dpdk[] = {
> > > > + [INNER] = 0,
> > > > [IPV4] = (ETH_RSS_IPV4 |
> > > > ETH_RSS_FRAG_IPV4 |
> > > > - ETH_RSS_NONFRAG_IPV4_TCP |
> > > > - ETH_RSS_NONFRAG_IPV4_UDP |
> > > > ETH_RSS_NONFRAG_IPV4_OTHER),
> > > > [IPV6] = (ETH_RSS_IPV6 |
> > > > ETH_RSS_FRAG_IPV6 |
> > > > - ETH_RSS_NONFRAG_IPV6_TCP |
> > > > - ETH_RSS_NONFRAG_IPV6_UDP |
> > > > ETH_RSS_NONFRAG_IPV6_OTHER |
> > > > - ETH_RSS_IPV6_EX |
> > > > - ETH_RSS_IPV6_TCP_EX |
> > > > - ETH_RSS_IPV6_UDP_EX),
> > > > - [TCP] = (ETH_RSS_NONFRAG_IPV4_TCP |
> > > > - ETH_RSS_NONFRAG_IPV6_TCP |
> > > > - ETH_RSS_IPV6_TCP_EX),
> > > > - [UDP] = (ETH_RSS_NONFRAG_IPV4_UDP |
> > > > - ETH_RSS_NONFRAG_IPV6_UDP |
> > > > - ETH_RSS_IPV6_UDP_EX),
> > > > + ETH_RSS_IPV6_EX),
> > > > + [TCP] = 0,
> > > > + [UDP] = 0,
> > > > + [IPV4_TCP] = ETH_RSS_NONFRAG_IPV4_TCP,
> > > > + [IPV4_UDP] = ETH_RSS_NONFRAG_IPV4_UDP,
> > > > + [IPV6_TCP] = (ETH_RSS_NONFRAG_IPV6_TCP |
> > > > + ETH_RSS_IPV6_TCP_EX),
> > > > + [IPV6_UDP] = (ETH_RSS_NONFRAG_IPV6_UDP |
> > > > + ETH_RSS_IPV6_UDP_EX),
> > > > };
> > > > - static const uint64_t out[RTE_DIM(in)] = {
> > > > + const uint64_t verbs[RTE_DIM(dpdk)] = {
> > > > + [INNER] = IBV_RX_HASH_INNER,
> > > > [IPV4] = IBV_RX_HASH_SRC_IPV4 |
> > IBV_RX_HASH_DST_IPV4,
> > > > [IPV6] = IBV_RX_HASH_SRC_IPV6 |
> > IBV_RX_HASH_DST_IPV6,
> > > > [TCP] = IBV_RX_HASH_SRC_PORT_TCP |
> > IBV_RX_HASH_DST_PORT_TCP,
> > > > [UDP] = IBV_RX_HASH_SRC_PORT_UDP |
> > IBV_RX_HASH_DST_PORT_UDP,
> > > > + [IPV4_TCP] = verbs[IPV4] | verbs[TCP],
> > > > + [IPV4_UDP] = verbs[IPV4] | verbs[UDP],
> > > > + [IPV6_TCP] = verbs[IPV6] | verbs[TCP],
> > > > + [IPV6_UDP] = verbs[IPV6] | verbs[UDP],
> > > > };
> > > > + const uint64_t *in = verbs_to_dpdk ? verbs : dpdk;
> > > > + const uint64_t *out = verbs_to_dpdk ? dpdk : verbs;
> > > > uint64_t seen = 0;
> > > > uint64_t conv = 0;
> > > > unsigned int i;
> > > >
> > > > - if (!types)
>
> Types != 0 , we skip.
>
> > > > - return priv->hw_rss_sup;
> > > > - for (i = 0; i != RTE_DIM(in); ++i)
> > > > + if (!types) {
> > > > + if (!verbs_to_dpdk)
> > > > + return priv->hw_rss_sup;
> > > > + types = priv->hw_rss_sup;
> > > > + }
> > > > + for (i = 0; i != RTE_DIM(dpdk); ++i)
> > > > if (types & in[i]) {
> > > > seen |= types & in[i];
> > > > conv |= out[i];
> > > > }
>
>
> After this loop:
> seen = (IBV_RX_HASH_SRC_PORT_IPV4 | IBV_RX_HASH_DST_IPV4)
> conv = dpdk[IPV4] | dpdk[IPV4_TCP] | dpdk[IPV4_UDP]
Now I understand the mistake.
> > > > - if ((conv & priv->hw_rss_sup) == conv && !(types & ~seen))
> > > > + if ((verbs_to_dpdk || (conv & priv->hw_rss_sup) == conv) &&
> > > > + !(types & ~seen))
>
> All types were seen && verbs_to_dpdk hence returning that PMD support RSS based on IPV4 | TCPV4 | UDPV4
> While according to the reported capabilities form kernel only IPV4 is supported.
Indeed, although this combination never occurs in practice, that's why I
didn't see any issue at first. I'll submit a fixed version. Thanks.
> > > > return conv;
> > > > rte_errno = ENOTSUP;
> > > > return (uint64_t)-1;
> > > > }
> > > >
> > >
@@ -587,8 +587,7 @@ mlx4_dev_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *info)
ETH_LINK_SPEED_20G |
ETH_LINK_SPEED_40G |
ETH_LINK_SPEED_56G;
- info->flow_type_rss_offloads =
- mlx4_ibv_to_rss_types(priv->hw_rss_sup);
+ info->flow_type_rss_offloads = mlx4_conv_rss_types(priv, 0, 1);
}
/**
@@ -42,41 +42,6 @@
#include "mlx4_rxtx.h"
#include "mlx4_utils.h"
-/** IBV supported RSS hash functions combinations */
-#define MLX4_IBV_IPV4_HF ( \
- IBV_RX_HASH_SRC_IPV4 | \
- IBV_RX_HASH_DST_IPV4)
-#define MLX4_IBV_IPV6_HF ( \
- IBV_RX_HASH_SRC_IPV6 | \
- IBV_RX_HASH_DST_IPV6)
-#define MLX4_IBV_TCP_HF ( \
- IBV_RX_HASH_SRC_PORT_TCP | \
- IBV_RX_HASH_DST_PORT_TCP)
-#define MLX4_IBV_UDP_HF ( \
- IBV_RX_HASH_SRC_PORT_UDP | \
- IBV_RX_HASH_DST_PORT_UDP)
-
-/** Supported RSS hash functions combinations */
-#define MLX4_RSS_IPV4_HF ( \
- ETH_RSS_IPV4 | \
- ETH_RSS_FRAG_IPV4 | \
- ETH_RSS_NONFRAG_IPV4_OTHER)
-#define MLX4_RSS_IPV6_HF ( \
- ETH_RSS_IPV6 | \
- ETH_RSS_FRAG_IPV6 | \
- ETH_RSS_NONFRAG_IPV6_OTHER | \
- ETH_RSS_IPV6_EX)
-#define MLX4_RSS_IPV4_TCP_HF ( \
- ETH_RSS_NONFRAG_IPV4_TCP)
-#define MLX4_RSS_IPV6_TCP_HF ( \
- ETH_RSS_NONFRAG_IPV6_TCP | \
- ETH_RSS_IPV6_TCP_EX)
-#define MLX4_RSS_IPV4_UDP_HF ( \
- ETH_RSS_NONFRAG_IPV4_UDP)
-#define MLX4_RSS_IPV6_UDP_HF ( \
- ETH_RSS_NONFRAG_IPV6_UDP | \
- ETH_RSS_IPV6_UDP_EX)
-
/** Static initializer for a list of subsequent item types. */
#define NEXT_ITEM(...) \
(const enum rte_flow_item_type []){ \
@@ -111,7 +76,7 @@ struct mlx4_drop {
};
/**
- * Convert DPDK RSS hash types to their Verbs equivalent.
+ * Convert supported RSS hash field types between DPDK and Verbs formats.
*
* This function returns the supported (default) set when @p types has
* special value 0.
@@ -119,106 +84,76 @@ struct mlx4_drop {
* @param priv
* Pointer to private structure.
* @param types
- * Hash types in DPDK format (see struct rte_eth_rss_conf).
+ * Depending on @p verbs_to_dpdk, hash types in either DPDK (see struct
+ * rte_eth_rss_conf) or Verbs format.
+ * @param verbs_to_dpdk
+ * A zero value converts @p types from DPDK to Verbs, a nonzero value
+ * performs the reverse operation.
*
* @return
- * A valid Verbs RSS hash fields mask for mlx4 on success, (uint64_t)-1
- * otherwise and rte_errno is set.
+ * Converted RSS hash fields on success, (uint64_t)-1 otherwise and
+ * rte_errno is set.
*/
uint64_t
-mlx4_conv_rss_types(struct priv *priv, uint64_t types)
+mlx4_conv_rss_types(struct priv *priv, uint64_t types, int verbs_to_dpdk)
{
- enum { IPV4, IPV6, TCP, UDP, };
- static const uint64_t in[] = {
+ enum {
+ INNER, IPV4, IPV6, TCP, UDP,
+ IPV4_TCP, IPV4_UDP, IPV6_TCP, IPV6_UDP,
+ };
+ static const uint64_t dpdk[] = {
+ [INNER] = 0,
[IPV4] = (ETH_RSS_IPV4 |
ETH_RSS_FRAG_IPV4 |
- ETH_RSS_NONFRAG_IPV4_TCP |
- ETH_RSS_NONFRAG_IPV4_UDP |
ETH_RSS_NONFRAG_IPV4_OTHER),
[IPV6] = (ETH_RSS_IPV6 |
ETH_RSS_FRAG_IPV6 |
- ETH_RSS_NONFRAG_IPV6_TCP |
- ETH_RSS_NONFRAG_IPV6_UDP |
ETH_RSS_NONFRAG_IPV6_OTHER |
- ETH_RSS_IPV6_EX |
- ETH_RSS_IPV6_TCP_EX |
- ETH_RSS_IPV6_UDP_EX),
- [TCP] = (ETH_RSS_NONFRAG_IPV4_TCP |
- ETH_RSS_NONFRAG_IPV6_TCP |
- ETH_RSS_IPV6_TCP_EX),
- [UDP] = (ETH_RSS_NONFRAG_IPV4_UDP |
- ETH_RSS_NONFRAG_IPV6_UDP |
- ETH_RSS_IPV6_UDP_EX),
+ ETH_RSS_IPV6_EX),
+ [TCP] = 0,
+ [UDP] = 0,
+ [IPV4_TCP] = ETH_RSS_NONFRAG_IPV4_TCP,
+ [IPV4_UDP] = ETH_RSS_NONFRAG_IPV4_UDP,
+ [IPV6_TCP] = (ETH_RSS_NONFRAG_IPV6_TCP |
+ ETH_RSS_IPV6_TCP_EX),
+ [IPV6_UDP] = (ETH_RSS_NONFRAG_IPV6_UDP |
+ ETH_RSS_IPV6_UDP_EX),
};
- static const uint64_t out[RTE_DIM(in)] = {
+ const uint64_t verbs[RTE_DIM(dpdk)] = {
+ [INNER] = IBV_RX_HASH_INNER,
[IPV4] = IBV_RX_HASH_SRC_IPV4 | IBV_RX_HASH_DST_IPV4,
[IPV6] = IBV_RX_HASH_SRC_IPV6 | IBV_RX_HASH_DST_IPV6,
[TCP] = IBV_RX_HASH_SRC_PORT_TCP | IBV_RX_HASH_DST_PORT_TCP,
[UDP] = IBV_RX_HASH_SRC_PORT_UDP | IBV_RX_HASH_DST_PORT_UDP,
+ [IPV4_TCP] = verbs[IPV4] | verbs[TCP],
+ [IPV4_UDP] = verbs[IPV4] | verbs[UDP],
+ [IPV6_TCP] = verbs[IPV6] | verbs[TCP],
+ [IPV6_UDP] = verbs[IPV6] | verbs[UDP],
};
+ const uint64_t *in = verbs_to_dpdk ? verbs : dpdk;
+ const uint64_t *out = verbs_to_dpdk ? dpdk : verbs;
uint64_t seen = 0;
uint64_t conv = 0;
unsigned int i;
- if (!types)
- return priv->hw_rss_sup;
- for (i = 0; i != RTE_DIM(in); ++i)
+ if (!types) {
+ if (!verbs_to_dpdk)
+ return priv->hw_rss_sup;
+ types = priv->hw_rss_sup;
+ }
+ for (i = 0; i != RTE_DIM(dpdk); ++i)
if (types & in[i]) {
seen |= types & in[i];
conv |= out[i];
}
- if ((conv & priv->hw_rss_sup) == conv && !(types & ~seen))
+ if ((verbs_to_dpdk || (conv & priv->hw_rss_sup) == conv) &&
+ !(types & ~seen))
return conv;
rte_errno = ENOTSUP;
return (uint64_t)-1;
}
/**
- * Convert verbs RSS types to their DPDK equivalents.
- *
- * This function returns a group of RSS DPDK types given their equivalent group
- * of verbs types.
- * For example both source IPv4 and destination IPv4 verbs types are converted
- * into their equivalent RSS group types. If each of these verbs types existed
- * exclusively - no conversion would take place.
- *
- * @param types
- * RSS hash types in verbs format.
- *
- * @return
- * DPDK RSS hash fields supported by mlx4.
- */
-uint64_t
-mlx4_ibv_to_rss_types(uint64_t types)
-{
- enum { IPV4, IPV6, IPV4_TCP, IPV6_TCP, IPV4_UDP, IPV6_UDP};
-
- static const uint64_t in[] = {
- [IPV4] = MLX4_IBV_IPV4_HF,
- [IPV6] = MLX4_IBV_IPV6_HF,
- [IPV4_TCP] = MLX4_IBV_IPV4_HF | MLX4_IBV_TCP_HF,
- [IPV6_TCP] = MLX4_IBV_IPV6_HF | MLX4_IBV_TCP_HF,
- [IPV4_UDP] = MLX4_IBV_IPV4_HF | MLX4_IBV_UDP_HF,
- [IPV6_UDP] = MLX4_IBV_IPV6_HF | MLX4_IBV_UDP_HF,
- };
- static const uint64_t out[RTE_DIM(in)] = {
- [IPV4] = MLX4_RSS_IPV4_HF,
- [IPV6] = MLX4_RSS_IPV6_HF,
- [IPV4_TCP] = MLX4_RSS_IPV4_HF | MLX4_RSS_IPV4_TCP_HF,
- [IPV6_TCP] = MLX4_RSS_IPV6_HF | MLX4_RSS_IPV6_TCP_HF,
- [IPV4_UDP] = MLX4_RSS_IPV4_HF | MLX4_RSS_IPV4_UDP_HF,
- [IPV6_UDP] = MLX4_RSS_IPV6_HF | MLX4_RSS_IPV6_UDP_HF,
- };
- uint64_t conv = 0;
- unsigned int i;
-
- for (i = 0; i != RTE_DIM(in); ++i)
- if ((types & in[i]) == in[i])
- conv |= out[i];
- return conv;
-}
-
-/**
* Merge Ethernet pattern item into flow rule handle.
*
* Additional mlx4-specific constraints on supported fields:
@@ -890,7 +825,7 @@ mlx4_flow_prepare(struct priv *priv,
goto exit_action_not_supported;
}
rte_errno = 0;
- fields = mlx4_conv_rss_types(priv, rss->types);
+ fields = mlx4_conv_rss_types(priv, rss->types, 0);
if (fields == (uint64_t)-1 && rte_errno) {
msg = "unsupported RSS hash type requested";
goto exit_action_not_supported;
@@ -48,8 +48,8 @@ struct rte_flow {
/* mlx4_flow.c */
-uint64_t mlx4_conv_rss_types(struct priv *priv, uint64_t types);
-uint64_t mlx4_ibv_to_rss_types(uint64_t ibv_rss_types);
+uint64_t mlx4_conv_rss_types(struct priv *priv, uint64_t types,
+ int verbs_to_dpdk);
int mlx4_flow_sync(struct priv *priv, struct rte_flow_error *error);
void mlx4_flow_clean(struct priv *priv);
int mlx4_filter_ctrl(struct rte_eth_dev *dev,