On Tue, Aug 19, 2025 at 5:15 PM Mark Michelson <mmich...@redhat.com> wrote:
>
> Hi Numan, I have not done an in-depth look through all the patches, but
> I did skim them, and I understand what's being done here. As the cover
> letter states, this provides a way to program OVS bridges using OVN
> logical flow language instead.
>
> It's clear looking at the code that ovn-bridge-controller is expected to
> be deployed on the same hypervisor where the OVS bridge is. When you
> picture this being deployed, do you see a single centralized ovn-br
> database that many ovn-br-controllers connect to, or are you picturing
> this more like each ovn-br-controller will have an independent instance
> of the ovn-br database that it interacts with? I think that an addition
> to the ovn-architecture document is useful with this series, since it
> will make it more clear how these pieces are meant to fit into the
> overall OVN picture.
>
Thanks Mark for taking a look at this and providing feedback.
I see the latter case, i.e ovn-br database running on each chassis and
ovn-br-controller connecting to it.
I'd expect a CMS agent running on each chassis and programming the
logical flows in the local ovn-br database.
Sounds good, I'll add a section in the ovn-architecture in the next version.
> Other than that, I like what I'm seeing. The code lives as a separate
> application from existing OVN applications, so there is very little risk
> adding this to the code base. It also duplicates code as little as
> possible (I think patches 6 and 9 are the biggest places where I see
> duplication of code already in ovn-controller). I think what you've
> presented here introduces the feature in a complete way and also leaves
> room for future optimizations/improvements.
>
> I'll give this a more in-depth review in the upcoming days and provide
> critiques on the individual patches.
Looking forward to it.
Numan
>
> On 8/11/25 6:09 AM, num...@ovn.org wrote:
> > From: Numan Siddique <num...@ovn.org>
> >
> > This patch series adds a new service OVN Bridge Controller
> > (ovn-br-controller) which can be used to program OpenFlow rules to
> > OVS bridges using OVN logical flows.
> >
> > The syntax and symantics of the logical flows is similar to the logical
> > flows generated by ovn-northd.
> >
> > Rational for adding this service:
> > There's a need to configure the provider bridges with specific OpenFlow
> > rules after packets leave the OVN pipeline and enters these provider
> > bridges via the patch ports. Since OVN has a very good abstraction
> > of the OpenFlow rules using OVN logical flows, we can leverage the same
> > here so that CMS doesn't have to deal with the specifics of OpenFlow
> > protocol. Also if the CMS is written in golang or other languages,
> > CMS has to mostly rely on ovs-vsctl/ovs-ofctl to program the flows.
> > Adding this support in OVN avoids the CMS to exec these utils to
> > add/delete and dump the existing OpenFlow rules.
> >
> > A user can also use this new service to program and manage any OVS
> > bridge without using OVN and hence this service is named as
> > "ovn-br-controller."
> >
> > Numan Siddique (12):
> > Add initial schema and skeleton for OVN bridge controller.
> > ovn-br-controller: Connect to OVS database and define engine nodes.
> > ovn-br-controller: Build en_bridge_data engine node.
> > ovn-br-controller: Add bridge flow table manager.
> > Move lflow-conj-ids module to lib.
> > ovn-br-controller: Process logical flows.
> > ovn-br-controller: Add en_pflow_output engine.
> > Move ofctrl-seqno module to lib.
> > ovn-br-controller: Program the openflows to the bridges.
> > ovn-br-controller: Extend the symbol table to add new fields.
> > ovn-ctl: Add commands to start OVN bridge controller services.
> > rhel: Add ovn-br-controller sub package.
> >
> > Makefile.am | 5 +-
> > automake.mk | 36 +
> > br-controller/automake.mk | 18 +
> > br-controller/br-flow-mgr.c | 1302 +++++++++++++++++
> > br-controller/br-flow-mgr.h | 65 +
> > br-controller/br-ofctrl.c | 731 +++++++++
> > br-controller/br-ofctrl.h | 33 +
> > br-controller/en-bridge-data.c | 199 +++
> > br-controller/en-bridge-data.h | 43 +
> > br-controller/en-lflow.c | 364 +++++
> > br-controller/en-lflow.h | 25 +
> > br-controller/en-pflow.c | 192 +++
> > br-controller/en-pflow.h | 16 +
> > br-controller/ovn-br-controller.8.xml | 5 +
> > br-controller/ovn-br-controller.c | 560 +++++++
> > controller/automake.mk | 4 -
> > controller/if-status.c | 2 +-
> > controller/lflow.h | 2 +-
> > controller/ovn-controller.c | 4 +-
> > lib/automake.mk | 21 +-
> > lib/inc-proc-eng.h | 11 +
> > {controller => lib}/lflow-conj-ids.c | 0
> > {controller => lib}/lflow-conj-ids.h | 2 +-
> > {controller => lib}/ofctrl-seqno.c | 0
> > {controller => lib}/ofctrl-seqno.h | 0
> > lib/ovn-br-idl.ann | 9 +
> > lib/ovn-util.c | 13 +
> > lib/ovn-util.h | 1 +
> > {controller => lib}/test-lflow-conj-ids.c | 0
> > {controller => lib}/test-ofctrl-seqno.c | 0
> > ovn-br.ovsschema | 94 ++
> > ovn-br.xml | 452 ++++++
> > rhel/automake.mk | 4 +-
> > rhel/ovn-fedora.spec.in | 22 +-
> > ...b_systemd_system_ovn-br-controller.service | 35 +
> > rhel/usr_lib_systemd_system_ovn-br-db.service | 32 +
> > tests/automake.mk | 11 +-
> > tests/ovn-br-controller.at | 355 +++++
> > tests/testsuite.at | 1 +
> > tutorial/ovn-sandbox | 24 +-
> > utilities/automake.mk | 8 +
> > utilities/ovn-brctl.c | 585 ++++++++
> > utilities/ovn-ctl | 163 +++
> > utilities/ovn-ctl.8.xml | 36 +
> > 44 files changed, 5465 insertions(+), 20 deletions(-)
> > create mode 100644 br-controller/automake.mk
> > create mode 100644 br-controller/br-flow-mgr.c
> > create mode 100644 br-controller/br-flow-mgr.h
> > create mode 100644 br-controller/br-ofctrl.c
> > create mode 100644 br-controller/br-ofctrl.h
> > create mode 100644 br-controller/en-bridge-data.c
> > create mode 100644 br-controller/en-bridge-data.h
> > create mode 100644 br-controller/en-lflow.c
> > create mode 100644 br-controller/en-lflow.h
> > create mode 100644 br-controller/en-pflow.c
> > create mode 100644 br-controller/en-pflow.h
> > create mode 100644 br-controller/ovn-br-controller.8.xml
> > create mode 100644 br-controller/ovn-br-controller.c
> > rename {controller => lib}/lflow-conj-ids.c (100%)
> > rename {controller => lib}/lflow-conj-ids.h (98%)
> > rename {controller => lib}/ofctrl-seqno.c (100%)
> > rename {controller => lib}/ofctrl-seqno.h (100%)
> > create mode 100644 lib/ovn-br-idl.ann
> > rename {controller => lib}/test-lflow-conj-ids.c (100%)
> > rename {controller => lib}/test-ofctrl-seqno.c (100%)
> > create mode 100644 ovn-br.ovsschema
> > create mode 100644 ovn-br.xml
> > create mode 100644 rhel/usr_lib_systemd_system_ovn-br-controller.service
> > create mode 100644 rhel/usr_lib_systemd_system_ovn-br-db.service
> > create mode 100644 tests/ovn-br-controller.at
> > create mode 100644 utilities/ovn-brctl.c
> >
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
