> 1. you're using tabbed indentation in places, please use 2-space
> indents everywhere.
Sorry about that, probably it's because I've been using various
programming enviroments, I'll set it all too 2-space indent
> 2. some spanish seems to have crept into debug zlog's :)
Oops :-)
> 3. further, unconditional logging of debug information.
Ok, I'll fix that.
> 4. bgp_process_rsclient and bgp_process_main - there is a section of
> code replicated 3 times here which probably should be consolidated.
Thanks for the advice!
> looks good! and it includes peer-group support too, hadnt realised
> that. nice :)
Thanks! Yes, I forgot to talk about peer-group support when I posted the
patch, but I've tried to support groups as you suggested, because in some
cases sharing the same bgp_table between several peers should improve both
performance and memory-usage.
By the way, there are some issues about peer-groups configured as rsclients
that probably worth some discussion. For example when an announce is
considered for insertion into a bgp_table which belongs to some peer-group
and we apply the corresponding 'export' route-map, the semantics of
'match peer X:X::X:X' is not very clear. My solution has been to return
MATCH if X:X::X:X belongs to the peer-group for that address family,
what do you think of this approach?
Another issue that I forgot to mention when I posted the patch is that I
have slightly changed the behaviour of 'call <rm>'. Now after a 'call'
statement the
route-map continues normally, unless the called route-map returns
DENYMATCH. Before this change route-maps always finished
inmediatly after a 'call' statement, now there might be various 'call's in a
route-map-entry and also 'next' and 'goto' exit policies are taked into
account.
> regards,
Regards,
Jose
> indents everywhere.
Sorry about that, probably it's because I've been using various
programming enviroments, I'll set it all too 2-space indent
> 2. some spanish seems to have crept into debug zlog's :)
Oops :-)
> 3. further, unconditional logging of debug information.
Ok, I'll fix that.
> 4. bgp_process_rsclient and bgp_process_main - there is a section of
> code replicated 3 times here which probably should be consolidated.
Thanks for the advice!
> looks good! and it includes peer-group support too, hadnt realised
> that. nice :)
Thanks! Yes, I forgot to talk about peer-group support when I posted the
patch, but I've tried to support groups as you suggested, because in some
cases sharing the same bgp_table between several peers should improve both
performance and memory-usage.
By the way, there are some issues about peer-groups configured as rsclients
that probably worth some discussion. For example when an announce is
considered for insertion into a bgp_table which belongs to some peer-group
and we apply the corresponding 'export' route-map, the semantics of
'match peer X:X::X:X' is not very clear. My solution has been to return
MATCH if X:X::X:X belongs to the peer-group for that address family,
what do you think of this approach?
Another issue that I forgot to mention when I posted the patch is that I
have slightly changed the behaviour of 'call <rm>'. Now after a 'call'
statement the
route-map continues normally, unless the called route-map returns
DENYMATCH. Before this change route-maps always finished
inmediatly after a 'call' statement, now there might be various 'call's in a
route-map-entry and also 'next' and 'goto' exit policies are taked into
account.
> regards,
Regards,
Jose