netdev - Re: [PATCH net-next 09/10] gve: Alloc and free QPLs with the rings

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

Message-ID: <20240501083132.1ba34a02@kernel.org>
Date: Wed, 1 May 2024 08:31:32 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Shailend Chand <shailend@...gle.com>
Cc: netdev@...r.kernel.org, almasrymina@...gle.com, davem@...emloft.net,
 edumazet@...gle.com, hramamurthy@...gle.com, jeroendb@...gle.com,
 pabeni@...hat.com, pkaligineedi@...gle.com, willemb@...gle.com
Subject: Re: [PATCH net-next 09/10] gve: Alloc and free QPLs with the rings

On Tue, 30 Apr 2024 23:14:18 +0000 Shailend Chand wrote:
> Every tx and rx ring has its own queue-page-list (QPL) that serves as
> the bounce buffer. Previously we were allocating QPLs for all queues
> before the queues themselves were allocated and later associating a QPL
> with a queue. This is avoidable complexity: it is much more natural for
> each queue to allocate and free its own QPL.
> 
> Moreover, the advent of new queue-manipulating ndo hooks make it hard to
> keep things as is: we would need to transfer a QPL from an old queue to
> a new queue, and that is unpleasant.

Some compiler unhappiness here:

drivers/net/ethernet/google/gve/gve_rx.c:315:7: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
  315 |                 if (!rx->data.qpl)
      |                     ^~~~~~~~~~~~~
drivers/net/ethernet/google/gve/gve_rx.c:376:9: note: uninitialized use occurs here
  376 |         return err;
      |                ^~~
drivers/net/ethernet/google/gve/gve_rx.c:315:3: note: remove the 'if' if its condition is always false
  315 |                 if (!rx->data.qpl)
      |                 ^~~~~~~~~~~~~~~~~~
  316 |                         goto abort_with_copy_pool;
      |                         ~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/google/gve/gve_rx.c:278:9: note: initialize the variable 'err' to silence this warning
  278 |         int err;
      |                ^
      |                 = 0
1 warning generated.
drivers/net/ethernet/google/gve/gve_main.c:1432:6: warning: variable 'qpl_start_id' set but not used [-Wunused-but-set-variable]
 1432 |         int qpl_start_id;
      |             ^
drivers/net/ethernet/google/gve/gve_main.c:1454:6: warning: variable 'start_id' set but not used [-Wunused-but-set-variable]
 1454 |         int start_id;
      |             ^
-- 
pw-bot: cr