DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] net/gve : Update EOP bit in txd rte_mbuf chain
@ 2024-07-31 16:38 Tathagat Priyadarshi
  2024-07-31 20:30 ` Joshua Washington
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Tathagat Priyadarshi @ 2024-07-31 16:38 UTC (permalink / raw)
  To: rushilg, joshwash; +Cc: dev, Tathagat Priyadarshi, Varun Lakkur Ambaji Rao

The EOP bit was not set for all the packets in mbuf chain
causing packet transmission stalls for packets split across
mbuf in chain.

Signed-off-by: Tathagat Priyadarshi <tathagat.dpdk@gmail.com>
Signed-off-by: Varun Lakkur Ambaji Rao <varun.la@gmail.com>

Fixes: 4022f99 ("net/gve: support basic Tx data path for DQO")
---
 drivers/net/gve/gve_tx_dqo.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/gve/gve_tx_dqo.c b/drivers/net/gve/gve_tx_dqo.c
index a65e6aa..579b8d6 100644
--- a/drivers/net/gve/gve_tx_dqo.c
+++ b/drivers/net/gve/gve_tx_dqo.c
@@ -126,6 +126,7 @@
 			txd->pkt.dtype = GVE_TX_PKT_DESC_DTYPE_DQO;
 			txd->pkt.compl_tag = rte_cpu_to_le_16(first_sw_id);
 			txd->pkt.buf_size = RTE_MIN(tx_pkt->data_len, GVE_TX_MAX_BUF_SIZE_DQO);
+			txd->pkt.end_of_packet = 0;
 
 			/* size of desc_ring and sw_ring could be different */
 			tx_id = (tx_id + 1) & mask;
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] net/gve : Update EOP bit in txd rte_mbuf chain
  2024-07-31 16:38 [PATCH] net/gve : Update EOP bit in txd rte_mbuf chain Tathagat Priyadarshi
@ 2024-07-31 20:30 ` Joshua Washington
  2024-08-01 10:23   ` Tathagat Priyadarshi
  2024-08-01 11:16 ` Ferruh Yigit
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Joshua Washington @ 2024-07-31 20:30 UTC (permalink / raw)
  To: Tathagat Priyadarshi; +Cc: Rushil Gupta, dev, Varun Lakkur Ambaji Rao

On Wed, Jul 31, 2024, 09:37 Tathagat Priyadarshi
<tathagat.dpdk@gmail.com> wrote:
>
> The EOP bit was not set for all the packets in mbuf chain
> causing packet transmission stalls for packets split across
> mbuf in chain.
>
> Signed-off-by: Tathagat Priyadarshi <tathagat.dpdk@gmail.com>
> Signed-off-by: Varun Lakkur Ambaji Rao <varun.la@gmail.com>
>
> Fixes: 4022f99 ("net/gve: support basic Tx data path for DQO")
> ---
>  drivers/net/gve/gve_tx_dqo.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/gve/gve_tx_dqo.c b/drivers/net/gve/gve_tx_dqo.c
> index a65e6aa..579b8d6 100644
> --- a/drivers/net/gve/gve_tx_dqo.c
> +++ b/drivers/net/gve/gve_tx_dqo.c
> @@ -126,6 +126,7 @@
>                         txd->pkt.dtype = GVE_TX_PKT_DESC_DTYPE_DQO;
>                         txd->pkt.compl_tag = rte_cpu_to_le_16(first_sw_id);
>                         txd->pkt.buf_size = RTE_MIN(tx_pkt->data_len, GVE_TX_MAX_BUF_SIZE_DQO);
> +                       txd->pkt.end_of_packet = 0;

Please also update checksum offload for each mbuf.
>
>
>                         /* size of desc_ring and sw_ring could be different */
>                         tx_id = (tx_id + 1) & mask;
> --
> 1.8.3.1
>

Thanks for all of the contributions! Let's try to get this applied to
stable release as well.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] net/gve : Update EOP bit in txd rte_mbuf chain
  2024-07-31 20:30 ` Joshua Washington
@ 2024-08-01 10:23   ` Tathagat Priyadarshi
  0 siblings, 0 replies; 16+ messages in thread
From: Tathagat Priyadarshi @ 2024-08-01 10:23 UTC (permalink / raw)
  To: Joshua Washington; +Cc: Rushil Gupta, dev, Varun Lakkur Ambaji Rao

Hi Joshua,

We have addressed the checksum offload update for each mbuf in the
following patch (net/gve: Add support for TSO in DQO RDA).
https://patches.dpdk.org/project/dpdk/patch/1722507548-2401507-1-git-send-email-tathagat.dpdk@gmail.com/

