diff mbox series

[v4,net-next,07/21] nvme-tcp: Add DDP data-path

Message ID 20210211211044.32701-8-borisp@mellanox.com
State New
Headers show
Series nvme-tcp receive offloads | expand

Commit Message

Boris Pismenny Feb. 11, 2021, 9:10 p.m. UTC
Introduce the NVMe-TCP DDP data-path offload.
Using this interface, the NIC hardware will scatter TCP payload directly
to the BIO pages according to the command_id in the PDU.
To maintain the correctness of the network stack, the driver is expected
to construct SKBs that point to the BIO pages.

The data-path interface contains two routines: tcp_ddp_setup/teardown.
The setup provides the mapping from command_id to the request buffers,
while the teardown removes this mapping.

For efficiency, we introduce an asynchronous nvme completion, which is
split between NVMe-TCP and the NIC driver as follows:
NVMe-TCP performs the specific completion, while NIC driver performs the
generic mq_blk completion.

Signed-off-by: Boris Pismenny <borisp@mellanox.com>
Signed-off-by: Ben Ben-Ishay <benishay@mellanox.com>
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Yoray Zack <yorayz@mellanox.com>
---
 drivers/nvme/host/tcp.c | 158 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 146 insertions(+), 12 deletions(-)

Comments

David Ahern Feb. 14, 2021, 6:27 p.m. UTC | #1
On 2/11/21 2:10 PM, Boris Pismenny wrote:
>  

> +static int nvme_tcp_teardown_ddp(struct nvme_tcp_queue *queue,

> +				 u16 command_id,

> +				 struct request *rq)

> +{

> +	struct nvme_tcp_request *req = blk_mq_rq_to_pdu(rq);

> +	struct net_device *netdev = queue->ctrl->offloading_netdev;

> +	int ret;

> +

> +	if (unlikely(!netdev)) {

> +		dev_info_ratelimited(queue->ctrl->ctrl.device, "netdev not found\n");


again, unnecessary. you only get here because the rquest is marked
offloaded and that only happens if the netdev exists and supports DDP.

> +		return -EINVAL;

> +	}

> +

> +	ret = netdev->tcp_ddp_ops->tcp_ddp_teardown(netdev, queue->sock->sk,

> +						    &req->ddp, rq);

> +	sg_free_table_chained(&req->ddp.sg_table, SG_CHUNK_SIZE);

> +	return ret;

> +}

> +

> +static void nvme_tcp_ddp_teardown_done(void *ddp_ctx)

> +{

> +	struct request *rq = ddp_ctx;

> +	struct nvme_tcp_request *req = blk_mq_rq_to_pdu(rq);

> +

> +	if (!nvme_try_complete_req(rq, req->status, req->result))

> +		nvme_complete_rq(rq);

> +}

> +

> +static int nvme_tcp_setup_ddp(struct nvme_tcp_queue *queue,

> +			      u16 command_id,

> +			      struct request *rq)

> +{

> +	struct nvme_tcp_request *req = blk_mq_rq_to_pdu(rq);

> +	struct net_device *netdev = queue->ctrl->offloading_netdev;

> +	int ret;

> +

> +	if (unlikely(!netdev)) {

> +		dev_info_ratelimited(queue->ctrl->ctrl.device, "netdev not found\n");


similarly here. you can't get here if netdev is null.

> +		return -EINVAL;

> +	}

> +

> +	req->ddp.command_id = command_id;

> +	ret = nvme_tcp_req_map_sg(req, rq);

> +	if (ret)

> +		return -ENOMEM;

> +

> +	ret = netdev->tcp_ddp_ops->tcp_ddp_setup(netdev,

> +						 queue->sock->sk,

> +						 &req->ddp);

> +	if (!ret)

> +		req->offloaded = true;

> +	return ret;

> +}

> +

>  static int nvme_tcp_offload_socket(struct nvme_tcp_queue *queue)

