On Tue, Jul 26, 2022 at 05:17:23PM +0200, Claudio Jeker wrote:
> On Tue, Jul 26, 2022 at 03:51:40PM +0200, Theo Buehler wrote:
> > On Tue, Jul 26, 2022 at 03:09:37PM +0200, Claudio Jeker wrote:
> > > This is another step in the epic kroute rework.
> > >
> > > Interfaces (kif) come with a list of kroutes attached to them which are
> > > only used to track the interface state and to fiddle with nexthop states.
> > > Now these lists are not really needed. One can just validate the nexthops
> > > without losing any relevant information.
> > >
> > > Struct knexthop gains a ifindex member which is used to track link state
> > > changes. As a benefit gateway routes will also pick up state changes now
> > > (connected routes already did that).
> > >
> > > With this F_DOWN becomes unused and I will remove this in another step.
> >
> > This generally reads fine. One question:
> >
> > > @@ -2097,31 +2048,23 @@ int
> > > kif_remove(struct kif_node *kif)
> > > {
> > > struct ktable *kt;
> > > - struct kif_kr *kkr;
> > > - struct kif_kr6 *kkr6;
> > > +
> > > + kif->flags &= ~IFF_UP;
> > > +
> > > + /*
> > > + * TODO, remove all kroutes using this interface,
> > > + * the kernel does this for us but better to do it
> > > + * here as well.
> > > + */
> > > +
> > > + if ((kt = ktable_get(kif->k.rdomain)) != NULL)
> > > + knexthop_track(kt, kif->k.ifindex);
> > >
> > > if (RB_REMOVE(kif_tree, &kit, kif) == NULL) {
> >
> > If I'm not mistaken, the only way RB_REMOVE() can return NULL is when
> > the third argument is NULL. So this check used to act as a NULL check
> > before dereferencing kif. This diff adds two derefs earlier, which will
> > now crash.
> >
> > Should kif_remove() in if_announce() only be called with non-NULL kif?
>
> I have never seen the log_warnx. So I assume it can not really happen.
> Maybe remove the if check and just adjust if_announce instead.
> I'll have a look and send another diff for that.
Fine with me.
ok
>
> > > log_warnx("RB_REMOVE(kif_tree, &kit, kif)");
> > > return (-1);
> > > }
>
>
> --
> :wq Claudio
