Re: MPLSv6 2/2 : bgpd diff
On Sun, Mar 24, 2019 at 10:03:15PM +1300, Richard Procter wrote: > The ldpd issue might merit a hint in the man page as I found it difficult to > diagnose > as a newbie (see attached patch), and the man page, while not wrong, threw me > by > stating that GTSM is mandatory for LDPv6; it is now but wasn’t in the past. > > ok for the patch? > Thank you very much for testing and the report. I like the idea of being more specific in the manual. OK denis@, I would however remove the parenthesis. > Are there any other tests you’d be interested in while I have the machine > configured? > > best, > Richard. > > Here’s the version info of JUNOS I was using (note the git -dirty commit > (!!!)) > > uname -a: FreeBSD mx480-lab-re0 JNPR-10.3-20171207.04b87e3_buil FreeBSD > JNPR-10.3-20171207.04b87e3_builder_stable_10 #0 r356532+04b87e3(HEAD)-dirty: > Thu Dec 7 09:13:19 PST 2017 > buil...@basith.juniper.net:/volume/build/junos/occam/freebsd/stable_10/20171116.200501_builder_stable_10.04b87e3/obj/amd64/juniper/kernels/JNPR-AMD64-PRD/kernel > amd64 > > Model: mx480 > Family: junos > Junos: 17.2R2-S2.1 > > RFC5036 (2007) "LDP Specification" > > RFC6720 (2012) "The Generalized TTL Security Mechanism (GTSM) for LDP” - 1 > >> GTSM specifies that "it SHOULD NOT be enabled by default in order to > >> remain backward compatible with the unmodified protocol" (see > >> Section 3 of [RFC5082]). > >> > >> This document specifies a "built-in dynamic GTSM capability negotiation" > >> for > >> LDP to suggest the use of GTSM. GTSM will be used as specified in this > >> document provided both peers on an LDP session can detect each others' > >> support > >> for GTSM procedures and agree to use it. That is, the desire to use GTSM > >> (i.e., its negotiation mechanics) is enabled by default without any > >> configuration. > > RFC7552 (2015) "Updates to LDP for IPv6” - 5.1 > >> Also, the LDP Link Hello packets MUST have their IPv6 Hop Limit set > >> to 255, be checked for the same upon receipt (before any LDP-specific > >> processing), and be handled as specified in Section 3 of [RFC5082]. > > > cvs server: Diffing . > Index: ldpd.conf.5 > === > RCS file: /cvs/src/usr.sbin/ldpd/ldpd.conf.5,v > retrieving revision 1.37 > diff -u -p -u -r1.37 ldpd.conf.5 > --- ldpd.conf.5 23 Jan 2019 02:02:04 - 1.37 > +++ ldpd.conf.5 24 Mar 2019 08:36:14 - > @@ -177,7 +177,8 @@ and RFC 7552 (for the IPv6 address-famil > Since GTSM is mandatory for LDPv6, the only effect of disabling GTSM for the > IPv6 address-family is that > .Xr ldpd 8 > -will not check the incoming packets' hop limit. > +will not check the incoming packets' hop limit. (This may be necessary to > +interoperate with implementations lacking RFC 7552 (2015) compliance.) > Outgoing packets will still be sent using a hop limit of 255 to guarantee > interoperability. > .Pp > >
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: 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
MPLSv6 2/2 : bgpd diff
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 ?
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[];
#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
