On 10/9/19 10:42 AM, Su, Simei wrote:
Hi, Andrew

-----Original Message-----
From: Andrew Rybchenko [mailto:arybche...@solarflare.com]
Sent: Wednesday, October 9, 2019 3:18 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 v11 2/3] ethdev: extend RSS offload types

On 10/9/19 9:57 AM, 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>

[snip]

a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 7722f70..ef59ed5 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -4034,6 +4048,27 @@ int rte_eth_dev_adjust_nb_rx_tx_desc(uint16_t port_id,
   void *
   rte_eth_dev_get_sec_ctx(uint16_t port_id);

+/**
+ * If SRC_ONLY and DST_ONLY of the same level are used
+ * simultaneously, it is the same case as none of them
+ * are added.
+ *
+ * @param rss_hf
+ *   RSS types with SRC/DST_ONLY.
+ * @return
+ *   RSS types.
+ */
+static inline uint64_t
+strip_out_src_dst_only(uint64_t rss_hf)
Inline function in public header without corresponding prefix is a bad idea.
Please, move it to C file and I think that inline should be removed.
Let the compiler do its job.
   Because I also need to check simultaneous use of SRC/DST_ONLY in PMD driver.
   In order to call strip_out_src_dst_only() function directly in driver,
   I put it in header file and declare it as inline function.

At bare minimum it should have rte_eth_dev_ prefix.
Also from the name it is not clear that it is about RSS etc.
Not sure why you need it in driver as well, hopefully I'll see.

Andrew.

Reply via email to