net: stmmac: set total length of the packet to be transmitted in TDES3

PATCH Applied Niklas Cassel 5 participants 7 messages Apr 10, 2017 – Apr 13, 2017

Subsystems (4)

Networking Network drivers Arm STM32 stmmac Ethernet driver

lore

Share & tools

lore

Niklas Cassel

author

2017-04-10 18:33 UTC #0 lore raw

PATCH 1/1 net: stmmac: set total length of the packet to be transmitted in TDES3

From: Niklas Cassel <[email protected]>

Field FL/TPL in register TDES3 is not correctly set on GMAC4. TX appears to be functional on GMAC 4.10a even if this field is not set, however, to avoid relying on undefined behavior, set the length in TDES3.

The field has a different meaning depending on if the TSE bit in TDES3 is set or not (TSO). However, regardless of the TSE bit, the field is not optional. The field is already set correctly when the TSE bit is set.

Since there is no limit for the number of descriptors that can be used for a single packet, the field should be set to the sum of the buffers contained in:
[<desc with First Descriptor bit set> ... <desc n> ...
<desc with Last Descriptor bit set>], which should be equal to skb->len.

Signed-off-by Niklas Cassel <[email protected]>

---
 drivers/net/ethernet/stmicro/stmmac/chain_mode.c   | 6 +++---
 drivers/net/ethernet/stmicro/stmmac/common.h       | 2 +-
 drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c | 3 ++-
 drivers/net/ethernet/stmicro/stmmac/enh_desc.c     | 2 +-
 drivers/net/ethernet/stmicro/stmmac/norm_desc.c    | 2 +-
 drivers/net/ethernet/stmicro/stmmac/ring_mode.c    | 9 ++++++---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 5 +++--
 7 files changed, 17 insertions(+), 12 deletions(-)

drivers/net/ethernet/stmicro/stmmac/chain_mode.c +3 -3

index 37881f81319e..e93c40b4631e 100644
@@ -52,7 +52,7 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
 	tx_q->tx_skbuff_dma[entry].len = bmax;
 	/* do not close the descriptor and do not set own bit */
 	priv->hw->desc->prepare_tx_desc(desc, 1, bmax, csum, STMMAC_CHAIN_MODE,
-					0, false);
+					0, false, skb->len);
 
 	while (len != 0) {
 		tx_q->tx_skbuff[entry] = NULL;

@@ -70,7 +70,7 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
 			tx_q->tx_skbuff_dma[entry].len = bmax;
 			priv->hw->desc->prepare_tx_desc(desc, 0, bmax, csum,
 							STMMAC_CHAIN_MODE, 1,
-							false);
+							false, skb->len);
 			len -= bmax;
 			i++;
 		} else {

@@ -85,7 +85,7 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
 			/* last descriptor can be set now */
 			priv->hw->desc->prepare_tx_desc(desc, 0, len, csum,
 							STMMAC_CHAIN_MODE, 1,
-							true);
+							true, skb->len);
 			len = 0;
 		}
 	}

drivers/net/ethernet/stmicro/stmmac/common.h +1 -1

index 90d28bcad880..b7ce3fbb5375 100644
@@ -373,7 +373,7 @@ struct stmmac_desc_ops {
 	/* Invoked by the xmit function to prepare the tx descriptor */
 	void (*prepare_tx_desc) (struct dma_desc *p, int is_fs, int len,
 				 bool csum_flag, int mode, bool tx_own,
-				 bool ls);
+				 bool ls, unsigned int tot_pkt_len);
 	void (*prepare_tso_tx_desc)(struct dma_desc *p, int is_fs, int len1,
 				    int len2, bool tx_own, bool ls,
 				    unsigned int tcphdrlen,

drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c +2 -1

index 843ec69222ea..aa6476439aee 100644
@@ -304,12 +304,13 @@ static void dwmac4_rd_init_tx_desc(struct dma_desc *p, int mode, int end)
 
 static void dwmac4_rd_prepare_tx_desc(struct dma_desc *p, int is_fs, int len,
 				      bool csum_flag, int mode, bool tx_own,
-				      bool ls)
+				      bool ls, unsigned int tot_pkt_len)
 {
 	unsigned int tdes3 = le32_to_cpu(p->des3);
 
 	p->des2 |= cpu_to_le32(len & TDES2_BUFFER1_SIZE_MASK);
 
+	tdes3 |= tot_pkt_len & TDES3_PACKET_SIZE_MASK;
 	if (is_fs)
 		tdes3 |= TDES3_FIRST_DESCRIPTOR;
 	else

drivers/net/ethernet/stmicro/stmmac/enh_desc.c +1 -1

index 323b59ec74a3..7546b3664113 100644
@@ -315,7 +315,7 @@ static void enh_desc_release_tx_desc(struct dma_desc *p, int mode)
 
 static void enh_desc_prepare_tx_desc(struct dma_desc *p, int is_fs, int len,
 				     bool csum_flag, int mode, bool tx_own,
-				     bool ls)
+				     bool ls, unsigned int tot_pkt_len)
 {
 	unsigned int tdes0 = le32_to_cpu(p->des0);

drivers/net/ethernet/stmicro/stmmac/norm_desc.c +1 -1

index efb818ebd55e..f817f8f36569 100644
@@ -191,7 +191,7 @@ static void ndesc_release_tx_desc(struct dma_desc *p, int mode)
 
 static void ndesc_prepare_tx_desc(struct dma_desc *p, int is_fs, int len,
 				  bool csum_flag, int mode, bool tx_own,
-				  bool ls)
+				  bool ls, unsigned int tot_pkt_len)
 {
 	unsigned int tdes1 = le32_to_cpu(p->des1);

drivers/net/ethernet/stmicro/stmmac/ring_mode.c +6 -3

index 31213e64513d..28e4b5d50ce6 100644
@@ -59,7 +59,8 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
 
 		desc->des3 = cpu_to_le32(des2 + BUF_SIZE_4KiB);
 		priv->hw->desc->prepare_tx_desc(desc, 1, bmax, csum,
-						STMMAC_RING_MODE, 0, false);
+						STMMAC_RING_MODE, 0,
+						false, skb->len);
 		tx_q->tx_skbuff[entry] = NULL;
 		entry = STMMAC_GET_ENTRY(entry, DMA_TX_SIZE);

