> -----Original Message----- > From: Michal Kubecek [mailto:mkube...@suse.cz] > Sent: Wednesday, February 6, 2019 2:33 AM > To: netdev@vger.kernel.org > Cc: Kirsher, Jeffrey T <jeffrey.t.kirs...@intel.com>; linvi...@tuxdriver.com; > Nunley, Nicholas D <nicholas.d.nun...@intel.com>; nhor...@redhat.com; > sassm...@redhat.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.
