netdev - Re: [PATCH bpf-next v4 17/17] net/mlx5e: Add XSK zero-copy support

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] [day] [month] [year] [list]

Date:   Tue, 18 Jun 2019 12:00:18 +0000
From:   Maxim Mikityanskiy <maximmi@...lanox.com>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>
CC:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Björn Töpel <bjorn.topel@...el.com>,
        Magnus Karlsson <magnus.karlsson@...el.com>,
        "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Saeed Mahameed <saeedm@...lanox.com>,
        Jonathan Lemon <bsd@...com>,
        Tariq Toukan <tariqt@...lanox.com>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        Maciej Fijalkowski <maciejromanfijalkowski@...il.com>
Subject: Re: [PATCH bpf-next v4 17/17] net/mlx5e: Add XSK zero-copy support

On 2019-06-15 18:42, Jakub Kicinski wrote:
> On Wed, 12 Jun 2019 15:57:09 +0000, Maxim Mikityanskiy wrote:
>> @@ -390,6 +391,12 @@ void mlx5e_ethtool_get_channels(struct mlx5e_priv *priv,
>>   {
>>   	ch->max_combined   = mlx5e_get_netdev_max_channels(priv->netdev);
>>   	ch->combined_count = priv->channels.params.num_channels;
>> +
>> +	/* XSK RQs */
>> +	ch->max_rx         = ch->max_combined;
>> +	/* rx_count shows the number of XSK RQs up to the highest active one. */
>> +	ch->rx_count       = mlx5e_xsk_first_unused_channel(&priv->channels.params,
>> +							    &priv->xsk);
>>   }
> 
> Ah, Maciej pointed out to me this is why you want the patch 7 to do
> what it does
You seem to be confusing cause and effect. The libbpf patch is good 
regardless of mlx5e's needs, because the current formula is incorrect, 
and I'm fixing it. Then I do the cited change in mlx5e, which perfectly 
fits the fixed formula. So, I'm not inserting some hack in libbpf just 
to make mlx5e work, I'm fixing an existing bug, and it allows me to do 
this stuff in mlx5e. It's not about "I need to use ethtool.rx in mlx5e, 
so I'm adapting libbpf to it", it's about "I see an issue in libbpf, so 
I'm fixing it, then I'm adapting mlx5e to fit the formula".

 > This count is for stack's queues.

Second, I disagree with this statement. XSK RX queues are not stack 
queues, but in i40e they are still registered as stack queues. Various 
boundary checks in the kernel use the "amount of stack queues" to check 
XSK QIDs. All the existing usage of this count in XSK code shows it's 
not for stack queues only, my usage is no different from that, so I 
don't see any issue in exposing XSK RX queues via ethtool.rx.

Anyway, I'm respinning without patch 7 and ethtool.rx.

> Nacked-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
>