netdev - RE: [PATCH v2 3/6] ethtool: introduce new ioctl for per-queue settings

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: <146CDE3B8C383A4B88452B498CB0FBBBFD9C3FA9@ORSMSX157.amr.corp.intel.com>
Date:   Thu, 7 Feb 2019 23:41:24 +0000
From:   "Nunley, Nicholas D" <nicholas.d.nunley@...el.com>
To:     Michal Kubecek <mkubecek@...e.cz>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:     "Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
        "linville@...driver.com" <linville@...driver.com>,
        "nhorman@...hat.com" <nhorman@...hat.com>,
        "sassmann@...hat.com" <sassmann@...hat.com>
Subject: RE: [PATCH v2 3/6] ethtool: introduce new ioctl for per-queue
 settings

> -----Original Message-----
> From: Michal Kubecek [mailto:mkubecek@...e.cz]
> Sent: Wednesday, February 6, 2019 2:33 AM
> To: netdev@...r.kernel.org
> Cc: Kirsher, Jeffrey T <jeffrey.t.kirsher@...el.com>; linville@...driver.com;
> Nunley, Nicholas D <nicholas.d.nunley@...el.com>; nhorman@...hat.com;
> sassmann@...hat.com
> Subject: Re: [PATCH v2 3/6] ethtool: introduce new ioctl for per-queue
> settings
> 
> On Tue, Feb 05, 2019 at 04:01:03PM -0800, Jeff Kirsher wrote:
> > +static int find_max_num_queues(struct cmd_context *ctx) {
> > +	struct ethtool_channels echannels;
> > +
> > +	echannels.cmd = ETHTOOL_GCHANNELS;
> > +	if (send_ioctl(ctx, &echannels))
> > +		return -1;
> > +
> > +	return MAX(MAX(echannels.rx_count, echannels.tx_count),
> > +		   echannels.combined_count);
> > +}
> 
> Is the outer MAX() correct here? From the documentation to -L option, it
> rather seems we might want
> 
> 	return MAX(echannels.rx_count, echannels.tx_count) +
> 	       echannels.combined_count;
> 
> But I can't find any NIC around which would have non-zero rx_count or
> tx_count so that I cannot check.

I think the original assumption here must have been that drivers either use separate Tx/Rx channels or support the combined approach, but never both at the same time. All Intel drivers only support the combined method so I didn't think to second guess this detail of the original implementation, however, I've since looked through the uses of get/set_channels elsewhere in the kernel and it looks like there are a few drivers that support both methods simultaneously, so that clearly needs to be supported too. Your suggestion above looks like the right thing to do.

The device queue-index-to-channel mapping could be a little ambiguous in the mixed mode (and is left as an implementation detail of the individual driver), but I suppose the most sensible approach would be to index through the combined channels first, then move on to the individual Tx/Rx channels, so there shouldn't be any future issues here if these drivers want to add support for per-queue commands.