On Thu, Apr 6, 2017 at 3:49 AM, Eric Dumazet <eric.duma...@gmail.com> wrote:
> On Wed, 2017-04-05 at 15:33 -0700, Cong Wang wrote:
>
>> Good find! I missed the refcnt in rt_set_nexthop() before that commit.
>>
>> We need to revert that commit to restore the refcnt for fib_info.
>
> Well, there are other spots , in decnet and IPv6.
IPv6 is very different, it copies or steals the metrics from mx6_config:
static int fib6_commit_metrics(struct dst_entry *dst, struct mx6_config *mxc)
{
if (!mxc->mx)
return 0;
if (dst->flags & DST_HOST) {
u32 *mp = dst_metrics_write_ptr(dst);
if (unlikely(!mp))
return -ENOMEM;
fib6_copy_metrics(mp, mxc);
} else {
dst_init_metrics(dst, mxc->mx, false);
/* We've stolen mx now. */
mxc->mx = NULL;
}
return 0;
}
so probably doesn't need a refcnt.
Decnet has already done the refcnt'ing, see dn_fib_semantic_match().
>
> This is why my original mail stated the problem was in the calls to :
>
> dst_init_metrics(&rt->dst, fi->fib_metrics, true);
>
> Lets do not think in "reverting" spirit, but adding the missing bits.
>
> The problem here is that the metrics should not be freed until last user
> is gone.
>
> So maybe a refcount should be added to metrics, and we do not have to
> add a fib pointer again in all dsts.
>
Good point, but it is harder than just revert given that fact that dst metrics
is a magic pointer to an array and COW.