Thanks a lot!


On Thu, Aug 1, 2024 at 2:00 AM Joshua Washington <joshwash@google.com> wrote:
>
> On Wed, Jul 31, 2024, 09:37 Tathagat Priyadarshi
> <tathagat.dpdk@gmail.com> wrote:
> >
> > The EOP bit was not set for all the packets in mbuf chain
> > causing packet transmission stalls for packets split across
> > mbuf in chain.
> >
> > Signed-off-by: Tathagat Priyadarshi <tathagat.dpdk@gmail.com>
> > Signed-off-by: Varun Lakkur Ambaji Rao <varun.la@gmail.com>
> >
> > Fixes: 4022f99 ("net/gve: support basic Tx data path for DQO")
> > ---
> >  drivers/net/gve/gve_tx_dqo.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/net/gve/gve_tx_dqo.c b/drivers/net/gve/gve_tx_dqo.c
> > index a65e6aa..579b8d6 100644
> > --- a/drivers/net/gve/gve_tx_dqo.c
> > +++ b/drivers/net/gve/gve_tx_dqo.c
> > @@ -126,6 +126,7 @@
> >                         txd->pkt.dtype = GVE_TX_PKT_DESC_DTYPE_DQO;
> >                         txd->pkt.compl_tag = rte_cpu_to_le_16(first_sw_id);
> >                         txd->pkt.buf_size = RTE_MIN(tx_pkt->data_len, GVE_TX_MAX_BUF_SIZE_DQO);
> > +                       txd->pkt.end_of_packet = 0;
>
> Please also update checksum offload for each mbuf.
> >
> >
> >                         /* size of desc_ring and sw_ring could be different */
> >                         tx_id = (tx_id + 1) & mask;
> > --
> > 1.8.3.1
> >
>
> Thanks for all of the contributions! Let's try to get this applied to
> stable release as well.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] net/gve : Update EOP bit in txd rte_mbuf chain
  2024-07-31 16:38 [PATCH] net/gve : Update EOP bit in txd rte_mbuf chain Tathagat Priyadarshi
  2024-07-31 20:30 ` Joshua Washington
@ 2024-08-01 11:16 ` Ferruh Yigit
  2024-08-01 16:24   ` Joshua Washington
  2024-08-01 11:31 ` [PATCH v2] " Tathagat Priyadarshi
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Ferruh Yigit @ 2024-08-01 11:16 UTC (permalink / raw)
  To: Tathagat Priyadarshi, rushilg, joshwash; +Cc: dev, Varun Lakkur Ambaji Rao

On 7/31/2024 5:38 PM, Tathagat Priyadarshi wrote:
> The EOP bit was not set for all the packets in mbuf chain
> causing packet transmission stalls for packets split across
> mbuf in chain.
> 
> Signed-off-by: Tathagat Priyadarshi <tathagat.dpdk@gmail.com>
> Signed-off-by: Varun Lakkur Ambaji Rao <varun.la@gmail.com>
> 
> Fixes: 4022f99 ("net/gve: support basic Tx data path for DQO")
>

Hi Tathagat,

Can you please address issues reported from
'./devtools/check-git-log.sh' script?



And there is a request to backport to stable trees, please include
following tag:
Cc: stable@dpdk.org

And please use following order/syntax in commit log:
```
<commit log>

Fixes: 4022f99 ("net/gve: support basic Tx data path for DQO")
Cc: stable@dpdk.org

Signed-off-by: Tathagat Priyadarshi <tathagat.dpdk@gmail.com>
Signed-off-by: Varun Lakkur Ambaji Rao <varun.la@gmail.com>
```

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2] net/gve : Update EOP bit in txd rte_mbuf chain
  2024-07-31 16:38 [PATCH] net/gve : Update EOP bit in txd rte_mbuf chain Tathagat Priyadarshi
  2024-07-31 20:30 ` Joshua Washington
  2024-08-01 11:16 ` Ferruh Yigit
@ 2024-08-01 11:31 ` Tathagat Priyadarshi
  2024-08-01 17:27 ` [PATCH v3] net/gve : Update EOP & csum " Tathagat Priyadarshi
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Tathagat Priyadarshi @ 2024-08-01 11:31 UTC (permalink / raw)
  To: dev; +Cc: Tathagat Priyadarshi, stable, Varun Lakkur Ambaji Rao

The EOP bit was not set for all the packets in mbuf chain
causing packet transmission stalls for packets split across
mbuf in chain.

