linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* RE: [PATCH net-next v5] cadence: Add LSO support.
@ 2016-11-14  9:32 Rafal Ozieblo
  2016-11-14 14:35 ` Eric Dumazet
  2016-11-14 17:30 ` David Miller
  0 siblings, 2 replies; 9+ messages in thread
From: Rafal Ozieblo @ 2016-11-14  9:32 UTC (permalink / raw)
  To: David Miller; +Cc: nicolas.ferre, netdev, linux-kernel

From: David Miller [mailto:davem@davemloft.net] 
Sent: 10 listopada 2016 18:01
To: Rafal Ozieblo
Cc: nicolas.ferre@atmel.com; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next v5]] cadence: Add LSO support.

From: Rafal Ozieblo <rafalo@cadence.com>
Date: Wed, 9 Nov 2016 13:41:02 +0000

> First, please remove the spurious closing bracket in your Subject line in future submittions.
>
>> +	if (is_udp) /* is_udp is only set when (is_lso) is checked */
>> +		/* zero UDP checksum, not calculated by h/w for UFO */
>> +		udp_hdr(skb)->check = 0;
>
> This is really not ok.
> 
> If UFO is in use it should not silently disable UDP checksums.
> 
> If you cannot support UFO with proper checksumming, then you cannot enable support for that feature.

According Cadence Gigabit Ethernet MAC documentation:

"Hardware will not calculate the UDP checksum or modify the UDP checksum field. Therefore software
must set a value of zero in the checksum field in the UDP header (in the first payload buffer)
to indicate to the receiver that the UDP datagram does not include a checksum."

It is hardware requirement.

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

* Re: [PATCH net-next v5] cadence: Add LSO support.
  2016-11-14  9:32 [PATCH net-next v5] cadence: Add LSO support Rafal Ozieblo
@ 2016-11-14 14:35 ` Eric Dumazet
  2016-11-14 17:30 ` David Miller
  1 sibling, 0 replies; 9+ messages in thread
From: Eric Dumazet @ 2016-11-14 14:35 UTC (permalink / raw)
  To: Rafal Ozieblo; +Cc: David Miller, nicolas.ferre, netdev, linux-kernel

On Mon, 2016-11-14 at 09:32 +0000, Rafal Ozieblo wrote:
> From: David Miller [mailto:davem@davemloft.net] 
> Sent: 10 listopada 2016 18:01
> To: Rafal Ozieblo
> Cc: nicolas.ferre@atmel.com; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH net-next v5]] cadence: Add LSO support.
> 
> From: Rafal Ozieblo <rafalo@cadence.com>
> Date: Wed, 9 Nov 2016 13:41:02 +0000
> 
> > First, please remove the spurious closing bracket in your Subject line in future submittions.
> >
> >> +	if (is_udp) /* is_udp is only set when (is_lso) is checked */
> >> +		/* zero UDP checksum, not calculated by h/w for UFO */
> >> +		udp_hdr(skb)->check = 0;
> >
> > This is really not ok.
> > 
> > If UFO is in use it should not silently disable UDP checksums.
> > 
> > If you cannot support UFO with proper checksumming, then you cannot enable support for that feature.
> 
> According Cadence Gigabit Ethernet MAC documentation:
> 
> "Hardware will not calculate the UDP checksum or modify the UDP checksum field. Therefore software
> must set a value of zero in the checksum field in the UDP header (in the first payload buffer)
> to indicate to the receiver that the UDP datagram does not include a checksum."
> 
> It is hardware requirement.

Then do not claim NETIF_F_UFO suport in the driver, if hardware is
absolutely not capable of handling this.

Linux stack will provide proper udp checksum.

Almost no driver sets NETIF_F_UFO.

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

