All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] tulip: Fix for MTU problems with 802.1q tagged frames
@ 2009-03-05 10:21 Ivan Vecera
  2009-03-06 12:48 ` Ben McKeegan
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Ivan Vecera @ 2009-03-05 10:21 UTC (permalink / raw)
  To: netdev; +Cc: grundler, kyle, Ivan Vecera, Tomasz Lemiech

The original patch was submitted last year but wasn't discussed or applied
because of missing maintainer's CCs. I only fixed some formatting errors,
but as I saw tulip is very badly formatted and needs further work.

Original description:
This patch fixes MTU problem, which occurs when using 802.1q VLANs. We
should allow receiving frames of up to 1518 bytes in length, instead of
1514.

Based on patch written by Ben McKeegan for 2.4.x kernels. It is archived
at http://www.candelatech.com/~greear/vlan/howto.html#tulip
I've adjusted a few things to make it apply on 2.6.x kernels.

Tested on D-Link DFE-570TX quad-fastethernet card.

Signed-off-by: Tomasz Lemiech <szpajder@staszic.waw.pl>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/net/tulip/interrupt.c |   84 +++++++++++++++++++++++++++--------------
 drivers/net/tulip/tulip.h     |   32 +++++++++++++++-
 2 files changed, 86 insertions(+), 30 deletions(-)

diff --git a/drivers/net/tulip/interrupt.c b/drivers/net/tulip/interrupt.c
index 6c3428a..c21193b 100644
--- a/drivers/net/tulip/interrupt.c
+++ b/drivers/net/tulip/interrupt.c
@@ -140,6 +140,7 @@ int tulip_poll(struct napi_struct *napi, int budget)
                /* If we own the next entry, it is a new packet. Send it up. */
                while ( ! (tp->rx_ring[entry].status & cpu_to_le32(DescOwned))) {
                        s32 status = le32_to_cpu(tp->rx_ring[entry].status);
+		       short pkt_len;
 
                        if (tp->dirty_rx + RX_RING_SIZE == tp->cur_rx)
                                break;
@@ -151,8 +152,28 @@ int tulip_poll(struct napi_struct *napi, int budget)
 		       if (++work_done >= budget)
                                goto not_done;
 
-                       if ((status & 0x38008300) != 0x0300) {
-                               if ((status & 0x38000300) != 0x0300) {
+		       /*
+			* Omit the four octet CRC from the length.
+			* (May not be considered valid until we have
+			* checked status for RxLengthOver2047 bits)
+			*/
+		       pkt_len = ((status >> 16) & 0x7ff) - 4;
+
+		       /*
+			* Maximum pkt_len is 1518 (1514 + vlan header)
+			* Anything higher than this is always invalid
+			* regardless of RxLengthOver2047 bits
+			*/
+
+		       if ((status & (RxLengthOver2047 |
+				      RxDescCRCError |
+				      RxDescCollisionSeen |
+				      RxDescRunt |
+				      RxDescDescErr |
+				      RxWholePkt)) != RxWholePkt
+			   || pkt_len > 1518) {
+			       if ((status & (RxLengthOver2047 |
+					      RxWholePkt)) != RxWholePkt) {
                                 /* Ingore earlier buffers. */
                                        if ((status & 0xffff) != 0x7fff) {
                                                if (tulip_debug > 1)
@@ -161,30 +182,23 @@ int tulip_poll(struct napi_struct *napi, int budget)
                                                               dev->name, status);
                                                tp->stats.rx_length_errors++;
                                        }
-                               } else if (status & RxDescFatalErr) {
+			       } else {
                                 /* There was a fatal error. */
                                        if (tulip_debug > 2)
                                                printk(KERN_DEBUG "%s: Receive error, Rx status %8.8x.\n",
                                                       dev->name, status);
                                        tp->stats.rx_errors++; /* end of a packet.*/
-                                       if (status & 0x0890) tp->stats.rx_length_errors++;
+				       if (pkt_len > 1518 ||
+					   (status & RxDescRunt))
+					       tp->stats.rx_length_errors++;
+
                                        if (status & 0x0004) tp->stats.rx_frame_errors++;
                                        if (status & 0x0002) tp->stats.rx_crc_errors++;
                                        if (status & 0x0001) tp->stats.rx_fifo_errors++;
                                }
                        } else {
-                               /* Omit the four octet CRC from the length. */
-                               short pkt_len = ((status >> 16) & 0x7ff) - 4;
                                struct sk_buff *skb;
 
-#ifndef final_version
-                               if (pkt_len > 1518) {
-                                       printk(KERN_WARNING "%s: Bogus packet size of %d (%#x).\n",
-                                              dev->name, pkt_len, pkt_len);
-                                       pkt_len = 1518;
-                                       tp->stats.rx_length_errors++;
-                               }
-#endif
                                /* Check if the packet is long enough to accept without copying
                                   to a minimally-sized skbuff. */
                                if (pkt_len < tulip_rx_copybreak
@@ -356,14 +370,35 @@ static int tulip_rx(struct net_device *dev)
 	/* If we own the next entry, it is a new packet. Send it up. */
 	while ( ! (tp->rx_ring[entry].status & cpu_to_le32(DescOwned))) {
 		s32 status = le32_to_cpu(tp->rx_ring[entry].status);
+		short pkt_len;
 
 		if (tulip_debug > 5)
 			printk(KERN_DEBUG "%s: In tulip_rx(), entry %d %8.8x.\n",
 				   dev->name, entry, status);
 		if (--rx_work_limit < 0)
 			break;
-		if ((status & 0x38008300) != 0x0300) {
-			if ((status & 0x38000300) != 0x0300) {
+
+		/*
+		  Omit the four octet CRC from the length.
+		  (May not be considered valid until we have
+		  checked status for RxLengthOver2047 bits)
+		*/
+		pkt_len = ((status >> 16) & 0x7ff) - 4;
+		/*
+		  Maximum pkt_len is 1518 (1514 + vlan header)
+		  Anything higher than this is always invalid
+		  regardless of RxLengthOver2047 bits
+		*/
+
+		if ((status & (RxLengthOver2047 |
+			       RxDescCRCError |
+			       RxDescCollisionSeen |
+			       RxDescRunt |
+			       RxDescDescErr |
+			       RxWholePkt))        != RxWholePkt
+		     || pkt_len > 1518) {
+			if ((status & (RxLengthOver2047 |
+			     RxWholePkt))         != RxWholePkt) {
 				/* Ingore earlier buffers. */
 				if ((status & 0xffff) != 0x7fff) {
 					if (tulip_debug > 1)
@@ -372,31 +407,22 @@ static int tulip_rx(struct net_device *dev)
 							   dev->name, status);
 					tp->stats.rx_length_errors++;
 				}
-			} else if (status & RxDescFatalErr) {
+			} else {
 				/* There was a fatal error. */
 				if (tulip_debug > 2)
 					printk(KERN_DEBUG "%s: Receive error, Rx status %8.8x.\n",
 						   dev->name, status);
 				tp->stats.rx_errors++; /* end of a packet.*/
-				if (status & 0x0890) tp->stats.rx_length_errors++;
+				if (pkt_len > 1518 ||
+				    (status & RxDescRunt))
+					tp->stats.rx_length_errors++;
 				if (status & 0x0004) tp->stats.rx_frame_errors++;
 				if (status & 0x0002) tp->stats.rx_crc_errors++;
 				if (status & 0x0001) tp->stats.rx_fifo_errors++;
 			}
 		} else {
-			/* Omit the four octet CRC from the length. */
-			short pkt_len = ((status >> 16) & 0x7ff) - 4;
 			struct sk_buff *skb;
 
-#ifndef final_version
-			if (pkt_len > 1518) {
-				printk(KERN_WARNING "%s: Bogus packet size of %d (%#x).\n",
-					   dev->name, pkt_len, pkt_len);
-				pkt_len = 1518;
-				tp->stats.rx_length_errors++;
-			}
-#endif
-
 			/* Check if the packet is long enough to accept without copying
 			   to a minimally-sized skbuff. */
 			if (pkt_len < tulip_rx_copybreak
diff --git a/drivers/net/tulip/tulip.h b/drivers/net/tulip/tulip.h
index 19abbc3..0afa2d4 100644
--- a/drivers/net/tulip/tulip.h
+++ b/drivers/net/tulip/tulip.h
@@ -201,8 +201,38 @@ enum desc_status_bits {
 	DescStartPkt = 0x20000000,
 	DescEndRing  = 0x02000000,
 	DescUseLink  = 0x01000000,
-	RxDescFatalErr = 0x008000,
+
+	/*
+	 * Error summary flag is logical or of 'CRC Error', 'Collision Seen',
+	 * 'Frame Too Long', 'Runt' and 'Descriptor Error' flags generated
+	 * within tulip chip.
+	 */
+	RxDescErrorSummary = 0x8000,
+	RxDescCRCError = 0x0002,
+	RxDescCollisionSeen = 0x0040,
+
+	/*
+	 * 'Frame Too Long' flag is set if packet length including CRC exceeds
+	 * 1518.  However, a full sized VLAN tagged frame is 1522 bytes
+	 * including CRC.
+	 *
+	 * The tulip chip does not block oversized frames, and if this flag is
+	 * set on a receive descriptor it does not indicate the frame has been
+	 * truncated.  The receive descriptor also includes the actual length.
+	 * Therefore we can safety ignore this flag and check the length
+	 * ourselves.
+	 */
+	RxDescFrameTooLong = 0x0080,
+	RxDescRunt = 0x0800,
+	RxDescDescErr = 0x4000,
 	RxWholePkt   = 0x00000300,
+	/*
+	 * Top three bits of 14 bit frame length (status bits 27-29) should
+	 * never be set as that would make frame over 2047 bytes. The Receive
+	 * Watchdog flag (bit 4) may indicate the length is over 2048 and the
+	 * length field is invalid.
+	 */
+	RxLengthOver2047 = 0x38000010
 };
 
 
-- 
1.6.0.4


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

* Re: [PATCH] tulip: Fix for MTU problems with 802.1q tagged frames
  2009-03-05 10:21 [PATCH] tulip: Fix for MTU problems with 802.1q tagged frames Ivan Vecera
@ 2009-03-06 12:48 ` Ben McKeegan
  2009-03-06 18:13 ` Grant Grundler
  2009-03-08 19:53 ` Kyle McMartin
  2 siblings, 0 replies; 6+ messages in thread
