netdev - Re: [RFC PATCH 1/3] veth: implement support for set

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 for Android: free password hash cracker in your pocket

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

Message-ID: <26432bbc3556fd23bd58f6d359395e5dfa2eaf8c.camel@redhat.com>
Date:   Mon, 12 Jul 2021 12:45:13 +0200
From:   Paolo Abeni <pabeni@...hat.com>
To:     David Ahern <dsahern@...il.com>, Jakub Kicinski <kuba@...nel.org>
Cc:     netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
        toke@...hat.com
Subject: Re: [RFC PATCH 1/3] veth: implement support for set_channel ethtool
 op

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