ospfd: type p2p

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

ospfd: type p2p

Remi Locherer
Hi tech@,

earlier this year I sent a diff that allowed to change an interface
from broadcast to point-to-point.

https://marc.info/?l=openbsd-tech&m=156132923203704&w=2

It turned out that this was not sufficient. It made the adjacency
come up in p2p mode (no selection of DR or BDR) but didn't set a valid
next hop for routes learned over this p2p link. Actually the next hop was
0.0.0.0 which was never installed into the routing table.

This is because for P2P interfaces the neighbor address is not taken from
the received hello but from the "destination" parameter configured on the
interface. Since this is not set on a broadcast interface the address is
0.0.0.0.

My new diff changes this. Now also for P2P links the IP address of the
neighbor is taken from the hello packets (src address). This on it's own
would make it simpler to interfere with the routing from remote. One could
send unicast ospf hello messages and potentially disrupt the routing setup.
I believe I mitigated this with an additional check I committed in August:
only hello messages sent to the multicast address are now processed.

The config looks like this:

area 0.0.0.0 {
        interface em0 {
                type p2p
        }
}

It would be nice to get test reports for this new feature (check the fib
and routing table!) and also test reports with real p2p2 interfaces (gif
or gre).

Of course OKs are also welcome. ;-)

Remi



Index: hello.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/hello.c,v
retrieving revision 1.24
diff -u -p -r1.24 hello.c
--- hello.c 12 Aug 2019 20:21:58 -0000 1.24
+++ hello.c 21 Sep 2019 22:06:17 -0000
@@ -189,14 +189,13 @@ recv_hello(struct iface *iface, struct i
  nbr->dr.s_addr = hello.d_rtr;
  nbr->bdr.s_addr = hello.bd_rtr;
  nbr->priority = hello.rtr_priority;
- /* XXX neighbor address shouldn't be stored on virtual links */
- nbr->addr.s_addr = src.s_addr;
+ nbr_update_addr(nbr->peerid, src);
  }
 
  if (nbr->addr.s_addr != src.s_addr) {
  log_warnx("%s: neighbor ID %s changed its IP address",
     __func__, inet_ntoa(nbr->id));
- nbr->addr.s_addr = src.s_addr;
+ nbr_update_addr(nbr->peerid, src);
  }
 
  nbr->options = hello.opts;
Index: lsupdate.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/lsupdate.c,v
retrieving revision 1.46
diff -u -p -r1.46 lsupdate.c
--- lsupdate.c 15 Jul 2019 18:26:39 -0000 1.46
+++ lsupdate.c 15 Aug 2019 21:10:13 -0000
@@ -470,7 +470,7 @@ ls_retrans_timer(int fd, short event, vo
  /* ls_retrans_list_free retriggers the timer */
  return;
  } else if (nbr->iface->type == IF_TYPE_POINTOPOINT)
- memcpy(&addr, &nbr->iface->dst, sizeof(addr));
+ memcpy(&addr, &nbr->addr, sizeof(addr));
  else
  inet_aton(AllDRouters, &addr);
  } else
Index: neighbor.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/neighbor.c,v
retrieving revision 1.48
diff -u -p -r1.48 neighbor.c
--- neighbor.c 9 Feb 2018 02:14:03 -0000 1.48
+++ neighbor.c 21 Sep 2019 15:28:43 -0000
@@ -312,6 +312,7 @@ nbr_new(u_int32_t nbr_id, struct iface *
  bzero(&rn, sizeof(rn));
  rn.id.s_addr = nbr->id.s_addr;
  rn.area_id.s_addr = nbr->iface->area->id.s_addr;
+ rn.addr.s_addr = nbr->addr.s_addr;
  rn.ifindex = nbr->iface->ifindex;
  rn.state = nbr->state;
  rn.self = self;
@@ -347,6 +348,23 @@ nbr_del(struct nbr *nbr)
  LIST_REMOVE(nbr, hash);
 
  free(nbr);
+}
+
+int
+nbr_update_addr(u_int32_t peerid, struct in_addr addr) {
+
+ struct nbr *nbr = NULL;
+
+ nbr = nbr_find_peerid(peerid);
+ if (nbr == NULL)
+ return (1);
+
+ /* XXX neighbor address shouldn't be stored on virtual links */
+ nbr->addr.s_addr = addr.s_addr;
+ ospfe_imsg_compose_rde(IMSG_NEIGHBOR_ADDR, peerid, 0, &addr,
+    sizeof(addr));
+
+ return (0);
 }
 
 struct nbr *
Index: ospfd.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/ospfd.c,v
retrieving revision 1.108
diff -u -p -r1.108 ospfd.c
--- ospfd.c 16 May 2019 05:49:22 -0000 1.108
+++ ospfd.c 23 Jun 2019 21:06:44 -0000
@@ -911,6 +911,22 @@ merge_interfaces(struct area *a, struct
  if_fsm(i, IF_EVT_UP);
  }
 
+ if (i->p2p != xi->p2p) {
+ /* re-add interface to enable or disable DR election */
+ if (ospfd_process == PROC_OSPF_ENGINE)
+ if_fsm(i, IF_EVT_DOWN);
+ else if (ospfd_process == PROC_RDE_ENGINE)
+ rde_nbr_iface_del(i);
+ LIST_REMOVE(i, entry);
+ if_del(i);
+ LIST_REMOVE(xi, entry);
+ LIST_INSERT_HEAD(&a->iface_list, xi, entry);
+ xi->area = a;
+ if (ospfd_process == PROC_OSPF_ENGINE)
+ xi->state = IF_STA_NEW;
+ continue;
+ }
+
  strlcpy(i->dependon, xi->dependon,
         sizeof(i->dependon));
  i->depend_ok = xi->depend_ok;
Index: ospfd.conf.5
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/ospfd.conf.5,v
retrieving revision 1.57
diff -u -p -r1.57 ospfd.conf.5
--- ospfd.conf.5 10 Jun 2019 06:07:15 -0000 1.57
+++ ospfd.conf.5 23 Jun 2019 22:10:32 -0000
@@ -419,6 +419,9 @@ Router.
 .It Ic transmit-delay Ar seconds
 Set the transmit delay.
 The default value is 1; valid range is 1\-3600 seconds.
+.It Ic type p2p
+Set the interface type to point to point.
+This disables the election of a DR and BDR for the given interface.
 .El
 .Sh FILES
 .Bl -tag -width "/etc/ospfd.conf" -compact
