On Thu, Jul 21, 2022 at 12:03:27PM +0200, Theo Buehler wrote:
> On Thu, Jul 21, 2022 at 11:10:41AM +0200, Claudio Jeker wrote:
> > On Wed, Jul 20, 2022 at 12:28:25PM +0200, Claudio Jeker wrote:
> > > On Wed, Jul 20, 2022 at 10:56:29AM +0200, Claudio Jeker wrote:
> > > > This diff moves kroute_insert to use struct kroute_full and do the
> > > > allocation for struct kroute / kroute6 inside kroute_insert. This 
> > > > removes
> > > > a lot of similar code all over kroute.c. While doing that also convert
> > > > kr_redistribute() to use struct kroute_full and kill the code 
> > > > duplication
> > > > there as well.
> > > > 
> > > > A lot more to clean up but that seems like a resonable step right now.
> > > 
> > > This diff is not ready yet. I missed that kr/kr6 are used after
> > > send_rtmsg(). Need to fix that first.
> > 
> > This version works now. I moved send_rtmsg() into kroute_insert() which is
> > not ideal but for no it allows us to move forward.
> 
> This looks good now.  In kr_redistribute, the scope_id is not copied in
> the AID_INET6 case. I assume that's not an issue since we won't try to
> redistribute link-local addresses.

It should actually be copied. Since this is a kroute_full and net.prefix
is a struct bgpd_addr the scope will be copied over. Now we don't
redistribute link-local addresses so it should not matter but I think the
kredist_compare function needs to also check the scope_id.
IPv6 the gift that keeps on giving ...
 
> Other than that, the only very important remark that I have is that
> kr4_change() and kroute_insert() now have two empty lines before the
> last return. I'd drop at least one of those.

Fixed.

> ok

Cheers
-- 
:wq Claudio

Reply via email to