On Wed, Jan 20, 2010 at 7:13 PM, Anatolij Gustschin <ag...@denx.de> wrote: > The FEC on 5121 has problems with misaligned tx buffers. > The RM says any alignment is ok but empirical results > show that packet buffers ending in 0x1E will sometimes > hang the FEC. Other bad alignment does not hang but will > cause silent TX failures resulting in about a 1% packet > loss as tested by ping -f from a remote host. > > This patch is a work around that copies every tx packet > to an aligned skb before sending. > > Signed-off-by: John Rigby <jcri...@gmail.com> > Signed-off-by: Piotr Ziecik <ko...@semihalf.com> > Signed-off-by: Wolfgang Denk <w...@denx.de> > Signed-off-by: Anatolij Gustschin <ag...@denx.de> > Cc: <linuxppc-...@ozlabs.org> > Cc: Grant Likely <grant.lik...@secretlab.ca> > --- > Changes since previous submited version: > > - don't use printk any more and use dev_warn(). > > drivers/net/fs_enet/fs_enet-main.c | 38 > ++++++++++++++++++++++++++++++++++++ > 1 files changed, 38 insertions(+), 0 deletions(-) > > diff --git a/drivers/net/fs_enet/fs_enet-main.c > b/drivers/net/fs_enet/fs_enet-main.c > index 6bce5c8..7f0bd5d 100644 > --- a/drivers/net/fs_enet/fs_enet-main.c > +++ b/drivers/net/fs_enet/fs_enet-main.c > @@ -580,6 +580,32 @@ void fs_cleanup_bds(struct net_device *dev) > > /**********************************************************************************/ > > +static struct sk_buff *tx_skb_align_workaround(struct net_device *dev, > + struct sk_buff *skb) > +{ > + struct sk_buff *new_skb; > + struct fs_enet_private *fep = netdev_priv(dev); > + > + /* Alloc new skb */ > + new_skb = dev_alloc_skb(ENET_RX_FRSIZE + 32); > + if (!new_skb) { > + dev_warn(fep->dev, "Memory squeeze, dropping tx packet.\n"); > + return NULL; > + } > + > + /* Make sure new skb is properly aligned */ > + skb_align(new_skb, 32); > + > + /* Copy data to new skb ... */ > + skb_copy_from_linear_data(skb, new_skb->data, skb->len); > + skb_put(new_skb, skb->len); > + > + /* ... and free an old one */ > + dev_kfree_skb_any(skb); > + > + return new_skb; > +}
This looks expensive. It's allocating a new skb on *every* packet transmission, rather than just on the ones that fail the alignment condition. > + > static int fs_enet_start_xmit(struct sk_buff *skb, struct net_device *dev) > { > struct fs_enet_private *fep = netdev_priv(dev); > @@ -588,6 +614,18 @@ static int fs_enet_start_xmit(struct sk_buff *skb, > struct net_device *dev) > u16 sc; > unsigned long flags; > > + if (fep->fec.fec_id == FS_ENET_MPC5121_FEC) { > + skb = tx_skb_align_workaround(dev, skb); > + if (!skb) { > + /* > + * We have lost packet due to memory allocation error > + * in tx_skb_align_workaround(). Hopefully original > skb > + * is still valid, so try transmit it later. > + */ > + return NETDEV_TX_BUSY; > + } > + } > + Rather than performing this test every time through on a fast path, consider creating a new start_xmit hook and registering the correct one in the ops structure at probe time. > spin_lock_irqsave(&fep->tx_lock, flags); > > /* > -- > 1.5.6.3 > > -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev