On 17/02/2021 19:00, David Ahern wrote: > On 2/17/21 7:01 AM, Or Gerlitz wrote: >>>> @@ -1136,6 +1265,10 @@ static int nvme_tcp_try_send_cmd_pdu(struct >>>> nvme_tcp_request *req) >>>> else >>>> flags |= MSG_EOR; >>>> >>>> + if (test_bit(NVME_TCP_Q_OFF_DDP, &queue->flags) && >>>> + blk_rq_nr_phys_segments(rq) && rq_data_dir(rq) == READ) >>>> + nvme_tcp_setup_ddp(queue, pdu->cmd.common.command_id, rq); >>>> + >>> >>> For consistency, shouldn't this be wrapped in the CONFIG_TCP_DDP check too? >> >> We tried to avoid the wrapping in some places where it was >> possible to do without adding confusion, this one is a good >> example IMOH. >> >> > > The above (and other locations like it) can easily be put into a helper > that has logic when the CONFIG is enabled and compiles out when not. > Consistency makes for simpler, cleaner code for optional features. >
The above is consistent in the sense that we wrap only places that are absolutely necessary, so as to avoid ifdefs as much as possible. Specifically, here and in ddp_teardown, we will add a wrapper to reduce clutter if offload is not used.
