| Message ID | 20100503151745.GA17997@Chamillionaire.breakpoint.cc |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Sebastian Andrzej Siewior <sebastian@breakpoint.cc> Date: Mon, 3 May 2010 17:17:45 +0200 > From: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > > The size for skb which is added to the recycled list is using the > current descriptor size which is current MTU. gfar_new_skb() is also > using this size. So after changing or alteast increasing the MTU all > recycled skbs should be dropped. > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > --- > I'm not 100% sure but it looks like it is wrong. It looks right to me, can I get an ACK from gianfar developers? -- 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 Mon, May 3, 2010 at 8:17 AM, Sebastian Andrzej Siewior <sebastian@breakpoint.cc> wrote: > From: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > > The size for skb which is added to the recycled list is using the > current descriptor size which is current MTU. gfar_new_skb() is also > using this size. So after changing or alteast increasing the MTU all > recycled skbs should be dropped. > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > --- > I'm not 100% sure but it looks like it is wrong. > > drivers/net/gianfar.c | 4 +++- > 1 files changed, 3 insertions(+), 1 deletions(-) > > diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c > index 5267c27..9093106 100644 > --- a/drivers/net/gianfar.c > +++ b/drivers/net/gianfar.c > @@ -2287,8 +2287,10 @@ static int gfar_change_mtu(struct net_device *dev, int new_mtu) > > /* Only stop and start the controller if it isn't already > * stopped, and we changed something */ > - if ((oldsize != tempsize) && (dev->flags & IFF_UP)) > + if ((oldsize != tempsize) && (dev->flags & IFF_UP)) { > stop_gfar(dev); > + skb_queue_purge(&priv->rx_recycle); > + } I think we should probably do this in free_skb_resources. And remove the call from gfar_close(). Andy -- 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
From: Andy Fleming <afleming@gmail.com> Date: Tue, 4 May 2010 08:29:06 -0700 > On Mon, May 3, 2010 at 8:17 AM, Sebastian Andrzej Siewior > <sebastian@breakpoint.cc> wrote: >> From: Sebastian Andrzej Siewior <bigeasy@linutronix.de> >> >> The size for skb which is added to the recycled list is using the >> current descriptor size which is current MTU. gfar_new_skb() is also >> using this size. So after changing or alteast increasing the MTU all >> recycled skbs should be dropped. >> >> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> >> --- >> I'm not 100% sure but it looks like it is wrong. >> >> drivers/net/gianfar.c | 4 +++- >> 1 files changed, 3 insertions(+), 1 deletions(-) >> >> diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c >> index 5267c27..9093106 100644 >> --- a/drivers/net/gianfar.c >> +++ b/drivers/net/gianfar.c >> @@ -2287,8 +2287,10 @@ static int gfar_change_mtu(struct net_device *dev, int new_mtu) >> >> /* Only stop and start the controller if it isn't already >> * stopped, and we changed something */ >> - if ((oldsize != tempsize) && (dev->flags & IFF_UP)) >> + if ((oldsize != tempsize) && (dev->flags & IFF_UP)) { >> stop_gfar(dev); >> + skb_queue_purge(&priv->rx_recycle); >> + } > > > I think we should probably do this in free_skb_resources. And remove > the call from gfar_close(). Ok, Sebastian please rework your patch as requested by Andy. Thanks. -- 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/gianfar.c b/drivers/net/gianfar.c index 5267c27..9093106 100644 --- a/drivers/net/gianfar.c +++ b/drivers/net/gianfar.c @@ -2287,8 +2287,10 @@ static int gfar_change_mtu(struct net_device *dev, int new_mtu) /* Only stop and start the controller if it isn't already * stopped, and we changed something */ - if ((oldsize != tempsize) && (dev->flags & IFF_UP)) + if ((oldsize != tempsize) && (dev->flags & IFF_UP)) { stop_gfar(dev); + skb_queue_purge(&priv->rx_recycle); + } priv->rx_buffer_size = tempsize;