On Tue, May 24, 2022 at 08:12:25PM +0000, Job Snijders wrote:
> Hi all,
>
> I based this off reading https://datatracker.ietf.org/doc/html/rfc9234
>
> This code is untested! I haven't had a chance yet to tcpdump a RFC 9234
> capable BGP speaker. There might be some out there, according to
> https://trac.ietf.org/trac/idr/wiki/draft-ietf-idr-bgp-open-policy
>
Reads good to me. A few minor things inline but OK otherwise.
> Index: print-bgp.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/tcpdump/print-bgp.c,v
> retrieving revision 1.30
> diff -u -p -r1.30 print-bgp.c
> --- print-bgp.c 17 Jun 2021 15:59:23 -0000 1.30
> +++ print-bgp.c 24 May 2022 20:06:25 -0000
> @@ -135,6 +135,7 @@ struct bgp_attr {
> #define BGPTYPE_AS4_PATH 17 /* RFC4893 */
> #define BGPTYPE_AGGREGATOR4 18 /* RFC4893 */
> #define BGPTYPE_LARGE_COMMUNITIES 32 /*
> draft-ietf-idr-large-community */
> +#define BGPTYPE_ONLY_TO_CUSTOMER 35 /* RFC9234 */
>
> #define BGP_AS_SET 1
> #define BGP_AS_SEQUENCE 2
> @@ -172,6 +173,7 @@ static const char *bgpopt_type[] = {
>
> #define BGP_CAPCODE_MP 1
> #define BGP_CAPCODE_REFRESH 2
> +#define BGP_CAPCODE_BGPROLE 9 /* RFC9234 */
> #define BGP_CAPCODE_RESTART 64 /* draft-ietf-idr-restart-05 */
> #define BGP_CAPCODE_AS4 65 /* RFC4893 */
>
> @@ -180,7 +182,9 @@ static const char *bgp_capcode[] = {
> /* 3: RFC5291 */ "OUTBOUND_ROUTE_FILTERING",
> /* 4: RFC3107 */ "MULTIPLE_ROUTES",
> /* 5: RFC5549 */ "EXTENDED_NEXTHOP_ENCODING",
> - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0,
> + /* 9: RFC9234 */ "BGP_ROLE",
> + 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,
> @@ -191,10 +195,17 @@ static const char *bgp_capcode[] = {
> /* 69: [draft-ietf-idr-add-paths] */ "ADD-PATH",
> /* 70: RFC7313 */ "ENHANCED_ROUTE_REFRESH"
> };
> -
> #define bgp_capcode(x) \
> num_or_str(bgp_capcode, sizeof(bgp_capcode)/sizeof(bgp_capcode[0]), (x))
>
> +static const char *bgp_roletype[] = {
> + NULL, "Provider", "Route Server", "Route Server Client", "Customer",
> + "Lateral Peer"
You need to remove the inital NULL here. The Provider Role has value 0.
The RFC uses the much shorter "RS", RS-Client" and "Peer".
I think using RS Client instead of "Route Server Client" makes sense.
Not sure why the role value at IANA uses "Peer (i.e., Lateral Peer)" it
makes it sound as if there is some other option Peer.
> +};
> +#define bgp_roletype(x) \
> + num_or_str(bgp_roletype, \
> + sizeof(bgp_roletype)/sizeof(bgp_roletype[0]), (x))
> +
> #define BGP_NOTIFY_MAJOR_CEASE 6
> static const char *bgpnotify_major[] = {
> NULL, "Message Header Error",
> @@ -215,7 +226,8 @@ static const char *bgpnotify_minor_open[
> NULL, "Unsupported Version Number",
> "Bad Peer AS", "Bad BGP Identifier",
> "Unsupported Optional Parameter", "Authentication Failure",
> - "Unacceptable Hold Time", "Unsupported Capability",
> + "Unacceptable Hold Time", "Unsupported Capability", "Deprecated",
> + "Deprecated", "Deprecated", "Role Mismatch"
Please use NULL here instead of "Deprecated" then str_or_num() will print
the number which is better.
> };
>
> static const char *bgpnotify_minor_update[] = {
> @@ -285,7 +297,7 @@ static const char *bgpattr_type[] = {
> "ADVERTISERS", "RCID_PATH", "MP_REACH_NLRI", "MP_UNREACH_NLRI",
> "EXTD_COMMUNITIES", "AS4_PATH", "AGGREGATOR4", NULL, NULL, NULL,
> NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> - "LARGE_COMMUNITIES",
> + "LARGE_COMMUNITIES", NULL, NULL, "ONLY_TO_CUSTOMER"
> };
> #define bgp_attr_type(x) \
> num_or_str(bgpattr_type, \
> @@ -590,6 +602,14 @@ bgp_attr_print(const struct bgp_attr *at
> p += 12;
> }
> break;
> + case BGPTYPE_ONLY_TO_CUSTOMER:
> + if (len != 4) {
> + printf(" invalid len");
> + break;
> + }
> + TCHECK2(p[0], 4);
> + printf(" AS%u", EXTRACT_32BITS(p));
> + break;
> case BGPTYPE_ORIGINATOR_ID:
> if (len != 4) {
> printf(" invalid len");
> @@ -769,6 +789,13 @@ bgp_open_capa_print(const u_char *opt, i
> printf(" BAD ENCODING");
> break;
> }
> + break;
> + case BGP_CAPCODE_BGPROLE:
> + if (cap_len != 1) {
> + printf(" BAD ENCODING");
> + break;
> + }
> + printf(" [%s]", bgp_roletype(opt[i]));
> break;
> case BGP_CAPCODE_RESTART:
> if (cap_len < 2 || (cap_len - 2) % 4) {
>
--
:wq Claudio