@@ -79,7 +80,8 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
 
 		desc->des3 = cpu_to_le32(des2 + BUF_SIZE_4KiB);
 		priv->hw->desc->prepare_tx_desc(desc, 0, len, csum,
-						STMMAC_RING_MODE, 1, true);
+						STMMAC_RING_MODE, 1,
+						true, skb->len);
 	} else {
 		des2 = dma_map_single(priv->device, skb->data,
 				      nopaged_len, DMA_TO_DEVICE);

@@ -91,7 +93,8 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
 		tx_q->tx_skbuff_dma[entry].is_jumbo = true;
 		desc->des3 = cpu_to_le32(des2 + BUF_SIZE_4KiB);
 		priv->hw->desc->prepare_tx_desc(desc, 1, nopaged_len, csum,
-						STMMAC_RING_MODE, 0, true);
+						STMMAC_RING_MODE, 0,
+						true, skb->len);
 	}
 
 	tx_q->cur_tx = entry;

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +3 -2

index a89f76b27ea2..5c1ebc70bebd 100644
@@ -3033,7 +3033,8 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
 
 		/* Prepare the descriptor and set the own bit too */
 		priv->hw->desc->prepare_tx_desc(desc, 0, len, csum_insertion,
-						priv->mode, 1, last_segment);
+						priv->mode, 1, last_segment,
+						skb->len);
 	}
 
 	entry = STMMAC_GET_ENTRY(entry, DMA_TX_SIZE);

@@ -3116,7 +3117,7 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
 		/* Prepare the first descriptor setting the OWN bit too */
 		priv->hw->desc->prepare_tx_desc(first, 1, nopaged_len,
 						csum_insertion, priv->mode, 1,
-						last_segment);
+						last_segment, skb->len);
 
 		/* The own bit must be the latest setting done when prepare the
 		 * descriptor and then barrier is needed to make sure that

Signature · 1 line

2.11.0

Giuseppe CAVALLARO

2017-04-11 05:40 UTC #1 lore raw

Hi Niklas

patch looks ok for me, Alex any feedback?

peppe

Quoted 173 lines · Niklas Cassel

From: Niklas Cassel <[email protected]>

Signed-off-by Niklas Cassel <[email protected]>

---
  drivers/net/ethernet/stmicro/stmmac/chain_mode.c   | 6 +++---
  drivers/net/ethernet/stmicro/stmmac/common.h       | 2 +-
  drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c | 3 ++-
  drivers/net/ethernet/stmicro/stmmac/enh_desc.c     | 2 +-
  drivers/net/ethernet/stmicro/stmmac/norm_desc.c    | 2 +-
  drivers/net/ethernet/stmicro/stmmac/ring_mode.c    | 9 ++++++---
  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 5 +++--
  7 files changed, 17 insertions(+), 12 deletions(-)

drivers/net/ethernet/stmicro/stmmac/chain_mode.c +3 -3

index 37881f81319e..e93c40b4631e 100644
@@ -52,7 +52,7 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
  	tx_q->tx_skbuff_dma[entry].len = bmax;
  	/* do not close the descriptor and do not set own bit */
  	priv->hw->desc->prepare_tx_desc(desc, 1, bmax, csum, STMMAC_CHAIN_MODE,
-					0, false);
+					0, false, skb->len);
 
  	while (len != 0) {
  		tx_q->tx_skbuff[entry] = NULL;

@@ -70,7 +70,7 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
  			tx_q->tx_skbuff_dma[entry].len = bmax;
  			priv->hw->desc->prepare_tx_desc(desc, 0, bmax, csum,
  							STMMAC_CHAIN_MODE, 1,
-							false);
+							false, skb->len);
  			len -= bmax;
  			i++;
  		} else {

@@ -85,7 +85,7 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
  			/* last descriptor can be set now */
  			priv->hw->desc->prepare_tx_desc(desc, 0, len, csum,
  							STMMAC_CHAIN_MODE, 1,
-							true);
+							true, skb->len);
  			len = 0;
  		}
  	}