From: Ben McKeegan @ 2009-03-06 12:48 UTC (permalink / raw)
  To: Ivan Vecera; +Cc: netdev, grundler, kyle, Tomasz Lemiech

Ivan Vecera wrote:
> The original patch was submitted last year but wasn't discussed or applied
> because of missing maintainer's CCs. I only fixed some formatting errors,
> but as I saw tulip is very badly formatted and needs further work.
> 
> Original description:
> This patch fixes MTU problem, which occurs when using 802.1q VLANs. We
> should allow receiving frames of up to 1518 bytes in length, instead of
> 1514.
> 
> Based on patch written by Ben McKeegan for 2.4.x kernels. It is archived
> at http://www.candelatech.com/~greear/vlan/howto.html#tulip
> I've adjusted a few things to make it apply on 2.6.x kernels.
> 
> Tested on D-Link DFE-570TX quad-fastethernet card.
> 
> Signed-off-by: Tomasz Lemiech <szpajder@staszic.waw.pl>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>

Looks good.  FWIW:

Signed-off-by: Ben McKeegan <ben@netservers.co.uk>




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

* Re: [PATCH] tulip: Fix for MTU problems with 802.1q tagged frames
  2009-03-05 10:21 [PATCH] tulip: Fix for MTU problems with 802.1q tagged frames Ivan Vecera
  2009-03-06 12:48 ` Ben McKeegan
@ 2009-03-06 18:13 ` Grant Grundler
  2009-03-13 22:43   ` David Miller
  2009-03-08 19:53 ` Kyle McMartin
  2 siblings, 1 reply; 6+ messages in thread
