Re: [PATCH v2 3/7] cxgbe: add support to update RSS hash configuration and key

[Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>]

On Thursday, March 03/08/18, 2018 at 19:04:31 +0530, Ferruh Yigit wrote:
> On 2/28/2018 6:04 PM, Rahul Lakkireddy wrote:
> > From: Kumar Sanghvi <kumaras@chelsio.com>
> > 
> > Add firmware API for updating RSS hash configuration and key.  Move
> > RSS hash configuration from cxgb4_write_rss() to a separate function
> > cxgbe_write_rss_conf().
> > 
> > Also, rename cxgb4_write_rss() to cxgbe_write_rss() for consistency.
> > 
> > Original work by Surendra Mobiya <surendra@chelsio.com>
> > 
> > Signed-off-by: Kumar Sanghvi <kumaras@chelsio.com>
> > Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
> 
> <...>
> 
> > @@ -985,6 +1016,7 @@ static const struct eth_dev_ops cxgbe_eth_dev_ops = {
> >  	.get_eeprom		= cxgbe_get_eeprom,
> >  	.set_eeprom		= cxgbe_set_eeprom,
> >  	.get_reg		= cxgbe_get_regs,
> > +	.rss_hash_update	= cxgbe_dev_rss_hash_update,
> 
> Also in cxgbe_dev_info_get(), rte_eth_dev_info->flow_type_rss_offloads should be
> updated with supported RSS hash functions.
> 
> I don't see any usage of "flow_type_rss_offloads" in sample apps except testpmd
> is using it to print the log, but that is the way to notify applications about
> support.
> 
> You don't need to send a new version of patchset for this, please send an
> incremental patch and I can squash it into this set.
> 
> <...>
> 

Ok, will update supported RSS hash functions in flow_type_rss_offloads.
Will send a fix.

> > +int cxgbe_write_rss_conf(const struct port_info *pi, uint64_t rss_hf)
> > +{
> > +	struct adapter *adapter = pi->adapter;
> > +	const struct sge_eth_rxq *rxq;
> > +	u64 flags = 0;
> > +	u16 rss;
> > +	int err;
> > +
> > +	/*  Should never be called before setting up sge eth rx queues */
> > +	if (!(adapter->flags & FULL_INIT_DONE)) {
> > +		dev_err(adap, "%s No RXQs available on port %d\n",
> > +			__func__, pi->port_id);
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (rss_hf & ETH_RSS_IPV4)
> > +		flags |= F_FW_RSS_VI_CONFIG_CMD_IP4TWOTUPEN;
> > +
> > +	if (rss_hf & ETH_RSS_NONFRAG_IPV4_TCP)
> > +		flags |= F_FW_RSS_VI_CONFIG_CMD_IP4FOURTUPEN;
> > +
> > +	if (rss_hf & ETH_RSS_NONFRAG_IPV4_UDP)
> > +		flags |= F_FW_RSS_VI_CONFIG_CMD_IP4FOURTUPEN |
> > +			 F_FW_RSS_VI_CONFIG_CMD_UDPEN;
> > +
> > +	if (rss_hf & ETH_RSS_IPV6)
> > +		flags |= F_FW_RSS_VI_CONFIG_CMD_IP6TWOTUPEN;
> > +
> > +	if (rss_hf & ETH_RSS_NONFRAG_IPV6_TCP)
> > +		flags |= F_FW_RSS_VI_CONFIG_CMD_IP6FOURTUPEN;
> > +
> > +	if (rss_hf & ETH_RSS_NONFRAG_IPV6_UDP)
> > +		flags |= F_FW_RSS_VI_CONFIG_CMD_IP6FOURTUPEN |
> > +			 F_FW_RSS_VI_CONFIG_CMD_UDPEN;
> > +
> > +	rxq = &adapter->sge.ethrxq[pi->first_qset];
> > +	rss = rxq[0].rspq.abs_id;
> 
> What if driver gets a rss_hf that is not supported, will the API report back an
> error to the application?
> 

Good catch!  Will send a fix.

Thanks,
Rahul