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/
>
>>
>> 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.
>>
>> 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 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
