[dpdk-dev,v2,1/2] ethdev: add supported hash function check

Message ID 20180417142425.159378-1-xuemingl@mellanox.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Xueming Li April 17, 2018, 2:24 p.m. UTC
  Add supported RSS hash function check in device configuration to
have better error verbosity for application developers.

Signed-off-by: Xueming Li <xuemingl@mellanox.com>
---
 lib/librte_ether/rte_ethdev.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
  

Comments

Thomas Monjalon April 17, 2018, 10 p.m. UTC | #1
17/04/2018 16:24, Xueming Li:
> +	/* Check that device supports requested rss hash functions. */
> +	if ((dev_info.flow_type_rss_offloads |
> +	     dev_conf->rx_adv_conf.rss_conf.rss_hf) !=
> +	    dev_info.flow_type_rss_offloads) {
> +		RTE_PMD_DEBUG_TRACE("ethdev port_id=%d invalid rss_hf: 0x%lx, valid value: 0x%lx\n",
> +				    port_id,
> +				    dev_conf->rx_adv_conf.rss_conf.rss_hf,
> +				    dev_info.flow_type_rss_offloads);
> +		return -EINVAL;
> +	}

Please use PRIx64 and test 32-bit compilation.

Reminder from this recent post:
        http://dpdk.org/ml/archives/dev/2018-February/090882.html
"
Most of the times, using %l is wrong (except when printing a long).
So next time you write %l, please think "I am probably wrong".
"
  
Xueming Li April 18, 2018, 11:55 a.m. UTC | #2
Hi Thomas,

> -----Original Message-----

> From: Thomas Monjalon <thomas@monjalon.net>

> Sent: Wednesday, April 18, 2018 6:01 AM

> To: Xueming(Steven) Li <xuemingl@mellanox.com>

> Cc: dev@dpdk.org; Shahaf Shuler <shahafs@mellanox.com>; Nelio Laranjeiro <notifications@github.com>;

> Wenzhuo Lu <wenzhuo.lu@intel.com>; Jingjing Wu <jingjing.wu@intel.com>

> Subject: Re: [dpdk-dev] [PATCH v2 1/2] ethdev: add supported hash function check

> 

> 17/04/2018 16:24, Xueming Li:

> > +	/* Check that device supports requested rss hash functions. */

> > +	if ((dev_info.flow_type_rss_offloads |

> > +	     dev_conf->rx_adv_conf.rss_conf.rss_hf) !=

> > +	    dev_info.flow_type_rss_offloads) {

> > +		RTE_PMD_DEBUG_TRACE("ethdev port_id=%d invalid rss_hf: 0x%lx, valid value: 0x%lx\n",

> > +				    port_id,

> > +				    dev_conf->rx_adv_conf.rss_conf.rss_hf,

> > +				    dev_info.flow_type_rss_offloads);

> > +		return -EINVAL;

> > +	}

> 

> Please use PRIx64 and test 32-bit compilation.

> 

> Reminder from this recent post:

> 

> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdpdk.org%2Fml%2Farchives%2Fdev%2F201

> 8-

> February%2F090882.html&data=02%7C01%7Cxuemingl%40mellanox.com%7C5508bc716aac41b932fb08d5a4aeb241%7Ca65

> 2971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636595992591300475&sdata=1hirZ1j7VqCMnrzngZFAuQj42OACfxFUgYgzy

> VQyw%2F4%3D&reserved=0

> "

> Most of the times, using %l is wrong (except when printing a long).

> So next time you write %l, please think "I am probably wrong".

> "


Thanks, got following warning from checkpatch when applying this rule to my other patchset, is it normal?
  CHECK:CAMELCASE: Avoid CamelCase: <PRIx64>
  #49: FILE: drivers/net/mlx5/mlx5_flow.c:2083:
  +               " hash:%" PRIx64 "/%u specs:%hhu(%hu), priority:%hu, type:%d,"
  
Thomas Monjalon April 18, 2018, 12:15 p.m. UTC | #3
18/04/2018 13:55, Xueming(Steven) Li:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 17/04/2018 16:24, Xueming Li:
> > > +	/* Check that device supports requested rss hash functions. */
> > > +	if ((dev_info.flow_type_rss_offloads |
> > > +	     dev_conf->rx_adv_conf.rss_conf.rss_hf) !=
> > > +	    dev_info.flow_type_rss_offloads) {
> > > +		RTE_PMD_DEBUG_TRACE("ethdev port_id=%d invalid rss_hf: 0x%lx, valid value: 0x%lx\n",
> > > +				    port_id,
> > > +				    dev_conf->rx_adv_conf.rss_conf.rss_hf,
> > > +				    dev_info.flow_type_rss_offloads);
> > > +		return -EINVAL;
> > > +	}
> > 
> > Please use PRIx64 and test 32-bit compilation.
> > 
> > Reminder from this recent post:
> > 
> > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdpdk.org%2Fml%2Farchives%2Fdev%2F201
> > 8-
> > February%2F090882.html&data=02%7C01%7Cxuemingl%40mellanox.com%7C5508bc716aac41b932fb08d5a4aeb241%7Ca65
> > 2971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636595992591300475&sdata=1hirZ1j7VqCMnrzngZFAuQj42OACfxFUgYgzy
> > VQyw%2F4%3D&reserved=0
> > "
> > Most of the times, using %l is wrong (except when printing a long).
> > So next time you write %l, please think "I am probably wrong".
> > "
> 
> Thanks, got following warning from checkpatch when applying this rule to my other patchset, is it normal?
>   CHECK:CAMELCASE: Avoid CamelCase: <PRIx64>
>   #49: FILE: drivers/net/mlx5/mlx5_flow.c:2083:
>   +               " hash:%" PRIx64 "/%u specs:%hhu(%hu), priority:%hu, type:%d,"
> 

Yes it is "normal".
Something we should fix in checkpatch.
  

Patch

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 209796d46..a8e122781 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1179,6 +1179,17 @@  rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
 							ETHER_MAX_LEN;
 	}
 
+	/* Check that device supports requested rss hash functions. */
+	if ((dev_info.flow_type_rss_offloads |
+	     dev_conf->rx_adv_conf.rss_conf.rss_hf) !=
+	    dev_info.flow_type_rss_offloads) {
+		RTE_PMD_DEBUG_TRACE("ethdev port_id=%d invalid rss_hf: 0x%lx, valid value: 0x%lx\n",
+				    port_id,
+				    dev_conf->rx_adv_conf.rss_conf.rss_hf,
+				    dev_info.flow_type_rss_offloads);
+		return -EINVAL;
+	}
+
 	/*
 	 * Setup new number of RX/TX queues and reconfigure device.
 	 */
@@ -2832,9 +2843,19 @@  rte_eth_dev_rss_hash_update(uint16_t port_id,
 			    struct rte_eth_rss_conf *rss_conf)
 {
 	struct rte_eth_dev *dev;
+	struct rte_eth_dev_info dev_info = {0};
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
+	rte_eth_dev_info_get(port_id, &dev_info);
+	if ((dev_info.flow_type_rss_offloads | rss_conf->rss_hf) !=
+	    dev_info.flow_type_rss_offloads) {
+		RTE_PMD_DEBUG_TRACE("ethdev port_id=%d invalid rss_hf: 0x%lx, valid value: 0x%lx\n",
+				    port_id,
+				    rss_conf->rss_hf,
+				    dev_info.flow_type_rss_offloads);
+		return -EINVAL;
+	}
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rss_hash_update, -ENOTSUP);
 	return eth_err(port_id, (*dev->dev_ops->rss_hash_update)(dev,
 								 rss_conf));