On Tue, Feb 04, 2025 at 12:21:53AM +0100, Lorenzo Bianconi wrote:
> On Jan 21, Felix Huettner via dev wrote:
> > This allows the ovn-controller to later find all ports that
> > participate in dynamic routing.
> >
> > Signed-off-by: Felix Huettner <[email protected]>
> > ---
> > v2->v3:
> > * A lot of minor review comments.
> > * Added more documentation and news
> >
> > NEWS | 8 ++++++++
> > northd/northd.c | 21 +++++++++++++++++++++
> > ovn-nb.xml | 42 ++++++++++++++++++++++++++++++++++++++++++
> > tests/ovn-northd.at | 32 ++++++++++++++++++++++++++++++++
> > 4 files changed, 103 insertions(+)
> >
> > diff --git a/NEWS b/NEWS
> > index 289cdd7f7..14390a564 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -43,6 +43,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
>
> %s/iterfaces/interfaces/
Hi Lorenzo,
thanks for the review. Unfortunately i saw the mail only after sending
out v6. I'll include it in a v7 once other reviews are there.
>
> > + routes for this LRP.
> >
> > OVN v24.09.0 - 13 Sep 2024
> > --------------------------
> > diff --git a/northd/northd.c b/northd/northd.c
> > index c80049bfd..4d08cf850 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -4065,6 +4065,27 @@ sync_pb_for_lrp(struct ovn_port *op,
> > }
> > }
> >
> > + if (is_cr_port(op) || chassis_name) {
> > + if (op->od->dynamic_routing) {
>
> nit: in order to reduce indentation I guess you could do something like:
>
> if ((is_cr_port(op) || chassis_name) && op->od->dynamic_routing) {
> ...
> }
thats a lot nicer.
Thanks a lot,
Felix
>
> Regards,
> Lorenzo
>
> > + 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->nbrp->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 4d4105a21..793a4f6bc 100644
> > --- a/ovn-nb.xml
> > +++ b/ovn-nb.xml
> > @@ -3777,6 +3777,48 @@ 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"/> is
> > set
> > + the vrf will be named "ovnvrf" with the datapath id of the Logical
> > + Router appended to it.
> > + </column>
> > +
> > + <column name="options" key="dynamic-routing-vrf-name"
> > + 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>.
> > +
> > + 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>
> > +
> > + <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 fa664599a..e949eef6e 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -14741,6 +14741,38 @@ 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-connected=true \
> > + option:dynamic-routing-static=true
> > +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 --wait=sb set Logical_Router_Port lr0-sw0
> > options:dynamic-routing-maintain-vrf=true \
> > +
> > options:dynamic-routing-vrf-name=myvrf \
> > +
> > 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])
> > --
> > 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
