Hi Lorenzo,

One note for the whole series is that there seems to be some inconsistency with the acronym "bfd". For instance, in patch 3, there is a function called "bpf_monitor_need_update()" that I assume was supposed to be "bfd_monitor_need_update()". In patch 4, the commit message refers to "bfp".

Also, in general, I'm a bit confused about the intention for IPv6 support. For instance, bfd_entry in pinctrl.c has an IPv4 ip_src and ip_dst field, bfd_monitor_put_bfd_msg() always creates an ipv4 packet, and bfd_monitor_check_sb_conf() performs an IPv4 validation check on the sb_bt parameter. This makes it seem like BFD is being added with IPv4 support only. However, pinctrl_check_bfd_msg() allows an ethertype of ETH_TYPE_IPV6, ovn-northd creates handle_bfd_msg() flows that match on IPv6 addresses, and there is nothing to prevent IPv6 dst_ips from being configured in the databases.

On 12/22/20 3:54 PM, Lorenzo Bianconi wrote:
Introduce BFD protocol in ovn-controller according to RFC5880 [0]
We added BFD implementation in ovn since layered protocols usually request to
enable it on ovn entities (e.g. logical router ports) while ovs implementation
relies on physical entities (e.g. ovs interfaces).
Moreover we would establish a BFD session between a given ovn-port and
multiple peers (1:n relation). A typical use-case is reported in [1].
In the current implementation Asynchronous mode is fully supported, while Demand
mode is supported only on rx side.

[0] - https://tools.ietf.org/html/rfc5880
[1] - https://bugzilla.redhat.com/show_bug.cgi?id=1847570

Changes since v4:
- fix open-bfdd restart issue
- do not send BFD packets in admin_down state
- fix BFD table sync in nb/sb db
- fix BFD and logical_router_static_route tables sync
- improve ovn-northd.at/system-ovn.at tests

Changes since v3:
- fix 100% cpu utilization if min_tx/min_rx are 0
- improve unit tests
- move system tests in patch 5
- set admin_down as default state if no static route is associated to
   the BFD session
- fix possible memory leak in bfd_monitor_destroy()

Changes since v2:
- introduce weak reference in Logical_Router_Static_Route table to
   the related BFD table
- remove idl_index for BFD table and perform lookup using local map
- remove unnecessary filed in bfd_entry struct
- add external_ids column in BFD nb/sb table
- make status info not mandatory configuring BFD table in nb db

Changes since v1:
- rebase ontop of ovn master
- rename handle_bfd_msg action in handle_bfd_msg()
- add ovn-northd unit-tests
- improve commit logs
- remove global BFD control check
- introduce BFD support for ovn static routes

Lorenzo Bianconi (5):
   controller: introduce BFD tx path in ovn-controller
   action: introduce handle_bfd_msg() action
   controller: bfd: introduce BFD state machine
   bfp: support demand mode on rx side
   ovn: integrate bfd for static routes

  NEWS                        |   3 +
  controller/ovn-controller.c |   1 +
  controller/pinctrl.c        | 642 +++++++++++++++++++++++++++++++++++-
  controller/pinctrl.h        |   2 +
  include/ovn/actions.h       |   7 +
  lib/actions.c               |  27 ++
  lib/ovn-l7.h                |  19 ++
  northd/ovn-northd.8.xml     |  21 ++
  northd/ovn-northd.c         | 315 +++++++++++++++++-
  ovn-nb.ovsschema            |  28 +-
  ovn-nb.xml                  |  73 ++++
  ovn-sb.ovsschema            |  27 +-
  ovn-sb.xml                  |  78 +++++
  tests/atlocal.in            |   3 +
  tests/ovn-nbctl.at          |   8 +-
  tests/ovn-northd.at         |  43 +++
  tests/ovn.at                |   4 +
  tests/system-ovn.at         | 126 +++++++
  utilities/ovn-trace.c       |   2 +
  19 files changed, 1407 insertions(+), 22 deletions(-)


_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to