drivers/net/ethernet/stmicro/stmmac/common.h +1 -1

index 90d28bcad880..b7ce3fbb5375 100644
@@ -373,7 +373,7 @@ struct stmmac_desc_ops {
  	/* Invoked by the xmit function to prepare the tx descriptor */
  	void (*prepare_tx_desc) (struct dma_desc *p, int is_fs, int len,
  				 bool csum_flag, int mode, bool tx_own,
-				 bool ls);
+				 bool ls, unsigned int tot_pkt_len);
  	void (*prepare_tso_tx_desc)(struct dma_desc *p, int is_fs, int len1,
  				    int len2, bool tx_own, bool ls,
  				    unsigned int tcphdrlen,

drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c +2 -1

index 843ec69222ea..aa6476439aee 100644
@@ -304,12 +304,13 @@ static void dwmac4_rd_init_tx_desc(struct dma_desc *p, int mode, int end)
 
  static void dwmac4_rd_prepare_tx_desc(struct dma_desc *p, int is_fs, int len,
  				      bool csum_flag, int mode, bool tx_own,
-				      bool ls)
+				      bool ls, unsigned int tot_pkt_len)
  {
  	unsigned int tdes3 = le32_to_cpu(p->des3);
 
  	p->des2 |= cpu_to_le32(len & TDES2_BUFFER1_SIZE_MASK);
 
+	tdes3 |= tot_pkt_len & TDES3_PACKET_SIZE_MASK;
  	if (is_fs)
  		tdes3 |= TDES3_FIRST_DESCRIPTOR;
  	else

drivers/net/ethernet/stmicro/stmmac/enh_desc.c +1 -1

index 323b59ec74a3..7546b3664113 100644
@@ -315,7 +315,7 @@ static void enh_desc_release_tx_desc(struct dma_desc *p, int mode)
 
  static void enh_desc_prepare_tx_desc(struct dma_desc *p, int is_fs, int len,
  				     bool csum_flag, int mode, bool tx_own,
-				     bool ls)
+				     bool ls, unsigned int tot_pkt_len)
  {
  	unsigned int tdes0 = le32_to_cpu(p->des0);

drivers/net/ethernet/stmicro/stmmac/norm_desc.c +1 -1

index efb818ebd55e..f817f8f36569 100644
@@ -191,7 +191,7 @@ static void ndesc_release_tx_desc(struct dma_desc *p, int mode)
 
  static void ndesc_prepare_tx_desc(struct dma_desc *p, int is_fs, int len,
  				  bool csum_flag, int mode, bool tx_own,
-				  bool ls)
+				  bool ls, unsigned int tot_pkt_len)
  {
  	unsigned int tdes1 = le32_to_cpu(p->des1);

drivers/net/ethernet/stmicro/stmmac/ring_mode.c +6 -3

index 31213e64513d..28e4b5d50ce6 100644
@@ -59,7 +59,8 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
 
  		desc->des3 = cpu_to_le32(des2 + BUF_SIZE_4KiB);
  		priv->hw->desc->prepare_tx_desc(desc, 1, bmax, csum,
-						STMMAC_RING_MODE, 0, false);
+						STMMAC_RING_MODE, 0,
+						false, skb->len);
  		tx_q->tx_skbuff[entry] = NULL;
  		entry = STMMAC_GET_ENTRY(entry, DMA_TX_SIZE);

@@ -79,7 +80,8 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
 
  		desc->des3 = cpu_to_le32(des2 + BUF_SIZE_4KiB);
  		priv->hw->desc->prepare_tx_desc(desc, 0, len, csum,
-						STMMAC_RING_MODE, 1, true);
+						STMMAC_RING_MODE, 1,
+						true, skb->len);
  	} else {
  		des2 = dma_map_single(priv->device, skb->data,
  				      nopaged_len, DMA_TO_DEVICE);

@@ -91,7 +93,8 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
  		tx_q->tx_skbuff_dma[entry].is_jumbo = true;
  		desc->des3 = cpu_to_le32(des2 + BUF_SIZE_4KiB);
  		priv->hw->desc->prepare_tx_desc(desc, 1, nopaged_len, csum,
-						STMMAC_RING_MODE, 0, true);
+						STMMAC_RING_MODE, 0,
+						true, skb->len);
  	}
 
  	tx_q->cur_tx = entry;

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +3 -2