Fixes: 4022f99 ("net/gve: support basic Tx data path for DQO")
Cc: stable@dpdk.org

Signed-off-by: Tathagat Priyadarshi <tathagat.dpdk@gmail.com>
Signed-off-by: Varun Lakkur Ambaji Rao <varun.la@gmail.com>
---
 drivers/net/gve/gve_tx_dqo.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/gve/gve_tx_dqo.c b/drivers/net/gve/gve_tx_dqo.c
index a65e6aa..579b8d6 100644
--- a/drivers/net/gve/gve_tx_dqo.c
+++ b/drivers/net/gve/gve_tx_dqo.c
@@ -126,6 +126,7 @@
 			txd->pkt.dtype = GVE_TX_PKT_DESC_DTYPE_DQO;
 			txd->pkt.compl_tag = rte_cpu_to_le_16(first_sw_id);
 			txd->pkt.buf_size = RTE_MIN(tx_pkt->data_len, GVE_TX_MAX_BUF_SIZE_DQO);
+			txd->pkt.end_of_packet = 0;
 
 			/* size of desc_ring and sw_ring could be different */
 			tx_id = (tx_id + 1) & mask;
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] net/gve : Update EOP bit in txd rte_mbuf chain
  2024-08-01 11:16 ` Ferruh Yigit
@ 2024-08-01 16:24   ` Joshua Washington
  2024-08-01 17:50     ` Tathagat Priyadarshi
  0 siblings, 1 reply; 16+ messages in thread
From: Joshua Washington @ 2024-08-01 16:24 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Tathagat Priyadarshi, Rushil Gupta, dev, Varun Lakkur Ambaji Rao

[-- Attachment #1: Type: text/plain, Size: 275 bytes --]

Hi, can we  try to serialize these patches? Let's fix the multidescriptor
packets issue present in the driver first (ensuring that the entire
descriptor is written each time). The TSO support is an entirely new
feature and should likely not be backported to stable releases.

[-- Attachment #2: Type: text/html, Size: 303 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v3] net/gve : Update EOP & csum bit in txd rte_mbuf chain
  2024-07-31 16:38 [PATCH] net/gve : Update EOP bit in txd rte_mbuf chain Tathagat Priyadarshi
                   ` (2 preceding siblings ...)
  2024-08-01 11:31 ` [PATCH v2] " Tathagat Priyadarshi
@ 2024-08-01 17:27 ` Tathagat Priyadarshi
  2024-08-01 18:54   ` Stephen Hemminger
  2024-08-01 17:48 ` [PATCH v4] " Tathagat Priyadarshi
  2024-08-02  5:08 ` [PATCH v5] " Tathagat Priyadarshi
  5 siblings, 1 reply; 16+ messages in thread
From: Tathagat Priyadarshi @ 2024-08-01 17:27 UTC (permalink / raw)
  To: dev; +Cc: Tathagat Priyadarshi, stable, Varun Lakkur Ambaji Rao

The EOP and csum bit was not set for all the packets in mbuf chain
causing packet transmission stalls for packets split across
mbuf in chain.

Fixes: 4022f99 ("net/gve: support basic Tx data path for DQO")
Cc: stable@dpdk.org

Signed-off-by: Tathagat Priyadarshi <tathagat.dpdk@gmail.com>
Signed-off-by: Varun Lakkur Ambaji Rao <varun.la@gmail.com>
---
 drivers/net/gve/gve_tx_dqo.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/gve/gve_tx_dqo.c b/drivers/net/gve/gve_tx_dqo.c
index a65e6aa..91db037 100644
--- a/drivers/net/gve/gve_tx_dqo.c
+++ b/drivers/net/gve/gve_tx_dqo.c
@@ -89,6 +89,7 @@
 	uint16_t sw_id;
 	uint64_t bytes;
 	uint16_t first_sw_id;
+	uint8_t csum;
 
 	sw_ring = txq->sw_ring;
 	txr = txq->tx_ring;
@@ -114,6 +115,12 @@
 		ol_flags = tx_pkt->ol_flags;
 		nb_used = tx_pkt->nb_segs;
 		first_sw_id = sw_id;
+
+		if (ol_flags & GVE_TX_CKSUM_OFFLOAD_MASK_DQO)
+			csum = 1;
+		else
+			cusm = 0;
+
 		do {
 			if (sw_ring[sw_id] != NULL)
 				PMD_DRV_LOG(DEBUG, "Overwriting an entry in sw_ring");
@@ -126,6 +133,8 @@
 			txd->pkt.dtype = GVE_TX_PKT_DESC_DTYPE_DQO;
 			txd->pkt.compl_tag = rte_cpu_to_le_16(first_sw_id);
 			txd->pkt.buf_size = RTE_MIN(tx_pkt->data_len, GVE_TX_MAX_BUF_SIZE_DQO);
+			txd->pkt.end_of_packet = 0;
+			txd->pkt.checksum_offload_enable = csum;
 
 			/* size of desc_ring and sw_ring could be different */
 			tx_id = (tx_id + 1) & mask;
@@ -138,9 +147,6 @@
 		/* fill the last descriptor with End of Packet (EOP) bit */
 		txd->pkt.end_of_packet = 1;
 
-		if (ol_flags & GVE_TX_CKSUM_OFFLOAD_MASK_DQO)
-			txd->pkt.checksum_offload_enable = 1;
-
 		txq->nb_free -= nb_used;
 		txq->nb_used += nb_used;
 	}
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v4] net/gve : Update EOP & csum bit in txd rte_mbuf chain
  2024-07-31 16:38 [PATCH] net/gve : Update EOP bit in txd rte_mbuf chain Tathagat Priyadarshi
                   ` (3 preceding siblings ...)
  2024-08-01 17:27 ` [PATCH v3] net/gve : Update EOP & csum " Tathagat Priyadarshi
@ 2024-08-01 17:48 ` Tathagat Priyadarshi
  2024-08-02  5:08 ` [PATCH v5] " Tathagat Priyadarshi
  5 siblings, 0 replies; 16+ messages in thread
From: Tathagat Priyadarshi @ 2024-08-01 17:48 UTC (permalink / raw)
  To: dev; +Cc: Tathagat Priyadarshi, stable, Varun Lakkur Ambaji Rao

The EOP and csum bit was not set for all the packets in mbuf chain
causing packet transmission stalls for packets split across
mbuf in chain.

Fixes: 4022f99 ("net/gve: support basic Tx data path for DQO")
Cc: stable@dpdk.org

Signed-off-by: Tathagat Priyadarshi <tathagat.dpdk@gmail.com>
Signed-off-by: Varun Lakkur Ambaji Rao <varun.la@gmail.com>
---
 drivers/net/gve/gve_tx_dqo.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/gve/gve_tx_dqo.c b/drivers/net/gve/gve_tx_dqo.c
index a65e6aa..1374e1a 100644
--- a/drivers/net/gve/gve_tx_dqo.c
+++ b/drivers/net/gve/gve_tx_dqo.c
@@ -89,6 +89,7 @@
 	uint16_t sw_id;
 	uint64_t bytes;
 	uint16_t first_sw_id;
+	uint8_t csum;
 
 	sw_ring = txq->sw_ring;
 	txr = txq->tx_ring;
@@ -114,6 +115,12 @@
 		ol_flags = tx_pkt->ol_flags;
 		nb_used = tx_pkt->nb_segs;
 		first_sw_id = sw_id;
+
+		if (ol_flags & GVE_TX_CKSUM_OFFLOAD_MASK_DQO)
+			csum = 1;
+		else
+			csum = 0;
+
 		do {
 			if (sw_ring[sw_id] != NULL)
 				PMD_DRV_LOG(DEBUG, "Overwriting an entry in sw_ring");
@@ -126,6 +133,8 @@
 			txd->pkt.dtype = GVE_TX_PKT_DESC_DTYPE_DQO;
 			txd->pkt.compl_tag = rte_cpu_to_le_16(first_sw_id);
 			txd->pkt.buf_size = RTE_MIN(tx_pkt->data_len, GVE_TX_MAX_BUF_SIZE_DQO);
+			txd->pkt.end_of_packet = 0;
+			txd->pkt.checksum_offload_enable = csum;
 
 			/* size of desc_ring and sw_ring could be different */
 			tx_id = (tx_id + 1) & mask;
@@ -138,9 +147,6 @@
 		/* fill the last descriptor with End of Packet (EOP) bit */
 		txd->pkt.end_of_packet = 1;
 
-		if (ol_flags & GVE_TX_CKSUM_OFFLOAD_MASK_DQO)
-			txd->pkt.checksum_offload_enable = 1;
-
 		txq->nb_free -= nb_used;
 		txq->nb_used += nb_used;
 	}
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] net/gve : Update EOP bit in txd rte_mbuf chain
  2024-08-01 16:24   ` Joshua Washington
