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.mazarg...@6wind.com>
Cc: Xueming Li <xuemi...@mellanox.com>
Cc: Ferruh Yigit <ferruh.yi...@intel.com>
Cc: Thomas Monjalon <tho...@monjalon.net>
Cc: Wenzhuo Lu <wenzhuo...@intel.com>
Cc: Jingjing Wu <jingjing...@intel.com>
Cc: Beilei Xing <beilei.x...@intel.com>
Cc: Qi Zhang <qi.z.zh...@intel.com>
Cc: Konstantin Ananyev <konstantin.anan...@intel.com>
Cc: Nelio Laranjeiro <nelio.laranje...@6wind.com>
Cc: Yongseok Koh <ys...@mellanox.com>
Cc: Andrew Rybchenko <arybche...@solarflare.com>
Cc: Pascal Mazon <pascal.ma...@6wind.com>
Cc: Radu Nicolau <radu.nico...@intel.com>
Cc: Akhil Goyal <akhil.go...@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;
<...>