From: Grant Grundler @ 2009-03-06 18:13 UTC (permalink / raw)
  To: Ivan Vecera; +Cc: netdev, grundler, kyle, Tomasz Lemiech

On Thu, Mar 05, 2009 at 11:21:34AM +0100, Ivan Vecera wrote:
> The original patch was submitted last year but wasn't discussed or applied
> because of missing maintainer's CCs. I only fixed some formatting errors,
> but as I saw tulip is very badly formatted and needs further work.
> 
> Original description:
> This patch fixes MTU problem, which occurs when using 802.1q VLANs. We
> should allow receiving frames of up to 1518 bytes in length, instead of
> 1514.
> 
> Based on patch written by Ben McKeegan for 2.4.x kernels. It is archived
> at http://www.candelatech.com/~greear/vlan/howto.html#tulip
> I've adjusted a few things to make it apply on 2.6.x kernels.
> 
> Tested on D-Link DFE-570TX quad-fastethernet card.
> 
> Signed-off-by: Tomasz Lemiech <szpajder@staszic.waw.pl>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>

Acked-by: Grant Grundler <grundler@parisc-linux.org>

I reviewed this earlier in the week (offlist email) and it looks
fine to me too.

And while I agree the white space isn't uhm, "optimal", I didn't
feel it was worthwhile to generate a massive white space diff.

thanks,
grant

