| Message ID | 20241203110853.201377-5-amusil@redhat.com |
|---|---|
| State | Superseded |
| Delegated to: | Numan Siddique |
| Headers | show |
| Series | Introduce the concept of transit router. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On Tue, Dec 3, 2024 at 12:09 PM Ales Musil <amusil@redhat.com> 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 <amusil@redhat.com> > --- > 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"); > } > > @@ -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 > > Recheck-request: github-robot-_Build_and_Test
> 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 <amusil@redhat.com> 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 <lorenzo.bianconi@redhat.com> > --- > 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 :)) > > @@ -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 > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Mon, Jan 20, 2025 at 4:48 PM Lorenzo Bianconi < lorenzo.bianconi@redhat.com> 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 <amusil@redhat.com> > > 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 <lorenzo.bianconi@redhat.com> > 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 > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > Thanks, Ales
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"); } @@ -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 +])
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 <amusil@redhat.com> --- 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(-)