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 <[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
> >> +       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
> >> [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

Reply via email to