[PATCH] feature to keep protocol's state while configuring
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.
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@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.
On Wed, Nov 28, 2018 at 04:30:02PM +0100, Alexander Zubkov 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.
Hello Sorry for not responding earlier. It seems to me that although the issue makes sense and deserves some solution, the approach in the patch is not the right one. There are other properties that could be changed in runtime (e.g. debug), possibly more in the future, and we do not want to make such options for all of them. IMHO more appropriate solution would be to have some option for the configure command that decides whether such changes in general should be kept or reset (analogously we have 'configure soft' for filters). -- Elen sila lumenn' omentielvo Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
Hello, Yes, sounds reasonable. But I worry configure command could be overabused with all that features, like: configue soft keepstate keepdebug keepsomethingelse :) May be some means to provide default configure variant could be useful... But lets leave it for futher generations for now. How this configure flag could be implemented? I see there are reconfigure type variable in code - sould it be extended to bitmask somehow? Or separate variable to keep this flag? I could try to provide some patch, but I'm not sure it would be easy for me to change parser's part in that case. It looks more complex than for adding new protocol option. On Thu, Nov 29, 2018 at 11:54 PM Ondrej Zajicek <santiago@crfreenet.org> wrote:
On Wed, Nov 28, 2018 at 04:30:02PM +0100, Alexander Zubkov 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.
Hello
Sorry for not responding earlier. It seems to me that although the issue makes sense and deserves some solution, the approach in the patch is not the right one. There are other properties that could be changed in runtime (e.g. debug), possibly more in the future, and we do not want to make such options for all of them. IMHO more appropriate solution would be to have some option for the configure command that decides whether such changes in general should be kept or reset (analogously we have 'configure soft' for filters).
-- Elen sila lumenn' omentielvo
Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
On Fri, Nov 30, 2018 at 09:30:36PM +0100, Alexander Zubkov wrote:
Hello,
Yes, sounds reasonable. But I worry configure command could be overabused with all that features, like: configue soft keepstate keepdebug keepsomethingelse :)
I would prefer one common keep flag for all cases where a running state deviates from config value due to a user changed the running state (e.g. enable/disable, debug, ...).
May be some means to provide default configure variant could be useful... But lets leave it for futher generations for now.
That makes sense.
How this configure flag could be implemented? I see there are reconfigure type variable in code - sould it be extended to bitmask somehow?
That is reasonable.
Or separate variable to keep this flag? I could try to provide some patch, but I'm not sure it would be easy for me to change parser's part in that case. It looks more complex than for adding new protocol option.
You could try it and if you hit a problem then you could just post a partial patch. -- Elen sila lumenn' omentielvo Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
Hello, On Wed, Nov 28, 2018, 16:34 Alexander Zubkov <green@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@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
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@gmail.com> wrote:
Hello,
On Wed, Nov 28, 2018, 16:34 Alexander Zubkov <green@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@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
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@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@gmail.com> wrote:
Hello,
On Wed, Nov 28, 2018, 16:34 Alexander Zubkov <green@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@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
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@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@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@gmail.com>
wrote:
Hello,
On Wed, Nov 28, 2018, 16:34 Alexander Zubkov <green@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@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
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@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@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@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@gmail.com> wrote:
Hello,
On Wed, Nov 28, 2018, 16:34 Alexander Zubkov <green@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@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
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@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@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@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@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@gmail.com>
wrote:
Hello,
On Wed, Nov 28, 2018, 16:34 Alexander Zubkov <green@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@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
Hi, Lets resurrect this one too. Please check the series of pathces in the attachment. 1) Convert reconfigure type variables to bitmask. 2) Add flag to keep running state 3) Add support in CLI Only the states of protocols are kept with this flag now. Anyone interested can add support of keeping other interesting runtime states, like Ondrej suggested. On Sun, Feb 10, 2019 at 11:23 PM Thomás S. Bregolin <thoms3rd@gmail.com> wrote:
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@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@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@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@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@gmail.com> wrote:
Hello,
On Wed, Nov 28, 2018, 16:34 Alexander Zubkov <green@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@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
Hi, Could we continue with these modifcations? On Mon, Jan 3, 2022 at 2:10 PM Alexander Zubkov <green@qrator.net> wrote:
Hi,
Lets resurrect this one too. Please check the series of pathces in the attachment. 1) Convert reconfigure type variables to bitmask. 2) Add flag to keep running state 3) Add support in CLI
Only the states of protocols are kept with this flag now. Anyone interested can add support of keeping other interesting runtime states, like Ondrej suggested.
On Sun, Feb 10, 2019 at 11:23 PM Thomás S. Bregolin <thoms3rd@gmail.com> wrote:
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@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@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@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@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@gmail.com> wrote: > > Hello, > > On Wed, Nov 28, 2018, 16:34 Alexander Zubkov <green@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@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 >
Hello, Not sure if those are forgotten or are unwanted modifications. Please let me know. And I also have another idea regarding the subject. The idea is to configure the file where bird will keep the states of the protocols (I mean enabled/disabled states). Then during loading the configuration it alters the states to reflect the saved ones. So the states are persistent during reconfiguration and also during restart of a daemon. The draft patch is attached. Please take a look. On Thu, Jun 30, 2022 at 3:44 PM Alexander Zubkov <green@qrator.net> wrote:
Hi,
Could we continue with these modifcations?
On Mon, Jan 3, 2022 at 2:10 PM Alexander Zubkov <green@qrator.net> wrote:
Hi,
Lets resurrect this one too. Please check the series of pathces in the attachment. 1) Convert reconfigure type variables to bitmask. 2) Add flag to keep running state 3) Add support in CLI
Only the states of protocols are kept with this flag now. Anyone interested can add support of keeping other interesting runtime states, like Ondrej suggested.
On Sun, Feb 10, 2019 at 11:23 PM Thomás S. Bregolin <thoms3rd@gmail.com>
wrote:
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@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@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@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@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@gmail.com> wrote: > > > > Hello, > > > > On Wed, Nov 28, 2018, 16:34 Alexander Zubkov <green@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@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 > >
On Mon, Jan 23, 2023 at 03:19:43AM +0100, Alexander Zubkov wrote:
Hello,
Not sure if those are forgotten or are unwanted modifications. Please let me know.
And I also have another idea regarding the subject. The idea is to configure the file where bird will keep the states of the protocols (I mean enabled/disabled states). Then during loading the configuration it alters the states to reflect the saved ones. So the states are persistent during reconfiguration and also during restart of a daemon. The draft patch is attached. Please take a look.
Hello Forgot about it, will check it. -- Elen sila lumenn' omentielvo Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
Updated the patch for keeping state in the file. Moved the read/write functions to sysdep/unix/main.c and made better parsing. So it is not a draft anymore, but something more or less "stable". I can add documentation patch in case there is interest to include that into upstream. On Tue, Jan 24, 2023 at 8:03 AM Ondrej Zajicek <santiago@crfreenet.org> wrote:
On Mon, Jan 23, 2023 at 03:19:43AM +0100, Alexander Zubkov wrote:
Hello,
Not sure if those are forgotten or are unwanted modifications. Please let me know.
And I also have another idea regarding the subject. The idea is to configure the file where bird will keep the states of the protocols (I mean enabled/disabled states). Then during loading the configuration it alters the states to reflect the saved ones. So the states are persistent during reconfiguration and also during restart of a daemon. The draft patch is attached. Please take a look.
Hello
Forgot about it, will check it.
-- Elen sila lumenn' omentielvo
Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
Further update. Do not write to the file the states of dynamic protocols as it does not have much sence. On Fri, Jan 27, 2023 at 2:53 AM Alexander Zubkov <green@qrator.net> wrote:
Updated the patch for keeping state in the file. Moved the read/write functions to sysdep/unix/main.c and made better parsing. So it is not a draft anymore, but something more or less "stable". I can add documentation patch in case there is interest to include that into upstream.
On Tue, Jan 24, 2023 at 8:03 AM Ondrej Zajicek <santiago@crfreenet.org> wrote:
On Mon, Jan 23, 2023 at 03:19:43AM +0100, Alexander Zubkov wrote:
Hello,
Not sure if those are forgotten or are unwanted modifications. Please let me know.
And I also have another idea regarding the subject. The idea is to configure the file where bird will keep the states of the protocols (I mean enabled/disabled states). Then during loading the configuration it alters the states to reflect the saved ones. So the states are persistent during reconfiguration and also during restart of a daemon. The draft patch is attached. Please take a look.
Hello
Forgot about it, will check it.
-- Elen sila lumenn' omentielvo
Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
participants (3)
-
Alexander Zubkov -
Ondrej Zajicek -
Thomás S. Bregolin