* Re: [PATCH net-next v5] cadence: Add LSO support.
  2016-11-14  9:32 [PATCH net-next v5] cadence: Add LSO support Rafal Ozieblo
  2016-11-14 14:35 ` Eric Dumazet
@ 2016-11-14 17:30 ` David Miller
  2016-11-15  7:07   ` Rafal Ozieblo
  1 sibling, 1 reply; 9+ messages in thread
From: David Miller @ 2016-11-14 17:30 UTC (permalink / raw)
  To: rafalo; +Cc: nicolas.ferre, netdev, linux-kernel

From: Rafal Ozieblo <rafalo@cadence.com>
Date: Mon, 14 Nov 2016 09:32:34 +0000

>> If UFO is in use it should not silently disable UDP checksums.
>> 
>> If you cannot support UFO with proper checksumming, then you cannot enable support for that feature.
> 
> According Cadence Gigabit Ethernet MAC documentation:
> 
> "Hardware will not calculate the UDP checksum or modify the UDP checksum field. Therefore software
> must set a value of zero in the checksum field in the UDP header (in the first payload buffer)
> to indicate to the receiver that the UDP datagram does not include a checksum."
> 
> It is hardware requirement.

I do not doubt that it is a hardware restriction.

But I am saying that you cannot enable this feature under Linux if
this is how it operates on your hardware.

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

* RE: [PATCH net-next v5] cadence: Add LSO support.
  2016-11-14 17:30 ` David Miller
@ 2016-11-15  7:07   ` Rafal Ozieblo
  2016-11-15 13:11     ` Eric Dumazet
  2016-11-15 15:15     ` David Miller
  0 siblings, 2 replies; 9+ messages in thread
From: Rafal Ozieblo @ 2016-11-15  7:07 UTC (permalink / raw)
  To: David Miller; +Cc: nicolas.ferre, netdev, linux-kernel

> > > If UFO is in use it should not silently disable UDP checksums.
> > > 
> > > If you cannot support UFO with proper checksumming, then you cannot enable support for that feature.
> > 
> > According Cadence Gigabit Ethernet MAC documentation:
> > 
> > "Hardware will not calculate the UDP checksum or modify the UDP 
> > checksum field. Therefore software must set a value of zero in the 
> > checksum field in the UDP header (in the first payload buffer) to indicate to the receiver that the UDP datagram does not include a checksum."
> > 
> > It is hardware requirement.
>
> I do not doubt that it is a hardware restriction.
>
> But I am saying that you cannot enable this feature under Linux if this is how it operates on your hardware.

Would it be good to enable UFO conditionally with some internal define? Ex.:

+#ifdef MACB_ENABLE_UFO
+#define MACB_NETIF_LSO         (NETIF_F_TSO | NETIF_F_UFO)
+#else
+#define MACB_NETIF_LSO         (NETIF_F_TSO)
+#endif

I could add precise comment here that ufo is possible only without checksum.

Or maybe I could enable it from module_params or device-tree (like: drivers/net/ethernet/neterion/s2io.c).

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

* Re: [PATCH net-next v5] cadence: Add LSO support.
  2016-11-15  7:07   ` Rafal Ozieblo
@ 2016-11-15 13:11     ` Eric Dumazet
  2016-11-15 14:20       ` Rafal Ozieblo
  2016-11-15 15:15     ` David Miller
  1 sibling, 1 reply; 9+ messages in thread
From: Eric Dumazet @ 2016-11-15 13:11 UTC (permalink / raw)
  To: Rafal Ozieblo; +Cc: David Miller, nicolas.ferre, netdev, linux-kernel

On Tue, 2016-11-15 at 07:07 +0000, Rafal Ozieblo wrote:
> > > > If UFO is in use it should not silently disable UDP checksums.
> > > > 
> > > > If you cannot support UFO with proper checksumming, then you cannot enable support for that feature.
> > > 
> > > According Cadence Gigabit Ethernet MAC documentation:
> > > 
> > > "Hardware will not calculate the UDP checksum or modify the UDP 
> > > checksum field. Therefore software must set a value of zero in the 
> > > checksum field in the UDP header (in the first payload buffer) to indicate to the receiver that the UDP datagram does not include a checksum."
> > > 
> > > It is hardware requirement.
> >
> > I do not doubt that it is a hardware restriction.
> >
> > But I am saying that you cannot enable this feature under Linux if this is how it operates on your hardware.
> 
> Would it be good to enable UFO conditionally with some internal define? Ex.:
> 
> +#ifdef MACB_ENABLE_UFO
> +#define MACB_NETIF_LSO         (NETIF_F_TSO | NETIF_F_UFO)
> +#else
> +#define MACB_NETIF_LSO         (NETIF_F_TSO)
> +#endif
> 
> I could add precise comment here that ufo is possible only without checksum.
> 
> Or maybe I could enable it from module_params or device-tree (like: drivers/net/ethernet/neterion/s2io.c).

No you can not do that.

1) That would violate UDP specs.
2) Module params are no longer accepted.
3) Comments in a driver source code would only help the driver
maintainer, not users to make their mind.

Only way would be to propagate the intent of the sender.

Only the sender application can decide to generate UDP checksums or not.

Your driver ndo_features_check() could then force software segmentation
fallback if the user did not asked to disable UDP checksums, and packet
is UFO.

(look for UDP_NO_CHECK6_TX, and SO_NO_CHECK )

Problem is complex, because the skb has no marker, only the socket has.

And socket state could change between packets, and packets can stay in
an intermediate qdisc before hitting device driver. So looking at
skb->sk from your ndo_features_check() would be racy.

What use case would you have precisely ?

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

* RE: [PATCH net-next v5] cadence: Add LSO support.
  2016-11-15 13:11     ` Eric Dumazet
