From 7fc55925beb06059759294e0e9b7bae45465395f Mon Sep 17 00:00:00 2001
From: "Ondrej Zajicek (work)" <santiago@crfreenet.org>
Date: Sun, 10 Dec 2017 00:55:34 +0100
Subject: [PATCH] Several minor fixes

---
 doc/bird.sgml       | 34 ++++++++++++++++++------------
 lib/net.h           |  7 +++++--
 nest/iface.c        |  4 ++--
 nest/rt-table.c     |  4 +++-
 proto/bgp/attrs.c   | 18 ++++++++++++++--
 proto/bgp/bgp.c     | 50 ++++++++++++++++++++++++---------------------
 proto/bgp/bgp.h     |  2 ++
 proto/bgp/packets.c |  8 ++++----
 proto/ospf/config.Y |  2 +-
 9 files changed, 81 insertions(+), 48 deletions(-)

diff --git a/doc/bird.sgml b/doc/bird.sgml
index c50d0cde6..3ec6612a9 100644
--- a/doc/bird.sgml
+++ b/doc/bird.sgml
@@ -2044,6 +2044,7 @@ avoid routing loops.
 <item> <rfc id="7911"> - Advertisement of Multiple Paths in BGP
 <item> <rfc id="7947"> - Internet Exchange BGP Route Server
 <item> <rfc id="8092"> - BGP Large Communities Attribute
+<item> <rfc id="8203"> - BGP Administrative Shutdown Communication
 </itemize>
 
 <sect1>Route selection rules
@@ -2258,6 +2259,20 @@ using the following configuration parameters:
 	related procedures. Note that even when disabled, BIRD can send route
 	refresh requests.  Default: on.
 
+	<tag><label id="bgp-graceful-restart">graceful restart <m/switch/|aware</tag>
+	When a BGP speaker restarts or crashes, neighbors will discard all
+	received paths from the speaker, which disrupts packet forwarding even
+	when the forwarding plane of the speaker remains intact. <rfc id="4724">
+	specifies an optional graceful restart mechanism to alleviate this
+	issue. This option controls the mechanism. It has three states:
+	Disabled, when no support is provided. Aware, when the graceful restart
+	support is announced and the support for restarting neighbors is
+	provided, but no local graceful restart is allowed (i.e. receiving-only
+	role). Enabled, when the full graceful restart support is provided
+	(i.e. both restarting and receiving role). Restarting role could be also
+	configured per-channel. Note that proper support for local graceful
+	restart requires also configuration of other protocols. Default: aware.
+
 	<tag><label id="bgp-graceful-restart-time">graceful restart time <m/number/</tag>
 	The restart time is announced in the BGP graceful restart capability
 	and specifies how long the neighbor would wait for the BGP session to
@@ -2487,19 +2502,12 @@ together with their appropriate channels follows.
 	TX direction. When active, all available routes accepted by the export
 	filter are advertised to the neighbor. Default: off.
 
-	<tag><label id="bgp-graceful-restart">graceful restart <m/switch/|aware</tag>
-	When a BGP speaker restarts or crashes, neighbors will discard all
-	received paths from the speaker, which disrupts packet forwarding even
-	when the forwarding plane of the speaker remains intact. <rfc
-	id="4724"> specifies an optional graceful restart mechanism to
-	alleviate this issue. This option controls the mechanism. It has three
-	states: Disabled, when no support is provided. Aware, when the graceful
-	restart support is announced and the support for restarting neighbors
-	is provided, but no local graceful restart is allowed (i.e.
-	receiving-only role). Enabled, when the full graceful restart
-	support is provided (i.e. both restarting and receiving role). Note
-	that proper support for local graceful restart requires also
-	configuration of other protocols.  Default: aware.
+	<tag><label id="bgp-graceful-restart-c">graceful restart <m/switch/</tag>
+	Although BGP graceful restart is configured mainly by protocol-wide
+	<ref id="bgp-graceful-restart" name="options">, it is possible to
+	configure restarting role per AFI/SAFI pair by this channel option.
+	The option is ignored if graceful restart is disabled by protocol-wide
+	option. Default: off in aware mode, on in full mode.
 </descrip>
 
 <sect1>Attributes