>  {

>  	struct net_device *netdev = queue->ctrl->offloading_netdev;

> @@ -343,7 +417,7 @@ static void nvme_tcp_resync_response(struct nvme_tcp_queue *queue,

>  		return;

>  

>  	if (unlikely(!netdev)) {

> -		pr_info_ratelimited("%s: netdev not found\n", __func__);

> +		dev_info_ratelimited(queue->ctrl->ctrl.device, "netdev not found\n");


and per comment on the last patch, this is not needed.

> @@ -849,10 +953,39 @@ static int nvme_tcp_recv_pdu(struct nvme_tcp_queue *queue, struct sk_buff *skb,

>  

>  static inline void nvme_tcp_end_request(struct request *rq, u16 status)

>  {

> +	struct nvme_tcp_request *req = blk_mq_rq_to_pdu(rq);

> +	struct nvme_tcp_queue *queue = req->queue;

> +	struct nvme_tcp_data_pdu *pdu = (void *)queue->pdu;

>  	union nvme_result res = {};

>  

> -	if (!nvme_try_complete_req(rq, cpu_to_le16(status << 1), res))

> -		nvme_complete_rq(rq);

> +	nvme_tcp_complete_request(rq, cpu_to_le16(status << 1), res, pdu->command_id);

> +}

> +

> +

> +static int nvme_tcp_consume_skb(struct nvme_tcp_queue *queue, struct sk_buff *skb,

> +				unsigned int *offset, struct iov_iter *iter, int recv_len)

> +{

> +	int ret;

> +

> +#ifdef CONFIG_TCP_DDP

> +	if (test_bit(NVME_TCP_Q_OFF_DDP, &queue->flags)) {

> +		if (queue->data_digest)

> +			ret = skb_ddp_copy_and_hash_datagram_iter(skb, *offset, iter, recv_len,

> +					queue->rcv_hash);

> +		else

> +			ret = skb_ddp_copy_datagram_iter(skb, *offset, iter, recv_len);

> +	} else {

> +#endif


why not make that a helper defined in the CONFIG_TCP_DDP section with an
inline for the unset case. Keeps this code from being polluted with the
ifdef checks.

> +		if (queue->data_digest)

> +			ret = skb_copy_and_hash_datagram_iter(skb, *offset, iter, recv_len,

> +					queue->rcv_hash);

> +		else

> +			ret = skb_copy_datagram_iter(skb, *offset, iter, recv_len);

> +#ifdef CONFIG_TCP_DDP

> +	}

> +#endif

> +

> +	return ret;

>  }

>  

>  static int nvme_tcp_recv_data(struct nvme_tcp_queue *queue, struct sk_buff *skb,

> @@ -899,12 +1032,7 @@ static int nvme_tcp_recv_data(struct nvme_tcp_queue *queue, struct sk_buff *skb,

>  		recv_len = min_t(size_t, recv_len,

>  				iov_iter_count(&req->iter));

>  

> -		if (queue->data_digest)

> -			ret = skb_copy_and_hash_datagram_iter(skb, *offset,

> -				&req->iter, recv_len, queue->rcv_hash);

> -		else

> -			ret = skb_copy_datagram_iter(skb, *offset,

> -					&req->iter, recv_len);

> +		ret = nvme_tcp_consume_skb(queue, skb, offset, &req->iter, recv_len);

>  		if (ret) {

>  			dev_err(queue->ctrl->ctrl.device,

>  				"queue %d failed to copy request %#x data",

> @@ -1128,6 +1256,7 @@ static int nvme_tcp_try_send_cmd_pdu(struct nvme_tcp_request *req)

>  	bool inline_data = nvme_tcp_has_inline_data(req);

>  	u8 hdgst = nvme_tcp_hdgst_len(queue);

>  	int len = sizeof(*pdu) + hdgst - req->offset;

> +	struct request *rq = blk_mq_rq_from_pdu(req);

>  	int flags = MSG_DONTWAIT;

>  	int ret;

>  

> @@ -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?

>  	if (queue->hdr_digest && !req->offset)

>  		nvme_tcp_hdgst(queue->snd_hash, pdu, sizeof(*pdu));

>
Or Gerlitz Feb. 17, 2021, 2:01 p.m. UTC | #2
On Sun, Feb 14, 2021 at 8:30 PM David Ahern <dsahern@gmail.com> wrote:
>

> On 2/11/21 2:10 PM, Boris Pismenny wrote:

> >

> > +static int nvme_tcp_teardown_ddp(struct nvme_tcp_queue *queue,

> > +                              u16 command_id,

> > +                              struct request *rq)

> > +{

> > +     struct nvme_tcp_request *req = blk_mq_rq_to_pdu(rq);

> > +     struct net_device *netdev = queue->ctrl->offloading_netdev;

> > +     int ret;

> > +

> > +     if (unlikely(!netdev)) {

> > +             dev_info_ratelimited(queue->ctrl->ctrl.device, "netdev not found\n");

>

> again, unnecessary. you only get here because the rquest is marked

> offloaded and that only happens if the netdev exists and supports DDP.

>

> > +             return -EINVAL;

> > +     }

> > +

> > +     ret = netdev->tcp_ddp_ops->tcp_ddp_teardown(netdev, queue->sock->sk,

> > +                                                 &req->ddp, rq);

> > +     sg_free_table_chained(&req->ddp.sg_table, SG_CHUNK_SIZE);

> > +     return ret;

> > +}

> > +

> > +static void nvme_tcp_ddp_teardown_done(void *ddp_ctx)

> > +{

> > +     struct request *rq = ddp_ctx;

> > +     struct nvme_tcp_request *req = blk_mq_rq_to_pdu(rq);

> > +

> > +     if (!nvme_try_complete_req(rq, req->status, req->result))

> > +             nvme_complete_rq(rq);

> > +}

> > +

> > +static int nvme_tcp_setup_ddp(struct nvme_tcp_queue *queue,

> > +                           u16 command_id,

> > +                           struct request *rq)

> > +{

> > +     struct nvme_tcp_request *req = blk_mq_rq_to_pdu(rq);

> > +     struct net_device *netdev = queue->ctrl->offloading_netdev;

> > +     int ret;

> > +

> > +     if (unlikely(!netdev)) {

> > +             dev_info_ratelimited(queue->ctrl->ctrl.device, "netdev not found\n");

>

> similarly here. you can't get here if netdev is null.

>

> > +             return -EINVAL;

> > +     }

> > +

> > +     req->ddp.command_id = command_id;

> > +     ret = nvme_tcp_req_map_sg(req, rq);

> > +     if (ret)

> > +             return -ENOMEM;

> > +

> > +     ret = netdev->tcp_ddp_ops->tcp_ddp_setup(netdev,

> > +                                              queue->sock->sk,

> > +                                              &req->ddp);

> > +     if (!ret)

> > +             req->offloaded = true;

> > +     return ret;

> > +}

> > +

> >  static int nvme_tcp_offload_socket(struct nvme_tcp_queue *queue)

> >  {

> >       struct net_device *netdev = queue->ctrl->offloading_netdev;

> > @@ -343,7 +417,7 @@ static void nvme_tcp_resync_response(struct nvme_tcp_queue *queue,

> >               return;

> >

> >       if (unlikely(!netdev)) {

> > -             pr_info_ratelimited("%s: netdev not found\n", __func__);

> > +             dev_info_ratelimited(queue->ctrl->ctrl.device, "netdev not found\n");

>

> and per comment on the last patch, this is not needed.

>

> > @@ -849,10 +953,39 @@ static int nvme_tcp_recv_pdu(struct nvme_tcp_queue *queue, struct sk_buff *skb,

> >

> >  static inline void nvme_tcp_end_request(struct request *rq, u16 status)

> >  {

> > +     struct nvme_tcp_request *req = blk_mq_rq_to_pdu(rq);

> > +     struct nvme_tcp_queue *queue = req->queue;

> > +     struct nvme_tcp_data_pdu *pdu = (void *)queue->pdu;

> >       union nvme_result res = {};

> >

> > -     if (!nvme_try_complete_req(rq, cpu_to_le16(status << 1), res))

> > -             nvme_complete_rq(rq);

> > +     nvme_tcp_complete_request(rq, cpu_to_le16(status << 1), res, pdu->command_id);

> > +}

> > +

> > +

> > +static int nvme_tcp_consume_skb(struct nvme_tcp_queue *queue, struct sk_buff *skb,

> > +                             unsigned int *offset, struct iov_iter *iter, int recv_len)

> > +{

> > +     int ret;

> > +

> > +#ifdef CONFIG_TCP_DDP

> > +     if (test_bit(NVME_TCP_Q_OFF_DDP, &queue->flags)) {

> > +             if (queue->data_digest)

> > +                     ret = skb_ddp_copy_and_hash_datagram_iter(skb, *offset, iter, recv_len,

> > +                                     queue->rcv_hash);

> > +             else

> > +                     ret = skb_ddp_copy_datagram_iter(skb, *offset, iter, recv_len);

> > +     } else {

> > +#endif

>

> why not make that a helper defined in the CONFIG_TCP_DDP section with an

> inline for the unset case. Keeps this code from being polluted with the

> ifdef checks.


will check that

>

> > +             if (queue->data_digest)

> > +                     ret = skb_copy_and_hash_datagram_iter(skb, *offset, iter, recv_len,

> > +                                     queue->rcv_hash);

> > +             else

> > +                     ret = skb_copy_datagram_iter(skb, *offset, iter, recv_len);

> > +#ifdef CONFIG_TCP_DDP

> > +     }

> > +#endif

> > +

> > +     return ret;

> >  }

> >

> >  static int nvme_tcp_recv_data(struct nvme_tcp_queue *queue, struct sk_buff *skb,

> > @@ -899,12 +1032,7 @@ static int nvme_tcp_recv_data(struct nvme_tcp_queue *queue, struct sk_buff *skb,

> >               recv_len = min_t(size_t, recv_len,

> >                               iov_iter_count(&req->iter));

> >

> > -             if (queue->data_digest)

> > -                     ret = skb_copy_and_hash_datagram_iter(skb, *offset,

> > -                             &req->iter, recv_len, queue->rcv_hash);

> > -             else

> > -                     ret = skb_copy_datagram_iter(skb, *offset,

> > -                                     &req->iter, recv_len);

> > +             ret = nvme_tcp_consume_skb(queue, skb, offset, &req->iter, recv_len);

> >               if (ret) {

> >                       dev_err(queue->ctrl->ctrl.device,

> >                               "queue %d failed to copy request %#x data",

> > @@ -1128,6 +1256,7 @@ static int nvme_tcp_try_send_cmd_pdu(struct nvme_tcp_request *req)

> >       bool inline_data = nvme_tcp_has_inline_data(req);

> >       u8 hdgst = nvme_tcp_hdgst_len(queue);

> >       int len = sizeof(*pdu) + hdgst - req->offset;

> > +     struct request *rq = blk_mq_rq_from_pdu(req);

> >       int flags = MSG_DONTWAIT;

> >       int ret;

> >

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


> >       if (queue->hdr_digest && !req->offset)

> >               nvme_tcp_hdgst(queue->snd_hash, pdu, sizeof(*pdu));

> >
David Ahern Feb. 17, 2021, 5 p.m. UTC | #3
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.
Boris Pismenny Feb. 21, 2021, 11:44 a.m. UTC | #4
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.
diff mbox series

Patch

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 36de4391ba76..188e26ab7116 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -57,6 +57,11 @@  struct nvme_tcp_request {
 	size_t			offset;
 	size_t			data_sent;
 	enum nvme_tcp_send_state state;
+
+	bool			offloaded;
+	struct tcp_ddp_io	ddp;
+	__le16			status;
+	union nvme_result	result;
 };
 
 enum nvme_tcp_queue_flags {
@@ -229,13 +234,82 @@  static inline size_t nvme_tcp_pdu_last_send(struct nvme_tcp_request *req,
 	return nvme_tcp_pdu_data_left(req) <= len;
 }
 
+static int nvme_tcp_req_map_sg(struct nvme_tcp_request *req, struct request *rq)
+{
+	int ret;
+
+	req->ddp.sg_table.sgl = req->ddp.first_sgl;
+	ret = sg_alloc_table_chained(&req->ddp.sg_table, blk_rq_nr_phys_segments(rq),
+				     req->ddp.sg_table.sgl, SG_CHUNK_SIZE);
+	if (ret)
+		return -ENOMEM;
+	req->ddp.nents = blk_rq_map_sg(rq->q, rq, req->ddp.sg_table.sgl);
+	return 0;
+}
+
 #ifdef CONFIG_TCP_DDP
 
 static bool nvme_tcp_resync_request(struct sock *sk, u32 seq, u32 flags);
+static void nvme_tcp_ddp_teardown_done(void *ddp_ctx);
 static const struct tcp_ddp_ulp_ops nvme_tcp_ddp_ulp_ops = {
 	.resync_request		= nvme_tcp_resync_request,
+	.ddp_teardown_done	= nvme_tcp_ddp_teardown_done,
 };
 
+static int nvme_tcp_teardown_ddp(struct nvme_tcp_queue *queue,
+				 u16 command_id,
+				 struct request *rq)
+{
+	struct nvme_tcp_request *req = blk_mq_rq_to_pdu(rq);
+	struct net_device *netdev = queue->ctrl->offloading_netdev;
+	int ret;
+
+	if (unlikely(!netdev)) {
+		dev_info_ratelimited(queue->ctrl->ctrl.device, "netdev not found\n");
+		return -EINVAL;
+	}
+
+	ret = netdev->tcp_ddp_ops->tcp_ddp_teardown(netdev, queue->sock->sk,
+						    &req->ddp, rq);
+	sg_free_table_chained(&req->ddp.sg_table, SG_CHUNK_SIZE);
+	return ret;
+}
+
+static void nvme_tcp_ddp_teardown_done(void *ddp_ctx)
+{
+	struct request *rq = ddp_ctx;
+	struct nvme_tcp_request *req = blk_mq_rq_to_pdu(rq);
+
+	if (!nvme_try_complete_req(rq, req->status, req->result))
+		nvme_complete_rq(rq);
+}
+
+static int nvme_tcp_setup_ddp(struct nvme_tcp_queue *queue,
+			      u16 command_id,
+			      struct request *rq)
+{
+	struct nvme_tcp_request *req = blk_mq_rq_to_pdu(rq);
+	struct net_device *netdev = queue->ctrl->offloading_netdev;
+	int ret;
+
+	if (unlikely(!netdev)) {
+		dev_info_ratelimited(queue->ctrl->ctrl.device, "netdev not found\n");
+		return -EINVAL;
+	}
+
+	req->ddp.command_id = command_id;
+	ret = nvme_tcp_req_map_sg(req, rq);
+	if (ret)
+		return -ENOMEM;
+
+	ret = netdev->tcp_ddp_ops->tcp_ddp_setup(netdev,
+						 queue->sock->sk,
+						 &req->ddp);
+	if (!ret)
+		req->offloaded = true;
+	return ret;
+}
+
 static int nvme_tcp_offload_socket(struct nvme_tcp_queue *queue)
 {
 	struct net_device *netdev = queue->ctrl->offloading_netdev;
@@ -343,7 +417,7 @@  static void nvme_tcp_resync_response(struct nvme_tcp_queue *queue,
 		return;
 
 	if (unlikely(!netdev)) {
-		pr_info_ratelimited("%s: netdev not found\n", __func__);
+		dev_info_ratelimited(queue->ctrl->ctrl.device, "netdev not found\n");
 		return;
 	}
 
@@ -368,6 +442,20 @@  static bool nvme_tcp_resync_request(struct sock *sk, u32 seq, u32 flags)
 
 #else
 
+static int nvme_tcp_setup_ddp(struct nvme_tcp_queue *queue,
+			      u16 command_id,
+			      struct request *rq)
+{
+	return -EINVAL;
+}
+
+static int nvme_tcp_teardown_ddp(struct nvme_tcp_queue *queue,
+				 u16 command_id,
+				 struct request *rq)
+{
+	return -EINVAL;
+}
+
 static int nvme_tcp_offload_socket(struct nvme_tcp_queue *queue)
 {
 	return -EINVAL;
@@ -643,6 +731,24 @@  static void nvme_tcp_error_recovery(struct nvme_ctrl *ctrl)
 	queue_work(nvme_reset_wq, &to_tcp_ctrl(ctrl)->err_work);
 }
 
+static void nvme_tcp_complete_request(struct request *rq,
+				      __le16 status,
+				      union nvme_result result,
+				      __u16 command_id)
+{
+	struct nvme_tcp_request *req = blk_mq_rq_to_pdu(rq);
+	struct nvme_tcp_queue *queue = req->queue;
+
+	if (req->offloaded) {
+		req->status = status;
+		req->result = result;
+		nvme_tcp_teardown_ddp(queue, command_id, rq);
+	} else {
+		if (!nvme_try_complete_req(rq, status, result))
+			nvme_complete_rq(rq);
+	}
+}
+
 static int nvme_tcp_process_nvme_cqe(struct nvme_tcp_queue *queue,
 		struct nvme_completion *cqe)
 {
@@ -657,10 +763,8 @@  static int nvme_tcp_process_nvme_cqe(struct nvme_tcp_queue *queue,
 		return -EINVAL;
 	}
 
-	if (!nvme_try_complete_req(rq, cqe->status, cqe->result))
-		nvme_complete_rq(rq);
+	nvme_tcp_complete_request(rq, cqe->status, cqe->result, cqe->command_id);
 	queue->nr_cqe++;
-
 	return 0;
 }
 
@@ -849,10 +953,39 @@  static int nvme_tcp_recv_pdu(struct nvme_tcp_queue *queue, struct sk_buff *skb,
 
 static inline void nvme_tcp_end_request(struct request *rq, u16 status)
 {
+	struct nvme_tcp_request *req = blk_mq_rq_to_pdu(rq);
+	struct nvme_tcp_queue *queue = req->queue;
+	struct nvme_tcp_data_pdu *pdu = (void *)queue->pdu;
 	union nvme_result res = {};
 
-	if (!nvme_try_complete_req(rq, cpu_to_le16(status << 1), res))
-		nvme_complete_rq(rq);
+	nvme_tcp_complete_request(rq, cpu_to_le16(status << 1), res, pdu->command_id);
+}
+
+
+static int nvme_tcp_consume_skb(struct nvme_tcp_queue *queue, struct sk_buff *skb,
+				unsigned int *offset, struct iov_iter *iter, int recv_len)
+{
+	int ret;
+
+#ifdef CONFIG_TCP_DDP
+	if (test_bit(NVME_TCP_Q_OFF_DDP, &queue->flags)) {
+		if (queue->data_digest)
+			ret = skb_ddp_copy_and_hash_datagram_iter(skb, *offset, iter, recv_len,
+					queue->rcv_hash);
+		else
+			ret = skb_ddp_copy_datagram_iter(skb, *offset, iter, recv_len);
+	} else {
+#endif
+		if (queue->data_digest)
+			ret = skb_copy_and_hash_datagram_iter(skb, *offset, iter, recv_len,
+					queue->rcv_hash);
+		else
+			ret = skb_copy_datagram_iter(skb, *offset, iter, recv_len);
+#ifdef CONFIG_TCP_DDP
+	}
+#endif
+
+	return ret;
 }
 
 static int nvme_tcp_recv_data(struct nvme_tcp_queue *queue, struct sk_buff *skb,
@@ -899,12 +1032,7 @@  static int nvme_tcp_recv_data(struct nvme_tcp_queue *queue, struct sk_buff *skb,
 		recv_len = min_t(size_t, recv_len,
 				iov_iter_count(&req->iter));
 
-		if (queue->data_digest)
-			ret = skb_copy_and_hash_datagram_iter(skb, *offset,
-				&req->iter, recv_len, queue->rcv_hash);
-		else
-			ret = skb_copy_datagram_iter(skb, *offset,
-					&req->iter, recv_len);
+		ret = nvme_tcp_consume_skb(queue, skb, offset, &req->iter, recv_len);
 		if (ret) {
 			dev_err(queue->ctrl->ctrl.device,
 				"queue %d failed to copy request %#x data",
@@ -1128,6 +1256,7 @@  static int nvme_tcp_try_send_cmd_pdu(struct nvme_tcp_request *req)
 	bool inline_data = nvme_tcp_has_inline_data(req);
 	u8 hdgst = nvme_tcp_hdgst_len(queue);
 	int len = sizeof(*pdu) + hdgst - req->offset;
+	struct request *rq = blk_mq_rq_from_pdu(req);
 	int flags = MSG_DONTWAIT;
 	int ret;
 
@@ -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);
+
 	if (queue->hdr_digest && !req->offset)
 		nvme_tcp_hdgst(queue->snd_hash, pdu, sizeof(*pdu));
 
@@ -2441,6 +2574,7 @@  static blk_status_t nvme_tcp_setup_cmd_pdu(struct nvme_ns *ns,
 	req->data_len = blk_rq_nr_phys_segments(rq) ?
 				blk_rq_payload_bytes(rq) : 0;
 	req->curr_bio = rq->bio;
+	req->offloaded = false;
 
 	if (rq_data_dir(rq) == WRITE &&
 	    req->data_len <= nvme_tcp_inline_data_size(queue))