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 <gr...@qrator.net> wrote:

> Hi,
>
> Could we continue with these modifcations?
>
> On Mon, Jan 3, 2022 at 2:10 PM Alexander Zubkov <gr...@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 <thoms...@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 <gr...@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 <
> thoms...@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 <gr...@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 <gr...@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 <
> thoms...@gmail.com> wrote:
> > >> >> > >
> > >> >> > > Hello,
> > >> >> > >
> > >> >> > > On Wed, Nov 28, 2018, 16:34 Alexander Zubkov <gr...@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 <
> gr...@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
> > >> >> > >
>
commit 89af0c12d4a2f40bc522b64c61daea6dac8ab3a5
Author: Alexander Zubkov <gr...@qrator.net>
Date:   Mon Jan 23 03:10:19 2023 +0100

    Keep protocol enabled states to configured file and restore them during reconfigure

diff --git a/conf/conf.c b/conf/conf.c
index 4e31de29..d4ea0ca2 100644
--- a/conf/conf.c
+++ b/conf/conf.c
@@ -287,6 +287,8 @@ config_do_commit(struct config *c, int type)
   if (old_config)
     old_config->obstacle_count++;
 
+  DBG("read_states\n");
+  protos_read_state(c);
   DBG("filter_commit\n");
   filter_commit(c, old_config);
   DBG("sysdep_commit\n");
diff --git a/conf/conf.h b/conf/conf.h
index b409750e..608e5c31 100644
--- a/conf/conf.h
+++ b/conf/conf.h
@@ -28,6 +28,7 @@ struct config {
 
   int mrtdump_file;			/* Configured MRTDump file (sysdep, fd in unix) */
   const char *syslog_name;		/* Name used for syslog (NULL -> no syslog) */
+  void *states_file;			/* Configured protocol states file (sysdep, FILE *) */
   struct rtable_config *def_tables[NET_MAX]; /* Default routing tables for each network */
   struct iface_patt *router_id_from;	/* Configured list of router ID iface patterns */
 
diff --git a/nest/proto.c b/nest/proto.c
index 885a0b75..93c75ae7 100644
--- a/nest/proto.c
+++ b/nest/proto.c
@@ -8,6 +8,8 @@
 
 #undef LOCAL_DEBUG
 
+#include <stdio.h>
+#include <unistd.h>
 #include "nest/bird.h"
 #include "nest/protocol.h"
 #include "lib/resource.h"
@@ -2091,6 +2093,55 @@ proto_cmd_show(struct proto *p, uintptr_t verbose, int cnt)
   }
 }
 
+void
+protos_read_state(struct config *c)
+{
+  FILE *f = c->states_file;
+  if (!f) return;
+
+  fseek(f, 0, SEEK_SET);
+  fflush(f);
+
+  while (1)
+  {
+    uint enabled;
+    char name[4096];
+    if (fscanf(f, "%4095s %d\n", name, &enabled) != 2)
+    {
+      if (!feof(f))
+	log(L_ERR "Error reading states file: %m");
+      break;
+    }
+
+    struct symbol *sym = cf_find_symbol(c, name);
+
+    if (sym && sym->class == SYM_PROTO)
+    {
+      struct proto_config *pc = sym->proto;
+      log(L_INFO "Applying state to protocol %s: %d", pc->name, enabled);
+      pc->disabled = !enabled;
+    }
+  }
+}
+
+void
+protos_write_state(struct proto *mp)
+{
+  FILE *f = mp->cf->global->states_file;
+  if (!f) return;
+
+  fseek(f, 0, SEEK_SET);
+  fflush(f);
+  ftruncate(fileno(f), 0);
+
+  struct proto *p;
+  WALK_LIST(p, proto_list)
+  {
+    fprintf(f, "%s %d\n", p->name, !p->disabled);
+  }
+  fflush(f);
+}
+
 void
 proto_cmd_disable(struct proto *p, uintptr_t arg, int cnt UNUSED)
 {
@@ -2105,6 +2156,7 @@ proto_cmd_disable(struct proto *p, uintptr_t arg, int cnt UNUSED)
   p->down_code = PDC_CMD_DISABLE;
   proto_set_message(p, (char *) arg, -1);
   proto_rethink_goal(p);
+  protos_write_state(p);
   cli_msg(-9, "%s: disabled", p->name);
 }
 
@@ -2121,6 +2173,7 @@ proto_cmd_enable(struct proto *p, uintptr_t arg, int cnt UNUSED)
   p->disabled = 0;
   proto_set_message(p, (char *) arg, -1);
   proto_rethink_goal(p);
+  protos_write_state(p);
   cli_msg(-11, "%s: enabled", p->name);
 }
 
diff --git a/nest/protocol.h b/nest/protocol.h
index fcbf0539..76fa5f09 100644
--- a/nest/protocol.h
+++ b/nest/protocol.h
@@ -90,6 +90,7 @@ void protos_preconfig(struct config *);
 void protos_commit(struct config *new, struct config *old, int force_restart, int type);
 struct proto * proto_spawn(struct proto_config *cf, uint disabled);
 void protos_dump_all(void);
+void protos_read_state(struct config *c);
 
 #define GA_UNKNOWN	0		/* Attribute not recognized */
 #define GA_NAME		1		/* Result = name */
diff --git a/sysdep/unix/config.Y b/sysdep/unix/config.Y
index 5c4b5bef..e99068cf 100644
--- a/sysdep/unix/config.Y
+++ b/sysdep/unix/config.Y
@@ -101,6 +101,20 @@ mrtdump_base:
  ;
 
 
+conf: keep_states ;
+
+keep_states: KEEP STATES text ';' {
+    if (!parse_and_exit)
+    {
+      if (new_config->states_file) cf_error("Redefine of states file");
+      struct rfile *f = rf_open(new_config->pool, $3, "a+");
+      if (!f) cf_error("Unable to open states file '%s': %m", $3);
+      new_config->states_file = rf_file(f);
+    }
+  }
+;
+
+
 conf: debug_unix ;
 
 debug_unix:

Reply via email to