From patchwork Fri Mar 31 20:42:02 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andy Zhou X-Patchwork-Id: 745801 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org 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 3vvtjy3hGgz9ryb for ; Sat, 1 Apr 2017 07:43:02 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 1E57AB4C; Fri, 31 Mar 2017 20:42:32 +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 9EBB6B1E for ; Fri, 31 Mar 2017 20:42:29 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from mail-pg0-f67.google.com (mail-pg0-f67.google.com [74.125.83.67]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 03471154 for ; Fri, 31 Mar 2017 20:42:28 +0000 (UTC) Received: by mail-pg0-f67.google.com with SMTP id g2so19697644pge.2 for ; Fri, 31 Mar 2017 13:42:28 -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=r1FY44k2BE7RnH3EloxdcqsPeIK/nQFRnJZR3W8kS8k=; b=YEP4GCucI4QeY4Oa/7TivHMch8ip0Zqos9syzj6xZ2T3U80NznJOwbN2egme2lKwqg yPxUWop6W19X07VNJ9ANg0V2URjLAitO0SuhXoMuA649iUYPhrwNkXLt4LAhoceoyzrn ZUWfb3ycieuJuzBSLgCimZGP5mfIN9Z2EYdzqnr7xnB0pa3RFoSUIPd3Kp2JotPtRp7H iOhlVSto4jwxwt6tDZo0iFaPse8RqyNwA4cLAXqSQqciRwNlFjIVF2mQd/RtQrUCX1j3 3E6GUyBCEzGQ0EU8ddz/5NrFmSwB7TgAcqd8yOfL4NyG3ptuTK6KxwajyZv9dCvBP13q NTEQ== X-Gm-Message-State: AFeK/H3vEiDaDqxc9/N0dTU5leOeKdr05lcjTuVcvgywnp21d5ezRRQv1aoRqKzJz7x2Aw== X-Received: by 10.98.22.71 with SMTP id 68mr4402514pfw.233.1490992948424; Fri, 31 Mar 2017 13:42:28 -0700 (PDT) Received: from centos.eng.vmware.com ([208.91.1.34]) by smtp.gmail.com with ESMTPSA id z62sm12183867pff.88.2017.03.31.13.42.27 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 31 Mar 2017 13:42:27 -0700 (PDT) From: Andy Zhou To: dev@openvswitch.org Date: Fri, 31 Mar 2017 13:42:02 -0700 Message-Id: <1490992923-28158-2-git-send-email-azhou@ovn.org> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1490992923-28158-1-git-send-email-azhou@ovn.org> References: <1490992923-28158-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] [PATCH 2/3] ofproto: Store meters into imap 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 Currently, meters are stored in a fixed pointer array. It is not very efficient since the controller, at least in theory, can pick any meter id (up to the limits to uint32_t), not necessarily within the lower end of a region, or in close range to each other. In particular, OFPM_SLOWPATH and OFPM_CONTROLLER meters are specified at the high region. Switching to use imap, so that ofproto layer does not restrict the number of meters that controller can add, nor does it care about the value of meter_id. Only datapth actually limits the number of meters ofproto layer can support. Signed-off-by: Andy Zhou --- ofproto/ofproto-provider.h | 8 ++- ofproto/ofproto.c | 125 +++++++++++++++++++++++++++------------------ 2 files changed, 79 insertions(+), 54 deletions(-) diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h index b7b12cdfd5f4..ccd574284f5d 100644 --- a/ofproto/ofproto-provider.h +++ b/ofproto/ofproto-provider.h @@ -38,6 +38,7 @@ #include "guarded-list.h" #include "heap.h" #include "hindex.h" +#include "imap.h" #include "object-collection.h" #include "ofproto/ofproto.h" #include "openvswitch/list.h" @@ -109,12 +110,9 @@ struct ofproto { /* List of expirable flows, in all flow tables. */ struct ovs_list expirable OVS_GUARDED_BY(ofproto_mutex); - /* Meter table. - * OpenFlow meters start at 1. To avoid confusion we leave the first - * pointer in the array un-used, and index directly with the OpenFlow - * meter_id. */ + /* Meter table. */ struct ofputil_meter_features meter_features; - struct meter **meters; /* 'meter_features.max_meter' + 1 pointers. */ + struct imap meters; /* uint32_t indexed 'struct meter *'. */ /* OpenFlow connections. */ struct connmgr *connmgr; diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 84ea95b0c2a2..e6d605549b07 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -281,7 +281,8 @@ static uint64_t pick_fallback_dpid(void); static void ofproto_destroy__(struct ofproto *); static void update_mtu(struct ofproto *, struct ofport *); static void update_mtu_ofproto(struct ofproto *); -static void meter_delete(struct ofproto *, uint32_t first, uint32_t last); +static void meter_delete(struct ofproto *, uint32_t); +static void meter_delete_all(struct ofproto *); static void meter_insert_rule(struct rule *); /* unixctl. */ @@ -566,8 +567,7 @@ ofproto_create(const char *datapath_name, const char *datapath_type, } else { memset(&ofproto->meter_features, 0, sizeof ofproto->meter_features); } - ofproto->meters = xzalloc((ofproto->meter_features.max_meters + 1) - * sizeof(struct meter *)); + imap_init(&ofproto->meters); /* Set the initial tables version. */ ofproto_bump_tables_version(ofproto); @@ -1635,11 +1635,9 @@ ofproto_destroy(struct ofproto *p, bool del) return; } - if (p->meters) { - meter_delete(p, 1, p->meter_features.max_meters); - p->meter_features.max_meters = 0; - free(p->meters); - p->meters = NULL; + if (!imap_is_empty(&p->meters)) { + meter_delete_all(p); + imap_destroy(&p->meters); } ofproto_flush__(p); @@ -6215,6 +6213,19 @@ struct meter { struct ofputil_meter_band *bands; }; +static struct meter * +ofproto_get_meter(const struct ofproto *ofproto, uint32_t index) +{ + return imap_get(&ofproto->meters, index); +} + +static void +ofproto_add_meter(struct ofproto *ofproto, uint32_t index, + struct meter *meter) +{ + imap_add(&ofproto->meters, index, meter); +} + /* * This is used in instruction validation at flow set-up time, to map * the OpenFlow meter ID to the corresponding datapath provider meter @@ -6226,7 +6237,7 @@ ofproto_fix_meter_action(const struct ofproto *ofproto, struct ofpact_meter *ma) { if (ma->meter_id && ma->meter_id <= ofproto->meter_features.max_meters) { - const struct meter *meter = ofproto->meters[ma->meter_id]; + const struct meter *meter = ofproto_get_meter(ofproto, ma->meter_id); if (meter && meter->provider_meter_id.uint32 != UINT32_MAX) { /* Update the action with the provider's meter ID, so that we @@ -6246,7 +6257,7 @@ meter_insert_rule(struct rule *rule) { const struct rule_actions *a = rule_get_actions(rule); uint32_t meter_id = ofpacts_get_meter(a->ofpacts, a->ofpacts_len); - struct meter *meter = rule->ofproto->meters[meter_id]; + struct meter *meter = ofproto_get_meter(rule->ofproto, meter_id); ovs_list_insert(&meter->rules, &rule->meter_list_node); } @@ -6279,31 +6290,50 @@ meter_create(const struct ofputil_meter_config *config, } static void -meter_delete(struct ofproto *ofproto, uint32_t first, uint32_t last) +meter_destroy(struct ofproto *ofproto, struct meter *meter) OVS_REQUIRES(ofproto_mutex) { - for (uint32_t mid = first; mid <= last; ++mid) { - struct meter *meter = ofproto->meters[mid]; - if (meter) { - /* First delete the rules that use this meter. */ - if (!ovs_list_is_empty(&meter->rules)) { - struct rule_collection rules; - struct rule *rule; - - rule_collection_init(&rules); + if (!meter) { + return; + } - LIST_FOR_EACH (rule, meter_list_node, &meter->rules) { - rule_collection_add(&rules, rule); - } - delete_flows__(&rules, OFPRR_METER_DELETE, NULL); - } + if (!ovs_list_is_empty(&meter->rules)) { + struct rule_collection rules; + struct rule *rule; - ofproto->meters[mid] = NULL; - ofproto->ofproto_class->meter_del(ofproto, - meter->provider_meter_id); - free(meter->bands); - free(meter); + rule_collection_init(&rules); + LIST_FOR_EACH (rule, meter_list_node, &meter->rules) { + rule_collection_add(&rules, rule); } + delete_flows__(&rules, OFPRR_METER_DELETE, NULL); + } + + ofproto->ofproto_class->meter_del(ofproto, meter->provider_meter_id); + free(meter->bands); + free(meter); +} + +static void +meter_delete(struct ofproto *ofproto, uint32_t meter_id) + OVS_REQUIRES(ofproto_mutex) +{ + struct meter *meter = ofproto_get_meter(ofproto, meter_id); + + imap_remove(&ofproto->meters, meter_id); + meter_destroy(ofproto, meter); +} + +static void +meter_delete_all(struct ofproto *ofproto) + OVS_REQUIRES(ofproto_mutex) +{ + struct imap_node *node, *next; + + IMAP_FOR_EACH_SAFE (node, next, &ofproto->meters) { + struct meter *meter = node->data; + + imap_remove_node(&ofproto->meters, node); + meter_destroy(ofproto, meter); } } @@ -6311,10 +6341,10 @@ static enum ofperr handle_add_meter(struct ofproto *ofproto, struct ofputil_meter_mod *mm) { ofproto_meter_id provider_meter_id = { UINT32_MAX }; - struct meter **meterp = &ofproto->meters[mm->meter.meter_id]; + struct meter *meter = ofproto_get_meter(ofproto, mm->meter.meter_id); enum ofperr error; - if (*meterp) { + if (meter) { return OFPERR_OFPMMFC_METER_EXISTS; } @@ -6322,7 +6352,8 @@ handle_add_meter(struct ofproto *ofproto, struct ofputil_meter_mod *mm) &mm->meter); if (!error) { ovs_assert(provider_meter_id.uint32 != UINT32_MAX); - *meterp = meter_create(&mm->meter, provider_meter_id); + meter = meter_create(&mm->meter, provider_meter_id); + ofproto_add_meter(ofproto, mm->meter.meter_id, meter); } return error; } @@ -6330,7 +6361,7 @@ handle_add_meter(struct ofproto *ofproto, struct ofputil_meter_mod *mm) static enum ofperr handle_modify_meter(struct ofproto *ofproto, struct ofputil_meter_mod *mm) { - struct meter *meter = ofproto->meters[mm->meter.meter_id]; + struct meter *meter = ofproto_get_meter(ofproto, mm->meter.meter_id); enum ofperr error; uint32_t provider_meter_id; @@ -6355,25 +6386,21 @@ handle_delete_meter(struct ofconn *ofconn, struct ofputil_meter_mod *mm) { struct ofproto *ofproto = ofconn_get_ofproto(ofconn); uint32_t meter_id = mm->meter.meter_id; - enum ofperr error = 0; - uint32_t first, last; + /* Delete the meters. */ + ovs_mutex_lock(&ofproto_mutex); if (meter_id == OFPM13_ALL) { - first = 1; - last = ofproto->meter_features.max_meters; - } else { - if (!meter_id || meter_id > ofproto->meter_features.max_meters) { - return 0; + meter_delete_all(ofproto); + } + else { + /* OpenFlow does not support Meter ID 0. */ + if (meter_id) { + meter_delete(ofproto, meter_id); } - first = last = meter_id; } - - /* Delete the meters. */ - ovs_mutex_lock(&ofproto_mutex); - meter_delete(ofproto, first, last); ovs_mutex_unlock(&ofproto_mutex); - return error; + return 0; } static enum ofperr @@ -6482,7 +6509,7 @@ handle_meter_request(struct ofconn *ofconn, const struct ofp_header *request, last = ofproto->meter_features.max_meters; } else { if (!meter_id || meter_id > ofproto->meter_features.max_meters || - !ofproto->meters[meter_id]) { + !ofproto_get_meter(ofproto, meter_id)) { return OFPERR_OFPMMFC_UNKNOWN_METER; } first = last = meter_id; @@ -6492,7 +6519,7 @@ handle_meter_request(struct ofconn *ofconn, const struct ofp_header *request, ofpmp_init(&replies, request); for (meter_id = first; meter_id <= last; ++meter_id) { - struct meter *meter = ofproto->meters[meter_id]; + struct meter *meter = ofproto_get_meter(ofproto, meter_id); if (!meter) { continue; /* Skip non-existing meters. */ }