diff --git a/lib/net.h b/lib/net.h
index cae3a7498..2138f275d 100644
--- a/lib/net.h
+++ b/lib/net.h
@@ -230,11 +230,14 @@ static inline int net_type_match(const net_addr *a, u32 mask)
 static inline int net_is_ip(const net_addr *a)
 { return (a->type == NET_IP4) || (a->type == NET_IP6); }
 
+static inline int net_is_vpn(const net_addr *a)
+{ return (a->type == NET_VPN4) || (a->type == NET_VPN6); }
+
 static inline int net_is_roa(const net_addr *a)
 { return (a->type == NET_ROA4) || (a->type == NET_ROA6); }
 
-static inline int net_is_vpn(const net_addr *a)
-{ return (a->type == NET_VPN4) || (a->type == NET_VPN6); }
+static inline int net_is_flow(const net_addr *a)
+{ return (a->type == NET_FLOW4) || (a->type == NET_FLOW6); }
 
 
 static inline ip4_addr net4_prefix(const net_addr *a)
diff --git a/nest/iface.c b/nest/iface.c
index a876e9ff5..303a52b2f 100644
--- a/nest/iface.c
+++ b/nest/iface.c
@@ -849,7 +849,7 @@ if_show(void)
       else if (i->master_index)
 	bsprintf(mbuf, " master=#%u", i->master_index);
 
-      cli_msg(-1001, "%s %s (index=%d%s)", i->name, (i->flags & IF_UP) ? "Up" : "Down", i->index, mbuf);
+      cli_msg(-1001, "%s %s (index=%d%s)", i->name, (i->flags & IF_UP) ? "up" : "down", i->index, mbuf);
       if (!(i->flags & IF_MULTIACCESS))
 	type = "PtP";
       else
@@ -897,7 +897,7 @@ if_show_summary(void)
 	a6[0] = 0;
 
       cli_msg(-1005, "%-10s %-6s %-18s %s",
-	      i->name, (i->flags & IF_UP) ? "Up" : "Down", a4, a6);
+	      i->name, (i->flags & IF_UP) ? "up" : "down", a4, a6);
     }
   cli_msg(0, "");
 }
diff --git a/nest/rt-table.c b/nest/rt-table.c
index c42d3a978..b0dd6d3f2 100644
--- a/nest/rt-table.c
+++ b/nest/rt-table.c
@@ -897,7 +897,9 @@ rte_validate(rte *e)
     return 0;
   }
 
