[ovs-dev] [PATCH v7 ovn 5/5] ovn: integrate bfd for static routes

Lorenzo Bianconi lorenzo.bianconi at redhat.com
Thu Jan 7 15:27:50 UTC 2021


> On Tue, Jan 5, 2021 at 11:26 PM Lorenzo Bianconi
> <lorenzo.bianconi at redhat.com> wrote:
> >
> > Introduce the bfd reference in logical_router_static_router table
> > in order to check if the next-hop is properly running using the BFD
> > protocol. The CMS is supposed to populate bfd column with the proper
> > reference otherwise the BFD status is set to admin_down.
> > Add BFD tests in system-ovn.at.
> >
> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
> 
> Hi Lorenzo,
> 
> I noticed a couple of issues.
> 
> With a gateway router configured with a static route and BFD, the
> status of the BFD never comes up.
> The BFD goes to up state for a few seconds and then it's down all the time.
> 
> I saw the similar situation with using a distributed router with a
> gateway router port configured with 3 gateway chassis.
> When I move the cr-xxx redirect-chassis port binding to another
> gateway chassis, I see the same issue. It doesn't
> happens 100% of the time though.
> 
> The status in the BFD packet shows "up". I think there is some issue
> in the pinctrl code which updates
> the SB DB status.

Hi Numan,

ack, thx for spotting this issue (gw-router use-case was not covered properly).
I will fix it in v8 and I will add some unit-tests for it.

Regards,
Lorenzo