@ 2016-11-15 14:20       ` Rafal Ozieblo
  0 siblings, 0 replies; 9+ messages in thread
From: Rafal Ozieblo @ 2016-11-15 14:20 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, nicolas.ferre, netdev, linux-kernel

> From: Eric Dumazet [mailto:eric.dumazet@gmail.com] 
> Sent: 15 listopada 2016 14:12
> To: Rafal Ozieblo
> Cc: David Miller; nicolas.ferre@atmel.com; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH net-next v5] cadence: Add LSO support.
>
> On Tue, 2016-11-15 at 07:07 +0000, Rafal Ozieblo wrote:
> > > > > If UFO is in use it should not silently disable UDP checksums.
> > > > > 
> > > > > If you cannot support UFO with proper checksumming, then you cannot enable support for that feature.
> > > > 
> > > > According Cadence Gigabit Ethernet MAC documentation:
> > > > 
> > > > "Hardware will not calculate the UDP checksum or modify the UDP 
> > > > checksum field. Therefore software must set a value of zero in the 
> > > > checksum field in the UDP header (in the first payload buffer) to indicate to the receiver that the UDP datagram does not include a checksum."
> > > > 
> > > > It is hardware requirement.
> > >
> > > I do not doubt that it is a hardware restriction.
> > >
> > > But I am saying that you cannot enable this feature under Linux if this is how it operates on your hardware.
> > 
> > Would it be good to enable UFO conditionally with some internal define? Ex.:
> > 
> > +#ifdef MACB_ENABLE_UFO
> > +#define MACB_NETIF_LSO         (NETIF_F_TSO | NETIF_F_UFO)
> > +#else
> > +#define MACB_NETIF_LSO         (NETIF_F_TSO)
> > +#endif
> > 
> > I could add precise comment here that ufo is possible only without checksum.
> > 
> > Or maybe I could enable it from module_params or device-tree (like: drivers/net/ethernet/neterion/s2io.c).
>
> No you can not do that.
>
> 1) That would violate UDP specs.
> 2) Module params are no longer accepted.
> 3) Comments in a driver source code would only help the driver maintainer, not users to make their mind.
>
> Only way would be to propagate the intent of the sender.
>
> Only the sender application can decide to generate UDP checksums or not.
>
> Your driver ndo_features_check() could then force software segmentation fallback if the user did not asked to disable UDP checksums, and packet is UFO.
>
> (look for UDP_NO_CHECK6_TX, and SO_NO_CHECK )
>
> Problem is complex, because the skb has no marker, only the socket has.
>
> And socket state could change between packets, and packets can stay in an intermediate qdisc before hitting device driver. So looking at
> skb->sk from your ndo_features_check() would be racy.
>
> What use case would you have precisely ?

I have talked with hardware team who designed and created gem IP.  The conclusion is that there is no need to zeroed checksum for UFO. 
I have tested UFO without zeroing UDP checksum on cadence gem. It works fine.
I'll deeply investigate and test UFO again. I'll send version 6 of LSO PATCH either with UFO without zeroing or without UFO at all.

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

* Re: [PATCH net-next v5] cadence: Add LSO support.
  2016-11-15  7:07   ` Rafal Ozieblo
  2016-11-15 13:11     ` Eric Dumazet
@ 2016-11-15 15:15     ` David Miller
  1 sibling, 0 replies; 9+ messages in thread
