[prev in list] [next in list] [prev in thread] [next in thread]
List: bird-users
Subject: Re: [PATCH] feature to keep protocol's state while configuring
From: Alexander Zubkov <green () qrator ! net>
Date: 2022-01-03 13:10:56
Message-ID: CABr+u0Y_89JK1AR52jA9A1v=F+DE-tH9u79ueBZBXw2tBLUF2g () mail ! gmail ! com
[Download RAW message or body]
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
> > > > > >
["0001-Conf-change-configure-type-variables-to-bitmask.patch" (text/x-patch)]
From 3cff33c09a171a245cfc892eb590833a4045e46e Mon Sep 17 00:00:00 2001
From: Alexander Zubkov <green@qrator.net>
Date: Mon, 3 Jan 2022 10:07:24 +0100
Subject: [PATCH 1/3] Conf: change configure type variables to bitmask
Change configure type variables to bitmask. So further reconfigure
options might be added later.
diff --git a/conf/conf.c b/conf/conf.c
index a2b01667..a2f808e5 100644
--- a/conf/conf.c
+++ b/conf/conf.c
@@ -64,7 +64,7 @@ struct config *config, *new_config;
static struct config *old_config; /* Old configuration */
static struct config *future_config; /* New config held here if recon requested \
during recon */
-static int old_cftype; /* Type of transition old_config -> config \
(RECONFIG_SOFT/HARD) */ +static int old_cftype; /* Type of transition old_config -> \
config (RECONFIG_DO,RECONFIG_SOFT) */ static int future_cftype; /* Type of \
scheduled transition, may also be RECONFIG_UNDO */ /* Note that when future_cftype \
is RECONFIG_UNDO, then future_config is NULL, therefore proper check for future \
scheduled config checks future_cftype */ @@ -249,7 +249,7 @@ global_commit(struct \
config *new, struct config *old) static int
config_do_commit(struct config *c, int type)
{
- if (type == RECONFIG_UNDO)
+ if (type & RECONFIG_UNDO)
{
c = old_config;
type = old_cftype;
@@ -313,7 +313,7 @@ config_done(void *unused UNUSED)
/**
* config_commit - commit a configuration
* @c: new configuration
- * @type: type of reconfiguration (RECONFIG_SOFT or RECONFIG_HARD)
+ * @type: type of reconfiguration (RECONFIG_DO_SOFT or RECONFIG_DO_HARD)
* @timeout: timeout for undo (in seconds; or 0 for no timeout)
*
* When a configuration is parsed and prepared for use, the
@@ -526,7 +526,7 @@ order_shutdown(int gr)
c->shutdown = 1;
c->gr_down = gr;
- config_commit(c, RECONFIG_HARD, 0);
+ config_commit(c, RECONFIG_DO_HARD, 0);
shutting_down = 1;
}
diff --git a/conf/conf.h b/conf/conf.h
index 3bc37959..61009b73 100644
--- a/conf/conf.h
+++ b/conf/conf.h
@@ -82,9 +82,12 @@ void config_del_obstacle(struct config *);
void order_shutdown(int gr);
#define RECONFIG_NONE 0
-#define RECONFIG_HARD 1
+#define RECONFIG_DO 1
#define RECONFIG_SOFT 2
-#define RECONFIG_UNDO 3
+#define RECONFIG_UNDO 4
+/* RECONFIG flag combinations */
+#define RECONFIG_DO_HARD RECONFIG_DO
+#define RECONFIG_DO_SOFT (RECONFIG_DO|RECONFIG_SOFT)
#define CONF_DONE 0
#define CONF_PROGRESS 1
diff --git a/nest/proto.c b/nest/proto.c
index 31ee1fa1..e28035ce 100644
--- a/nest/proto.c
+++ b/nest/proto.c
@@ -880,7 +880,7 @@ channel_reconfigure(struct channel *c, struct channel_config *cf)
}
}
- if (reconfigure_type == RECONFIG_SOFT)
+ if (reconfigure_type & RECONFIG_SOFT)
{
if (import_changed)
log(L_INFO "Channel %s.%s changed import", c->proto->name, c->name);
@@ -1215,7 +1215,7 @@ proto_reconfigure(struct proto *p, struct proto_config *oc, \
struct proto_config
* @old: old configuration or %NULL if it's boot time config
* @force_reconfig: force restart of all protocols (used for example
* when the router ID changes)
- * @type: type of reconfiguration (RECONFIG_SOFT or RECONFIG_HARD)
+ * @type: type of reconfiguration (RECONFIG_DO_SOFT or RECONFIG_DO_HARD)
*
* Scan differences between @old and @new configuration and adjust all
* protocol instances to conform to the new configuration.
@@ -1231,7 +1231,7 @@ proto_reconfigure(struct proto *p, struct proto_config *oc, \
struct proto_config
* When a protocol exists in both configurations, the core decides
* whether it's possible to reconfigure it dynamically - it checks all
* the core properties of the protocol (changes in filters are ignored
- * if type is RECONFIG_SOFT) and if they match, it asks the
+ * if flag RECONFIG_SOFT is set) and if they match, it asks the
* reconfigure() hook of the protocol to see if the protocol is able
* to switch to the new configuration. If it isn't possible, the
* protocol is shut down and a new instance is started with the new
diff --git a/sysdep/unix/config.Y b/sysdep/unix/config.Y
index 5c4b5bef..f8d6b036 100644
--- a/sysdep/unix/config.Y
+++ b/sysdep/unix/config.Y
@@ -116,10 +116,10 @@ debug_unix:
CF_CLI_HELP(CONFIGURE, ..., [[Reload configuration]])
CF_CLI(CONFIGURE, cfg_name cfg_timeout, [\"<file>\"] [timeout [<sec>]], [[Reload \
configuration]])
-{ cmd_reconfig($2, RECONFIG_HARD, $3); } ;
+{ cmd_reconfig($2, RECONFIG_DO_HARD, $3); } ;
CF_CLI(CONFIGURE SOFT, cfg_name cfg_timeout, [\"<file>\"] [timeout [<sec>]], \
[[Reload configuration and ignore changes in filters]])
-{ cmd_reconfig($3, RECONFIG_SOFT, $4); } ;
+{ cmd_reconfig($3, RECONFIG_DO_SOFT, $4); } ;
/* Hack to get input completion for 'timeout' */
CF_CLI_CMD(CONFIGURE TIMEOUT, [<sec>], [[Reload configuration with undo timeout]])
diff --git a/sysdep/unix/main.c b/sysdep/unix/main.c
index 392aff9d..d82058a3 100644
--- a/sysdep/unix/main.c
+++ b/sysdep/unix/main.c
@@ -252,7 +252,7 @@ async_config(void)
config_free(conf);
}
else
- config_commit(conf, RECONFIG_HARD, 0);
+ config_commit(conf, RECONFIG_DO_HARD, 0);
}
static struct config *
@@ -926,7 +926,7 @@ main(int argc, char **argv)
signal_init();
- config_commit(conf, RECONFIG_HARD, 0);
+ config_commit(conf, RECONFIG_DO_HARD, 0);
graceful_restart_init();
diff --git a/test/bt-utils.c b/test/bt-utils.c
index cbca3a6b..b909d2e9 100644
--- a/test/bt-utils.c
+++ b/test/bt-utils.c
@@ -159,7 +159,7 @@ bt_config_parse__(struct config *cfg)
return NULL;
}
- config_commit(cfg, RECONFIG_HARD, 0);
+ config_commit(cfg, RECONFIG_DO_HARD, 0);
new_config = cfg;
return cfg;
--
2.34.0
["0002-Conf-add-RECONFIG_KEEP-flag-to-keep-runtime-states-d.patch" (text/x-patch)]
From a953535b1820ec6f738652c30ad0b9e4a74faf60 Mon Sep 17 00:00:00 2001
From: Alexander Zubkov <green@qrator.net>
Date: Mon, 3 Jan 2022 12:16:09 +0100
Subject: [PATCH 2/3] Conf: add RECONFIG_KEEP flag to keep runtime states
during reconfig
diff --git a/conf/conf.h b/conf/conf.h
index 61009b73..9f5010c1 100644
--- a/conf/conf.h
+++ b/conf/conf.h
@@ -83,8 +83,9 @@ void order_shutdown(int gr);
#define RECONFIG_NONE 0
#define RECONFIG_DO 1
-#define RECONFIG_SOFT 2
+#define RECONFIG_SOFT 2 /* do not restart protocols when filters changed */
#define RECONFIG_UNDO 4
+#define RECONFIG_KEEP 8 /* keep runtime states of protocols etc. */
/* RECONFIG flag combinations */
#define RECONFIG_DO_HARD RECONFIG_DO
#define RECONFIG_DO_SOFT (RECONFIG_DO|RECONFIG_SOFT)
diff --git a/nest/proto.c b/nest/proto.c
index e28035ce..2bac5698 100644
--- a/nest/proto.c
+++ b/nest/proto.c
@@ -1278,6 +1278,13 @@ protos_commit(struct config *new, struct config *old, int force_reconfig, int ty
nc = sym->proto;
nc->proto = p;
+ /* Keep protocol state if RECONFIG_KEEP flag is set */
+ if (type & RECONFIG_KEEP && (p->disabled != nc->disabled))
+ {
+ log(L_INFO "Keeping state of protocol %s", p->name);
+ nc->disabled = p->disabled;
+ }
+
/* We will try to reconfigure protocol p */
if (! force_reconfig && proto_reconfigure(p, oc, nc, type))
continue;
--
2.34.0
["0003-CLI-add-keep-option-to-configure-command.patch" (text/x-patch)]
From 8c241675de8bfdb5a97350894f791b65912ddd60 Mon Sep 17 00:00:00 2001
From: Alexander Zubkov <green@qrator.net>
Date: Mon, 3 Jan 2022 13:48:29 +0100
Subject: [PATCH 3/3] CLI: add "keep" option to "configure" command
Parse "configure" command parameters like permutable options and add
"keep" option to set RECONFIG_KEEP flag for reconfiguration.
diff --git a/sysdep/unix/config.Y b/sysdep/unix/config.Y
index f8d6b036..67efd75f 100644
--- a/sysdep/unix/config.Y
+++ b/sysdep/unix/config.Y
@@ -14,14 +14,15 @@ CF_HDR
CF_DEFINES
static struct log_config *this_log;
+static struct reconfig_config *this_rc;
CF_DECLS
CF_KEYWORDS(LOG, SYSLOG, ALL, DEBUG, TRACE, INFO, REMOTE, WARNING, ERROR, AUTH, \
FATAL, BUG, STDERR, SOFT) CF_KEYWORDS(NAME, CONFIRM, UNDO, CHECK, TIMEOUT, DEBUG, \
LATENCY, LIMIT, WATCHDOG, WARNING, STATUS)
-CF_KEYWORDS(GRACEFUL, RESTART)
+CF_KEYWORDS(GRACEFUL, RESTART, KEEP)
-%type <i> log_mask log_mask_list log_cat cfg_timeout
+%type <i> log_mask log_mask_list log_cat
%type <t> cfg_name
%type <tf> timeformat_which
%type <t> syslog_name
@@ -115,16 +116,22 @@ debug_unix:
CF_CLI_HELP(CONFIGURE, ..., [[Reload configuration]])
-CF_CLI(CONFIGURE, cfg_name cfg_timeout, [\"<file>\"] [timeout [<sec>]], [[Reload \
configuration]])
-{ cmd_reconfig($2, RECONFIG_DO_HARD, $3); } ;
+CF_CLI(CONFIGURE, cfg_opts, [soft] [keep] [\"<file>\"] [timeout [<sec>]], [[Reload \
configuration]]) +{ cmd_reconfig(this_rc->name, this_rc->type|RECONFIG_DO, \
this_rc->timeout); } ;
-CF_CLI(CONFIGURE SOFT, cfg_name cfg_timeout, [\"<file>\"] [timeout [<sec>]], \
[[Reload configuration and ignore changes in filters]])
-{ cmd_reconfig($3, RECONFIG_DO_SOFT, $4); } ;
-
-/* Hack to get input completion for 'timeout' */
+/* Hack to get input completion */
+CF_CLI_CMD(CONFIGURE SOFT,, [[Reload configuration and ignore changes in filters]])
+CF_CLI_CMD(CONFIGURE KEEP,, [[Reload configuration and keep runtime states]])
CF_CLI_CMD(CONFIGURE TIMEOUT, [<sec>], [[Reload configuration with undo timeout]])
+
+CF_CLI_CMD(CONFIGURE KEEP SOFT,,,)
+CF_CLI_CMD(CONFIGURE SOFT KEEP,,,)
+CF_CLI_CMD(CONFIGURE KEEP TIMEOUT, [<sec>], [[Reload configuration with undo \
timeout]]) CF_CLI_CMD(CONFIGURE SOFT TIMEOUT, [<sec>], [[Reload configuration with \
undo timeout]]) +CF_CLI_CMD(CONFIGURE KEEP SOFT TIMEOUT, [<sec>], [[Reload \
configuration with undo timeout]]) +CF_CLI_CMD(CONFIGURE SOFT KEEP TIMEOUT, [<sec>], \
[[Reload configuration with undo timeout]])
+/* Other configure commands */
CF_CLI(CONFIGURE CONFIRM,,, [[Confirm last configuration change - deactivate undo \
timeout]]) { cmd_reconfig_confirm(); } ;
@@ -145,18 +152,42 @@ CF_CLI_HELP(GRACEFUL, restart, [[Shut the daemon down for \
graceful restart]]) CF_CLI(GRACEFUL RESTART,,, [[Shut the daemon down for graceful \
restart]]) { cmd_graceful_restart(); } ;
+cfg_opts:
+ /* empty */ {
+ this_rc = cfg_allocz(sizeof(struct reconfig_config));
+ }
+ | cfg_opts SOFT {
+ if (this_rc->type & RECONFIG_SOFT)
+ cf_error("Only one soft flag expected");
+ this_rc->type |= RECONFIG_SOFT;
+ }
+ | cfg_opts KEEP {
+ if (this_rc->type & RECONFIG_KEEP)
+ cf_error("Only one soft keep expected");
+ this_rc->type |= RECONFIG_KEEP;
+ }
+ | cfg_opts TEXT {
+ if (this_rc->name)
+ cf_error("Only one config file expected");
+ this_rc->name = $2;
+ }
+ | cfg_opts TIMEOUT {
+ if (this_rc->timeout)
+ cf_error("Only one timeout expected");
+ this_rc->timeout = UNIX_DEFAULT_CONFIGURE_TIMEOUT;
+ }
+ | cfg_opts TIMEOUT expr {
+ if (this_rc->timeout)
+ cf_error("Only one timeout expected");
+ this_rc->timeout = $3;
+ }
+ ;
cfg_name:
/* empty */ { $$ = NULL; }
| TEXT
;
-cfg_timeout:
- /* empty */ { $$ = 0; }
- | TIMEOUT { $$ = UNIX_DEFAULT_CONFIGURE_TIMEOUT; }
- | TIMEOUT expr { $$ = $2; }
- ;
-
CF_CODE
CF_END
diff --git a/sysdep/unix/unix.h b/sysdep/unix/unix.h
index ad85d1ea..3af3ff52 100644
--- a/sysdep/unix/unix.h
+++ b/sysdep/unix/unix.h
@@ -38,6 +38,12 @@ void cmd_graceful_restart(void);
#define UNIX_DEFAULT_LATENCY_LIMIT (1 S_)
#define UNIX_DEFAULT_WATCHDOG_WARNING (5 S_)
+struct reconfig_config {
+ uint timeout; /* Reconfig timeout */
+ int type; /* Reconfig type (RECONFIG_SOFT,RECONFIG_KEEP,...) */
+ const char *name; /* Config file name */
+};
+
/* io.c */
#define ERR(c) do { s->err = c; return -1; } while (0)
--
2.34.0
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic