bgpd 'depend on' refactor

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

bgpd 'depend on' refactor

Claudio Jeker
To make bgpd more portable it is better to move all the link state code
into kroute.c. Especially for something as simple as a single up / down
flag.  This is doing that and removes the net/if_types.h include from
session.c. Result should be the same just computation is moved.

OK?
--
:wq Claudio

Index: bgpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
retrieving revision 1.368
diff -u -p -r1.368 bgpd.h
--- bgpd.h 14 Feb 2019 13:13:33 -0000 1.368
+++ bgpd.h 15 Feb 2019 10:40:59 -0000
@@ -638,6 +638,7 @@ struct kif {
  u_int8_t if_type;
  u_int8_t link_state;
  u_int8_t nh_reachable; /* for nexthop verification */
+ u_int8_t depend_state; /* for session depend on */
 };
 
 struct session_up {
Index: kroute.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
retrieving revision 1.230
diff -u -p -r1.230 kroute.c
--- kroute.c 11 Feb 2019 15:44:25 -0000 1.230
+++ kroute.c 15 Feb 2019 10:50:24 -0000
@@ -25,6 +25,7 @@
 #include <arpa/inet.h>
 #include <net/if.h>
 #include <net/if_dl.h>
+#include <net/if_types.h>
 #include <net/route.h>
 #include <netmpls/mpls.h>
 #include <err.h>
@@ -154,7 +155,6 @@ int kif_kr_remove(struct kroute_node
 int kif_kr6_insert(struct kroute6_node *);
 int kif_kr6_remove(struct kroute6_node *);
 
-int kif_validate(struct kif *);
 int kroute_validate(struct kroute *);
 int kroute6_validate(struct kroute6 *);
 void knexthop_validate(struct ktable *,
@@ -2235,7 +2235,7 @@ kif_kr6_remove(struct kroute6_node *kr)
  * nexthop validation
  */
 
-int
+static int
 kif_validate(struct kif *kif)
 {
  if (!(kif->flags & IFF_UP))
@@ -2253,6 +2253,26 @@ kif_validate(struct kif *kif)
  return (1);
 }
 
+/*
+ * return 1 when the interface is up and the link state is up or unknwown
+ * except when this is a carp interface, then return 1 only when link state
+ * is up
+ */
+static int
+kif_depend_state(struct kif *kif)
+{
+ if (!(kif->flags & IFF_UP))
+ return (0);
+
+
+ if (kif->if_type == IFT_CARP &&
+    kif->link_state == LINK_STATE_UNKNOWN)
+ return (0);
+
+ return LINK_STATE_IS_UP(kif->link_state);
+}
+
+
 int
 kroute_validate(struct kroute *kr)
 {
@@ -2654,6 +2674,7 @@ if_change(u_short ifindex, int flags, st
  kif->k.if_type = ifd->ifi_type;
  kif->k.rdomain = ifd->ifi_rdomain;
  kif->k.baudrate = ifd->ifi_baudrate;
+ kif->k.depend_state = kif_depend_state(&kif->k);
 
  send_imsg_session(IMSG_IFINFO, 0, &kif->k, sizeof(kif->k));
 
@@ -3255,6 +3276,7 @@ fetchifs(int ifindex)
  kif->k.rdomain = ifm.ifm_data.ifi_rdomain;
  kif->k.baudrate = ifm.ifm_data.ifi_baudrate;
  kif->k.nh_reachable = kif_validate(&kif->k);
+ kif->k.depend_state = kif_depend_state(&kif->k);
 
  if ((sa = rti_info[RTAX_IFP]) != NULL)
  if (sa->sa_family == AF_LINK) {
Index: session.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
retrieving revision 1.371
diff -u -p -r1.371 session.c
--- session.c 20 Jan 2019 23:27:48 -0000 1.371
+++ session.c 15 Feb 2019 10:52:13 -0000
@@ -24,7 +24,6 @@
 #include <sys/time.h>
 #include <sys/resource.h>
 #include <sys/un.h>
-#include <net/if_types.h>
 #include <netinet/in.h>
 #include <netinet/ip.h>
 #include <netinet/tcp.h>
@@ -96,7 +95,6 @@ void session_up(struct peer *);
 void session_down(struct peer *);
 int imsg_rde(int, u_int32_t, void *, u_int16_t);
 void session_demote(struct peer *, int);
-int session_link_state_is_up(int, int, int);
 
 int la_cmp(struct listen_addr *, struct listen_addr *);
 struct peer *getpeerbyip(struct sockaddr *);
@@ -2812,8 +2810,7 @@ session_dispatch_imsg(struct imsgbuf *ib
     sizeof(struct kif))
  fatalx("IFINFO imsg with wrong len");
  kif = imsg.data;
- depend_ok = session_link_state_is_up(kif->flags,
-    kif->if_type, kif->link_state);
+ depend_ok = kif->depend_state;
 
  for (p = peers; p != NULL; p = p->next)
  if (!strcmp(p->conf.if_depend, kif->ifname)) {
@@ -3319,23 +3316,4 @@ session_stop(struct peer *peer, u_int8_t
  break;
  }
  bgp_fsm(peer, EVNT_STOP);
-}
-
-/*
- * return 1 when the interface is up
- * and the link state is up or unknwown
- * except when this is a carp interface, then
- * return 1 only when link state is up
- */
-int
-session_link_state_is_up(int flags, int type, int link_state)
-{
- if (!(flags & IFF_UP))
- return (0);
-
- if (type == IFT_CARP &&
-    link_state == LINK_STATE_UNKNOWN)
- return (0);
-
- return LINK_STATE_IS_UP(link_state);
 }

Reply | Threaded
Open this post in threaded view
|

Re: bgpd 'depend on' refactor

Sebastian Benoit-3
ok

Claudio Jeker([hidden email]) on 2019.02.15 12:01:20 +0100:

> To make bgpd more portable it is better to move all the link state code
> into kroute.c. Especially for something as simple as a single up / down
> flag.  This is doing that and removes the net/if_types.h include from
> session.c. Result should be the same just computation is moved.
>
> OK?
> --
> :wq Claudio
>
> Index: bgpd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> retrieving revision 1.368
> diff -u -p -r1.368 bgpd.h
> --- bgpd.h 14 Feb 2019 13:13:33 -0000 1.368
> +++ bgpd.h 15 Feb 2019 10:40:59 -0000
> @@ -638,6 +638,7 @@ struct kif {
>   u_int8_t if_type;
>   u_int8_t link_state;
>   u_int8_t nh_reachable; /* for nexthop verification */
> + u_int8_t depend_state; /* for session depend on */
>  };
>  
>  struct session_up {
> Index: kroute.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
> retrieving revision 1.230
> diff -u -p -r1.230 kroute.c
> --- kroute.c 11 Feb 2019 15:44:25 -0000 1.230
> +++ kroute.c 15 Feb 2019 10:50:24 -0000
> @@ -25,6 +25,7 @@
>  #include <arpa/inet.h>
>  #include <net/if.h>
>  #include <net/if_dl.h>
> +#include <net/if_types.h>
>  #include <net/route.h>
>  #include <netmpls/mpls.h>
>  #include <err.h>
> @@ -154,7 +155,6 @@ int kif_kr_remove(struct kroute_node
>  int kif_kr6_insert(struct kroute6_node *);
>  int kif_kr6_remove(struct kroute6_node *);
>  
> -int kif_validate(struct kif *);
>  int kroute_validate(struct kroute *);
>  int kroute6_validate(struct kroute6 *);
>  void knexthop_validate(struct ktable *,
> @@ -2235,7 +2235,7 @@ kif_kr6_remove(struct kroute6_node *kr)
>   * nexthop validation
>   */
>  
> -int
> +static int
>  kif_validate(struct kif *kif)
>  {
>   if (!(kif->flags & IFF_UP))
> @@ -2253,6 +2253,26 @@ kif_validate(struct kif *kif)
>   return (1);
>  }
>  
> +/*
> + * return 1 when the interface is up and the link state is up or unknwown
> + * except when this is a carp interface, then return 1 only when link state
> + * is up
> + */
> +static int
> +kif_depend_state(struct kif *kif)
> +{
> + if (!(kif->flags & IFF_UP))
> + return (0);
> +
> +
> + if (kif->if_type == IFT_CARP &&
> +    kif->link_state == LINK_STATE_UNKNOWN)
> + return (0);
> +
> + return LINK_STATE_IS_UP(kif->link_state);
> +}
> +
> +
>  int
>  kroute_validate(struct kroute *kr)
>  {
> @@ -2654,6 +2674,7 @@ if_change(u_short ifindex, int flags, st
>   kif->k.if_type = ifd->ifi_type;
>   kif->k.rdomain = ifd->ifi_rdomain;
>   kif->k.baudrate = ifd->ifi_baudrate;
> + kif->k.depend_state = kif_depend_state(&kif->k);
>  
>   send_imsg_session(IMSG_IFINFO, 0, &kif->k, sizeof(kif->k));
>  
> @@ -3255,6 +3276,7 @@ fetchifs(int ifindex)
>   kif->k.rdomain = ifm.ifm_data.ifi_rdomain;
>   kif->k.baudrate = ifm.ifm_data.ifi_baudrate;
>   kif->k.nh_reachable = kif_validate(&kif->k);
> + kif->k.depend_state = kif_depend_state(&kif->k);
>  
>   if ((sa = rti_info[RTAX_IFP]) != NULL)
>   if (sa->sa_family == AF_LINK) {
> Index: session.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
> retrieving revision 1.371
> diff -u -p -r1.371 session.c
> --- session.c 20 Jan 2019 23:27:48 -0000 1.371
> +++ session.c 15 Feb 2019 10:52:13 -0000
> @@ -24,7 +24,6 @@
>  #include <sys/time.h>
>  #include <sys/resource.h>
>  #include <sys/un.h>
> -#include <net/if_types.h>
>  #include <netinet/in.h>
>  #include <netinet/ip.h>
>  #include <netinet/tcp.h>
> @@ -96,7 +95,6 @@ void session_up(struct peer *);
>  void session_down(struct peer *);
>  int imsg_rde(int, u_int32_t, void *, u_int16_t);
>  void session_demote(struct peer *, int);
> -int session_link_state_is_up(int, int, int);
>  
>  int la_cmp(struct listen_addr *, struct listen_addr *);
>  struct peer *getpeerbyip(struct sockaddr *);
> @@ -2812,8 +2810,7 @@ session_dispatch_imsg(struct imsgbuf *ib
>      sizeof(struct kif))
>   fatalx("IFINFO imsg with wrong len");
>   kif = imsg.data;
> - depend_ok = session_link_state_is_up(kif->flags,
> -    kif->if_type, kif->link_state);
> + depend_ok = kif->depend_state;
>  
>   for (p = peers; p != NULL; p = p->next)
>   if (!strcmp(p->conf.if_depend, kif->ifname)) {
> @@ -3319,23 +3316,4 @@ session_stop(struct peer *peer, u_int8_t
>   break;
>   }
>   bgp_fsm(peer, EVNT_STOP);
> -}
> -
> -/*
> - * return 1 when the interface is up
> - * and the link state is up or unknwown
> - * except when this is a carp interface, then
> - * return 1 only when link state is up
> - */
> -int
> -session_link_state_is_up(int flags, int type, int link_state)
> -{
> - if (!(flags & IFF_UP))
> - return (0);
> -
> - if (type == IFT_CARP &&
> -    link_state == LINK_STATE_UNKNOWN)
> - return (0);
> -
> - return LINK_STATE_IS_UP(link_state);
>  }
>