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");
