[PATCH] bgmac: omit the fcs

[PATCH] bgmac: omit the fcs

[Hauke Mehrtens]

Do not include the frame check sequence when adding the skb to
netif_receive_skb(). This causes problems when this interface was
bridged to a wifi ap and a big package should be forwarded from this
Ethernet driver through a bride to the wifi client.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/net/ethernet/broadcom/bgmac.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index bf985c0..bce30e7 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -301,12 +301,16 @@ static int bgmac_dma_rx_read(struct bgmac *bgmac, struct bgmac_dma_ring *ring,
 			bgmac_err(bgmac, "Found poisoned packet at slot %d, DMA issue!\n",
 				  ring->start);
 		} else {
+			/* Omit CRC. */
+			len -= ETH_FCS_LEN;
+
 			new_skb = netdev_alloc_skb_ip_align(bgmac->net_dev, len);
 			if (new_skb) {
 				skb_put(new_skb, len);
 				skb_copy_from_linear_data_offset(skb, BGMAC_RX_FRAME_OFFSET,
 								 new_skb->data,
 								 len);
+				skb_checksum_none_assert(skb);
 				new_skb->protocol =
 					eth_type_trans(new_skb, bgmac->net_dev);
 				netif_receive_skb(new_skb);
-- 
1.7.10.4

Re: [PATCH] bgmac: omit the fcs

[Rafał Miłecki]

2013/2/28 Hauke Mehrtens <hauke@hauke-m.de>:
> Do not include the frame check sequence when adding the skb to
> netif_receive_skb(). This causes problems when this interface was
> bridged to a wifi ap and a big package should be forwarded from this
> Ethernet driver through a bride to the wifi client.

Is this a real fix?

Don't get me wrong, but it sounds a little like a workaround for some
issue in another network layer ;)

-- 
Rafał

Re: [PATCH] bgmac: omit the fcs

[Hauke Mehrtens]

On 02/28/2013 06:57 PM, Rafał Miłecki wrote:
> 2013/2/28 Hauke Mehrtens <hauke@hauke-m.de>:
>> Do not include the frame check sequence when adding the skb to
>> netif_receive_skb(). This causes problems when this interface was
>> bridged to a wifi ap and a big package should be forwarded from this
>> Ethernet driver through a bride to the wifi client.
> 
> Is this a real fix?
> 
> Don't get me wrong, but it sounds a little like a workaround for some
> issue in another network layer ;)
> 
Many drivers are doing similar things, b44 is one of them. Hopefully
someone more familiar with the network stack could say if this is the
right way to do this.

On a wireless client I saw that all packages directly from my router did
not had a FCS, but all the packages send from an other LAN client which
went through this driver had a FCS and where 4 bytes bigger and this FCS
was wrong.

Hauke

Re: [PATCH] bgmac: omit the fcs

[David Miller]

From: Rafał Miłecki <zajec5@gmail.com>
Date: Thu, 28 Feb 2013 18:57:03 +0100

> 2013/2/28 Hauke Mehrtens <hauke@hauke-m.de>:
>> Do not include the frame check sequence when adding the skb to
>> netif_receive_skb(). This causes problems when this interface was
>> bridged to a wifi ap and a big package should be forwarded from this
>> Ethernet driver through a bride to the wifi client.
> 
> Is this a real fix?
> 
> Don't get me wrong, but it sounds a little like a workaround for some
> issue in another network layer ;)

FCS should never be included in the SKB unless a specific debugging
configuration knob has enabled it.

Having the FCS there will screw up things like device provided partial
checksums (CHECKSUM_PARTIAL), which operate over the entire contents
of the packet starting at a particular offset, so if the FCS is there
we'll include it.

This change is therefore about as correct as can possibly be.

Re: [PATCH] bgmac: omit the fcs

[Rafał Miłecki]

2013/2/28 David Miller <davem@davemloft.net>:
> From: Rafał Miłecki <zajec5@gmail.com>
> Date: Thu, 28 Feb 2013 18:57:03 +0100
>
>> 2013/2/28 Hauke Mehrtens <hauke@hauke-m.de>:
>>> Do not include the frame check sequence when adding the skb to
>>> netif_receive_skb(). This causes problems when this interface was
>>> bridged to a wifi ap and a big package should be forwarded from this
>>> Ethernet driver through a bride to the wifi client.
>>
>> Is this a real fix?
>>
>> Don't get me wrong, but it sounds a little like a workaround for some
>> issue in another network layer ;)
>
> FCS should never be included in the SKB unless a specific debugging
> configuration knob has enabled it.
>
> Having the FCS there will screw up things like device provided partial
> checksums (CHECKSUM_PARTIAL), which operate over the entire contents
> of the packet starting at a particular offset, so if the FCS is there
> we'll include it.
>
> This change is therefore about as correct as can possibly be.

OK, thanks! :)

-- 
Rafał

Re: [PATCH] bgmac: omit the fcs

[David Miller]

From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Thu, 28 Feb 2013 18:16:54 +0100

> Do not include the frame check sequence when adding the skb to
> netif_receive_skb(). This causes problems when this interface was
> bridged to a wifi ap and a big package should be forwarded from this
> Ethernet driver through a bride to the wifi client.
> 
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>

Applied.

Re: [PATCH] bgmac: omit the fcs

[Eric Dumazet]

On Thu, 2013-02-28 at 18:57 +0100, Rafał Miłecki wrote:

> Is this a real fix?
> 
> Don't get me wrong, but it sounds a little like a workaround for some
> issue in another network layer ;)

It looks a real fix in a forwarding workload, or else MTU would need 4
extra bytes.