On Tue, Feb 4, 2025 at 1:30 PM Lorenzo Bianconi <lorenzo.bianc...@redhat.com>
wrote:
> [...]
> >
> > /* If the router is for l3 gateway, it resides on a chassis
> > * and its port type is "l3gateway". */
> > - const char *chassis_name = smap_get(&op->od->nbr->options,
> "chassis");
> > + const char *lr_chassis = smap_get(&op->od->nbr->options,
> "chassis");
> > +
> > + /* If the LRP has requested chassis, that is remote, set the
> type to
> > + * remote and add the appropriate chassis. */
> > + const char *req_chassis = smap_get(&op->nbrp->options,
> > + "requested-chassis");
> > + const struct sbrec_chassis *sb_chassis = NULL;
> > + const char *type = "patch";
> > if (is_cr_port(op)) {
> > - sbrec_port_binding_set_type(op->sb, "chassisredirect");
> > - } else if (chassis_name) {
> > - sbrec_port_binding_set_type(op->sb, "l3gateway");
> > - } else {
> > - sbrec_port_binding_set_type(op->sb, "patch");
> > + type = "chassisredirect";
> > + } else if (lr_chassis) {
> > + type = "l3gateway";
> > + } else if (req_chassis) {
> > + const struct sbrec_chassis *tr_chassis = chassis_lookup(
> > + sbrec_chassis_by_name, sbrec_chassis_by_hostname,
> req_chassis);
> > + bool trp = tr_chassis &&
> smap_get_bool(&tr_chassis->other_config,
> > + "is-remote", false);
> > + sb_chassis = trp ? tr_chassis : NULL;
> > + type = trp ? "remote" : "patch";
> > + }
> > +
> > + sbrec_port_binding_set_type(op->sb, type);
> > + sbrec_port_binding_set_requested_chassis(op->sb, sb_chassis);
>
Hi Lorenzo,
thank you for the review.
> nit: is it ok to set sbrec_port_binding_set_requested_chassis() even if
> sb_chassis is NULL? (since you check it for
> sbrec_port_binding_set_chassis()).
>
It is, because this is the only use case within LRP for requested-chassis,
however it needs to be checked for chassis because we might otherwise
overwrite what was set by ovn-controller. Does that make sense?
>
> Regards,
> Lorenzo
>
>
Thanks,
Ales
> > + if (sb_chassis) {
> > + sbrec_port_binding_set_chassis(op->sb, sb_chassis);
> > }
> >
> > if (is_cr_port(op)) {
> > @@ -4167,6 +4185,14 @@ ovn_port_assign_requested_tnl_id(struct ovn_port
> *op)
> > return true;
> > }
> >
> > +static bool
> > +ovn_port_is_trp(struct ovn_port *op)
> > +{
> > + return op->nbrp &&
> > + op->sb->chassis &&
> > + smap_get_bool(&op->sb->chassis->other_config, "is-remote",
> false);
> > +}
> > +
> > static bool
> > ovn_port_allocate_key(struct ovn_port *op)
> > {
> > @@ -4272,6 +4298,7 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn,
> > sbrec_mirror_table,
> > op, queue_id_bitmap,
> > &active_ha_chassis_grps);
> > + op->od->is_transit_router |= ovn_port_is_trp(op);
> > ovs_list_remove(&op->list);
> > }
> >
> > @@ -4285,6 +4312,7 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn,
> > op, queue_id_bitmap,
> > &active_ha_chassis_grps);
> > sbrec_port_binding_set_logical_port(op->sb, op->key);
> > + op->od->is_transit_router |= ovn_port_is_trp(op);
> > ovs_list_remove(&op->list);
> > }
> >
> > diff --git a/northd/northd.h b/northd/northd.h
> > index 6bca6bb1a..0da05d518 100644
> > --- a/northd/northd.h
> > +++ b/northd/northd.h
> > @@ -356,6 +356,10 @@ struct ovn_datapath {
> > * If this is true, then 'l3dgw_ports' will be ignored. */
> > bool is_gw_router;
> >
> > + /* Indicates whether the router should be considered a transit
> router.
> > + * This is applicable only to routers with "remote" ports. */
> > + bool is_transit_router;
> > +
> > /* OVN northd only needs to know about logical router gateway ports
> for
> > * NAT/LB on a distributed router. The "distributed gateway ports"
> are
> > * populated only when there is a gateway chassis or ha chassis
> group
> > diff --git a/ovn-nb.xml b/ovn-nb.xml
> > index d82f9872b..07bd24fce 100644
> > --- a/ovn-nb.xml
> > +++ b/ovn-nb.xml
> > @@ -3104,6 +3104,32 @@ or
> > <column name="external_ids">
> > See <em>External IDs</em> at the beginning of this document.
> > </column>
> > +
> > + <group title="Transit router">
> > + <p>
> > + In order to achieve status of <code>Transit Router</code> for
> > + <ref table="Logical_Router"/> there needs to be at least one
> > + <ref table="Logical_Router_Port"/> that is considered remote.
> > + The LRP can be <code>remote</code> only if it has
> > + <code>options:requested-chassis</code> set to chassis that is
> > + considered remote. See <ref table="Logical_Router_Port"/> for
> more
> > + details.
> > + </p>
> > +
> > + <p>
> > + In order for the <code>Transit Router</code> to work properly
> all the
> > + tunnel keys for the <code>Transit Router</code> itself and the
> remote
> > + ports keys needs to match in all AZs e.g. TR in AZ1 and AZ2
> needs to
> > + have the same tunnel key. Remote port for AZ2 in AZ1 needs to
> have the
> > + same tunnel key as local port in AZ2 and vice vers.
> > + </p>
> > +
> > + <p>
> > + The <code>Transit Router</code> behaves as distributed router
> which
> > + means that it has the same limitations for stateful flows like
> > + <code>NAT and LBs</code> and it will lose the CT state between
> AZs.
> > + </p>
> > + </group>
> > </table>
> >
> > <table name="Meter" title="Meter entry">
> > @@ -3680,6 +3706,23 @@ or
> > learned by the <code>ovn-ic</code> daemon.
> > </p>
> > </column>
> > +
> > + <column name="options" key="requested-chassis">
> > + <p>
> > + If set, identifies a specific chassis (by name or hostname)
> that
> > + is allowed to bind this port. This option is valid only for
> chassis
> > + that have <code>options:is-remote=true</code>, in other words
> for
> > + chassis that are in different Availability zone. The option
> accepts
> > + only single value.
> > + </p>
> > +
> > + <p>
> > + By assigning remote chassis the <ref table="Logical_Router"/>
> gains
> > + status of <code>Transit Router</code> see
> > + <ref table="Logical_Router"/> table for more details.
> > + </p>
> > +
> > + </column>
> > </group>
> >
> > <group title="Attachment">
> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > index e64b5a660..40dcf338a 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -14560,3 +14560,54 @@ wait_row_count Multicast_Group 0 name=239.0.1.68
> > OVN_CLEANUP([hv1], [hv2])
> > AT_CLEANUP
> > ])
> > +
> > +OVN_FOR_EACH_NORTHD_NO_HV([
> > +AT_SETUP([Transit router - remote ports])
> > +ovn_start NORTHD_TYPE
> > +
> > +check ovn-sbctl chassis-add hv2 geneve 192.168.0.12 \
> > + -- set chassis hv2 other_config:is-remote=true
> > +
> > +check ovn-sbctl chassis-add hv3 geneve 192.168.0.13 \
> > + -- set chassis hv3 other_config:is-remote=true
> > +
> > +check ovn-sbctl chassis-add hv4 geneve 192.168.0.14
> > +
> > +check ovn-nbctl lr-add tr
> > +ovn-nbctl lrp-add tr tr-local 00:00:00:00:30:1 192.168.100.1/31
> > +
> > +ovn-nbctl lrp-add tr tr-az2 00:00:00:00:30:03 192.168.100.3/31 \
> > + -- set Logical_Router_Port tr-az2 options:requested-chassis=hv2
> > +
> > +ovn-nbctl lrp-add tr tr-az3 00:00:00:00:30:04 192.168.100.4/31 \
> > + -- set Logical_Router_Port tr-az3 options:requested-chassis=hv3
> > +
> > +ovn-nbctl lrp-add tr tr-az4 00:00:00:00:30:04 192.168.100.4/31 \
> > + -- set Logical_Router_Port tr-az4 options:requested-chassis=hv4
> > +
> > +check ovn-nbctl --wait=sb sync
> > +
> > +hv2_uuid=$(fetch_column Chassis _uuid name=hv2)
> > +hv3_uuid=$(fetch_column Chassis _uuid name=hv3)
> > +hv4_uuid=$(fetch_column Chassis _uuid name=hv4)
> > +
> > +check_row_count Port_Binding 1 logical_port=tr-az2 type=remote
> chassis=$hv2_uuid requested-chassis=$hv2_uuid
> > +check_row_count Port_Binding 1 logical_port=tr-az3 type=remote
> chassis=$hv3_uuid requested-chassis=$hv3_uuid
> > +
> > +# hv4 is not set as remote, so the port should remain as patch port
> > +check_row_count Port_Binding 1 logical_port=tr-az4 type=patch
> > +
> > +check ovn-sbctl set chassis hv4 other_config:is-remote=true
> > +check ovn-nbctl --wait=sb sync
> > +
> > +# Setting hv4 as remote should change the tr-az4 to remote too
> > +check_row_count Port_Binding 1 logical_port=tr-az4 type=remote
> chassis=$hv4_uuid requested-chassis=$hv4_uuid
> > +
> > +check ovn-sbctl remove chassis hv4 other_config is-remote
> > +check ovn-nbctl --wait=sb sync
> > +
> > +# Removing "is-remote" from hv4 should change the tr-az4 back to patch
> port
> > +check_row_count Port_Binding 1 logical_port=tr-az4 type=patch
> > +
> > +AT_CLEANUP
> > +])
> > --
> > 2.48.1
> >
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
