| Message ID | 20210928155325.2290444-2-frode.nordahl@canonical.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Introduce infrastructure for plug providers | 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 | success | github build: passed |
On Tue, Sep 28, 2021 at 11:54 AM Frode Nordahl <frode.nordahl@canonical.com> wrote: > > To allow for ovn-controller to efficiently monitor all > Port_Binding records destined to it, we add a new > requested_chassis column with weakRef to the Chassis table. > > The ovn-controller can monitor this column and only process > records for its chassis UUID before having claimed the port. > > northd will fill this column with UUID of Chassis referenced > in Logical_Switch_Port options:requested-chassis by name or > hostname. > > In a subsequent update to the controller we will improve the > efficiency of the requested-chassis feature by using the new > column instead of each chassis performing string comparison on > this option for every Port_Binding record processed. > > Note that the CMS facing Northbound Logical_Switch_Port:options > API remains the same. > > Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com> Acked-by: Numan Siddique <numans@ovn.org> Numan > --- > lib/chassis-index.c | 24 +++++++++ > lib/chassis-index.h | 3 ++ > northd/northd.c | 45 ++++++++++++++-- > northd/northd.h | 1 + > northd/ovn-northd.c | 9 +++- > northd/ovn_northd.dl | 123 ++++++++++++++++++++++++++++++++++++++++--- > ovn-sb.ovsschema | 10 ++-- > ovn-sb.xml | 21 ++++++++ > tests/ovn-northd.at | 45 ++++++++++++++++ > 9 files changed, 266 insertions(+), 15 deletions(-) > > diff --git a/lib/chassis-index.c b/lib/chassis-index.c > index 13120fe3e..4b38036cb 100644 > --- a/lib/chassis-index.c > +++ b/lib/chassis-index.c > @@ -22,6 +22,12 @@ chassis_index_create(struct ovsdb_idl *idl) > return ovsdb_idl_index_create1(idl, &sbrec_chassis_col_name); > } > > +struct ovsdb_idl_index * > +chassis_hostname_index_create(struct ovsdb_idl *idl) > +{ > + return ovsdb_idl_index_create1(idl, &sbrec_chassis_col_hostname); > +} > + > /* Finds and returns the chassis with the given 'name', or NULL if no such > * chassis exists. */ > const struct sbrec_chassis * > @@ -40,6 +46,24 @@ chassis_lookup_by_name(struct ovsdb_idl_index *sbrec_chassis_by_name, > return retval; > } > > +/* Finds and returns the chassis with the given 'hostname', or NULL if no such > + * chassis exists. */ > +const struct sbrec_chassis * > +chassis_lookup_by_hostname(struct ovsdb_idl_index *sbrec_chassis_by_hostname, > + const char *hostname) > +{ > + struct sbrec_chassis *target = sbrec_chassis_index_init_row( > + sbrec_chassis_by_hostname); > + sbrec_chassis_index_set_hostname(target, hostname); > + > + struct sbrec_chassis *retval = sbrec_chassis_index_find( > + sbrec_chassis_by_hostname, target); > + > + sbrec_chassis_index_destroy_row(target); > + > + return retval; > +} > + > struct ovsdb_idl_index * > chassis_private_index_create(struct ovsdb_idl *idl) > { > diff --git a/lib/chassis-index.h b/lib/chassis-index.h > index b9b331f34..bc654da13 100644 > --- a/lib/chassis-index.h > +++ b/lib/chassis-index.h > @@ -19,9 +19,12 @@ > struct ovsdb_idl; > > struct ovsdb_idl_index *chassis_index_create(struct ovsdb_idl *); > +struct ovsdb_idl_index *chassis_hostname_index_create(struct ovsdb_idl *); > > const struct sbrec_chassis *chassis_lookup_by_name( > struct ovsdb_idl_index *sbrec_chassis_by_name, const char *name); > +const struct sbrec_chassis *chassis_lookup_by_hostname( > + struct ovsdb_idl_index *sbrec_chassis_by_hostname, const char *hostname); > > struct ovsdb_idl_index *chassis_private_index_create(struct ovsdb_idl *); > > diff --git a/northd/northd.c b/northd/northd.c > index cf2467fe1..eb0e5d6de 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -2982,6 +2982,7 @@ ovn_update_ipv6_prefix(struct hmap *ports) > static void > ovn_port_update_sbrec(struct northd_context *ctx, > struct ovsdb_idl_index *sbrec_chassis_by_name, > + struct ovsdb_idl_index *sbrec_chassis_by_hostname, > const struct ovn_port *op, > struct hmap *chassis_qdisc_queues, > struct sset *active_ha_chassis_grps) > @@ -3163,6 +3164,36 @@ ovn_port_update_sbrec(struct northd_context *ctx, > * ha_chassis_group cleared in the same transaction. */ > sbrec_port_binding_set_ha_chassis_group(op->sb, NULL); > } > + > + const char *requested_chassis; /* May be NULL. */ > + bool reset_requested_chassis = false; > + requested_chassis = smap_get(&op->nbsp->options, > + "requested-chassis"); > + if (requested_chassis) { > + const struct sbrec_chassis *chassis; /* May be NULL. */ > + chassis = chassis_lookup_by_name(sbrec_chassis_by_name, > + requested_chassis); > + chassis = chassis ? chassis : chassis_lookup_by_hostname( > + sbrec_chassis_by_hostname, requested_chassis); > + > + if (chassis) { > + sbrec_port_binding_set_requested_chassis(op->sb, chassis); > + } else { > + reset_requested_chassis = true; > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT( > + 1, 1); > + VLOG_WARN_RL( > + &rl, > + "Unknown chassis '%s' set as " > + "options:requested-chassis on LSP '%s'.", > + requested_chassis, op->nbsp->name); > + } > + } else if (op->sb->requested_chassis) { > + reset_requested_chassis = true; > + } > + if (reset_requested_chassis) { > + sbrec_port_binding_set_requested_chassis(op->sb, NULL); > + } > } else { > const char *chassis = NULL; > if (op->peer && op->peer->od && op->peer->od->nbr) { > @@ -3791,6 +3822,7 @@ ovn_port_allocate_key(struct hmap *ports, struct ovn_port *op) > static void > build_ports(struct northd_context *ctx, > struct ovsdb_idl_index *sbrec_chassis_by_name, > + struct ovsdb_idl_index *sbrec_chassis_by_hostname, > struct hmap *datapaths, struct hmap *ports) > { > struct ovs_list sb_only, nb_only, both; > @@ -3846,6 +3878,7 @@ build_ports(struct northd_context *ctx, > tag_alloc_create_new_tag(&tag_alloc_table, op->nbsp); > } > ovn_port_update_sbrec(ctx, sbrec_chassis_by_name, > + sbrec_chassis_by_hostname, > op, &chassis_qdisc_queues, > &active_ha_chassis_grps); > } > @@ -3853,7 +3886,8 @@ build_ports(struct northd_context *ctx, > /* Add southbound record for each unmatched northbound record. */ > LIST_FOR_EACH_SAFE (op, next, list, &nb_only) { > op->sb = sbrec_port_binding_insert(ctx->ovnsb_txn); > - ovn_port_update_sbrec(ctx, sbrec_chassis_by_name, op, > + ovn_port_update_sbrec(ctx, sbrec_chassis_by_name, > + sbrec_chassis_by_hostname, op, > &chassis_qdisc_queues, > &active_ha_chassis_grps); > sbrec_port_binding_set_logical_port(op->sb, op->key); > @@ -14208,6 +14242,7 @@ build_meter_groups(struct northd_context *ctx, > static void > ovnnb_db_run(struct northd_context *ctx, > struct ovsdb_idl_index *sbrec_chassis_by_name, > + struct ovsdb_idl_index *sbrec_chassis_by_hostname, > struct ovsdb_idl_loop *sb_loop, > struct hmap *datapaths, struct hmap *ports, > struct ovs_list *lr_list, > @@ -14309,7 +14344,8 @@ ovnnb_db_run(struct northd_context *ctx, > build_datapaths(ctx, datapaths, lr_list); > build_ovn_lbs(ctx, datapaths, &lbs); > build_lrouter_lbs(datapaths, &lbs); > - build_ports(ctx, sbrec_chassis_by_name, datapaths, ports); > + build_ports(ctx, sbrec_chassis_by_name, sbrec_chassis_by_hostname, > + datapaths, ports); > build_ovn_lr_lbs(datapaths, &lbs); > build_ovn_lb_svcs(ctx, ports, &lbs); > build_ipam(datapaths, ports); > @@ -14656,6 +14692,7 @@ ovnsb_db_run(struct northd_context *ctx, > void > ovn_db_run(struct northd_context *ctx, > struct ovsdb_idl_index *sbrec_chassis_by_name, > + struct ovsdb_idl_index *sbrec_chassis_by_hostname, > struct ovsdb_idl_loop *ovnsb_idl_loop, > const char *ovn_internal_version) > { > @@ -14668,8 +14705,8 @@ ovn_db_run(struct northd_context *ctx, > > int64_t start_time = time_wall_msec(); > stopwatch_start(OVNNB_DB_RUN_STOPWATCH_NAME, time_msec()); > - ovnnb_db_run(ctx, sbrec_chassis_by_name, ovnsb_idl_loop, > - &datapaths, &ports, &lr_list, start_time, > + ovnnb_db_run(ctx, sbrec_chassis_by_name, sbrec_chassis_by_hostname, > + ovnsb_idl_loop, &datapaths, &ports, &lr_list, start_time, > ovn_internal_version); > stopwatch_stop(OVNNB_DB_RUN_STOPWATCH_NAME, time_msec()); > stopwatch_start(OVNSB_DB_RUN_STOPWATCH_NAME, time_msec()); > diff --git a/northd/northd.h b/northd/northd.h > index ffa2bbb4e..d2a931ada 100644 > --- a/northd/northd.h > +++ b/northd/northd.h > @@ -34,6 +34,7 @@ struct northd_context { > void > ovn_db_run(struct northd_context *ctx, > struct ovsdb_idl_index *sbrec_chassis_by_name, > + struct ovsdb_idl_index *sbrec_chassis_by_hostname, > struct ovsdb_idl_loop *ovnsb_idl_loop, > const char *ovn_internal_version); > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index 39aa96055..9c33378fb 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -716,6 +716,8 @@ main(int argc, char *argv[]) > add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_mac); > add_column_noalert(ovnsb_idl_loop.idl, > &sbrec_port_binding_col_nat_addresses); > + add_column_noalert(ovnsb_idl_loop.idl, > + &sbrec_port_binding_col_requested_chassis); > ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_port_binding_col_chassis); > ovsdb_idl_add_column(ovnsb_idl_loop.idl, > &sbrec_port_binding_col_gateway_chassis); > @@ -787,6 +789,7 @@ main(int argc, char *argv[]) > > ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_chassis); > ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_chassis_col_name); > + ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_chassis_col_hostname); > ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_chassis_col_other_config); > ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_chassis_col_encaps); > > @@ -895,6 +898,9 @@ main(int argc, char *argv[]) > struct ovsdb_idl_index *sbrec_chassis_by_name > = chassis_index_create(ovnsb_idl_loop.idl); > > + struct ovsdb_idl_index *sbrec_chassis_by_hostname > + = chassis_hostname_index_create(ovnsb_idl_loop.idl); > + > struct ovsdb_idl_index *sbrec_ha_chassis_grp_by_name > = ha_chassis_group_index_create(ovnsb_idl_loop.idl); > > @@ -975,7 +981,8 @@ main(int argc, char *argv[]) > } > > if (ovsdb_idl_has_lock(ovnsb_idl_loop.idl)) { > - ovn_db_run(&ctx, sbrec_chassis_by_name, &ovnsb_idl_loop, > + ovn_db_run(&ctx, sbrec_chassis_by_name, > + sbrec_chassis_by_hostname, &ovnsb_idl_loop, > ovn_internal_version); > if (ctx.ovnsb_txn) { > check_and_add_supported_dhcp_opts_to_sb_db(&ctx); > diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl > index 22913f05a..e379623a6 100644 > --- a/northd/ovn_northd.dl > +++ b/northd/ovn_northd.dl > @@ -105,6 +105,22 @@ sb::Out_Datapath_Binding(uuid, tunkey, load_balancers, external_ids) :- > */ > var load_balancers = set_empty(). > > +function get_requested_chassis(options: Map<istring,istring>) : istring = { > + var requested_chassis = match(options.get(i"requested-chassis")) { > + None -> i"", > + Some{requested_chassis} -> requested_chassis, > + }; > + requested_chassis > +} > + > +relation RequestedChassis( > + name: istring, > + chassis: uuid, > +) > +RequestedChassis(name, chassis) :- > + sb::Chassis(._uuid = chassis, .name=name). > +RequestedChassis(hostname, chassis) :- > + sb::Chassis(._uuid = chassis, .hostname=hostname). > > /* Proxy table for Out_Datapath_Binding: contains all Datapath_Binding fields, > * except tunnel id, which is allocated separately (see PortTunKeyAllocation). */ > @@ -120,10 +136,95 @@ relation OutProxy_Port_Binding ( > tag: Option<integer>, > mac: Set<istring>, > nat_addresses: Set<istring>, > - external_ids: Map<istring,istring> > + external_ids: Map<istring,istring>, > + requested_chassis: Option<uuid> > ) > > -/* Case 1: Create a Port_Binding per logical switch port that is not of type "router" */ > +/* Case 1a: Create a Port_Binding per logical switch port that is not of type > + * "router" */ > +OutProxy_Port_Binding(._uuid = lsp._uuid, > + .logical_port = lsp.name, > + .__type = lsp.__type, > + .gateway_chassis = set_empty(), > + .ha_chassis_group = sp.hac_group_uuid, > + .options = options, > + .datapath = sw._uuid, > + .parent_port = lsp.parent_name, > + .tag = tag, > + .mac = lsp.addresses, > + .nat_addresses = set_empty(), > + .external_ids = eids, > + .requested_chassis = None) :- > + sp in &SwitchPort(.lsp = lsp, .sw = sw), > + SwitchPortNewDynamicTag(lsp._uuid, opt_tag), > + var tag = match (opt_tag) { > + None -> lsp.tag, > + Some{t} -> Some{t} > + }, > + lsp.__type != i"router", > + var chassis_name_or_hostname = get_requested_chassis(lsp.options), > + chassis_name_or_hostname == i"", > + var eids = { > + var eids = lsp.external_ids; > + match (lsp.external_ids.get(i"neutron:port_name")) { > + None -> (), > + Some{name} -> eids.insert(i"name", name) > + }; > + eids > + }, > + var options = { > + var options = lsp.options; > + if (sw.other_config.get(i"vlan-passthru") == Some{i"true"}) { > + options.insert(i"vlan-passthru", i"true") > + }; > + options > + }. > + > +/* Case 1b: Create a Port_Binding per logical switch port that is not of type > + * "router" and has options "requested-chassis" pointing at chassis name or > + * hostname. */ > +OutProxy_Port_Binding(._uuid = lsp._uuid, > + .logical_port = lsp.name, > + .__type = lsp.__type, > + .gateway_chassis = set_empty(), > + .ha_chassis_group = sp.hac_group_uuid, > + .options = options, > + .datapath = sw._uuid, > + .parent_port = lsp.parent_name, > + .tag = tag, > + .mac = lsp.addresses, > + .nat_addresses = set_empty(), > + .external_ids = eids, > + .requested_chassis = Some{requested_chassis}) :- > + sp in &SwitchPort(.lsp = lsp, .sw = sw), > + SwitchPortNewDynamicTag(lsp._uuid, opt_tag), > + var tag = match (opt_tag) { > + None -> lsp.tag, > + Some{t} -> Some{t} > + }, > + lsp.__type != i"router", > + var chassis_name_or_hostname = get_requested_chassis(lsp.options), > + chassis_name_or_hostname != i"", > + RequestedChassis(chassis_name_or_hostname, requested_chassis), > + var eids = { > + var eids = lsp.external_ids; > + match (lsp.external_ids.get(i"neutron:port_name")) { > + None -> (), > + Some{name} -> eids.insert(i"name", name) > + }; > + eids > + }, > + var options = { > + var options = lsp.options; > + if (sw.other_config.get(i"vlan-passthru") == Some{i"true"}) { > + options.insert(i"vlan-passthru", i"true") > + }; > + options > + }. > + > +/* Case 1c: Create a Port_Binding per logical switch port that is not of type > + * "router" and has options "requested-chassis" pointing at non-existent > + * chassis name or hostname. */ > OutProxy_Port_Binding(._uuid = lsp._uuid, > .logical_port = lsp.name, > .__type = lsp.__type, > @@ -135,7 +236,8 @@ OutProxy_Port_Binding(._uuid = lsp._uuid, > .tag = tag, > .mac = lsp.addresses, > .nat_addresses = set_empty(), > - .external_ids = eids) :- > + .external_ids = eids, > + .requested_chassis = None) :- > sp in &SwitchPort(.lsp = lsp, .sw = sw), > SwitchPortNewDynamicTag(lsp._uuid, opt_tag), > var tag = match (opt_tag) { > @@ -143,6 +245,9 @@ OutProxy_Port_Binding(._uuid = lsp._uuid, > Some{t} -> Some{t} > }, > lsp.__type != i"router", > + var chassis_name_or_hostname = get_requested_chassis(lsp.options), > + chassis_name_or_hostname != i"", > + not RequestedChassis(chassis_name_or_hostname, _), > var eids = { > var eids = lsp.external_ids; > match (lsp.external_ids.get(i"neutron:port_name")) { > @@ -186,7 +291,8 @@ OutProxy_Port_Binding(._uuid = lsp._uuid, > .tag = None, > .mac = lsp.addresses, > .nat_addresses = nat_addresses, > - .external_ids = eids) :- > + .external_ids = eids, > + .requested_chassis = None) :- > SwitchPortLBIPs(.port = &SwitchPort{.lsp = lsp, .sw = sw, .peer = peer}, > .lbips = lbips), > var eids = { > @@ -280,7 +386,8 @@ OutProxy_Port_Binding(._uuid = lrp._uuid, > .tag = None, // always empty for router ports > .mac = set_singleton(i"${lrp.mac} ${lrp.networks.map(ival).to_vec().join(\" \")}"), > .nat_addresses = set_empty(), > - .external_ids = lrp.external_ids) :- > + .external_ids = lrp.external_ids, > + .requested_chassis = None) :- > rp in &RouterPort(.lrp = lrp, .router = router, .peer = peer), > RouterPortRAOptionsComplete(lrp._uuid, options0), > (var __type, var options1) = match (router.options.get(i"chassis")) { > @@ -475,7 +582,8 @@ OutProxy_Port_Binding(._uuid = cr_lrp_uuid, > .tag = None, //always empty for router ports > .mac = set_singleton(i"${lrp.mac} ${lrp.networks.map(ival).to_vec().join(\" \")}"), > .nat_addresses = set_empty(), > - .external_ids = lrp.external_ids) :- > + .external_ids = lrp.external_ids, > + .requested_chassis = None) :- > DistributedGatewayPort(lrp, lr_uuid, cr_lrp_uuid), > DistributedGatewayPortHAChassisGroup(lrp, hacg_uuid), > var redirect_type = match (lrp.options.get(i"redirect-type")) { > @@ -520,7 +628,8 @@ sb::Out_Port_Binding(._uuid = pbinding._uuid, > .mac = pbinding.mac, > .nat_addresses = pbinding.nat_addresses, > .external_ids = pbinding.external_ids, > - .up = Some{up}) :- > + .up = Some{up}, > + .requested_chassis = pbinding.requested_chassis) :- > pbinding in OutProxy_Port_Binding(), > PortTunKeyAllocation(pbinding._uuid, tunkey), > QueueIDAllocation(pbinding._uuid, qid), > diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema > index e5ab41db9..122614dd5 100644 > --- a/ovn-sb.ovsschema > +++ b/ovn-sb.ovsschema > @@ -1,7 +1,7 @@ > { > "name": "OVN_Southbound", > - "version": "20.20.0", > - "cksum": "605270161 26670", > + "version": "20.21.0", > + "cksum": "2362446865 26963", > "tables": { > "SB_Global": { > "columns": { > @@ -232,7 +232,11 @@ > "external_ids": {"type": {"key": "string", > "value": "string", > "min": 0, > - "max": "unlimited"}}}, > + "max": "unlimited"}}, > + "requested_chassis": {"type": {"key": {"type": "uuid", > + "refTable": "Chassis", > + "refType": "weak"}, > + "min": 0, "max": 1}}}, > "indexes": [["datapath", "tunnel_key"], ["logical_port"]], > "isRoot": true}, > "MAC_Binding": { > diff --git a/ovn-sb.xml b/ovn-sb.xml > index 2d4d47d10..150051f26 100644 > --- a/ovn-sb.xml > +++ b/ovn-sb.xml > @@ -2983,6 +2983,27 @@ tcp.flags = RST; > </dd> > </dl> > </column> > + <column name="requested_chassis"> > + This column exists so that the ovn-controller can effectively monitor > + all <ref table="Port_Binding"/> records destined for it, and is a > + supplement to the <ref > + table="Port_Binding" > + column="options" > + key="requested-chassis"/> option. The option is still required so that > + the ovn-controller can check the CMS intent when the chassis pointed > + to does not currently exist, which for example occurs when the > + ovn-controller is stopped without passing the --restart argument. > + > + This column must be a > + <ref table="Chassis"/> record. This is populated by > + <code>ovn-northd</code> when the <ref > + table="Logical_Switch_Port" > + column="options" > + key="requested-chassis" > + db="OVN_Northbound"/> > + is defined and contains a string matching the name or hostname of an > + existing chassis. > + </column> > </group> > > <group title="Patch Options"> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 5de554455..258b58e98 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -5223,3 +5223,48 @@ AT_CHECK([grep lr_in_gw_redirect lrflows | grep cr-DR | sed 's/table=../table=?? > > AT_CLEANUP > ]) > + > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([check options:requested-chassis fills requested_chassis col]) > +ovn_start NORTHD_TYPE > + > +# Add chassis ch1. > +check ovn-sbctl chassis-add ch1 geneve 127.0.0.2 > +check ovn-sbctl chassis-add ch2 geneve 127.0.0.3 > + > +wait_row_count Chassis 2 > + > +ch1_uuid=`ovn-sbctl --bare --columns _uuid find Chassis name="ch1"` > +ch2_uuid=`ovn-sbctl --bare --columns _uuid find Chassis name="ch2"` > + > +check ovn-sbctl set chassis $ch2_uuid hostname=ch2-hostname > + > +ovn-nbctl ls-add S1 > +ovn-nbctl --wait=sb lsp-add S1 S1-vm1 > +ovn-nbctl --wait=sb lsp-add S1 S1-vm2 > + > +wait_row_count Port_Binding 1 logical_port=S1-vm1 requested_chassis!=$ch1_uuid > +wait_row_count Port_Binding 1 logical_port=S1-vm2 requested_chassis!=$ch2_uuid > + > +ovn-nbctl --wait=sb set logical_switch_port S1-vm1 \ > + options:requested-chassis=ch1 > + > +wait_row_count Port_Binding 1 logical_port=S1-vm1 requested_chassis=$ch1_uuid > + > +ovn-nbctl --wait=sb set logical_switch_port S1-vm2 \ > + options:requested-chassis=ch2-hostname > + > +wait_row_count Port_binding 1 logical-port=S1-vm2 requested_chassis=$ch2_uuid > + > +ovn-nbctl --wait=sb remove logical_switch_port S1-vm2 \ > + options requested-chassis=ch2-hostname > + > +wait_row_count Port_binding 1 logical-port=S1-vm2 requested_chassis!=$ch2_uuid > + > +ovn-nbctl --wait=sb set logical_switch_port S1-vm2 \ > + options:requested-chassis=ch2 > + > +wait_row_count Port_binding 1 logical-port=S1-vm2 requested_chassis=$ch2_uuid > + > +AT_CLEANUP > +]) > -- > 2.32.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/lib/chassis-index.c b/lib/chassis-index.c index 13120fe3e..4b38036cb 100644 --- a/lib/chassis-index.c +++ b/lib/chassis-index.c @@ -22,6 +22,12 @@ chassis_index_create(struct ovsdb_idl *idl) return ovsdb_idl_index_create1(idl, &sbrec_chassis_col_name); } +struct ovsdb_idl_index * +chassis_hostname_index_create(struct ovsdb_idl *idl) +{ + return ovsdb_idl_index_create1(idl, &sbrec_chassis_col_hostname); +} + /* Finds and returns the chassis with the given 'name', or NULL if no such * chassis exists. */ const struct sbrec_chassis * @@ -40,6 +46,24 @@ chassis_lookup_by_name(struct ovsdb_idl_index *sbrec_chassis_by_name, return retval; } +/* Finds and returns the chassis with the given 'hostname', or NULL if no such + * chassis exists. */ +const struct sbrec_chassis * +chassis_lookup_by_hostname(struct ovsdb_idl_index *sbrec_chassis_by_hostname, + const char *hostname) +{ + struct sbrec_chassis *target = sbrec_chassis_index_init_row( + sbrec_chassis_by_hostname); + sbrec_chassis_index_set_hostname(target, hostname); + + struct sbrec_chassis *retval = sbrec_chassis_index_find( + sbrec_chassis_by_hostname, target); + + sbrec_chassis_index_destroy_row(target); + + return retval; +} + struct ovsdb_idl_index * chassis_private_index_create(struct ovsdb_idl *idl) { diff --git a/lib/chassis-index.h b/lib/chassis-index.h index b9b331f34..bc654da13 100644 --- a/lib/chassis-index.h +++ b/lib/chassis-index.h @@ -19,9 +19,12 @@ struct ovsdb_idl; struct ovsdb_idl_index *chassis_index_create(struct ovsdb_idl *); +struct ovsdb_idl_index *chassis_hostname_index_create(struct ovsdb_idl *); const struct sbrec_chassis *chassis_lookup_by_name( struct ovsdb_idl_index *sbrec_chassis_by_name, const char *name); +const struct sbrec_chassis *chassis_lookup_by_hostname( + struct ovsdb_idl_index *sbrec_chassis_by_hostname, const char *hostname); struct ovsdb_idl_index *chassis_private_index_create(struct ovsdb_idl *); diff --git a/northd/northd.c b/northd/northd.c index cf2467fe1..eb0e5d6de 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -2982,6 +2982,7 @@ ovn_update_ipv6_prefix(struct hmap *ports) static void ovn_port_update_sbrec(struct northd_context *ctx, struct ovsdb_idl_index *sbrec_chassis_by_name, + struct ovsdb_idl_index *sbrec_chassis_by_hostname, const struct ovn_port *op, struct hmap *chassis_qdisc_queues, struct sset *active_ha_chassis_grps) @@ -3163,6 +3164,36 @@ ovn_port_update_sbrec(struct northd_context *ctx, * ha_chassis_group cleared in the same transaction. */ sbrec_port_binding_set_ha_chassis_group(op->sb, NULL); } + + const char *requested_chassis; /* May be NULL. */ + bool reset_requested_chassis = false; + requested_chassis = smap_get(&op->nbsp->options, + "requested-chassis"); + if (requested_chassis) { + const struct sbrec_chassis *chassis; /* May be NULL. */ + chassis = chassis_lookup_by_name(sbrec_chassis_by_name, + requested_chassis); + chassis = chassis ? chassis : chassis_lookup_by_hostname( + sbrec_chassis_by_hostname, requested_chassis); + + if (chassis) { + sbrec_port_binding_set_requested_chassis(op->sb, chassis); + } else { + reset_requested_chassis = true; + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT( + 1, 1); + VLOG_WARN_RL( + &rl, + "Unknown chassis '%s' set as " + "options:requested-chassis on LSP '%s'.", + requested_chassis, op->nbsp->name); + } + } else if (op->sb->requested_chassis) { + reset_requested_chassis = true; + } + if (reset_requested_chassis) { + sbrec_port_binding_set_requested_chassis(op->sb, NULL); + } } else { const char *chassis = NULL; if (op->peer && op->peer->od && op->peer->od->nbr) { @@ -3791,6 +3822,7 @@ ovn_port_allocate_key(struct hmap *ports, struct ovn_port *op) static void build_ports(struct northd_context *ctx, struct ovsdb_idl_index *sbrec_chassis_by_name, + struct ovsdb_idl_index *sbrec_chassis_by_hostname, struct hmap *datapaths, struct hmap *ports) { struct ovs_list sb_only, nb_only, both; @@ -3846,6 +3878,7 @@ build_ports(struct northd_context *ctx, tag_alloc_create_new_tag(&tag_alloc_table, op->nbsp); } ovn_port_update_sbrec(ctx, sbrec_chassis_by_name, + sbrec_chassis_by_hostname, op, &chassis_qdisc_queues, &active_ha_chassis_grps); } @@ -3853,7 +3886,8 @@ build_ports(struct northd_context *ctx, /* Add southbound record for each unmatched northbound record. */ LIST_FOR_EACH_SAFE (op, next, list, &nb_only) { op->sb = sbrec_port_binding_insert(ctx->ovnsb_txn); - ovn_port_update_sbrec(ctx, sbrec_chassis_by_name, op, + ovn_port_update_sbrec(ctx, sbrec_chassis_by_name, + sbrec_chassis_by_hostname, op, &chassis_qdisc_queues, &active_ha_chassis_grps); sbrec_port_binding_set_logical_port(op->sb, op->key); @@ -14208,6 +14242,7 @@ build_meter_groups(struct northd_context *ctx, static void ovnnb_db_run(struct northd_context *ctx, struct ovsdb_idl_index *sbrec_chassis_by_name, + struct ovsdb_idl_index *sbrec_chassis_by_hostname, struct ovsdb_idl_loop *sb_loop, struct hmap *datapaths, struct hmap *ports, struct ovs_list *lr_list, @@ -14309,7 +14344,8 @@ ovnnb_db_run(struct northd_context *ctx, build_datapaths(ctx, datapaths, lr_list); build_ovn_lbs(ctx, datapaths, &lbs); build_lrouter_lbs(datapaths, &lbs); - build_ports(ctx, sbrec_chassis_by_name, datapaths, ports); + build_ports(ctx, sbrec_chassis_by_name, sbrec_chassis_by_hostname, + datapaths, ports); build_ovn_lr_lbs(datapaths, &lbs); build_ovn_lb_svcs(ctx, ports, &lbs); build_ipam(datapaths, ports); @@ -14656,6 +14692,7 @@ ovnsb_db_run(struct northd_context *ctx, void ovn_db_run(struct northd_context *ctx, struct ovsdb_idl_index *sbrec_chassis_by_name, + struct ovsdb_idl_index *sbrec_chassis_by_hostname, struct ovsdb_idl_loop *ovnsb_idl_loop, const char *ovn_internal_version) { @@ -14668,8 +14705,8 @@ ovn_db_run(struct northd_context *ctx, int64_t start_time = time_wall_msec(); stopwatch_start(OVNNB_DB_RUN_STOPWATCH_NAME, time_msec()); - ovnnb_db_run(ctx, sbrec_chassis_by_name, ovnsb_idl_loop, - &datapaths, &ports, &lr_list, start_time, + ovnnb_db_run(ctx, sbrec_chassis_by_name, sbrec_chassis_by_hostname, + ovnsb_idl_loop, &datapaths, &ports, &lr_list, start_time, ovn_internal_version); stopwatch_stop(OVNNB_DB_RUN_STOPWATCH_NAME, time_msec()); stopwatch_start(OVNSB_DB_RUN_STOPWATCH_NAME, time_msec()); diff --git a/northd/northd.h b/northd/northd.h index ffa2bbb4e..d2a931ada 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -34,6 +34,7 @@ struct northd_context { void ovn_db_run(struct northd_context *ctx, struct ovsdb_idl_index *sbrec_chassis_by_name, + struct ovsdb_idl_index *sbrec_chassis_by_hostname, struct ovsdb_idl_loop *ovnsb_idl_loop, const char *ovn_internal_version); diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 39aa96055..9c33378fb 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -716,6 +716,8 @@ main(int argc, char *argv[]) add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_mac); add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_nat_addresses); + add_column_noalert(ovnsb_idl_loop.idl, + &sbrec_port_binding_col_requested_chassis); ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_port_binding_col_chassis); ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_port_binding_col_gateway_chassis); @@ -787,6 +789,7 @@ main(int argc, char *argv[]) ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_chassis); ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_chassis_col_name); + ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_chassis_col_hostname); ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_chassis_col_other_config); ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_chassis_col_encaps); @@ -895,6 +898,9 @@ main(int argc, char *argv[]) struct ovsdb_idl_index *sbrec_chassis_by_name = chassis_index_create(ovnsb_idl_loop.idl); + struct ovsdb_idl_index *sbrec_chassis_by_hostname + = chassis_hostname_index_create(ovnsb_idl_loop.idl); + struct ovsdb_idl_index *sbrec_ha_chassis_grp_by_name = ha_chassis_group_index_create(ovnsb_idl_loop.idl); @@ -975,7 +981,8 @@ main(int argc, char *argv[]) } if (ovsdb_idl_has_lock(ovnsb_idl_loop.idl)) { - ovn_db_run(&ctx, sbrec_chassis_by_name, &ovnsb_idl_loop, + ovn_db_run(&ctx, sbrec_chassis_by_name, + sbrec_chassis_by_hostname, &ovnsb_idl_loop, ovn_internal_version); if (ctx.ovnsb_txn) { check_and_add_supported_dhcp_opts_to_sb_db(&ctx); diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl index 22913f05a..e379623a6 100644 --- a/northd/ovn_northd.dl +++ b/northd/ovn_northd.dl @@ -105,6 +105,22 @@ sb::Out_Datapath_Binding(uuid, tunkey, load_balancers, external_ids) :- */ var load_balancers = set_empty(). +function get_requested_chassis(options: Map<istring,istring>) : istring = { + var requested_chassis = match(options.get(i"requested-chassis")) { + None -> i"", + Some{requested_chassis} -> requested_chassis, + }; + requested_chassis +} + +relation RequestedChassis( + name: istring, + chassis: uuid, +) +RequestedChassis(name, chassis) :- + sb::Chassis(._uuid = chassis, .name=name). +RequestedChassis(hostname, chassis) :- + sb::Chassis(._uuid = chassis, .hostname=hostname). /* Proxy table for Out_Datapath_Binding: contains all Datapath_Binding fields, * except tunnel id, which is allocated separately (see PortTunKeyAllocation). */ @@ -120,10 +136,95 @@ relation OutProxy_Port_Binding ( tag: Option<integer>, mac: Set<istring>, nat_addresses: Set<istring>, - external_ids: Map<istring,istring> + external_ids: Map<istring,istring>, + requested_chassis: Option<uuid> ) -/* Case 1: Create a Port_Binding per logical switch port that is not of type "router" */ +/* Case 1a: Create a Port_Binding per logical switch port that is not of type + * "router" */ +OutProxy_Port_Binding(._uuid = lsp._uuid, + .logical_port = lsp.name, + .__type = lsp.__type, + .gateway_chassis = set_empty(), + .ha_chassis_group = sp.hac_group_uuid, + .options = options, + .datapath = sw._uuid, + .parent_port = lsp.parent_name, + .tag = tag, + .mac = lsp.addresses, + .nat_addresses = set_empty(), + .external_ids = eids, + .requested_chassis = None) :- + sp in &SwitchPort(.lsp = lsp, .sw = sw), + SwitchPortNewDynamicTag(lsp._uuid, opt_tag), + var tag = match (opt_tag) { + None -> lsp.tag, + Some{t} -> Some{t} + }, + lsp.__type != i"router", + var chassis_name_or_hostname = get_requested_chassis(lsp.options), + chassis_name_or_hostname == i"", + var eids = { + var eids = lsp.external_ids; + match (lsp.external_ids.get(i"neutron:port_name")) { + None -> (), + Some{name} -> eids.insert(i"name", name) + }; + eids + }, + var options = { + var options = lsp.options; + if (sw.other_config.get(i"vlan-passthru") == Some{i"true"}) { + options.insert(i"vlan-passthru", i"true") + }; + options + }. + +/* Case 1b: Create a Port_Binding per logical switch port that is not of type + * "router" and has options "requested-chassis" pointing at chassis name or + * hostname. */ +OutProxy_Port_Binding(._uuid = lsp._uuid, + .logical_port = lsp.name, + .__type = lsp.__type, + .gateway_chassis = set_empty(), + .ha_chassis_group = sp.hac_group_uuid, + .options = options, + .datapath = sw._uuid, + .parent_port = lsp.parent_name, + .tag = tag, + .mac = lsp.addresses, + .nat_addresses = set_empty(), + .external_ids = eids, + .requested_chassis = Some{requested_chassis}) :- + sp in &SwitchPort(.lsp = lsp, .sw = sw), + SwitchPortNewDynamicTag(lsp._uuid, opt_tag), + var tag = match (opt_tag) { + None -> lsp.tag, + Some{t} -> Some{t} + }, + lsp.__type != i"router", + var chassis_name_or_hostname = get_requested_chassis(lsp.options), + chassis_name_or_hostname != i"", + RequestedChassis(chassis_name_or_hostname, requested_chassis), + var eids = { + var eids = lsp.external_ids; + match (lsp.external_ids.get(i"neutron:port_name")) { + None -> (), + Some{name} -> eids.insert(i"name", name) + }; + eids + }, + var options = { + var options = lsp.options; + if (sw.other_config.get(i"vlan-passthru") == Some{i"true"}) { + options.insert(i"vlan-passthru", i"true") + }; + options + }. + +/* Case 1c: Create a Port_Binding per logical switch port that is not of type + * "router" and has options "requested-chassis" pointing at non-existent + * chassis name or hostname. */ OutProxy_Port_Binding(._uuid = lsp._uuid, .logical_port = lsp.name, .__type = lsp.__type, @@ -135,7 +236,8 @@ OutProxy_Port_Binding(._uuid = lsp._uuid, .tag = tag, .mac = lsp.addresses, .nat_addresses = set_empty(), - .external_ids = eids) :- + .external_ids = eids, + .requested_chassis = None) :- sp in &SwitchPort(.lsp = lsp, .sw = sw), SwitchPortNewDynamicTag(lsp._uuid, opt_tag), var tag = match (opt_tag) { @@ -143,6 +245,9 @@ OutProxy_Port_Binding(._uuid = lsp._uuid, Some{t} -> Some{t} }, lsp.__type != i"router", + var chassis_name_or_hostname = get_requested_chassis(lsp.options), + chassis_name_or_hostname != i"", + not RequestedChassis(chassis_name_or_hostname, _), var eids = { var eids = lsp.external_ids; match (lsp.external_ids.get(i"neutron:port_name")) { @@ -186,7 +291,8 @@ OutProxy_Port_Binding(._uuid = lsp._uuid, .tag = None, .mac = lsp.addresses, .nat_addresses = nat_addresses, - .external_ids = eids) :- + .external_ids = eids, + .requested_chassis = None) :- SwitchPortLBIPs(.port = &SwitchPort{.lsp = lsp, .sw = sw, .peer = peer}, .lbips = lbips), var eids = { @@ -280,7 +386,8 @@ OutProxy_Port_Binding(._uuid = lrp._uuid, .tag = None, // always empty for router ports .mac = set_singleton(i"${lrp.mac} ${lrp.networks.map(ival).to_vec().join(\" \")}"), .nat_addresses = set_empty(), - .external_ids = lrp.external_ids) :- + .external_ids = lrp.external_ids, + .requested_chassis = None) :- rp in &RouterPort(.lrp = lrp, .router = router, .peer = peer), RouterPortRAOptionsComplete(lrp._uuid, options0), (var __type, var options1) = match (router.options.get(i"chassis")) { @@ -475,7 +582,8 @@ OutProxy_Port_Binding(._uuid = cr_lrp_uuid, .tag = None, //always empty for router ports .mac = set_singleton(i"${lrp.mac} ${lrp.networks.map(ival).to_vec().join(\" \")}"), .nat_addresses = set_empty(), - .external_ids = lrp.external_ids) :- + .external_ids = lrp.external_ids, + .requested_chassis = None) :- DistributedGatewayPort(lrp, lr_uuid, cr_lrp_uuid), DistributedGatewayPortHAChassisGroup(lrp, hacg_uuid), var redirect_type = match (lrp.options.get(i"redirect-type")) { @@ -520,7 +628,8 @@ sb::Out_Port_Binding(._uuid = pbinding._uuid, .mac = pbinding.mac, .nat_addresses = pbinding.nat_addresses, .external_ids = pbinding.external_ids, - .up = Some{up}) :- + .up = Some{up}, + .requested_chassis = pbinding.requested_chassis) :- pbinding in OutProxy_Port_Binding(), PortTunKeyAllocation(pbinding._uuid, tunkey), QueueIDAllocation(pbinding._uuid, qid), diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema index e5ab41db9..122614dd5 100644 --- a/ovn-sb.ovsschema +++ b/ovn-sb.ovsschema @@ -1,7 +1,7 @@ { "name": "OVN_Southbound", - "version": "20.20.0", - "cksum": "605270161 26670", + "version": "20.21.0", + "cksum": "2362446865 26963", "tables": { "SB_Global": { "columns": { @@ -232,7 +232,11 @@ "external_ids": {"type": {"key": "string", "value": "string", "min": 0, - "max": "unlimited"}}}, + "max": "unlimited"}}, + "requested_chassis": {"type": {"key": {"type": "uuid", + "refTable": "Chassis", + "refType": "weak"}, + "min": 0, "max": 1}}}, "indexes": [["datapath", "tunnel_key"], ["logical_port"]], "isRoot": true}, "MAC_Binding": { diff --git a/ovn-sb.xml b/ovn-sb.xml index 2d4d47d10..150051f26 100644 --- a/ovn-sb.xml +++ b/ovn-sb.xml @@ -2983,6 +2983,27 @@ tcp.flags = RST; </dd> </dl> </column> + <column name="requested_chassis"> + This column exists so that the ovn-controller can effectively monitor + all <ref table="Port_Binding"/> records destined for it, and is a + supplement to the <ref + table="Port_Binding" + column="options" + key="requested-chassis"/> option. The option is still required so that + the ovn-controller can check the CMS intent when the chassis pointed + to does not currently exist, which for example occurs when the + ovn-controller is stopped without passing the --restart argument. + + This column must be a + <ref table="Chassis"/> record. This is populated by + <code>ovn-northd</code> when the <ref + table="Logical_Switch_Port" + column="options" + key="requested-chassis" + db="OVN_Northbound"/> + is defined and contains a string matching the name or hostname of an + existing chassis. + </column> </group> <group title="Patch Options"> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 5de554455..258b58e98 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -5223,3 +5223,48 @@ AT_CHECK([grep lr_in_gw_redirect lrflows | grep cr-DR | sed 's/table=../table=?? AT_CLEANUP ]) + +OVN_FOR_EACH_NORTHD([ +AT_SETUP([check options:requested-chassis fills requested_chassis col]) +ovn_start NORTHD_TYPE + +# Add chassis ch1. +check ovn-sbctl chassis-add ch1 geneve 127.0.0.2 +check ovn-sbctl chassis-add ch2 geneve 127.0.0.3 + +wait_row_count Chassis 2 + +ch1_uuid=`ovn-sbctl --bare --columns _uuid find Chassis name="ch1"` +ch2_uuid=`ovn-sbctl --bare --columns _uuid find Chassis name="ch2"` + +check ovn-sbctl set chassis $ch2_uuid hostname=ch2-hostname + +ovn-nbctl ls-add S1 +ovn-nbctl --wait=sb lsp-add S1 S1-vm1 +ovn-nbctl --wait=sb lsp-add S1 S1-vm2 + +wait_row_count Port_Binding 1 logical_port=S1-vm1 requested_chassis!=$ch1_uuid +wait_row_count Port_Binding 1 logical_port=S1-vm2 requested_chassis!=$ch2_uuid + +ovn-nbctl --wait=sb set logical_switch_port S1-vm1 \ + options:requested-chassis=ch1 + +wait_row_count Port_Binding 1 logical_port=S1-vm1 requested_chassis=$ch1_uuid + +ovn-nbctl --wait=sb set logical_switch_port S1-vm2 \ + options:requested-chassis=ch2-hostname + +wait_row_count Port_binding 1 logical-port=S1-vm2 requested_chassis=$ch2_uuid + +ovn-nbctl --wait=sb remove logical_switch_port S1-vm2 \ + options requested-chassis=ch2-hostname + +wait_row_count Port_binding 1 logical-port=S1-vm2 requested_chassis!=$ch2_uuid + +ovn-nbctl --wait=sb set logical_switch_port S1-vm2 \ + options:requested-chassis=ch2 + +wait_row_count Port_binding 1 logical-port=S1-vm2 requested_chassis=$ch2_uuid + +AT_CLEANUP +])
To allow for ovn-controller to efficiently monitor all Port_Binding records destined to it, we add a new requested_chassis column with weakRef to the Chassis table. The ovn-controller can monitor this column and only process records for its chassis UUID before having claimed the port. northd will fill this column with UUID of Chassis referenced in Logical_Switch_Port options:requested-chassis by name or hostname. In a subsequent update to the controller we will improve the efficiency of the requested-chassis feature by using the new column instead of each chassis performing string comparison on this option for every Port_Binding record processed. Note that the CMS facing Northbound Logical_Switch_Port:options API remains the same. Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com> --- lib/chassis-index.c | 24 +++++++++ lib/chassis-index.h | 3 ++ northd/northd.c | 45 ++++++++++++++-- northd/northd.h | 1 + northd/ovn-northd.c | 9 +++- northd/ovn_northd.dl | 123 ++++++++++++++++++++++++++++++++++++++++--- ovn-sb.ovsschema | 10 ++-- ovn-sb.xml | 21 ++++++++ tests/ovn-northd.at | 45 ++++++++++++++++ 9 files changed, 266 insertions(+), 15 deletions(-)