Re: [RFC PATCH 1/3] veth: implement support for set_channel ethtool op

[Paolo Abeni <pabeni@redhat.com>]

Hello,

On Sun, 2021-07-11 at 19:44 -0600, David Ahern wrote:
> On 7/9/21 1:54 PM, Jakub Kicinski wrote:
> > On Fri,  9 Jul 2021 11:39:48 +0200 Paolo Abeni wrote:
> > > +	/* accept changes only on rx/tx */
> > > +	if (ch->combined_count != min(dev->real_num_rx_queues, dev->real_num_tx_queues))
> > > +		return -EINVAL;
> > 
> > Ah damn, I must have missed the get_channels being added. I believe the
> > correct interpretation of the params is rx means NAPI with just Rx
> > queue(s), tx NAPI with just Tx queue(s) and combined has both.
> > IOW combined != min(rx, tx).
> > Instead real_rx = combined + rx; real_tx = combined + tx.
> > Can we still change this?
> 
> Is it not an 'either' / 'or' situation? ie., you can either control the
> number of Rx and Tx queues or you control the combined value but not
> both. That is what I recall from nics (e.g., ConnectX).

Thanks for the feedback. My understanding was quite alike what David
stated - and indeed that is what ConnectX enforces AFAICS. Anyhow the
core ethtool code allows for what Jackub said, so I guess I need to
deal with that.

@Jakub: if we are still on time about changing the veth_get_channel()
exposed behaviour, what about just showing nr combined == 0 and
enforcing comined_max == 0? that would both describe more closely the
veth architecture and will make the code simpler - beyond fixing the
current uncorrect nr channels report.

Thanks!

Paolo