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]

Date:   Fri, 6 Aug 2021 12:46:37 -0700
From:   Sagi Grimberg <sagi@...mberg.me>
To:     Or Gerlitz <gerlitz.or@...il.com>
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 8/4/21 6:51 AM, Or Gerlitz wrote:
> 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

I was more referring to routines that now grew the ddp path
and changed in the same time like:
nvme_tcp_complete_request
nvme_tcp_consume_skb
etc..