On Mon, Jan 20, 2025 at 4:48 PM Lorenzo Bianconi < [email protected]> wrote:
> > In order to allow the concept of transit router set the port binding > > as "remote" and assign the chassis whenever the requested-chassis > > for LRP is remote chassis. The only supported requested-chassis for > > now is remote, it might be extended in the future if needed. > > > > The transit router has very similar behavior to distributed router > > that it will lose the CT state between chassis, in this case between > > AZs. > > > > Reported-at: https://issues.redhat.com/browse/FDP-872 > > Signed-off-by: Ales Musil <[email protected]> > > Hi Ales, > > I think this patch is fine, it just needs a trivial rebase for a conflict > in > NEWS (but I guess it can be addressed applying the patch). Just a nit > inline > (I guess you can fix it if you need to send another revision). > > Acked-by: Lorenzo Bianconi <[email protected]> > Hi Lorenzo, thank you for the review. > > > --- > > NEWS | 3 +++ > > lib/ovn-util.c | 2 +- > > northd/northd.c | 31 +++++++++++++++++++++++++-- > > northd/northd.h | 4 ++++ > > ovn-nb.xml | 43 ++++++++++++++++++++++++++++++++++++++ > > tests/ovn-northd.at | 51 +++++++++++++++++++++++++++++++++++++++++++++ > > 6 files changed, 131 insertions(+), 3 deletions(-) > > > > diff --git a/NEWS b/NEWS > > index da3aba739..0ef98a305 100644 > > --- a/NEWS > > +++ b/NEWS > > @@ -4,6 +4,9 @@ Post v24.09.0 > > hash (with specified hash fields) for ECMP routes > > while choosing nexthop. > > - ovn-ic: Add support for route tag to prevent route learning. > > + - Add concept of Transit Routers, the routers not allow CMS to > specify > > + options:requested-chassis, if the chassis is remote the port will > behave > > + as remote port. > > > > OVN v24.09.0 - 13 Sep 2024 > > -------------------------- > > diff --git a/lib/ovn-util.c b/lib/ovn-util.c > > index 1ad347419..a1c8d8fb0 100644 > > --- a/lib/ovn-util.c > > +++ b/lib/ovn-util.c > > @@ -1226,6 +1226,7 @@ is_pb_router_type(const struct sbrec_port_binding > *pb) > > case LP_CHASSISREDIRECT: > > case LP_L3GATEWAY: > > case LP_L2GATEWAY: > > + case LP_REMOTE: > > return true; > > > > case LP_VIF: > > @@ -1233,7 +1234,6 @@ is_pb_router_type(const struct sbrec_port_binding > *pb) > > case LP_VIRTUAL: > > case LP_LOCALNET: > > case LP_LOCALPORT: > > - case LP_REMOTE: > > case LP_VTEP: > > case LP_EXTERNAL: > > case LP_UNKNOWN: > > diff --git a/northd/northd.c b/northd/northd.c > > index 3a488ff3d..f3ef090f4 100644 > > --- a/northd/northd.c > > +++ b/northd/northd.c > > @@ -3082,12 +3082,29 @@ ovn_port_update_sbrec(struct ovsdb_idl_txn > *ovnsb_txn, > > > > /* 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"); > > if (is_cr_port(op)) { > > + sbrec_port_binding_set_requested_chassis(op->sb, NULL); > > sbrec_port_binding_set_type(op->sb, "chassisredirect"); > > - } else if (chassis_name) { > > + } else if (lr_chassis) { > > + sbrec_port_binding_set_requested_chassis(op->sb, NULL); > > sbrec_port_binding_set_type(op->sb, "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); > > + sbrec_port_binding_set_requested_chassis(op->sb, > > + trp ? tr_chassis : > NULL); > > + sbrec_port_binding_set_chassis(op->sb, trp ? tr_chassis : > NULL); > > + sbrec_port_binding_set_type(op->sb, trp ? "remote" : > "patch"); > > } else { > > + sbrec_port_binding_set_requested_chassis(op->sb, NULL); > > sbrec_port_binding_set_type(op->sb, "patch"); > > } > > nit: since you always need to set requested_chassis(), I guess you can > move it > at the end of the if/else conditions (just personal taste :)) > You are right, it's probably better to call the function outside of the block. I have adjusted that in v2. > > > > @@ -4149,6 +4166,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) > > { > > @@ -4254,6 +4279,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); > > } > > > > @@ -4267,6 +4293,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 d60c944df..7df57ba19 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 5114bbc2e..3b208052b 100644 > > --- a/ovn-nb.xml > > +++ b/ovn-nb.xml > > @@ -3095,6 +3095,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"> > > @@ -3705,6 +3731,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 4eae1c67c..e34649f10 100644 > > --- a/tests/ovn-northd.at > > +++ b/tests/ovn-northd.at > > @@ -14253,3 +14253,54 @@ AT_CHECK([grep "lr_in_dnat" lr1flows | > ovn_strip_lflows | grep "30.0.0.1"], [0], > > > > 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.47.0 > > > > _______________________________________________ > > dev mailing list > > [email protected] > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > Thanks, Ales _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
