Re: [EXPERIMENTAL PATCH] 2.4 tulip jumbo frames

[Rask Ingemann Lambertsen <rask@sygehus.dk>]

On Tue, Dec 09, 2003 at 11:45:42AM -0500, Jeff Garzik wrote:
> Two questions and a comment...
> 
> Would you split this into two patches?  The first simply adds, and uses, 
> tp->rx_buf_sz.  The second adds PKT_BUF_SZ_MAX and mtu-related changes.

That sounds like a good idea. I'll do that.
I tried to find a good place in the private structure to add the rx_buf_sz
field. By good, I mean using the cache efficiently. Any comments about that?

> Have you looked at Donald Becker's changes to tulip.c?  He went through 
> most of his drivers and made the changes necessary to support larger 
> MTUs.  IIRC his tulip.c changes (which should be easily translate-able 
> to 2.6.x tulip) were a bit more minimal than your patch, but still 
> served the purpose.

I have not looked at this particular part of Becker's tulip.c. I'll have a
look at it.

> For the comment:  I am curious why a VLAN_xxx constant is included in 
> the calculation of max MTU, in the ->change_mtu hook?

This is to ensure that the receive buffers are large enough to hold a frame
of the requested MTU also when using VLAN tags.

> IMO ->change_mtu 
> simply needs to bind the MTU to the min and max h/w limits.  If 
> VLAN_ETH_HLEN ever figures into the calculations, those calculations 
> should occur elsewhere, not in ->change_mtu.

What do you propose? Do we need something like

int vlan_adjust_mtu (int mtu)
{
#ifdef CONFIG_VLANN_8021Q
	return (mtu - VLAN_HLEN);
#else
	return (mtu);
#endif
}

and

int foobar_change_mtu (struct net_device *dev, int mtu)
{
	mtu = vlan_adjust_mtu (mtu);
	/* check hardware limits. */
	...
	dev->mtu = mtu;
	return (0);
}

? Ben, this would also keep you happy, right?

-- 
Regards,
Rask Ingemann Lambertsen