Re: bgpd MRT RFC8050 support (add-path for mrt dumps)
On Mon, Aug 09, 2021 at 12:17:47PM +0200, Claudio Jeker wrote:
> This diff adds the bits needed to support add-path in MRT dumps.
> The problem here is that MRT as a stateless protocol has no chance
> to know what kind of encoding (add-path or not) is used for the NLRI in
> message dumps. And for table dumps there is a need to add an extra field
> to the dumps to show the path-id.
>
> There are two issues:
> - for message dumps: it is necessary to peek into UPDATE messages to
> figure out if that update is using add-path or not. This comes from the
> fact that the add-path RFC allows to set the option per AFI/SAFI and
> also per direction. This is a major pain in bgpd since UPDATE messages
> are actually parsed in the RDE and not in the SE. The SE just does the
> basic lenght checks (header size, total length). So this peak into the
> packet needs to be done with some care (especialy for MP encoded
> UPDATEs).
>
> - for table dumps the RFC did a major fobar and defined a extra special
> encoding for non-IPv4/IPv6 prefixes. In the general encoding the path-id
> is not part of the rib entries sub-struct but is instead part of the
> NLRI. This encoding is to complex to build into the bgpd codebase and it
> seems the only other BGP implementation supporting RIB_GENERIC_ADDPATH,
> gobgp, is also not implementing it according to the RFC but instead is
> using the same encoding as for the other _ADDPATH types. OpenBGPD will
> do the same.
>
> This seems to work for me and results in the right output in bgpctl.
>
> Please review mrt_bgp_guess_aid() and check if all checks are correct to
> ensure we don't run off the rails.
Ping
--
:wq Claudio
Index: mrt.c
===
RCS file: /cvs/src/usr.sbin/bgpd/mrt.c,v
retrieving revision 1.104
diff -u -p -r1.104 mrt.c
--- mrt.c 24 Jun 2021 10:04:05 - 1.104
+++ mrt.c 9 Aug 2021 10:13:43 -
@@ -91,23 +91,128 @@ int mrt_open(struct mrt *, time_t);
x == MRT_TABLE_DUMP_V2) ? RDEIDX : SEIDX\
)
-void
-mrt_dump_bgp_msg(struct mrt *mrt, void *pkg, u_int16_t pkglen,
-struct peer *peer)
+static u_int8_t
+mrt_bgp_guess_aid(u_int8_t *pkg, u_int16_t pkglen)
+{
+ u_int16_t wlen, alen, len, afi;
+ u_int8_t type, aid;
+
+ pkg += MSGSIZE_HEADER;
+ pkglen -= MSGSIZE_HEADER;
+
+ if (pkglen < 4)
+ goto bad;
+
+ memcpy(, pkg, 2);
+ wlen = ntohs(wlen);
+ pkg += 2;
+ pkglen -= 2;
+
+ memcpy(, pkg, 2);
+ alen = ntohs(alen);
+ pkg += 2;
+ pkglen -= 2;
+
+ if (wlen > 0 || alen < pkglen || (wlen == 0 && alen == 0)) {
+ /* UPDATE has withdraw or NLRI prefixes or IPv4 EoR */
+ return AID_INET;
+ }
+
+ /* bad attribute length */
+ if (alen > pkglen)
+ goto bad;
+
+ /* try to extract AFI/SAFI from the MP attributes */
+ while (alen > 0) {
+ if (alen < 3)
+ goto bad;
+ type = pkg[1];
+ if (pkg[0] & ATTR_EXTLEN) {
+ if (alen < 4)
+ goto bad;
+ memcpy(, pkg + 2, 2);
+ len = ntohs(len);
+ pkg += 4;
+ alen -= 4;
+ } else {
+ len = pkg[2];
+ pkg += 3;
+ alen -= 3;
+ }
+ if (len > alen)
+ goto bad;
+
+ if (type == ATTR_MP_REACH_NLRI ||
+ type == ATTR_MP_UNREACH_NLRI) {
+ if (alen < 3)
+ goto bad;
+ memcpy(, pkg, 2);
+ afi = ntohs(afi);
+ if (afi2aid(afi, pkg[2], ) == -1)
+ goto bad;
+ return aid;
+ }
+
+ pkg += len;
+ alen -= len;
+ }
+
+bad:
+ return AID_UNSPEC;
+}
+
+static u_int16_t
+mrt_bgp_msg_subtype(struct mrt *mrt, void *pkg, u_int16_t pkglen,
+struct peer *peer, enum msg_type msgtype, int in)
{
- struct ibuf *buf;
- int incoming = 0;
u_int16_tsubtype = BGP4MP_MESSAGE;
+ u_int8_t aid, mask;
if (peer->capa.neg.as4byte)
subtype = BGP4MP_MESSAGE_AS4;
+ if (msgtype != UPDATE)
+ return subtype;
+
+ /*
+* RFC8050 adjust types for add-path enabled sessions.
+* It is necessary to extract the AID from UPDATES to decide
+* if the add-path types are needed or not. The ADDPATH
+* subtypes only matter for BGP UPDATES.
+*/
+
+ mask = in ? CAPA_AP_RECV : CAPA_AP_SEND;
+ /* only guess if add-path could be active */
+ if
bgpd MRT RFC8050 support (add-path for mrt dumps)
This diff adds the bits needed to support add-path in MRT dumps.
The problem here is that MRT as a stateless protocol has no chance
to know what kind of encoding (add-path or not) is used for the NLRI in
message dumps. And for table dumps there is a need to add an extra field
to the dumps to show the path-id.
There are two issues:
- for message dumps: it is necessary to peek into UPDATE messages to
figure out if that update is using add-path or not. This comes from the
fact that the add-path RFC allows to set the option per AFI/SAFI and
also per direction. This is a major pain in bgpd since UPDATE messages
are actually parsed in the RDE and not in the SE. The SE just does the
basic lenght checks (header size, total length). So this peak into the
packet needs to be done with some care (especialy for MP encoded
UPDATEs).
- for table dumps the RFC did a major fobar and defined a extra special
encoding for non-IPv4/IPv6 prefixes. In the general encoding the path-id
is not part of the rib entries sub-struct but is instead part of the
NLRI. This encoding is to complex to build into the bgpd codebase and it
seems the only other BGP implementation supporting RIB_GENERIC_ADDPATH,
gobgp, is also not implementing it according to the RFC but instead is
using the same encoding as for the other _ADDPATH types. OpenBGPD will
do the same.
This seems to work for me and results in the right output in bgpctl.
Please review mrt_bgp_guess_aid() and check if all checks are correct to
ensure we don't run off the rails.
--
:wq Claudio
Index: mrt.c
===
RCS file: /cvs/src/usr.sbin/bgpd/mrt.c,v
retrieving revision 1.104
diff -u -p -r1.104 mrt.c
--- mrt.c 24 Jun 2021 10:04:05 - 1.104
+++ mrt.c 9 Aug 2021 10:13:43 -
@@ -91,23 +91,128 @@ int mrt_open(struct mrt *, time_t);
x == MRT_TABLE_DUMP_V2) ? RDEIDX : SEIDX\
)
-void
-mrt_dump_bgp_msg(struct mrt *mrt, void *pkg, u_int16_t pkglen,
-struct peer *peer)
+static u_int8_t
+mrt_bgp_guess_aid(u_int8_t *pkg, u_int16_t pkglen)
+{
+ u_int16_t wlen, alen, len, afi;
+ u_int8_t type, aid;
+
+ pkg += MSGSIZE_HEADER;
+ pkglen -= MSGSIZE_HEADER;
+
+ if (pkglen < 4)
+ goto bad;
+
+ memcpy(, pkg, 2);
+ wlen = ntohs(wlen);
+ pkg += 2;
+ pkglen -= 2;
+
+ memcpy(, pkg, 2);
+ alen = ntohs(alen);
+ pkg += 2;
+ pkglen -= 2;
+
+ if (wlen > 0 || alen < pkglen || (wlen == 0 && alen == 0)) {
+ /* UPDATE has withdraw or NLRI prefixes or IPv4 EoR */
+ return AID_INET;
+ }
+
+ /* bad attribute length */
+ if (alen > pkglen)
+ goto bad;
+
+ /* try to extract AFI/SAFI from the MP attributes */
+ while (alen > 0) {
+ if (alen < 3)
+ goto bad;
+ type = pkg[1];
+ if (pkg[0] & ATTR_EXTLEN) {
+ if (alen < 4)
+ goto bad;
+ memcpy(, pkg + 2, 2);
+ len = ntohs(len);
+ pkg += 4;
+ alen -= 4;
+ } else {
+ len = pkg[2];
+ pkg += 3;
+ alen -= 3;
+ }
+ if (len > alen)
+ goto bad;
+
+ if (type == ATTR_MP_REACH_NLRI ||
+ type == ATTR_MP_UNREACH_NLRI) {
+ if (alen < 3)
+ goto bad;
+ memcpy(, pkg, 2);
+ afi = ntohs(afi);
+ if (afi2aid(afi, pkg[2], ) == -1)
+ goto bad;
+ return aid;
+ }
+
+ pkg += len;
+ alen -= len;
+ }
+
+bad:
+ return AID_UNSPEC;
+}
+
+static u_int16_t
+mrt_bgp_msg_subtype(struct mrt *mrt, void *pkg, u_int16_t pkglen,
+struct peer *peer, enum msg_type msgtype, int in)
{
- struct ibuf *buf;
- int incoming = 0;
u_int16_tsubtype = BGP4MP_MESSAGE;
+ u_int8_t aid, mask;
if (peer->capa.neg.as4byte)
subtype = BGP4MP_MESSAGE_AS4;
+ if (msgtype != UPDATE)
+ return subtype;
+
+ /*
+* RFC8050 adjust types for add-path enabled sessions.
+* It is necessary to extract the AID from UPDATES to decide
+* if the add-path types are needed or not. The ADDPATH
+* subtypes only matter for BGP UPDATES.
+*/
+
+ mask = in ? CAPA_AP_RECV : CAPA_AP_SEND;
+ /* only guess if add-path could be active */
+ if (peer->capa.neg.add_path[0] & mask) {
+ aid = mrt_bgp_guess_aid(pkg, pkglen);
+ if (aid != AID_UNSPEC &&
+