> 
> Thanks
> Numan
> 
> > ---
> >  NEWS                |   3 +-
> >  northd/ovn-northd.c |  45 ++++++++++++----
> >  ovn-nb.ovsschema    |   6 ++-
> >  ovn-nb.xml          |   7 +++
> >  tests/atlocal.in    |   3 ++
> >  tests/ovn-nbctl.at  |   8 ++-
> >  tests/system-ovn.at | 126 ++++++++++++++++++++++++++++++++++++++++++++
> >  7 files changed, 185 insertions(+), 13 deletions(-)
> >
> > diff --git a/NEWS b/NEWS
> > index f7198d2a2..d6ac4e736 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -1,7 +1,8 @@
> >  Post-v20.12.0
> >  -------------------------
> >    - Support ECMP multiple nexthops for reroute router policies.
> > -   - BFD protocol support according to RFC880 [0]. IPv6 is not suported yet.
> > +   - BFD protocol support according to RFC880 [0]. Introduce next-hop BFD
> > +     availability check for OVN static routes. IPv6 is not suported yet.
> >       [0] https://tools.ietf.org/html/rfc5880)
> >
> >  OVN v20.12.0 - xx xxx xxxx
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index e72693670..f4103d4ec 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -7948,7 +7948,8 @@ route_hash(struct parsed_route *route)
> >   * Otherwise return NULL. */
> >  static struct parsed_route *
> >  parsed_routes_add(struct ovs_list *routes,
> > -                  const struct nbrec_logical_router_static_route *route)
> > +                  const struct nbrec_logical_router_static_route *route,
> > +                  struct hmap *bfd_connections)
> >  {
> >      /* Verify that the next hop is an IP address with an all-ones mask. */
> >      struct in6_addr nexthop;
> > @@ -7989,6 +7990,25 @@ parsed_routes_add(struct ovs_list *routes,
> >          return NULL;
> >      }
> >
> > +    const struct nbrec_bfd *nb_bt = route->bfd;
> > +    if (nb_bt && !strcmp(nb_bt->dst_ip, route->nexthop)) {
> > +        struct bfd_entry *bfd_e;
> > +
> > +        bfd_e = bfd_port_lookup(bfd_connections, nb_bt->logical_port,
> > +                                nb_bt->dst_ip);
> > +        if (bfd_e) {
> > +            bfd_e->ref = true;
> > +        }
> > +
> > +        if (!strcmp(nb_bt->status, "admin_down")) {
> > +            nbrec_bfd_set_status(nb_bt, "down");
> > +        }
> > +
> > +        if (!strcmp(nb_bt->status, "down")) {
> > +            return NULL;
> > +        }
> > +    }
> > +
> >      struct parsed_route *pr = xzalloc(sizeof *pr);
> >      pr->prefix = prefix;
> >      pr->plen = plen;
> > @@ -10546,7 +10566,7 @@ build_ip_routing_flows_for_lrouter_port(
> >  static void
> >  build_static_route_flows_for_lrouter(
> >          struct ovn_datapath *od, struct hmap *lflows,
> > -        struct hmap *ports)
> > +        struct hmap *ports, struct hmap *bfd_connections)
> >  {
> >      if (od->nbr) {
> >          ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING_ECMP, 150,
> > @@ -10558,7 +10578,8 @@ build_static_route_flows_for_lrouter(
> >          struct ecmp_groups_node *group;
> >          for (int i = 0; i < od->nbr->n_static_routes; i++) {
> >              struct parsed_route *route =
> > -                parsed_routes_add(&parsed_routes, od->nbr->static_routes[i]);
> > +                parsed_routes_add(&parsed_routes, od->nbr->static_routes[i],
> > +                                  bfd_connections);
> >              if (!route) {
> >                  continue;
> >              }
> > @@ -11578,7 +11599,8 @@ struct lswitch_flow_build_info {
> >
> >  static void
> >  build_lswitch_and_lrouter_iterate_by_od(struct ovn_datapath *od,
> > -                                        struct lswitch_flow_build_info *lsi)
> > +                                        struct lswitch_flow_build_info *lsi,
> > +                                        struct hmap *bfd_connections)
> >  {
> >      /* Build Logical Switch Flows. */
> >      build_lswitch_lflows_pre_acl_and_acl(od, lsi->port_groups, lsi->lflows,
> > @@ -11593,7 +11615,8 @@ build_lswitch_and_lrouter_iterate_by_od(struct ovn_datapath *od,
> >      build_neigh_learning_flows_for_lrouter(od, lsi->lflows, &lsi->match,
> >                                             &lsi->actions);
> >      build_ND_RA_flows_for_lrouter(od, lsi->lflows);
> > -    build_static_route_flows_for_lrouter(od, lsi->lflows, lsi->ports);
> > +    build_static_route_flows_for_lrouter(od, lsi->lflows, lsi->ports,
> > +                                         bfd_connections);
> >      build_mcast_lookup_flows_for_lrouter(od, lsi->lflows, &lsi->match,
> >                                           &lsi->actions);
> >      build_ingress_policy_flows_for_lrouter(od, lsi->lflows, lsi->ports);
> > @@ -11640,7 +11663,8 @@ build_lswitch_and_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
> >                                  struct hmap *port_groups, struct hmap *lflows,
> >                                  struct hmap *mcgroups,
> >                                  struct hmap *igmp_groups,
> > -                                struct shash *meter_groups, struct hmap *lbs)
> > +                                struct shash *meter_groups, struct hmap *lbs,
> > +                                struct hmap *bfd_connections)
> >  {
> >      struct ovn_datapath *od;
> >      struct ovn_port *op;
> > @@ -11665,7 +11689,7 @@ build_lswitch_and_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
> >       * will move here and will be reogranized by iterator type.
> >       */
> >      HMAP_FOR_EACH (od, key_node, datapaths) {
> > -        build_lswitch_and_lrouter_iterate_by_od(od, &lsi);
> > +        build_lswitch_and_lrouter_iterate_by_od(od, &lsi, bfd_connections);
> >      }
> >      HMAP_FOR_EACH (op, key_node, ports) {
> >          build_lswitch_and_lrouter_iterate_by_op(op, &lsi);
> > @@ -11757,13 +11781,14 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths,
> >               struct hmap *ports, struct hmap *port_groups,
> >               struct hmap *mcgroups, struct hmap *igmp_groups,
> >               struct shash *meter_groups,
> > -             struct hmap *lbs)
> > +             struct hmap *lbs, struct hmap *bfd_connections)
> >  {
> >      struct hmap lflows = HMAP_INITIALIZER(&lflows);
> >
> >      build_lswitch_and_lrouter_flows(datapaths, ports,
> >                                      port_groups, &lflows, mcgroups,
> > -                                    igmp_groups, meter_groups, lbs);
> > +                                    igmp_groups, meter_groups, lbs,
> > +                                    bfd_connections);
> >
> >      /* Collecting all unique datapath groups. */
> >      struct hmap dp_groups = HMAP_INITIALIZER(&dp_groups);
> > @@ -12772,7 +12797,7 @@ ovnnb_db_run(struct northd_context *ctx,
> >      build_meter_groups(ctx, &meter_groups);
> >      build_bfd_table(ctx, &bfd_connections, ports);
> >      build_lflows(ctx, datapaths, ports, &port_groups, &mcast_groups,
> > -                 &igmp_groups, &meter_groups, &lbs);
> > +                 &igmp_groups, &meter_groups, &lbs, &bfd_connections);
> >      ovn_update_ipv6_prefix(ports);
> >
> >      sync_address_sets(ctx);
> > diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
> > index 3b8a94276..f6e89176d 100644
> > --- a/ovn-nb.ovsschema
> > +++ b/ovn-nb.ovsschema
> > @@ -1,7 +1,7 @@
> >  {
> >      "name": "OVN_Northbound",
> >      "version": "5.31.0",
> > -    "cksum": "1925435483 28290",
> > +    "cksum": "4128092472 28518",
> >      "tables": {
> >          "NB_Global": {
> >              "columns": {
> > @@ -374,6 +374,10 @@
> >                                      "min": 0, "max": 1}},
> >                  "nexthop": {"type": "string"},
> >                  "output_port": {"type": {"key": "string", "min": 0, "max": 1}},
> > +                "bfd": {"type": {"key": {"type": "uuid", "refTable": "BFD",
> > +                                          "refType": "weak"},
> > +                                  "min": 0,
> > +                                  "max": 1}},
> >                  "options": {
> >                      "type": {"key": "string", "value": "string",
> >                               "min": 0, "max": "unlimited"}},
> > diff --git a/ovn-nb.xml b/ovn-nb.xml
> > index ab4adb814..ac3561e84 100644
> > --- a/ovn-nb.xml
> > +++ b/ovn-nb.xml
> > @@ -2644,6 +2644,13 @@
> >        </p>
> >      </column>
> >
> > +    <column name="bfd">
> > +      <p>
> > +        Reference to <ref table="BFD"/> row if the route has associated a
> > +        BFD session
> > +      </p>
> > +    </column>
> > +
> >      <column name="external_ids" key="ic-learned-route">
> >        <code>ovn-ic</code> populates this key if the route is learned from the
> >        global <ref db="OVN_IC_Southbound"/> database.  In this case the value
> > diff --git a/tests/atlocal.in b/tests/atlocal.in
> > index d9a4c91d4..5ebc8e117 100644
> > --- a/tests/atlocal.in
> > +++ b/tests/atlocal.in
> > @@ -181,6 +181,9 @@ fi
> >  # Set HAVE_DIBBLER-SERVER
> >  find_command dibbler-server
> >
> > +# Set HAVE_BFDD_BEACON
> > +find_command bfdd-beacon
> > +
> >  # Turn off proxies.
> >  unset http_proxy
> >  unset https_proxy
> > diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
> > index 01edfcbc1..2827b223c 100644
> > --- a/tests/ovn-nbctl.at
> > +++ b/tests/ovn-nbctl.at
> > @@ -1617,7 +1617,13 @@ IPv6 Routes
> >              2001:db8::/64        2001:db8:0:f102::1 dst-ip lp0
> >            2001:db8:1::/64        2001:db8:0:f103::1 dst-ip
> >                       ::/0        2001:db8:0:f101::1 dst-ip
> > -])])
> > +])
> > +
> > +AT_CHECK([ovn-nbctl lrp-add lr0 lr0-p0 00:00:01:01:02:03 192.168.10.1/24])
> > +bfd_uuid=$(ovn-nbctl create bfd logical_port=lr0-p0 dst_ip=100.0.0.50 status=down min_tx=250 min_rx=250 detect_mult=10)
> > +AT_CHECK([ovn-nbctl lr-route-add lr0 100.0.0.0/24 192.168.0.1])
> > +route_uuid=$(fetch_column nb:logical_router_static_route _uuid ip_prefix="100.0.0.0/24")
> > +AT_CHECK([ovn-nbctl set logical_router_static_route $route_uuid bfd=$bfd_uuid])])
> >
> >  dnl ---------------------------------------------------------------------
> >
> > diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> > index 1e73001ab..bb908123b 100644
> > --- a/tests/system-ovn.at
> > +++ b/tests/system-ovn.at
> > @@ -5531,3 +5531,129 @@ as
> >  OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d
> >  /.*terminating with signal 15.*/d"])
> >  AT_CLEANUP
> > +
> > +AT_SETUP([ovn -- BFD])
> > +AT_SKIP_IF([test $HAVE_BFDD_BEACON = no])
> > +AT_SKIP_IF([test $HAVE_TCPDUMP = no])
> > +AT_KEYWORDS([ovn-bfd])
> > +
> > +ovn_start
> > +OVS_TRAFFIC_VSWITCHD_START()
> > +
> > +ADD_BR([br-int])
> > +ADD_BR([br-ext])
> > +
> > +check ovs-ofctl add-flow br-ext action=normal
> > +# Set external-ids in br-int needed for ovn-controller
> > +check ovs-vsctl \
> > +        -- set Open_vSwitch . external-ids:system-id=hv1 \
> > +        -- set Open_vSwitch . external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
> > +        -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
> > +        -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
> > +        -- set bridge br-int fail-mode=secure other-config:disable-in-band=true
> > +
> > +# Start ovn-controller
> > +start_daemon ovn-controller
> > +
> > +check ovn-nbctl lr-add R1
> > +
> > +check ovn-nbctl ls-add sw0
> > +check ovn-nbctl ls-add sw1
> > +check ovn-nbctl ls-add public
> > +
> > +check ovn-nbctl lrp-add R1 rp-sw0 00:00:01:01:02:03 192.168.1.1/24
> > +check ovn-nbctl lrp-add R1 rp-sw1 00:00:03:01:02:03 192.168.2.1/24
> > +check ovn-nbctl lrp-add R1 rp-public 00:00:02:01:02:03 172.16.1.1/24 \
> > +    -- lrp-set-gateway-chassis rp-public hv1
> > +
> > +check ovn-nbctl lsp-add sw0 sw0-rp -- set Logical_Switch_Port sw0-rp \
> > +    type=router options:router-port=rp-sw0 \
> > +    -- lsp-set-addresses sw0-rp router
> > +check ovn-nbctl lsp-add sw1 sw1-rp -- set Logical_Switch_Port sw1-rp \
> > +    type=router options:router-port=rp-sw1 \
> > +    -- lsp-set-addresses sw1-rp router
> > +
> > +check ovn-nbctl lsp-add public public-rp -- set Logical_Switch_Port public-rp \
> > +    type=router options:router-port=rp-public \
> > +    -- lsp-set-addresses public-rp router
> > +
> > +ADD_NAMESPACES(sw01)
> > +ADD_VETH(sw01, sw01, br-int, "192.168.1.2/24", "f0:00:00:01:02:03", \
> > +         "192.168.1.1")
> > +check ovn-nbctl lsp-add sw0 sw01 \
> > +    -- lsp-set-addresses sw01 "f0:00:00:01:02:03 192.168.1.2"
> > +
> > +ADD_NAMESPACES(sw11)
> > +ADD_VETH(sw11, sw11, br-int, "192.168.2.2/24", "f0:00:00:02:02:03", \
> > +         "192.168.2.1")
> > +check ovn-nbctl lsp-add sw1 sw11 \
> > +    -- lsp-set-addresses sw11 "f0:00:00:02:02:03 192.168.2.2"
> > +
> > +ADD_NAMESPACES(server)
> > +NS_CHECK_EXEC([server], [ip link set dev lo up])
> > +ADD_VETH(s1, server, br-ext, "172.16.1.50/24", "f0:00:00:01:02:05", \
> > +         "172.16.1.1")
> > +
> > +AT_CHECK([ovs-vsctl set Open_vSwitch . external-ids:ovn-bridge-mappings=phynet:br-ext])
> > +check ovn-nbctl lsp-add public public1 \
> > +        -- lsp-set-addresses public1 unknown \
> > +        -- lsp-set-type public1 localnet \
> > +        -- lsp-set-options public1 network_name=phynet
> > +
> > +NS_CHECK_EXEC([server], [bfdd-beacon --listen=172.16.1.50], [0])
> > +NS_CHECK_EXEC([server], [bfdd-control allow 172.16.1.1], [0], [dnl
> > +Allowing connections from 172.16.1.1
> > +])
> > +
> > +uuid=$(ovn-nbctl create bfd logical_port=rp-public dst_ip=172.16.1.50 min_tx=250 min_rx=250 detect_mult=10)
> > +check ovn-nbctl lr-route-add R1 100.0.0.0/8 172.16.1.50
> > +route_uuid=$(fetch_column nb:logical_router_static_route _uuid ip_prefix="100.0.0.0/8")
> > +check ovn-nbctl set logical_router_static_route $route_uuid bfd=$uuid
> > +check ovn-nbctl --wait=hv sync
> > +
> > +wait_column "up" nb:bfd status logical_port=rp-public
> > +OVS_WAIT_UNTIL([ovn-sbctl dump-flows R1 | grep 'match=(ip4.dst == 100.0.0.0/8)' | grep -q 172.16.1.50])
> > +
> > +# un-associate the bfd connection and the static route
> > +check ovn-nbctl clear logical_router_static_route $route_uuid bfd
> > +wait_column "admin_down" nb:bfd status logical_port=rp-public
> > +NS_CHECK_EXEC([server], [tcpdump -nni s1 udp port 3784 -Q in > bfd.pcap &])
> > +sleep 5
> > +kill $(pidof tcpdump)
> > +AT_CHECK([grep -qi bfd bfd.pcap],[1])
> > +
> > +# restart the connection
> > +check ovn-nbctl set logical_router_static_route $route_uuid bfd=$uuid
> > +wait_column "up" nb:bfd status logical_port=rp-public
> > +
> > +# stop bfd endpoint
> > +NS_CHECK_EXEC([server], [bfdd-control stop], [0], [dnl
> > +stopping
> > +])
> > +
> > +wait_column "down" nb:bfd status logical_port=rp-public
> > +OVS_WAIT_UNTIL([test "$(ovn-sbctl dump-flows R1 | grep 'match=(ip4.dst == 100.0.0.0/8)' | grep 172.16.1.50)" = ""])
> > +
> > +# remove bfd entry
> > +ovn-nbctl destroy bfd $uuid
> > +check_row_count bfd 0
> > +NS_CHECK_EXEC([server], [tcpdump -nni s1 udp port 3784 -Q in > bfd.pcap &])
> > +sleep 5
> > +kill $(pidof tcpdump)
> > +AT_CHECK([grep -qi bfd bfd.pcap],[1])
> > +
> > +kill $(pidof ovn-controller)
> > +
> > +as ovn-sb
> > +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> > +
> > +as ovn-nb
> > +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> > +
> > +as northd
> > +OVS_APP_EXIT_AND_WAIT([ovn-northd])
> > +
> > +as
> > +OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d
> > +/.*terminating with signal 15.*/d"])
> > +AT_CLEANUP
> > --
> > 2.29.2
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
> 


More information about the dev mailing list