Message ID | 20210903192748.1408062-2-frode.nordahl@canonical.com |
---|---|
State | Changes Requested |
Headers | show |
Series | Introduce infrastructure for plugging 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 | fail | github build: failed |
On Fri, Sep 3, 2021 at 3:28 PM Frode Nordahl <frode.nordahl@canonical.com> wrote: > > To allow for ovn-controller to efficiently process unbound ports > that may be destined to it, for example for use in the optional > plugging support, we add a new requested_chassis column with > weakRef to the Chassis table. The ovn-controller can monitor > this column and only process events for its chassis UUID even > before a local binding appears. Since this patch doesn't add the plugging support, maybe the commit message can avoid mentioning it ? > > northd will fill this column with UUID of Chassis referenced > in Logical_Switch_Port options:requested-chassis by name or > hostname. > > Deprecate the OVN_Southbound:Port_Binding:options > "requested-chassis" key. 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 > option processing and string comparison. > > Note that the CMS facing Northbound Logical_Switch_Port:options > API remains the same. > > Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com> Overall the patch looks good to me. I've a few minor comments. Please see below. > --- > lib/chassis-index.c | 24 +++++++++ > lib/chassis-index.h | 3 ++ > northd/ovn-northd.c | 55 +++++++++++++++++-- > northd/ovn_northd.dl | 124 ++++++++++++++++++++++++++++++++++++++++--- > ovn-nb.xml | 33 ++++++++++-- > ovn-sb.ovsschema | 10 ++-- > ovn-sb.xml | 37 ++++++++++--- > tests/ovn-northd.at | 45 ++++++++++++++++ > 8 files changed, 307 insertions(+), 24 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/ovn-northd.c b/northd/ovn-northd.c > index ee761cef0..fdcc58e28 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -76,6 +76,7 @@ struct northd_context { > struct ovsdb_idl_txn *ovnnb_txn; > struct ovsdb_idl_txn *ovnsb_txn; > struct ovsdb_idl_index *sbrec_chassis_by_name; > + struct ovsdb_idl_index *sbrec_chassis_by_hostname; > struct ovsdb_idl_index *sbrec_ha_chassis_grp_by_name; > struct ovsdb_idl_index *sbrec_mcast_group_by_name_dp; > struct ovsdb_idl_index *sbrec_ip_mcast_by_dp; > @@ -3047,6 +3048,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) > @@ -3228,6 +3230,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) { > @@ -3856,6 +3888,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; > @@ -3911,6 +3944,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); > } > @@ -3918,7 +3952,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); > @@ -14157,6 +14192,7 @@ get_probe_interval(const char *db, const struct nbrec_nb_global *nb) > 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, > @@ -14260,7 +14296,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); > @@ -14974,6 +15011,7 @@ ovnsb_db_run(struct northd_context *ctx, > static 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) > { > @@ -14985,8 +15023,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()); > @@ -15237,6 +15275,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); > @@ -15308,6 +15348,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); > > @@ -15416,6 +15457,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); > > @@ -15493,7 +15537,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 ff92c989c..85c62c547 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<string,string>) : string = { > + var requested_chassis = match(options.get("requested-chassis")) { > + None -> "", > + Some{requested_chassis} -> requested_chassis, > + }; > + requested_chassis > +} > + > +relation RequestedChassis( > + name: string, > + 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,12 @@ relation OutProxy_Port_Binding ( > tag: Option<integer>, > mac: Set<string>, > nat_addresses: Set<string>, > - external_ids: Map<string,string> > + external_ids: Map<string,string>, > + 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, > @@ -135,7 +153,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 +162,8 @@ OutProxy_Port_Binding(._uuid = lsp._uuid, > Some{t} -> Some{t} > }, > lsp.__type != "router", > + var chassis_name_or_hostname = get_requested_chassis(lsp.options), > + chassis_name_or_hostname == "", > var eids = { > var eids = lsp.external_ids; > match (lsp.external_ids.get("neutron:port_name")) { > @@ -160,6 +181,91 @@ OutProxy_Port_Binding(._uuid = lsp._uuid, > 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 != "router", > + var chassis_name_or_hostname = get_requested_chassis(lsp.options), > + chassis_name_or_hostname != "", > + RequestedChassis(chassis_name_or_hostname, requested_chassis), > + var eids = { > + var eids = lsp.external_ids; > + match (lsp.external_ids.get("neutron:port_name")) { > + None -> (), > + Some{name} -> eids.insert("name", name) > + }; > + eids > + }, > + var options = { > + var options = lsp.options; > + match (sw.other_config.get("vlan-passthru")) { > + Some{"true"} -> options.insert("vlan-passthru", "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, > + .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 != "router", > + var chassis_name_or_hostname = get_requested_chassis(lsp.options), > + chassis_name_or_hostname != "", > + not RequestedChassis(chassis_name_or_hostname, _), > + var eids = { > + var eids = lsp.external_ids; > + match (lsp.external_ids.get("neutron:port_name")) { > + None -> (), > + Some{name} -> eids.insert("name", name) > + }; > + eids > + }, > + var options = { > + var options = lsp.options; > + match (sw.other_config.get("vlan-passthru")) { > + Some{"true"} -> options.insert("vlan-passthru", "true"), > + _ -> () > + }; > + options > + }. > > /* Case 2: Create a Port_Binding per logical switch port of type "router" */ > OutProxy_Port_Binding(._uuid = lsp._uuid, > @@ -173,7 +279,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) :- > &SwitchPort(.lsp = lsp, .sw = sw, .peer = peer), > var eids = { > var eids = lsp.external_ids; > @@ -263,7 +370,8 @@ OutProxy_Port_Binding(._uuid = lrp._uuid, > .tag = None, // always empty for router ports > .mac = set_singleton("${lrp.mac} ${lrp.networks.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("chassis")) { > @@ -471,7 +579,8 @@ OutProxy_Port_Binding(// lrp._uuid is already in use; generate a new UUID by > .tag = None, //always empty for router ports > .mac = set_singleton("${lrp.mac} ${lrp.networks.join(\" \")}"), > .nat_addresses = set_empty(), > - .external_ids = lrp.external_ids) :- > + .external_ids = lrp.external_ids, > + .requested_chassis = None) :- > DistributedGatewayPort(lrp, lr_uuid), > DistributedGatewayPortHAChassisGroup(lrp, hacg_uuid), > var redirect_type = match (lrp.options.get("redirect-type")) { > @@ -516,7 +625,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-nb.xml b/ovn-nb.xml > index 390cc5a44..ef2677d94 100644 > --- a/ovn-nb.xml > +++ b/ovn-nb.xml > @@ -995,11 +995,16 @@ > > <column name="options" key="requested-chassis"> > If set, identifies a specific chassis (by name or hostname) that > - is allowed to bind this port. Using this option will prevent > + is allowed to bind or plug this port. Using this option will prevent > thrashing between two chassis trying to bind the same port during > - a live migration. It can also prevent similar thrashing due to a > + a live migration. It can also prevent similar thrashing due to a > mis-configuration, if a port is accidentally created on more than > - one chassis. > + one chassis. This is also used to allow the controller consider > + unbound ports for plugging without having to process ports not > + destined for its chassis. > + > + Setting this option is a prerequisite for using the > + <ref column="options" key="plug-type"/> option (see below). > </column> > > <column name="options" key="iface-id-ver"> > @@ -1028,6 +1033,28 @@ > DHCP reply. > </p> > </column> > + > + <group title="VIF Plugging Options"> > + <column name="options" key="plug-type"> > + If set, OVN will attempt to to perform plugging of this VIF. In s/attempt to to/attempt to > + order to get this port plugged by the OVN controller, OVN must be > + built with support for VIF plugging. The default behavior is for > + the CMS to do the VIF plugging. Each plug provider have their own > + options namespaced by name, for example "plug:representor:key". > + Please refer to the plug provider documentation for more > + information. > + > + Supported values: representor > + </column> > + > + <column name="options" key="plug-mtu-request"> > + Requested MTU for plugged interfaces. When set the OVN controller > + will fill the <ref table="Interface" column="mtu_request"/> column > + of the Open vSwitch database's > + <ref table="Interface" db="vswitch"/> table. This in turn will > + make OVS vswitchd update the MTU of the linked interface. > + </column> > + </group> I don't see the above 2 options referenced anywhere in the code. Does another patch in the series make use of it ? If so, I think this documentation part should be moved to the patch which makes use of these options. Thanks Numan > </group> > > <group title="Virtual port Options"> > 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..c966542e7 100644 > --- a/ovn-sb.xml > +++ b/ovn-sb.xml > @@ -2983,6 +2983,26 @@ tcp.flags = RST; > </dd> > </dl> > </column> > + <column name="requested_chassis"> > + If set, identifies a specific chassis that is allowed to bind or plug > + this port. Having a value in this column will prevent thrashing > + between two chassis trying to bind the same port during a live > + migration. > + It can also prevent similar thrashing due to a mis-configuration, if a > + port is accidentally created on more than one chassis. This is also > + used to allow the controller consider unbound ports for plugging > + without having to process ports not destined for its chassis. > + > + 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"> > @@ -3141,12 +3161,17 @@ tcp.flags = RST; > </p> > > <column name="options" key="requested-chassis"> > - If set, identifies a specific chassis (by name or hostname) that > - is allowed to bind this port. Using this option will prevent > - thrashing between two chassis trying to bind the same port during > - a live migration. It can also prevent similar thrashing due to a > - mis-configuration, if a port is accidentally created on more than > - one chassis. > + Deprecated. This option has been replaced by a separate column > + <ref > + table="Port_Binding" > + column="requested_chassis" > + db="OVN_Southbound"/> > + and may at some point in the future no longer be copied over from > + <ref > + table="Logical_Switch_Port" > + column="options" > + key="requested-chassis" > + db="OVN_Northbound"/>. > </column> > > <column name="options" key="iface-id-ver"> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 11886b94e..8ccf806c7 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -5225,3 +5225,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/ovn-northd.c b/northd/ovn-northd.c index ee761cef0..fdcc58e28 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -76,6 +76,7 @@ struct northd_context { struct ovsdb_idl_txn *ovnnb_txn; struct ovsdb_idl_txn *ovnsb_txn; struct ovsdb_idl_index *sbrec_chassis_by_name; + struct ovsdb_idl_index *sbrec_chassis_by_hostname; struct ovsdb_idl_index *sbrec_ha_chassis_grp_by_name; struct ovsdb_idl_index *sbrec_mcast_group_by_name_dp; struct ovsdb_idl_index *sbrec_ip_mcast_by_dp; @@ -3047,6 +3048,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) @@ -3228,6 +3230,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) { @@ -3856,6 +3888,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; @@ -3911,6 +3944,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); } @@ -3918,7 +3952,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); @@ -14157,6 +14192,7 @@ get_probe_interval(const char *db, const struct nbrec_nb_global *nb) 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, @@ -14260,7 +14296,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); @@ -14974,6 +15011,7 @@ ovnsb_db_run(struct northd_context *ctx, static 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) { @@ -14985,8 +15023,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()); @@ -15237,6 +15275,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); @@ -15308,6 +15348,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); @@ -15416,6 +15457,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); @@ -15493,7 +15537,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 ff92c989c..85c62c547 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<string,string>) : string = { + var requested_chassis = match(options.get("requested-chassis")) { + None -> "", + Some{requested_chassis} -> requested_chassis, + }; + requested_chassis +} + +relation RequestedChassis( + name: string, + 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,12 @@ relation OutProxy_Port_Binding ( tag: Option<integer>, mac: Set<string>, nat_addresses: Set<string>, - external_ids: Map<string,string> + external_ids: Map<string,string>, + 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, @@ -135,7 +153,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 +162,8 @@ OutProxy_Port_Binding(._uuid = lsp._uuid, Some{t} -> Some{t} }, lsp.__type != "router", + var chassis_name_or_hostname = get_requested_chassis(lsp.options), + chassis_name_or_hostname == "", var eids = { var eids = lsp.external_ids; match (lsp.external_ids.get("neutron:port_name")) { @@ -160,6 +181,91 @@ OutProxy_Port_Binding(._uuid = lsp._uuid, 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 != "router", + var chassis_name_or_hostname = get_requested_chassis(lsp.options), + chassis_name_or_hostname != "", + RequestedChassis(chassis_name_or_hostname, requested_chassis), + var eids = { + var eids = lsp.external_ids; + match (lsp.external_ids.get("neutron:port_name")) { + None -> (), + Some{name} -> eids.insert("name", name) + }; + eids + }, + var options = { + var options = lsp.options; + match (sw.other_config.get("vlan-passthru")) { + Some{"true"} -> options.insert("vlan-passthru", "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, + .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 != "router", + var chassis_name_or_hostname = get_requested_chassis(lsp.options), + chassis_name_or_hostname != "", + not RequestedChassis(chassis_name_or_hostname, _), + var eids = { + var eids = lsp.external_ids; + match (lsp.external_ids.get("neutron:port_name")) { + None -> (), + Some{name} -> eids.insert("name", name) + }; + eids + }, + var options = { + var options = lsp.options; + match (sw.other_config.get("vlan-passthru")) { + Some{"true"} -> options.insert("vlan-passthru", "true"), + _ -> () + }; + options + }. /* Case 2: Create a Port_Binding per logical switch port of type "router" */ OutProxy_Port_Binding(._uuid = lsp._uuid, @@ -173,7 +279,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) :- &SwitchPort(.lsp = lsp, .sw = sw, .peer = peer), var eids = { var eids = lsp.external_ids; @@ -263,7 +370,8 @@ OutProxy_Port_Binding(._uuid = lrp._uuid, .tag = None, // always empty for router ports .mac = set_singleton("${lrp.mac} ${lrp.networks.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("chassis")) { @@ -471,7 +579,8 @@ OutProxy_Port_Binding(// lrp._uuid is already in use; generate a new UUID by .tag = None, //always empty for router ports .mac = set_singleton("${lrp.mac} ${lrp.networks.join(\" \")}"), .nat_addresses = set_empty(), - .external_ids = lrp.external_ids) :- + .external_ids = lrp.external_ids, + .requested_chassis = None) :- DistributedGatewayPort(lrp, lr_uuid), DistributedGatewayPortHAChassisGroup(lrp, hacg_uuid), var redirect_type = match (lrp.options.get("redirect-type")) { @@ -516,7 +625,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-nb.xml b/ovn-nb.xml index 390cc5a44..ef2677d94 100644 --- a/ovn-nb.xml +++ b/ovn-nb.xml @@ -995,11 +995,16 @@ <column name="options" key="requested-chassis"> If set, identifies a specific chassis (by name or hostname) that - is allowed to bind this port. Using this option will prevent + is allowed to bind or plug this port. Using this option will prevent thrashing between two chassis trying to bind the same port during - a live migration. It can also prevent similar thrashing due to a + a live migration. It can also prevent similar thrashing due to a mis-configuration, if a port is accidentally created on more than - one chassis. + one chassis. This is also used to allow the controller consider + unbound ports for plugging without having to process ports not + destined for its chassis. + + Setting this option is a prerequisite for using the + <ref column="options" key="plug-type"/> option (see below). </column> <column name="options" key="iface-id-ver"> @@ -1028,6 +1033,28 @@ DHCP reply. </p> </column> + + <group title="VIF Plugging Options"> + <column name="options" key="plug-type"> + If set, OVN will attempt to to perform plugging of this VIF. In + order to get this port plugged by the OVN controller, OVN must be + built with support for VIF plugging. The default behavior is for + the CMS to do the VIF plugging. Each plug provider have their own + options namespaced by name, for example "plug:representor:key". + Please refer to the plug provider documentation for more + information. + + Supported values: representor + </column> + + <column name="options" key="plug-mtu-request"> + Requested MTU for plugged interfaces. When set the OVN controller + will fill the <ref table="Interface" column="mtu_request"/> column + of the Open vSwitch database's + <ref table="Interface" db="vswitch"/> table. This in turn will + make OVS vswitchd update the MTU of the linked interface. + </column> + </group> </group> <group title="Virtual port Options"> 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..c966542e7 100644 --- a/ovn-sb.xml +++ b/ovn-sb.xml @@ -2983,6 +2983,26 @@ tcp.flags = RST; </dd> </dl> </column> + <column name="requested_chassis"> + If set, identifies a specific chassis that is allowed to bind or plug + this port. Having a value in this column will prevent thrashing + between two chassis trying to bind the same port during a live + migration. + It can also prevent similar thrashing due to a mis-configuration, if a + port is accidentally created on more than one chassis. This is also + used to allow the controller consider unbound ports for plugging + without having to process ports not destined for its chassis. + + 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"> @@ -3141,12 +3161,17 @@ tcp.flags = RST; </p> <column name="options" key="requested-chassis"> - If set, identifies a specific chassis (by name or hostname) that - is allowed to bind this port. Using this option will prevent - thrashing between two chassis trying to bind the same port during - a live migration. It can also prevent similar thrashing due to a - mis-configuration, if a port is accidentally created on more than - one chassis. + Deprecated. This option has been replaced by a separate column + <ref + table="Port_Binding" + column="requested_chassis" + db="OVN_Southbound"/> + and may at some point in the future no longer be copied over from + <ref + table="Logical_Switch_Port" + column="options" + key="requested-chassis" + db="OVN_Northbound"/>. </column> <column name="options" key="iface-id-ver"> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 11886b94e..8ccf806c7 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -5225,3 +5225,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 process unbound ports that may be destined to it, for example for use in the optional plugging support, we add a new requested_chassis column with weakRef to the Chassis table. The ovn-controller can monitor this column and only process events for its chassis UUID even before a local binding appears. northd will fill this column with UUID of Chassis referenced in Logical_Switch_Port options:requested-chassis by name or hostname. Deprecate the OVN_Southbound:Port_Binding:options "requested-chassis" key. 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 option processing and string comparison. 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/ovn-northd.c | 55 +++++++++++++++++-- northd/ovn_northd.dl | 124 ++++++++++++++++++++++++++++++++++++++++--- ovn-nb.xml | 33 ++++++++++-- ovn-sb.ovsschema | 10 ++-- ovn-sb.xml | 37 ++++++++++--- tests/ovn-northd.at | 45 ++++++++++++++++ 8 files changed, 307 insertions(+), 24 deletions(-)