Skip to content

Commit

Permalink
Factor out afi/safi processing and the "is this supported" check
Browse files Browse the repository at this point in the history 
It turns out the reason that the default: case was confusing between
MP_REACH and MP_UNREACH was because the unsupported case for MP_REACH
was handled already, so it didn't matter that the code was wrong, it
was never executed.

Instead, factor the afi/safi display and supported check into
its own function, and delete the wrong code.  Leave it to print
an "ERROR:" message if the case statements do get out of sync,
but don't try to print out the attribute in this case, since we
don't have the right length.
  • Loading branch information
@fenner
fenner committed Apr 18, 2019
1 parent 8193828 commit 5d11f84
Showing 1 changed file with 81 additions and 69 deletions.
150 changes: 81 additions & 69 deletions print-bgp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1539,6 +1539,67 @@ check_add_path(netdissect_options *ndo, const u_char *pptr, u_int length,
return 0;
}

static int
bgp_mp_af_print(netdissect_options *ndo,
const u_char *tptr, u_int tlen,
uint16_t *afp, uint8_t *safip)
{
uint16_t af;
uint8_t safi;

af = GET_BE_U_2(tptr);
*afp = af;
safi = GET_U_1(tptr + 2);
*safip = safi;

ND_PRINT("\n\t AFI: %s (%u), %sSAFI: %s (%u)",
tok2str(af_values, "Unknown AFI", af),
af,
(safi>128) ? "vendor specific " : "", /* 128 is meanwhile wellknown */
tok2str(bgp_safi_values, "Unknown SAFI", safi),
safi);

switch(af<<8 | safi) {
case (AFNUM_INET<<8 | SAFNUM_UNICAST):
case (AFNUM_INET<<8 | SAFNUM_MULTICAST):
case (AFNUM_INET<<8 | SAFNUM_UNIMULTICAST):
case (AFNUM_INET<<8 | SAFNUM_LABUNICAST):
case (AFNUM_INET<<8 | SAFNUM_RT_ROUTING_INFO):
case (AFNUM_INET<<8 | SAFNUM_VPNUNICAST):
case (AFNUM_INET<<8 | SAFNUM_VPNMULTICAST):
case (AFNUM_INET<<8 | SAFNUM_VPNUNIMULTICAST):
case (AFNUM_INET<<8 | SAFNUM_MULTICAST_VPN):
case (AFNUM_INET<<8 | SAFNUM_MDT):
case (AFNUM_INET6<<8 | SAFNUM_UNICAST):
case (AFNUM_INET6<<8 | SAFNUM_MULTICAST):
case (AFNUM_INET6<<8 | SAFNUM_UNIMULTICAST):
case (AFNUM_INET6<<8 | SAFNUM_LABUNICAST):
case (AFNUM_INET6<<8 | SAFNUM_VPNUNICAST):
case (AFNUM_INET6<<8 | SAFNUM_VPNMULTICAST):
case (AFNUM_INET6<<8 | SAFNUM_VPNUNIMULTICAST):
case (AFNUM_NSAP<<8 | SAFNUM_UNICAST):
case (AFNUM_NSAP<<8 | SAFNUM_MULTICAST):
case (AFNUM_NSAP<<8 | SAFNUM_UNIMULTICAST):
case (AFNUM_NSAP<<8 | SAFNUM_VPNUNICAST):
case (AFNUM_NSAP<<8 | SAFNUM_VPNMULTICAST):
case (AFNUM_NSAP<<8 | SAFNUM_VPNUNIMULTICAST):
case (AFNUM_L2VPN<<8 | SAFNUM_VPNUNICAST):
case (AFNUM_L2VPN<<8 | SAFNUM_VPNMULTICAST):
case (AFNUM_L2VPN<<8 | SAFNUM_VPNUNIMULTICAST):
case (AFNUM_VPLS<<8 | SAFNUM_VPLS):
break;
default:
ND_TCHECK_LEN(tptr, tlen);
ND_PRINT("\n\t no AFI %u / SAFI %u decoder", af, safi);
if (ndo->ndo_vflag <= 1)
print_unknown_data(ndo, tptr, "\n\t ", tlen);
return -1;
}
return 0;
trunc:
return -2;
}

