From: Grant Grundler <grundler@parisc-linux.org>
To: Ivan Vecera <ivecera@redhat.com>
Cc: netdev@vger.kernel.org, grundler@parisc-linux.org,
kyle@mcmartin.ca, Tomasz Lemiech <szpajder@staszic.waw.pl>
Subject: Re: [PATCH] tulip: Fix for MTU problems with 802.1q tagged frames
Date: Fri, 6 Mar 2009 11:13:22 -0700 [thread overview]
Message-ID: <20090306181322.GA3616@colo.lackof.org> (raw)
In-Reply-To: <1236248494-1599-1-git-send-email-ivecera@redhat.com>
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
next prev parent reply other threads:[~2009-03-06 18:13 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20090306181322.GA3616@colo.lackof.org \
--to=grundler@parisc-linux.org \
--cc=ivecera@redhat.com \
--cc=kyle@mcmartin.ca \
--cc=netdev@vger.kernel.org \
--cc=szpajder@staszic.waw.pl \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).