index a89f76b27ea2..5c1ebc70bebd 100644
@@ -3033,7 +3033,8 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
 
  		/* Prepare the descriptor and set the own bit too */
  		priv->hw->desc->prepare_tx_desc(desc, 0, len, csum_insertion,
-						priv->mode, 1, last_segment);
+						priv->mode, 1, last_segment,
+						skb->len);
  	}
 
  	entry = STMMAC_GET_ENTRY(entry, DMA_TX_SIZE);

@@ -3116,7 +3117,7 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
  		/* Prepare the first descriptor setting the OWN bit too */
  		priv->hw->desc->prepare_tx_desc(first, 1, nopaged_len,
  						csum_insertion, priv->mode, 1,
-						last_segment);
+						last_segment, skb->len);
 
  		/* The own bit must be the latest setting done when prepare the
  		 * descriptor and then barrier is needed to make sure that

Alexandre Torgue

2017-04-11 07:20 UTC #2 lore raw

Hi,

Quoted 3 lines · Giuseppe CAVALLARO

Hi Niklas
patch looks ok for me, Alex any feedback?

It sounds good for me to.

Quoted 188 lines

peppe

Quoted 182 lines · Niklas Cassel

From: Niklas Cassel <[email protected]>

Signed-off-by Niklas Cassel <[email protected]>

---
  drivers/net/ethernet/stmicro/stmmac/chain_mode.c   | 6 +++---
  drivers/net/ethernet/stmicro/stmmac/common.h       | 2 +-
  drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c | 3 ++-
  drivers/net/ethernet/stmicro/stmmac/enh_desc.c     | 2 +-
  drivers/net/ethernet/stmicro/stmmac/norm_desc.c    | 2 +-
  drivers/net/ethernet/stmicro/stmmac/ring_mode.c    | 9 ++++++---
  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 5 +++--
  7 files changed, 17 insertions(+), 12 deletions(-)

drivers/net/ethernet/stmicro/stmmac/chain_mode.c +1 -1

index 37881f81319e..e93c40b4631e 100644
@@ -52,7 +52,7 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff
 *skb, int csum)
      tx_q->tx_skbuff_dma[entry].len = bmax;
      /* do not close the descriptor and do not set own bit */
      priv->hw->desc->prepare_tx_desc(desc, 1, bmax, csum,
 STMMAC_CHAIN_MODE,
-                    0, false);
+                    0, false, skb->len);
        while (len != 0) {

          tx_q->tx_skbuff[entry] = NULL;

drivers/net/ethernet/stmicro/stmmac/chain_mode.c +1 -1

@@ -70,7 +70,7 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff
 *skb, int csum)
              tx_q->tx_skbuff_dma[entry].len = bmax;
              priv->hw->desc->prepare_tx_desc(desc, 0, bmax, csum,
                              STMMAC_CHAIN_MODE, 1,
-                            false);
+                            false, skb->len);
              len -= bmax;
              i++;

          } else {

drivers/net/ethernet/stmicro/stmmac/chain_mode.c +1 -1

@@ -85,7 +85,7 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff
 *skb, int csum)
              /* last descriptor can be set now */
              priv->hw->desc->prepare_tx_desc(desc, 0, len, csum,
                              STMMAC_CHAIN_MODE, 1,
-                            true);
+                            true, skb->len);
              len = 0;
          }

drivers/net/ethernet/stmicro/stmmac/common.h +1 -1

