On Thu, Feb 6, 2025 at 10:57 AM Dumitru Ceara <[email protected]> wrote:
>
> On 2/6/25 10:43 AM, Felix Huettner via dev wrote:
> > On Thu, Feb 06, 2025 at 09:26:26AM +0100, Frode Nordahl wrote:
> >> On Tue, Feb 4, 2025 at 3:01 PM Felix Huettner via dev
> >> <[email protected]> wrote:
> >>>
> >>> This allows the ovn-controller to later find all ports that
> >>> participate in dynamic routing.
> >>>
> >>> Signed-off-by: Felix Huettner <[email protected]>
> >>> ---
> >>> v5->v6:
> >>> * addressed review comments
> >>> v4->v5: skipped
> >>> v2->v3:
> >>> * A lot of minor review comments.
> >>> * Added more documentation and news
> >>>
> >>> NEWS | 8 ++++++++
> >>> northd/northd.c | 20 ++++++++++++++++++++
> >>> ovn-nb.xml | 41 +++++++++++++++++++++++++++++++++++++++++
> >>> tests/ovn-northd.at | 31 +++++++++++++++++++++++++++++++
> >>> 4 files changed, 100 insertions(+)
> >>>
> >>> diff --git a/NEWS b/NEWS
> >>> index 3547f659f..78245e1b4 100644
> >>> --- a/NEWS
> >>> +++ b/NEWS
> >>> @@ -53,6 +53,14 @@ Post v24.09.0
> >>> * Add the option "dynamic-routing-connected-as-host-routes" to
> >>> LRPs. If
> >>> set to true then connected routes are announced as individual host
> >>> routes.
> >>> + * Add the option "dynamic-routing-maintain-vrf" to LRPs. If set the
> >>> + ovn-controller will create a vrf named "ovnvrf" + datapath id that
> >>> + includes all advertised and learned routes.
> >>> + The vrf name can be overwritten with the
> >>> "dynamic-routing-vrf-name"
> >>> + setting.
> >>> + * Add the option "dynamic-routing-ifname" to LRPs. If set only
> >>> routes
> >>> + learned from a linux iterfaces with that name are treated as
> >>> relevant
> >>> + routes for this LRP.
> >>>
> >>> OVN v24.09.0 - 13 Sep 2024
> >>> --------------------------
> >>> diff --git a/northd/northd.c b/northd/northd.c
> >>> index 3f14cc75c..573d3cb24 100644
> >>> --- a/northd/northd.c
> >>> +++ b/northd/northd.c
> >>> @@ -4111,6 +4111,26 @@ sync_pb_for_lrp(struct ovn_port *op,
> >>> }
> >>> }
> >>>
> >>> + if (is_cr_port(op) || chassis_name) {
> >>> + if (op->od->dynamic_routing) {
> >>> + smap_add(&new, "dynamic-routing", "true");
> >>> + if (smap_get_bool(&op->nbrp->options,
> >>> + "dynamic-routing-maintain-vrf", false)) {
> >>> + smap_add(&new, "dynamic-routing-maintain-vrf", "true");
> >>> + }
> >>> + const char *vrfname = smap_get(&op->od->nbr->options,
> >>> + "dynamic-routing-vrf-name");
> >>> + if (vrfname) {
> >>> + smap_add(&new, "dynamic-routing-vrf-name", vrfname);
> >>> + }
> >>> + const char *ifname = smap_get(&op->nbrp->options,
> >>> + "dynamic-routing-ifname");
> >>> + if (ifname) {
> >>> + smap_add(&new, "dynamic-routing-ifname", ifname);
> >>> + }
> >>> + }
> >>> + }
> >>> +
> >>> const char *ipv6_pd_list = smap_get(&op->sb->options,
> >>> "ipv6_ra_pd_list");
> >>> if (ipv6_pd_list) {
> >>> smap_add(&new, "ipv6_ra_pd_list", ipv6_pd_list);
> >>> diff --git a/ovn-nb.xml b/ovn-nb.xml
> >>> index d0bdb5058..cf417884e 100644
> >>> --- a/ovn-nb.xml
> >>> +++ b/ovn-nb.xml
> >>> @@ -3005,6 +3005,16 @@ or
> >>> table="Logical_Router_Port"/>.
> >>> >>>>>>> 20721b41b (northd: Add filtering which routes to advertise.)
> >>> </column>
> >>> +
> >>> + <column name="options" key="dynamic-routing-vrf-name"
> >>> + type='{"type": "string"}'>
> >>> + Only relevant if <ref column="options" key="dynamic-routing"/>
> >>> + is set to <code>true</code>.
> >>> +
> >>> + This defines the name of the vrf the ovn-controller will use to
> >>> + advertise and learn routes. If not set the vrf will be named
> >>> "ovnvrf"
> >>> + with the datapath id of the Logical Router appended to it.
> >>> + </column>
> >>> </group>
> >>>
> >>> <group title="Common Columns">
> >>> @@ -3779,6 +3789,37 @@ or
> >>> </li>
> >>> </ul>
> >>> </column>
> >>> +
> >>> + <column name="options" key="dynamic-routing-maintain-vrf"
> >>> + type='{"type": "boolean"}'>
> >>> + Only relevant if <ref column="options" key="dynamic-routing"
> >>> + table="Logical_Router"/> on the respective Logical_Router is set
> >>> + to <code>true</code>.
> >>> +
> >>> + If this LRP is bound to a specific chassis then the
> >>> ovn-controller of
> >>> + this chassis will maintain a vrf.
> >>> + This vrf will contain all the routes that should be announced
> >>> from
> >>> + this LRP.
> >>> + Unless <ref column="options" key="dynamic-routing-vrf-name"
> >>> + table="Logical_Router"/> is set the vrf will be named "ovnvrf"
> >>> with
> >>> + the datapath id of the Logical Router appended to it.
> >
> > Hi Frode,
> >
> >>
> >> IIUIC from reading the code in patch 17, this name is in reality a
> >> prefix, as it will still be suffixed by the LR data path ID, and the
> >> routing table number will also come from the LR data path ID.
> >
> > that should not be the case. We only fall back to the old name if the
> > vrf name is too long.
> > I will add this information to the docs.
> >
> > https://patchwork.ozlabs.org/project/ovn/patch/a1b6028cc4f7b76174b055a31152492cdebee287.1738676767.git.felix.huettner@stackit.cloud/
Ah, thank you for clarifying.
> >>
> >> Both the prefix and the routing table ID have downstream consequences
> >> for configuration in other systems, such as the routing protocol
> >> daemon.
> >>
> >> Adding Vladislav to Cc as I remember he pointed out that they would
> >> likely want to maintain the VRF with their deployment tooling and I
> >> expect they would want to then also control the routing table number,
> >> not just the VRF prefix.
> >
> > Would it be ok for everyone if we do that in a subsequent patch after we
> > are done with this series?
> > I will be absent in a few hours for the rest of the week, so i would
> > like to send out v7 without any larger changes before that.
> >
>
> If Frode agrees, this sounds reasonable to me. I'm not sure whether a
> new knob to configure the table-id would be acceptable or not as part of
> the 25.03 release. It mainly depends on the amount of code that implies
> I guess.
Yes, with updates to documentation it's fine.
> >>
> >> Would it perhaps make more sense to have options named
> >> dynamic-routing-vrf-prefix and dynamic-routing-vrf-table-id?
> >>
> >> In either case I think the details above deserve mention in the
> >> documentation and the name/id options deserve their own paragraph.
> >
> > Documentation will be updated with all these changes.
Thanks! Make sure to put emphasis on the fact that route table ID
still comes from data path ID, because people might otherwise be
surprised why setting the name to 'myvrf42' gives routes in table 51
;)
--
Frode Nordahl
> > Thanks a lot,
> > Felix
> >
>
> Regards,
> Dumitru
>
> >>
> >> --
> >> Frode Nordahl
> >>
> >>> + </column>
> >>> +
> >>> + <column name="options" key="dynamic-routing-ifname"
> >>> + type='{"type": "string"}'>
> >>> + Only relevant if <ref column="options" key="dynamic-routing"
> >>> + table="Logical_Router"/> on the respective Logical_Router is set
> >>> + to <code>true</code>.
> >>> +
> >>> + Only learn routes associated with the interface specified here.
> >>> + This allows a single chassis to learn different routes on
> >>> separate
> >>> + LRPs bound to this chassis.
> >>> +
> >>> + This is usefully e.g. in the case of a chassis with multiple
> >>> links
> >>> + towards the network fabric where all of them run BGP
> >>> individually.
> >>> + This option allows to have a 1:1 mapping between a single LRP
> >>> and an
> >>> + individual link.
> >>> + </column>
> >>> </group>
> >>>
> >>> <group title="Attachment">
> >>> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> >>> index 18d367fa5..873e5c45e 100644
> >>> --- a/tests/ovn-northd.at
> >>> +++ b/tests/ovn-northd.at
> >>> @@ -14905,6 +14905,37 @@ check_row_count Advertised_Route 1
> >>> datapath=$datapath logical_port=$sw0 ip_prefi
> >>> AT_CLEANUP
> >>> ])
> >>>
> >>> +OVN_FOR_EACH_NORTHD_NO_HV([
> >>> +AT_SETUP([dynamic-routing - lrp options])
> >>> +AT_KEYWORDS([dynamic-routing])
> >>> +ovn_start
> >>> +
> >>> +check ovn-nbctl lr-add lr0
> >>> +check ovn-nbctl set Logical_Router lr0 option:dynamic-routing=true \
> >>> +
> >>> option:dynamic-routing-redistribute="connected;static"
> >>> +check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24
> >>> +check ovn-nbctl set Logical_Router lr0 options:chassis=hv1
> >>> +check ovn-nbctl ls-add sw0
> >>> +check ovn-nbctl lsp-add sw0 sw0-lr0
> >>> +check ovn-nbctl --wait=sb set Logical_Switch_Port sw0-lr0 type=router
> >>> options:router-port=lr0-sw0
> >>> +
> >>> +AT_CHECK([fetch_column sb:Port_Binding options logical_port=lr0-sw0 |
> >>> grep -q 'dynamic-routing=true'])
> >>> +AT_CHECK([fetch_column sb:Port_Binding options logical_port=lr0-sw0 |
> >>> grep -qv 'dynamic-routing-maintain-vrf'])
> >>> +AT_CHECK([fetch_column sb:Port_Binding options logical_port=lr0-sw0 |
> >>> grep -qv 'dynamic-routing-vrf-name'])
> >>> +AT_CHECK([fetch_column sb:Port_Binding options logical_port=lr0-sw0 |
> >>> grep -qv 'dynamic-routing-ifname'])
> >>> +
> >>> +check ovn-nbctl set Logical_Router lr0
> >>> options:dynamic-routing-vrf-name=myvrf
> >>> +check ovn-nbctl --wait=sb set Logical_Router_Port lr0-sw0
> >>> options:dynamic-routing-maintain-vrf=true \
> >>> +
> >>> options:dynamic-routing-ifname=myif
> >>> +
> >>> +AT_CHECK([fetch_column sb:Port_Binding options logical_port=lr0-sw0 |
> >>> grep -q 'dynamic-routing=true'])
> >>> +AT_CHECK([fetch_column sb:Port_Binding options logical_port=lr0-sw0 |
> >>> grep -q 'dynamic-routing-maintain-vrf=true'])
> >>> +AT_CHECK([fetch_column sb:Port_Binding options logical_port=lr0-sw0 |
> >>> grep -q 'dynamic-routing-vrf-name=myvrf'])
> >>> +AT_CHECK([fetch_column sb:Port_Binding options logical_port=lr0-sw0 |
> >>> grep -q 'dynamic-routing-ifname=myif'])
> >>> +
> >>> +AT_CLEANUP
> >>> +])
> >>> +
> >>> OVN_FOR_EACH_NORTHD_NO_HV([
> >>> AT_SETUP([dynamic-routing incremental processing])
> >>> AT_KEYWORDS([dynamic-routing])
> >>> --
> >>> 2.47.1
> >>>
> >>>
> >>> _______________________________________________
> >>> dev mailing list
> >>> [email protected]
> >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > _______________________________________________
> > dev mailing list
> > [email protected]
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
