e1000: Problem with "disable CRC stripping workaround" patch

e1000: Problem with "disable CRC stripping workaround" patch

[Mark McLoughlin]

[-- Attachment #1: Type: text/plain, Size: 967 bytes --]

Hi Jesse,
	I just came across this:

  http://www.mail-archive.com/netdev@vger.kernel.org/msg14547.html

	I'm seeing a problem with this currently under Xen's bridging
configuration.

	Basically, with the patch, packets are being dropped at this point in
net/bridge/br_forward.c:

---
int br_dev_queue_push_xmit(struct sk_buff *skb)
{
        /* drop mtu oversized packets except gso */
	if (packet_length(skb) > skb->dev->mtu && !skb_is_gso(skb)) {
		kfree_skb(skb);
--

	What's happening that a 1500 byte packet comes in from e1000, onto the
bridge and is to be forwarded to a device whose mtu is 1500. Because the
CRC hasn't been stripped, skb->len is 1504 and the packet is dropped.

	One option is to fix this specific problem is to subtract the CRC
length from skb->len in e1000, another is to raise the MTU on the
receive side of Xen's loopback interface. I've attached a patch for the
latter, but I've no real opinion on which is more correct.

Cheers,
Mark.

[-- Attachment #2: xen-netback-jumbo-mtu-on-vif.patch --]
[-- Type: text/x-patch, Size: 1363 bytes --]

--- ./xen/netback/loopback.c.jumbo-mtu-on-vif	2006-07-20 16:21:52.000000000 +0100
+++ ./xen/netback/loopback.c	2006-07-20 16:23:08.000000000 +0100
@@ -161,16 +161,6 @@
 				NETIF_F_IP_CSUM);
 
 	SET_ETHTOOL_OPS(dev, &network_ethtool_ops);
-
-	/*
-	 * We do not set a jumbo MTU on the interface. Otherwise the network
-	 * stack will try to send large packets that will get dropped by the
-	 * Ethernet bridge (unless the physical Ethernet interface is
-	 * configured to transfer jumbo packets). If a larger MTU is desired
-	 * then the system administrator can specify it using the 'ifconfig'
-	 * command.
-	 */
-	/*dev->mtu             = 16*1024;*/
 }
 
 static int __init make_loopback(int i)
@@ -193,6 +183,16 @@
 	loopback_construct(dev2, dev1);
 
 	/*
+	 * We only set a jumbo MTU on vif0.0. Otherwise the network
+	 * stack will try to send large packets that will get dropped by the
+	 * Ethernet bridge (unless the physical Ethernet interface is
+	 * configured to transfer jumbo packets). If a larger MTU is desired
+	 * then the system administrator can specify it using the 'ifconfig'
+	 * command.
+	 */
+	dev1->mtu = 16*1024;
+
+	/*
 	 * Initialise a dummy MAC address for the 'dummy backend' interface. We
 	 * choose the numerically largest non-broadcast address to prevent the
 	 * address getting stolen by an Ethernet bridge for STP purposes.

Re: e1000: Problem with "disable CRC stripping workaround" patch

[Auke Kok]

Mark McLoughlin wrote:
> Hi Jesse,
> 	I just came across this:
> 
>   http://www.mail-archive.com/netdev@vger.kernel.org/msg14547.html
> 
> 	I'm seeing a problem with this currently under Xen's bridging
> configuration.


 > 	One option is to fix this specific problem is to subtract the CRC
 > length from skb->len in e1000, another is to raise the MTU on the
 > receive side of Xen's loopback interface. I've attached a patch for the
 > latter, but I've no real opinion on which is more correct.

We were sort of expecting this and sent the following patch upstream already - 
it's already queued for 2.6.18-rc3:

http://kernel.org/git/?p=linux/kernel/git/jgarzik/netdev-2.6.git;a=commitdiff_plain;h=f235a2abb27b9396d2108dd2987fb8262cb508a3;hp=d3d9e484b2ca502c87156b69fa6b8f8fd5fa18a0

Please give it a try and let us know if it fixes the issue for you, it should 
be much better than patching xen's code.

Cheers,

Auke

Re: e1000: Problem with "disable CRC stripping workaround" patch

[Herbert Xu]

On Thu, Jul 20, 2006 at 09:42:48AM -0700, Auke Kok wrote:
>
> Please give it a try and let us know if it fixes the issue for you, it 
> should be much better than patching xen's code.

I don't really care on way or another whether we get the FCS.
However, the important thing to note here for Xen is that MTU
!= MRU.  Just because you've set the MTU to 1500 does not imply
that you won't receive packets > 1500.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: e1000: Problem with "disable CRC stripping workaround" patch

[Mark McLoughlin]

Hi,

On Thu, 2006-07-20 at 09:42 -0700, Auke Kok wrote:
> Mark McLoughlin wrote:
> > Hi Jesse,
> > 	I just came across this:
> > 
> >   http://www.mail-archive.com/netdev@vger.kernel.org/msg14547.html
> > 
> > 	I'm seeing a problem with this currently under Xen's bridging
> > configuration.

>  > 	One option is to fix this specific problem is to subtract the CRC
>  > length from skb->len in e1000, another is to raise the MTU on the
>  > receive side of Xen's loopback interface. I've attached a patch for the
>  > latter, but I've no real opinion on which is more correct.
> 
> We were sort of expecting this and sent the following patch upstream already - 
> it's already queued for 2.6.18-rc3:
> 
> http://kernel.org/git/?p=linux/kernel/git/jgarzik/netdev-2.6.git;a=commitdiff_plain;h=f235a2abb27b9396d2108dd2987fb8262cb508a3;hp=d3d9e484b2ca502c87156b69fa6b8f8fd5fa18a0
> 
> Please give it a try and let us know if it fixes the issue for you, it should 
> be much better than patching xen's code.

	Yep, this fixes the problem for me.

Thanks,
Mark.