From: David Miller @ 2016-11-15 15:15 UTC (permalink / raw)
  To: rafalo; +Cc: nicolas.ferre, netdev, linux-kernel

From: Rafal Ozieblo <rafalo@cadence.com>
Date: Tue, 15 Nov 2016 07:07:14 +0000

> Would it be good to enable UFO conditionally with some internal
> define? Ex.:

Absolutely not.

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

* Re: [PATCH net-next v5]] cadence: Add LSO support.
  2016-11-09 13:41 ` [PATCH net-next v5]] " Rafal Ozieblo
@ 2016-11-10 17:01   ` David Miller
  0 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2016-11-10 17:01 UTC (permalink / raw)
  To: rafalo; +Cc: nicolas.ferre, netdev, linux-kernel

From: Rafal Ozieblo <rafalo@cadence.com>
Date: Wed, 9 Nov 2016 13:41:02 +0000

First, please remove the spurious closing bracket in your Subject line
in future submittions.

> +	if (is_udp) /* is_udp is only set when (is_lso) is checked */
> +		/* zero UDP checksum, not calculated by h/w for UFO */
> +		udp_hdr(skb)->check = 0;

This is really not ok.

If UFO is in use it should not silently disable UDP checksums.

If you cannot support UFO with proper checksumming, then you cannot
enable support for that feature.

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

* [PATCH net-next v5]] cadence: Add LSO support.
  2016-11-08 13:41 [PATCH net-next v4] " Rafal Ozieblo
@ 2016-11-09 13:41 ` Rafal Ozieblo
  2016-11-10 17:01   ` David Miller
  0 siblings, 1 reply; 9+ messages in thread
From: Rafal Ozieblo @ 2016-11-09 13:41 UTC (permalink / raw)
  To: nicolas.ferre, netdev, linux-kernel; +Cc: Rafal Ozieblo

New Cadence GEM hardware support Large Segment Offload (LSO):
TCP segmentation offload (TSO) as well as UDP fragmentation
offload (UFO). Support for those features was added to the driver.

Signed-off-by: Rafal Ozieblo <rafalo@cadence.com>
---
Changed in v2:
macb_lso_check_compatibility() changed to macb_features_check()
(with little modifications) and bind to .ndo_features_check.
(after Eric Dumazet suggestion)
---
Changed in v3:
Respin to net-next.
---
Changed in v4:
(struct iphdr*)skb_network_header(skb) changed to ip_hdr(skb)
---
Changed in v5:
Changes after Florian Fainelli comments
---
 drivers/net/ethernet/cadence/macb.c | 142 +++++++++++++++++++++++++++++++++---
 drivers/net/ethernet/cadence/macb.h |  14 ++++
 2 files changed, 144 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index e1847ce..dd38ef7 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -32,7 +32,9 @@
 #include <linux/of_gpio.h>
 #include <linux/of_mdio.h>
 #include <linux/of_net.h>
-
+#include <linux/ip.h>
+#include <linux/udp.h>
+#include <linux/tcp.h>
 #include "macb.h"
 
 #define MACB_RX_BUFFER_SIZE	128
@@ -60,10 +62,13 @@
 					| MACB_BIT(TXERR))
 #define MACB_TX_INT_FLAGS	(MACB_TX_ERR_FLAGS | MACB_BIT(TCOMP))
 
-#define MACB_MAX_TX_LEN		((unsigned int)((1 << MACB_TX_FRMLEN_SIZE) - 1))
-#define GEM_MAX_TX_LEN		((unsigned int)((1 << GEM_TX_FRMLEN_SIZE) - 1))
+/* Max length of transmit frame must be a multiple of 8 bytes */
+#define MACB_TX_LEN_ALIGN	8
+#define MACB_MAX_TX_LEN		((unsigned int)((1 << MACB_TX_FRMLEN_SIZE) - 1) & ~((unsigned int)(MACB_TX_LEN_ALIGN - 1)))
+#define GEM_MAX_TX_LEN		((unsigned int)((1 << GEM_TX_FRMLEN_SIZE) - 1) & ~((unsigned int)(MACB_TX_LEN_ALIGN - 1)))
 
 #define GEM_MTU_MIN_SIZE	ETH_MIN_MTU
