On 24/03/2019 7:23 am, Theo de Raadt wrote:
> Sebastian Benoit <be...@openbsd.org> wrote:
>
>> Mitchell Krome(mitchellkr...@gmail.com) on 2019.03.23 20:27:17 +1000:
>>> Was messing around with ospf and got myself into a situation where the
>>> router ID's were the same on two boxes because I only did a reload on
>>> one of them when I changed the loopback IP's.
>>
>> Thats sub optimal i believe...
>>
>>> This adds a warning when reloading if the router ID changes (there was
>>> already a comment saying as much). Same patch can probably be applied to
>>> ospf6d if people think it's useful.
>>
>> I think it would be better to abort the reload if the router-id is changed,
>> i.e. not load the new config at all.
>
> That's the right approach in all our other daemons:
>
> if the configuration change cannot be installed correctly, consider
> it invalid and abort. Someone would need to write code to make it
> valid..
>
That makes sense. I checked the manuals for the routers I use at work
and they also required the ospf process to be restarted for the config
to take effect after changing the router id.
I moved the check up into ospf_reload because it doesn't make sense
sending the config to all the children if we know we're going to abort.
Mitchell
diff --git a/usr.sbin/ospfd/ospfd.c b/usr.sbin/ospfd/ospfd.c
index d01a2fa66..bc2d007a7 100644
--- a/usr.sbin/ospfd/ospfd.c
+++ b/usr.sbin/ospfd/ospfd.c
@@ -642,6 +642,13 @@ ospf_reload(void)
if ((xconf = parse_config(conffile, ospfd_conf->opts)) == NULL)
return (-1);
+ /* Abort the reload if rtr_id changed */
+ if (ospfd_conf->rtr_id.s_addr != xconf->rtr_id.s_addr) {
+ log_warnx("Router-ID changed in new configuration. This "
+ "requires ospfd to be restarted");
+ return (-1);
+ }
+
/* send config to childs */
if (ospf_sendboth(IMSG_RECONF_CONF, xconf, sizeof(*xconf)) == -1)
return (-1);
@@ -693,7 +700,6 @@ merge_config(struct ospfd_conf *conf, struct
ospfd_conf *xconf)
struct redistribute *r;
int rchange = 0;
- /* change of rtr_id needs a restart */
conf->flags = xconf->flags;
conf->spf_delay = xconf->spf_delay;
conf->spf_hold_time = xconf->spf_hold_time;
