netdev - Re: [PATCH net-next] hinic: add set_channels ethtool

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200515195726.GE21714@lion.mk-sys.cz>
Date:   Fri, 15 May 2020 21:57:26 +0200
From:   Michal Kubecek <mkubecek@...e.cz>
To:     netdev@...r.kernel.org
Cc:     Luo bin <luobin9@...wei.com>, davem@...emloft.net,
        linux-kernel@...r.kernel.org, luoxianjun@...wei.com,
        yin.yinshi@...wei.com, cloud.wangxiaoyun@...wei.com
Subject: Re: [PATCH net-next] hinic: add set_channels ethtool_ops support

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