> ---
>  drivers/net/tulip/interrupt.c |   84 +++++++++++++++++++++++++++--------------
>  drivers/net/tulip/tulip.h     |   32 +++++++++++++++-
>  2 files changed, 86 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/net/tulip/interrupt.c b/drivers/net/tulip/interrupt.c
> index 6c3428a..c21193b 100644
> --- a/drivers/net/tulip/interrupt.c
> +++ b/drivers/net/tulip/interrupt.c
> @@ -140,6 +140,7 @@ int tulip_poll(struct napi_struct *napi, int budget)
>                 /* If we own the next entry, it is a new packet. Send it up. */
>                 while ( ! (tp->rx_ring[entry].status & cpu_to_le32(DescOwned))) {
>                         s32 status = le32_to_cpu(tp->rx_ring[entry].status);
> +		       short pkt_len;
>  
>                         if (tp->dirty_rx + RX_RING_SIZE == tp->cur_rx)
>                                 break;
> @@ -151,8 +152,28 @@ int tulip_poll(struct napi_struct *napi, int budget)
>  		       if (++work_done >= budget)
>                                 goto not_done;
>  
> -                       if ((status & 0x38008300) != 0x0300) {
> -                               if ((status & 0x38000300) != 0x0300) {
> +		       /*
> +			* Omit the four octet CRC from the length.
> +			* (May not be considered valid until we have
> +			* checked status for RxLengthOver2047 bits)
> +			*/
> +		       pkt_len = ((status >> 16) & 0x7ff) - 4;
> +
> +		       /*
> +			* Maximum pkt_len is 1518 (1514 + vlan header)
> +			* Anything higher than this is always invalid
> +			* regardless of RxLengthOver2047 bits
> +			*/
> +
> +		       if ((status & (RxLengthOver2047 |
> +				      RxDescCRCError |
> +				      RxDescCollisionSeen |
> +				      RxDescRunt |
> +				      RxDescDescErr |
> +				      RxWholePkt)) != RxWholePkt
> +			   || pkt_len > 1518) {
> +			       if ((status & (RxLengthOver2047 |
> +					      RxWholePkt)) != RxWholePkt) {
>                                  /* Ingore earlier buffers. */
>                                         if ((status & 0xffff) != 0x7fff) {
>                                                 if (tulip_debug > 1)
> @@ -161,30 +182,23 @@ int tulip_poll(struct napi_struct *napi, int budget)
>                                                                dev->name, status);
>                                                 tp->stats.rx_length_errors++;
>                                         }
> -                               } else if (status & RxDescFatalErr) {
> +			       } else {
>                                  /* There was a fatal error. */
>                                         if (tulip_debug > 2)
>                                                 printk(KERN_DEBUG "%s: Receive error, Rx status %8.8x.\n",
>                                                        dev->name, status);
>                                         tp->stats.rx_errors++; /* end of a packet.*/
> -                                       if (status & 0x0890) tp->stats.rx_length_errors++;
> +				       if (pkt_len > 1518 ||
> +					   (status & RxDescRunt))
> +					       tp->stats.rx_length_errors++;
> +
>                                         if (status & 0x0004) tp->stats.rx_frame_errors++;
>                                         if (status & 0x0002) tp->stats.rx_crc_errors++;
>                                         if (status & 0x0001) tp->stats.rx_fifo_errors++;
>                                 }
>                         } else {
> -                               /* Omit the four octet CRC from the length. */
> -                               short pkt_len = ((status >> 16) & 0x7ff) - 4;
>                                 struct sk_buff *skb;
>  
> -#ifndef final_version
> -                               if (pkt_len > 1518) {
> -                                       printk(KERN_WARNING "%s: Bogus packet size of %d (%#x).\n",
> -                                              dev->name, pkt_len, pkt_len);
> -                                       pkt_len = 1518;
> -                                       tp->stats.rx_length_errors++;
> -                               }
> -#endif
>                                 /* Check if the packet is long enough to accept without copying
>                                    to a minimally-sized skbuff. */
>                                 if (pkt_len < tulip_rx_copybreak
> @@ -356,14 +370,35 @@ static int tulip_rx(struct net_device *dev)
>  	/* If we own the next entry, it is a new packet. Send it up. */
>  	while ( ! (tp->rx_ring[entry].status & cpu_to_le32(DescOwned))) {
>  		s32 status = le32_to_cpu(tp->rx_ring[entry].status);
> +		short pkt_len;
>  
>  		if (tulip_debug > 5)
>  			printk(KERN_DEBUG "%s: In tulip_rx(), entry %d %8.8x.\n",
>  				   dev->name, entry, status);
>  		if (--rx_work_limit < 0)
>  			break;
> -		if ((status & 0x38008300) != 0x0300) {
> -			if ((status & 0x38000300) != 0x0300) {
> +
> +		/*
> +		  Omit the four octet CRC from the length.
> +		  (May not be considered valid until we have
> +		  checked status for RxLengthOver2047 bits)
> +		*/
> +		pkt_len = ((status >> 16) & 0x7ff) - 4;
> +		/*
> +		  Maximum pkt_len is 1518 (1514 + vlan header)
> +		  Anything higher than this is always invalid
> +		  regardless of RxLengthOver2047 bits
> +		*/
> +
> +		if ((status & (RxLengthOver2047 |
> +			       RxDescCRCError |
> +			       RxDescCollisionSeen |
> +			       RxDescRunt |
> +			       RxDescDescErr |
> +			       RxWholePkt))        != RxWholePkt
> +		     || pkt_len > 1518) {
> +			if ((status & (RxLengthOver2047 |
> +			     RxWholePkt))         != RxWholePkt) {
>  				/* Ingore earlier buffers. */
>  				if ((status & 0xffff) != 0x7fff) {
>  					if (tulip_debug > 1)
> @@ -372,31 +407,22 @@ static int tulip_rx(struct net_device *dev)
>  							   dev->name, status);
>  					tp->stats.rx_length_errors++;
>  				}
> -			} else if (status & RxDescFatalErr) {
> +			} else {
>  				/* There was a fatal error. */
>  				if (tulip_debug > 2)
>  					printk(KERN_DEBUG "%s: Receive error, Rx status %8.8x.\n",
>  						   dev->name, status);
>  				tp->stats.rx_errors++; /* end of a packet.*/
> -				if (status & 0x0890) tp->stats.rx_length_errors++;
> +				if (pkt_len > 1518 ||
> +				    (status & RxDescRunt))
> +					tp->stats.rx_length_errors++;
>  				if (status & 0x0004) tp->stats.rx_frame_errors++;
>  				if (status & 0x0002) tp->stats.rx_crc_errors++;
>  				if (status & 0x0001) tp->stats.rx_fifo_errors++;
>  			}
>  		} else {
> -			/* Omit the four octet CRC from the length. */
> -			short pkt_len = ((status >> 16) & 0x7ff) - 4;
>  			struct sk_buff *skb;
>  
> -#ifndef final_version
> -			if (pkt_len > 1518) {
> -				printk(KERN_WARNING "%s: Bogus packet size of %d (%#x).\n",
> -					   dev->name, pkt_len, pkt_len);
> -				pkt_len = 1518;
> -				tp->stats.rx_length_errors++;
> -			}
> -#endif
> -
>  			/* Check if the packet is long enough to accept without copying
>  			   to a minimally-sized skbuff. */
>  			if (pkt_len < tulip_rx_copybreak
> diff --git a/drivers/net/tulip/tulip.h b/drivers/net/tulip/tulip.h
> index 19abbc3..0afa2d4 100644
> --- a/drivers/net/tulip/tulip.h
> +++ b/drivers/net/tulip/tulip.h
> @@ -201,8 +201,38 @@ enum desc_status_bits {
>  	DescStartPkt = 0x20000000,
>  	DescEndRing  = 0x02000000,
>  	DescUseLink  = 0x01000000,
> -	RxDescFatalErr = 0x008000,
> +
> +	/*
> +	 * Error summary flag is logical or of 'CRC Error', 'Collision Seen',
> +	 * 'Frame Too Long', 'Runt' and 'Descriptor Error' flags generated
> +	 * within tulip chip.
> +	 */
> +	RxDescErrorSummary = 0x8000,
> +	RxDescCRCError = 0x0002,
> +	RxDescCollisionSeen = 0x0040,
> +
> +	/*
> +	 * 'Frame Too Long' flag is set if packet length including CRC exceeds
> +	 * 1518.  However, a full sized VLAN tagged frame is 1522 bytes
> +	 * including CRC.
> +	 *
> +	 * The tulip chip does not block oversized frames, and if this flag is
> +	 * set on a receive descriptor it does not indicate the frame has been
> +	 * truncated.  The receive descriptor also includes the actual length.
> +	 * Therefore we can safety ignore this flag and check the length
> +	 * ourselves.
> +	 */
> +	RxDescFrameTooLong = 0x0080,
> +	RxDescRunt = 0x0800,
> +	RxDescDescErr = 0x4000,
>  	RxWholePkt   = 0x00000300,
> +	/*
> +	 * Top three bits of 14 bit frame length (status bits 27-29) should
> +	 * never be set as that would make frame over 2047 bytes. The Receive
> +	 * Watchdog flag (bit 4) may indicate the length is over 2048 and the
> +	 * length field is invalid.
> +	 */
> +	RxLengthOver2047 = 0x38000010
>  };
>  
>  
> -- 
> 1.6.0.4

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

* Re: [PATCH] tulip: Fix for MTU problems with 802.1q tagged frames
  2009-03-05 10:21 [PATCH] tulip: Fix for MTU problems with 802.1q tagged frames Ivan Vecera
  2009-03-06 12:48 ` Ben McKeegan
  2009-03-06 18:13 ` Grant Grundler
@ 2009-03-08 19:53 ` Kyle McMartin
  2 siblings, 0 replies; 6+ messages in thread
From: Kyle McMartin @ 2009-03-08 19:53 UTC (permalink / raw)
  To: Ivan Vecera; +Cc: netdev, grundler, kyle, Tomasz Lemiech

On Thu, Mar 05, 2009 at 11:21:34AM +0100, Ivan Vecera wrote:
> The original patch was submitted last year but wasn't discussed or applied
> because of missing maintainer's CCs. I only fixed some formatting errors,
> but as I saw tulip is very badly formatted and needs further work.
> 
> Original description:
> This patch fixes MTU problem, which occurs when using 802.1q VLANs. We
> should allow receiving frames of up to 1518 bytes in length, instead of
> 1514.
> 
> Based on patch written by Ben McKeegan for 2.4.x kernels. It is archived
> at http://www.candelatech.com/~greear/vlan/howto.html#tulip
> I've adjusted a few things to make it apply on 2.6.x kernels.
> 
> Tested on D-Link DFE-570TX quad-fastethernet card.
> 
> Signed-off-by: Tomasz Lemiech <szpajder@staszic.waw.pl>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
>

Feel free to add Acked-by or Tested-by: Kyle McMartin
<kyle@mcmartin.ca>, it doesn't cause any regressions on any of my tulip
cards.

regards, Kyle

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

* Re: [PATCH] tulip: Fix for MTU problems with 802.1q tagged frames
  2009-03-06 18:13 ` Grant Grundler
@ 2009-03-13 22:43   ` David Miller
  0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2009-03-13 22:43 UTC (permalink / raw)
  To: grundler; +Cc: ivecera, netdev, kyle, szpajder

From: Grant Grundler <grundler@parisc-linux.org>
Date: Fri, 6 Mar 2009 11:13:22 -0700

> On Thu, Mar 05, 2009 at 11:21:34AM +0100, Ivan Vecera wrote:
> > The original patch was submitted last year but wasn't discussed or applied
> > because of missing maintainer's CCs. I only fixed some formatting errors,
> > but as I saw tulip is very badly formatted and needs further work.
> > 
> > Original description:
> > This patch fixes MTU problem, which occurs when using 802.1q VLANs. We
> > should allow receiving frames of up to 1518 bytes in length, instead of
> > 1514.
> > 
> > Based on patch written by Ben McKeegan for 2.4.x kernels. It is archived
> > at http://www.candelatech.com/~greear/vlan/howto.html#tulip
> > I've adjusted a few things to make it apply on 2.6.x kernels.
> > 
> > Tested on D-Link DFE-570TX quad-fastethernet card.
> > 
> > Signed-off-by: Tomasz Lemiech <szpajder@staszic.waw.pl>
> > Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> 
> Acked-by: Grant Grundler <grundler@parisc-linux.org>

Applied to net-next-2.6, thanks everyone.

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

* [PATCH] [TULIP] Fix for MTU problems with 802.1q tagged frames
@ 2008-04-15 21:18 Tomasz Lemiech
  0 siblings, 0 replies; 6+ messages in thread
From: Tomasz Lemiech @ 2008-04-15 21:18 UTC (permalink / raw)
  To: netdev; +Cc: tulip-users


This patch fixes MTU problem, which occurs when using 802.1q VLANs.
We should allow receiving frames of up to 1518 bytes in length,
instead of 1514.

Based on patch written by Ben McKeegan for 2.4.x kernels. It is
archived at http://www.candelatech.com/~greear/vlan/howto.html#tulip .
I've adjusted a few things to make it apply on 2.6.x kernels.

Tested on D-Link DFE-570TX quad-fastethernet card.

Signed-off-by: Tomasz Lemiech <szpajder@staszic.waw.pl>
---

 drivers/net/tulip/interrupt.c |   81 ++++++++++++++++++++------------
 drivers/net/tulip/tulip.h     |   32 ++++++++++++
 2 files changed, 83 insertions(+), 30 deletions(-)

diff -urp a/drivers/net/tulip/interrupt.c b/drivers/net/tulip/interrupt.c
--- a/drivers/net/tulip/interrupt.c	2008-04-13 14:43:47.000000000 +0200
+++ b/drivers/net/tulip/interrupt.c	2008-04-13 14:51:20.000000000 +0200
@@ -141,6 +141,7 @@ int tulip_poll(struct napi_struct *napi,
                /* If we own the next entry, it is a new packet. Send it up. */
                while ( ! (tp->rx_ring[entry].status & cpu_to_le32(DescOwned))) {
                        s32 status = le32_to_cpu(tp->rx_ring[entry].status);
+		       short pkt_len;
 
                        if (tp->dirty_rx + RX_RING_SIZE == tp->cur_rx)
                                break;
@@ -152,8 +153,28 @@ int tulip_poll(struct napi_struct *napi,
 		       if (++work_done >= budget)
                                goto not_done;
 
-                       if ((status & 0x38008300) != 0x0300) {
-                               if ((status & 0x38000300) != 0x0300) {
+		/*
+		  Omit the four octet CRC from the length.
+		  (May not be considered valid until we have
+		  checked status for RxLengthOver2047 bits)
+		*/
+		       pkt_len = ((status >> 16) & 0x7ff) - 4;
+
+		/*
+		  Maximum pkt_len is 1518 (1514 + vlan header)
+		  Anything higher than this is always invalid
+		  regardless of RxLengthOver2047 bits
+		*/
+
+		       if ((status & (RxLengthOver2047 |
+				RxDescCRCError |
+				RxDescCollisionSeen |
+				RxDescRunt |
+				RxDescDescErr |
+				RxWholePkt))        != RxWholePkt
+			    || pkt_len > 1518) {
+			       if ((status & (RxLengthOver2047 |
+					RxWholePkt))         != RxWholePkt) {
                                 /* Ingore earlier buffers. */
                                        if ((status & 0xffff) != 0x7fff) {
                                                if (tulip_debug > 1)
@@ -162,30 +183,21 @@ int tulip_poll(struct napi_struct *napi,
                                                               dev->name, status);
                                                tp->stats.rx_length_errors++;
                                        }
-                               } else if (status & RxDescFatalErr) {
+                               } else {
                                 /* There was a fatal error. */
                                        if (tulip_debug > 2)
                                                printk(KERN_DEBUG "%s: Receive error, Rx status %8.8x.\n",
                                                       dev->name, status);
                                        tp->stats.rx_errors++; /* end of a packet.*/
-                                       if (status & 0x0890) tp->stats.rx_length_errors++;
+				       if (pkt_len > 1518 ||
+					   status & RxDescRunt) tp->stats.rx_length_errors++;
                                        if (status & 0x0004) tp->stats.rx_frame_errors++;
                                        if (status & 0x0002) tp->stats.rx_crc_errors++;
                                        if (status & 0x0001) tp->stats.rx_fifo_errors++;
                                }
                        } else {
-                               /* Omit the four octet CRC from the length. */
-                               short pkt_len = ((status >> 16) & 0x7ff) - 4;
                                struct sk_buff *skb;
 
-#ifndef final_version
-                               if (pkt_len > 1518) {
-                                       printk(KERN_WARNING "%s: Bogus packet size of %d (%#x).\n",
-                                              dev->name, pkt_len, pkt_len);
-                                       pkt_len = 1518;
-                                       tp->stats.rx_length_errors++;
-                               }
-#endif
                                /* Check if the packet is long enough to accept without copying
                                   to a minimally-sized skbuff. */
                                if (pkt_len < tulip_rx_copybreak
@@ -358,14 +370,35 @@ static int tulip_rx(struct net_device *d
 	/* If we own the next entry, it is a new packet. Send it up. */
 	while ( ! (tp->rx_ring[entry].status & cpu_to_le32(DescOwned))) {
 		s32 status = le32_to_cpu(tp->rx_ring[entry].status);
+		short pkt_len;
 
 		if (tulip_debug > 5)
 			printk(KERN_DEBUG "%s: In tulip_rx(), entry %d %8.8x.\n",
 				   dev->name, entry, status);
 		if (--rx_work_limit < 0)
 			break;
-		if ((status & 0x38008300) != 0x0300) {
-			if ((status & 0x38000300) != 0x0300) {
+
+		/*
+		  Omit the four octet CRC from the length.
+		  (May not be considered valid until we have
+		  checked status for RxLengthOver2047 bits)
+		*/
+		pkt_len = ((status >> 16) & 0x7ff) - 4;
+		/*
+		  Maximum pkt_len is 1518 (1514 + vlan header)
+		  Anything higher than this is always invalid
+		  regardless of RxLengthOver2047 bits
+		*/
+
+		if ((status & (RxLengthOver2047 |
+			       RxDescCRCError |
+			       RxDescCollisionSeen |
+			       RxDescRunt |
+			       RxDescDescErr |
+			       RxWholePkt))        != RxWholePkt
+		     || pkt_len > 1518) {
+			if ((status & (RxLengthOver2047 |
+			     RxWholePkt))         != RxWholePkt) {
 				/* Ingore earlier buffers. */
 				if ((status & 0xffff) != 0x7fff) {
 					if (tulip_debug > 1)
@@ -374,31 +407,21 @@ static int tulip_rx(struct net_device *d
 							   dev->name, status);
 					tp->stats.rx_length_errors++;
 				}
-			} else if (status & RxDescFatalErr) {
+			} else {
 				/* There was a fatal error. */
 				if (tulip_debug > 2)
 					printk(KERN_DEBUG "%s: Receive error, Rx status %8.8x.\n",
 						   dev->name, status);
 				tp->stats.rx_errors++; /* end of a packet.*/
-				if (status & 0x0890) tp->stats.rx_length_errors++;
+				if (pkt_len > 1518 ||
+				    status & RxDescRunt) tp->stats.rx_length_errors++;
 				if (status & 0x0004) tp->stats.rx_frame_errors++;
 				if (status & 0x0002) tp->stats.rx_crc_errors++;
 				if (status & 0x0001) tp->stats.rx_fifo_errors++;
 			}
 		} else {
-			/* Omit the four octet CRC from the length. */
-			short pkt_len = ((status >> 16) & 0x7ff) - 4;
 			struct sk_buff *skb;
 
-#ifndef final_version
-			if (pkt_len > 1518) {
-				printk(KERN_WARNING "%s: Bogus packet size of %d (%#x).\n",
-					   dev->name, pkt_len, pkt_len);
-				pkt_len = 1518;
-				tp->stats.rx_length_errors++;
-			}
-#endif
-
 			/* Check if the packet is long enough to accept without copying
 			   to a minimally-sized skbuff. */
 			if (pkt_len < tulip_rx_copybreak
diff -urp a/drivers/net/tulip/tulip.h b/drivers/net/tulip/tulip.h
--- a/drivers/net/tulip/tulip.h	2008-04-13 14:43:47.000000000 +0200
+++ b/drivers/net/tulip/tulip.h	2008-04-13 14:48:29.000000000 +0200
@@ -200,8 +200,38 @@ enum desc_status_bits {
 	DescStartPkt = 0x20000000,
 	DescEndRing  = 0x02000000,
 	DescUseLink  = 0x01000000,
-	RxDescFatalErr = 0x008000,
+
+	/*
+	  Error summary flag is logical or of 'CRC Error',
+	  'Collision Seen', 'Frame Too Long', 'Runt' and
+	  'Descriptor Error' flags generated within tulip chip.
+	*/
+	RxDescErrorSummary = 0x8000,
+	RxDescCRCError = 0x0002,
+	RxDescCollisionSeen = 0x0040,
+
+	/*
+	  'Frame Too Long' flag is set if packet length including CRC
+	  exceeds 1518.  However, a full sized VLAN tagged frame is
+	  1522 bytes including CRC.
+
+	  The tulip chip does not block oversized frames, and if this
+	  flag is set on a receive descriptor it does not indicate
+	  the frame has been truncated.  The receive descriptor also
+	  includes the actual length.  Therefore we can safety ignore
+	  this flag and check the length ourselves.
+	*/
+	RxDescFrameTooLong = 0x0080,
+	RxDescRunt = 0x0800,
+	RxDescDescErr = 0x4000,
 	RxWholePkt   = 0x00000300,
+	/*
+	  Top three bits of 14 bit frame length (status bits 27-29)
+	    should never be set as that would make frame over 2047 bytes.
+	  The Receive Watchdog flag (bit 4) may indicate the length is
+	    over 2048 and the length field is invalid.
+	*/
+	RxLengthOver2047 = 0x38000010
 };
 
 

-- 
Tomasz Lemiech
RLU#189399
TL1942-RIPE

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

end of thread, other threads:[~2009-03-13 22:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-03-05 10:21 [PATCH] tulip: Fix for MTU problems with 802.1q tagged frames Ivan Vecera
2009-03-06 12:48 ` Ben McKeegan
2009-03-06 18:13 ` Grant Grundler
2009-03-13 22:43   ` David Miller
2009-03-08 19:53 ` Kyle McMartin
  -- strict thread matches above, loose matches on Subject: below --
2008-04-15 21:18 [PATCH] [TULIP] " Tomasz Lemiech

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.