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

[Sagi Grimberg]

On 8/4/21 6:51 AM, Or Gerlitz wrote:
> On Fri, Jul 23, 2021 at 10:59 PM Sagi Grimberg <sagi at grimberg.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..