-  c = net_classify(n->n.addr);
+  /* FIXME: better handling different nettypes */
+  c = !net_is_flow(n->n.addr) ?
+    net_classify(n->n.addr): (IADDR_HOST | SCOPE_UNIVERSE);
   if ((c < 0) || !(c & IADDR_HOST) || ((c & IADDR_SCOPE_MASK) <= SCOPE_LINK))
   {
     log(L_WARN "Ignoring bogus route %N received via %s",
diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c
index 882ba44ea..dea3c4a6b 100644
--- a/proto/bgp/attrs.c
+++ b/proto/bgp/attrs.c
@@ -64,8 +64,6 @@
  * format - Optional hook that converts eattr to textual representation.
  */
 
-// XXXX review pool usage : c->c.proto->pool
-
 
 struct bgp_attr_desc {
   const char *name;
@@ -1175,6 +1173,22 @@ bgp_init_bucket_table(struct bgp_channel *c)
   c->withdraw_bucket = NULL;
 }
 
+void
+bgp_free_bucket_table(struct bgp_channel *c)
+{
+  HASH_FREE(c->bucket_hash);
+
+  struct bgp_bucket *b;
+  WALK_LIST_FIRST(b, c->bucket_queue)
+  {
+    rem_node(&b->send_node);
+    mb_free(b);
+  }
+
+  mb_free(c->withdraw_bucket);
+  c->withdraw_bucket = NULL;
+}
+
 static struct bgp_bucket *
 bgp_get_bucket(struct bgp_channel *c, ea_list *new)
 {
diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c
index f6d266397..705c5e917 100644
--- a/proto/bgp/bgp.c
+++ b/proto/bgp/bgp.c
@@ -98,6 +98,7 @@
  * <item> <rfc id="7911"> - Advertisement of Multiple Paths in BGP
  * <item> <rfc id="7947"> - Internet Exchange BGP Route Server
  * <item> <rfc id="8092"> - BGP Large Communities Attribute
+ * <item> <rfc id="8203"> - BGP Administrative Shutdown Communication
  * </itemize>
 */
 
@@ -601,10 +602,6 @@ bgp_conn_leave_established_state(struct bgp_proto *p)
   BGP_TRACE(D_EVENTS, "BGP session closed");
   p->conn = NULL;
 
-  // XXXX free these tables to avoid memory leak during graceful restart
-  // bgp_free_prefix_table(p);
-  // bgp_free_bucket_table(p);
-
   if (p->p.proto_state == PS_UP)
     bgp_stop(p, 0, NULL, 0);
 }
@@ -664,6 +661,10 @@ bgp_handle_graceful_restart(struct bgp_proto *p)
   struct bgp_channel *c;
   WALK_LIST(c, p->p.channels)
   {
+    /* FIXME: perhaps check for channel state instead of disabled flag? */
+    if (c->c.disabled)
+      continue;
+
     if (c->gr_ready)
     {
       if (c->gr_active)
@@ -679,6 +680,13 @@ bgp_handle_graceful_restart(struct bgp_proto *p)
       rt_refresh_begin(c->c.table, &c->c);
       rt_refresh_end(c->c.table, &c->c);
     }
+
+    /* Reset bucket and prefix tables */
+    bgp_free_bucket_table(c);
+    bgp_free_prefix_table(c);
+    bgp_init_bucket_table(c);
+    bgp_init_prefix_table(c);
+    c->packets_to_send = 0;
   }
 
   proto_notify_state(&p->p, PS_START);
@@ -1315,7 +1323,7 @@ bgp_start(struct proto *P)
   p->source_addr = p->cf->local_ip;
   p->link_addr = IPA_NONE;
 
-  /* XXXX */
+  /* Lock all channels when in GR recovery mode */
   if (p->p.gr_recovery && p->cf->gr_mode)
   {
     struct bgp_channel *c;
@@ -1546,10 +1554,9 @@ bgp_channel_shutdown(struct channel *C)
 {
   struct bgp_channel *c = (void *) C;
 
-  /* XXXX: cleanup bucket and prefix tables */
-
   c->next_hop_addr = IPA_NONE;
   c->link_addr = IPA_NONE;
+  c->packets_to_send = 0;
 }
 
 static void
@@ -2064,17 +2071,12 @@ bgp_show_proto_info(struct proto *P)
     bgp_show_capabilities(p, p->conn->local_caps);
     cli_msg(-1006, "    Neighbor capabilities");
     bgp_show_capabilities(p, p->conn->remote_caps);
-/* XXXX
-      cli_msg(-1006, "    Session:          %s%s%s%s%s%s%s%s",
-	      p->is_internal ? "internal" : "external",
-	      p->cf->multihop ? " multihop" : "",
-	      p->rr_client ? " route-reflector" : "",
-	      p->rs_client ? " route-server" : "",
-	      p->as4_session ? " AS4" : "",
-	      p->add_path_rx ? " add-path-rx" : "",
-	      p->add_path_tx ? " add-path-tx" : "",
-	      p->ext_messages ? " ext-messages" : "");
-*/
+    cli_msg(-1006, "    Session:          %s%s%s%s%s",
+	    p->is_internal ? "internal" : "external",
+	    p->cf->multihop ? " multihop" : "",
+	    p->rr_client ? " route-reflector" : "",
+	    p->rs_client ? " route-server" : "",
+	    p->as4_session ? " AS4" : "");
     cli_msg(-1006, "    Source address:   %I", p->source_addr);
     cli_msg(-1006, "    Hold timer:       %t/%u",
 	    tm_remains(p->conn->hold_timer), p->conn->hold_time);
@@ -2091,16 +2093,18 @@ bgp_show_proto_info(struct proto *P)
   }
 
   {
-    /* XXXX ?? */
     struct bgp_channel *c;
     WALK_LIST(c, p->p.channels)
     {
       channel_show_info(&c->c);
 
-      if (ipa_zero(c->link_addr))
-	cli_msg(-1006, "    BGP Next hop:   %I", c->next_hop_addr);
-      else
-	cli_msg(-1006, "    BGP Next hop:   %I %I", c->next_hop_addr, c->link_addr);
+      if (c->c.channel_state == CS_UP)
+      {
+	if (ipa_zero(c->link_addr))
+	  cli_msg(-1006, "    BGP Next hop:   %I", c->next_hop_addr);
+	else
+	  cli_msg(-1006, "    BGP Next hop:   %I %I", c->next_hop_addr, c->link_addr);
+      }
 
       if (c->igp_table_ip4)
 	cli_msg(-1006, "    IGP IPv4 table: %s", c->igp_table_ip4->name);
diff --git a/proto/bgp/bgp.h b/proto/bgp/bgp.h
index 40c4b3f0f..1310582b9 100644
--- a/proto/bgp/bgp.h
+++ b/proto/bgp/bgp.h
@@ -492,11 +492,13 @@ int bgp_encode_attrs(struct bgp_write_state *s, ea_list *attrs, byte *buf, byte
 ea_list * bgp_decode_attrs(struct bgp_parse_state *s, byte *data, uint len);
 
 void bgp_init_bucket_table(struct bgp_channel *c);
+void bgp_free_bucket_table(struct bgp_channel *c);
 void bgp_free_bucket(struct bgp_channel *c, struct bgp_bucket *b);
 void bgp_defer_bucket(struct bgp_channel *c, struct bgp_bucket *b);
 void bgp_withdraw_bucket(struct bgp_channel *c, struct bgp_bucket *b);
 
 void bgp_init_prefix_table(struct bgp_channel *c);
+void bgp_free_prefix_table(struct bgp_channel *c);
 void bgp_free_prefix(struct bgp_channel *c, struct bgp_prefix *bp);
 
 int bgp_rte_better(struct rte *, struct rte *);
diff --git a/proto/bgp/packets.c b/proto/bgp/packets.c
index aa69bdd68..ca9428804 100644
--- a/proto/bgp/packets.c
+++ b/proto/bgp/packets.c
@@ -1637,8 +1637,8 @@ bgp_decode_nlri_flow4(struct bgp_parse_state *s, byte *pos, uint len, rta *a)
     uint pxlen = data[1];
 
     // FIXME: Use some generic function
-    memcpy(&px, data, BYTES(pxlen));
-    px = ip4_and(px, ip4_mkmask(pxlen));
+    memcpy(&px, data+2, BYTES(pxlen));
+    px = ip4_and(ip4_ntoh(px), ip4_mkmask(pxlen));
 
     /* Prepare the flow */
     net_addr *n = alloca(sizeof(struct net_addr_flow4) + flen);
@@ -1729,8 +1729,8 @@ bgp_decode_nlri_flow6(struct bgp_parse_state *s, byte *pos, uint len, rta *a)
     uint pxlen = data[1];
 
     // FIXME: Use some generic function
-    memcpy(&px, data, BYTES(pxlen));
-    px = ip6_and(px, ip6_mkmask(pxlen));
+    memcpy(&px, data+2, BYTES(pxlen));
+    px = ip6_and(ip6_ntoh(px), ip6_mkmask(pxlen));
 
     /* Prepare the flow */
     net_addr *n = alloca(sizeof(struct net_addr_flow6) + flen);
diff --git a/proto/ospf/config.Y b/proto/ospf/config.Y
index 04d5e2e95..ce409a3a4 100644
--- a/proto/ospf/config.Y
+++ b/proto/ospf/config.Y
@@ -499,7 +499,7 @@ CF_ADDTO(dynamic_attr, OSPF_TAG { $$ = f_new_dynamic_attr(EAF_TYPE_INT | EAF_TEM
 CF_ADDTO(dynamic_attr, OSPF_ROUTER_ID { $$ = f_new_dynamic_attr(EAF_TYPE_ROUTER_ID | EAF_TEMP, T_QUAD, EA_OSPF_ROUTER_ID); })
 
 CF_CLI_HELP(SHOW OSPF, ..., [[Show information about OSPF protocol]]);
-CF_CLI(SHOW OSPF, optsym, [<name>], [[Show information about OSPF protocol XXX]])
+CF_CLI(SHOW OSPF, optsym, [<name>], [[Show information about OSPF protocol]])
 { ospf_sh(proto_get_named($3, &proto_ospf)); };
 
 CF_CLI(SHOW OSPF NEIGHBORS, optsym opttext, [<name>] [\"<interface>\"], [[Show information about OSPF neighbors]])
-- 
2.39.2