On Sun, Mar 24 2019, Mitchell Krome <[email protected]> wrote:
> On 24/03/2019 7:23 am, Theo de Raadt wrote:
>> Sebastian Benoit <[email protected]> wrote:
>>
>>> Mitchell Krome([email protected]) 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.
ospf6d currently doesn't support config reloads at all. It might be
worth adding an XXX comment there.
>>> 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.
Your patch was mangled (long line wrapped) but the changes looked good.
Here's an updated version which tweaks punctuation and case (to match
the router-id keyword). Works for me in my simple test setup.
Comments/oks?
Index: ospfd.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/ospfd.c,v
retrieving revision 1.105
diff -u -p -r1.105 ospfd.c
--- ospfd.c 15 Jan 2019 22:18:10 -0000 1.105
+++ ospfd.c 25 Mar 2019 13:33:43 -0000
@@ -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, st
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;
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