@ 2024-08-01 17:50     ` Tathagat Priyadarshi
  0 siblings, 0 replies; 16+ messages in thread
From: Tathagat Priyadarshi @ 2024-08-01 17:50 UTC (permalink / raw)
  To: Joshua Washington
  Cc: Ferruh Yigit, Rushil Gupta, dev, Varun Lakkur Ambaji Rao

Hi Josh,

Updated the patch
(https://patches.dpdk.org/project/dpdk/patch/1722534481-2405601-1-git-send-email-tathagat.dpdk@gmail.com/),
please ACK if it looks good.

Thanks!

On Thu, Aug 1, 2024 at 9:55 PM Joshua Washington <joshwash@google.com> wrote:
>
> Hi, can we  try to serialize these patches? Let's fix the multidescriptor packets issue present in the driver first (ensuring that the entire descriptor is written each time). The TSO support is an entirely new feature and should likely not be backported to stable releases.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] net/gve : Update EOP & csum bit in txd rte_mbuf chain
  2024-08-01 17:27 ` [PATCH v3] net/gve : Update EOP & csum " Tathagat Priyadarshi
@ 2024-08-01 18:54   ` Stephen Hemminger
  2024-08-01 18:58     ` Tathagat Priyadarshi
  0 siblings, 1 reply; 16+ messages in thread
