Message ID | 1266418530-2727-4-git-send-email-agust@denx.de |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
On Wed, Feb 17, 2010 at 7:55 AM, Anatolij Gustschin <agust@denx.de> wrote: > MPC5121 FEC requeries 4-byte alignmnent for TX data buffers. > This patch is a work around that copies misaligned tx packets > to an aligned skb before sending. > > Signed-off-by: John Rigby <jcrigby@gmail.com> > Signed-off-by: Piotr Ziecik <kosmo@semihalf.com> > Signed-off-by: Wolfgang Denk <wd@denx.de> > Signed-off-by: Anatolij Gustschin <agust@denx.de> Acked-by: Grant Likely <grant.likely@secretlab.ca> > --- > drivers/net/fs_enet/fs_enet-main.c | 44 ++++++++++++++++++++++++++++++++++++ > 1 files changed, 44 insertions(+), 0 deletions(-) > > diff --git a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c > index 4297021..166a89d 100644 > --- a/drivers/net/fs_enet/fs_enet-main.c > +++ b/drivers/net/fs_enet/fs_enet-main.c > @@ -580,6 +580,37 @@ void fs_cleanup_bds(struct net_device *dev) > > /**********************************************************************************/ > > +#ifdef CONFIG_FS_ENET_MPC5121_FEC > +/* > + * MPC5121 FEC requeries 4-byte alignment for TX data buffer! > + */ > +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 + 4); > + 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, 4); > + > + /* 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; > +} > +#endif > + > static int fs_enet_start_xmit(struct sk_buff *skb, struct net_device *dev) > { > struct fs_enet_private *fep = netdev_priv(dev); > @@ -588,6 +619,19 @@ static int fs_enet_start_xmit(struct sk_buff *skb, struct net_device *dev) > u16 sc; > unsigned long flags; > > +#ifdef CONFIG_FS_ENET_MPC5121_FEC > + if (((unsigned long)skb->data) & 0x3) { > + 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; > + } > + } > +#endif Instead of > spin_lock_irqsave(&fep->tx_lock, flags); > > /* > -- > 1.6.3.3 > >
Le mercredi 17 février 2010 à 15:55 +0100, Anatolij Gustschin a écrit : > MPC5121 FEC requeries 4-byte alignmnent for TX data buffers. > This patch is a work around that copies misaligned tx packets > to an aligned skb before sending. > > Signed-off-by: John Rigby <jcrigby@gmail.com> > Signed-off-by: Piotr Ziecik <kosmo@semihalf.com> > Signed-off-by: Wolfgang Denk <wd@denx.de> > Signed-off-by: Anatolij Gustschin <agust@denx.de> > --- > drivers/net/fs_enet/fs_enet-main.c | 44 ++++++++++++++++++++++++++++++++++++ > 1 files changed, 44 insertions(+), 0 deletions(-) > > diff --git a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c > index 4297021..166a89d 100644 > --- a/drivers/net/fs_enet/fs_enet-main.c > +++ b/drivers/net/fs_enet/fs_enet-main.c > @@ -580,6 +580,37 @@ void fs_cleanup_bds(struct net_device *dev) > > /**********************************************************************************/ > > +#ifdef CONFIG_FS_ENET_MPC5121_FEC > +/* > + * MPC5121 FEC requeries 4-byte alignment for TX data buffer! > + */ > +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 + 4); ENET_RX_FRSIZE looks strange in TX path Why not using skb->len + 4 instead of ENET_RX_FRSIZE + 4 ? > + if (!new_skb) { > + dev_warn(fep->dev, "Memory squeeze, dropping tx packet.\n"); I am just wondering if this is ratelimited ? > + return NULL; > + } > + > + /* Make sure new skb is properly aligned */ > + skb_align(new_skb, 4); > + > + /* 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; > +} > +#endif > + > static int fs_enet_start_xmit(struct sk_buff *skb, struct net_device *dev) > { > struct fs_enet_private *fep = netdev_priv(dev); > @@ -588,6 +619,19 @@ static int fs_enet_start_xmit(struct sk_buff *skb, struct net_device *dev) > u16 sc; > unsigned long flags; > > +#ifdef CONFIG_FS_ENET_MPC5121_FEC > + if (((unsigned long)skb->data) & 0x3) { > + 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. > + */ Could you define 'try to transmit later' ? Who is responsible to trigger this event ? > + return NETDEV_TX_BUSY; > + } > + } > +#endif > spin_lock_irqsave(&fep->tx_lock, flags); > > /* -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 17 Feb 2010 16:17:16 +0100 Eric Dumazet <eric.dumazet@gmail.com> wrote: > Le mercredi 17 février 2010 à 15:55 +0100, Anatolij Gustschin a écrit : ... > > +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 + 4); > > > ENET_RX_FRSIZE looks strange in TX path > > Why not using skb->len + 4 instead of ENET_RX_FRSIZE + 4 ? I will fix it. > > + if (!new_skb) { > > + dev_warn(fep->dev, "Memory squeeze, dropping tx packet.\n"); > > I am just wondering if this is ratelimited ? Right, it should be ratelimited, will fix it, too. ... > > static int fs_enet_start_xmit(struct sk_buff *skb, struct net_device *dev) > > { > > struct fs_enet_private *fep = netdev_priv(dev); > > @@ -588,6 +619,19 @@ static int fs_enet_start_xmit(struct sk_buff *skb, struct net_device *dev) > > u16 sc; > > unsigned long flags; > > > > +#ifdef CONFIG_FS_ENET_MPC5121_FEC > > + if (((unsigned long)skb->data) & 0x3) { > > + 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. > > + */ > > Could you define 'try to transmit later' ? > Who is responsible to trigger this event ? The function returns NETDEV_TX_BUSY here, skb is not consumed and will be requeued by sch_direct_xmit(), so it is scheduled for resending later. Thanks, Anatolij -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c index 4297021..166a89d 100644 --- a/drivers/net/fs_enet/fs_enet-main.c +++ b/drivers/net/fs_enet/fs_enet-main.c @@ -580,6 +580,37 @@ void fs_cleanup_bds(struct net_device *dev) /**********************************************************************************/ +#ifdef CONFIG_FS_ENET_MPC5121_FEC +/* + * MPC5121 FEC requeries 4-byte alignment for TX data buffer! + */ +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 + 4); + 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, 4); + + /* 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; +} +#endif + static int fs_enet_start_xmit(struct sk_buff *skb, struct net_device *dev) { struct fs_enet_private *fep = netdev_priv(dev); @@ -588,6 +619,19 @@ static int fs_enet_start_xmit(struct sk_buff *skb, struct net_device *dev) u16 sc; unsigned long flags; +#ifdef CONFIG_FS_ENET_MPC5121_FEC + if (((unsigned long)skb->data) & 0x3) { + 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; + } + } +#endif spin_lock_irqsave(&fep->tx_lock, flags); /*