Eric Dumazet <eric.duma...@gmail.com> wrote:
> > -static inline void skb_dst_force(struct sk_buff *skb)
> > +static inline bool skb_dst_force(struct sk_buff *skb)
> > {
> > if (skb_dst_is_noref(skb)) {
> > struct dst_entry *dst = skb_dst(skb);
> > @@ -313,7 +314,10 @@ static inline void skb_dst_force(struct sk_buff *skb)
> > dst = NULL;
> >
> > skb->_skb_refdst = (unsigned long)dst;
> > + return dst != NULL;
> > }
> > +
> > + return true;
>
> This will return true, even if skb has a NULL dst.
Yes, that was intentional -- it should return false to
let caller know that no reference could be obtained and
that the dst was invalidated as a result.
> Say if we have two skb_dst_force() calls for some reason
> on the same skb, only the first one will return false.
What would you suggest instead?
Alternative is something like
if (skb_dst(skb)) {
skb_dst_force(skb);
if (!skb_dst(skb)) {
kfree_skb(skb);
goto err;
}
}
... i find this a bit ugly.
