On 10/1/2019 5:02 PM, Iremonger, Bernard wrote: > Hi Simei, > >> -----Original Message----- >> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Andrew Rybchenko >> Sent: Tuesday, October 1, 2019 3:49 PM >> To: Su, Simei <simei...@intel.com>; Zhang, Qi Z <qi.z.zh...@intel.com>; Ye, >> Xiaolong <xiaolong...@intel.com>; Yigit, Ferruh <ferruh.yi...@intel.com> >> Cc: dev@dpdk.org >> Subject: Re: [dpdk-dev] [PATCH v8 2/3] ethdev: extend RSS offload types >> >> On 10/1/19 5:36 PM, Simei Su wrote: >>> This patch reserves 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. >>> >>> Signed-off-by: Simei Su <simei...@intel.com> >>> Reviewed-by: Qi Zhang <qi.z.zh...@intel.com> >>> Acked-by: Ori Kam <or...@mellanox.com> >>> --- >>> lib/librte_ethdev/rte_ethdev.c | 22 ++++++++++++++++++++++ >>> lib/librte_ethdev/rte_ethdev.h | 14 ++++++++++++++ >>> 2 files changed, 36 insertions(+) >>> >>> diff --git a/lib/librte_ethdev/rte_ethdev.c >>> b/lib/librte_ethdev/rte_ethdev.c index af82360..5e5a974 100644 >>> --- a/lib/librte_ethdev/rte_ethdev.c >>> +++ b/lib/librte_ethdev/rte_ethdev.c >>> @@ -1269,6 +1269,17 @@ struct rte_eth_dev * >>> goto rollback; >>> } >>> >>> + /* simplified the SRC/DST_ONLY RSS offload modificaiton */ >>> + if (dev_conf->rx_adv_conf.rss_conf.rss_hf & >> ETH_RSS_L3_SRC_ONLY && >>> + dev_conf->rx_adv_conf.rss_conf.rss_hf & >> ETH_RSS_L3_DST_ONLY) >> >> I'm afraid some compiler versions could bark about missing parenthesis here. >> Consider to use something like: >> (rss_conf->rss_hf & (ETH_RSS_L3_SRC_ONLY | ETH_RSS_L3_DST_ONLY) == >> (ETH_RSS_L3_SRC_ONLY | ETH_RSS_L3_DST_ONLY)) May be parenthesis >> around & is not required. >> >>> + dev->data->dev_conf.rx_adv_conf.rss_conf.rss_hf >> &= >>> + ~(ETH_RSS_L3_SRC_ONLY | >> ETH_RSS_L3_DST_ONLY); >>> + >>> + if (dev_conf->rx_adv_conf.rss_conf.rss_hf & >> ETH_RSS_L4_SRC_ONLY && >>> + dev_conf->rx_adv_conf.rss_conf.rss_hf & >> ETH_RSS_L4_DST_ONLY) >>> + dev->data->dev_conf.rx_adv_conf.rss_conf.rss_hf >> &= >>> + ~(ETH_RSS_L4_SRC_ONLY | >> ETH_RSS_L4_DST_ONLY); >>> + >> >> I think it is wrong to duplicate the logic twice (here and below). >> Helper static function should be used to avoid it. >> >>> /* Check that device supports requested rss hash functions. */ >>> if ((dev_info.flow_type_rss_offloads | >>> dev_conf->rx_adv_conf.rss_conf.rss_hf) != @@ -3112,6 +3123,17 >>> @@ struct rte_eth_dev * >>> if (ret != 0) >>> return ret; >>> >>> + /* simplified the SRC/DST_ONLY RSS offload modificaiton */ >>> + if (rss_conf->rss_hf & ETH_RSS_L3_SRC_ONLY && >>> + rss_conf->rss_hf & ETH_RSS_L3_DST_ONLY) >>> + rss_conf->rss_hf &= >>> + ~(ETH_RSS_L3_SRC_ONLY | >> ETH_RSS_L3_DST_ONLY); >>> + >>> + if (rss_conf->rss_hf & ETH_RSS_L4_SRC_ONLY && >>> + rss_conf->rss_hf & ETH_RSS_L4_DST_ONLY) >>> + rss_conf->rss_hf &= >>> + ~(ETH_RSS_L4_SRC_ONLY | >> ETH_RSS_L4_DST_ONLY); >>> + >>> dev = &rte_eth_devices[port_id]; >>> if ((dev_info.flow_type_rss_offloads | rss_conf->rss_hf) != >>> dev_info.flow_type_rss_offloads) { diff --git >>> a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h >>> index 7722f70..6d61b84 100644 >>> --- a/lib/librte_ethdev/rte_ethdev.h >>> +++ b/lib/librte_ethdev/rte_ethdev.h >>> @@ -505,6 +505,20 @@ struct rte_eth_rss_conf { >>> #define ETH_RSS_GENEVE (1ULL << 20) >>> #define ETH_RSS_NVGRE (1ULL << 21) >>> >>> +/* >>> + * We use the following macros to combine with above ETH_RSS_* for >>> + * more specific input set selection. These bits are defined starting >>> + * from the high end of the 64 bits. >>> + * Note: If we use above ETH_RSS_* without SRC/DST_ONLY, it >>> +represents >>> + * both SRC and DST are taken into account. If SRC_ONLY and DST_ONLY >>> +of >>> + * the same level be used simultaneously, it is the same case as none >>> +of >>> + * them are added. >>> + */ >>> +#define ETH_RSS_L3_SRC_ONLY (1ULL << 63) >>> +#define ETH_RSS_L3_DST_ONLY (1ULL << 62) >>> +#define ETH_RSS_L4_SRC_ONLY (1ULL << 61) >>> +#define ETH_RSS_L4_DST_ONLY (1ULL << 60) >>> + >>> #define ETH_RSS_IP ( \ >>> ETH_RSS_IPV4 | \ >>> ETH_RSS_FRAG_IPV4 | \ > > This patch fails to apply to the latest master branch, a rebase may be needed.
Patch applies well to next-net, which this patch should be for, no rebase required.
