netdev - Re: [PATCH v5 net-next 00/36] nvme-tcp receive and tarnsmit offloads

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: <CAJ3xEMjzRqrj-EN7gbqKmD5txAV-gZn828V+6QAf5wfwYsqySQ@mail.gmail.com>
Date:   Wed, 4 Aug 2021 16:51:27 +0300
From:   Or Gerlitz <gerlitz.or@...il.com>
To:     Sagi Grimberg <sagi@...mberg.me>
Cc:     Christoph Hellwig <hch@....de>, Boris Pismenny <borisp@...dia.com>,
        David Ahern <dsahern@...il.com>,
        Jakub Kicinski <kuba@...nel.org>,
        David Miller <davem@...emloft.net>,
        Saeed Mahameed <saeedm@...dia.com>, axboe@...com,
        Keith Busch <kbusch@...nel.org>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Eric Dumazet <edumazet@...gle.com>,
        Shai Malin <smalin@...vell.com>, boris.pismenny@...il.com,
        linux-nvme@...ts.infradead.org,
        Linux Netdev List <netdev@...r.kernel.org>,
        benishay@...dia.com, Or Gerlitz <ogerlitz@...dia.com>,
        Yoray Zack <yorayz@...dia.com>,
        Boris Pismenny <borisp@...lanox.com>
Subject: Re: [PATCH v5 net-next 00/36] nvme-tcp receive and tarnsmit offloads

On Fri, Jul 23, 2021 at 10:59 PM Sagi Grimberg <sagi@...mberg.me> wrote:

> [.. ] It is difficult to review.
> The order should be:
> 1. ulp_ddp interface
> 2. nvme-tcp changes
> 3. mlx5e changes

.. and this is exactly how the series is organized, for v6 we will drop the
TX offload part and stick to completing the review on the RX offload part.

> Also even beyond grouping patches together I have 2 requests:
> 1. Please consolidate ddp routines under a single ifdef (also minimize
> the ifdef in call-sites).

ok, will make an effort to be better in that respect

> 2. When consolidating functions, try to do this as prep patches
> documenting in the change log that it is preparing to add ddp. Its
> difficult digesting both at times.

to clarify, you would like patch #5 "nvme-tcp: Add DDP offload control path"
to only add the call sites and if-not-deffed implementation for the added knobs:

nvme_tcp_offload_socket
nvme_tcp_unoffload_socket
nvme_tcp_offload_limits
nvme_tcp_resync_response

and a 2nd patch to add the if-yes-deffed implementation?

This makes sense, however IMHO repeating this prep exercise for
the data-path patch (#6 "nvme-tcp: Add DDP data-path") doesn't
seem to provide notable value  b/c you will only see two call sites
for the two added empty knobs:

nvme_tcp_setup_ddp
nvme_tcp_teardown_ddp

but whatever you prefer, so.. let us know