From f50b1f7c226753c1f4c0ccfeeb78271106924cd8 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Fri, 4 Oct 2024 09:38:25 -0400 Subject: [PATCH 1/2] zebra: Move pw status settting until after we get results Currently the pw code sets the status of the pw for install and uninstall immediately when notifying the dplane. This is incorrect in that we do not actually know the status at this point in time. When we get the result is when to set the status. Signed-off-by: Donald Sharp --- zebra/zebra_pw.c | 26 +++++++++++++++++++++----- zebra/zebra_pw.h | 1 + zebra/zebra_rib.c | 25 +------------------------ 3 files changed, 23 insertions(+), 29 deletions(-) diff --git a/zebra/zebra_pw.c b/zebra/zebra_pw.c index deed3b6ad3c3..d7128a1f3946 100644 --- a/zebra/zebra_pw.c +++ b/zebra/zebra_pw.c @@ -170,9 +170,6 @@ static void zebra_pw_install(struct zebra_pw *pw) zebra_pw_install_failure(pw, PW_NOT_FORWARDING); return; } - - if (pw->status != PW_FORWARDING) - zebra_pw_update_status(pw, PW_FORWARDING); } static void zebra_pw_uninstall(struct zebra_pw *pw) @@ -188,9 +185,28 @@ static void zebra_pw_uninstall(struct zebra_pw *pw) /* ignore any possible error */ hook_call(pw_uninstall, pw); dplane_pw_uninstall(pw); +} + +void zebra_pw_handle_dplane_results(struct zebra_dplane_ctx *ctx) +{ + struct zebra_pw *pw; + struct zebra_vrf *vrf; + enum dplane_op_e op; + + op = dplane_ctx_get_op(ctx); + + vrf = zebra_vrf_lookup_by_id(dplane_ctx_get_vrf(ctx)); + pw = zebra_pw_find(vrf, dplane_ctx_get_ifname(ctx)); - if (zebra_pw_enabled(pw)) - zebra_pw_update_status(pw, PW_NOT_FORWARDING); + if (dplane_ctx_get_status(ctx) != ZEBRA_DPLANE_REQUEST_SUCCESS) { + if (pw) + zebra_pw_install_failure(pw, dplane_ctx_get_pw_status(ctx)); + } else { + if (op == DPLANE_OP_PW_INSTALL && pw->status != PW_FORWARDING) + zebra_pw_update_status(pw, PW_FORWARDING); + else if (op == DPLANE_OP_PW_UNINSTALL && zebra_pw_enabled(pw)) + zebra_pw_update_status(pw, PW_NOT_FORWARDING); + } } /* diff --git a/zebra/zebra_pw.h b/zebra/zebra_pw.h index 431d663f7ce1..e037a55048d8 100644 --- a/zebra/zebra_pw.h +++ b/zebra/zebra_pw.h @@ -64,6 +64,7 @@ void zebra_pw_init_vrf(struct zebra_vrf *); void zebra_pw_exit_vrf(struct zebra_vrf *); void zebra_pw_terminate(void); void zebra_pw_vty_init(void); +void zebra_pw_handle_dplane_results(struct zebra_dplane_ctx *ctx); #ifdef __cplusplus } diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c index 2d2be4fc780b..8ebc193fba99 100644 --- a/zebra/zebra_rib.c +++ b/zebra/zebra_rib.c @@ -4850,29 +4850,6 @@ void rib_close_table(struct route_table *table) } } -/* - * Handler for async dataplane results after a pseudowire installation - */ -static void handle_pw_result(struct zebra_dplane_ctx *ctx) -{ - struct zebra_pw *pw; - struct zebra_vrf *vrf; - - /* The pseudowire code assumes success - we act on an error - * result for installation attempts here. - */ - if (dplane_ctx_get_op(ctx) != DPLANE_OP_PW_INSTALL) - return; - - if (dplane_ctx_get_status(ctx) != ZEBRA_DPLANE_REQUEST_SUCCESS) { - vrf = zebra_vrf_lookup_by_id(dplane_ctx_get_vrf(ctx)); - pw = zebra_pw_find(vrf, dplane_ctx_get_ifname(ctx)); - if (pw) - zebra_pw_install_failure(pw, - dplane_ctx_get_pw_status(ctx)); - } -} - /* * Handle results from the dataplane system. Dequeue update context * structs, dispatch to appropriate internal handlers. @@ -4979,7 +4956,7 @@ static void rib_process_dplane_results(struct event *thread) case DPLANE_OP_PW_INSTALL: case DPLANE_OP_PW_UNINSTALL: - handle_pw_result(ctx); + zebra_pw_handle_dplane_results(ctx); break; case DPLANE_OP_SYS_ROUTE_ADD: From a8af2b2a9d0f9fe059f645ee033087a293ab6b35 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Fri, 4 Oct 2024 09:51:46 -0400 Subject: [PATCH 2/2] zebra: Do not retry in 30 seconds on pw reachability failure Currently the zebra pw code has setup a retry to install the pw after 30 seconds when it is decided that reachability to the pw is gone. This causes a failure mode where the pw code just goes and re-installs the pw after 30 seconds in the non-reachability case. Instead it should just be reinstalling after reachability is restored. Signed-off-by: Donald Sharp --- zebra/zebra_pw.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/zebra/zebra_pw.c b/zebra/zebra_pw.c index d7128a1f3946..c8ffaf0bbe36 100644 --- a/zebra/zebra_pw.c +++ b/zebra/zebra_pw.c @@ -147,7 +147,6 @@ void zebra_pw_update(struct zebra_pw *pw) { if (zebra_pw_check_reachability(pw) < 0) { zebra_pw_uninstall(pw); - zebra_pw_install_failure(pw, PW_NOT_FORWARDING); /* wait for NHT and try again later */ } else { /* @@ -167,6 +166,14 @@ static void zebra_pw_install(struct zebra_pw *pw) hook_call(pw_install, pw); if (dplane_pw_install(pw) == ZEBRA_DPLANE_REQUEST_FAILURE) { + /* + * Realistically this is never going to fail passing + * the pw data down to the dplane. The failure modes + * look like impossible events but we still return + * on them.... but I don't see a real clean way to remove this + * at all. So let's just leave the retry mechanism for + * the moment. + */ zebra_pw_install_failure(pw, PW_NOT_FORWARDING); return; }