On 8/4/21 6:51 AM, Or Gerlitz wrote:

On Fri, Jul 23, 2021 at 10:59 PM Sagi Grimberg [off-list ref] wrote:

quoted

[.. ] 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.

quoted

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

quoted

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..

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme