From: Andrew Rybchenko [mailto:arybche...@solarflare.com]
Sent: Thursday, September 26, 2019 5:16 PM
To: Su, Simei <simei...@intel.com>; Zhang, Qi Z <qi.z.zh...@intel.com>; Wu,
Jingjing <jingjing...@intel.com>; Ye, Xiaolong <xiaolong...@intel.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v3 1/2] ethdev: extend RSS offload types
On 9/25/19 5:06 PM, Simei Su wrote:
This patch cover two aspects:
(1)decouple RTE_ETH_FLOW_* and ETH_RSS_*. Because both serve
different purposes.
(2)reserve several bits as input set selection from the high
end of the 64 bits. It is combined with exisiting ETH_RSS_*
to represent rss types.
If the patch covers two aspects why it is one patch instead of two?
It would be useful to motivate decouple a bit getter and provide
details since "different purposes" are hardly very useful. Which
purposes?
[Qi: ]Though the second purpose depends on the first one, but I agree, it’s
better to separate them in two patches
for example:
ETH_RSS_IPV4 | ETH_RSS_L3_SRC_ONLY: hash on src ip address only
Is (ETH_RSS_IPV4 | ETH_RSS_L3_SRC_ONLY | ETH_RSS_L3_DST_ONLY)
valid and an equivalent to ETH_RSS_IPV4 only?
[Qi:] The word “only” should be an exclusive attribute , so I don’t think
ETH_RSS_L3_SRC_ONLY | ETH_RSS_L3_DST_ONLY is a valid case,
Such configure should be rejected by all PMD ( better be handled in generic
API, but I did see a good place to add so far)
But I guess that should not be a problem, since nobody will intend to use with
that way, also exist PMD may reject any new bit it don't know.
If yes, shouldn't generic API care about it or each driver should do it?
Similar question is applicable to L4.
ETH_RSS_IPV4_UDP | ETH_RSS_L4_DST_ONLY: hash on src/dst IP and
dst UDP port
ETH_RSS_L2_PAYLOAD | ETH_RSS_L2_DST_ONLY: hash on dst mac address
I'm a bit confused by L2_PAYLOAD | L2_DST_ONLY. Does L2_PAYLOAD
mean entire L2 frame including header and payload?
[Qi:] yes, seems we are missing L2_RSS_ETH.
Regards
Qi
Signed-off-by: Simei Su <simei...@intel.com>
[snip]
