On Wed, Feb 26, 2025 at 01:43:49PM +0100, Dumitru Ceara wrote: > On 2/26/25 1:31 PM, Frode Nordahl wrote: > > On Wed, Feb 26, 2025 at 11:59 AM Dumitru Ceara <dce...@redhat.com> wrote: > >> > >> On 2/26/25 11:48 AM, Felix Huettner wrote: > >>> On Wed, Feb 26, 2025 at 11:43:26AM +0100, Dumitru Ceara wrote: > >>>> On 2/26/25 11:40 AM, Felix Huettner wrote: > >>>>> On Tue, Feb 11, 2025 at 03:38:48PM +0100, Dumitru Ceara wrote: > >>>>>> On 2/11/25 9:36 AM, Felix Huettner via dev wrote: > >>>>>>> From: Frode Nordahl <fnord...@ubuntu.com> > >>>>>>> > >>>>>>> Introduce route-exchange-netlink module which implements interface > >>>>>>> for maintaining VRFs [0] and routes through Netlink. > >>>>>>> > >>>>>>> There is a desire to do this without having to (re-)implement > >>>>>>> routing protocol state machines in OVN, and to accomplish this we > >>>>>>> make use of Netlink. > >>>>>>> > >>>>>>> Netlink was chosen because: > >>>>>>> * Its ubiquitous nature with availability on any Linux system as > >>>>>>> as well other platforms. > >>>>>>> * Presence of a very good Netlink library implementation in our > >>>>>>> sibling project and library, Open vSwitch. > >>>>>>> * Popular routing protocol software conveniently already have > >>>>>>> support for redistributing routes to/from Netlink. > >>>>>>> * Support for interacting with Virtual Routing and Forwarding > >>>>>>> domains [0], allowing full isolation between virtual network > >>>>>>> resources defined within OVN and the hosting system while > >>>>>>> retaining access to all system network interfaces. > >>>>>>> > >>>>>>> It is important to note that the purpose of this integration is > >>>>>>> generic exchange of control plane information, while allowing to > >>>>>>> keep the datapath in OVS/OVN, enabling users to leverage its full > >>>>>>> range of user-, kernel- and mixed- space datapath implementations. > >>>>>>> > >>>>>>> 0: https://docs.kernel.org/networking/vrf.html > >>>>>>> > >>>>>>> Acked-by: Dumitru Ceara <dce...@redhat.com> > >>>>>>> Co-Authored-by: Felix Huettner <felix.huettner@stackit.cloud> > >>>>>>> Signed-off-by: Felix Huettner <felix.huettner@stackit.cloud> > >>>>>>> Signed-off-by: Frode Nordahl <fnord...@ubuntu.com> > >>>>>>> --- > >>>>>> > >>>>>> Applied to main, thanks! > >>>>>> > >>>>> > >>>>> Hi everyone, > >>>>> > >>>>> regarding: > >>>>> ``` > >>>>> /* This value is arbitrary but currently unused. > >>>>> * See the kernel rtnetlink UAPI at > >>>>> * > >>>>> https://github.com/torvalds/linux/blob/master/include/uapi/linux/rtnetlink.h > >>>>> * */ > >>>>> #define RTPROT_OVN 84 > >>>>> ``` > >>>>> > >>>>> I just wanted to share that this value has been now claimed upstream > >>>>> and is > >>>>> now part of the net-next tree: > >>>>> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=6002850fdfe0b4343136670a9895b6ba4ee3285b > >>>>> > >>>> > >>>> Awesome! Should we change the comment in OVN too? > > > > That is indeed great news, thank you for sorting that, Felix! > > > >>> We can do that. > >>> > >>> Should we keep the #define as it is right now, or should we wrap in in a > >>> #ifndef? > >>> > >>> My feeling is the ifndef, but i am not sure. > >>> > >> > >> ifndef sounds good to me. Would it make sense to also add a build time > >> assertion? E.g.: > >> > >> BUILD_ASSERT_DECL(RTPROT_OVN == 84); > >> > >> Or is that too much? > > > > I see we have that other places in the code base, we also have direct > > references to the number 84 in the test code, so I guess there is > > value in explicitly stating that we expect this specific value for the > > definition. > > > > > > One other thing on my mind is that it will take some time before this > > new header is available in all build environments, so I think it would > > be worth forward declaring it to avoid breaking workflow for people. > > > > Open vSwitch has precedence in handling these types of situations, and > > an example can be viewed in > > https://github.com/openvswitch/ovs/blob/main/include/linux/netlink.h > > > > So it would be great if we could do something similar for RTPROT_OVN. > > > > Ack. > > > If this should live in OVS or OVN I'm not sure, but there is an angle > > to update OVS if we add support for expanding RTPROT_OVN to a string > > in https://github.com/openvswitch/ovs/blob/main/tests/test-lib-route-table.c > > > > Good point. This might make more sense to have as part of OVS. On the > downside though we'd have to make sure that change is backported or > we'll have to release v25.03 with an OVS submodule pointing to the OVS > main branch. That is, if we want to change the OVN definition before we > release 25.03. > > However, keeping OVN 25.03 as is is not a real problem either IMO.
I am currently building a patch for all of this. I think keeping the #define in OVN 25.03 should be fine. However there is also a patch to iproute2 which adds the rtprot definition there as well [1]. This will cause the dynamic-routing tests to break once someone has that newer version, since then "proto 84" will be replaced by "proto ovn". I'll add a patch to support both versions in the tests as well. That might be the only thing we would want to also apply to 25.03. I'll keep it separate from the rest, so that it is easy to backport. [1]: https://patchwork.kernel.org/project/netdevbpf/patch/z77szialjemsn...@sit-sdelap4051.int.lidl.net/ Thanks a lot, Felix > > Regards, > Dumitru > > > -- > > Frode Nordahl > > > >> Thanks, > >> Dumitru > >> > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
