Message ID | 4A130C0E.6000507@cosmosbay.com |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
On Tue, May 19, 2009 at 09:44:14PM +0200, Eric Dumazet wrote: > David Miller a écrit : > > From: Eric Dumazet <dada1@cosmosbay.com> > > Date: Tue, 12 May 2009 21:26:35 +0200 > > > >> [PATCH] net: release dst entry in dev_hard_start_xmit() > > ... > >> Signed-off-by: Eric Dumazet <dada1@cosmosbay.com> > > > > Applied, thanks Eric. > > > > Eric, please followup and double-check the pppoe paths > > that Jarek mentioned. I never saw that fully resolved. > > > > [PATCH] ppp: unset IFF_XMIT_DST_RELEASE in ppp_setup() > > Jarek pointed pppoe can call back dev_queue_xmit(), and might need > skb->dst, so its safer to unset IFF_XMIT_DST_RELEASE on ppp devices. Hmm... Of course, this patch looks OK to me, but actually my main concern was more general. We avoid adding such flags for each "real" dev, but if so IMHO it would be safer to generally add them to all "virtual" devs - needed or not. You prefer to do this only where necessary, but it's not always clear if it's omitted on purpose or by chance. So, now I'm wondering about xen-netfront - needlessly I hope ;-) Jarek P. > > Signed-off-by: Eric Dumazet <dada1@cosmosbay.com> > --- > diff --git a/drivers/net/ppp_generic.c b/drivers/net/ppp_generic.c > index 8ee9142..639d11b 100644 > --- a/drivers/net/ppp_generic.c > +++ b/drivers/net/ppp_generic.c > @@ -1054,6 +1054,7 @@ static void ppp_setup(struct net_device *dev) > dev->type = ARPHRD_PPP; > dev->flags = IFF_POINTOPOINT | IFF_NOARP | IFF_MULTICAST; > dev->features |= NETIF_F_NETNS_LOCAL; > + dev->priv_flags &= ~IFF_XMIT_DST_RELEASE; > } > > /* > -- 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
Jarek Poplawski a écrit : > On Tue, May 19, 2009 at 09:44:14PM +0200, Eric Dumazet wrote: >> David Miller a écrit : >>> From: Eric Dumazet <dada1@cosmosbay.com> >>> Date: Tue, 12 May 2009 21:26:35 +0200 >>> >>>> [PATCH] net: release dst entry in dev_hard_start_xmit() >>> ... >>>> Signed-off-by: Eric Dumazet <dada1@cosmosbay.com> >>> Applied, thanks Eric. >>> >>> Eric, please followup and double-check the pppoe paths >>> that Jarek mentioned. I never saw that fully resolved. >>> >> [PATCH] ppp: unset IFF_XMIT_DST_RELEASE in ppp_setup() >> >> Jarek pointed pppoe can call back dev_queue_xmit(), and might need >> skb->dst, so its safer to unset IFF_XMIT_DST_RELEASE on ppp devices. > > Hmm... Of course, this patch looks OK to me, but actually my main > concern was more general. We avoid adding such flags for each "real" > dev, but if so IMHO it would be safer to generally add them to all > "virtual" devs - needed or not. You prefer to do this only where > necessary, but it's not always clear if it's omitted on purpose or > by chance. So, now I'm wondering about xen-netfront - needlessly I > hope ;-) > This is the deal in fact, tracking all valid uses, and I'll check this. Another path would have to set the flag only for fast devices (Gb and 10Gb) -- 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: Jarek Poplawski <jarkao2@gmail.com> Date: Tue, 19 May 2009 23:09:37 +0200 > Hmm... Of course, this patch looks OK to me, but actually my main > concern was more general. We avoid adding such flags for each "real" > dev, but if so IMHO it would be safer to generally add them to all > "virtual" devs - needed or not. You prefer to do this only where > necessary, but it's not always clear if it's omitted on purpose or > by chance. So, now I'm wondering about xen-netfront - needlessly I > hope ;-) It is an issue that surely needs to be fleshed out, to make sure we use this where possible without breaking odd cases too easily. But for now I'm applying Eric's patch because it does fix something until we have these issues sorted more generally. -- 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/ppp_generic.c b/drivers/net/ppp_generic.c index 8ee9142..639d11b 100644 --- a/drivers/net/ppp_generic.c +++ b/drivers/net/ppp_generic.c @@ -1054,6 +1054,7 @@ static void ppp_setup(struct net_device *dev) dev->type = ARPHRD_PPP; dev->flags = IFF_POINTOPOINT | IFF_NOARP | IFF_MULTICAST; dev->features |= NETIF_F_NETNS_LOCAL; + dev->priv_flags &= ~IFF_XMIT_DST_RELEASE; } /*