[ovs-dev] [PATCH] ofproto-dpif-xlate: Allow sending BFD messages even when RSTP port is not forwarding

Joe Stringer joe at ovn.org
Mon Mar 6 20:16:59 UTC 2017


On 20 February 2017 at 04:27, Mika Väisänen <mika.vaisanen at gmail.com> wrote:
> Interworking of BFD and RSTP does not work, as currently BFD messages are
> dropped if RSTP port is not in forwarding mode. To correct this problem,
> an extra check is added to allow BFD messages to be sent even when
> rstp_forward_state is false.
>
> Note: This patch is made against branch-2.5.
>
> Signed-off-by: Mika Vaisanen <mika.vaisanen at gmail.com>
>
> ---

There's something a bit off about the formatting of the patch, but
it's simple enough that I can just make the equivalent change locally.
The test seems to show the expected behaviour well.

I see now, looking at the codepaths, the bfd packet generation link
through to the compose_output_action() goes like this:

monitor_mport_run()
ofproto_dpif_send_packet()
xlate_send_packet()
ofproto_dpif_execute_actions()
ofproto_dpif_execute_actions__()
xlate_actions()
do_xlate_actions()
xlate_output_action()
compose_output_action()

I didn't realise that when these various protocols (STP, RSTP, CFM,
BFD, etc.) send packets from OVS, it goes through the standard
translation like this. I guess it makes sense :-)

The only concern I had about this patch is if there was a way to end
up broadcasting BFD in a loop because BFD is skipping past the
STP/RSTP forwarding checks. However, I believe that on receive,
xlate_actions() will handle BFD via process_special(), so typically it
should not end up in the path where it's attempting to forward the
packet. If it somehow does get past there, the check that is being
added by this patch will ensure that the BFD packet matches the
settings for this link, or otherwise it will respect the STP/RSTP
forwarding state. So I think it's fine. I wouldn't mind bouncing it
off someone like Jarno just to double-check my reasoning.

As a matter of style, I think this diff in ofproto-dpif-xlate.c is a
little easier to follow---and would cover the CFM case as well:

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 89fc3a44a0d1..578fef168b30 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3127,6 +3127,10 @@ compose_output_action__(struct xlate_ctx *ctx,
ofp_port_t ofp_port,
                 }
                 return;
             }
+        } else if ((xport->cfm && cfm_should_process_flow(xport->cfm,
flow, wc))
+                   || (xport->bfd && bfd_should_process_flow(xport->bfd, flow,
+                                                             wc))) {
+            /* Pass; STP should not block link health detection. */
         } else if (!xport_stp_forward_state(xport) ||
                    !xport_rstp_forward_state(xport)) {
             if (ctx->xbridge->stp != NULL) {


More information about the dev mailing list