index 90d28bcad880..b7ce3fbb5375 100644
@@ -373,7 +373,7 @@ struct stmmac_desc_ops {
      /* Invoked by the xmit function to prepare the tx descriptor */
      void (*prepare_tx_desc) (struct dma_desc *p, int is_fs, int len,
                   bool csum_flag, int mode, bool tx_own,
-                 bool ls);
+                 bool ls, unsigned int tot_pkt_len);
      void (*prepare_tso_tx_desc)(struct dma_desc *p, int is_fs, int
 len1,
                      int len2, bool tx_own, bool ls,

unsigned int tcphdrlen,

drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c +1 -1

index 843ec69222ea..aa6476439aee 100644
@@ -304,12 +304,13 @@ static void dwmac4_rd_init_tx_desc(struct
 dma_desc *p, int mode, int end)
    static void dwmac4_rd_prepare_tx_desc(struct dma_desc *p, int
 is_fs, int len,
                        bool csum_flag, int mode, bool tx_own,
-                      bool ls)
+                      bool ls, unsigned int tot_pkt_len)
  {
      unsigned int tdes3 = le32_to_cpu(p->des3);
        p->des2 |= cpu_to_le32(len & TDES2_BUFFER1_SIZE_MASK);
  +    tdes3 |= tot_pkt_len & TDES3_PACKET_SIZE_MASK;
      if (is_fs)
          tdes3 |= TDES3_FIRST_DESCRIPTOR;
      else

drivers/net/ethernet/stmicro/stmmac/enh_desc.c +1 -1

index 323b59ec74a3..7546b3664113 100644
@@ -315,7 +315,7 @@ static void enh_desc_release_tx_desc(struct
 dma_desc *p, int mode)
    static void enh_desc_prepare_tx_desc(struct dma_desc *p, int
 is_fs, int len,
                       bool csum_flag, int mode, bool tx_own,
-                     bool ls)
+                     bool ls, unsigned int tot_pkt_len)
  {
      unsigned int tdes0 = le32_to_cpu(p->des0);

diff --git a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c index efb818ebd55e..f817f8f36569 100644

drivers/net/ethernet/stmicro/stmmac/norm_desc.c +1 -1

@@ -191,7 +191,7 @@ static void ndesc_release_tx_desc(struct dma_desc
 *p, int mode)
    static void ndesc_prepare_tx_desc(struct dma_desc *p, int is_fs,
 int len,
                    bool csum_flag, int mode, bool tx_own,
-                  bool ls)
+                  bool ls, unsigned int tot_pkt_len)
  {
      unsigned int tdes1 = le32_to_cpu(p->des1);

diff --git a/drivers/net/ethernet/stmicro/stmmac/ring_mode.c
b/drivers/net/ethernet/stmicro/stmmac/ring_mode.c index 31213e64513d..28e4b5d50ce6 100644

drivers/net/ethernet/stmicro/stmmac/ring_mode.c +2 -1

@@ -59,7 +59,8 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff
 *skb, int csum)
            desc->des3 = cpu_to_le32(des2 + BUF_SIZE_4KiB);
          priv->hw->desc->prepare_tx_desc(desc, 1, bmax, csum,
-                        STMMAC_RING_MODE, 0, false);
+                        STMMAC_RING_MODE, 0,
+                        false, skb->len);
          tx_q->tx_skbuff[entry] = NULL;
          entry = STMMAC_GET_ENTRY(entry, DMA_TX_SIZE);
  @@ -79,7 +80,8 @@ static int stmmac_jumbo_frm(void *p, struct

sk_buff *skb, int csum)

            desc->des3 = cpu_to_le32(des2 + BUF_SIZE_4KiB);
          priv->hw->desc->prepare_tx_desc(desc, 0, len, csum,

drivers/net/ethernet/stmicro/stmmac/ring_mode.c +4 -2

-                        STMMAC_RING_MODE, 1, true);
+                        STMMAC_RING_MODE, 1,
+                        true, skb->len);
      } else {
          des2 = dma_map_single(priv->device, skb->data,
                        nopaged_len, DMA_TO_DEVICE);

@@ -91,7 +93,8 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff
 *skb, int csum)
          tx_q->tx_skbuff_dma[entry].is_jumbo = true;
          desc->des3 = cpu_to_le32(des2 + BUF_SIZE_4KiB);
          priv->hw->desc->prepare_tx_desc(desc, 1, nopaged_len, csum,
-                        STMMAC_RING_MODE, 0, true);
+                        STMMAC_RING_MODE, 0,
+                        true, skb->len);
      }
        tx_q->cur_tx = entry;

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +3 -2

index a89f76b27ea2..5c1ebc70bebd 100644
@@ -3033,7 +3033,8 @@ static netdev_tx_t stmmac_xmit(struct sk_buff
 *skb, struct net_device *dev)
            /* Prepare the descriptor and set the own bit too */
          priv->hw->desc->prepare_tx_desc(desc, 0, len, csum_insertion,
-                        priv->mode, 1, last_segment);
+                        priv->mode, 1, last_segment,
+                        skb->len);
      }
        entry = STMMAC_GET_ENTRY(entry, DMA_TX_SIZE);

