Subject: Re: [PATCH, v2] net: release dst entry in dev_hard_start_xmit()

From: Jarek Poplawski <[email-masked]>
Date: Tue, 19 May 2009 23:09:37 +0200

On Tue, May 19, 2009 at 09:44:14PM +0200, Eric Dumazet wrote:
> David Miller a écrit :
> > From: Eric Dumazet <[email-masked]>
> > Date: Tue, 12 May 2009 21:26:35 +0200
> > 
> >> [PATCH] net: release dst entry in dev_hard_start_xmit()
> >  ...
> >> Signed-off-by: Eric Dumazet <[email-masked]>
> > 
> > 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 <[email-masked]>
> ---
> 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;
>  }
>  
>  /*
>