+#define MACB_NETIF_LSO		(NETIF_F_TSO | NETIF_F_UFO)
 
 #define MACB_WOL_HAS_MAGIC_PACKET	(0x1 << 0)
 #define MACB_WOL_ENABLED		(0x1 << 1)
@@ -1223,7 +1228,8 @@ static void macb_poll_controller(struct net_device *dev)
 
 static unsigned int macb_tx_map(struct macb *bp,
 				struct macb_queue *queue,
-				struct sk_buff *skb)
+				struct sk_buff *skb,
+				unsigned int hdrlen)
 {
 	dma_addr_t mapping;
 	unsigned int len, entry, i, tx_head = queue->tx_head;
@@ -1231,14 +1237,27 @@ static unsigned int macb_tx_map(struct macb *bp,
 	struct macb_dma_desc *desc;
 	unsigned int offset, size, count = 0;
 	unsigned int f, nr_frags = skb_shinfo(skb)->nr_frags;
-	unsigned int eof = 1;
-	u32 ctrl;
+	unsigned int eof = 1, mss_mfs = 0;
+	u32 ctrl, lso_ctrl = 0, seq_ctrl = 0;
+
+	/* LSO */
+	if (skb_shinfo(skb)->gso_size != 0) {
+		if (ip_hdr(skb)->protocol == IPPROTO_UDP)
+			/* UDP - UFO */
+			lso_ctrl = MACB_LSO_UFO_ENABLE;
+		else
+			/* TCP - TSO */
+			lso_ctrl = MACB_LSO_TSO_ENABLE;
+	}
 
 	/* First, map non-paged data */
 	len = skb_headlen(skb);
+
+	/* first buffer length */
+	size = hdrlen;
+
 	offset = 0;
 	while (len) {
-		size = min(len, bp->max_tx_length);
 		entry = macb_tx_ring_wrap(bp, tx_head);
 		tx_skb = &queue->tx_skb[entry];
 
@@ -1258,6 +1277,8 @@ static unsigned int macb_tx_map(struct macb *bp,
 		offset += size;
 		count++;
 		tx_head++;
+
+		size = min(len, bp->max_tx_length);
 	}
 
 	/* Then, map paged data from fragments */
@@ -1311,6 +1332,21 @@ static unsigned int macb_tx_map(struct macb *bp,
 	desc = &queue->tx_ring[entry];
 	desc->ctrl = ctrl;
 
+	if (lso_ctrl) {
+		if (lso_ctrl == MACB_LSO_UFO_ENABLE)
+			/* include header and FCS in value given to h/w */
+			mss_mfs = skb_shinfo(skb)->gso_size +
+					skb_transport_offset(skb) +
+					ETH_FCS_LEN;
+		else /* TSO */ {
+			mss_mfs = skb_shinfo(skb)->gso_size;
+			/* TCP Sequence Number Source Select
+			 * can be set only for TSO
+			 */
+			seq_ctrl = 0;
+		}
+	}
+
 	do {
 		i--;
 		entry = macb_tx_ring_wrap(bp, i);
@@ -1325,6 +1361,16 @@ static unsigned int macb_tx_map(struct macb *bp,
 		if (unlikely(entry == (bp->tx_ring_size - 1)))
 			ctrl |= MACB_BIT(TX_WRAP);
 
+		/* First descriptor is header descriptor */
+		if (i == queue->tx_head) {
+			ctrl |= MACB_BF(TX_LSO, lso_ctrl);
+			ctrl |= MACB_BF(TX_TCP_SEQ_SRC, seq_ctrl);
+		} else
+			/* Only set MSS/MFS on payload descriptors
+			 * (second or later descriptor)
+			 */
+			ctrl |= MACB_BF(MSS_MFS, mss_mfs);
+
 		/* Set TX buffer descriptor */
 		macb_set_addr(desc, tx_skb->mapping);
 		/* desc->addr must be visible to hardware before clearing
@@ -1350,6 +1396,43 @@ static unsigned int macb_tx_map(struct macb *bp,
 	return 0;
 }
 
+static netdev_features_t macb_features_check(struct sk_buff *skb,
+					     struct net_device *dev,
+					     netdev_features_t features)
+{
+	unsigned int nr_frags, f;
+	unsigned int hdrlen;
+
+	/* Validate LSO compatibility */
+
+	/* there is only one buffer */
+	if (!skb_is_nonlinear(skb))
+		return features;
+
+	/* length of header */
+	hdrlen = skb_transport_offset(skb);
+	if (ip_hdr(skb)->protocol == IPPROTO_TCP)
+		hdrlen += tcp_hdrlen(skb);
+
+	/* For LSO:
+	 * When software supplies two or more payload buffers all payload buffers
+	 * apart from the last must be a multiple of 8 bytes in size.
+	 */
+	if (!IS_ALIGNED(skb_headlen(skb) - hdrlen, MACB_TX_LEN_ALIGN))
+		return features & ~MACB_NETIF_LSO;
+
+	nr_frags = skb_shinfo(skb)->nr_frags;
+	/* No need to check last fragment */
+	nr_frags--;
+	for (f = 0; f < nr_frags; f++) {
+		const skb_frag_t *frag = &skb_shinfo(skb)->frags[f];
+
+		if (!IS_ALIGNED(skb_frag_size(frag), MACB_TX_LEN_ALIGN))
+			return features & ~MACB_NETIF_LSO;
+	}
+	return features;
+}
+
 static inline int macb_clear_csum(struct sk_buff *skb)
 {
 	/* no change for packets without checksum offloading */
@@ -1374,7 +1457,28 @@ static int macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	struct macb *bp = netdev_priv(dev);
 	struct macb_queue *queue = &bp->queues[queue_index];
 	unsigned long flags;
-	unsigned int count, nr_frags, frag_size, f;
+	unsigned int desc_cnt, nr_frags, frag_size, f;
+	unsigned int hdrlen;
+	bool is_lso, is_udp = 0;
+
+	is_lso = (skb_shinfo(skb)->gso_size != 0);
+
+	if (is_lso) {
+		is_udp = !!(ip_hdr(skb)->protocol == IPPROTO_UDP);
+
+		/* length of headers */
+		if (is_udp)
+			/* only queue eth + ip headers separately for UDP */
+			hdrlen = skb_transport_offset(skb);
+		else
+			hdrlen = skb_transport_offset(skb) + tcp_hdrlen(skb);
+		if (skb_headlen(skb) < hdrlen) {
+			netdev_err(bp->dev, "Error - LSO headers fragmented!!!\n");
+			/* if this is required, would need to copy to single buffer */
+			return NETDEV_TX_BUSY;
+		}
+	} else
+		hdrlen = min(skb_headlen(skb), bp->max_tx_length);
 
 #if defined(DEBUG) && defined(VERBOSE_DEBUG)
 	netdev_vdbg(bp->dev,
@@ -1389,18 +1493,22 @@ static int macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	 * socket buffer: skb fragments of jumbo frames may need to be
 	 * split into many buffer descriptors.
 	 */
-	count = DIV_ROUND_UP(skb_headlen(skb), bp->max_tx_length);
+	if (is_lso && (skb_headlen(skb) > hdrlen))
+		/* extra header descriptor if also payload in first buffer */
+		desc_cnt = DIV_ROUND_UP((skb_headlen(skb) - hdrlen), bp->max_tx_length) + 1;
+	else
+		desc_cnt = DIV_ROUND_UP(skb_headlen(skb), bp->max_tx_length);
 	nr_frags = skb_shinfo(skb)->nr_frags;
 	for (f = 0; f < nr_frags; f++) {
 		frag_size = skb_frag_size(&skb_shinfo(skb)->frags[f]);
-		count += DIV_ROUND_UP(frag_size, bp->max_tx_length);
+		desc_cnt += DIV_ROUND_UP(frag_size, bp->max_tx_length);
 	}
 
 	spin_lock_irqsave(&bp->lock, flags);
 
 	/* This is a hard error, log it. */
 	if (CIRC_SPACE(queue->tx_head, queue->tx_tail,
-		       bp->tx_ring_size) < count) {
+		       bp->tx_ring_size) < desc_cnt) {
 		netif_stop_subqueue(dev, queue_index);
 		spin_unlock_irqrestore(&bp->lock, flags);
 		netdev_dbg(bp->dev, "tx_head = %u, tx_tail = %u\n",
@@ -1408,13 +1516,17 @@ static int macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
 		return NETDEV_TX_BUSY;
 	}
 
+	if (is_udp) /* is_udp is only set when (is_lso) is checked */
+		/* zero UDP checksum, not calculated by h/w for UFO */
+		udp_hdr(skb)->check = 0;
+
 	if (macb_clear_csum(skb)) {
 		dev_kfree_skb_any(skb);
 		goto unlock;
 	}
 
 	/* Map socket buffer for DMA transfer */
-	if (!macb_tx_map(bp, queue, skb)) {
+	if (!macb_tx_map(bp, queue, skb, hdrlen)) {
 		dev_kfree_skb_any(skb);
 		goto unlock;
 	}
@@ -2354,6 +2466,7 @@ static const struct net_device_ops macb_netdev_ops = {
 	.ndo_poll_controller	= macb_poll_controller,
 #endif
 	.ndo_set_features	= macb_set_features,
+	.ndo_features_check	= macb_features_check,
 };
 
 /* Configure peripheral capabilities according to device tree
@@ -2560,6 +2673,11 @@ static int macb_init(struct platform_device *pdev)
 
 	/* Set features */
 	dev->hw_features = NETIF_F_SG;
+
+	/* Check LSO capability */
+	if (GEM_BFEXT(PBUF_LSO, gem_readl(bp, DCFG6)))
+		dev->hw_features |= MACB_NETIF_LSO;
+
 	/* Checksum offload is only available on gem with packet buffer */
 	if (macb_is_gem(bp) && !(bp->caps & MACB_CAPS_FIFO_MODE))
 		dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_RXCSUM;
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 1216950..d67adad 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -382,6 +382,10 @@
 #define GEM_TX_PKT_BUFF_OFFSET			21
 #define GEM_TX_PKT_BUFF_SIZE			1
 
+/* Bitfields in DCFG6. */
+#define GEM_PBUF_LSO_OFFSET			27
+#define GEM_PBUF_LSO_SIZE			1
+
 /* Constants for CLK */
 #define MACB_CLK_DIV8				0
 #define MACB_CLK_DIV16				1
@@ -414,6 +418,10 @@
 #define MACB_CAPS_SG_DISABLED			0x40000000
 #define MACB_CAPS_MACB_IS_GEM			0x80000000
 
+/* LSO settings */
+#define MACB_LSO_UFO_ENABLE			0x01
+#define MACB_LSO_TSO_ENABLE			0x02
+
 /* Bit manipulation macros */
 #define MACB_BIT(name)					\
 	(1 << MACB_##name##_OFFSET)
@@ -545,6 +553,12 @@ struct macb_dma_desc {
 #define MACB_TX_LAST_SIZE			1
 #define MACB_TX_NOCRC_OFFSET			16
 #define MACB_TX_NOCRC_SIZE			1
+#define MACB_MSS_MFS_OFFSET			16
+#define MACB_MSS_MFS_SIZE			14
+#define MACB_TX_LSO_OFFSET			17
+#define MACB_TX_LSO_SIZE			2
+#define MACB_TX_TCP_SEQ_SRC_OFFSET		19
+#define MACB_TX_TCP_SEQ_SRC_SIZE		1
 #define MACB_TX_BUF_EXHAUSTED_OFFSET		27
 #define MACB_TX_BUF_EXHAUSTED_SIZE		1
 #define MACB_TX_UNDERRUN_OFFSET			28
-- 
2.4.5

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

end of thread, other threads:[~2016-11-15 15:15 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-14  9:32 [PATCH net-next v5] cadence: Add LSO support Rafal Ozieblo
2016-11-14 14:35 ` Eric Dumazet
2016-11-14 17:30 ` David Miller
2016-11-15  7:07   ` Rafal Ozieblo
2016-11-15 13:11     ` Eric Dumazet
2016-11-15 14:20       ` Rafal Ozieblo
2016-11-15 15:15     ` David Miller
  -- strict thread matches above, loose matches on Subject: below --
2016-11-08 13:41 [PATCH net-next v4] " Rafal Ozieblo
2016-11-09 13:41 ` [PATCH net-next v5]] " Rafal Ozieblo
2016-11-10 17:01   ` David Miller

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