Index: ospfd.h
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/ospfd.h,v
retrieving revision 1.104
diff -u -p -r1.104 ospfd.h
--- ospfd.h 16 May 2019 05:49:22 -0000 1.104
+++ ospfd.h 21 Sep 2019 10:25:32 -0000
@@ -107,6 +107,7 @@ enum imsg_type {
  IMSG_IFINFO,
  IMSG_NEIGHBOR_UP,
  IMSG_NEIGHBOR_DOWN,
+ IMSG_NEIGHBOR_ADDR,
  IMSG_NEIGHBOR_CHANGE,
  IMSG_NEIGHBOR_CAPA,
  IMSG_NETWORK_ADD,
@@ -363,6 +364,7 @@ struct iface {
  u_int8_t linkstate;
  u_int8_t priority;
  u_int8_t passive;
+ u_int8_t p2p;
 };
 
 struct ifaddrchange {
Index: ospfe.h
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/ospfe.h,v
retrieving revision 1.46
diff -u -p -r1.46 ospfe.h
--- ospfe.h 25 Oct 2014 03:23:49 -0000 1.46
+++ ospfe.h 21 Sep 2019 15:27:08 -0000
@@ -204,6 +204,7 @@ void lsa_cache_put(struct lsa_ref *, s
 void nbr_init(u_int32_t);
 struct nbr *nbr_new(u_int32_t, struct iface *, int);
 void nbr_del(struct nbr *);
+int nbr_update_addr(u_int32_t, struct in_addr);
 
 struct nbr *nbr_find_id(struct iface *, u_int32_t);
 struct nbr *nbr_find_peerid(u_int32_t);
Index: parse.y
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/parse.y,v
retrieving revision 1.98
diff -u -p -r1.98 parse.y
--- parse.y 7 Jun 2019 04:57:45 -0000 1.98
+++ parse.y 24 Oct 2019 20:09:11 -0000
@@ -129,7 +129,7 @@ typedef struct {
 %token AREA INTERFACE ROUTERID FIBPRIORITY FIBUPDATE REDISTRIBUTE RTLABEL
 %token RDOMAIN RFC1583COMPAT STUB ROUTER SPFDELAY SPFHOLDTIME EXTTAG
 %token AUTHKEY AUTHTYPE AUTHMD AUTHMDKEYID
-%token METRIC PASSIVE
+%token METRIC P2P PASSIVE
 %token HELLOINTERVAL FASTHELLOINTERVAL TRANSMITDELAY
 %token RETRANSMITINTERVAL ROUTERDEADTIME ROUTERPRIORITY
 %token SET TYPE
@@ -743,6 +743,10 @@ interfaceopts_l : interfaceopts_l interf
  ;
 
 interfaceoptsl : PASSIVE { iface->passive = 1; }
+ | TYPE P2P {
+ iface->p2p = 1;
+ iface->type = IF_TYPE_POINTOPOINT;
+ }
  | DEMOTE STRING {
  if (strlcpy(iface->demote_group, $2,
     sizeof(iface->demote_group)) >=
@@ -833,6 +837,7 @@ lookup(char *s)
  {"msec", MSEC},
  {"no", NO},
  {"on", ON},
+ {"p2p", P2P},
  {"passive", PASSIVE},
  {"rdomain", RDOMAIN},
  {"redistribute", REDISTRIBUTE},
Index: printconf.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/printconf.c,v
retrieving revision 1.20
diff -u -p -r1.20 printconf.c
--- printconf.c 28 Dec 2018 19:25:10 -0000 1.20
+++ printconf.c 23 Jun 2019 22:05:55 -0000
@@ -149,6 +149,9 @@ print_iface(struct iface *iface)
  printf("\t\trouter-priority %d\n", iface->priority);
  printf("\t\ttransmit-delay %d\n", iface->transmit_delay);
 
+ if (iface->p2p)
+ printf("\t\ttype p2p\n");
+
  printf("\t\tauth-type %s\n", if_auth_name(iface->auth_type));
  switch (iface->auth_type) {
  case AUTH_TYPE_NONE:
Index: rde.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/rde.c,v
retrieving revision 1.109
diff -u -p -r1.109 rde.c
--- rde.c 5 Feb 2018 12:11:28 -0000 1.109
+++ rde.c 21 Sep 2019 12:17:09 -0000
@@ -257,6 +257,7 @@ rde_dispatch_imsg(int fd, short event, v
  struct timespec tp;
  struct lsa *lsa;
  struct area *area;
+ struct in_addr addr;
  struct vertex *v;
  char *buf;
  ssize_t n;
@@ -300,6 +301,17 @@ rde_dispatch_imsg(int fd, short event, v
  break;
  case IMSG_NEIGHBOR_DOWN:
  rde_nbr_del(rde_nbr_find(imsg.hdr.peerid));
+ break;
+ case IMSG_NEIGHBOR_ADDR:
+ if (imsg.hdr.len - IMSG_HEADER_SIZE != sizeof(addr))
+ fatalx("invalid size of OE request");
+ memcpy(&addr, imsg.data, sizeof(addr));
+
+ nbr = rde_nbr_find(imsg.hdr.peerid);
+ if (nbr == NULL)
+ break;
+
+ nbr->addr.s_addr = addr.s_addr;
  break;
  case IMSG_NEIGHBOR_CHANGE:
  if (imsg.hdr.len - IMSG_HEADER_SIZE != sizeof(state))
Index: rde.h
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/rde.h,v
retrieving revision 1.39
diff -u -p -r1.39 rde.h
--- rde.h 14 Mar 2015 02:22:09 -0000 1.39
+++ rde.h 19 Sep 2019 13:46:43 -0000
@@ -66,6 +66,7 @@ struct rde_nbr {
  LIST_ENTRY(rde_nbr) entry, hash;
  struct in_addr id;
  struct in_addr area_id;
+ struct in_addr addr;
  TAILQ_HEAD(, rde_req_entry) req_list;
  struct area *area;
  struct iface *iface;
Index: rde_spf.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/rde_spf.c,v
retrieving revision 1.77
diff -u -p -r1.77 rde_spf.c
--- rde_spf.c 4 Apr 2019 19:57:08 -0000 1.77
+++ rde_spf.c 29 Sep 2019 21:08:49 -0000
@@ -373,6 +373,7 @@ calc_nexthop(struct vertex *dst, struct
 {
  struct v_nexthop *vn;
  struct iface *iface;
+ struct rde_nbr *nbr;
  int i;
 
  /* case 1 */
@@ -382,10 +383,14 @@ calc_nexthop(struct vertex *dst, struct
  if (rtr_link->type != LINK_TYPE_POINTTOPOINT)
  fatalx("inconsistent SPF tree");
  LIST_FOREACH(iface, &area->iface_list, entry) {
- if (rtr_link->data == iface->addr.s_addr) {
- vertex_nexthop_add(dst, parent,
-    iface->dst.s_addr);
- return;
+ if (rtr_link->data != iface->addr.s_addr)
+ continue;
+ LIST_FOREACH(nbr, &area->nbr_list, entry) {
+ if (nbr->ifindex == iface->ifindex) {
+ vertex_nexthop_add(dst, parent,
+    nbr->addr.s_addr);
+ return;
+ }
  }
  }
  fatalx("no interface found for interface");

Reply | Threaded
Open this post in threaded view
|

Re: ospfd: type p2p

Kapetanakis Giannis
On 25/10/2019 13:57, Remi Locherer wrote:

> Hi tech@,
>
> earlier this year I sent a diff that allowed to change an interface
> from broadcast to point-to-point.
>
> https://marc.info/?l=openbsd-tech&m=156132923203704&w=2
>
> It turned out that this was not sufficient. It made the adjacency
> come up in p2p mode (no selection of DR or BDR) but didn't set a valid
> next hop for routes learned over this p2p link. Actually the next hop was
> 0.0.0.0 which was never installed into the routing table.
>
> This is because for P2P interfaces the neighbor address is not taken from
> the received hello but from the "destination" parameter configured on the
> interface. Since this is not set on a broadcast interface the address is
> 0.0.0.0.
>
> My new diff changes this. Now also for P2P links the IP address of the
> neighbor is taken from the hello packets (src address). This on it's own
> would make it simpler to interfere with the routing from remote. One could
> send unicast ospf hello messages and potentially disrupt the routing setup.
> I believe I mitigated this with an additional check I committed in August:
> only hello messages sent to the multicast address are now processed.
>
> The config looks like this:
>
> area 0.0.0.0 {
> interface em0 {
> type p2p
> }
> }
>
> It would be nice to get test reports for this new feature (check the fib
> and routing table!) and also test reports with real p2p2 interfaces (gif
> or gre).
>
> Of course OKs are also welcome. ;-)
>
> Remi


Hi,

From first test seems to work :)

looking forward test it for IPv6 as well

thanks

Giannis

>
>
>
> Index: hello.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/hello.c,v
> retrieving revision 1.24
> diff -u -p -r1.24 hello.c
> --- hello.c 12 Aug 2019 20:21:58 -0000 1.24
> +++ hello.c 21 Sep 2019 22:06:17 -0000
> @@ -189,14 +189,13 @@ recv_hello(struct iface *iface, struct i
>   nbr->dr.s_addr = hello.d_rtr;
>   nbr->bdr.s_addr = hello.bd_rtr;
>   nbr->priority = hello.rtr_priority;
> - /* XXX neighbor address shouldn't be stored on virtual links */
> - nbr->addr.s_addr = src.s_addr;
> + nbr_update_addr(nbr->peerid, src);
>   }
>  
>   if (nbr->addr.s_addr != src.s_addr) {
>   log_warnx("%s: neighbor ID %s changed its IP address",
>      __func__, inet_ntoa(nbr->id));
> - nbr->addr.s_addr = src.s_addr;
> + nbr_update_addr(nbr->peerid, src);
>   }
>  
>   nbr->options = hello.opts;
> Index: lsupdate.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/lsupdate.c,v
> retrieving revision 1.46
> diff -u -p -r1.46 lsupdate.c
> --- lsupdate.c 15 Jul 2019 18:26:39 -0000 1.46
> +++ lsupdate.c 15 Aug 2019 21:10:13 -0000
> @@ -470,7 +470,7 @@ ls_retrans_timer(int fd, short event, vo
>   /* ls_retrans_list_free retriggers the timer */
>   return;
>   } else if (nbr->iface->type == IF_TYPE_POINTOPOINT)
> - memcpy(&addr, &nbr->iface->dst, sizeof(addr));
> + memcpy(&addr, &nbr->addr, sizeof(addr));
>   else
>   inet_aton(AllDRouters, &addr);
>   } else
> Index: neighbor.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/neighbor.c,v
> retrieving revision 1.48
> diff -u -p -r1.48 neighbor.c
> --- neighbor.c 9 Feb 2018 02:14:03 -0000 1.48
> +++ neighbor.c 21 Sep 2019 15:28:43 -0000
> @@ -312,6 +312,7 @@ nbr_new(u_int32_t nbr_id, struct iface *
>   bzero(&rn, sizeof(rn));
>   rn.id.s_addr = nbr->id.s_addr;
>   rn.area_id.s_addr = nbr->iface->area->id.s_addr;
> + rn.addr.s_addr = nbr->addr.s_addr;
>   rn.ifindex = nbr->iface->ifindex;
>   rn.state = nbr->state;
>   rn.self = self;
> @@ -347,6 +348,23 @@ nbr_del(struct nbr *nbr)
>   LIST_REMOVE(nbr, hash);
>  
>   free(nbr);
> +}
> +
> +int
> +nbr_update_addr(u_int32_t peerid, struct in_addr addr) {
> +
> + struct nbr *nbr = NULL;
> +
> + nbr = nbr_find_peerid(peerid);
> + if (nbr == NULL)
> + return (1);
> +
> + /* XXX neighbor address shouldn't be stored on virtual links */
> + nbr->addr.s_addr = addr.s_addr;
> + ospfe_imsg_compose_rde(IMSG_NEIGHBOR_ADDR, peerid, 0, &addr,
> +    sizeof(addr));
> +
> + return (0);
>  }
>  
>  struct nbr *
> Index: ospfd.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/ospfd.c,v
> retrieving revision 1.108
> diff -u -p -r1.108 ospfd.c
> --- ospfd.c 16 May 2019 05:49:22 -0000 1.108
> +++ ospfd.c 23 Jun 2019 21:06:44 -0000
> @@ -911,6 +911,22 @@ merge_interfaces(struct area *a, struct
>   if_fsm(i, IF_EVT_UP);
>   }
>  
> + if (i->p2p != xi->p2p) {
> + /* re-add interface to enable or disable DR election */
> + if (ospfd_process == PROC_OSPF_ENGINE)
> + if_fsm(i, IF_EVT_DOWN);
> + else if (ospfd_process == PROC_RDE_ENGINE)
> + rde_nbr_iface_del(i);
> + LIST_REMOVE(i, entry);
> + if_del(i);
> + LIST_REMOVE(xi, entry);
> + LIST_INSERT_HEAD(&a->iface_list, xi, entry);
> + xi->area = a;
> + if (ospfd_process == PROC_OSPF_ENGINE)
> + xi->state = IF_STA_NEW;
> + continue;
> + }
> +
>   strlcpy(i->dependon, xi->dependon,
>          sizeof(i->dependon));
>   i->depend_ok = xi->depend_ok;
> Index: ospfd.conf.5
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/ospfd.conf.5,v
> retrieving revision 1.57
> diff -u -p -r1.57 ospfd.conf.5
> --- ospfd.conf.5 10 Jun 2019 06:07:15 -0000 1.57
> +++ ospfd.conf.5 23 Jun 2019 22:10:32 -0000
> @@ -419,6 +419,9 @@ Router.
>  .It Ic transmit-delay Ar seconds
>  Set the transmit delay.
>  The default value is 1; valid range is 1\-3600 seconds.
> +.It Ic type p2p
> +Set the interface type to point to point.
> +This disables the election of a DR and BDR for the given interface.
>  .El
>  .Sh FILES
>  .Bl -tag -width "/etc/ospfd.conf" -compact
> Index: ospfd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/ospfd.h,v
> retrieving revision 1.104
> diff -u -p -r1.104 ospfd.h
> --- ospfd.h 16 May 2019 05:49:22 -0000 1.104
> +++ ospfd.h 21 Sep 2019 10:25:32 -0000
> @@ -107,6 +107,7 @@ enum imsg_type {
>   IMSG_IFINFO,
>   IMSG_NEIGHBOR_UP,
>   IMSG_NEIGHBOR_DOWN,
> + IMSG_NEIGHBOR_ADDR,
>   IMSG_NEIGHBOR_CHANGE,
>   IMSG_NEIGHBOR_CAPA,
>   IMSG_NETWORK_ADD,
> @@ -363,6 +364,7 @@ struct iface {
>   u_int8_t linkstate;
>   u_int8_t priority;
>   u_int8_t passive;
> + u_int8_t p2p;
>  };
>  
>  struct ifaddrchange {
> Index: ospfe.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/ospfe.h,v
> retrieving revision 1.46
> diff -u -p -r1.46 ospfe.h
> --- ospfe.h 25 Oct 2014 03:23:49 -0000 1.46
> +++ ospfe.h 21 Sep 2019 15:27:08 -0000
> @@ -204,6 +204,7 @@ void lsa_cache_put(struct lsa_ref *, s
>  void nbr_init(u_int32_t);
>  struct nbr *nbr_new(u_int32_t, struct iface *, int);
>  void nbr_del(struct nbr *);
> +int nbr_update_addr(u_int32_t, struct in_addr);
>  
>  struct nbr *nbr_find_id(struct iface *, u_int32_t);
>  struct nbr *nbr_find_peerid(u_int32_t);
> Index: parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/parse.y,v
> retrieving revision 1.98
> diff -u -p -r1.98 parse.y
> --- parse.y 7 Jun 2019 04:57:45 -0000 1.98
> +++ parse.y 24 Oct 2019 20:09:11 -0000
> @@ -129,7 +129,7 @@ typedef struct {
>  %token AREA INTERFACE ROUTERID FIBPRIORITY FIBUPDATE REDISTRIBUTE RTLABEL
>  %token RDOMAIN RFC1583COMPAT STUB ROUTER SPFDELAY SPFHOLDTIME EXTTAG
>  %token AUTHKEY AUTHTYPE AUTHMD AUTHMDKEYID
> -%token METRIC PASSIVE
> +%token METRIC P2P PASSIVE
>  %token HELLOINTERVAL FASTHELLOINTERVAL TRANSMITDELAY
>  %token RETRANSMITINTERVAL ROUTERDEADTIME ROUTERPRIORITY
>  %token SET TYPE
> @@ -743,6 +743,10 @@ interfaceopts_l : interfaceopts_l interf
>   ;
>  
>  interfaceoptsl : PASSIVE { iface->passive = 1; }
> + | TYPE P2P {
> + iface->p2p = 1;
> + iface->type = IF_TYPE_POINTOPOINT;
> + }
>   | DEMOTE STRING {
>   if (strlcpy(iface->demote_group, $2,
>      sizeof(iface->demote_group)) >=
> @@ -833,6 +837,7 @@ lookup(char *s)
>   {"msec", MSEC},
>   {"no", NO},
>   {"on", ON},
> + {"p2p", P2P},
>   {"passive", PASSIVE},
>   {"rdomain", RDOMAIN},
>   {"redistribute", REDISTRIBUTE},
> Index: printconf.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/printconf.c,v
> retrieving revision 1.20
> diff -u -p -r1.20 printconf.c
> --- printconf.c 28 Dec 2018 19:25:10 -0000 1.20
> +++ printconf.c 23 Jun 2019 22:05:55 -0000
> @@ -149,6 +149,9 @@ print_iface(struct iface *iface)
>   printf("\t\trouter-priority %d\n", iface->priority);
>   printf("\t\ttransmit-delay %d\n", iface->transmit_delay);
>  
> + if (iface->p2p)
> + printf("\t\ttype p2p\n");
> +
>   printf("\t\tauth-type %s\n", if_auth_name(iface->auth_type));
>   switch (iface->auth_type) {
>   case AUTH_TYPE_NONE:
> Index: rde.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/rde.c,v
> retrieving revision 1.109
> diff -u -p -r1.109 rde.c
> --- rde.c 5 Feb 2018 12:11:28 -0000 1.109
> +++ rde.c 21 Sep 2019 12:17:09 -0000
> @@ -257,6 +257,7 @@ rde_dispatch_imsg(int fd, short event, v
>   struct timespec tp;
>   struct lsa *lsa;
>   struct area *area;
> + struct in_addr addr;
>   struct vertex *v;
>   char *buf;
>   ssize_t n;
> @@ -300,6 +301,17 @@ rde_dispatch_imsg(int fd, short event, v
>   break;
>   case IMSG_NEIGHBOR_DOWN:
>   rde_nbr_del(rde_nbr_find(imsg.hdr.peerid));
> + break;
> + case IMSG_NEIGHBOR_ADDR:
> + if (imsg.hdr.len - IMSG_HEADER_SIZE != sizeof(addr))
> + fatalx("invalid size of OE request");
> + memcpy(&addr, imsg.data, sizeof(addr));
> +
> + nbr = rde_nbr_find(imsg.hdr.peerid);
> + if (nbr == NULL)
> + break;
> +
> + nbr->addr.s_addr = addr.s_addr;
>   break;
>   case IMSG_NEIGHBOR_CHANGE:
>   if (imsg.hdr.len - IMSG_HEADER_SIZE != sizeof(state))
> Index: rde.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/rde.h,v
> retrieving revision 1.39
> diff -u -p -r1.39 rde.h
> --- rde.h 14 Mar 2015 02:22:09 -0000 1.39
> +++ rde.h 19 Sep 2019 13:46:43 -0000
> @@ -66,6 +66,7 @@ struct rde_nbr {
>   LIST_ENTRY(rde_nbr) entry, hash;
>   struct in_addr id;
>   struct in_addr area_id;
> + struct in_addr addr;
>   TAILQ_HEAD(, rde_req_entry) req_list;
>   struct area *area;
>   struct iface *iface;
> Index: rde_spf.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/rde_spf.c,v
> retrieving revision 1.77
> diff -u -p -r1.77 rde_spf.c
> --- rde_spf.c 4 Apr 2019 19:57:08 -0000 1.77
> +++ rde_spf.c 29 Sep 2019 21:08:49 -0000
> @@ -373,6 +373,7 @@ calc_nexthop(struct vertex *dst, struct
>  {
>   struct v_nexthop *vn;
>   struct iface *iface;
> + struct rde_nbr *nbr;
>   int i;
>  
>   /* case 1 */
> @@ -382,10 +383,14 @@ calc_nexthop(struct vertex *dst, struct
>   if (rtr_link->type != LINK_TYPE_POINTTOPOINT)
>   fatalx("inconsistent SPF tree");
>   LIST_FOREACH(iface, &area->iface_list, entry) {
> - if (rtr_link->data == iface->addr.s_addr) {
> - vertex_nexthop_add(dst, parent,
> -    iface->dst.s_addr);
> - return;
> + if (rtr_link->data != iface->addr.s_addr)
> + continue;
> + LIST_FOREACH(nbr, &area->nbr_list, entry) {
> + if (nbr->ifindex == iface->ifindex) {
> + vertex_nexthop_add(dst, parent,
> +    nbr->addr.s_addr);
> + return;
> + }
>   }
>   }
>   fatalx("no interface found for interface");
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: ospfd: type p2p

Remi Locherer
On Mon, Nov 04, 2019 at 02:01:57PM +0200, Kapetanakis Giannis wrote:

> On 25/10/2019 13:57, Remi Locherer wrote:
> > Hi tech@,
> >
> > earlier this year I sent a diff that allowed to change an interface
> > from broadcast to point-to-point.
> >
> > https://marc.info/?l=openbsd-tech&m=156132923203704&w=2
> >
> > It turned out that this was not sufficient. It made the adjacency
> > come up in p2p mode (no selection of DR or BDR) but didn't set a valid
> > next hop for routes learned over this p2p link. Actually the next hop was
> > 0.0.0.0 which was never installed into the routing table.
> >
> > This is because for P2P interfaces the neighbor address is not taken from
> > the received hello but from the "destination" parameter configured on the
> > interface. Since this is not set on a broadcast interface the address is
> > 0.0.0.0.
> >
> > My new diff changes this. Now also for P2P links the IP address of the
> > neighbor is taken from the hello packets (src address). This on it's own
> > would make it simpler to interfere with the routing from remote. One could
> > send unicast ospf hello messages and potentially disrupt the routing setup.
> > I believe I mitigated this with an additional check I committed in August:
> > only hello messages sent to the multicast address are now processed.
> >
> > The config looks like this:
> >
> > area 0.0.0.0 {
> > interface em0 {
> > type p2p
> > }
> > }
> >
> > It would be nice to get test reports for this new feature (check the fib
> > and routing table!) and also test reports with real p2p2 interfaces (gif
> > or gre).
> >
> > Of course OKs are also welcome. ;-)
> >
> > Remi
>
>
> Hi,
>
> From first test seems to work :)

Thank you for testing!

>
> looking forward test it for IPv6 as well

Sure, I plan to also do this this for ospf6d.

Reply | Threaded
Open this post in threaded view
|

Re: ospfd: type p2p

Remi Locherer
In reply to this post by Kapetanakis Giannis
On Mon, Nov 04, 2019 at 02:01:57PM +0200, Kapetanakis Giannis wrote:

> On 25/10/2019 13:57, Remi Locherer wrote:
> > Hi tech@,
> >
> > earlier this year I sent a diff that allowed to change an interface
> > from broadcast to point-to-point.
> >
> > https://marc.info/?l=openbsd-tech&m=156132923203704&w=2
> >
> > It turned out that this was not sufficient. It made the adjacency
> > come up in p2p mode (no selection of DR or BDR) but didn't set a valid
> > next hop for routes learned over this p2p link. Actually the next hop was
> > 0.0.0.0 which was never installed into the routing table.
> >
> > This is because for P2P interfaces the neighbor address is not taken from
> > the received hello but from the "destination" parameter configured on the
> > interface. Since this is not set on a broadcast interface the address is
> > 0.0.0.0.
> >
> > My new diff changes this. Now also for P2P links the IP address of the
> > neighbor is taken from the hello packets (src address). This on it's own
> > would make it simpler to interfere with the routing from remote. One could
> > send unicast ospf hello messages and potentially disrupt the routing setup.
> > I believe I mitigated this with an additional check I committed in August:
> > only hello messages sent to the multicast address are now processed.
> >
> > The config looks like this:
> >
> > area 0.0.0.0 {
> > interface em0 {
> > type p2p
> > }
> > }
> >
> > It would be nice to get test reports for this new feature (check the fib
> > and routing table!) and also test reports with real p2p2 interfaces (gif
> > or gre).
> >
> > Of course OKs are also welcome. ;-)
> >
> > Remi
>
>
> Hi,
>
> From first test seems to work :)
>
> looking forward test it for IPv6 as well
>
> thanks
>
> Giannis


Anyone willing to OK this?



Index: hello.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/hello.c,v
retrieving revision 1.24
diff -u -p -r1.24 hello.c
--- hello.c 12 Aug 2019 20:21:58 -0000 1.24
+++ hello.c 21 Sep 2019 22:06:17 -0000
@@ -189,14 +189,13 @@ recv_hello(struct iface *iface, struct i
  nbr->dr.s_addr = hello.d_rtr;
  nbr->bdr.s_addr = hello.bd_rtr;
  nbr->priority = hello.rtr_priority;
- /* XXX neighbor address shouldn't be stored on virtual links */
- nbr->addr.s_addr = src.s_addr;
+ nbr_update_addr(nbr->peerid, src);
  }
 
  if (nbr->addr.s_addr != src.s_addr) {
  log_warnx("%s: neighbor ID %s changed its IP address",
     __func__, inet_ntoa(nbr->id));
- nbr->addr.s_addr = src.s_addr;
+ nbr_update_addr(nbr->peerid, src);
  }
 
  nbr->options = hello.opts;
Index: lsupdate.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/lsupdate.c,v
retrieving revision 1.46
diff -u -p -r1.46 lsupdate.c
--- lsupdate.c 15 Jul 2019 18:26:39 -0000 1.46
+++ lsupdate.c 15 Aug 2019 21:10:13 -0000
@@ -470,7 +470,7 @@ ls_retrans_timer(int fd, short event, vo
  /* ls_retrans_list_free retriggers the timer */
  return;
  } else if (nbr->iface->type == IF_TYPE_POINTOPOINT)
- memcpy(&addr, &nbr->iface->dst, sizeof(addr));
+ memcpy(&addr, &nbr->addr, sizeof(addr));
  else
  inet_aton(AllDRouters, &addr);
  } else
Index: neighbor.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/neighbor.c,v
retrieving revision 1.48
diff -u -p -r1.48 neighbor.c
--- neighbor.c 9 Feb 2018 02:14:03 -0000 1.48
+++ neighbor.c 21 Sep 2019 15:28:43 -0000
@@ -312,6 +312,7 @@ nbr_new(u_int32_t nbr_id, struct iface *
  bzero(&rn, sizeof(rn));
  rn.id.s_addr = nbr->id.s_addr;
  rn.area_id.s_addr = nbr->iface->area->id.s_addr;
+ rn.addr.s_addr = nbr->addr.s_addr;
  rn.ifindex = nbr->iface->ifindex;
  rn.state = nbr->state;
  rn.self = self;
@@ -347,6 +348,23 @@ nbr_del(struct nbr *nbr)
  LIST_REMOVE(nbr, hash);
 
  free(nbr);
+}
+
+int
+nbr_update_addr(u_int32_t peerid, struct in_addr addr) {
+
+ struct nbr *nbr = NULL;
+
+ nbr = nbr_find_peerid(peerid);
+ if (nbr == NULL)
+ return (1);
+
+ /* XXX neighbor address shouldn't be stored on virtual links */
+ nbr->addr.s_addr = addr.s_addr;
+ ospfe_imsg_compose_rde(IMSG_NEIGHBOR_ADDR, peerid, 0, &addr,
+    sizeof(addr));
+
+ return (0);
 }
 
 struct nbr *
Index: ospfd.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/ospfd.c,v
retrieving revision 1.108
diff -u -p -r1.108 ospfd.c
--- ospfd.c 16 May 2019 05:49:22 -0000 1.108
+++ ospfd.c 23 Jun 2019 21:06:44 -0000
@@ -911,6 +911,22 @@ merge_interfaces(struct area *a, struct
  if_fsm(i, IF_EVT_UP);
  }
 
+ if (i->p2p != xi->p2p) {
+ /* re-add interface to enable or disable DR election */
+ if (ospfd_process == PROC_OSPF_ENGINE)
+ if_fsm(i, IF_EVT_DOWN);
+ else if (ospfd_process == PROC_RDE_ENGINE)
+ rde_nbr_iface_del(i);
+ LIST_REMOVE(i, entry);
+ if_del(i);
+ LIST_REMOVE(xi, entry);
+ LIST_INSERT_HEAD(&a->iface_list, xi, entry);
+ xi->area = a;
+ if (ospfd_process == PROC_OSPF_ENGINE)
+ xi->state = IF_STA_NEW;
+ continue;
+ }
+
  strlcpy(i->dependon, xi->dependon,
         sizeof(i->dependon));
  i->depend_ok = xi->depend_ok;
Index: ospfd.conf.5
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/ospfd.conf.5,v
retrieving revision 1.57
diff -u -p -r1.57 ospfd.conf.5
--- ospfd.conf.5 10 Jun 2019 06:07:15 -0000 1.57
+++ ospfd.conf.5 23 Jun 2019 22:10:32 -0000
@@ -419,6 +419,9 @@ Router.
 .It Ic transmit-delay Ar seconds
 Set the transmit delay.
 The default value is 1; valid range is 1\-3600 seconds.
+.It Ic type p2p
+Set the interface type to point to point.
+This disables the election of a DR and BDR for the given interface.
 .El
 .Sh FILES
 .Bl -tag -width "/etc/ospfd.conf" -compact
Index: ospfd.h
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/ospfd.h,v
retrieving revision 1.104
diff -u -p -r1.104 ospfd.h
--- ospfd.h 16 May 2019 05:49:22 -0000 1.104
+++ ospfd.h 21 Sep 2019 10:25:32 -0000
@@ -107,6 +107,7 @@ enum imsg_type {
  IMSG_IFINFO,
  IMSG_NEIGHBOR_UP,
  IMSG_NEIGHBOR_DOWN,
+ IMSG_NEIGHBOR_ADDR,
  IMSG_NEIGHBOR_CHANGE,
  IMSG_NEIGHBOR_CAPA,
  IMSG_NETWORK_ADD,
@@ -363,6 +364,7 @@ struct iface {
  u_int8_t linkstate;
  u_int8_t priority;
  u_int8_t passive;
+ u_int8_t p2p;
 };
 
 struct ifaddrchange {
Index: ospfe.h
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/ospfe.h,v
retrieving revision 1.46
diff -u -p -r1.46 ospfe.h
--- ospfe.h 25 Oct 2014 03:23:49 -0000 1.46
+++ ospfe.h 21 Sep 2019 15:27:08 -0000
@@ -204,6 +204,7 @@ void lsa_cache_put(struct lsa_ref *, s
 void nbr_init(u_int32_t);
 struct nbr *nbr_new(u_int32_t, struct iface *, int);
 void nbr_del(struct nbr *);
+int nbr_update_addr(u_int32_t, struct in_addr);
 
 struct nbr *nbr_find_id(struct iface *, u_int32_t);
 struct nbr *nbr_find_peerid(u_int32_t);
Index: parse.y
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/parse.y,v
retrieving revision 1.98
diff -u -p -r1.98 parse.y
--- parse.y 7 Jun 2019 04:57:45 -0000 1.98
+++ parse.y 24 Oct 2019 20:09:11 -0000
@@ -129,7 +129,7 @@ typedef struct {
 %token AREA INTERFACE ROUTERID FIBPRIORITY FIBUPDATE REDISTRIBUTE RTLABEL
 %token RDOMAIN RFC1583COMPAT STUB ROUTER SPFDELAY SPFHOLDTIME EXTTAG
 %token AUTHKEY AUTHTYPE AUTHMD AUTHMDKEYID
-%token METRIC PASSIVE
+%token METRIC P2P PASSIVE
 %token HELLOINTERVAL FASTHELLOINTERVAL TRANSMITDELAY
 %token RETRANSMITINTERVAL ROUTERDEADTIME ROUTERPRIORITY
 %token SET TYPE
@@ -743,6 +743,10 @@ interfaceopts_l : interfaceopts_l interf
  ;
 
 interfaceoptsl : PASSIVE { iface->passive = 1; }
+ | TYPE P2P {
+ iface->p2p = 1;
+ iface->type = IF_TYPE_POINTOPOINT;
+ }
  | DEMOTE STRING {
  if (strlcpy(iface->demote_group, $2,
     sizeof(iface->demote_group)) >=
@@ -833,6 +837,7 @@ lookup(char *s)
  {"msec", MSEC},
  {"no", NO},
  {"on", ON},
+ {"p2p", P2P},
  {"passive", PASSIVE},
  {"rdomain", RDOMAIN},
  {"redistribute", REDISTRIBUTE},
Index: printconf.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/printconf.c,v
retrieving revision 1.20
diff -u -p -r1.20 printconf.c
--- printconf.c 28 Dec 2018 19:25:10 -0000 1.20
+++ printconf.c 23 Jun 2019 22:05:55 -0000
@@ -149,6 +149,9 @@ print_iface(struct iface *iface)
  printf("\t\trouter-priority %d\n", iface->priority);
  printf("\t\ttransmit-delay %d\n", iface->transmit_delay);
 
+ if (iface->p2p)
+ printf("\t\ttype p2p\n");
+
  printf("\t\tauth-type %s\n", if_auth_name(iface->auth_type));
  switch (iface->auth_type) {
  case AUTH_TYPE_NONE:
Index: rde.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/rde.c,v
retrieving revision 1.109
diff -u -p -r1.109 rde.c
--- rde.c 5 Feb 2018 12:11:28 -0000 1.109
+++ rde.c 21 Sep 2019 12:17:09 -0000
@@ -257,6 +257,7 @@ rde_dispatch_imsg(int fd, short event, v
  struct timespec tp;
  struct lsa *lsa;
  struct area *area;
+ struct in_addr addr;
  struct vertex *v;
  char *buf;
  ssize_t n;
@@ -300,6 +301,17 @@ rde_dispatch_imsg(int fd, short event, v
  break;
  case IMSG_NEIGHBOR_DOWN:
  rde_nbr_del(rde_nbr_find(imsg.hdr.peerid));
+ break;
+ case IMSG_NEIGHBOR_ADDR:
+ if (imsg.hdr.len - IMSG_HEADER_SIZE != sizeof(addr))
+ fatalx("invalid size of OE request");
+ memcpy(&addr, imsg.data, sizeof(addr));
+
+ nbr = rde_nbr_find(imsg.hdr.peerid);
+ if (nbr == NULL)
+ break;
+
+ nbr->addr.s_addr = addr.s_addr;
  break;
  case IMSG_NEIGHBOR_CHANGE:
  if (imsg.hdr.len - IMSG_HEADER_SIZE != sizeof(state))
Index: rde.h
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/rde.h,v
retrieving revision 1.39
diff -u -p -r1.39 rde.h
--- rde.h 14 Mar 2015 02:22:09 -0000 1.39
+++ rde.h 19 Sep 2019 13:46:43 -0000
@@ -66,6 +66,7 @@ struct rde_nbr {
  LIST_ENTRY(rde_nbr) entry, hash;
  struct in_addr id;
  struct in_addr area_id;
+ struct in_addr addr;
  TAILQ_HEAD(, rde_req_entry) req_list;
  struct area *area;
  struct iface *iface;
Index: rde_spf.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/rde_spf.c,v
retrieving revision 1.77
diff -u -p -r1.77 rde_spf.c
--- rde_spf.c 4 Apr 2019 19:57:08 -0000 1.77
+++ rde_spf.c 29 Sep 2019 21:08:49 -0000
@@ -373,6 +373,7 @@ calc_nexthop(struct vertex *dst, struct
 {
  struct v_nexthop *vn;
  struct iface *iface;
+ struct rde_nbr *nbr;
  int i;
 
  /* case 1 */
@@ -382,10 +383,14 @@ calc_nexthop(struct vertex *dst, struct
  if (rtr_link->type != LINK_TYPE_POINTTOPOINT)
  fatalx("inconsistent SPF tree");
  LIST_FOREACH(iface, &area->iface_list, entry) {
- if (rtr_link->data == iface->addr.s_addr) {
- vertex_nexthop_add(dst, parent,
-    iface->dst.s_addr);
- return;
+ if (rtr_link->data != iface->addr.s_addr)
+ continue;
+ LIST_FOREACH(nbr, &area->nbr_list, entry) {
+ if (nbr->ifindex == iface->ifindex) {
+ vertex_nexthop_add(dst, parent,
+    nbr->addr.s_addr);
+ return;
+ }
  }
  }
  fatalx("no interface found for interface");

Reply | Threaded
Open this post in threaded view
|

Re: ospfd: type p2p

Claudio Jeker
On Fri, Nov 15, 2019 at 06:06:42PM +0100, Remi Locherer wrote:

> On Mon, Nov 04, 2019 at 02:01:57PM +0200, Kapetanakis Giannis wrote:
> > On 25/10/2019 13:57, Remi Locherer wrote:
> > > Hi tech@,
> > >
> > > earlier this year I sent a diff that allowed to change an interface
> > > from broadcast to point-to-point.
> > >
> > > https://marc.info/?l=openbsd-tech&m=156132923203704&w=2
> > >
> > > It turned out that this was not sufficient. It made the adjacency
> > > come up in p2p mode (no selection of DR or BDR) but didn't set a valid
> > > next hop for routes learned over this p2p link. Actually the next hop was
> > > 0.0.0.0 which was never installed into the routing table.
> > >
> > > This is because for P2P interfaces the neighbor address is not taken from
> > > the received hello but from the "destination" parameter configured on the
> > > interface. Since this is not set on a broadcast interface the address is
> > > 0.0.0.0.
> > >
> > > My new diff changes this. Now also for P2P links the IP address of the
> > > neighbor is taken from the hello packets (src address). This on it's own
> > > would make it simpler to interfere with the routing from remote. One could
> > > send unicast ospf hello messages and potentially disrupt the routing setup.
> > > I believe I mitigated this with an additional check I committed in August:
> > > only hello messages sent to the multicast address are now processed.
> > >
> > > The config looks like this:
> > >
> > > area 0.0.0.0 {
> > > interface em0 {
> > > type p2p
> > > }
> > > }
> > >
> > > It would be nice to get test reports for this new feature (check the fib
> > > and routing table!) and also test reports with real p2p2 interfaces (gif
> > > or gre).
> > >
> > > Of course OKs are also welcome. ;-)
> > >
> > > Remi
> >
> >
> > Hi,
> >
> > From first test seems to work :)
> >
> > looking forward test it for IPv6 as well
> >
> > thanks
> >
> > Giannis
>
>
> Anyone willing to OK this?

See inline.

> Index: hello.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/hello.c,v
> retrieving revision 1.24
> diff -u -p -r1.24 hello.c
> --- hello.c 12 Aug 2019 20:21:58 -0000 1.24
> +++ hello.c 21 Sep 2019 22:06:17 -0000
> @@ -189,14 +189,13 @@ recv_hello(struct iface *iface, struct i
>   nbr->dr.s_addr = hello.d_rtr;
>   nbr->bdr.s_addr = hello.bd_rtr;
>   nbr->priority = hello.rtr_priority;
> - /* XXX neighbor address shouldn't be stored on virtual links */
> - nbr->addr.s_addr = src.s_addr;
> + nbr_update_addr(nbr->peerid, src);
>   }
>  
>   if (nbr->addr.s_addr != src.s_addr) {
>   log_warnx("%s: neighbor ID %s changed its IP address",
>      __func__, inet_ntoa(nbr->id));
> - nbr->addr.s_addr = src.s_addr;
> + nbr_update_addr(nbr->peerid, src);
>   }
>  
>   nbr->options = hello.opts;
> Index: lsupdate.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/lsupdate.c,v
> retrieving revision 1.46
> diff -u -p -r1.46 lsupdate.c
> --- lsupdate.c 15 Jul 2019 18:26:39 -0000 1.46
> +++ lsupdate.c 15 Aug 2019 21:10:13 -0000
> @@ -470,7 +470,7 @@ ls_retrans_timer(int fd, short event, vo
>   /* ls_retrans_list_free retriggers the timer */
>   return;
>   } else if (nbr->iface->type == IF_TYPE_POINTOPOINT)
> - memcpy(&addr, &nbr->iface->dst, sizeof(addr));
> + memcpy(&addr, &nbr->addr, sizeof(addr));
>   else
>   inet_aton(AllDRouters, &addr);
>   } else
> Index: neighbor.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/neighbor.c,v
> retrieving revision 1.48
> diff -u -p -r1.48 neighbor.c
> --- neighbor.c 9 Feb 2018 02:14:03 -0000 1.48
> +++ neighbor.c 21 Sep 2019 15:28:43 -0000
> @@ -312,6 +312,7 @@ nbr_new(u_int32_t nbr_id, struct iface *
>   bzero(&rn, sizeof(rn));
>   rn.id.s_addr = nbr->id.s_addr;
>   rn.area_id.s_addr = nbr->iface->area->id.s_addr;
> + rn.addr.s_addr = nbr->addr.s_addr;
>   rn.ifindex = nbr->iface->ifindex;
>   rn.state = nbr->state;
>   rn.self = self;
> @@ -347,6 +348,23 @@ nbr_del(struct nbr *nbr)
>   LIST_REMOVE(nbr, hash);
>  
>   free(nbr);
> +}
> +
> +int

Since the callers of nbr_update_addr() don't use the return value make
this a void function.

> +nbr_update_addr(u_int32_t peerid, struct in_addr addr) {
> +

Fix bad placement of {

> + struct nbr *nbr = NULL;
> +
> + nbr = nbr_find_peerid(peerid);
> + if (nbr == NULL)
> + return (1);

Why passing peerid to this function to look up the neighbor again when the
functions are called like this?
                nbr_update_addr(nbr->peerid, src);

Why not just pass struct nbr *nbr? Would also remove a possible error case.

> +
> + /* XXX neighbor address shouldn't be stored on virtual links */
> + nbr->addr.s_addr = addr.s_addr;
> + ospfe_imsg_compose_rde(IMSG_NEIGHBOR_ADDR, peerid, 0, &addr,
> +    sizeof(addr));

Another option is to just add this ospfe_imsg_compose_rde() to the two
places where the addr is updated.

> +
> + return (0);
>  }
>  
>  struct nbr *
> Index: ospfd.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/ospfd.c,v
> retrieving revision 1.108
> diff -u -p -r1.108 ospfd.c
> --- ospfd.c 16 May 2019 05:49:22 -0000 1.108
> +++ ospfd.c 23 Jun 2019 21:06:44 -0000
> @@ -911,6 +911,22 @@ merge_interfaces(struct area *a, struct
>   if_fsm(i, IF_EVT_UP);
>   }
>  
> + if (i->p2p != xi->p2p) {
> + /* re-add interface to enable or disable DR election */
> + if (ospfd_process == PROC_OSPF_ENGINE)
> + if_fsm(i, IF_EVT_DOWN);
> + else if (ospfd_process == PROC_RDE_ENGINE)
> + rde_nbr_iface_del(i);
> + LIST_REMOVE(i, entry);
> + if_del(i);
> + LIST_REMOVE(xi, entry);
> + LIST_INSERT_HEAD(&a->iface_list, xi, entry);
> + xi->area = a;
> + if (ospfd_process == PROC_OSPF_ENGINE)
> + xi->state = IF_STA_NEW;
> + continue;
> + }

This is one big hammer. Would be nice to be a bit softer, also should this
code be before
                log_debug("merge_interfaces: proc %d area %s merging "
                    "interface %s", ospfd_process, inet_ntoa(a->id), i->name);

Since it re-adds an interface. If so it should also have its own
log_debug(). At least I think it makes little sense to merge most of the
interface to just remove it and re-add it.

> +
>   strlcpy(i->dependon, xi->dependon,
>          sizeof(i->dependon));
>   i->depend_ok = xi->depend_ok;

Unrelated but I feel this code should be moved up before
                if (i->passive != xi->passive) {
                        ...
                }

> Index: ospfd.conf.5
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/ospfd.conf.5,v
> retrieving revision 1.57
> diff -u -p -r1.57 ospfd.conf.5
> --- ospfd.conf.5 10 Jun 2019 06:07:15 -0000 1.57
> +++ ospfd.conf.5 23 Jun 2019 22:10:32 -0000
> @@ -419,6 +419,9 @@ Router.
>  .It Ic transmit-delay Ar seconds
>  Set the transmit delay.
>  The default value is 1; valid range is 1\-3600 seconds.
> +.It Ic type p2p
> +Set the interface type to point to point.
> +This disables the election of a DR and BDR for the given interface.
>  .El
>  .Sh FILES
>  .Bl -tag -width "/etc/ospfd.conf" -compact
> Index: ospfd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/ospfd.h,v
> retrieving revision 1.104
> diff -u -p -r1.104 ospfd.h
> --- ospfd.h 16 May 2019 05:49:22 -0000 1.104
> +++ ospfd.h 21 Sep 2019 10:25:32 -0000
> @@ -107,6 +107,7 @@ enum imsg_type {
>   IMSG_IFINFO,
>   IMSG_NEIGHBOR_UP,
>   IMSG_NEIGHBOR_DOWN,
> + IMSG_NEIGHBOR_ADDR,
>   IMSG_NEIGHBOR_CHANGE,
>   IMSG_NEIGHBOR_CAPA,
>   IMSG_NETWORK_ADD,
> @@ -363,6 +364,7 @@ struct iface {
>   u_int8_t linkstate;
>   u_int8_t priority;
>   u_int8_t passive;
> + u_int8_t p2p;
>  };
>  
>  struct ifaddrchange {
> Index: ospfe.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/ospfe.h,v
> retrieving revision 1.46
> diff -u -p -r1.46 ospfe.h
> --- ospfe.h 25 Oct 2014 03:23:49 -0000 1.46
> +++ ospfe.h 21 Sep 2019 15:27:08 -0000
> @@ -204,6 +204,7 @@ void lsa_cache_put(struct lsa_ref *, s
>  void nbr_init(u_int32_t);
>  struct nbr *nbr_new(u_int32_t, struct iface *, int);
>  void nbr_del(struct nbr *);
> +int nbr_update_addr(u_int32_t, struct in_addr);
>  
>  struct nbr *nbr_find_id(struct iface *, u_int32_t);
>  struct nbr *nbr_find_peerid(u_int32_t);
> Index: parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/parse.y,v
> retrieving revision 1.98
> diff -u -p -r1.98 parse.y
> --- parse.y 7 Jun 2019 04:57:45 -0000 1.98
> +++ parse.y 24 Oct 2019 20:09:11 -0000
> @@ -129,7 +129,7 @@ typedef struct {
>  %token AREA INTERFACE ROUTERID FIBPRIORITY FIBUPDATE REDISTRIBUTE RTLABEL
>  %token RDOMAIN RFC1583COMPAT STUB ROUTER SPFDELAY SPFHOLDTIME EXTTAG
>  %token AUTHKEY AUTHTYPE AUTHMD AUTHMDKEYID
> -%token METRIC PASSIVE
> +%token METRIC P2P PASSIVE
>  %token HELLOINTERVAL FASTHELLOINTERVAL TRANSMITDELAY
>  %token RETRANSMITINTERVAL ROUTERDEADTIME ROUTERPRIORITY
>  %token SET TYPE
> @@ -743,6 +743,10 @@ interfaceopts_l : interfaceopts_l interf
>   ;
>  
>  interfaceoptsl : PASSIVE { iface->passive = 1; }
> + | TYPE P2P {
> + iface->p2p = 1;
> + iface->type = IF_TYPE_POINTOPOINT;
> + }
>   | DEMOTE STRING {
>   if (strlcpy(iface->demote_group, $2,
>      sizeof(iface->demote_group)) >=
> @@ -833,6 +837,7 @@ lookup(char *s)
>   {"msec", MSEC},
>   {"no", NO},
>   {"on", ON},
> + {"p2p", P2P},
>   {"passive", PASSIVE},
>   {"rdomain", RDOMAIN},
>   {"redistribute", REDISTRIBUTE},
> Index: printconf.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/printconf.c,v
> retrieving revision 1.20
> diff -u -p -r1.20 printconf.c
> --- printconf.c 28 Dec 2018 19:25:10 -0000 1.20
> +++ printconf.c 23 Jun 2019 22:05:55 -0000
> @@ -149,6 +149,9 @@ print_iface(struct iface *iface)
>   printf("\t\trouter-priority %d\n", iface->priority);
>   printf("\t\ttransmit-delay %d\n", iface->transmit_delay);
>  
> + if (iface->p2p)
> + printf("\t\ttype p2p\n");
> +
>   printf("\t\tauth-type %s\n", if_auth_name(iface->auth_type));
>   switch (iface->auth_type) {
>   case AUTH_TYPE_NONE:
> Index: rde.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/rde.c,v
> retrieving revision 1.109
> diff -u -p -r1.109 rde.c
> --- rde.c 5 Feb 2018 12:11:28 -0000 1.109
> +++ rde.c 21 Sep 2019 12:17:09 -0000
> @@ -257,6 +257,7 @@ rde_dispatch_imsg(int fd, short event, v
>   struct timespec tp;
>   struct lsa *lsa;
>   struct area *area;
> + struct in_addr addr;
>   struct vertex *v;
>   char *buf;
>   ssize_t n;
> @@ -300,6 +301,17 @@ rde_dispatch_imsg(int fd, short event, v
>   break;
>   case IMSG_NEIGHBOR_DOWN:
>   rde_nbr_del(rde_nbr_find(imsg.hdr.peerid));
> + break;
> + case IMSG_NEIGHBOR_ADDR:
> + if (imsg.hdr.len - IMSG_HEADER_SIZE != sizeof(addr))
> + fatalx("invalid size of OE request");
> + memcpy(&addr, imsg.data, sizeof(addr));
> +
> + nbr = rde_nbr_find(imsg.hdr.peerid);
> + if (nbr == NULL)
> + break;
> +
> + nbr->addr.s_addr = addr.s_addr;
>   break;
>   case IMSG_NEIGHBOR_CHANGE:
>   if (imsg.hdr.len - IMSG_HEADER_SIZE != sizeof(state))
> Index: rde.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/rde.h,v
> retrieving revision 1.39
> diff -u -p -r1.39 rde.h
> --- rde.h 14 Mar 2015 02:22:09 -0000 1.39
> +++ rde.h 19 Sep 2019 13:46:43 -0000
> @@ -66,6 +66,7 @@ struct rde_nbr {
>   LIST_ENTRY(rde_nbr) entry, hash;
>   struct in_addr id;
>   struct in_addr area_id;
> + struct in_addr addr;
>   TAILQ_HEAD(, rde_req_entry) req_list;
>   struct area *area;
>   struct iface *iface;
> Index: rde_spf.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/rde_spf.c,v
> retrieving revision 1.77
> diff -u -p -r1.77 rde_spf.c
> --- rde_spf.c 4 Apr 2019 19:57:08 -0000 1.77
> +++ rde_spf.c 29 Sep 2019 21:08:49 -0000
> @@ -373,6 +373,7 @@ calc_nexthop(struct vertex *dst, struct
>  {
>   struct v_nexthop *vn;
>   struct iface *iface;
> + struct rde_nbr *nbr;
>   int i;
>  
>   /* case 1 */
> @@ -382,10 +383,14 @@ calc_nexthop(struct vertex *dst, struct
>   if (rtr_link->type != LINK_TYPE_POINTTOPOINT)
>   fatalx("inconsistent SPF tree");
>   LIST_FOREACH(iface, &area->iface_list, entry) {
> - if (rtr_link->data == iface->addr.s_addr) {
> - vertex_nexthop_add(dst, parent,
> -    iface->dst.s_addr);
> - return;
> + if (rtr_link->data != iface->addr.s_addr)
> + continue;
> + LIST_FOREACH(nbr, &area->nbr_list, entry) {
> + if (nbr->ifindex == iface->ifindex) {
> + vertex_nexthop_add(dst, parent,
> +    nbr->addr.s_addr);
> + return;
> + }
>   }
>   }
>   fatalx("no interface found for interface");
>

Apart for the comments about nbr_update_addr() this diff is fine by me.

--
:wq Claudio

Reply | Threaded
Open this post in threaded view
|

Re: ospfd: type p2p

Remi Locherer
On Sat, Nov 16, 2019 at 06:58:35AM +0100, Claudio Jeker wrote:

> On Fri, Nov 15, 2019 at 06:06:42PM +0100, Remi Locherer wrote:
> > On Mon, Nov 04, 2019 at 02:01:57PM +0200, Kapetanakis Giannis wrote:
> > > On 25/10/2019 13:57, Remi Locherer wrote:
> > > > Hi tech@,
> > > >
> > > > earlier this year I sent a diff that allowed to change an interface
> > > > from broadcast to point-to-point.
> > > >
> > > > https://marc.info/?l=openbsd-tech&m=156132923203704&w=2
> > > >
> > > > It turned out that this was not sufficient. It made the adjacency
> > > > come up in p2p mode (no selection of DR or BDR) but didn't set a valid
> > > > next hop for routes learned over this p2p link. Actually the next hop was
> > > > 0.0.0.0 which was never installed into the routing table.
> > > >
> > > > This is because for P2P interfaces the neighbor address is not taken from
> > > > the received hello but from the "destination" parameter configured on the
> > > > interface. Since this is not set on a broadcast interface the address is
> > > > 0.0.0.0.
> > > >
> > > > My new diff changes this. Now also for P2P links the IP address of the
> > > > neighbor is taken from the hello packets (src address). This on it's own
> > > > would make it simpler to interfere with the routing from remote. One could
> > > > send unicast ospf hello messages and potentially disrupt the routing setup.
> > > > I believe I mitigated this with an additional check I committed in August:
> > > > only hello messages sent to the multicast address are now processed.
> > > >
> > > > The config looks like this:
> > > >
> > > > area 0.0.0.0 {
> > > > interface em0 {
> > > > type p2p
> > > > }
> > > > }
> > > >
> > > > It would be nice to get test reports for this new feature (check the fib
> > > > and routing table!) and also test reports with real p2p2 interfaces (gif
> > > > or gre).
> > > >
> > > > Of course OKs are also welcome. ;-)
> > > >
> > > > Remi
> > >
> > >
> > > Hi,
> > >
> > > From first test seems to work :)
> > >
> > > looking forward test it for IPv6 as well
> > >
> > > thanks
> > >
> > > Giannis
> >
> >
> > Anyone willing to OK this?
>
> See inline.
>

[...]

>
> Another option is to just add this ospfe_imsg_compose_rde() to the two
> places where the addr is updated.

Right, that is actually simpler.

>
> > +
> > + return (0);
> >  }
> >  
> >  struct nbr *
> > Index: ospfd.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/ospfd/ospfd.c,v
> > retrieving revision 1.108
> > diff -u -p -r1.108 ospfd.c
> > --- ospfd.c 16 May 2019 05:49:22 -0000 1.108
> > +++ ospfd.c 23 Jun 2019 21:06:44 -0000
> > @@ -911,6 +911,22 @@ merge_interfaces(struct area *a, struct
> >   if_fsm(i, IF_EVT_UP);
> >   }
> >  
> > + if (i->p2p != xi->p2p) {
> > + /* re-add interface to enable or disable DR election */
> > + if (ospfd_process == PROC_OSPF_ENGINE)
> > + if_fsm(i, IF_EVT_DOWN);
> > + else if (ospfd_process == PROC_RDE_ENGINE)
> > + rde_nbr_iface_del(i);
> > + LIST_REMOVE(i, entry);
> > + if_del(i);
> > + LIST_REMOVE(xi, entry);
> > + LIST_INSERT_HEAD(&a->iface_list, xi, entry);
> > + xi->area = a;
> > + if (ospfd_process == PROC_OSPF_ENGINE)
> > + xi->state = IF_STA_NEW;
> > + continue;
> > + }
>
> This is one big hammer. Would be nice to be a bit softer, also should this
> code be before
> log_debug("merge_interfaces: proc %d area %s merging "
>    "interface %s", ospfd_process, inet_ntoa(a->id), i->name);
>
> Since it re-adds an interface. If so it should also have its own
> log_debug(). At least I think it makes little sense to merge most of the
> interface to just remove it and re-add it.

I somehow added this with my first attempt and didn't re-evaluate. It turns
out that a restart is actually sufficient.

>
> > +
> >   strlcpy(i->dependon, xi->dependon,
> >          sizeof(i->dependon));
> >   i->depend_ok = xi->depend_ok;
>
> Unrelated but I feel this code should be moved up before
> if (i->passive != xi->passive) {
> ...
> }

Yes, I'll send a separate diff for that later.

OK for the new diff?



Index: hello.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/hello.c,v
retrieving revision 1.24
diff -u -p -r1.24 hello.c
--- hello.c 12 Aug 2019 20:21:58 -0000 1.24
+++ hello.c 16 Nov 2019 14:52:10 -0000
@@ -191,12 +191,16 @@ recv_hello(struct iface *iface, struct i
  nbr->priority = hello.rtr_priority;
  /* XXX neighbor address shouldn't be stored on virtual links */
  nbr->addr.s_addr = src.s_addr;
+ ospfe_imsg_compose_rde(IMSG_NEIGHBOR_ADDR, nbr->peerid, 0,
+    &src, sizeof(src));
  }
 
  if (nbr->addr.s_addr != src.s_addr) {
  log_warnx("%s: neighbor ID %s changed its IP address",
     __func__, inet_ntoa(nbr->id));
  nbr->addr.s_addr = src.s_addr;
+ ospfe_imsg_compose_rde(IMSG_NEIGHBOR_ADDR, nbr->peerid, 0,
+    &src, sizeof(src));
  }
 
  nbr->options = hello.opts;
Index: lsupdate.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/lsupdate.c,v
retrieving revision 1.46
diff -u -p -r1.46 lsupdate.c
--- lsupdate.c 15 Jul 2019 18:26:39 -0000 1.46
+++ lsupdate.c 15 Aug 2019 21:10:13 -0000
@@ -470,7 +470,7 @@ ls_retrans_timer(int fd, short event, vo
  /* ls_retrans_list_free retriggers the timer */
  return;
  } else if (nbr->iface->type == IF_TYPE_POINTOPOINT)
- memcpy(&addr, &nbr->iface->dst, sizeof(addr));
+ memcpy(&addr, &nbr->addr, sizeof(addr));
  else
  inet_aton(AllDRouters, &addr);
  } else
Index: neighbor.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/neighbor.c,v
retrieving revision 1.48
diff -u -p -r1.48 neighbor.c
--- neighbor.c 9 Feb 2018 02:14:03 -0000 1.48
+++ neighbor.c 16 Nov 2019 14:53:52 -0000
@@ -312,6 +312,7 @@ nbr_new(u_int32_t nbr_id, struct iface *
  bzero(&rn, sizeof(rn));
  rn.id.s_addr = nbr->id.s_addr;
  rn.area_id.s_addr = nbr->iface->area->id.s_addr;
+ rn.addr.s_addr = nbr->addr.s_addr;
  rn.ifindex = nbr->iface->ifindex;
  rn.state = nbr->state;
  rn.self = self;
Index: ospfd.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/ospfd.c,v
retrieving revision 1.108
diff -u -p -r1.108 ospfd.c
--- ospfd.c 16 May 2019 05:49:22 -0000 1.108
+++ ospfd.c 16 Nov 2019 21:36:45 -0000
@@ -911,6 +911,15 @@ merge_interfaces(struct area *a, struct
  if_fsm(i, IF_EVT_UP);
  }
 
+ if (i->p2p != xi->p2p) {
+ /* restart interface to enable or disable DR election */
+ if (ospfd_process == PROC_OSPF_ENGINE)
+ if_fsm(i, IF_EVT_DOWN);
+ i->p2p = xi->p2p;
+ if (ospfd_process == PROC_OSPF_ENGINE)
+ if_fsm(i, IF_EVT_UP);
+ }
+
  strlcpy(i->dependon, xi->dependon,
         sizeof(i->dependon));
  i->depend_ok = xi->depend_ok;
Index: ospfd.conf.5
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/ospfd.conf.5,v
retrieving revision 1.57
diff -u -p -r1.57 ospfd.conf.5
--- ospfd.conf.5 10 Jun 2019 06:07:15 -0000 1.57
+++ ospfd.conf.5 23 Jun 2019 22:10:32 -0000
@@ -419,6 +419,9 @@ Router.
 .It Ic transmit-delay Ar seconds
 Set the transmit delay.
 The default value is 1; valid range is 1\-3600 seconds.
+.It Ic type p2p
+Set the interface type to point to point.
+This disables the election of a DR and BDR for the given interface.
 .El
 .Sh FILES
 .Bl -tag -width "/etc/ospfd.conf" -compact
Index: ospfd.h
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/ospfd.h,v
retrieving revision 1.104
diff -u -p -r1.104 ospfd.h
--- ospfd.h 16 May 2019 05:49:22 -0000 1.104
+++ ospfd.h 21 Sep 2019 10:25:32 -0000
@@ -107,6 +107,7 @@ enum imsg_type {
  IMSG_IFINFO,
  IMSG_NEIGHBOR_UP,
  IMSG_NEIGHBOR_DOWN,
+ IMSG_NEIGHBOR_ADDR,
  IMSG_NEIGHBOR_CHANGE,
  IMSG_NEIGHBOR_CAPA,
  IMSG_NETWORK_ADD,
@@ -363,6 +364,7 @@ struct iface {
  u_int8_t linkstate;
  u_int8_t priority;
  u_int8_t passive;
+ u_int8_t p2p;
 };
 
 struct ifaddrchange {
Index: parse.y
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/parse.y,v
retrieving revision 1.98
diff -u -p -r1.98 parse.y
--- parse.y 7 Jun 2019 04:57:45 -0000 1.98
+++ parse.y 24 Oct 2019 20:09:11 -0000
@@ -129,7 +129,7 @@ typedef struct {
 %token AREA INTERFACE ROUTERID FIBPRIORITY FIBUPDATE REDISTRIBUTE RTLABEL
 %token RDOMAIN RFC1583COMPAT STUB ROUTER SPFDELAY SPFHOLDTIME EXTTAG
 %token AUTHKEY AUTHTYPE AUTHMD AUTHMDKEYID
-%token METRIC PASSIVE
+%token METRIC P2P PASSIVE
 %token HELLOINTERVAL FASTHELLOINTERVAL TRANSMITDELAY
 %token RETRANSMITINTERVAL ROUTERDEADTIME ROUTERPRIORITY
 %token SET TYPE
@@ -743,6 +743,10 @@ interfaceopts_l : interfaceopts_l interf
  ;
 
 interfaceoptsl : PASSIVE { iface->passive = 1; }
+ | TYPE P2P {
+ iface->p2p = 1;
+ iface->type = IF_TYPE_POINTOPOINT;
+ }
  | DEMOTE STRING {
  if (strlcpy(iface->demote_group, $2,
     sizeof(iface->demote_group)) >=
@@ -833,6 +837,7 @@ lookup(char *s)
  {"msec", MSEC},
  {"no", NO},
  {"on", ON},
+ {"p2p", P2P},
  {"passive", PASSIVE},
  {"rdomain", RDOMAIN},
  {"redistribute", REDISTRIBUTE},
Index: printconf.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/printconf.c,v
retrieving revision 1.20
diff -u -p -r1.20 printconf.c
--- printconf.c 28 Dec 2018 19:25:10 -0000 1.20
+++ printconf.c 23 Jun 2019 22:05:55 -0000
@@ -149,6 +149,9 @@ print_iface(struct iface *iface)
  printf("\t\trouter-priority %d\n", iface->priority);
  printf("\t\ttransmit-delay %d\n", iface->transmit_delay);
 
+ if (iface->p2p)
+ printf("\t\ttype p2p\n");
+
  printf("\t\tauth-type %s\n", if_auth_name(iface->auth_type));
  switch (iface->auth_type) {
  case AUTH_TYPE_NONE:
Index: rde.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/rde.c,v
retrieving revision 1.109
diff -u -p -r1.109 rde.c
--- rde.c 5 Feb 2018 12:11:28 -0000 1.109
+++ rde.c 21 Sep 2019 12:17:09 -0000
@@ -257,6 +257,7 @@ rde_dispatch_imsg(int fd, short event, v
  struct timespec tp;
  struct lsa *lsa;
  struct area *area;
+ struct in_addr addr;
  struct vertex *v;
  char *buf;
  ssize_t n;
@@ -300,6 +301,17 @@ rde_dispatch_imsg(int fd, short event, v
  break;
  case IMSG_NEIGHBOR_DOWN:
  rde_nbr_del(rde_nbr_find(imsg.hdr.peerid));
+ break;
+ case IMSG_NEIGHBOR_ADDR:
+ if (imsg.hdr.len - IMSG_HEADER_SIZE != sizeof(addr))
+ fatalx("invalid size of OE request");
+ memcpy(&addr, imsg.data, sizeof(addr));
+
+ nbr = rde_nbr_find(imsg.hdr.peerid);
+ if (nbr == NULL)
+ break;
+
+ nbr->addr.s_addr = addr.s_addr;
  break;
  case IMSG_NEIGHBOR_CHANGE:
  if (imsg.hdr.len - IMSG_HEADER_SIZE != sizeof(state))
Index: rde.h
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/rde.h,v
retrieving revision 1.39
diff -u -p -r1.39 rde.h
--- rde.h 14 Mar 2015 02:22:09 -0000 1.39
+++ rde.h 19 Sep 2019 13:46:43 -0000
@@ -66,6 +66,7 @@ struct rde_nbr {
  LIST_ENTRY(rde_nbr) entry, hash;
  struct in_addr id;
  struct in_addr area_id;
+ struct in_addr addr;
  TAILQ_HEAD(, rde_req_entry) req_list;
  struct area *area;
  struct iface *iface;
Index: rde_spf.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/rde_spf.c,v
retrieving revision 1.77
diff -u -p -r1.77 rde_spf.c
--- rde_spf.c 4 Apr 2019 19:57:08 -0000 1.77
+++ rde_spf.c 29 Sep 2019 21:08:49 -0000
@@ -373,6 +373,7 @@ calc_nexthop(struct vertex *dst, struct
 {
  struct v_nexthop *vn;
  struct iface *iface;
+ struct rde_nbr *nbr;
  int i;
 
  /* case 1 */
@@ -382,10 +383,14 @@ calc_nexthop(struct vertex *dst, struct
  if (rtr_link->type != LINK_TYPE_POINTTOPOINT)
  fatalx("inconsistent SPF tree");
  LIST_FOREACH(iface, &area->iface_list, entry) {
- if (rtr_link->data == iface->addr.s_addr) {
- vertex_nexthop_add(dst, parent,
-    iface->dst.s_addr);
- return;
+ if (rtr_link->data != iface->addr.s_addr)
+ continue;
+ LIST_FOREACH(nbr, &area->nbr_list, entry) {
+ if (nbr->ifindex == iface->ifindex) {
+ vertex_nexthop_add(dst, parent,
+    nbr->addr.s_addr);
+ return;
+ }
  }
  }
  fatalx("no interface found for interface");

Reply | Threaded
Open this post in threaded view
|

Re: ospfd: type p2p

Claudio Jeker
On Sun, Nov 17, 2019 at 12:44:44PM +0100, Remi Locherer wrote:

> On Sat, Nov 16, 2019 at 06:58:35AM +0100, Claudio Jeker wrote:
> > On Fri, Nov 15, 2019 at 06:06:42PM +0100, Remi Locherer wrote:
> > > On Mon, Nov 04, 2019 at 02:01:57PM +0200, Kapetanakis Giannis wrote:
> > > > On 25/10/2019 13:57, Remi Locherer wrote:
> > > > > Hi tech@,
> > > > >
> > > > > earlier this year I sent a diff that allowed to change an interface
> > > > > from broadcast to point-to-point.
> > > > >
> > > > > https://marc.info/?l=openbsd-tech&m=156132923203704&w=2
> > > > >
> > > > > It turned out that this was not sufficient. It made the adjacency
> > > > > come up in p2p mode (no selection of DR or BDR) but didn't set a valid
> > > > > next hop for routes learned over this p2p link. Actually the next hop was
> > > > > 0.0.0.0 which was never installed into the routing table.
> > > > >
> > > > > This is because for P2P interfaces the neighbor address is not taken from
> > > > > the received hello but from the "destination" parameter configured on the
> > > > > interface. Since this is not set on a broadcast interface the address is
> > > > > 0.0.0.0.
> > > > >
> > > > > My new diff changes this. Now also for P2P links the IP address of the
> > > > > neighbor is taken from the hello packets (src address). This on it's own
> > > > > would make it simpler to interfere with the routing from remote. One could
> > > > > send unicast ospf hello messages and potentially disrupt the routing setup.
> > > > > I believe I mitigated this with an additional check I committed in August:
> > > > > only hello messages sent to the multicast address are now processed.
> > > > >
> > > > > The config looks like this:
> > > > >
> > > > > area 0.0.0.0 {
> > > > > interface em0 {
> > > > > type p2p
> > > > > }
> > > > > }
> > > > >
> > > > > It would be nice to get test reports for this new feature (check the fib
> > > > > and routing table!) and also test reports with real p2p2 interfaces (gif
> > > > > or gre).
> > > > >
> > > > > Of course OKs are also welcome. ;-)
> > > > >
> > > > > Remi
> > > >
> > > >
> > > > Hi,
> > > >
> > > > From first test seems to work :)
> > > >
> > > > looking forward test it for IPv6 as well
> > > >
> > > > thanks
> > > >
> > > > Giannis
> > >
> > >
> > > Anyone willing to OK this?
> >
> > See inline.
> >
>
> [...]
>
> >
> > Another option is to just add this ospfe_imsg_compose_rde() to the two
> > places where the addr is updated.
>
> Right, that is actually simpler.
>
> >
> > > +
> > > + return (0);
> > >  }
> > >  
> > >  struct nbr *
> > > Index: ospfd.c
> > > ===================================================================
> > > RCS file: /cvs/src/usr.sbin/ospfd/ospfd.c,v
> > > retrieving revision 1.108
> > > diff -u -p -r1.108 ospfd.c
> > > --- ospfd.c 16 May 2019 05:49:22 -0000 1.108
> > > +++ ospfd.c 23 Jun 2019 21:06:44 -0000
> > > @@ -911,6 +911,22 @@ merge_interfaces(struct area *a, struct
> > >   if_fsm(i, IF_EVT_UP);
> > >   }
> > >  
> > > + if (i->p2p != xi->p2p) {
> > > + /* re-add interface to enable or disable DR election */
> > > + if (ospfd_process == PROC_OSPF_ENGINE)
> > > + if_fsm(i, IF_EVT_DOWN);
> > > + else if (ospfd_process == PROC_RDE_ENGINE)
> > > + rde_nbr_iface_del(i);
> > > + LIST_REMOVE(i, entry);
> > > + if_del(i);
> > > + LIST_REMOVE(xi, entry);
> > > + LIST_INSERT_HEAD(&a->iface_list, xi, entry);
> > > + xi->area = a;
> > > + if (ospfd_process == PROC_OSPF_ENGINE)
> > > + xi->state = IF_STA_NEW;
> > > + continue;
> > > + }
> >
> > This is one big hammer. Would be nice to be a bit softer, also should this
> > code be before
> > log_debug("merge_interfaces: proc %d area %s merging "
> >    "interface %s", ospfd_process, inet_ntoa(a->id), i->name);
> >
> > Since it re-adds an interface. If so it should also have its own
> > log_debug(). At least I think it makes little sense to merge most of the
> > interface to just remove it and re-add it.
>
> I somehow added this with my first attempt and didn't re-evaluate. It turns
> out that a restart is actually sufficient.
>
> >
> > > +
> > >   strlcpy(i->dependon, xi->dependon,
> > >          sizeof(i->dependon));
> > >   i->depend_ok = xi->depend_ok;
> >
> > Unrelated but I feel this code should be moved up before
> > if (i->passive != xi->passive) {
> > ...
> > }
>
> Yes, I'll send a separate diff for that later.
>
> OK for the new diff?
>
>
>
> Index: hello.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/hello.c,v
> retrieving revision 1.24
> diff -u -p -r1.24 hello.c
> --- hello.c 12 Aug 2019 20:21:58 -0000 1.24
> +++ hello.c 16 Nov 2019 14:52:10 -0000
> @@ -191,12 +191,16 @@ recv_hello(struct iface *iface, struct i
>   nbr->priority = hello.rtr_priority;
>   /* XXX neighbor address shouldn't be stored on virtual links */
>   nbr->addr.s_addr = src.s_addr;
> + ospfe_imsg_compose_rde(IMSG_NEIGHBOR_ADDR, nbr->peerid, 0,
> +    &src, sizeof(src));
>   }
>  
>   if (nbr->addr.s_addr != src.s_addr) {
>   log_warnx("%s: neighbor ID %s changed its IP address",
>      __func__, inet_ntoa(nbr->id));
>   nbr->addr.s_addr = src.s_addr;
> + ospfe_imsg_compose_rde(IMSG_NEIGHBOR_ADDR, nbr->peerid, 0,
> +    &src, sizeof(src));
>   }
>  
>   nbr->options = hello.opts;
> Index: lsupdate.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/lsupdate.c,v
> retrieving revision 1.46
> diff -u -p -r1.46 lsupdate.c
> --- lsupdate.c 15 Jul 2019 18:26:39 -0000 1.46
> +++ lsupdate.c 15 Aug 2019 21:10:13 -0000
> @@ -470,7 +470,7 @@ ls_retrans_timer(int fd, short event, vo
>   /* ls_retrans_list_free retriggers the timer */
>   return;
>   } else if (nbr->iface->type == IF_TYPE_POINTOPOINT)
> - memcpy(&addr, &nbr->iface->dst, sizeof(addr));
> + memcpy(&addr, &nbr->addr, sizeof(addr));
>   else
>   inet_aton(AllDRouters, &addr);
>   } else
> Index: neighbor.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/neighbor.c,v
> retrieving revision 1.48
> diff -u -p -r1.48 neighbor.c
> --- neighbor.c 9 Feb 2018 02:14:03 -0000 1.48
> +++ neighbor.c 16 Nov 2019 14:53:52 -0000
> @@ -312,6 +312,7 @@ nbr_new(u_int32_t nbr_id, struct iface *
>   bzero(&rn, sizeof(rn));
>   rn.id.s_addr = nbr->id.s_addr;
>   rn.area_id.s_addr = nbr->iface->area->id.s_addr;
> + rn.addr.s_addr = nbr->addr.s_addr;
>   rn.ifindex = nbr->iface->ifindex;
>   rn.state = nbr->state;
>   rn.self = self;
> Index: ospfd.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/ospfd.c,v
> retrieving revision 1.108
> diff -u -p -r1.108 ospfd.c
> --- ospfd.c 16 May 2019 05:49:22 -0000 1.108
> +++ ospfd.c 16 Nov 2019 21:36:45 -0000
> @@ -911,6 +911,15 @@ merge_interfaces(struct area *a, struct
>   if_fsm(i, IF_EVT_UP);
>   }
>  
> + if (i->p2p != xi->p2p) {
> + /* restart interface to enable or disable DR election */
> + if (ospfd_process == PROC_OSPF_ENGINE)
> + if_fsm(i, IF_EVT_DOWN);
> + i->p2p = xi->p2p;
> + if (ospfd_process == PROC_OSPF_ENGINE)
> + if_fsm(i, IF_EVT_UP);
> + }
> +
>   strlcpy(i->dependon, xi->dependon,
>          sizeof(i->dependon));
>   i->depend_ok = xi->depend_ok;
> Index: ospfd.conf.5
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/ospfd.conf.5,v
> retrieving revision 1.57
> diff -u -p -r1.57 ospfd.conf.5
> --- ospfd.conf.5 10 Jun 2019 06:07:15 -0000 1.57
> +++ ospfd.conf.5 23 Jun 2019 22:10:32 -0000
> @@ -419,6 +419,9 @@ Router.
>  .It Ic transmit-delay Ar seconds
>  Set the transmit delay.
>  The default value is 1; valid range is 1\-3600 seconds.
> +.It Ic type p2p
> +Set the interface type to point to point.
> +This disables the election of a DR and BDR for the given interface.
>  .El
>  .Sh FILES
>  .Bl -tag -width "/etc/ospfd.conf" -compact
> Index: ospfd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/ospfd.h,v
> retrieving revision 1.104
> diff -u -p -r1.104 ospfd.h
> --- ospfd.h 16 May 2019 05:49:22 -0000 1.104
> +++ ospfd.h 21 Sep 2019 10:25:32 -0000
> @@ -107,6 +107,7 @@ enum imsg_type {
>   IMSG_IFINFO,
>   IMSG_NEIGHBOR_UP,
>   IMSG_NEIGHBOR_DOWN,
> + IMSG_NEIGHBOR_ADDR,
>   IMSG_NEIGHBOR_CHANGE,
>   IMSG_NEIGHBOR_CAPA,
>   IMSG_NETWORK_ADD,
> @@ -363,6 +364,7 @@ struct iface {
>   u_int8_t linkstate;
>   u_int8_t priority;
>   u_int8_t passive;
> + u_int8_t p2p;
>  };
>  
>  struct ifaddrchange {
> Index: parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/parse.y,v
> retrieving revision 1.98
> diff -u -p -r1.98 parse.y
> --- parse.y 7 Jun 2019 04:57:45 -0000 1.98
> +++ parse.y 24 Oct 2019 20:09:11 -0000
> @@ -129,7 +129,7 @@ typedef struct {
>  %token AREA INTERFACE ROUTERID FIBPRIORITY FIBUPDATE REDISTRIBUTE RTLABEL
>  %token RDOMAIN RFC1583COMPAT STUB ROUTER SPFDELAY SPFHOLDTIME EXTTAG
>  %token AUTHKEY AUTHTYPE AUTHMD AUTHMDKEYID
> -%token METRIC PASSIVE
> +%token METRIC P2P PASSIVE
>  %token HELLOINTERVAL FASTHELLOINTERVAL TRANSMITDELAY
>  %token RETRANSMITINTERVAL ROUTERDEADTIME ROUTERPRIORITY
>  %token SET TYPE
> @@ -743,6 +743,10 @@ interfaceopts_l : interfaceopts_l interf
>   ;
>  
>  interfaceoptsl : PASSIVE { iface->passive = 1; }
> + | TYPE P2P {
> + iface->p2p = 1;
> + iface->type = IF_TYPE_POINTOPOINT;
> + }
>   | DEMOTE STRING {
>   if (strlcpy(iface->demote_group, $2,
>      sizeof(iface->demote_group)) >=
> @@ -833,6 +837,7 @@ lookup(char *s)
>   {"msec", MSEC},
>   {"no", NO},
>   {"on", ON},
> + {"p2p", P2P},
>   {"passive", PASSIVE},
>   {"rdomain", RDOMAIN},
>   {"redistribute", REDISTRIBUTE},
> Index: printconf.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/printconf.c,v
> retrieving revision 1.20
> diff -u -p -r1.20 printconf.c
> --- printconf.c 28 Dec 2018 19:25:10 -0000 1.20
> +++ printconf.c 23 Jun 2019 22:05:55 -0000
> @@ -149,6 +149,9 @@ print_iface(struct iface *iface)
>   printf("\t\trouter-priority %d\n", iface->priority);
>   printf("\t\ttransmit-delay %d\n", iface->transmit_delay);
>  
> + if (iface->p2p)
> + printf("\t\ttype p2p\n");
> +
>   printf("\t\tauth-type %s\n", if_auth_name(iface->auth_type));
>   switch (iface->auth_type) {
>   case AUTH_TYPE_NONE:
> Index: rde.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/rde.c,v
> retrieving revision 1.109
> diff -u -p -r1.109 rde.c
> --- rde.c 5 Feb 2018 12:11:28 -0000 1.109
> +++ rde.c 21 Sep 2019 12:17:09 -0000
> @@ -257,6 +257,7 @@ rde_dispatch_imsg(int fd, short event, v
>   struct timespec tp;
>   struct lsa *lsa;
>   struct area *area;
> + struct in_addr addr;
>   struct vertex *v;
>   char *buf;
>   ssize_t n;
> @@ -300,6 +301,17 @@ rde_dispatch_imsg(int fd, short event, v
>   break;
>   case IMSG_NEIGHBOR_DOWN:
>   rde_nbr_del(rde_nbr_find(imsg.hdr.peerid));
> + break;
> + case IMSG_NEIGHBOR_ADDR:
> + if (imsg.hdr.len - IMSG_HEADER_SIZE != sizeof(addr))
> + fatalx("invalid size of OE request");
> + memcpy(&addr, imsg.data, sizeof(addr));
> +
> + nbr = rde_nbr_find(imsg.hdr.peerid);
> + if (nbr == NULL)
> + break;
> +
> + nbr->addr.s_addr = addr.s_addr;
>   break;
>   case IMSG_NEIGHBOR_CHANGE:
>   if (imsg.hdr.len - IMSG_HEADER_SIZE != sizeof(state))
> Index: rde.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/rde.h,v
> retrieving revision 1.39
> diff -u -p -r1.39 rde.h
> --- rde.h 14 Mar 2015 02:22:09 -0000 1.39
> +++ rde.h 19 Sep 2019 13:46:43 -0000
> @@ -66,6 +66,7 @@ struct rde_nbr {
>   LIST_ENTRY(rde_nbr) entry, hash;
>   struct in_addr id;
>   struct in_addr area_id;
> + struct in_addr addr;
>   TAILQ_HEAD(, rde_req_entry) req_list;
>   struct area *area;
>   struct iface *iface;
> Index: rde_spf.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/rde_spf.c,v
> retrieving revision 1.77
> diff -u -p -r1.77 rde_spf.c
> --- rde_spf.c 4 Apr 2019 19:57:08 -0000 1.77
> +++ rde_spf.c 29 Sep 2019 21:08:49 -0000
> @@ -373,6 +373,7 @@ calc_nexthop(struct vertex *dst, struct
>  {
>   struct v_nexthop *vn;
>   struct iface *iface;
> + struct rde_nbr *nbr;
>   int i;
>  
>   /* case 1 */
> @@ -382,10 +383,14 @@ calc_nexthop(struct vertex *dst, struct
>   if (rtr_link->type != LINK_TYPE_POINTTOPOINT)
>   fatalx("inconsistent SPF tree");
>   LIST_FOREACH(iface, &area->iface_list, entry) {
> - if (rtr_link->data == iface->addr.s_addr) {
> - vertex_nexthop_add(dst, parent,
> -    iface->dst.s_addr);
> - return;
> + if (rtr_link->data != iface->addr.s_addr)
> + continue;
> + LIST_FOREACH(nbr, &area->nbr_list, entry) {
> + if (nbr->ifindex == iface->ifindex) {
> + vertex_nexthop_add(dst, parent,
> +    nbr->addr.s_addr);
> + return;
> + }
>   }
>   }
>   fatalx("no interface found for interface");
>

Yes, this is OK claudio@

--
:wq Claudio

Reply | Threaded
Open this post in threaded view
|

Re: ospfd: type p2p

Kapetanakis Giannis
In reply to this post by Remi Locherer
On 17/11/2019 13:44, Remi Locherer wrote:
> Yes, I'll send a separate diff for that later.
>
> OK for the new diff?


Works for me.

G