From patchwork Fri Jul 21 22:46:28 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andy Zhou X-Patchwork-Id: 792358 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3xDmCB0cXSz9s72 for ; Sat, 22 Jul 2017 08:48:38 +1000 (AEST) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 282CAB9E; Fri, 21 Jul 2017 22:47:27 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 0BF03B30 for ; Fri, 21 Jul 2017 22:47:26 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from mail-pg0-f65.google.com (mail-pg0-f65.google.com [74.125.83.65]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 2E18E17E for ; Fri, 21 Jul 2017 22:47:25 +0000 (UTC) Received: by mail-pg0-f65.google.com with SMTP id s4so6336635pgr.5 for ; Fri, 21 Jul 2017 15:47:25 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:content-transfer-encoding; bh=I9hn9ZEYW7DNg0cd44nLmMNwJkc943EdN6FUM7F8sbg=; b=pt1xJ4UepydIBJERwj/5jKMrQl6p1a4qAdk4BlV93ptMRDvc+CGGTOXd/p0p6pI9a6 beHgdulo486EjidMsyaWCV27ISbkznNawl5zL2mIwdWQk3+yeNhE+6Gqlzwhq8wTqNeZ 946ZgXm8i9J3N5qbpDaD9TQHNe/EkRHYQC7M4tVIpMgJ3xWSpTx6vJN3OCbDvB0me+qB gmn1ydZdlAzby8fk4TOWa7SRBazTxRzRDojUV+au9iCcDarFxczU/2abDp9qI1DstjcN Aa+LPw5zA/8PNBD3zPH2v1IJzOssVX/ApCQCmY64iR0wT//C/8Q5Q/ePSO7n5961LLqx uhLQ== X-Gm-Message-State: AIVw113hZHsy3jOIRCG8DjrveXTBNIcYTPAJtqWNzV88XgOIB1lBffSu w4vslFVLMQVbMqZ3 X-Received: by 10.84.136.1 with SMTP id 1mr9707633plk.74.1500677244523; Fri, 21 Jul 2017 15:47:24 -0700 (PDT) Received: from centos.eng.vmware.com ([208.91.1.34]) by smtp.gmail.com with ESMTPSA id q3sm10019886pgf.69.2017.07.21.15.47.23 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 21 Jul 2017 15:47:23 -0700 (PDT) From: Andy Zhou To: dev@openvswitch.org Date: Fri, 21 Jul 2017 15:46:28 -0700 Message-Id: <1500677193-28512-3-git-send-email-azhou@ovn.org> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1500677193-28512-1-git-send-email-azhou@ovn.org> References: <1500677193-28512-1-git-send-email-azhou@ovn.org> X-Spam-Status: No, score=-1.4 required=5.0 tests=BAYES_00,FREEMAIL_FROM, RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM autolearn=no version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [clone optmization v2 2/7] ofproto-dpif: Add boottime support field. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org When changing support fields, it may be unsafe to set support level beyond what datapath can support. This patch introduce the notion of boot time support and runtime support fields. Boot time support are set only once during ofproto start up phase, and not changed during runtime. The runtime support fields are the same as boot time support fields at the startup time, but can be changed via the 'ovs-appctl' command. However, each change will be checked against the corresponding boot time support field. Only feature reduction from the boot time support is allowed. Signed-off-by: Andy Zhou --- ofproto/bond.c | 2 +- ofproto/ofproto-dpif-upcall.c | 6 +- ofproto/ofproto-dpif-xlate.c | 12 ++++ ofproto/ofproto-dpif-xlate.h | 2 + ofproto/ofproto-dpif.c | 135 +++++++++++++++++++++++++++--------------- ofproto/ofproto-dpif.h | 9 ++- 6 files changed, 113 insertions(+), 53 deletions(-) diff --git a/ofproto/bond.c b/ofproto/bond.c index cb25a1df7369..65cbf7ed6200 100644 --- a/ofproto/bond.c +++ b/ofproto/bond.c @@ -1163,7 +1163,7 @@ bond_rebalance(struct bond *bond) } bond->next_rebalance = time_msec() + bond->rebalance_interval; - use_recirc = bond->ofproto->backer->support.odp.recirc && + use_recirc = bond->ofproto->backer->rt_support.odp.recirc && bond_may_recirc(bond); if (use_recirc) { diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index b2f9d91d2d9c..c5623d1d37ac 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -548,7 +548,7 @@ udpif_start_threads(struct udpif *udpif, size_t n_handlers, "handler", udpif_upcall_handler, handler); } - enable_ufid = udpif->backer->support.ufid; + enable_ufid = udpif->backer->rt_support.ufid; atomic_init(&udpif->enable_ufid, enable_ufid); dpif_enable_upcall(udpif->dpif); @@ -707,7 +707,7 @@ udpif_use_ufid(struct udpif *udpif) bool enable; atomic_read_relaxed(&enable_ufid, &enable); - return enable && udpif->backer->support.ufid; + return enable && udpif->backer->rt_support.ufid; } @@ -1543,7 +1543,7 @@ ukey_create_from_upcall(struct upcall *upcall, struct flow_wildcards *wc) .mask = wc ? &wc->masks : NULL, }; - odp_parms.support = upcall->ofproto->backer->support.odp; + odp_parms.support = upcall->ofproto->backer->rt_support.odp; if (upcall->key_len) { ofpbuf_use_const(&keybuf, upcall->key, upcall->key_len); } else { diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 7f7adb280eaf..44074b37320c 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -7002,3 +7002,15 @@ xlate_disable_dp_clone(const struct ofproto_dpif *ofproto) xbridge->support.clone = false; } } + +void +xlate_set_support(const struct ofproto_dpif *ofproto, + const struct dpif_backer_support *support) +{ + struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp); + struct xbridge *xbridge = xbridge_lookup(xcfg, ofproto); + + if (xbridge) { + xbridge->support = *support; + } +} diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h index 3de7dec8765d..916a15c67b5b 100644 --- a/ofproto/ofproto-dpif-xlate.h +++ b/ofproto/ofproto-dpif-xlate.h @@ -234,6 +234,8 @@ void xlate_mac_learning_update(const struct ofproto_dpif *ofproto, int vlan, bool is_grat_arp); void xlate_disable_dp_clone(const struct ofproto_dpif *); +void xlate_set_support(const struct ofproto_dpif *, + const struct dpif_backer_support *); void xlate_txn_start(void); void xlate_txn_commit(void); diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 28d1eb5c648d..ddf0d643bf7b 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -446,7 +446,7 @@ type_run(const char *type) ofproto->netflow, ofproto->up.forward_bpdu, connmgr_has_in_band(ofproto->up.connmgr), - &ofproto->backer->support); + &ofproto->backer->rt_support); HMAP_FOR_EACH (bundle, hmap_node, &ofproto->bundles) { xlate_bundle_set(ofproto, bundle, bundle->name, @@ -786,7 +786,7 @@ open_dpif_backer(const char *type, struct dpif_backer **backerp) /* This check fails if performed before udpif threads have been set, * as the kernel module checks that the 'pid' in userspace action * is non-zero. */ - backer->support.variable_length_userdata + backer->rt_support.variable_length_userdata = check_variable_length_userdata(backer); backer->dp_version_string = dpif_get_dp_version(backer->dpif); @@ -799,14 +799,21 @@ open_dpif_backer(const char *type, struct dpif_backer **backerp) backer->meter_ids = NULL; } + /* Make a pristine snapshot of 'support' into 'boottime_support'. + * 'boottime_support' can be checked to prevent 'support' to be changed + * beyond the datapath capabilities. In case 'support' is changed by + * the user, 'boottime_support' can be used to restore it. */ + backer->bt_support = backer->rt_support; + return error; } bool ovs_native_tunneling_is_on(struct ofproto_dpif *ofproto) { - return ofproto_use_tnl_push_pop && ofproto->backer->support.tnl_push_pop && - atomic_count_get(&ofproto->backer->tnl_count); + return ofproto_use_tnl_push_pop + && ofproto->backer->rt_support.tnl_push_pop + && atomic_count_get(&ofproto->backer->tnl_count); } /* Tests whether 'backer''s datapath supports recirculation. Only newer @@ -1366,29 +1373,29 @@ static void check_support(struct dpif_backer *backer) { /* This feature needs to be tested after udpif threads are set. */ - backer->support.variable_length_userdata = false; + backer->rt_support.variable_length_userdata = false; /* Actions. */ - backer->support.odp.recirc = check_recirc(backer); - backer->support.odp.max_vlan_headers = check_max_vlan_headers(backer); - backer->support.odp.max_mpls_depth = check_max_mpls_depth(backer); - backer->support.masked_set_action = check_masked_set_action(backer); - backer->support.trunc = check_trunc_action(backer); - backer->support.ufid = check_ufid(backer); - backer->support.tnl_push_pop = dpif_supports_tnl_push_pop(backer->dpif); - backer->support.clone = check_clone(backer); - backer->support.sample_nesting = check_max_sample_nesting(backer); - backer->support.ct_eventmask = check_ct_eventmask(backer); + backer->rt_support.odp.recirc = check_recirc(backer); + backer->rt_support.odp.max_vlan_headers = check_max_vlan_headers(backer); + backer->rt_support.odp.max_mpls_depth = check_max_mpls_depth(backer); + backer->rt_support.masked_set_action = check_masked_set_action(backer); + backer->rt_support.trunc = check_trunc_action(backer); + backer->rt_support.ufid = check_ufid(backer); + backer->rt_support.tnl_push_pop = dpif_supports_tnl_push_pop(backer->dpif); + backer->rt_support.clone = check_clone(backer); + backer->rt_support.sample_nesting = check_max_sample_nesting(backer); + backer->rt_support.ct_eventmask = check_ct_eventmask(backer); /* Flow fields. */ - backer->support.odp.ct_state = check_ct_state(backer); - backer->support.odp.ct_zone = check_ct_zone(backer); - backer->support.odp.ct_mark = check_ct_mark(backer); - backer->support.odp.ct_label = check_ct_label(backer); + backer->rt_support.odp.ct_state = check_ct_state(backer); + backer->rt_support.odp.ct_zone = check_ct_zone(backer); + backer->rt_support.odp.ct_mark = check_ct_mark(backer); + backer->rt_support.odp.ct_label = check_ct_label(backer); - backer->support.odp.ct_state_nat = check_ct_state_nat(backer); - backer->support.odp.ct_orig_tuple = check_ct_orig_tuple(backer); - backer->support.odp.ct_orig_tuple6 = check_ct_orig_tuple6(backer); + backer->rt_support.odp.ct_state_nat = check_ct_state_nat(backer); + backer->rt_support.odp.ct_orig_tuple = check_ct_orig_tuple(backer); + backer->rt_support.odp.ct_orig_tuple6 = check_ct_orig_tuple6(backer); } static int @@ -4259,7 +4266,7 @@ check_mask(struct ofproto_dpif *ofproto, const struct miniflow *flow) ovs_u128 ct_label; uint32_t ct_mark; - support = &ofproto->backer->support.odp; + support = &ofproto->backer->rt_support.odp; ct_state = MINIFLOW_GET_U8(flow, ct_state); /* Do not bother dissecting the flow further if the datapath supports all @@ -4319,7 +4326,7 @@ check_actions(const struct ofproto_dpif *ofproto, const struct rule_actions *const actions) { const struct ofpact *ofpact; - const struct odp_support *support = &ofproto->backer->support.odp; + const struct odp_support *support = &ofproto->backer->rt_support.odp; OFPACT_FOR_EACH (ofpact, actions->ofpacts, actions->ofpacts_len) { if (ofpact->type == OFPACT_CT) { @@ -5149,13 +5156,14 @@ enum dpif_support_field_type { }; struct dpif_support_field { - void *ptr; + void *rt_ptr; /* Points to the 'rt_support' field. */ + const void *bt_ptr; /* Points to the 'bt_support' field. */ const char *title; enum dpif_support_field_type type; }; -#define DPIF_SUPPORT_FIELD_INTIALIZER(PTR, TITLE, TYPE) \ - (struct dpif_support_field) {PTR, TITLE, TYPE} +#define DPIF_SUPPORT_FIELD_INTIALIZER(RT_PTR, BT_PTR, TITLE, TYPE) \ + (struct dpif_support_field) {RT_PTR, BT_PTR, TITLE, TYPE} static void dpif_show_support(const struct dpif_backer_support *support, struct ds *ds) @@ -5178,34 +5186,44 @@ display_support_field(const char *name, { switch (field->type) { case DPIF_SUPPORT_FIELD_bool: { - bool b = *(bool *)field->ptr; - ds_put_format(ds, "%s (%s) : %s\n", name, - field->title, b ? "true" : "false"); + bool v = *(bool *)field->rt_ptr; + bool b = *(bool *)field->bt_ptr; + ds_put_format(ds, "%s (%s) : [run time]:%s, [boot time]:%s\n", name, + field->title, v ? "true" : "false", + b ? "true" : "false"); break; } case DPIF_SUPPORT_FIELD_size_t: - ds_put_format(ds, "%s (%s) : %"PRIuSIZE"\n", name, - field->title, *(size_t *)field->ptr); + ds_put_format(ds, "%s (%s) : [run time]:%"PRIuSIZE + ", [boot time]:%"PRIuSIZE"\n", name, + field->title, *(size_t *)field->rt_ptr, + *(size_t *)field->bt_ptr); break; default: OVS_NOT_REACHED(); } } -static void -dpif_set_support(struct dpif_backer_support *support, +/* Set a field of 'rt_support' to a new value. + * + * Returns 'true' if the value is actually set. */ +static bool +dpif_set_support(struct dpif_backer_support *rt_support, + struct dpif_backer_support *bt_support, const char *name, const char *value, struct ds *ds) { struct shash all_fields = SHASH_INITIALIZER(&all_fields); struct dpif_support_field *field; struct shash_node *node; + bool changed = false; #define DPIF_SUPPORT_FIELD(TYPE, NAME, TITLE) \ {\ struct dpif_support_field *f = xmalloc(sizeof *f); \ - *f = DPIF_SUPPORT_FIELD_INTIALIZER(&support->NAME, \ - TITLE, \ - DPIF_SUPPORT_FIELD_##TYPE); \ + *f = DPIF_SUPPORT_FIELD_INTIALIZER(&rt_support->NAME, \ + &bt_support->NAME, \ + TITLE, \ + DPIF_SUPPORT_FIELD_##TYPE);\ shash_add_once(&all_fields, #NAME, f); \ } DPIF_SUPPORT_FIELDS; @@ -5214,9 +5232,10 @@ dpif_set_support(struct dpif_backer_support *support, #define ODP_SUPPORT_FIELD(TYPE, NAME, TITLE) \ {\ struct dpif_support_field *f = xmalloc(sizeof *f); \ - *f = DPIF_SUPPORT_FIELD_INTIALIZER(&support->odp.NAME, \ - TITLE, \ - DPIF_SUPPORT_FIELD_##TYPE); \ + *f = DPIF_SUPPORT_FIELD_INTIALIZER(&rt_support->odp.NAME, \ + &bt_support->odp.NAME, \ + TITLE, \ + DPIF_SUPPORT_FIELD_##TYPE);\ shash_add_once(&all_fields, #NAME, f); \ } ODP_SUPPORT_FIELDS; @@ -5244,10 +5263,16 @@ dpif_set_support(struct dpif_backer_support *support, } if (field->type == DPIF_SUPPORT_FIELD_bool) { - if (strcasecmp(value, "true") == 0) { - *(bool *)field->ptr = true; - } else if (strcasecmp(value, "false") == 0) { - *(bool *)field->ptr = false; + if (!strcasecmp(value, "true")) { + if (*(bool *)field->bt_ptr) { + *(bool *)field->rt_ptr = true; + changed = true; + } else { + ds_put_cstr(ds, "Can not enable features not supported by the datapth"); + } + } else if (!strcasecmp(value, "false")) { + *(bool *)field->rt_ptr = false; + changed = true; } else { ds_put_cstr(ds, "Boolean value expected"); } @@ -5255,7 +5280,12 @@ dpif_set_support(struct dpif_backer_support *support, int v; if (str_to_int(value, 10, &v)) { if (v >= 0) { - *(size_t *)field->ptr = v; + if (v <= *(size_t *)field->bt_ptr) { + *(size_t *)field->rt_ptr = v; + changed = true; + } else { + ds_put_cstr(ds, "Can not set value beyond the datapath capability"); + } } else { ds_put_format(ds, "Negative number not expected"); } @@ -5266,6 +5296,7 @@ dpif_set_support(struct dpif_backer_support *support, done: shash_destroy_free_data(&all_fields); + return changed; } static void @@ -5489,7 +5520,7 @@ disable_datapath_truncate(struct unixctl_conn *conn OVS_UNUSED, backers = shash_sort(&all_dpif_backers); for (i = 0; i < shash_count(&all_dpif_backers); i++) { struct dpif_backer *backer = backers[i]->data; - backer->support.trunc = false; + backer->rt_support.trunc = false; } free(backers); unixctl_command_reply(conn, "Datapath truncate action diabled"); @@ -5530,7 +5561,7 @@ ofproto_unixctl_dpif_show_dp_features(struct unixctl_conn *conn, return; } - dpif_show_support(&ofproto->backer->support, &ds); + dpif_show_support(&ofproto->backer->bt_support, &ds); unixctl_command_reply(conn, ds_cstr(&ds)); } @@ -5543,6 +5574,7 @@ ofproto_unixctl_dpif_set_dp_features(struct unixctl_conn *conn, const char *br = argv[1]; const char *name, *value; struct ofproto_dpif *ofproto = ofproto_dpif_lookup(br); + bool changed; if (!ofproto) { unixctl_command_reply_error(conn, "no such bridge"); @@ -5551,8 +5583,15 @@ ofproto_unixctl_dpif_set_dp_features(struct unixctl_conn *conn, name = argc > 2 ? argv[2] : NULL; value = argc > 3 ? argv[3] : NULL; - dpif_set_support(&ofproto->backer->support, name, value, &ds); + changed = dpif_set_support(&ofproto->backer->rt_support, + &ofproto->backer->bt_support, + name, value, &ds); + if (changed) { + xlate_set_support(ofproto, &ofproto->backer->rt_support); + udpif_flush(ofproto->backer->udpif); + } unixctl_command_reply(conn, ds_cstr(&ds)); + ds_destroy(&ds); } static void diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h index a3a6f1fab7da..0857c070c8ac 100644 --- a/ofproto/ofproto-dpif.h +++ b/ofproto/ofproto-dpif.h @@ -232,7 +232,14 @@ struct dpif_backer { char *dp_version_string; /* Datapath feature support. */ - struct dpif_backer_support support; + struct dpif_backer_support bt_support; /* Boot time support. Set once + when vswitch starts up, then + it is read only through out + the life time of vswitchd. */ + struct dpif_backer_support rt_support; /* Runtime support. Can be + set to a lower level in + feature than 'bt_support'. */ + struct atomic_count tnl_count; };