[dpdk-dev] [PATCH v2 07/15] ethdev: flatten RSS configuration in flow API
Andrew Rybchenko
arybchenko at solarflare.com
Sat Apr 7 11:05:51 CEST 2018
On 04/06/2018 04:25 PM, Adrien Mazarguil wrote:
> Since its inception, the rte_flow RSS action has been relying in part on
> external struct rte_eth_rss_conf for compatibility with the legacy RSS API.
> This structure lacks parameters such as the hash algorithm to use, and more
> recently, a method to tell which layer RSS should be performed on [1].
>
> Given struct rte_eth_rss_conf will never be flexible enough to represent a
> complete RSS configuration (e.g. RETA table), this patch supersedes it by
> extending the rte_flow RSS action directly.
>
> A subsequent patch will add a field to use a non-default RSS hash
> algorithm. To that end, a field named "types" replaces the field formerly
> known as "rss_hf" and standing for "RSS hash functions" as it was
> confusing. Actual RSS hash function types are defined by enum
> rte_eth_hash_function.
> This patch updates all PMDs and example applications accordingly.
>
> It breaks ABI compatibility for the following public functions:
>
> - rte_flow_copy()
> - rte_flow_create()
> - rte_flow_query()
> - rte_flow_validate()
>
> [1] commit 676b605182a5 ("doc: announce ethdev API change for RSS
> configuration")
>
> Signed-off-by: Adrien Mazarguil <adrien.mazarguil at 6wind.com>
> Cc: Xueming Li <xuemingl at mellanox.com>
> Cc: Ferruh Yigit <ferruh.yigit at intel.com>
> Cc: Thomas Monjalon <thomas at monjalon.net>
> Cc: Wenzhuo Lu <wenzhuo.lu at intel.com>
> Cc: Jingjing Wu <jingjing.wu at intel.com>
> Cc: Beilei Xing <beilei.xing at intel.com>
> Cc: Qi Zhang <qi.z.zhang at intel.com>
> Cc: Konstantin Ananyev <konstantin.ananyev at intel.com>
> Cc: Nelio Laranjeiro <nelio.laranjeiro at 6wind.com>
> Cc: Yongseok Koh <yskoh at mellanox.com>
> Cc: Andrew Rybchenko <arybchenko at solarflare.com>
> Cc: Pascal Mazon <pascal.mazon at 6wind.com>
> Cc: Radu Nicolau <radu.nicolau at intel.com>
> Cc: Akhil Goyal <akhil.goyal at nxp.com>
> ---
> app/test-pmd/cmdline_flow.c | 59 +++++-----
> app/test-pmd/config.c | 39 +++----
> doc/guides/prog_guide/rte_flow.rst | 22 ++--
> drivers/net/e1000/e1000_ethdev.h | 13 ++-
> drivers/net/e1000/igb_ethdev.c | 4 +-
> drivers/net/e1000/igb_flow.c | 31 ++---
> drivers/net/e1000/igb_rxtx.c | 51 +++++++--
> drivers/net/i40e/i40e_ethdev.c | 53 +++++++--
> drivers/net/i40e/i40e_ethdev.h | 15 ++-
> drivers/net/i40e/i40e_flow.c | 47 ++++----
> drivers/net/ixgbe/ixgbe_ethdev.c | 4 +-
> drivers/net/ixgbe/ixgbe_ethdev.h | 13 ++-
> drivers/net/ixgbe/ixgbe_flow.c | 30 ++---
> drivers/net/ixgbe/ixgbe_rxtx.c | 51 +++++++--
> drivers/net/mlx4/mlx4.c | 2 +-
> drivers/net/mlx4/mlx4_flow.c | 61 +++++-----
> drivers/net/mlx4/mlx4_flow.h | 2 +-
> drivers/net/mlx4/mlx4_rxq.c | 2 +-
> drivers/net/mlx4/mlx4_rxtx.h | 2 +-
> drivers/net/mlx5/mlx5_flow.c | 193 +++++++++++++++-----------------
> drivers/net/mlx5/mlx5_rxq.c | 22 ++--
> drivers/net/mlx5/mlx5_rxtx.h | 26 +++--
> drivers/net/sfc/sfc_flow.c | 21 ++--
> drivers/net/tap/tap_flow.c | 8 +-
> examples/ipsec-secgw/ipsec.c | 10 +-
> lib/librte_ether/rte_flow.c | 39 +++----
> lib/librte_ether/rte_flow.h | 6 +-
> 27 files changed, 473 insertions(+), 353 deletions(-)
<...>
> diff --git a/drivers/net/sfc/sfc_flow.c b/drivers/net/sfc/sfc_flow.c
> index 056405515..1a2c0299c 100644
> --- a/drivers/net/sfc/sfc_flow.c
> +++ b/drivers/net/sfc/sfc_flow.c
> @@ -1234,13 +1234,11 @@ sfc_flow_parse_rss(struct sfc_adapter *sa,
> struct sfc_rxq *rxq;
> unsigned int rxq_hw_index_min;
> unsigned int rxq_hw_index_max;
> - const struct rte_eth_rss_conf *rss_conf = rss->rss_conf;
> - uint64_t rss_hf;
> - uint8_t *rss_key = NULL;
> + const uint8_t *rss_key;
> struct sfc_flow_rss *sfc_rss_conf = &flow->rss_conf;
> unsigned int i;
>
> - if (rss->num == 0)
> + if (rss->queue_num == 0)
> return -EINVAL;
>
> rxq_sw_index = sa->rxq_count - 1;
> @@ -1248,7 +1246,7 @@ sfc_flow_parse_rss(struct sfc_adapter *sa,
> rxq_hw_index_min = rxq->hw_index;
> rxq_hw_index_max = 0;
>
> - for (i = 0; i < rss->num; ++i) {
> + for (i = 0; i < rss->queue_num; ++i) {
> rxq_sw_index = rss->queue[i];
>
> if (rxq_sw_index >= sa->rxq_count)
> @@ -1263,15 +1261,14 @@ sfc_flow_parse_rss(struct sfc_adapter *sa,
> rxq_hw_index_max = rxq->hw_index;
> }
>
> - rss_hf = (rss_conf != NULL) ? rss_conf->rss_hf : SFC_RSS_OFFLOADS;
Here we had a fallback to default rss_hf (now types) if rss_conf is
unspecified.
> - if ((rss_hf & ~SFC_RSS_OFFLOADS) != 0)
> + if ((rss->types & ~SFC_RSS_OFFLOADS) != 0)
> return -EINVAL;
>
> - if (rss_conf != NULL) {
> - if (rss_conf->rss_key_len != sizeof(sa->rss_key))
> + if (rss->key_len) {
> + if (rss->key_len != sizeof(sa->rss_key))
> return -EINVAL;
>
> - rss_key = rss_conf->rss_key;
> + rss_key = rss->key;
> } else {
> rss_key = sa->rss_key;
> }
> @@ -1280,11 +1277,11 @@ sfc_flow_parse_rss(struct sfc_adapter *sa,
>
> sfc_rss_conf->rxq_hw_index_min = rxq_hw_index_min;
> sfc_rss_conf->rxq_hw_index_max = rxq_hw_index_max;
> - sfc_rss_conf->rss_hash_types = sfc_rte_to_efx_hash_type(rss_hf);
> + sfc_rss_conf->rss_hash_types = sfc_rte_to_efx_hash_type(rss->types);
Now types go directly to mapping function and unspecified types (0)
will result in 0 rss_hash_types. Of course, it is a question how to treat
types==0. It is possible to say that it no RSS, but it does not make sense.
So, real options are device defaults (regardless configured on device level)
or device config (rx_adv.conf.rss_conf.rss_hf). I would prefer the later.
Please, document the intended behaviour in rte_flow.rst.
If the later is chosen, above we'll have a bug since fallback to fixed
default.
Just use sa->rss_hash_types as fallback. Something like:
if (rss->types)
sfc_rss_conf->rss_hash_types = sfc_rte_to_efx_hash_type(rss->types);
else
sfc_rss_conf->rss_hash_types =sa->rss_hash_types;
> rte_memcpy(sfc_rss_conf->rss_key, rss_key, sizeof(sa->rss_key));
>
> for (i = 0; i < RTE_DIM(sfc_rss_conf->rss_tbl); ++i) {
> - unsigned int rxq_sw_index = rss->queue[i % rss->num];
> + unsigned int rxq_sw_index = rss->queue[i % rss->queue_num];
> struct sfc_rxq *rxq = sa->rxq_info[rxq_sw_index].rxq;
>
> sfc_rss_conf->rss_tbl[i] = rxq->hw_index - rxq_hw_index_min;
<...>
More information about the dev
mailing list