@@ -3116,7 +3117,7 @@ static netdev_tx_t stmmac_xmit(struct sk_buff
 *skb, struct net_device *dev)
          /* Prepare the first descriptor setting the OWN bit too */
          priv->hw->desc->prepare_tx_desc(first, 1, nopaged_len,
                          csum_insertion, priv->mode, 1,
-                        last_segment);
+                        last_segment, skb->len);
            /* The own bit must be the latest setting done when

prepare the
* descriptor and then barrier is needed to make sure that

Joao Pinto

2017-04-11 08:48 UTC #3 lore raw

In reply to Niklas Cassel

Hi Niklas,

Às 7:33 PM de 4/10/2017, Niklas Cassel escreveu:

Quoted 10 lines

From: Niklas Cassel <[email protected]>
Field FL/TPL in register TDES3 is not correctly set on GMAC4. TX appears to be functional on GMAC 4.10a even if this field is not set, however, to avoid relying on undefined behavior, set the length in TDES3.
The field has a different meaning depending on if the TSE bit in TDES3 is set or not (TSO). However, regardless of the TSE bit, the field is not optional. The field is already set correctly when the TSE bit is set.

Quoted 7 lines

Since there is no limit for the number of descriptors that can be used for a single packet, the field should be set to the sum of the buffers contained in:
[<desc with First Descriptor bit set> ... <desc n> ...
<desc with Last Descriptor bit set>], which should be equal to skb->len.
Signed-off-by Niklas Cassel <[email protected]>

Sounds fine to me. Did you check for performance improvement? Thanks.

Joao

Niklas Cassel

author

2017-04-11 09:10 UTC #4 lore raw

Quoted 20 lines · Joao Pinto

Hi Niklas,
Às 7:33 PM de 4/10/2017, Niklas Cassel escreveu:
Quoted 16 lines
From: Niklas Cassel <[email protected]>
Field FL/TPL in register TDES3 is not correctly set on GMAC4. TX appears to be functional on GMAC 4.10a even if this field is not set, however, to avoid relying on undefined behavior, set the length in TDES3.
The field has a different meaning depending on if the TSE bit in TDES3 is set or not (TSO). However, regardless of the TSE bit, the field is not optional. The field is already set correctly when the TSE bit is set.
Since there is no limit for the number of descriptors that can be used for a single packet, the field should be set to the sum of the buffers contained in:
[<desc with First Descriptor bit set> ... <desc n> ...
<desc with Last Descriptor bit set>], which should be equal to skb->len.

Quoted 4 lines

Quoted 1 line
Signed-off-by Niklas Cassel <[email protected]>
Sounds fine to me. Did you check for performance improvement? Thanks.

Hello Joao,

I could not see any difference with 1 TCP stream in iperf3

Without the patch:

# ./iperf3-armhf -c 192.168.0.1
Connecting to host 192.168.0.1, port 5201
[  4] local 192.168.0.105 port 51878 connected to 192.168.0.1 port 5201
[   23.486426] random: fast init done
[ ID] Interval           Transfer     Bandwidth       Retr  Cwnd
[  4]   0.00-1.00   sec  61.7 MBytes   517 Mbits/sec    0    294 KBytes       
[  4]   1.00-2.00   sec  61.3 MBytes   515 Mbits/sec    0    294 KBytes       
[   26.043850] random: crng init done
[  4]   2.00-3.00   sec  61.4 MBytes   515 Mbits/sec    0    294 KBytes       
[  4]   3.00-4.00   sec  61.0 MBytes   512 Mbits/sec    0    294 KBytes       
[  4]   4.00-5.00   sec  61.5 MBytes   516 Mbits/sec    0    294 KBytes       
[  4]   5.00-6.00   sec  61.2 MBytes   514 Mbits/sec    0    294 KBytes       
[  4]   6.00-7.00   sec  61.5 MBytes   516 Mbits/sec    0    294 KBytes       
[  4]   7.00-8.00   sec  61.4 MBytes   515 Mbits/sec    0    294 KBytes       
[  4]   8.00-9.00   sec  61.2 MBytes   514 Mbits/sec    0    294 KBytes       
[  4]   9.00-10.00  sec  61.3 MBytes   514 Mbits/sec    0    294 KBytes       
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bandwidth       Retr
[  4]   0.00-10.00  sec   614 MBytes   515 Mbits/sec    0             sender
[  4]   0.00-10.00  sec   613 MBytes   514 Mbits/sec                  receiver

With the patch:

# ./iperf3-armhf -c 192.168.0.1
Connecting to host 192.168.0.1, port 5201
[  4] local 192.168.0.105 port 37696 connected to 192.168.0.1 port 5201
[   85.266262] random: fast init done
[ ID] Interval           Transfer     Bandwidth       Retr  Cwnd
[  4]   0.00-1.00   sec  62.1 MBytes   520 Mbits/sec    0    288 KBytes       
[  4]   1.00-2.00   sec  61.0 MBytes   512 Mbits/sec    0    288 KBytes       
[   87.782547] random: crng init done
[  4]   2.00-3.00   sec  61.3 MBytes   515 Mbits/sec    0    300 KBytes       
[  4]   3.00-4.00   sec  61.1 MBytes   512 Mbits/sec    0    300 KBytes       
[  4]   4.00-5.00   sec  61.4 MBytes   515 Mbits/sec    0    300 KBytes       
[  4]   5.00-6.00   sec  61.1 MBytes   513 Mbits/sec    0    300 KBytes       
[  4]   6.00-7.00   sec  61.1 MBytes   513 Mbits/sec    0    300 KBytes       
[  4]   7.00-8.00   sec  61.6 MBytes   517 Mbits/sec    0    300 KBytes       
[  4]   8.00-9.00   sec  61.3 MBytes   515 Mbits/sec    0    300 KBytes       
[  4]   9.00-10.00  sec  61.6 MBytes   517 Mbits/sec    0    300 KBytes       
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bandwidth       Retr
[  4]   0.00-10.00  sec   614 MBytes   515 Mbits/sec    0             sender
[  4]   0.00-10.00  sec   613 MBytes   514 Mbits/sec                  receiver

Note that the results above are with TSO off.
With TSO on, I get 921 Mbps.
However, the patch should not affect the case where TSO is on.

Regards,
Niklas

Giuseppe CAVALLARO

Acked-by on v1 1/1

2017-04-11 12:41 UTC #5 lore raw

In reply to Niklas Cassel

Quoted 10 lines · Niklas Cassel

From: Niklas Cassel <[email protected]>
Field FL/TPL in register TDES3 is not correctly set on GMAC4. TX appears to be functional on GMAC 4.10a even if this field is not set, however, to avoid relying on undefined behavior, set the length in TDES3.
The field has a different meaning depending on if the TSE bit in TDES3 is set or not (TSO). However, regardless of the TSE bit, the field is not optional. The field is already set correctly when the TSE bit is set.

Quoted 7 lines

Since there is no limit for the number of descriptors that can be used for a single packet, the field should be set to the sum of the buffers contained in:
[<desc with First Descriptor bit set> ... <desc n> ...
<desc with Last Descriptor bit set>], which should be equal to skb->len.
Signed-off-by Niklas Cassel <[email protected]>

Acked-by Giuseppe Cavallaro <[email protected]>

Quoted 156 lines

---
  drivers/net/ethernet/stmicro/stmmac/chain_mode.c   | 6 +++---
  drivers/net/ethernet/stmicro/stmmac/common.h       | 2 +-
  drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c | 3 ++-
  drivers/net/ethernet/stmicro/stmmac/enh_desc.c     | 2 +-
  drivers/net/ethernet/stmicro/stmmac/norm_desc.c    | 2 +-
  drivers/net/ethernet/stmicro/stmmac/ring_mode.c    | 9 ++++++---
  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 5 +++--
  7 files changed, 17 insertions(+), 12 deletions(-)

drivers/net/ethernet/stmicro/stmmac/chain_mode.c +3 -3

index 37881f81319e..e93c40b4631e 100644
@@ -52,7 +52,7 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
  	tx_q->tx_skbuff_dma[entry].len = bmax;
  	/* do not close the descriptor and do not set own bit */
  	priv->hw->desc->prepare_tx_desc(desc, 1, bmax, csum, STMMAC_CHAIN_MODE,
-					0, false);
+					0, false, skb->len);
 
  	while (len != 0) {
  		tx_q->tx_skbuff[entry] = NULL;

@@ -70,7 +70,7 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
  			tx_q->tx_skbuff_dma[entry].len = bmax;
  			priv->hw->desc->prepare_tx_desc(desc, 0, bmax, csum,
  							STMMAC_CHAIN_MODE, 1,
-							false);
+							false, skb->len);
  			len -= bmax;
  			i++;
  		} else {

@@ -85,7 +85,7 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
  			/* last descriptor can be set now */
  			priv->hw->desc->prepare_tx_desc(desc, 0, len, csum,
  							STMMAC_CHAIN_MODE, 1,
-							true);
+							true, skb->len);
  			len = 0;
  		}
  	}

