Re: [PATCH net-next] hinic: add set_channels ethtool_ops support

[Michal Kubecek]

On Fri, May 15, 2020 at 08:13:30PM +0200, Michal Kubecek wrote:
[...]
> > +int hinic_set_channels(struct net_device *netdev,
> > +		       struct ethtool_channels *channels)
> > +{
> > +	struct hinic_dev *nic_dev = netdev_priv(netdev);
> > +	unsigned int count = channels->combined_count;
> > +	int err;
> > +
> > +	if (!count) {
> > +		netif_err(nic_dev, drv, netdev,
> > +			  "Unsupported combined_count: 0\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (channels->tx_count || channels->rx_count || channels->other_count) {
> > +		netif_err(nic_dev, drv, netdev,
> > +			  "Setting rx/tx/other count not supported\n");
> > +		return -EINVAL;
> > +	}
> 
> With max_* reported as 0, these will be caught in ethnl_set_channels()
> or ethtool_set_channels().
> 
> > +	if (!(nic_dev->flags & HINIC_RSS_ENABLE)) {
> > +		netif_err(nic_dev, drv, netdev,
> > +			  "This function doesn't support RSS, only support 1 queue pair\n");
> > +		return -EOPNOTSUPP;
> > +	}
> 
> I'm not sure if the request should fail even if requested count is
> actually 1.

Thinking about it again, as long as you report max_combined=1 in this
case, anything higher than 1 would be rejected by general ethtool code
and 0 is rejected by the first check above so that you can in fact only
get here for combined_count=1 - and only for ioctl requests as netlink
code path won't call the ethtool_ops callback if there is no change.

Michal