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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20210712082330.12c2a7c9@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Mon, 12 Jul 2021 08:23:30 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Paolo Abeni <pabeni@...hat.com>
Cc:     David Ahern <dsahern@...il.com>, 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

On Mon, 12 Jul 2021 12:45:13 +0200 Paolo Abeni wrote:
> On Sun, 2021-07-11 at 19:44 -0600, David Ahern wrote:
> > On 7/9/21 1:54 PM, Jakub Kicinski wrote:  
> > > 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.

I thought Mellanox was doing something funny to reuse the rx as the
number of AF_XDP queues. Normal rings are not reported twice, they're
only reported as combined.

ethtool man page is relatively clear, unfortunately the kernel code 
is not and few read the man page. A channel is approximately an IRQ, 
not a queue, and IRQ can't be dedicated and combined simultaneously:

 "A channel is an IRQ and the set of queues that can trigger that IRQ."

 " rx N   Changes the number of channels with only receive queues."

> @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.

SGTM.