From: Stephen Hemminger @ 2024-08-01 18:54 UTC (permalink / raw)
  To: Tathagat Priyadarshi; +Cc: dev, stable, Varun Lakkur Ambaji Rao

On Thu,  1 Aug 2024 17:27:53 +0000
Tathagat Priyadarshi <tathagat.dpdk@gmail.com> wrote:

> +		if (ol_flags & GVE_TX_CKSUM_OFFLOAD_MASK_DQO)
> +			csum = 1;
> +		else
> +			cusm = 0;
> +

Obvious typo, did you do a final test build?

Could also use logical inverse operator instead of if() which will
generate better code sometimes.
		
		csum = !!(ol_flags & GVE_TX_CKSUM_OFFLOAD_MASK_DQO);


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] net/gve : Update EOP & csum bit in txd rte_mbuf chain
  2024-08-01 18:54   ` Stephen Hemminger
@ 2024-08-01 18:58     ` Tathagat Priyadarshi
  0 siblings, 0 replies; 16+ messages in thread
From: Tathagat Priyadarshi @ 2024-08-01 18:58 UTC (permalink / raw)
  To: stephen; +Cc: Varun Lakkur Ambaji Rao, dev, stable

[-- Attachment #1: Type: text/plain, Size: 736 bytes --]

Thanks for your suggestion Stephen, I have already updated the patch with
v4 & fixed the typo. Will consider your suggestion in the next version of
the patch.


On Fri, 2 Aug 2024 at 00:24, Stephen Hemminger <stephen@networkplumber.org>
wrote:

> On Thu,  1 Aug 2024 17:27:53 +0000
> Tathagat Priyadarshi <tathagat.dpdk@gmail.com> wrote:
>
> > +             if (ol_flags & GVE_TX_CKSUM_OFFLOAD_MASK_DQO)
> > +                     csum = 1;
> > +             else
> > +                     cusm = 0;
> > +
>
> Obvious typo, did you do a final test build?
>
> Could also use logical inverse operator instead of if() which will
> generate better code sometimes.
>
>                 csum = !!(ol_flags & GVE_TX_CKSUM_OFFLOAD_MASK_DQO);
>
>

[-- Attachment #2: Type: text/html, Size: 1291 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v5] net/gve : Update EOP & csum bit in txd rte_mbuf chain
  2024-07-31 16:38 [PATCH] net/gve : Update EOP bit in txd rte_mbuf chain Tathagat Priyadarshi
                   ` (4 preceding siblings ...)
  2024-08-01 17:48 ` [PATCH v4] " Tathagat Priyadarshi
@ 2024-08-02  5:08 ` Tathagat Priyadarshi
  2024-08-02  5:10   ` Tathagat Priyadarshi
  2024-08-07  9:39   ` Ferruh Yigit
  5 siblings, 2 replies; 16+ messages in thread
From: Tathagat Priyadarshi @ 2024-08-02  5:08 UTC (permalink / raw)
  To: stephen, dev; +Cc: Tathagat Priyadarshi, stable, Varun Lakkur Ambaji Rao

The EOP and csum bit was not set for all the packets in mbuf chain
causing packet transmission stalls for packets split across
mbuf in chain.

Fixes: 4022f99 ("net/gve: support basic Tx data path for DQO")
Cc: stable@dpdk.org

Signed-off-by: Tathagat Priyadarshi <tathagat.dpdk@gmail.com>
Signed-off-by: Varun Lakkur Ambaji Rao <varun.la@gmail.com>

Acked-by: Joshua Washington <joshwash@google.com>
---
 drivers/net/gve/gve_tx_dqo.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/gve/gve_tx_dqo.c b/drivers/net/gve/gve_tx_dqo.c
index a65e6aa..bbaf46d 100644
--- a/drivers/net/gve/gve_tx_dqo.c
+++ b/drivers/net/gve/gve_tx_dqo.c
@@ -89,6 +89,7 @@
 	uint16_t sw_id;
 	uint64_t bytes;
 	uint16_t first_sw_id;
+	uint8_t csum;
 
 	sw_ring = txq->sw_ring;
 	txr = txq->tx_ring;
@@ -114,6 +115,9 @@
 		ol_flags = tx_pkt->ol_flags;
 		nb_used = tx_pkt->nb_segs;
 		first_sw_id = sw_id;
+
+		csum = !!(ol_flags & GVE_TX_CKSUM_OFFLOAD_MASK_DQO);
+
 		do {
 			if (sw_ring[sw_id] != NULL)
 				PMD_DRV_LOG(DEBUG, "Overwriting an entry in sw_ring");
@@ -126,6 +130,8 @@
 			txd->pkt.dtype = GVE_TX_PKT_DESC_DTYPE_DQO;
 			txd->pkt.compl_tag = rte_cpu_to_le_16(first_sw_id);
 			txd->pkt.buf_size = RTE_MIN(tx_pkt->data_len, GVE_TX_MAX_BUF_SIZE_DQO);
+			txd->pkt.end_of_packet = 0;
+			txd->pkt.checksum_offload_enable = csum;
 
 			/* size of desc_ring and sw_ring could be different */
 			tx_id = (tx_id + 1) & mask;
@@ -138,9 +144,6 @@
 		/* fill the last descriptor with End of Packet (EOP) bit */
 		txd->pkt.end_of_packet = 1;
 
-		if (ol_flags & GVE_TX_CKSUM_OFFLOAD_MASK_DQO)
-			txd->pkt.checksum_offload_enable = 1;
-
 		txq->nb_free -= nb_used;
 		txq->nb_used += nb_used;
 	}
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5] net/gve : Update EOP & csum bit in txd rte_mbuf chain
  2024-08-02  5:08 ` [PATCH v5] " Tathagat Priyadarshi
@ 2024-08-02  5:10   ` Tathagat Priyadarshi
  2024-08-06 14:44     ` Tathagat Priyadarshi
  2024-08-07  9:39   ` Ferruh Yigit
  1 sibling, 1 reply; 16+ messages in thread
From: Tathagat Priyadarshi @ 2024-08-02  5:10 UTC (permalink / raw)
  To: stephen, dev; +Cc: stable, Varun Lakkur Ambaji Rao, Joshua Washington

Updated the if-else block with an optimised inverse operator. Thanks
for your suggestion Stephen.

On Fri, Aug 2, 2024 at 10:36 AM Tathagat Priyadarshi
<tathagat.dpdk@gmail.com> wrote:
>
> The EOP and csum bit was not set for all the packets in mbuf chain
> causing packet transmission stalls for packets split across
> mbuf in chain.
>
> Fixes: 4022f99 ("net/gve: support basic Tx data path for DQO")
> Cc: stable@dpdk.org
>
> Signed-off-by: Tathagat Priyadarshi <tathagat.dpdk@gmail.com>
> Signed-off-by: Varun Lakkur Ambaji Rao <varun.la@gmail.com>
>
> Acked-by: Joshua Washington <joshwash@google.com>
> ---
>  drivers/net/gve/gve_tx_dqo.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/gve/gve_tx_dqo.c b/drivers/net/gve/gve_tx_dqo.c
> index a65e6aa..bbaf46d 100644
> --- a/drivers/net/gve/gve_tx_dqo.c
> +++ b/drivers/net/gve/gve_tx_dqo.c
> @@ -89,6 +89,7 @@
>         uint16_t sw_id;
>         uint64_t bytes;
>         uint16_t first_sw_id;
> +       uint8_t csum;
>
>         sw_ring = txq->sw_ring;
>         txr = txq->tx_ring;
> @@ -114,6 +115,9 @@
>                 ol_flags = tx_pkt->ol_flags;
>                 nb_used = tx_pkt->nb_segs;
>                 first_sw_id = sw_id;
> +
> +               csum = !!(ol_flags & GVE_TX_CKSUM_OFFLOAD_MASK_DQO);
> +
>                 do {
>                         if (sw_ring[sw_id] != NULL)
>                                 PMD_DRV_LOG(DEBUG, "Overwriting an entry in sw_ring");
> @@ -126,6 +130,8 @@
>                         txd->pkt.dtype = GVE_TX_PKT_DESC_DTYPE_DQO;
>                         txd->pkt.compl_tag = rte_cpu_to_le_16(first_sw_id);
>                         txd->pkt.buf_size = RTE_MIN(tx_pkt->data_len, GVE_TX_MAX_BUF_SIZE_DQO);
> +                       txd->pkt.end_of_packet = 0;
> +                       txd->pkt.checksum_offload_enable = csum;
>
>                         /* size of desc_ring and sw_ring could be different */
>                         tx_id = (tx_id + 1) & mask;
> @@ -138,9 +144,6 @@
>                 /* fill the last descriptor with End of Packet (EOP) bit */
>                 txd->pkt.end_of_packet = 1;
>
> -               if (ol_flags & GVE_TX_CKSUM_OFFLOAD_MASK_DQO)
> -                       txd->pkt.checksum_offload_enable = 1;
> -
>                 txq->nb_free -= nb_used;
>                 txq->nb_used += nb_used;
>         }
> --
> 1.8.3.1
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5] net/gve : Update EOP & csum bit in txd rte_mbuf chain
  2024-08-02  5:10   ` Tathagat Priyadarshi
@ 2024-08-06 14:44     ` Tathagat Priyadarshi
  2024-08-07  7:36       ` Ferruh Yigit
  0 siblings, 1 reply; 16+ messages in thread
From: Tathagat Priyadarshi @ 2024-08-06 14:44 UTC (permalink / raw)
  To: dev, Ferruh Yigit

hi Ferruh,

Could you please accept the updated patch?! let us know what's pending.

https://patches.dpdk.org/project/dpdk/patch/1722575288-2408630-1-git-send-email-tathagat.dpdk@gmail.com/

TIA

On Fri, Aug 2, 2024 at 10:40 AM Tathagat Priyadarshi
<tathagat.dpdk@gmail.com> wrote:
>
> Updated the if-else block with an optimised inverse operator. Thanks
> for your suggestion Stephen.
>
> On Fri, Aug 2, 2024 at 10:36 AM Tathagat Priyadarshi
> <tathagat.dpdk@gmail.com> wrote:
> >
> > The EOP and csum bit was not set for all the packets in mbuf chain
> > causing packet transmission stalls for packets split across
> > mbuf in chain.
> >
> > Fixes: 4022f99 ("net/gve: support basic Tx data path for DQO")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Tathagat Priyadarshi <tathagat.dpdk@gmail.com>
> > Signed-off-by: Varun Lakkur Ambaji Rao <varun.la@gmail.com>
> >
> > Acked-by: Joshua Washington <joshwash@google.com>
> > ---
> >  drivers/net/gve/gve_tx_dqo.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/gve/gve_tx_dqo.c b/drivers/net/gve/gve_tx_dqo.c
> > index a65e6aa..bbaf46d 100644
> > --- a/drivers/net/gve/gve_tx_dqo.c
> > +++ b/drivers/net/gve/gve_tx_dqo.c
> > @@ -89,6 +89,7 @@
> >         uint16_t sw_id;
> >         uint64_t bytes;
> >         uint16_t first_sw_id;
> > +       uint8_t csum;
> >
> >         sw_ring = txq->sw_ring;
> >         txr = txq->tx_ring;
> > @@ -114,6 +115,9 @@
> >                 ol_flags = tx_pkt->ol_flags;
> >                 nb_used = tx_pkt->nb_segs;
> >                 first_sw_id = sw_id;
> > +
> > +               csum = !!(ol_flags & GVE_TX_CKSUM_OFFLOAD_MASK_DQO);
> > +
> >                 do {
> >                         if (sw_ring[sw_id] != NULL)
> >                                 PMD_DRV_LOG(DEBUG, "Overwriting an entry in sw_ring");
> > @@ -126,6 +130,8 @@
> >                         txd->pkt.dtype = GVE_TX_PKT_DESC_DTYPE_DQO;
> >                         txd->pkt.compl_tag = rte_cpu_to_le_16(first_sw_id);
> >                         txd->pkt.buf_size = RTE_MIN(tx_pkt->data_len, GVE_TX_MAX_BUF_SIZE_DQO);
> > +                       txd->pkt.end_of_packet = 0;
> > +                       txd->pkt.checksum_offload_enable = csum;
> >
> >                         /* size of desc_ring and sw_ring could be different */
> >                         tx_id = (tx_id + 1) & mask;
> > @@ -138,9 +144,6 @@
> >                 /* fill the last descriptor with End of Packet (EOP) bit */
> >                 txd->pkt.end_of_packet = 1;
> >
> > -               if (ol_flags & GVE_TX_CKSUM_OFFLOAD_MASK_DQO)
> > -                       txd->pkt.checksum_offload_enable = 1;
> > -
> >                 txq->nb_free -= nb_used;
> >                 txq->nb_used += nb_used;
> >         }
> > --
> > 1.8.3.1
> >

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5] net/gve : Update EOP & csum bit in txd rte_mbuf chain
  2024-08-06 14:44     ` Tathagat Priyadarshi
@ 2024-08-07  7:36       ` Ferruh Yigit
  0 siblings, 0 replies; 16+ messages in thread
From: Ferruh Yigit @ 2024-08-07  7:36 UTC (permalink / raw)
  To: Tathagat Priyadarshi; +Cc: dev

On 8/6/2024 3:44 PM, Tathagat Priyadarshi wrote:
> hi Ferruh,
> 
> Could you please accept the updated patch?! let us know what's pending.
> 
> https://patches.dpdk.org/project/dpdk/patch/1722575288-2408630-1-git-
> send-email-tathagat.dpdk@gmail.com/
> 
> TIA
> 

Hi Tathagat,

It is not waiting anything in particular, just waiting for processing, I
will try to check today.

Btw, a few housekeeping items,
- I have commented a few times before to run
'./devtools/check-git-log.sh' script and fix issues, if you are not
fixing them then maintainers need to spend time and fix them, please use
the script next time.

- Please do not top-post in the mail list, meaning put your notes below
the email, and if you are addressing anything specific, just put your
comment below it.
The combination of top-post and bottom-post makes long email threads
unreadable, so we go with bottom-post option.

Thanks,
ferruh

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5] net/gve : Update EOP & csum bit in txd rte_mbuf chain
  2024-08-02  5:08 ` [PATCH v5] " Tathagat Priyadarshi
  2024-08-02  5:10   ` Tathagat Priyadarshi