drivers/net/ethernet/stmicro/stmmac/common.h +1 -1

index 90d28bcad880..b7ce3fbb5375 100644
@@ -373,7 +373,7 @@ struct stmmac_desc_ops {
  	/* Invoked by the xmit function to prepare the tx descriptor */
  	void (*prepare_tx_desc) (struct dma_desc *p, int is_fs, int len,
  				 bool csum_flag, int mode, bool tx_own,
-				 bool ls);
+				 bool ls, unsigned int tot_pkt_len);
  	void (*prepare_tso_tx_desc)(struct dma_desc *p, int is_fs, int len1,
  				    int len2, bool tx_own, bool ls,
  				    unsigned int tcphdrlen,

drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c +2 -1

index 843ec69222ea..aa6476439aee 100644
@@ -304,12 +304,13 @@ static void dwmac4_rd_init_tx_desc(struct dma_desc *p, int mode, int end)
 
  static void dwmac4_rd_prepare_tx_desc(struct dma_desc *p, int is_fs, int len,
  				      bool csum_flag, int mode, bool tx_own,
-				      bool ls)
+				      bool ls, unsigned int tot_pkt_len)
  {
  	unsigned int tdes3 = le32_to_cpu(p->des3);
 
  	p->des2 |= cpu_to_le32(len & TDES2_BUFFER1_SIZE_MASK);
 
+	tdes3 |= tot_pkt_len & TDES3_PACKET_SIZE_MASK;
  	if (is_fs)
  		tdes3 |= TDES3_FIRST_DESCRIPTOR;
  	else

drivers/net/ethernet/stmicro/stmmac/enh_desc.c +1 -1

index 323b59ec74a3..7546b3664113 100644
@@ -315,7 +315,7 @@ static void enh_desc_release_tx_desc(struct dma_desc *p, int mode)
 
  static void enh_desc_prepare_tx_desc(struct dma_desc *p, int is_fs, int len,
  				     bool csum_flag, int mode, bool tx_own,
-				     bool ls)
+				     bool ls, unsigned int tot_pkt_len)
  {
  	unsigned int tdes0 = le32_to_cpu(p->des0);

drivers/net/ethernet/stmicro/stmmac/norm_desc.c +1 -1

index efb818ebd55e..f817f8f36569 100644
@@ -191,7 +191,7 @@ static void ndesc_release_tx_desc(struct dma_desc *p, int mode)
 
  static void ndesc_prepare_tx_desc(struct dma_desc *p, int is_fs, int len,
  				  bool csum_flag, int mode, bool tx_own,
-				  bool ls)
+				  bool ls, unsigned int tot_pkt_len)
  {
  	unsigned int tdes1 = le32_to_cpu(p->des1);

drivers/net/ethernet/stmicro/stmmac/ring_mode.c +6 -3

index 31213e64513d..28e4b5d50ce6 100644
@@ -59,7 +59,8 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
 
  		desc->des3 = cpu_to_le32(des2 + BUF_SIZE_4KiB);
  		priv->hw->desc->prepare_tx_desc(desc, 1, bmax, csum,
-						STMMAC_RING_MODE, 0, false);
+						STMMAC_RING_MODE, 0,
+						false, skb->len);
  		tx_q->tx_skbuff[entry] = NULL;
  		entry = STMMAC_GET_ENTRY(entry, DMA_TX_SIZE);

