rtalloc()/rtfree() problems on CURRENT

[Andre Oppermann]

Gleb Smirnoff wrote:
> 
>   Dear Andre,
> 
>   first, thanks for your feedback.
> 
> On Tue, Feb 24, 2004 at 11:14:59AM +0100, Andre Oppermann wrote:
> A> Most of the time, if you don't need a cloned route allocated, you can
> A> just use rtalloc_ign(&ro, RTF_CLONING) to just get a reference to the
> A> existing rtentry.  Not allocating a new cloned route saves work and
> A> time for the rtalloc code.
> 
> Thanks for this hint. So if using rtalloc_ign() I do not need to call rtfree()?

You still have to call rtfree() to decrement the refcount of the rtentry.

> Why does this situation is not clear in manpage? :)

I should update it.

> A> >         if (ro.ro_rt != NULL) {
> A> >                 struct rtentry *rt = ro.ro_rt;
> A> >
> A> >                 /* here some read-only things are done with rt */
> A>
> A> Maybe you do some nasty things to rt here?
> 
> Really, I don't :) Exact code is given at end of the mail.

You indeed do some nasty things.

> A> >                 rtfree(ro.ro_rt);
> A> >         }
> A> >
> A> > But on CURRENT rtfree() causes panic. Here is backtrace:
> A> ...
> A> > Does this mean that something is broken in CURRENT or I am doing something wrong?
> A>
> A> rtalloc() and rtfree() are used quite often in the network code and
> A> otherwise work fine.  The odds are high that you are doing something
> A> wrong.
> 
> Here is exact code:
> 
>         bzero((caddr_t)&ro, sizeof(ro));
>         sin = (struct sockaddr_in *)&ro.ro_dst;
>         sin->sin_len = sizeof(*sin);
>         sin->sin_family = AF_INET;
>         sin->sin_addr = fle->r.r_dst;
>         rtalloc(&ro);
>         if (ro.ro_rt != NULL) {
>                 struct rtentry *rt = ro.ro_rt;
> 
>                 /* This is cloned route, use its parent */
>                 if (ro.ro_rt->rt_flags & RTF_WASCLONED &&
>                     ro.ro_rt->rt_parent)
>                         rt = ro.ro_rt->rt_parent;

With rtalloc() you get a cloned route essentially every time.  Use
rtalloc_ign(&ro, RTF_CLONING) to get the parent route directly.

>                 fle->o_ifx = rt->rt_ifp->if_index;
> 
>                 if (rt->rt_flags & RTF_GATEWAY &&
>                     rt->rt_gateway->sa_family == AF_INET)
>                         fle->next_hop =
>                             ((struct sockaddr_in *)(rt->rt_gateway))->sin_addr;
> 
>                 if (rt_mask(rt))
>                         fle->dst_mask =
>                             bit_count(((struct sockaddr_in *)rt_mask(rt))->sin_addr.s_addr);
>                 else if (rt->rt_flags & RTF_HOST)
>                         /* Give up. We can't determine mask :( */
>                         fle->dst_mask = 32;
> 
>                 rtfree(ro.ro_rt);

Use the macro RTFREE() instead of rtfree(), it will take care of some
locking issues.

>         }

-- 
Andre