[PATCH] feature to keep protocol's state while configuring

[Thomás S. Bregolin]

Nice!

Ondrej,

I disagree somewhat with your first response, unless I misunderstand it. I
don't think it makes sense at all to persist a whole bunch of settings and
still do a soft reconfiguration.

There is a very good, non-remote use case to keep the session up on
reconfigurations regardless of the 'disabled' option on startup.

Would you be able to point us to the way forward?

Cheers,

Thomás

On Sun, Feb 10, 2019, 19:05 Alexander Zubkov <green at qrator.net wrote:

> Hi,
>
> I had almost the same idea in my original patch. But Ondrej have
> slightly better ideas regarding this.
>
> On Sun, Feb 10, 2019 at 6:59 PM Thomás S. Bregolin <thoms3rd at gmail.com>
> wrote:
> >
> > My attempt was a bit more crude:
> >
> > diff --git a/nest/config.Y b/nest/config.Y
> > index aef5ed46..829bf96c 100644
> > --- a/nest/config.Y
> > +++ b/nest/config.Y
> > @@ -64,7 +64,7 @@ proto_postconfig(void)
> >
> >  CF_DECLS
> >
> > -CF_KEYWORDS(ROUTER, ID, PROTOCOL, TEMPLATE, PREFERENCE, DISABLED,
> DEBUG, ALL, OFF, DIRECT)
> > +CF_KEYWORDS(ROUTER, ID, PROTOCOL, TEMPLATE, PREFERENCE, DISABLED,
> KEEP_STATE, DEBUG, ALL, OFF, DIRECT)
> >  CF_KEYWORDS(INTERFACE, IMPORT, EXPORT, FILTER, NONE, VRF, TABLE,
> STATES, ROUTES, FILTERS)
> >  CF_KEYWORDS(IPV4, IPV6, VPN4, VPN6, ROA4, ROA6, FLOW4, FLOW6, SADR,
> MPLS)
> >  CF_KEYWORDS(RECEIVE, LIMIT, ACTION, WARN, BLOCK, RESTART, DISABLE,
> KEEP, FILTERED)
> > @@ -205,6 +205,7 @@ proto_name:
> >  proto_item:
> >     /* EMPTY */
> >   | DISABLED bool { this_proto->disabled = $2; }
> > + | KEEP_STATE bool { this_proto->keep_state = $2 }
> >   | DEBUG debug_mask { this_proto->debug = $2; }
> >   | MRTDUMP mrtdump_mask { this_proto->mrtdump = $2; }
> >   | ROUTER ID idval { this_proto->router_id = $3; }
> > diff --git a/nest/proto.c b/nest/proto.c
> > index d4a333d0..397d6fb2 100644
> > --- a/nest/proto.c
> > +++ b/nest/proto.c
> > @@ -984,6 +984,8 @@ protos_commit(struct config *new, struct config
> *old, int force_reconfig, int ty
> >      if (! force_reconfig && proto_reconfigure(p, oc, nc, type))
> >        continue;
> >
> > +    nc->disabled = nc->keep_state ? p->disabled : nc->disabled;
> > +
> >      /* Unsuccessful, we will restart it */
> >      if (!p->disabled && !nc->disabled)
> >        log(L_INFO "Restarting protocol %s", p->name);
> > diff --git a/nest/protocol.h b/nest/protocol.h
> > index 6c04071b..d984b4c2 100644
> > --- a/nest/protocol.h
> > +++ b/nest/protocol.h
> > @@ -118,6 +118,7 @@ struct proto_config {
> >    int class;                /* SYM_PROTO or SYM_TEMPLATE */
> >    u8 net_type;                /* Protocol network type (NET_*), 0 for
> undefined */
> >    u8 disabled;                /* Protocol enabled/disabled by default */
> > +  u8 keep_state;            /* Keep current enabled/disabled state
> during reconfiguration */
> >    u32 debug, mrtdump;            /* Debugging bitfields, both use D_*
> constants */
> >    u32 router_id;            /* Protocol specific router ID */
> >
> >
> > On Tue, Dec 4, 2018 at 12:07 PM Alexander Zubkov <green at qrator.net>
> wrote:
> >>
> >> And implementation as a separate flag. But I'm not sure here how to
> >> add another parameter to configure command. What I could imagine -
> >> would add multiple numerous combinations and look terrible. And not
> >> sure yet with the naming so it is not too long and not too ambiguous:
> >> keep, keepstate, keeprun, ...?
> >> On Tue, Dec 4, 2018 at 12:21 PM Alexander Zubkov <green at qrator.net>
> wrote:
> >> >
> >> > The easiest patch would be to implement this behaviour for soft
> >> > reconfig. :) But that is not backward-compatible and might break
> >> > something for somebody. I'm also working on implementing it as
> >> > additional option.
> >> > On Mon, Dec 3, 2018 at 2:51 PM Thomás S. Bregolin <thoms3rd at gmail.com>
> wrote:
> >> > >
> >> > > Hello,
> >> > >
> >> > > On Wed, Nov 28, 2018, 16:34 Alexander Zubkov <green at qrator.net
> wrote:
> >> > >>
> >> > >> Hello,
> >> > >>
> >> > >> I have received no feedback on this suggestion and suppose it got
> >> > >> lost. I would be glad to hear some comments about this improvement.
> >> > >> On Fri, Nov 16, 2018 at 4:36 PM Alexander Zubkov <green at qrator.net>
> wrote:
> >> > >> >
> >> > >> > Hello.
> >> > >> >
> >> > >> > I have created a patch (attached) with new protocol option:
> disabled
> >> > >> > keep on|off. To keep the protocol's state while loading new
> config. It
> >> > >> > is useful when protocols disabled manually in the runtime, but
> we want
> >> > >> > to keep that state when loading new config. Patch is attached. I
> have
> >> > >> > made it against the current int-new branch.
> >> > >
> >> > >
> >> > > I will second this would be nice to have. I use bird with protocols
> set to disabled to keep them from coming up at the same time as the daemon,
> and have actually resorted to a wrapper that changes the configuration file
> in-place to "disabled no" before doing soft reconfiguration, and then back
> to "yes".
> >> > >
> >> > > Regards,
> >> > >
> >> > > Thomás
> >> > >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://trubka.network.cz/pipermail/bird-users/attachments/20190210/09151c4e/attachment.html>