@ 2024-08-07  9:39   ` Ferruh Yigit
  1 sibling, 0 replies; 16+ messages in thread
From: Ferruh Yigit @ 2024-08-07  9:39 UTC (permalink / raw)
  To: Tathagat Priyadarshi, stephen, dev; +Cc: stable, Varun Lakkur Ambaji Rao

On 8/2/2024 6:08 AM, Tathagat Priyadarshi wrote:
> The EOP and csum bit was not set for all the packets in mbuf chain
> causing packet transmission stalls for packets split across
> mbuf in chain.
> 
> Fixes: 4022f99 ("net/gve: support basic Tx data path for DQO")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Tathagat Priyadarshi <tathagat.dpdk@gmail.com>
> Signed-off-by: Varun Lakkur Ambaji Rao <varun.la@gmail.com>
> 
> Acked-by: Joshua Washington <joshwash@google.com>
>

Updated commit log slightly, please check the merged version.

Applied to dpdk-next-net/main, thanks.


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2024-08-07  9:39 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-07-31 16:38 [PATCH] net/gve : Update EOP bit in txd rte_mbuf chain Tathagat Priyadarshi
2024-07-31 20:30 ` Joshua Washington
2024-08-01 10:23   ` Tathagat Priyadarshi
2024-08-01 11:16 ` Ferruh Yigit
2024-08-01 16:24   ` Joshua Washington
2024-08-01 17:50     ` Tathagat Priyadarshi
2024-08-01 11:31 ` [PATCH v2] " Tathagat Priyadarshi
2024-08-01 17:27 ` [PATCH v3] net/gve : Update EOP & csum " Tathagat Priyadarshi
2024-08-01 18:54   ` Stephen Hemminger
2024-08-01 18:58     ` Tathagat Priyadarshi
2024-08-01 17:48 ` [PATCH v4] " Tathagat Priyadarshi
2024-08-02  5:08 ` [PATCH v5] " Tathagat Priyadarshi
2024-08-02  5:10   ` Tathagat Priyadarshi
2024-08-06 14:44     ` Tathagat Priyadarshi
2024-08-07  7:36       ` Ferruh Yigit
2024-08-07  9:39   ` Ferruh Yigit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).