Re: [PATCH] MPC5121 FEC support

[Scott Wood <scottwood@freescale.com>]

This patch should go through Jeff Garzik and be CCed to 
netdev@vger.kernel.org.

John Rigby wrote:
>   drivers/net/fs_enet/fs_enet-main.c
>     Add a routine for conditionally copying TX skb's that
>     are not aligned.  Only do this if fep->fpi->align_tx_packets
>     is set.
>     In fs_enet_probe set fpi->align_tx_packets to 1 if
>     the device node has the property fsl,align-tx-packets.

How about setting the property to the size that it needs to be aligned to?

> +/*
> + * The 5121 FEC doc says transmit buffers must have 4 byte alignment,
> + * however testing has shown that 2 byte alignment works fine except
> + * for buffers that end in 0x1e.
> + *
> + * Testing with iperf shows about 30% cpu load when copying all packets
> + * vs 19% when only copying 0x1e packets.
> + */
> +static struct sk_buff *align_tx_skb(struct net_device *dev, struct sk_buff *skb)
> +{
> +	struct sk_buff *skbn;
> +	int align = ((unsigned long)skb->data) & (0x1e);
> +
> +	if (align != 0x1e)
> +		return skb;

Are we guaranteed that skb->data is always 2-byte aligned, even with 
obscure protocols?

> +	if (!skbn) {
> +		printk(KERN_WARNING DRV_MODULE_NAME
> +			": %s Memory squeeze, dropping tx packet.\n",
> +			dev->name);

Does this need to be rate-limited?

> -	if (!IS_FEC(match)) {
> +	if (IS_FEC(match)) {
> +		if (of_get_property(ofdev->node, "fsl,align-tx-packets", NULL))
> +			fpi->align_tx_packets = 1;

Might as well check for the property regardless of which ethernet type 
it is, in case a similar bug crops up elsewhere.

-Scott