static int
bgp_nlri_print(netdissect_options *ndo, uint16_t af, uint8_t safi,
const u_char *tptr, u_int len,
Expand Down Expand Up @@ -1696,15 +1757,10 @@ bgp_nlri_print(netdissect_options *ndo, uint16_t af, uint8_t safi,
break;
default:
/*
* We're just confused here.
* tlen was the next-hop length.
ND_TCHECK_LEN(tptr, tlen);
*/
ND_PRINT("\n\t no AFI %u / SAFI %u decoder", af, safi);
/*
if (ndo->ndo_vflag <= 1)
print_unknown_data(ndo, tptr, "\n\t ", tlen);
* This should not happen, we should have been protected
* by bgp_mp_af_print()'s return value.
*/
ND_PRINT("\n\t ERROR: no AFI %u / SAFI %u decoder", af, safi);
advance = -4;
break;
}
Expand All @@ -1727,6 +1783,7 @@ bgp_attr_print(netdissect_options *ndo,
u_int as_size;
int add_path4, add_path6;
u_int path_id = 0;
int ret;

tptr = pptr;
tlen = len;
Expand Down Expand Up @@ -1899,53 +1956,11 @@ bgp_attr_print(netdissect_options *ndo,
ND_TCHECK_3(tptr);
if (tlen < 3)
goto trunc;
af = GET_BE_U_2(tptr);
safi = GET_U_1(tptr + 2);

ND_PRINT("\n\t AFI: %s (%u), %sSAFI: %s (%u)",
tok2str(af_values, "Unknown AFI", af),
af,
(safi>128) ? "vendor specific " : "", /* 128 is meanwhile wellknown */
tok2str(bgp_safi_values, "Unknown SAFI", safi),
safi);

switch(af<<8 | safi) {
case (AFNUM_INET<<8 | SAFNUM_UNICAST):
case (AFNUM_INET<<8 | SAFNUM_MULTICAST):
case (AFNUM_INET<<8 | SAFNUM_UNIMULTICAST):
case (AFNUM_INET<<8 | SAFNUM_LABUNICAST):
case (AFNUM_INET<<8 | SAFNUM_RT_ROUTING_INFO):
case (AFNUM_INET<<8 | SAFNUM_VPNUNICAST):
case (AFNUM_INET<<8 | SAFNUM_VPNMULTICAST):
case (AFNUM_INET<<8 | SAFNUM_VPNUNIMULTICAST):
case (AFNUM_INET<<8 | SAFNUM_MULTICAST_VPN):
case (AFNUM_INET<<8 | SAFNUM_MDT):
case (AFNUM_INET6<<8 | SAFNUM_UNICAST):
case (AFNUM_INET6<<8 | SAFNUM_MULTICAST):
case (AFNUM_INET6<<8 | SAFNUM_UNIMULTICAST):
case (AFNUM_INET6<<8 | SAFNUM_LABUNICAST):
case (AFNUM_INET6<<8 | SAFNUM_VPNUNICAST):
case (AFNUM_INET6<<8 | SAFNUM_VPNMULTICAST):
case (AFNUM_INET6<<8 | SAFNUM_VPNUNIMULTICAST):
case (AFNUM_NSAP<<8 | SAFNUM_UNICAST):
case (AFNUM_NSAP<<8 | SAFNUM_MULTICAST):
case (AFNUM_NSAP<<8 | SAFNUM_UNIMULTICAST):
case (AFNUM_NSAP<<8 | SAFNUM_VPNUNICAST):
case (AFNUM_NSAP<<8 | SAFNUM_VPNMULTICAST):
case (AFNUM_NSAP<<8 | SAFNUM_VPNUNIMULTICAST):
case (AFNUM_L2VPN<<8 | SAFNUM_VPNUNICAST):
case (AFNUM_L2VPN<<8 | SAFNUM_VPNMULTICAST):
case (AFNUM_L2VPN<<8 | SAFNUM_VPNUNIMULTICAST):
case (AFNUM_VPLS<<8 | SAFNUM_VPLS):
break;
default:
ND_TCHECK_LEN(tptr, tlen);
ND_PRINT("\n\t no AFI %u / SAFI %u decoder", af, safi);
if (ndo->ndo_vflag <= 1)
print_unknown_data(ndo, tptr, "\n\t ", tlen);
goto done;
break;
}
ret = bgp_mp_af_print(ndo, tptr, tlen, &af, &safi);
if (ret == -2)
goto trunc;
if (ret < 0)
break;

tptr +=3;

Expand Down Expand Up @@ -2070,10 +2085,11 @@ bgp_attr_print(netdissect_options *ndo,
}
break;
default:
ND_TCHECK_LEN(tptr, tlen);
ND_PRINT("no AFI %u/SAFI %u decoder", af, safi);
if (ndo->ndo_vflag <= 1)
print_unknown_data(ndo, tptr, "\n\t ", tlen);
/*
* bgp_mp_af_print() should have saved us from
* an unsupported AFI/SAFI.
*/
ND_PRINT("ERROR: no AFI %u/SAFI %u nexthop decoder", af, safi);
tptr += tlen;
tlen = 0;
goto done;
Expand Down Expand Up @@ -2111,20 +2127,15 @@ bgp_attr_print(netdissect_options *ndo,
break;
tptr += advance;
}
done:
break;

case BGPTYPE_MP_UNREACH_NLRI:
ND_TCHECK_LEN(tptr, BGP_MP_NLRI_MINSIZE);
af = GET_BE_U_2(tptr);
safi = GET_U_1(tptr + 2);

ND_PRINT("\n\t AFI: %s (%u), %sSAFI: %s (%u)",
tok2str(af_values, "Unknown AFI", af),
af,
(safi>128) ? "vendor specific " : "", /* 128 is meanwhile wellknown */
tok2str(bgp_safi_values, "Unknown SAFI", safi),
safi);
ret = bgp_mp_af_print(ndo, tptr, tlen, &af, &safi);
if (ret == -2)
goto trunc;
if (ret < 0)
break;

if (len == BGP_MP_NLRI_MINSIZE)
ND_PRINT("\n\t End-of-Rib Marker (empty NLRI)");
Expand Down Expand Up @@ -2376,6 +2387,7 @@ bgp_attr_print(netdissect_options *ndo,
print_unknown_data(ndo, pptr, "\n\t ", len);
break;
}
done:
if (ndo->ndo_vflag > 1 && len) { /* omit zero length attributes*/
ND_TCHECK_LEN(pptr, len);
print_unknown_data(ndo, pptr, "\n\t ", len);
Expand Down

0 comments on commit 5d11f84

Please sign in to comment.