Re: NMEA: add more gps sensor values (2nd take)
> so now that the sensor types got adjusted, here's the tty_nmea.c diff
> again, adding 7 new sensors:
> - # satellites (raw)
> - GPS/DGPS fix (or not) (raw)
> - HDOP/VDOP/PDOP (raw)
> - Altitude (m)
> - Speed (m/s)
Yay sensors!
> i'd welcome eyes on the logic, and especially on the nmea_atoi()
> implementation - it is inspired by nmea_degrees() just below but it
> feels awkward. I can of course only add altitude/speed as a start..
I had a stab at it. Read ok, but I also tested it. If this is the
desired output in the corner cases (e.g. ".1"), then its OK by me.
%--
$ cat test_nmea_atoi.c
#include
#include
int
nmea_atoi(int64_t *dst, char *src, int decimal)
{
char *p;
int i = 3; /* take 3 digits */
*dst = 0;
for (p = src; *p && *p != '.' && *p >= '0' && *p <= '9' ; )
*dst = *dst * 10 + (*p++ - '0');
if (! decimal)
return (0);
/* *p should be '.' at that point */
if (*p != '.')
return (-1);/* no decimal point, or bogus value ? */
p++;
/* read digits after decimal point */
for (; *p && i > 0 && *p >= '0' && *p <= '9' ; i--)
*dst = *dst * 10 + (*p++ - '0');
for (; i > 0 ; i--)
*dst *= 10;
return 0;
}
void
test_atoi(char *str)
{
int64_t i64;
nmea_atoi(, str, 1);
printf("%lld\n", i64);
}
int main()
{
test_atoi("123");
test_atoi("123.1");
test_atoi(".1");
test_atoi("123.456");
test_atoi("1.3");
test_atoi("landry");
test_atoi("1.2.3");
test_atoi(".1.2.3");
return 0;
}
$
$ cc test_nmea_atoi.c -o test_nmea_atoi && ./test_nmea_atoi
123
123100
100
123456
1333
0
1200
100
%--
> And testing with other nmea devices would be great ! So far i've tested
> this in various static locations and recording data points when driving,
> and it matched the gps data from my smartphone.
Unfortunately I don't have any devices (that I know of) to test.
The diff looks OK to me. The only thing that bugs me is a few scalar
values that you use instead of defines. Mainly fld[] entries and vdop
checks.
> Index: tty_nmea.c
> ===
> RCS file: /cvs/src/sys/kern/tty_nmea.c,v
> retrieving revision 1.47
> diff -u -r1.47 tty_nmea.c
> --- tty_nmea.c1 Sep 2018 06:09:26 - 1.47
> +++ tty_nmea.c28 Dec 2018 20:47:20 -
> @@ -29,6 +29,10 @@
> #ifdef NMEA_DEBUG
> #define DPRINTFN(n, x) do { if (nmeadebug > (n)) printf x; } while (0)
> int nmeadebug = 0;
> +/*
> + * 1 = print interesting messages
> + * 2 = print all messages
> + */
> #else
> #define DPRINTFN(n, x)
> #endif
> @@ -52,6 +56,13 @@
> struct ksensor signal; /* signal status */
> struct ksensor latitude;
> struct ksensor longitude;
> + struct ksensor altitude;
> + struct ksensor speed;
> + struct ksensor nsat;
> + struct ksensor quality;
> + struct ksensor hdop;
> + struct ksensor vdop;
> + struct ksensor pdop;
> struct ksensordev timedev;
> struct timespec ts; /* current timestamp */
> struct timespec lts;/* timestamp of last '$' seen */
> @@ -70,6 +81,8 @@
> /* NMEA decoding */
> void nmea_scan(struct nmea *, struct tty *);
> void nmea_gprmc(struct nmea *, struct tty *, char *fld[], int fldcnt);
> +void nmea_decode_gsa(struct nmea *, struct tty *, char *fld[], int fldcnt);
> +void nmea_decode_gga(struct nmea *, struct tty *, char *fld[], int fldcnt);
>
> /* date and time conversion */
> int nmea_date_to_nano(char *s, int64_t *nano);
> @@ -77,6 +90,7 @@
>
> /* longitude and latitude conversion */
> int nmea_degrees(int64_t *dst, char *src, int neg);
> +int nmea_atoi(int64_t *dst, char *src, int decimal);
>
> /* degrade the timedelta sensor */
> void nmea_timeout(void *);
> @@ -126,6 +140,55 @@
> strlcpy(np->longitude.desc, "Longitude", sizeof(np->longitude.desc));
> sensor_attach(>timedev, >longitude);
>
> + np->altitude.type = SENSOR_DISTANCE;
> + np->altitude.status = SENSOR_S_UNKNOWN;
> + np->altitude.flags = SENSOR_FINVALID;
> + np->altitude.value = 0;
> + strlcpy(np->altitude.desc, "Altitude", sizeof(np->altitude.desc));
> + sensor_attach(>timedev, >altitude);
> +
> + np->speed.type = SENSOR_VELOCITY;
> + np->speed.status = SENSOR_S_UNKNOWN;
> + np->speed.flags = SENSOR_FINVALID;
> + np->speed.value = 0;
> + strlcpy(np->speed.desc, "Ground speed", sizeof(np->speed.desc));
> + sensor_attach(>timedev, >speed);
> +
> + np->nsat.type = SENSOR_INTEGER;
> + np->nsat.status = SENSOR_S_UNKNOWN;
NMEA: add more gps sensor values (2nd take)
Hi,
so now that the sensor types got adjusted, here's the tty_nmea.c diff
again, adding 7 new sensors:
- # satellites (raw)
- GPS/DGPS fix (or not) (raw)
- HDOP/VDOP/PDOP (raw)
- Altitude (m)
- Speed (m/s)
i'd welcome eyes on the logic, and especially on the nmea_atoi()
implementation - it is inspired by nmea_degrees() just below but it
feels awkward. I can of course only add altitude/speed as a start..
And testing with other nmea devices would be great ! So far i've tested
this in various static locations and recording data points when driving,
and it matched the gps data from my smartphone.
Landry
Index: tty_nmea.c
===
RCS file: /cvs/src/sys/kern/tty_nmea.c,v
retrieving revision 1.47
diff -u -r1.47 tty_nmea.c
--- tty_nmea.c 1 Sep 2018 06:09:26 - 1.47
+++ tty_nmea.c 28 Dec 2018 20:47:20 -
@@ -29,6 +29,10 @@
#ifdef NMEA_DEBUG
#define DPRINTFN(n, x) do { if (nmeadebug > (n)) printf x; } while (0)
int nmeadebug = 0;
+/*
+ * 1 = print interesting messages
+ * 2 = print all messages
+ */
#else
#define DPRINTFN(n, x)
#endif
@@ -52,6 +56,13 @@
struct ksensor signal; /* signal status */
struct ksensor latitude;
struct ksensor longitude;
+ struct ksensor altitude;
+ struct ksensor speed;
+ struct ksensor nsat;
+ struct ksensor quality;
+ struct ksensor hdop;
+ struct ksensor vdop;
+ struct ksensor pdop;
struct ksensordev timedev;
struct timespec ts; /* current timestamp */
struct timespec lts;/* timestamp of last '$' seen */
@@ -70,6 +81,8 @@
/* NMEA decoding */
void nmea_scan(struct nmea *, struct tty *);
void nmea_gprmc(struct nmea *, struct tty *, char *fld[], int fldcnt);
+void nmea_decode_gsa(struct nmea *, struct tty *, char *fld[], int fldcnt);
+void nmea_decode_gga(struct nmea *, struct tty *, char *fld[], int fldcnt);
/* date and time conversion */
intnmea_date_to_nano(char *s, int64_t *nano);
@@ -77,6 +90,7 @@
/* longitude and latitude conversion */
intnmea_degrees(int64_t *dst, char *src, int neg);
+intnmea_atoi(int64_t *dst, char *src, int decimal);
/* degrade the timedelta sensor */
void nmea_timeout(void *);
@@ -126,6 +140,55 @@
strlcpy(np->longitude.desc, "Longitude", sizeof(np->longitude.desc));
sensor_attach(>timedev, >longitude);
+ np->altitude.type = SENSOR_DISTANCE;
+ np->altitude.status = SENSOR_S_UNKNOWN;
+ np->altitude.flags = SENSOR_FINVALID;
+ np->altitude.value = 0;
+ strlcpy(np->altitude.desc, "Altitude", sizeof(np->altitude.desc));
+ sensor_attach(>timedev, >altitude);
+
+ np->speed.type = SENSOR_VELOCITY;
+ np->speed.status = SENSOR_S_UNKNOWN;
+ np->speed.flags = SENSOR_FINVALID;
+ np->speed.value = 0;
+ strlcpy(np->speed.desc, "Ground speed", sizeof(np->speed.desc));
+ sensor_attach(>timedev, >speed);
+
+ np->nsat.type = SENSOR_INTEGER;
+ np->nsat.status = SENSOR_S_UNKNOWN;
+ np->nsat.flags = SENSOR_FINVALID;
+ np->nsat.value = 0;
+ strlcpy(np->nsat.desc, "Nb satellites", sizeof(np->nsat.desc));
+ sensor_attach(>timedev, >nsat);
+
+ np->quality.type = SENSOR_INTEGER;
+ np->quality.status = SENSOR_S_UNKNOWN;
+ np->quality.flags = SENSOR_FINVALID;
+ np->quality.value = 0;
+ strlcpy(np->quality.desc, "Fix Quality", sizeof(np->quality.desc));
+ sensor_attach(>timedev, >quality);
+
+ np->hdop.type = SENSOR_INTEGER;
+ np->hdop.status = SENSOR_S_UNKNOWN;
+ np->hdop.flags = SENSOR_FINVALID;
+ np->hdop.value = 0;
+ strlcpy(np->hdop.desc, "HDOP", sizeof(np->hdop.desc));
+ sensor_attach(>timedev, >hdop);
+
+ np->vdop.type = SENSOR_INTEGER;
+ np->vdop.status = SENSOR_S_UNKNOWN;
+ np->vdop.flags = SENSOR_FINVALID;
+ np->vdop.value = 0;
+ strlcpy(np->vdop.desc, "VDOP", sizeof(np->vdop.desc));
+ sensor_attach(>timedev, >vdop);
+
+ np->pdop.type = SENSOR_INTEGER;
+ np->pdop.status = SENSOR_S_UNKNOWN;
+ np->pdop.flags = SENSOR_FINVALID;
+ np->pdop.value = 0;
+ strlcpy(np->pdop.desc, "PDOP", sizeof(np->pdop.desc));
+ sensor_attach(>timedev, >pdop);
+
np->sync = 1;
tp->t_sc = (caddr_t)np;
@@ -182,7 +245,7 @@
np->ts.tv_nsec = ts.tv_nsec;
#ifdef NMEA_DEBUG
- if (nmeadebug > 0) {
+ if (nmeadebug > 1) {
linesw[TTYDISC].l_rint('[', tp);
linesw[TTYDISC].l_rint('0' + np->gapno++, tp);
linesw[TTYDISC].l_rint(']', tp);
@@ -261,7 +324,7 @@
}
/*
-* we only look at the RMC message, which can come from different
'talkers',
+*
ospf6d: fib-priority
Hi tech,
this allows to adjust the priority of the routes that ospf6d inserts
into the kernel routing table.
It's basically the same change that I commited to ospfd today (without
reload support).
OK?
Remi
Index: kroute.c
===
RCS file: /cvs/src/usr.sbin/ospf6d/kroute.c,v
retrieving revision 1.58
diff -u -p -r1.58 kroute.c
--- kroute.c12 Jul 2018 13:45:03 - 1.58
+++ kroute.c28 Dec 2018 19:58:05 -
@@ -45,6 +45,7 @@ struct {
u_int32_t rtseq;
pid_t pid;
int fib_sync;
+ u_int8_tfib_prio;
int fd;
struct eventev;
u_int rdomain;
@@ -95,13 +96,14 @@ RB_PROTOTYPE(kroute_tree, kroute_node, e
RB_GENERATE(kroute_tree, kroute_node, entry, kroute_compare)
int
-kr_init(int fs, u_int rdomain)
+kr_init(int fs, u_int rdomain, u_int8_t fib_prio)
{
int opt = 0, rcvbuf, default_rcvbuf;
socklen_t optlen;
kr_state.fib_sync = fs;
kr_state.rdomain = rdomain;
+ kr_state.fib_prio = fib_prio;
if ((kr_state.fd = socket(AF_ROUTE,
SOCK_RAW | SOCK_CLOEXEC | SOCK_NONBLOCK, AF_INET6)) == -1) {
@@ -218,7 +220,7 @@ kr_change_fib(struct kroute_node *kr, st
kn->r.nexthop = kroute[i].nexthop;
kn->r.scope = kroute[i].scope;
kn->r.flags = kroute[i].flags | F_OSPFD_INSERTED;
- kn->r.priority = RTP_OSPF;
+ kn->r.priority = kr_state.fib_prio;
kn->r.ext_tag = kroute[i].ext_tag;
rtlabel_unref(kn->r.rtlabel); /* for RTM_CHANGE */
kn->r.rtlabel = kroute[i].rtlabel;
@@ -242,7 +244,7 @@ kr_change(struct kroute *kroute, int krc
kroute->rtlabel = rtlabel_tag2id(kroute->ext_tag);
- kr = kroute_find(>prefix, kroute->prefixlen, RTP_OSPF);
+ kr = kroute_find(>prefix, kroute->prefixlen, kr_state.fib_prio);
if (kr != NULL && kr->next == NULL && krcount == 1) {
/*
* single path OSPF route.
@@ -272,7 +274,7 @@ kr_change(struct kroute *kroute, int krc
int
kr_delete_fib(struct kroute_node *kr)
{
- if (kr->r.priority != RTP_OSPF)
+ if (kr->r.priority != kr_state.fib_prio)
log_warn("kr_delete_fib: %s/%d has wrong priority %d",
log_in6addr(>r.prefix), kr->r.prefixlen,
kr->r.priority);
@@ -292,7 +294,7 @@ kr_delete(struct kroute *kroute)
struct kroute_node *kr, *nkr;
if ((kr = kroute_find(>prefix, kroute->prefixlen,
- RTP_OSPF)) == NULL)
+ kr_state.fib_prio)) == NULL)
return (0);
while (kr != NULL) {
@@ -324,7 +326,7 @@ kr_fib_couple(void)
kr_state.fib_sync = 1;
RB_FOREACH(kr, kroute_tree, )
- if (kr->r.priority == RTP_OSPF)
+ if (kr->r.priority == kr_state.fib_prio)
for (kn = kr; kn != NULL; kn = kn->next)
send_rtmsg(kr_state.fd, RTM_ADD, >r);
@@ -341,7 +343,7 @@ kr_fib_decouple(void)
return;
RB_FOREACH(kr, kroute_tree, )
- if (kr->r.priority == RTP_OSPF)
+ if (kr->r.priority == kr_state.fib_prio)
for (kn = kr; kn != NULL; kn = kn->next)
send_rtmsg(kr_state.fd, RTM_DELETE, >r);
@@ -1013,7 +1015,7 @@ send_rtmsg(int fd, int action, struct kr
bzero(, sizeof(hdr));
hdr.rtm_version = RTM_VERSION;
hdr.rtm_type = action;
- hdr.rtm_priority = RTP_OSPF;
+ hdr.rtm_priority = kr_state.fib_prio;
hdr.rtm_tableid = kr_state.rdomain; /* rtableid */
if (action == RTM_CHANGE)
hdr.rtm_fmask = RTF_REJECT|RTF_BLACKHOLE;
@@ -1241,7 +1243,7 @@ fetchtable(void)
break;
}
- if (rtm->rtm_priority == RTP_OSPF) {
+ if (rtm->rtm_priority == kr_state.fib_prio) {
send_rtmsg(kr_state.fd, RTM_DELETE, >r);
free(kr);
} else {
Index: ospf6d.c
===
RCS file: /cvs/src/usr.sbin/ospf6d/ospf6d.c,v
retrieving revision 1.40
diff -u -p -r1.40 ospf6d.c
--- ospf6d.c30 Oct 2018 16:52:19 - 1.40
+++ ospf6d.c28 Dec 2018 19:50:29 -
@@ -280,7 +280,7 @@ main(int argc, char *argv[])
fatal("unveil");
if (kr_init(!(ospfd_conf->flags & OSPFD_FLAG_NO_FIB_UPDATE),
- ospfd_conf->rdomain) == -1)
+ ospfd_conf->rdomain, ospfd_conf->fib_priority) == -1)
fatalx("kr_init failed");
event_dispatch();
Index: ospf6d.conf.5
video(1) pledge (& updated kernel diff)
Hi,
so i've updated my 'video' class for pledge and also did an initial
naive pledging of xenocara/app/video:
the kernel diff is the same subset as the diff from some months ago,
except that it adds:
- VIDIOC_QUERYCTRL/VIDIOC_G_CTRL/VIDIOC_S_CTRL: those 3 are used by
video(1) to set gamma/contrast/etc controls on the device on the fly
(via A/a, B/b, C/c - etc when running)
- VIDIOC_TRY_FMT, used by
https://searchfox.org/mozilla-central/source/media/webrtc/trunk/webrtc/modules/video_capture/linux/device_info_linux.cc#418
since https://bugzilla.mozilla.org/show_bug.cgi?id=1376873 (yes, i
checked with ktrace, all the other ioctls are still used)
with that kernel diff, i'm able to record video in firefox with the
video pledge added to the main process.
as for the video(1) diff:
- fixed a typo
- renamed err to errs to avoid shadowing err()
and i've identified 4 main modes:
- -o needs to write to the fs (so wpath cpath) and read from the device
(so video)
- -O needs the same subset and dns unix in addition, as it talks to Xv
- -i only needs rpath & dns/unix for xv, no access to the video device
needed
- no option needs dns unix for xv, video for ioctls, and wpath as the
video device is open with O_RDWR.
tested those 4 modes without aborts so far, video -q exits before
reaching pledge. Not sure if those pledge calls are at the best spot,
but that's a start.
I know video(1) sucks and doesnt work directly with Xv with most modern
builtin devices on laptops as they often dont support the few encodings
supported by video(1) for Xv, but with my old-school cameras lying
around it works fine.
Landry
? ktrace.out
? video
? video.d
Index: video.c
===
RCS file: /cvs/xenocara/app/video/video.c,v
retrieving revision 1.25
diff -u -r1.25 video.c
--- video.c 9 Apr 2018 18:16:44 - 1.25
+++ video.c 28 Dec 2018 20:22:36 -
@@ -1854,7 +1854,7 @@
struct xdsp *x =
const char *errstr;
size_t len;
- int ch, err = 0;
+ int ch, errs = 0;
bzero(, sizeof(struct video));
@@ -1872,21 +1872,21 @@
x->cur_adap = strtonum(optarg, 0, 4, );
if (errstr != NULL) {
warnx("Xv adaptor '%s' is %s", optarg, errstr);
- err++;
+ errs++;
}
break;
case 'e':
vid.enc = find_enc(optarg);
if (vid.enc >= ENC_LAST) {
warnx("encoding '%s' is invalid", optarg);
- err++;
+ errs++;
}
break;
case 'f':
len = strlcpy(d->path, optarg, sizeof(d->path));
if (len >= sizeof(d->path)) {
warnx("file path is too long: %s", optarg);
- err++;
+ errs++;
}
break;
case 'g':
@@ -1894,8 +1894,8 @@
break;
case 'i':
if (vid.mode & (M_IN_FILE | M_OUT_FILE)) {
- warnx("only one input or ouput file allowed");
- err++;
+ warnx("only one input or output file allowed");
+ errs++;
} else {
vid.mode = (vid.mode & ~M_IN_DEV) | M_IN_FILE;
vid.mmap_on = 0; /* mmap mode does not work for
files */
@@ -1904,15 +1904,15 @@
if (len >= sizeof(vid.iofile)) {
warnx("input path is too long: %s",
optarg);
- err++;
+ errs++;
}
}
break;
case 'o':
case 'O':
if (vid.mode & (M_IN_FILE | M_OUT_FILE)) {
- warnx("only one input or ouput file allowed");
- err++;
+ warnx("only one input or output file allowed");
+ errs++;
} else {
vid.mode |= M_OUT_FILE;
if (ch != 'O')
@@ -1922,7 +1922,7 @@
if (len >= sizeof(vid.iofile)) {
warnx("output path is too long: %s",
optarg);
- err++;
+
Re: MPLSv6 2/2 : bgpd diff
On Fri, Dec 28, 2018 at 05:21:02PM +0100, Denis Fondras wrote:
> On Fri, Dec 28, 2018 at 03:15:35PM +0100, Claudio Jeker wrote:
> > > /*
> > > * This function will have undefined behaviour if the passed in
> > > prefixlen is
> > > - * to large for the respective bgpd_addr address family.
> > > + * too large for the respective bgpd_addr address family.
> > > */
> > > int
> > > prefix_compare(const struct bgpd_addr *a, const struct bgpd_addr *b,
> > > @@ -620,6 +695,8 @@ prefix_compare(const struct bgpd_addr *a
> > > }
> > > return (0);
> > > case AID_VPN_IPv4:
> > > + if (prefixlen == 0)
> > > + return (0);
> >
> > I think this is not right. If at all then this check should happen after
> > checking the RD for equality. Else two different VPN default routes (with
> > different RD value) would be conidered the same.
> >
> > > if (prefixlen > 32)
> > > return (-1);
> > > if (betoh64(a->vpn4.rd) > betoh64(b->vpn4.rd))
> > > @@ -637,6 +714,34 @@ prefix_compare(const struct bgpd_addr *a
> > > return (-1);
> > > return (memcmp(a->vpn4.labelstack, b->vpn4.labelstack,
> > > a->vpn4.labellen));
> > > + case AID_VPN_IPv6:
> > > + if (prefixlen == 0)
> > > + return (0);
> >
> > See above.
> >
> > > + if (prefixlen > 128)
> > > + return (-1);
> > > + for (i = 0; i < prefixlen / 8; i++)
> > > + if (a->vpn6.addr.s6_addr[i] != b->vpn6.addr.s6_addr[i])
> > > + return (a->vpn6.addr.s6_addr[i] -
> > > + b->vpn6.addr.s6_addr[i]);
> > > + i = prefixlen % 8;
> > > + if (i) {
> > > + m = 0xff00 >> i;
> > > + if ((a->vpn6.addr.s6_addr[prefixlen / 8] & m) !=
> > > + (b->vpn6.addr.s6_addr[prefixlen / 8] & m))
> > > + return ((a->vpn6.addr.s6_addr[prefixlen / 8] &
> > > + m) - (b->vpn6.addr.s6_addr[prefixlen / 8] &
> > > + m));
> > > + }
> > > + if (betoh64(a->vpn6.rd) > betoh64(b->vpn6.rd))
> > > + return (1);
> > > + if (betoh64(a->vpn6.rd) < betoh64(b->vpn6.rd))
> > > + return (-1);
> >
> > I would check the RD first like it is done in the AID_VPN_IPv4 case.
> > Then address and then labelstack.
> >
> > > + if (a->vpn6.labellen > b->vpn6.labellen)
> > > + return (1);
> > > + if (a->vpn6.labellen < b->vpn6.labellen)
> > > + return (-1);
> > > + return (memcmp(a->vpn6.labelstack, b->vpn6.labelstack,
> > > + a->vpn6.labellen));
> > > }
> > > return (-1);
> > > }
> > >
> >
> > The diff is OK apart from that bit in prefix_compare().
> >
>
> Thank you for the comments.
> I removed the "if (prefixlen == 0)". I added it because it was missing but I
> did
> not think it was on purpose. I also reordered the tests as you suggest. I
> copied
> the order of up_prefix_cmp() (rd, address then labelstack) and updated
> pt_prefix_cmp() as well.
Not fully conviced about the pt_prefix_cmp() change but lets go with it
now. My reason for not being convinced is that until now the show rib
output would be sorted by address first. So the same prefix in multiple RD
would group by prefix. Now with your change the prefixes will be grouped
by RD. Since I'm not a big user of MPLS VPN I don't mind this change.
Diff is OK claudio@
> Index: bgpd.h
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> retrieving revision 1.360
> diff -u -p -r1.360 bgpd.h
> --- bgpd.h27 Dec 2018 20:23:24 - 1.360
> +++ bgpd.h28 Dec 2018 16:00:23 -
> @@ -154,7 +154,8 @@ extern const struct aid aid_vals[];
> #define AID_INET1
> #define AID_INET6 2
> #define AID_VPN_IPv43
> -#define AID_MAX 4
> +#define AID_VPN_IPv64
> +#define AID_MAX 5
> #define AID_MIN 1 /* skip AID_UNSPEC since that is a
> dummy */
>
> #define AID_VALS { \
> @@ -162,14 +163,16 @@ extern const struct aid aid_vals[];
> { AFI_UNSPEC, AF_UNSPEC, SAFI_NONE, "unspec"}, \
> { AFI_IPv4, AF_INET, SAFI_UNICAST, "IPv4 unicast" },\
> { AFI_IPv6, AF_INET6, SAFI_UNICAST, "IPv6 unicast" }, \
> - { AFI_IPv4, AF_INET, SAFI_MPLSVPN, "IPv4 vpn" } \
> + { AFI_IPv4, AF_INET, SAFI_MPLSVPN, "IPv4 vpn" },\
> + { AFI_IPv6, AF_INET6, SAFI_MPLSVPN, "IPv6 vpn" }\
> }
>
> #define AID_PTSIZE { \
> 0, \
> sizeof(struct pt_entry4), \
> sizeof(struct pt_entry6), \
> -
Re: MPLSv6 2/2 : bgpd diff
On Fri, Dec 28, 2018 at 07:50:08PM +0100, Denis Fondras wrote:
> On Fri, Dec 28, 2018 at 06:08:16PM +0100, Klemens Nanni wrote:
> > On Fri, Dec 28, 2018 at 05:21:02PM +0100, Denis Fondras wrote:
> > > int
> > > +krVPN6_change(struct ktable *kt, struct kroute_full *kl, u_int8_t
> > > fib_prio)
> > > +{
> > > + struct kroute6_node *kr6;
> > > + struct in6_addr lo6 = IN6ADDR_LOOPBACK_INIT;
> > > + int action = RTM_ADD;
> > > + u_int32_tmplslabel = 0;
> > > + u_int16_tlabelid;
> > > +
> > > + if ((kr6 = kroute6_find(kt, >prefix.vpn6.addr, kl->prefixlen,
> > > + fib_prio)) != NULL)
> > > + action = RTM_CHANGE;
> > Can this be moved below the conditional returns or does kroute6_find()
> > have side effects? `actions' is not used until much later.
> >
> > > + /* nexthop to loopback -> ignore silently */
> > > + if (IN6_IS_ADDR_LOOPBACK(>nexthop.v6))
> > > + return (0);
> > > +
> > > + /* only single MPLS label are supported for now */
> > > + if (kl->prefix.vpn6.labellen != 3) {
> > > + log_warnx("%s: %s/%u has not a single label", __func__,
> > > + log_addr(>prefix), kl->prefixlen);
> > > + return (0);
> > > + }
> > Here.
> >
> > > + mplslabel = (kl->prefix.vpn6.labelstack[0] << 24) |
> > > + (kl->prefix.vpn6.labelstack[1] << 16) |
> > > + (kl->prefix.vpn6.labelstack[2] << 8);
> > > + mplslabel = htonl(mplslabel);
> > > +
> > > + /* for blackhole and reject routes nexthop needs to be ::1 */
> > > + if (kl->flags & (F_BLACKHOLE|F_REJECT))
> > > + bcopy(, >nexthop.v6, sizeof(kl->nexthop.v6));
> > > +
> > > + labelid = rtlabel_name2id(kl->label);
> > Or even here right before it's used. `kr6' is not used above.
> >
> > > + if (action == RTM_ADD) {
>
> Yes, we can, it may save a few cycles. Here is a diff to update the current
> revision (I updated my MPLS diff with similar change).
>
> Index: kroute.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
> retrieving revision 1.226
> diff -u -p -r1.226 kroute.c
> --- kroute.c 6 Dec 2018 13:04:40 - 1.226
> +++ kroute.c 28 Dec 2018 18:46:07 -
> @@ -487,10 +487,6 @@ kr4_change(struct ktable *kt, struct kro
> int action = RTM_ADD;
> u_int16_tlabelid;
>
> - if ((kr = kroute_find(kt, kl->prefix.v4.s_addr, kl->prefixlen,
> - fib_prio)) != NULL)
> - action = RTM_CHANGE;
> -
> /* for blackhole and reject routes nexthop needs to be 127.0.0.1 */
> if (kl->flags & (F_BLACKHOLE|F_REJECT))
> kl->nexthop.v4.s_addr = htonl(INADDR_LOOPBACK);
> @@ -501,6 +497,10 @@ kr4_change(struct ktable *kt, struct kro
>
> labelid = rtlabel_name2id(kl->label);
>
> + if ((kr = kroute_find(kt, kl->prefix.v4.s_addr, kl->prefixlen,
> + fib_prio)) != NULL)
> + action = RTM_CHANGE;
> +
> if (action == RTM_ADD) {
> if ((kr = calloc(1, sizeof(struct kroute_node))) == NULL) {
> log_warn("%s", __func__);
> @@ -545,10 +545,6 @@ kr6_change(struct ktable *kt, struct kro
> int action = RTM_ADD;
> u_int16_tlabelid;
>
> - if ((kr6 = kroute6_find(kt, >prefix.v6, kl->prefixlen, fib_prio)) !=
> - NULL)
> - action = RTM_CHANGE;
> -
> /* for blackhole and reject routes nexthop needs to be ::1 */
> if (kl->flags & (F_BLACKHOLE|F_REJECT))
> bcopy(, >nexthop.v6, sizeof(kl->nexthop.v6));
> @@ -558,6 +554,10 @@ kr6_change(struct ktable *kt, struct kro
>
> labelid = rtlabel_name2id(kl->label);
>
> + if ((kr6 = kroute6_find(kt, >prefix.v6, kl->prefixlen, fib_prio)) !=
> + NULL)
> + action = RTM_CHANGE;
> +
> if (action == RTM_ADD) {
> if ((kr6 = calloc(1, sizeof(struct kroute6_node))) == NULL) {
> log_warn("%s", __func__);
> @@ -604,10 +604,6 @@ krVPN4_change(struct ktable *kt, struct
> u_int32_tmplslabel = 0;
> u_int16_tlabelid;
>
> - if ((kr = kroute_find(kt, kl->prefix.vpn4.addr.s_addr, kl->prefixlen,
> - fib_prio)) != NULL)
> - action = RTM_CHANGE;
> -
> /* nexthop within 127/8 -> ignore silently */
> if ((kl->nexthop.v4.s_addr & htonl(IN_CLASSA_NET)) ==
> htonl(INADDR_LOOPBACK & IN_CLASSA_NET))
> @@ -629,6 +625,10 @@ krVPN4_change(struct ktable *kt, struct
> /* for blackhole and reject routes nexthop needs to be 127.0.0.1 */
> if (kl->flags & (F_BLACKHOLE|F_REJECT))
> kl->nexthop.v4.s_addr = htonl(INADDR_LOOPBACK);
> +
> + if ((kr = kroute_find(kt, kl->prefix.vpn4.addr.s_addr, kl->prefixlen,
> + fib_prio)) != NULL)
> + action = RTM_CHANGE;
>
> if (action == RTM_ADD) {
> if ((kr = calloc(1,
Re: MPLSv6 2/2 : bgpd diff
On Fri, Dec 28, 2018 at 06:08:16PM +0100, Klemens Nanni wrote:
> On Fri, Dec 28, 2018 at 05:21:02PM +0100, Denis Fondras wrote:
> > int
> > +krVPN6_change(struct ktable *kt, struct kroute_full *kl, u_int8_t fib_prio)
> > +{
> > + struct kroute6_node *kr6;
> > + struct in6_addr lo6 = IN6ADDR_LOOPBACK_INIT;
> > + int action = RTM_ADD;
> > + u_int32_tmplslabel = 0;
> > + u_int16_tlabelid;
> > +
> > + if ((kr6 = kroute6_find(kt, >prefix.vpn6.addr, kl->prefixlen,
> > + fib_prio)) != NULL)
> > + action = RTM_CHANGE;
> Can this be moved below the conditional returns or does kroute6_find()
> have side effects? `actions' is not used until much later.
>
> > + /* nexthop to loopback -> ignore silently */
> > + if (IN6_IS_ADDR_LOOPBACK(>nexthop.v6))
> > + return (0);
> > +
> > + /* only single MPLS label are supported for now */
> > + if (kl->prefix.vpn6.labellen != 3) {
> > + log_warnx("%s: %s/%u has not a single label", __func__,
> > + log_addr(>prefix), kl->prefixlen);
> > + return (0);
> > + }
> Here.
>
> > + mplslabel = (kl->prefix.vpn6.labelstack[0] << 24) |
> > + (kl->prefix.vpn6.labelstack[1] << 16) |
> > + (kl->prefix.vpn6.labelstack[2] << 8);
> > + mplslabel = htonl(mplslabel);
> > +
> > + /* for blackhole and reject routes nexthop needs to be ::1 */
> > + if (kl->flags & (F_BLACKHOLE|F_REJECT))
> > + bcopy(, >nexthop.v6, sizeof(kl->nexthop.v6));
> > +
> > + labelid = rtlabel_name2id(kl->label);
> Or even here right before it's used. `kr6' is not used above.
>
> > + if (action == RTM_ADD) {
Yes, we can, it may save a few cycles. Here is a diff to update the current
revision (I updated my MPLS diff with similar change).
Index: kroute.c
===
RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
retrieving revision 1.226
diff -u -p -r1.226 kroute.c
--- kroute.c6 Dec 2018 13:04:40 - 1.226
+++ kroute.c28 Dec 2018 18:46:07 -
@@ -487,10 +487,6 @@ kr4_change(struct ktable *kt, struct kro
int action = RTM_ADD;
u_int16_tlabelid;
- if ((kr = kroute_find(kt, kl->prefix.v4.s_addr, kl->prefixlen,
- fib_prio)) != NULL)
- action = RTM_CHANGE;
-
/* for blackhole and reject routes nexthop needs to be 127.0.0.1 */
if (kl->flags & (F_BLACKHOLE|F_REJECT))
kl->nexthop.v4.s_addr = htonl(INADDR_LOOPBACK);
@@ -501,6 +497,10 @@ kr4_change(struct ktable *kt, struct kro
labelid = rtlabel_name2id(kl->label);
+ if ((kr = kroute_find(kt, kl->prefix.v4.s_addr, kl->prefixlen,
+ fib_prio)) != NULL)
+ action = RTM_CHANGE;
+
if (action == RTM_ADD) {
if ((kr = calloc(1, sizeof(struct kroute_node))) == NULL) {
log_warn("%s", __func__);
@@ -545,10 +545,6 @@ kr6_change(struct ktable *kt, struct kro
int action = RTM_ADD;
u_int16_tlabelid;
- if ((kr6 = kroute6_find(kt, >prefix.v6, kl->prefixlen, fib_prio)) !=
- NULL)
- action = RTM_CHANGE;
-
/* for blackhole and reject routes nexthop needs to be ::1 */
if (kl->flags & (F_BLACKHOLE|F_REJECT))
bcopy(, >nexthop.v6, sizeof(kl->nexthop.v6));
@@ -558,6 +554,10 @@ kr6_change(struct ktable *kt, struct kro
labelid = rtlabel_name2id(kl->label);
+ if ((kr6 = kroute6_find(kt, >prefix.v6, kl->prefixlen, fib_prio)) !=
+ NULL)
+ action = RTM_CHANGE;
+
if (action == RTM_ADD) {
if ((kr6 = calloc(1, sizeof(struct kroute6_node))) == NULL) {
log_warn("%s", __func__);
@@ -604,10 +604,6 @@ krVPN4_change(struct ktable *kt, struct
u_int32_tmplslabel = 0;
u_int16_tlabelid;
- if ((kr = kroute_find(kt, kl->prefix.vpn4.addr.s_addr, kl->prefixlen,
- fib_prio)) != NULL)
- action = RTM_CHANGE;
-
/* nexthop within 127/8 -> ignore silently */
if ((kl->nexthop.v4.s_addr & htonl(IN_CLASSA_NET)) ==
htonl(INADDR_LOOPBACK & IN_CLASSA_NET))
@@ -629,6 +625,10 @@ krVPN4_change(struct ktable *kt, struct
/* for blackhole and reject routes nexthop needs to be 127.0.0.1 */
if (kl->flags & (F_BLACKHOLE|F_REJECT))
kl->nexthop.v4.s_addr = htonl(INADDR_LOOPBACK);
+
+ if ((kr = kroute_find(kt, kl->prefix.vpn4.addr.s_addr, kl->prefixlen,
+ fib_prio)) != NULL)
+ action = RTM_CHANGE;
if (action == RTM_ADD) {
if ((kr = calloc(1, sizeof(struct kroute_node))) == NULL) {
Re: MPLSv6 2/2 : bgpd diff
On Fri, Dec 28, 2018 at 05:21:02PM +0100, Denis Fondras wrote:
> int
> +krVPN6_change(struct ktable *kt, struct kroute_full *kl, u_int8_t fib_prio)
> +{
> + struct kroute6_node *kr6;
> + struct in6_addr lo6 = IN6ADDR_LOOPBACK_INIT;
> + int action = RTM_ADD;
> + u_int32_tmplslabel = 0;
> + u_int16_tlabelid;
> +
> + if ((kr6 = kroute6_find(kt, >prefix.vpn6.addr, kl->prefixlen,
> + fib_prio)) != NULL)
> + action = RTM_CHANGE;
Can this be moved below the conditional returns or does kroute6_find()
have side effects? `actions' is not used until much later.
> + /* nexthop to loopback -> ignore silently */
> + if (IN6_IS_ADDR_LOOPBACK(>nexthop.v6))
> + return (0);
> +
> + /* only single MPLS label are supported for now */
> + if (kl->prefix.vpn6.labellen != 3) {
> + log_warnx("%s: %s/%u has not a single label", __func__,
> + log_addr(>prefix), kl->prefixlen);
> + return (0);
> + }
Here.
> + mplslabel = (kl->prefix.vpn6.labelstack[0] << 24) |
> + (kl->prefix.vpn6.labelstack[1] << 16) |
> + (kl->prefix.vpn6.labelstack[2] << 8);
> + mplslabel = htonl(mplslabel);
> +
> + /* for blackhole and reject routes nexthop needs to be ::1 */
> + if (kl->flags & (F_BLACKHOLE|F_REJECT))
> + bcopy(, >nexthop.v6, sizeof(kl->nexthop.v6));
> +
> + labelid = rtlabel_name2id(kl->label);
Or even here right before it's used. `kr6' is not used above.
> + if (action == RTM_ADD) {
Re: MPLSv6 2/2 : bgpd diff
On Fri, Dec 28, 2018 at 03:15:35PM +0100, Claudio Jeker wrote:
> > /*
> > * This function will have undefined behaviour if the passed in prefixlen
> > is
> > - * to large for the respective bgpd_addr address family.
> > + * too large for the respective bgpd_addr address family.
> > */
> > int
> > prefix_compare(const struct bgpd_addr *a, const struct bgpd_addr *b,
> > @@ -620,6 +695,8 @@ prefix_compare(const struct bgpd_addr *a
> > }
> > return (0);
> > case AID_VPN_IPv4:
> > + if (prefixlen == 0)
> > + return (0);
>
> I think this is not right. If at all then this check should happen after
> checking the RD for equality. Else two different VPN default routes (with
> different RD value) would be conidered the same.
>
> > if (prefixlen > 32)
> > return (-1);
> > if (betoh64(a->vpn4.rd) > betoh64(b->vpn4.rd))
> > @@ -637,6 +714,34 @@ prefix_compare(const struct bgpd_addr *a
> > return (-1);
> > return (memcmp(a->vpn4.labelstack, b->vpn4.labelstack,
> > a->vpn4.labellen));
> > + case AID_VPN_IPv6:
> > + if (prefixlen == 0)
> > + return (0);
>
> See above.
>
> > + if (prefixlen > 128)
> > + return (-1);
> > + for (i = 0; i < prefixlen / 8; i++)
> > + if (a->vpn6.addr.s6_addr[i] != b->vpn6.addr.s6_addr[i])
> > + return (a->vpn6.addr.s6_addr[i] -
> > + b->vpn6.addr.s6_addr[i]);
> > + i = prefixlen % 8;
> > + if (i) {
> > + m = 0xff00 >> i;
> > + if ((a->vpn6.addr.s6_addr[prefixlen / 8] & m) !=
> > + (b->vpn6.addr.s6_addr[prefixlen / 8] & m))
> > + return ((a->vpn6.addr.s6_addr[prefixlen / 8] &
> > + m) - (b->vpn6.addr.s6_addr[prefixlen / 8] &
> > + m));
> > + }
> > + if (betoh64(a->vpn6.rd) > betoh64(b->vpn6.rd))
> > + return (1);
> > + if (betoh64(a->vpn6.rd) < betoh64(b->vpn6.rd))
> > + return (-1);
>
> I would check the RD first like it is done in the AID_VPN_IPv4 case.
> Then address and then labelstack.
>
> > + if (a->vpn6.labellen > b->vpn6.labellen)
> > + return (1);
> > + if (a->vpn6.labellen < b->vpn6.labellen)
> > + return (-1);
> > + return (memcmp(a->vpn6.labelstack, b->vpn6.labelstack,
> > + a->vpn6.labellen));
> > }
> > return (-1);
> > }
> >
>
> The diff is OK apart from that bit in prefix_compare().
>
Thank you for the comments.
I removed the "if (prefixlen == 0)". I added it because it was missing but I did
not think it was on purpose. I also reordered the tests as you suggest. I copied
the order of up_prefix_cmp() (rd, address then labelstack) and updated
pt_prefix_cmp() as well.
Index: bgpd.h
===
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
retrieving revision 1.360
diff -u -p -r1.360 bgpd.h
--- bgpd.h 27 Dec 2018 20:23:24 - 1.360
+++ bgpd.h 28 Dec 2018 16:00:23 -
@@ -154,7 +154,8 @@ extern const struct aid aid_vals[];
#defineAID_INET1
#defineAID_INET6 2
#defineAID_VPN_IPv43
-#defineAID_MAX 4
+#defineAID_VPN_IPv64
+#defineAID_MAX 5
#defineAID_MIN 1 /* skip AID_UNSPEC since that is a
dummy */
#define AID_VALS { \
@@ -162,14 +163,16 @@ extern const struct aid aid_vals[];
{ AFI_UNSPEC, AF_UNSPEC, SAFI_NONE, "unspec"}, \
{ AFI_IPv4, AF_INET, SAFI_UNICAST, "IPv4 unicast" },\
{ AFI_IPv6, AF_INET6, SAFI_UNICAST, "IPv6 unicast" }, \
- { AFI_IPv4, AF_INET, SAFI_MPLSVPN, "IPv4 vpn" } \
+ { AFI_IPv4, AF_INET, SAFI_MPLSVPN, "IPv4 vpn" },\
+ { AFI_IPv6, AF_INET6, SAFI_MPLSVPN, "IPv6 vpn" }\
}
#define AID_PTSIZE { \
0, \
sizeof(struct pt_entry4), \
sizeof(struct pt_entry6), \
- sizeof(struct pt_entry_vpn4)\
+ sizeof(struct pt_entry_vpn4), \
+ sizeof(struct pt_entry_vpn6)\
}
struct vpn4_addr {
@@ -181,6 +184,15 @@ struct vpn4_addr {
u_int8_tpad2;
};
+struct vpn6_addr {
+ u_int64_t rd;
+ struct in6_addr addr;
+ u_int8_tlabelstack[21]; /* max that makes sense */
+ u_int8_tlabellen;
+ u_int8_tpad1;
+ u_int8_tpad2;
+};
+
#define BGP_MPLS_BOS 0x01
struct
Re: bgpd: remove intermediate value in MP capabilities
On Fri, Dec 28, 2018 at 02:58:23PM +0100, Denis Fondras wrote:
> The parser sets curpeer->conf.capabilities.mp to -1 before setting it to
> either
> 1 or 0 by default. Set it to 0 by default and change it to 1 when needed.
>
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
> retrieving revision 1.366
> diff -u -p -r1.366 parse.y
> --- parse.y 19 Dec 2018 15:26:42 - 1.366
> +++ parse.y 28 Dec 2018 13:51:03 -
> @@ -1192,10 +1192,8 @@ neighbor : { curpeer = new_peer(); }
> if (($3.prefix.aid == AID_INET && $3.len != 32) ||
> ($3.prefix.aid == AID_INET6 && $3.len != 128))
> curpeer->conf.template = 1;
> - if (curpeer->conf.capabilities.mp[
> - curpeer->conf.remote_addr.aid] == -1)
> - curpeer->conf.capabilities.mp[
> - curpeer->conf.remote_addr.aid] = 1;
> + curpeer->conf.capabilities.mp[
> + curpeer->conf.remote_addr.aid] = 1;
> if (get_id(curpeer)) {
> yyerror("get_id failed");
> YYERROR;
> @@ -3761,7 +3759,7 @@ alloc_peer(void)
> p->conf.export_type = EXPORT_UNSET;
> p->conf.announce_capa = 1;
> for (i = 0; i < AID_MAX; i++)
> - p->conf.capabilities.mp[i] = -1;
> + p->conf.capabilities.mp[i] = 0;
> p->conf.capabilities.refresh = 1;
> p->conf.capabilities.grestart.restart = 1;
> p->conf.capabilities.as4byte = 1;
> @@ -4143,8 +4141,6 @@ str2key(char *s, char *dest, size_t max_
> int
> neighbor_consistent(struct peer *p)
> {
> - u_int8_ti;
> -
> /* local-address and peer's address: same address family */
> if (p->conf.local_addr.aid &&
> p->conf.local_addr.aid != p->conf.remote_addr.aid) {
> @@ -4197,11 +4193,6 @@ neighbor_consistent(struct peer *p)
> "reflector clusters");
> return (-1);
> }
> -
> - /* the default MP capability is NONE */
> - for (i = 0; i < AID_MAX; i++)
> - if (p->conf.capabilities.mp[i] == -1)
> - p->conf.capabilities.mp[i] = 0;
>
> return (0);
> }
>
Makes sense. OK claudio
--
:wq Claudio
Re: MPLSv6 2/2 : bgpd diff
On Tue, Dec 18, 2018 at 12:13:38PM +0100, Denis Fondras wrote:
> Here is a serie of diffs to enable MPLSv6, MPLS transport over IPv6.
>
> Second diff : add support for IPv6 MPLS routes exchange with bgpd(8).
>
> (***)
> pe1# cat /etc/hostname.mpe0
>
> rdomain 2
> mplslabel 42
> inet6 2001:db8::2/128
> up
> (***)
> pe1# cat /etc/hostname.vio0
>
> rdomain 2
> inet6 2001:db8::2 126
> up
> (***)
> pe1# cat /etc/hostname.vio1
>
> mpls
> inet6 2001:db8:1::2 126
> up
> (***)
> pe1# cat /etc/hostname.lo0
>
> inet6 2001:db8:fffe::1 128
> up
> (***)
> pe1# cat /etc/bgpd.conf
>
> router-id 10.0.0.2
> AS 65530
>
> rdomain 2 {
> descr "CUSTOMER1"
> rd 65530:2
> import-target rt 65530:2
> export-target rt 65530:2
> depend on mpe0
> network inet connected
> network inet6 connected
> }
> group "ibgp" {
> announce IPv4 vpn
> announce IPv4 unicast
> announce IPv6 vpn
> announce IPv6 unicast
> remote-as 65530
> neighbor 10.255.254.2 {
> local-address 10.255.254.1
> descr PE2v4
> down
> }
> neighbor 2001:db8:fffe::2 {
> local-address 2001:db8:fffe::1
> descr PE2v6
> }
> }
> allow from ibgp
> allow to ibgp
> (***)
> pe1# bgpctl sh rib
>
> flags: * = Valid, > = Selected, I = via IBGP, A = Announced,
>S = Stale, E = Error
> origin validation state: N = not-found, V = valid, ! = invalid
> origin: i = IGP, e = EGP, ? = Incomplete
>
> flags ovs destination gateway lpref med aspath origin
>
> AI*>N rd 65530:2 2001:db8::/126 rd 0:0 :: 100 0 i
> I*> N rd 65530:2 2001:db8:::/126 2001:db8:fffe::2100 0 i
>
> (***)
> pe2# tcpdump -n -i vio1 mpls
> tcpdump: listening on vio1, link-type EN10MB
> 08:13:01.870005 MPLS(label 42, exp 0, ttl 62) 2001:db8::1 > 2001:db8:::1:
> icmp6: echo request
> 08:13:01.870882 MPLS(label 26, exp 0, ttl 63) MPLS(label 42, exp 0, ttl 63)
> 2001:db8:::1 > 2001:db8::1: icmp6: echo reply
> 08:13:02.362564 MPLS(label 42, exp 0, ttl 62) 2001:db8::1 > 2001:db8:::1:
> icmp6: echo request
> 08:13:02.363173 MPLS(label 26, exp 0, ttl 63) MPLS(label 42, exp 0, ttl 63)
> 2001:db8:::1 > 2001:db8::1: icmp6: echo reply
> 08:13:02.865183 MPLS(label 42, exp 0, ttl 62) 2001:db8::1 > 2001:db8:::1:
> icmp6: echo request
> (***)
>
> We can only exchange MPLS routes with the same address family as the
> transport AF.
>
> Unfortunately I don't have gear to test interoperability. It seems there is
> very
> few support that. Has anyone access to such hardware ?
I don't think I have capable hardware but diff looks good apart from one
small thing at the end.
> Index: bgpd/bgpd.h
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> retrieving revision 1.357
> diff -u -p -r1.357 bgpd.h
> --- bgpd/bgpd.h 11 Dec 2018 09:02:14 - 1.357
> +++ bgpd/bgpd.h 18 Dec 2018 11:04:07 -
> @@ -154,7 +154,8 @@ extern const struct aid aid_vals[];
> #define AID_INET1
> #define AID_INET6 2
> #define AID_VPN_IPv43
> -#define AID_MAX 4
> +#define AID_VPN_IPv64
> +#define AID_MAX 5
> #define AID_MIN 1 /* skip AID_UNSPEC since that is a
> dummy */
>
> #define AID_VALS { \
> @@ -162,14 +163,16 @@ extern const struct aid aid_vals[];
> { AFI_UNSPEC, AF_UNSPEC, SAFI_NONE, "unspec"}, \
> { AFI_IPv4, AF_INET, SAFI_UNICAST, "IPv4 unicast" },\
> { AFI_IPv6, AF_INET6, SAFI_UNICAST, "IPv6 unicast" }, \
> - { AFI_IPv4, AF_INET, SAFI_MPLSVPN, "IPv4 vpn" } \
> + { AFI_IPv4, AF_INET, SAFI_MPLSVPN, "IPv4 vpn" },\
> + { AFI_IPv6, AF_INET6, SAFI_MPLSVPN, "IPv6 vpn" }\
> }
>
> #define AID_PTSIZE { \
> 0, \
> sizeof(struct pt_entry4), \
> sizeof(struct pt_entry6), \
> - sizeof(struct pt_entry_vpn4)\
> + sizeof(struct pt_entry_vpn4), \
> + sizeof(struct
bgpd: remove intermediate value in MP capabilities
The parser sets curpeer->conf.capabilities.mp to -1 before setting it to either
1 or 0 by default. Set it to 0 by default and change it to 1 when needed.
Index: parse.y
===
RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
retrieving revision 1.366
diff -u -p -r1.366 parse.y
--- parse.y 19 Dec 2018 15:26:42 - 1.366
+++ parse.y 28 Dec 2018 13:51:03 -
@@ -1192,10 +1192,8 @@ neighbor : { curpeer = new_peer(); }
if (($3.prefix.aid == AID_INET && $3.len != 32) ||
($3.prefix.aid == AID_INET6 && $3.len != 128))
curpeer->conf.template = 1;
- if (curpeer->conf.capabilities.mp[
- curpeer->conf.remote_addr.aid] == -1)
- curpeer->conf.capabilities.mp[
- curpeer->conf.remote_addr.aid] = 1;
+ curpeer->conf.capabilities.mp[
+ curpeer->conf.remote_addr.aid] = 1;
if (get_id(curpeer)) {
yyerror("get_id failed");
YYERROR;
@@ -3761,7 +3759,7 @@ alloc_peer(void)
p->conf.export_type = EXPORT_UNSET;
p->conf.announce_capa = 1;
for (i = 0; i < AID_MAX; i++)
- p->conf.capabilities.mp[i] = -1;
+ p->conf.capabilities.mp[i] = 0;
p->conf.capabilities.refresh = 1;
p->conf.capabilities.grestart.restart = 1;
p->conf.capabilities.as4byte = 1;
@@ -4143,8 +4141,6 @@ str2key(char *s, char *dest, size_t max_
int
neighbor_consistent(struct peer *p)
{
- u_int8_ti;
-
/* local-address and peer's address: same address family */
if (p->conf.local_addr.aid &&
p->conf.local_addr.aid != p->conf.remote_addr.aid) {
@@ -4197,11 +4193,6 @@ neighbor_consistent(struct peer *p)
"reflector clusters");
return (-1);
}
-
- /* the default MP capability is NONE */
- for (i = 0; i < AID_MAX; i++)
- if (p->conf.capabilities.mp[i] == -1)
- p->conf.capabilities.mp[i] = 0;
return (0);
}
Re: ospfd: fib-priority
On Fri, Dec 28, 2018 at 02:32:54PM +0100, Remi Locherer wrote:
> ping
OK claudio@
> On Mon, Dec 10, 2018 at 10:40:22AM +0100, Remi Locherer wrote:
> > Hi,
> >
> > below patch adds "fib-priority" to ospfd.conf which allows to set a
> > custom priority to routes. 32 is still the default if not set. Changing
> > the priority with a reload is also supported.
> >
> > A discussion about the feature can be found here:
> > https://marc.info/?l=openbsd-tech=138360663119816=2
> >
> > My first idea was to add an additional parameter to the functions that
> > need it. But that that is not practical since then need the event that calls
> > kr_dispatch_msg() needs to be reset. Because of that I added fib_prio to
> > struct kr_state.
> >
> >
> > OK?
> >
> > Remi
> >
> >
> >
> > cvs diff: Diffing .
> > Index: kroute.c
> > ===
> > RCS file: /cvs/src/usr.sbin/ospfd/kroute.c,v
> > retrieving revision 1.111
> > diff -u -p -r1.111 kroute.c
> > --- kroute.c10 Jul 2018 11:49:04 - 1.111
> > +++ kroute.c9 Dec 2018 21:39:46 -
> > @@ -45,6 +45,7 @@ struct {
> > pid_t pid;
> > int fib_sync;
> > int fib_serial;
> > + u_int8_tfib_prio;
> > int fd;
> > struct eventev;
> > struct eventreload;
> > @@ -127,14 +128,15 @@ kif_init(void)
> > }
> >
> > int
> > -kr_init(int fs, u_int rdomain, int redis_label_or_prefix)
> > +kr_init(int fs, u_int rdomain, int redis_label_or_prefix, u_int8_t
> > fib_prio)
> > {
> > int opt = 0, rcvbuf, default_rcvbuf;
> > socklen_t optlen;
> > - int filter_prio = RTP_OSPF;
> > + int filter_prio = fib_prio;
> >
> > kr_state.fib_sync = fs;
> > kr_state.rdomain = rdomain;
> > + kr_state.fib_prio = fib_prio;
> >
> > if ((kr_state.fd = socket(AF_ROUTE,
> > SOCK_RAW | SOCK_CLOEXEC | SOCK_NONBLOCK, AF_INET)) == -1) {
> > @@ -262,7 +264,7 @@ kr_change_fib(struct kroute_node *kr, st
> > kn->r.prefixlen = kroute[i].prefixlen;
> > kn->r.nexthop.s_addr = kroute[i].nexthop.s_addr;
> > kn->r.flags = kroute[i].flags | F_OSPFD_INSERTED;
> > - kn->r.priority = RTP_OSPF;
> > + kn->r.priority = kr_state.fib_prio;
> > kn->r.ext_tag = kroute[i].ext_tag;
> > rtlabel_unref(kn->r.rtlabel); /* for RTM_CHANGE */
> > kn->r.rtlabel = kroute[i].rtlabel;
> > @@ -286,7 +288,8 @@ kr_change(struct kroute *kroute, int krc
> >
> > kroute->rtlabel = rtlabel_tag2id(kroute->ext_tag);
> >
> > - kr = kroute_find(kroute->prefix.s_addr, kroute->prefixlen, RTP_OSPF);
> > + kr = kroute_find(kroute->prefix.s_addr, kroute->prefixlen,
> > + kr_state.fib_prio);
> > if (kr != NULL && kr->next == NULL && krcount == 1)
> > /* single path OSPF route */
> > action = RTM_CHANGE;
> > @@ -297,7 +300,7 @@ kr_change(struct kroute *kroute, int krc
> > int
> > kr_delete_fib(struct kroute_node *kr)
> > {
> > - if (kr->r.priority != RTP_OSPF)
> > + if (kr->r.priority != kr_state.fib_prio)
> > log_warn("kr_delete_fib: %s/%d has wrong priority %d",
> > inet_ntoa(kr->r.prefix), kr->r.prefixlen, kr->r.priority);
> >
> > @@ -316,7 +319,7 @@ kr_delete(struct kroute *kroute)
> > struct kroute_node *kr, *nkr;
> >
> > if ((kr = kroute_find(kroute->prefix.s_addr, kroute->prefixlen,
> > - RTP_OSPF)) == NULL)
> > + kr_state.fib_prio)) == NULL)
> > return (0);
> >
> > while (kr != NULL) {
> > @@ -348,7 +351,7 @@ kr_fib_couple(void)
> > kr_state.fib_sync = 1;
> >
> > RB_FOREACH(kr, kroute_tree, )
> > - if (kr->r.priority == RTP_OSPF)
> > + if (kr->r.priority == kr_state.fib_prio)
> > for (kn = kr; kn != NULL; kn = kn->next)
> > send_rtmsg(kr_state.fd, RTM_ADD, >r);
> >
> > @@ -365,7 +368,7 @@ kr_fib_decouple(void)
> > return;
> >
> > RB_FOREACH(kr, kroute_tree, )
> > - if (kr->r.priority == RTP_OSPF)
> > + if (kr->r.priority == kr_state.fib_prio)
> > for (kn = kr; kn != NULL; kn = kn->next)
> > send_rtmsg(kr_state.fd, RTM_DELETE, >r);
> >
> > @@ -418,7 +421,7 @@ kr_fib_reload()
> > kn = kr->next;
> >
> > if (kr->serial != kr_state.fib_serial) {
> > - if (kr->r.priority == RTP_OSPF) {
> > + if (kr->r.priority == kr_state.fib_prio) {
> > kr->serial = kr_state.fib_serial;
> > if (send_rtmsg(kr_state.fd,
> > RTM_ADD, >r) != 0)
> > @@ -431,6 +434,21 @@ kr_fib_reload()
> > }
Re: ospfd: fib-priority
ping
On Mon, Dec 10, 2018 at 10:40:22AM +0100, Remi Locherer wrote:
> Hi,
>
> below patch adds "fib-priority" to ospfd.conf which allows to set a
> custom priority to routes. 32 is still the default if not set. Changing
> the priority with a reload is also supported.
>
> A discussion about the feature can be found here:
> https://marc.info/?l=openbsd-tech=138360663119816=2
>
> My first idea was to add an additional parameter to the functions that
> need it. But that that is not practical since then need the event that calls
> kr_dispatch_msg() needs to be reset. Because of that I added fib_prio to
> struct kr_state.
>
>
> OK?
>
> Remi
>
>
>
> cvs diff: Diffing .
> Index: kroute.c
> ===
> RCS file: /cvs/src/usr.sbin/ospfd/kroute.c,v
> retrieving revision 1.111
> diff -u -p -r1.111 kroute.c
> --- kroute.c 10 Jul 2018 11:49:04 - 1.111
> +++ kroute.c 9 Dec 2018 21:39:46 -
> @@ -45,6 +45,7 @@ struct {
> pid_t pid;
> int fib_sync;
> int fib_serial;
> + u_int8_tfib_prio;
> int fd;
> struct eventev;
> struct eventreload;
> @@ -127,14 +128,15 @@ kif_init(void)
> }
>
> int
> -kr_init(int fs, u_int rdomain, int redis_label_or_prefix)
> +kr_init(int fs, u_int rdomain, int redis_label_or_prefix, u_int8_t fib_prio)
> {
> int opt = 0, rcvbuf, default_rcvbuf;
> socklen_t optlen;
> - int filter_prio = RTP_OSPF;
> + int filter_prio = fib_prio;
>
> kr_state.fib_sync = fs;
> kr_state.rdomain = rdomain;
> + kr_state.fib_prio = fib_prio;
>
> if ((kr_state.fd = socket(AF_ROUTE,
> SOCK_RAW | SOCK_CLOEXEC | SOCK_NONBLOCK, AF_INET)) == -1) {
> @@ -262,7 +264,7 @@ kr_change_fib(struct kroute_node *kr, st
> kn->r.prefixlen = kroute[i].prefixlen;
> kn->r.nexthop.s_addr = kroute[i].nexthop.s_addr;
> kn->r.flags = kroute[i].flags | F_OSPFD_INSERTED;
> - kn->r.priority = RTP_OSPF;
> + kn->r.priority = kr_state.fib_prio;
> kn->r.ext_tag = kroute[i].ext_tag;
> rtlabel_unref(kn->r.rtlabel); /* for RTM_CHANGE */
> kn->r.rtlabel = kroute[i].rtlabel;
> @@ -286,7 +288,8 @@ kr_change(struct kroute *kroute, int krc
>
> kroute->rtlabel = rtlabel_tag2id(kroute->ext_tag);
>
> - kr = kroute_find(kroute->prefix.s_addr, kroute->prefixlen, RTP_OSPF);
> + kr = kroute_find(kroute->prefix.s_addr, kroute->prefixlen,
> + kr_state.fib_prio);
> if (kr != NULL && kr->next == NULL && krcount == 1)
> /* single path OSPF route */
> action = RTM_CHANGE;
> @@ -297,7 +300,7 @@ kr_change(struct kroute *kroute, int krc
> int
> kr_delete_fib(struct kroute_node *kr)
> {
> - if (kr->r.priority != RTP_OSPF)
> + if (kr->r.priority != kr_state.fib_prio)
> log_warn("kr_delete_fib: %s/%d has wrong priority %d",
> inet_ntoa(kr->r.prefix), kr->r.prefixlen, kr->r.priority);
>
> @@ -316,7 +319,7 @@ kr_delete(struct kroute *kroute)
> struct kroute_node *kr, *nkr;
>
> if ((kr = kroute_find(kroute->prefix.s_addr, kroute->prefixlen,
> - RTP_OSPF)) == NULL)
> + kr_state.fib_prio)) == NULL)
> return (0);
>
> while (kr != NULL) {
> @@ -348,7 +351,7 @@ kr_fib_couple(void)
> kr_state.fib_sync = 1;
>
> RB_FOREACH(kr, kroute_tree, )
> - if (kr->r.priority == RTP_OSPF)
> + if (kr->r.priority == kr_state.fib_prio)
> for (kn = kr; kn != NULL; kn = kn->next)
> send_rtmsg(kr_state.fd, RTM_ADD, >r);
>
> @@ -365,7 +368,7 @@ kr_fib_decouple(void)
> return;
>
> RB_FOREACH(kr, kroute_tree, )
> - if (kr->r.priority == RTP_OSPF)
> + if (kr->r.priority == kr_state.fib_prio)
> for (kn = kr; kn != NULL; kn = kn->next)
> send_rtmsg(kr_state.fd, RTM_DELETE, >r);
>
> @@ -418,7 +421,7 @@ kr_fib_reload()
> kn = kr->next;
>
> if (kr->serial != kr_state.fib_serial) {
> - if (kr->r.priority == RTP_OSPF) {
> + if (kr->r.priority == kr_state.fib_prio) {
> kr->serial = kr_state.fib_serial;
> if (send_rtmsg(kr_state.fd,
> RTM_ADD, >r) != 0)
> @@ -431,6 +434,21 @@ kr_fib_reload()
> }
> }
>
> +void
> +kr_fib_update_prio(u_int8_t fib_prio)
> +{
> + struct kroute_node *kr;
> +
> + RB_FOREACH(kr, kroute_tree, )
> + if ((kr->r.flags & F_OSPFD_INSERTED))
> +
Re: typo in getgrent.3
On Fri, Dec 28, 2018 at 01:25:02PM +0200, Lauri Tirkkonen wrote: > Hi, trivial typo fix for getgrent.3. fixed, thanks.
Re: tcpdump: print SAFI name
On Fri, Dec 28, 2018 at 12:16:05PM +0100, Denis Fondras wrote: > Print well-known SAFI name instead of value. > > * Before: > BGP (OPEN: Version 4, AS #65530, Holdtime 90, ID 10.2.2.2, Option length 44 > ((CAP MULTI_PROTOCOL [IPv4 Unicast], CAP MULTI_PROTOCOL [IPv4 #128], CAP > MULTI_PROTOCOL [IPv6 #128], CAP ROUTE_REFRESH, CAP GRACEFUL_RESTART [R], Time > 90s (IPv4 Unicast) (IPv4 #128) (IPv6 #128) CAP AS4 #65530))) > > * After: > BGP (OPEN: Version 4, AS #65530, Holdtime 90, ID 10.2.2.2, Option length 44 > ((CAP MULTI_PROTOCOL [IPv4 Unicast], CAP MULTI_PROTOCOL [IPv4 L3VPN Unicast], > CAP MULTI_PROTOCOL [IPv6 L3VPN Unicast], CAP ROUTE_REFRESH, CAP > GRACEFUL_RESTART > [R], Time 90s (IPv4 Unicast) (IPv4 L3VPN Unicast) (IPv6 L3VPN Unicast) CAP AS4 > #65530))) > > Index: print-bgp.c > === > RCS file: /cvs/src/usr.sbin/tcpdump/print-bgp.c,v > retrieving revision 1.26 > diff -u -p -r1.26 print-bgp.c > --- print-bgp.c 22 Oct 2018 16:12:45 - 1.26 > +++ print-bgp.c 28 Dec 2018 10:53:58 - > @@ -300,6 +300,11 @@ static const char *bgpattr_nlri_safi[] = > 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, > /* 64-66: MPLS BGP RFC3107 */ > "Tunnel", "VPLS", "MDT", > + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, > + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, > + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, > + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, > + "L3VPN Unicast", "L3VPN Multicast", > }; > #define bgp_attr_nlri_safi(x) \ > num_or_str(bgpattr_nlri_safi, \ > Sure, OK claudio@ -- :wq Claudio
typo in getgrent.3
Hi, trivial typo fix for getgrent.3. diff --git a/lib/libc/gen/getgrent.3 b/lib/libc/gen/getgrent.3 index d3c12bad042..f26ae625997 100644 --- a/lib/libc/gen/getgrent.3 +++ b/lib/libc/gen/getgrent.3 @@ -193,7 +193,7 @@ group database file .El .Sh ERRORS The -.Fn getgruid_r +.Fn getgrgid_r and .Fn getgrnam_r functions may fail if: -- Lauri Tirkkonen | lotheac @ IRCnet
tcpdump: print SAFI name
Print well-known SAFI name instead of value. * Before: BGP (OPEN: Version 4, AS #65530, Holdtime 90, ID 10.2.2.2, Option length 44 ((CAP MULTI_PROTOCOL [IPv4 Unicast], CAP MULTI_PROTOCOL [IPv4 #128], CAP MULTI_PROTOCOL [IPv6 #128], CAP ROUTE_REFRESH, CAP GRACEFUL_RESTART [R], Time 90s (IPv4 Unicast) (IPv4 #128) (IPv6 #128) CAP AS4 #65530))) * After: BGP (OPEN: Version 4, AS #65530, Holdtime 90, ID 10.2.2.2, Option length 44 ((CAP MULTI_PROTOCOL [IPv4 Unicast], CAP MULTI_PROTOCOL [IPv4 L3VPN Unicast], CAP MULTI_PROTOCOL [IPv6 L3VPN Unicast], CAP ROUTE_REFRESH, CAP GRACEFUL_RESTART [R], Time 90s (IPv4 Unicast) (IPv4 L3VPN Unicast) (IPv6 L3VPN Unicast) CAP AS4 #65530))) Index: print-bgp.c === RCS file: /cvs/src/usr.sbin/tcpdump/print-bgp.c,v retrieving revision 1.26 diff -u -p -r1.26 print-bgp.c --- print-bgp.c 22 Oct 2018 16:12:45 - 1.26 +++ print-bgp.c 28 Dec 2018 10:53:58 - @@ -300,6 +300,11 @@ static const char *bgpattr_nlri_safi[] = 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, /* 64-66: MPLS BGP RFC3107 */ "Tunnel", "VPLS", "MDT", + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + "L3VPN Unicast", "L3VPN Multicast", }; #define bgp_attr_nlri_safi(x) \ num_or_str(bgpattr_nlri_safi, \