@@ -79,7 +80,8 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
 
  		desc->des3 = cpu_to_le32(des2 + BUF_SIZE_4KiB);
  		priv->hw->desc->prepare_tx_desc(desc, 0, len, csum,
-						STMMAC_RING_MODE, 1, true);
+						STMMAC_RING_MODE, 1,
+						true, skb->len);
  	} else {
  		des2 = dma_map_single(priv->device, skb->data,
  				      nopaged_len, DMA_TO_DEVICE);

@@ -91,7 +93,8 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
  		tx_q->tx_skbuff_dma[entry].is_jumbo = true;
  		desc->des3 = cpu_to_le32(des2 + BUF_SIZE_4KiB);
  		priv->hw->desc->prepare_tx_desc(desc, 1, nopaged_len, csum,
-						STMMAC_RING_MODE, 0, true);
+						STMMAC_RING_MODE, 0,
+						true, skb->len);
  	}
 
  	tx_q->cur_tx = entry;

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +3 -2

index a89f76b27ea2..5c1ebc70bebd 100644
@@ -3033,7 +3033,8 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
 
  		/* Prepare the descriptor and set the own bit too */
  		priv->hw->desc->prepare_tx_desc(desc, 0, len, csum_insertion,
-						priv->mode, 1, last_segment);
+						priv->mode, 1, last_segment,
+						skb->len);
  	}
 
  	entry = STMMAC_GET_ENTRY(entry, DMA_TX_SIZE);

@@ -3116,7 +3117,7 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
  		/* Prepare the first descriptor setting the OWN bit too */
  		priv->hw->desc->prepare_tx_desc(first, 1, nopaged_len,
  						csum_insertion, priv->mode, 1,
-						last_segment);
+						last_segment, skb->len);
 
  		/* The own bit must be the latest setting done when prepare the
  		 * descriptor and then barrier is needed to make sure that

David Miller

maintainer Applied

2017-04-13 16:40 UTC #6 lore raw

In reply to Niklas Cassel

From: Niklas Cassel <[email protected]>
Date: Mon, 10 Apr 2017 20:33:29 +0200

Quoted 10 lines

From: Niklas Cassel <[email protected]>
Field FL/TPL in register TDES3 is not correctly set on GMAC4. TX appears to be functional on GMAC 4.10a even if this field is not set, however, to avoid relying on undefined behavior, set the length in TDES3.
The field has a different meaning depending on if the TSE bit in TDES3 is set or not (TSO). However, regardless of the TSE bit, the field is not optional. The field is already set correctly when the TSE bit is set.

Quoted 7 lines

Since there is no limit for the number of descriptors that can be used for a single packet, the field should be set to the sum of the buffers contained in:
[<desc with First Descriptor bit set> ... <desc n> ...
<desc with Last Descriptor bit set>], which should be equal to skb->len.
Signed-off-by Niklas Cassel <[email protected]>

Applied, thanks.