| Message ID | 0236ff31f79abbf95b07a6a247d42fcfc348cad3.1737472971.git.felix.huettner@stackit.cloud |
|---|---|
| State | Changes Requested |
| Delegated to: | Dumitru Ceara |
| Headers | show |
| Series | OVN Fabric integration: Northd. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On Tue, Jan 21, 2025 at 04:47:40PM +0100, 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 <felix.huettner@stackit.cloud> > --- > 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 > + 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) { > + 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> Hi everyone, Just wanted to share that i am currently thinking about if this should rather be a setting on the Logical_Router. Otherwise a chassis with multiple LRPs bound locally might have different vrfs for the same datapath. What are your opinions? Thanks a lot Felix > + > + <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 > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On 1/22/25 11:42 AM, Felix Huettner via dev wrote: > On Tue, Jan 21, 2025 at 04:47:40PM +0100, 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 <felix.huettner@stackit.cloud> >> --- >> 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 >> + 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) { >> + 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> > > Hi everyone, > Hi Felix, > Just wanted to share that i am currently thinking about if this should > rather be a setting on the Logical_Router. Otherwise a chassis with > multiple LRPs bound locally might have different vrfs for the same > datapath. > > What are your opinions? > If I understand correctly, with the current implementation, if dynamic-routing-vrf-name is _not_ set then we anyway program routes in a single VRF for all LRPs (vrf name will be "ovnvrf-<datapath-id>"). In my opinion it makes sense that the dynamic-routing-vrf-name is configured for the whole router. Regards, Dumitru > Thanks a lot > Felix > >> + >> + <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 >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Wed, Jan 22, 2025 at 11:50:37AM +0100, Dumitru Ceara wrote: > On 1/22/25 11:42 AM, Felix Huettner via dev wrote: > > On Tue, Jan 21, 2025 at 04:47:40PM +0100, 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 <felix.huettner@stackit.cloud> > >> --- > >> 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 > >> + 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) { > >> + 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> > > > > Hi everyone, > > > > Hi Felix, > > > Just wanted to share that i am currently thinking about if this should > > rather be a setting on the Logical_Router. Otherwise a chassis with > > multiple LRPs bound locally might have different vrfs for the same > > datapath. > > > > What are your opinions? > > > > If I understand correctly, with the current implementation, if > dynamic-routing-vrf-name is _not_ set then we anyway program routes in a > single VRF for all LRPs (vrf name will be "ovnvrf-<datapath-id>"). > > In my opinion it makes sense that the dynamic-routing-vrf-name is > configured for the whole router. I just saw that we anyway use the datapath id as the vrf ID. So as long as that is the case we anyway need to scope this to a per Logical_Router basis. I would update that in the next version. But i would wait for reviews of all of these patches. > > Regards, > Dumitru > > > Thanks a lot > > Felix > > > >> + > >> + <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 > >> dev@openvswitch.org > >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > >
On 1/22/25 12:22 PM, Felix Huettner wrote: > On Wed, Jan 22, 2025 at 11:50:37AM +0100, Dumitru Ceara wrote: >> On 1/22/25 11:42 AM, Felix Huettner via dev wrote: >>> On Tue, Jan 21, 2025 at 04:47:40PM +0100, 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 <felix.huettner@stackit.cloud> >>>> --- >>>> 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 >>>> + 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) { >>>> + 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> >>> >>> Hi everyone, >>> >> >> Hi Felix, >> >>> Just wanted to share that i am currently thinking about if this should >>> rather be a setting on the Logical_Router. Otherwise a chassis with >>> multiple LRPs bound locally might have different vrfs for the same >>> datapath. >>> >>> What are your opinions? >>> >> >> If I understand correctly, with the current implementation, if >> dynamic-routing-vrf-name is _not_ set then we anyway program routes in a >> single VRF for all LRPs (vrf name will be "ovnvrf-<datapath-id>"). >> >> In my opinion it makes sense that the dynamic-routing-vrf-name is >> configured for the whole router. > > I just saw that we anyway use the datapath id as the vrf ID. So as long > as that is the case we anyway need to scope this to a per Logical_Router > basis. > > I would update that in the next version. But i would wait for reviews of > all of these patches. > Sounds good. I do plan to look at this version in the coming days. >> >> Regards, >> Dumitru >> >>> Thanks a lot >>> Felix >>> >>>> + >>>> + <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 >>>> dev@openvswitch.org >>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>> _______________________________________________ >>> dev mailing list >>> dev@openvswitch.org >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>> >> >
Hi Felix, After having a closer look at this patch I only have a few tiny comments for v5. On 1/22/25 1:36 PM, Dumitru Ceara wrote: > On 1/22/25 12:22 PM, Felix Huettner wrote: >> On Wed, Jan 22, 2025 at 11:50:37AM +0100, Dumitru Ceara wrote: >>> On 1/22/25 11:42 AM, Felix Huettner via dev wrote: >>>> On Tue, Jan 21, 2025 at 04:47:40PM +0100, 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 <felix.huettner@stackit.cloud> >>>>> --- >>>>> 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 >>>>> + 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) { >>>>> + 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); >>>>> + } >>>>> + } >>>>> + } >>>>> + >>>>> + Nit: no need for newline. >>>>> 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> >>>> >>>> Hi everyone, >>>> >>> >>> Hi Felix, >>> >>>> Just wanted to share that i am currently thinking about if this should >>>> rather be a setting on the Logical_Router. Otherwise a chassis with >>>> multiple LRPs bound locally might have different vrfs for the same >>>> datapath. >>>> >>>> What are your opinions? >>>> >>> >>> If I understand correctly, with the current implementation, if >>> dynamic-routing-vrf-name is _not_ set then we anyway program routes in a >>> single VRF for all LRPs (vrf name will be "ovnvrf-<datapath-id>"). >>> >>> In my opinion it makes sense that the dynamic-routing-vrf-name is >>> configured for the whole router. >> >> I just saw that we anyway use the datapath id as the vrf ID. So as long >> as that is the case we anyway need to scope this to a per Logical_Router >> basis. >> >> I would update that in the next version. But i would wait for reviews of >> all of these patches. >> > > Sounds good. I do plan to look at this version in the coming days. > >>> >>> Regards, >>> Dumitru >>> >>>> Thanks a lot >>>> Felix >>>> >>>>> + >>>>> + <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']) Nit: Missing space before grep. >>>>> + >>>>> +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']) Nit: Here too. >>>>> + >>>>> +AT_CLEANUP >>>>> +]) >>>>> + >>>>> >>>>> OVN_FOR_EACH_NORTHD_NO_HV([ >>>>> AT_SETUP([dynamic-routing incremental processing]) >>>>> -- >>>>> 2.47.1 >>>>> Thanks, Dumitru
On Fri, Jan 31, 2025 at 01:17:38PM +0100, Dumitru Ceara wrote: > Hi Felix, > > After having a closer look at this patch I only have a few tiny comments > for v5. Hi Dumitru, thanks for the review. All things will be addressed in the next version. Thanks a lot, Felix > > On 1/22/25 1:36 PM, Dumitru Ceara wrote: > > On 1/22/25 12:22 PM, Felix Huettner wrote: > >> On Wed, Jan 22, 2025 at 11:50:37AM +0100, Dumitru Ceara wrote: > >>> On 1/22/25 11:42 AM, Felix Huettner via dev wrote: > >>>> On Tue, Jan 21, 2025 at 04:47:40PM +0100, 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 <felix.huettner@stackit.cloud> > >>>>> --- > >>>>> 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 > >>>>> + 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) { > >>>>> + 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); > >>>>> + } > >>>>> + } > >>>>> + } > >>>>> + > >>>>> + > > Nit: no need for newline. > > >>>>> 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> > >>>> > >>>> Hi everyone, > >>>> > >>> > >>> Hi Felix, > >>> > >>>> Just wanted to share that i am currently thinking about if this should > >>>> rather be a setting on the Logical_Router. Otherwise a chassis with > >>>> multiple LRPs bound locally might have different vrfs for the same > >>>> datapath. > >>>> > >>>> What are your opinions? > >>>> > >>> > >>> If I understand correctly, with the current implementation, if > >>> dynamic-routing-vrf-name is _not_ set then we anyway program routes in a > >>> single VRF for all LRPs (vrf name will be "ovnvrf-<datapath-id>"). > >>> > >>> In my opinion it makes sense that the dynamic-routing-vrf-name is > >>> configured for the whole router. > >> > >> I just saw that we anyway use the datapath id as the vrf ID. So as long > >> as that is the case we anyway need to scope this to a per Logical_Router > >> basis. > >> > >> I would update that in the next version. But i would wait for reviews of > >> all of these patches. > >> > > > > Sounds good. I do plan to look at this version in the coming days. > > > >>> > >>> Regards, > >>> Dumitru > >>> > >>>> Thanks a lot > >>>> Felix > >>>> > >>>>> + > >>>>> + <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']) > > Nit: Missing space before grep. > > >>>>> + > >>>>> +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']) > > Nit: Here too. > > >>>>> + > >>>>> +AT_CLEANUP > >>>>> +]) > >>>>> + > >>>>> > >>>>> OVN_FOR_EACH_NORTHD_NO_HV([ > >>>>> AT_SETUP([dynamic-routing incremental processing]) > >>>>> -- > >>>>> 2.47.1 > >>>>> > > Thanks, > Dumitru >
On Jan 22, Dumitru Ceara wrote: > On 1/22/25 11:42 AM, Felix Huettner via dev wrote: > > On Tue, Jan 21, 2025 at 04:47:40PM +0100, 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 <felix.huettner@stackit.cloud> > >> --- > >> 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 > >> + 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) { > >> + 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> > > > > Hi everyone, > > > > Hi Felix, > > > Just wanted to share that i am currently thinking about if this should > > rather be a setting on the Logical_Router. Otherwise a chassis with > > multiple LRPs bound locally might have different vrfs for the same > > datapath. > > > > What are your opinions? > > > > If I understand correctly, with the current implementation, if > dynamic-routing-vrf-name is _not_ set then we anyway program routes in a > single VRF for all LRPs (vrf name will be "ovnvrf-<datapath-id>"). > > In my opinion it makes sense that the dynamic-routing-vrf-name is > configured for the whole router. +1 Regards, Lorenzo > > Regards, > Dumitru > > > Thanks a lot > > Felix > > > >> + > >> + <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 > >> dev@openvswitch.org > >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
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 <felix.huettner@stackit.cloud> > --- > 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/ > + 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) { ... } 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 > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
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 <felix.huettner@stackit.cloud> > > --- > > 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 > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
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 + 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) { + 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])
This allows the ovn-controller to later find all ports that participate in dynamic routing. Signed-off-by: Felix Huettner <felix.huettner@stackit.cloud> --- 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(+)