Message ID | 20250210153649.12676-1-numans@ovn.org |
---|---|
State | Superseded |
Headers | show |
Series | controller: Optimize adding 'dps' to the local datapaths. | 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 | fail | github build: failed |
On Mon, Feb 10, 2025 at 10:37 AM <numans@ovn.org> wrote: > > From: Numan Siddique <numans@ovn.org> > > This flag is added to the external_ids column of SB Datapath_binding > table only if a logical switch meets the following > criteria: > - has one or more localnet ports. > - has one or more router ports and all its peers are distributed > gateway ports. > > ovn-controller in the following patch makes use of this flag. > > Signed-off-by: Numan Siddique <numans@ovn.org> > --- > northd/northd.c | 30 ++++++++++++++++++++--- > northd/northd.h | 4 +++ > tests/ovn-northd.at | 59 +++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 89 insertions(+), 4 deletions(-) > > diff --git a/northd/northd.c b/northd/northd.c > index 7e2e6881ab..bf6485e02d 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -777,6 +777,14 @@ ovn_datapath_update_external_ids(struct ovn_datapath *od) > smap_add_format(&ids, "fdb_age_threshold", > "%u", age_threshold); > } > + > + /* A logical switch datapath is a true provider switch if Please ignore this comment. It's incorrect. Forgot to clean this up. I'll remove it in the next version. Numan > + * - It has (a) localnet port(s) and > + * - All its logical router ports' peers are DGPs. */ > + if (od->n_router_ports && od->n_router_ports == od->n_peer_dgw_ports > + && od->n_localnet_ports) { > + smap_add(&ids, "only_dgp_peer_ports", "true"); > + } > } > > /* Set snat-ct-zone */ > @@ -812,6 +820,20 @@ ovn_datapath_update_external_ids(struct ovn_datapath *od) > smap_destroy(&ids); > } > > +static void > +update_datapaths_external_ids(struct ovn_datapaths *ls_datapaths, > + struct ovn_datapaths *lr_datapaths) > +{ > + struct ovn_datapath *od; > + HMAP_FOR_EACH (od, key_node, &ls_datapaths->datapaths) { > + ovn_datapath_update_external_ids(od); > + } > + > + HMAP_FOR_EACH (od, key_node, &lr_datapaths->datapaths) { > + ovn_datapath_update_external_ids(od); > + } > +} > + > static void > join_datapaths(const struct nbrec_logical_switch_table *nbrec_ls_table, > const struct nbrec_logical_router_table *nbrec_lr_table, > @@ -864,7 +886,6 @@ join_datapaths(const struct nbrec_logical_switch_table *nbrec_ls_table, > od->nbs = nbs; > ovs_list_remove(&od->list); > ovs_list_push_back(both, &od->list); > - ovn_datapath_update_external_ids(od); > } else { > od = ovn_datapath_create(datapaths, &nbs->header_.uuid, > nbs, NULL, NULL); > @@ -892,7 +913,6 @@ join_datapaths(const struct nbrec_logical_switch_table *nbrec_ls_table, > od->nbr = nbr; > ovs_list_remove(&od->list); > ovs_list_push_back(both, &od->list); > - ovn_datapath_update_external_ids(od); > } else { > /* Can't happen! */ > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > @@ -1061,11 +1081,9 @@ build_datapaths(struct ovsdb_idl_txn *ovnsb_txn, > if (od->sb->tunnel_key != od->tunnel_key) { > sbrec_datapath_binding_set_tunnel_key(od->sb, od->tunnel_key); > } > - ovn_datapath_update_external_ids(od); > } > LIST_FOR_EACH (od, list, &nb_only) { > od->sb = sbrec_datapath_binding_insert(ovnsb_txn); > - ovn_datapath_update_external_ids(od); > sbrec_datapath_binding_set_tunnel_key(od->sb, od->tunnel_key); > } > ovn_destroy_tnlids(&dp_tnlids); > @@ -2528,6 +2546,8 @@ join_logical_ports(const struct sbrec_port_binding_table *sbrec_pb_table, > /* Only used for the router type LSP whose peer is l3dgw_port */ > op->peer->enable_router_port_acl = smap_get_bool( > &op->peer->nbsp->options, "enable_router_port_acl", false); > + > + op->peer->od->n_peer_dgw_ports++; > } > } > > @@ -18772,6 +18792,8 @@ ovnnb_db_run(struct northd_input *input_data, > input_data->sbrec_ha_chassis_grp_by_name, > &data->ls_datapaths.datapaths, &data->lr_datapaths.datapaths, > &data->ls_ports, &data->lr_ports); > + update_datapaths_external_ids(&data->ls_datapaths, > + &data->lr_datapaths); > build_lb_port_related_data(ovnsb_txn, > input_data->sbrec_service_monitor_table, > input_data->svc_monitor_mac, > diff --git a/northd/northd.h b/northd/northd.h > index 5fca3526bd..a334415c66 100644 > --- a/northd/northd.h > +++ b/northd/northd.h > @@ -338,6 +338,10 @@ struct ovn_datapath { > size_t n_router_ports; > size_t n_allocated_router_ports; > > + /* Indicates the number of router port's peers which are distributed > + * gateway ports (DGPs). */ > + size_t n_peer_dgw_ports; > + > struct hmap port_tnlids; > uint32_t port_key_hint; > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 352339296c..01f0fe4f18 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -14605,6 +14605,65 @@ check rbr_match_custom > > AT_CLEANUP > > +OVN_FOR_EACH_NORTHD_NO_HV([ > +AT_SETUP([ovn-northd -- only_dgp_peer_ports flag in SB datapath_binding]) > +ovn_start > + > +check ovn-nbctl ls-add sw0 > +check ovn-nbctl --wait=sb sync > + > +# Check that there is no option "only_dgp_peer_ports" in the > +# SB.datapath_binding's external_ids. > +AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [1], [ignore], [ignore]) > + > +# only_dgp_peer_ports flag will be set in the SB:datapath_binding's external_ids > +# only if the logical switch has > +# - one or more localnet ports. > +# - All its router ports' peeers are DGPs if one or more router > +# ports are present. > + > +check ovn-nbctl --wait=sb lsp-add sw0 ln-sw0 -- lsp-set-type ln-sw0 localnet > + > +AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [1], [ignore], [ignore]) > + > +check ovn-nbctl lsp-add sw0 sw0-lr0 -- lsp-set-type sw0-lr0 router > +check ovn-nbctl --wait=sb lsp-set-options sw0-lr0 router-port=lr0-sw0 > + > +AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [1], [ignore], [ignore]) > + > +check ovn-nbctl lr-add lr0 > +check ovn-nbctl --wait=sb lrp-add lr0 lr0-sw0 00:00:02:01:02:03 172.16.1.1/24 > + > +AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [1], [ignore], [ignore]) > + > +check ovn-nbctl --wait=sb lrp-set-gateway-chassis lr0-sw0 ovn-gw-1 > +AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [0], [dnl > +"true" > +]) > + > +check ovn-nbctl lsp-add sw0 sw0-lr1 -- lsp-set-type sw0-lr1 router > +check ovn-nbctl --wait=sb lsp-set-options sw0-lr1 router-port=lr1-sw0 > +check ovn-nbctl lr-add lr1 > +check ovn-nbctl --wait=sb lrp-add lr1 lr1-sw0 00:00:02:01:02:03 172.16.1.1/24 > + > +AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [1], [ignore], [ignore]) > + > +check ovn-nbctl --wait=sb lrp-set-gateway-chassis lr1-sw0 ovn-gw-1 > +AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [0], [dnl > +"true" > +]) > + > +# Just add a normal port. > +check ovn-nbctl --wait=sb lsp-add sw0 sw0p1 > + > +check ovn-nbctl --wait=sb lrp-set-gateway-chassis lr1-sw0 ovn-gw-1 > +AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [0], [dnl > +"true" > +]) > + > +AT_CLEANUP > +]) > + > OVN_FOR_EACH_NORTHD_NO_HV([ > AT_SETUP([ACL mismatched tiers]) > ovn_start > -- > 2.48.1 >
diff --git a/northd/northd.c b/northd/northd.c index 7e2e6881ab..bf6485e02d 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -777,6 +777,14 @@ ovn_datapath_update_external_ids(struct ovn_datapath *od) smap_add_format(&ids, "fdb_age_threshold", "%u", age_threshold); } + + /* A logical switch datapath is a true provider switch if + * - It has (a) localnet port(s) and + * - All its logical router ports' peers are DGPs. */ + if (od->n_router_ports && od->n_router_ports == od->n_peer_dgw_ports + && od->n_localnet_ports) { + smap_add(&ids, "only_dgp_peer_ports", "true"); + } } /* Set snat-ct-zone */ @@ -812,6 +820,20 @@ ovn_datapath_update_external_ids(struct ovn_datapath *od) smap_destroy(&ids); } +static void +update_datapaths_external_ids(struct ovn_datapaths *ls_datapaths, + struct ovn_datapaths *lr_datapaths) +{ + struct ovn_datapath *od; + HMAP_FOR_EACH (od, key_node, &ls_datapaths->datapaths) { + ovn_datapath_update_external_ids(od); + } + + HMAP_FOR_EACH (od, key_node, &lr_datapaths->datapaths) { + ovn_datapath_update_external_ids(od); + } +} + static void join_datapaths(const struct nbrec_logical_switch_table *nbrec_ls_table, const struct nbrec_logical_router_table *nbrec_lr_table, @@ -864,7 +886,6 @@ join_datapaths(const struct nbrec_logical_switch_table *nbrec_ls_table, od->nbs = nbs; ovs_list_remove(&od->list); ovs_list_push_back(both, &od->list); - ovn_datapath_update_external_ids(od); } else { od = ovn_datapath_create(datapaths, &nbs->header_.uuid, nbs, NULL, NULL); @@ -892,7 +913,6 @@ join_datapaths(const struct nbrec_logical_switch_table *nbrec_ls_table, od->nbr = nbr; ovs_list_remove(&od->list); ovs_list_push_back(both, &od->list); - ovn_datapath_update_external_ids(od); } else { /* Can't happen! */ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); @@ -1061,11 +1081,9 @@ build_datapaths(struct ovsdb_idl_txn *ovnsb_txn, if (od->sb->tunnel_key != od->tunnel_key) { sbrec_datapath_binding_set_tunnel_key(od->sb, od->tunnel_key); } - ovn_datapath_update_external_ids(od); } LIST_FOR_EACH (od, list, &nb_only) { od->sb = sbrec_datapath_binding_insert(ovnsb_txn); - ovn_datapath_update_external_ids(od); sbrec_datapath_binding_set_tunnel_key(od->sb, od->tunnel_key); } ovn_destroy_tnlids(&dp_tnlids); @@ -2528,6 +2546,8 @@ join_logical_ports(const struct sbrec_port_binding_table *sbrec_pb_table, /* Only used for the router type LSP whose peer is l3dgw_port */ op->peer->enable_router_port_acl = smap_get_bool( &op->peer->nbsp->options, "enable_router_port_acl", false); + + op->peer->od->n_peer_dgw_ports++; } } @@ -18772,6 +18792,8 @@ ovnnb_db_run(struct northd_input *input_data, input_data->sbrec_ha_chassis_grp_by_name, &data->ls_datapaths.datapaths, &data->lr_datapaths.datapaths, &data->ls_ports, &data->lr_ports); + update_datapaths_external_ids(&data->ls_datapaths, + &data->lr_datapaths); build_lb_port_related_data(ovnsb_txn, input_data->sbrec_service_monitor_table, input_data->svc_monitor_mac, diff --git a/northd/northd.h b/northd/northd.h index 5fca3526bd..a334415c66 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -338,6 +338,10 @@ struct ovn_datapath { size_t n_router_ports; size_t n_allocated_router_ports; + /* Indicates the number of router port's peers which are distributed + * gateway ports (DGPs). */ + size_t n_peer_dgw_ports; + struct hmap port_tnlids; uint32_t port_key_hint; diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 352339296c..01f0fe4f18 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -14605,6 +14605,65 @@ check rbr_match_custom AT_CLEANUP +OVN_FOR_EACH_NORTHD_NO_HV([ +AT_SETUP([ovn-northd -- only_dgp_peer_ports flag in SB datapath_binding]) +ovn_start + +check ovn-nbctl ls-add sw0 +check ovn-nbctl --wait=sb sync + +# Check that there is no option "only_dgp_peer_ports" in the +# SB.datapath_binding's external_ids. +AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [1], [ignore], [ignore]) + +# only_dgp_peer_ports flag will be set in the SB:datapath_binding's external_ids +# only if the logical switch has +# - one or more localnet ports. +# - All its router ports' peeers are DGPs if one or more router +# ports are present. + +check ovn-nbctl --wait=sb lsp-add sw0 ln-sw0 -- lsp-set-type ln-sw0 localnet + +AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [1], [ignore], [ignore]) + +check ovn-nbctl lsp-add sw0 sw0-lr0 -- lsp-set-type sw0-lr0 router +check ovn-nbctl --wait=sb lsp-set-options sw0-lr0 router-port=lr0-sw0 + +AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [1], [ignore], [ignore]) + +check ovn-nbctl lr-add lr0 +check ovn-nbctl --wait=sb lrp-add lr0 lr0-sw0 00:00:02:01:02:03 172.16.1.1/24 + +AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [1], [ignore], [ignore]) + +check ovn-nbctl --wait=sb lrp-set-gateway-chassis lr0-sw0 ovn-gw-1 +AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [0], [dnl +"true" +]) + +check ovn-nbctl lsp-add sw0 sw0-lr1 -- lsp-set-type sw0-lr1 router +check ovn-nbctl --wait=sb lsp-set-options sw0-lr1 router-port=lr1-sw0 +check ovn-nbctl lr-add lr1 +check ovn-nbctl --wait=sb lrp-add lr1 lr1-sw0 00:00:02:01:02:03 172.16.1.1/24 + +AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [1], [ignore], [ignore]) + +check ovn-nbctl --wait=sb lrp-set-gateway-chassis lr1-sw0 ovn-gw-1 +AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [0], [dnl +"true" +]) + +# Just add a normal port. +check ovn-nbctl --wait=sb lsp-add sw0 sw0p1 + +check ovn-nbctl --wait=sb lrp-set-gateway-chassis lr1-sw0 ovn-gw-1 +AT_CHECK([ovn-sbctl get datapath_binding sw0 external_ids:only_dgp_peer_ports], [0], [dnl +"true" +]) + +AT_CLEANUP +]) + OVN_FOR_EACH_NORTHD_NO_HV([ AT_SETUP([ACL mismatched tiers]) ovn_start