From 99e66e520b2cd79b7c42ec59ac215cd87e10a1a1 Mon Sep 17 00:00:00 2001 From: Trey Aspelund Date: Fri, 30 Sep 2022 19:58:31 +0000 Subject: [PATCH 1/2] zebra: show local/rx values in RA mismatch debugs When we process a received Router Advertisement we have some logic in place to detect and log mismatches in a handful of flags/values. However, these logs do not include what the actual values are, which means it's up to the operator to grab a packet capture and compare that against the local configuration... So let's make life a little easier by including those in the log itself. Before: ``` 2022/09/30 20:37:16 ZEBRA: [KV2V1-7GM7G][EC 4043309149] enp1s0(2): Rx RA - our AdvCurHopLimit doesn't agree with fe80::5054:ff:feca:b085 2022/09/30 20:37:16 ZEBRA: [KS0BP-4GR8K][EC 4043309149] enp1s0(2): Rx RA - our AdvManagedFlag doesn't agree with fe80::5054:ff:feca:b085 2022/09/30 20:37:16 ZEBRA: [RE4EC-VYEJ2][EC 4043309149] enp1s0(2): Rx RA - our AdvOtherConfigFlag doesn't agree with fe80::5054:ff:feca:b085 2022/09/30 20:37:16 ZEBRA: [X6794-9MW18][EC 4043309149] enp1s0(2): Rx RA - our AdvReachableTime doesn't agree with fe80::5054:ff:feca:b085 2022/09/30 20:37:16 ZEBRA: [S1KXC-H8F4W][EC 4043309149] enp1s0(2): Rx RA - our AdvRetransTimer doesn't agree with fe80::5054:ff:feca:b085 ``` After: ``` Sep 30 20:45:18 ub20-2 zebra[47487]: [GSW5Z-V7DZN][EC 4043309149] enp1s0(2): Rx RA - our AdvCurHopLimit (14) doesn't agree with fe80::5054:ff:fe9a:e2ca (64) Sep 30 20:45:18 ub20-2 zebra[47487]: [RHHTS-F96DR][EC 4043309149] enp1s0(2): Rx RA - our AdvManagedFlag (0) doesn't agree with fe80::5054:ff:fe9a:e2ca (1) Sep 30 20:45:18 ub20-2 zebra[47487]: [MNBY3-FTN6W][EC 4043309149] enp1s0(2): Rx RA - our AdvOtherConfigFlag (0) doesn't agree with fe80::5054:ff:fe9a:e2ca (1) Sep 30 20:45:18 ub20-2 zebra[47487]: [GG62B-XXWR0][EC 4043309149] enp1s0(2): Rx RA - our AdvReachableTime (20) doesn't agree with fe80::5054:ff:fe9a:e2ca (777) Sep 30 20:45:18 ub20-2 zebra[47487]: [YG220-D6B4H][EC 4043309149] enp1s0(2): Rx RA - our AdvRetransTimer (13) doesn't agree with fe80::5054:ff:fe9a:e2ca (0) ``` Signed-off-by: Trey Aspelund --- zebra/rtadv.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/zebra/rtadv.c b/zebra/rtadv.c index 93590a2f16..8059a545e4 100644 --- a/zebra/rtadv.c +++ b/zebra/rtadv.c @@ -664,8 +664,9 @@ static void rtadv_process_advert(uint8_t *msg, unsigned int len, zif->rtadv.lastadvcurhoplimit.tv_sec == 0)) { flog_warn( EC_ZEBRA_RA_PARAM_MISMATCH, - "%s(%u): Rx RA - our AdvCurHopLimit doesn't agree with %s", - ifp->name, ifp->ifindex, addr_str); + "%s(%u): Rx RA - our AdvCurHopLimit (%u) doesn't agree with %s (%u)", + ifp->name, ifp->ifindex, zif->rtadv.AdvCurHopLimit, + addr_str, radvert->nd_ra_curhoplimit); monotime(&zif->rtadv.lastadvcurhoplimit); } @@ -676,8 +677,11 @@ static void rtadv_process_advert(uint8_t *msg, unsigned int len, zif->rtadv.lastadvmanagedflag.tv_sec == 0)) { flog_warn( EC_ZEBRA_RA_PARAM_MISMATCH, - "%s(%u): Rx RA - our AdvManagedFlag doesn't agree with %s", - ifp->name, ifp->ifindex, addr_str); + "%s(%u): Rx RA - our AdvManagedFlag (%u) doesn't agree with %s (%u)", + ifp->name, ifp->ifindex, zif->rtadv.AdvManagedFlag, + addr_str, + !!CHECK_FLAG(radvert->nd_ra_flags_reserved, + ND_RA_FLAG_MANAGED)); monotime(&zif->rtadv.lastadvmanagedflag); } @@ -688,8 +692,11 @@ static void rtadv_process_advert(uint8_t *msg, unsigned int len, zif->rtadv.lastadvotherconfigflag.tv_sec == 0)) { flog_warn( EC_ZEBRA_RA_PARAM_MISMATCH, - "%s(%u): Rx RA - our AdvOtherConfigFlag doesn't agree with %s", - ifp->name, ifp->ifindex, addr_str); + "%s(%u): Rx RA - our AdvOtherConfigFlag (%u) doesn't agree with %s (%u)", + ifp->name, ifp->ifindex, zif->rtadv.AdvOtherConfigFlag, + addr_str, + !!CHECK_FLAG(radvert->nd_ra_flags_reserved, + ND_RA_FLAG_OTHER)); monotime(&zif->rtadv.lastadvotherconfigflag); } @@ -700,8 +707,9 @@ static void rtadv_process_advert(uint8_t *msg, unsigned int len, zif->rtadv.lastadvreachabletime.tv_sec == 0)) { flog_warn( EC_ZEBRA_RA_PARAM_MISMATCH, - "%s(%u): Rx RA - our AdvReachableTime doesn't agree with %s", - ifp->name, ifp->ifindex, addr_str); + "%s(%u): Rx RA - our AdvReachableTime (%u) doesn't agree with %s (%u)", + ifp->name, ifp->ifindex, zif->rtadv.AdvReachableTime, + addr_str, ntohl(radvert->nd_ra_reachable)); monotime(&zif->rtadv.lastadvreachabletime); } @@ -712,8 +720,9 @@ static void rtadv_process_advert(uint8_t *msg, unsigned int len, zif->rtadv.lastadvretranstimer.tv_sec == 0)) { flog_warn( EC_ZEBRA_RA_PARAM_MISMATCH, - "%s(%u): Rx RA - our AdvRetransTimer doesn't agree with %s", - ifp->name, ifp->ifindex, addr_str); + "%s(%u): Rx RA - our AdvRetransTimer (%u) doesn't agree with %s (%u)", + ifp->name, ifp->ifindex, zif->rtadv.AdvRetransTimer, + addr_str, ntohl(radvert->nd_ra_retransmit)); monotime(&zif->rtadv.lastadvretranstimer); } From 820a9cadb2b3ad4c57c9e193455cd26a3cc081d4 Mon Sep 17 00:00:00 2001 From: Trey Aspelund Date: Fri, 30 Sep 2022 20:47:54 +0000 Subject: [PATCH 2/2] zebra: ignore unspec RetransTimer in RA validation Section 6.2.7 of RFC 4861 states that a router SHOULD log inconsistencies in RA information detected on a given link: ``` - Cur Hop Limit values (except for the unspecified value of zero other inconsistencies SHOULD be logged to system network management). - Values of the M or O flags. - Reachable Time values (except for the unspecified value of zero). - Retrans Timer values (except for the unspecified value of zero). - Values in the MTU options. - Preferred and Valid Lifetimes for the same prefix. If AdvPreferredLifetime and/or AdvValidLifetime decrement in real time as specified in Section 6.2.1 then the comparison of the lifetimes cannot compare the content of the fields in the Router Advertisement, but must instead compare the time at which the prefix will become deprecated and invalidated, respectively. Due to link propagation delays and potentially poorly synchronized clocks between the routers such comparison SHOULD allow some time skew. ``` We were not logging inconsistencies if "the unspecified value of zero" was used for Reachable Time but were logging them for Retrans Timer. This updates the validation check to also skip the logging of Retrans Timer inconsistencies if either local/rx value is 0. Signed-off-by: Trey Aspelund --- zebra/rtadv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/zebra/rtadv.c b/zebra/rtadv.c index 8059a545e4..127888d655 100644 --- a/zebra/rtadv.c +++ b/zebra/rtadv.c @@ -713,7 +713,8 @@ static void rtadv_process_advert(uint8_t *msg, unsigned int len, monotime(&zif->rtadv.lastadvreachabletime); } - if ((ntohl(radvert->nd_ra_retransmit) != + if ((radvert->nd_ra_retransmit && zif->rtadv.AdvRetransTimer) && + (ntohl(radvert->nd_ra_retransmit) != (unsigned int)zif->rtadv.AdvRetransTimer) && (monotime_since(&zif->rtadv.lastadvretranstimer, NULL) > SIXHOUR2USEC ||