From patchwork Tue Jan 30 15:56:07 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yifeng Sun X-Patchwork-Id: 867680 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=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="R5BYeak5"; dkim-atps=neutral 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 3zWJfZ5qn0z9sNw for ; Wed, 31 Jan 2018 07:59:50 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 6FE77D83; Tue, 30 Jan 2018 20:59:48 +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 B8881D3D for ; Tue, 30 Jan 2018 20:59:46 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from mail-wm0-f65.google.com (mail-wm0-f65.google.com [74.125.82.65]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 70F443A0 for ; Tue, 30 Jan 2018 20:59:45 +0000 (UTC) Received: by mail-wm0-f65.google.com with SMTP id r78so3885697wme.0 for ; Tue, 30 Jan 2018 12:59:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=E1NAJPvXmhJHSRHwjEVUz74Rxato0e0bbilIEDu/bMQ=; b=R5BYeak5WCfegzTdjfZU0rKoCTPhKqLeWRJTzbllQNLzKb5Gsd/A/Dl9rhQkxZ2gvH C62qF9JI9Am0xrMZAbeGgDNPcSfYw0x8ljj1o6Hsh1HgC2cpa+xTc9zYxIqo+8xAJB57 MaSeGpPkmzIbyk/vgYIUtNYpyrl2svOKu2YNdA/EAPZhWf6uFH/2GfJ7XKslM4OBsvNR R8beTT1hqA5S20pLiW61j0HoAP67opfHlnGFTZ3ZObjnDXBGAK4Hu6igBokUmJqgswlJ /NttIui4jgbISLsZacr32yNndqd+FFpPDJABTL8cm9AsHb6RFxgvy3ucwKhwUVAq+xMu wIBA== 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; bh=E1NAJPvXmhJHSRHwjEVUz74Rxato0e0bbilIEDu/bMQ=; b=UXTJ2u+UhqAHh0VlsvLzjfL8WDhdCnkn/MwyiJt0ZM3ikVXbt1Zldwd0RD4Zqg1Uni twpE5V5qC0l261dFdYrj3pxnP1L4OoRsQhcxfknTFXQBSEhQ3Q3oVBwizZqZV2WcmGFM hDowBpsDDsz2Z9nPUsmjPZHljh0q2fAkxpYPhYw9Duqw60L+fPYz3BX/NqHhHLv72/Sn r5TXIDhX0a46wDWdKmy+AuM0D3LdonpZKWsD94W343xZqeL2fzRmk9cIoYxrCMHQnfjn 1s0dBAUBkHVZIASAwi+fn39Cnp9AQbTcqgi00g8k38Uwltt6R/4XjOZyBfP7RW3xA56n Y/mw== X-Gm-Message-State: AKwxytekbsk+HR8MhB5OkZ+Z+t1JrRT5/EhjRkzadhXBQFze2fBn++Gh n9mU9JRQSIMsvDTUcsdrYZB7FA== X-Google-Smtp-Source: AH8x2259Qw6z1PacCcPDady+OwMgB9LvMHwc7g9h6v408kExoS3lCXxPdltyh2d2Rrc+uJ9fGC2anA== X-Received: by 10.80.152.14 with SMTP id g14mr52548144edb.160.1517345983825; Tue, 30 Jan 2018 12:59:43 -0800 (PST) Received: from yfsovs.eng.vmware.com ([208.91.1.34]) by smtp.gmail.com with ESMTPSA id k11sm7984091eda.22.2018.01.30.12.59.42 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 30 Jan 2018 12:59:43 -0800 (PST) From: Yifeng Sun To: dev@openvswitch.org Date: Tue, 30 Jan 2018 07:56:07 -0800 Message-Id: <1517327767-30406-1-git-send-email-pkusunyifeng@gmail.com> X-Mailer: git-send-email 2.7.4 X-Spam-Status: No, score=-0.4 required=5.0 tests=BAYES_00, DATE_IN_PAST_03_06, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE 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] classifier: refactor classifier_remove and introduce classifier_remove_assert 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 The return type of classifier_remove is changed to bool. Besides, classifier_remove_assert is introduced to assert that the classifier must contain the rule. This patch is based on Ben's advice. Signed-off-by: Yifeng Sun --- lib/classifier.c | 21 +++++++++++++++++---- lib/classifier.h | 6 ++++-- lib/ovs-router.c | 15 +++++---------- lib/tnl-ports.c | 5 ++--- ofproto/ofproto.c | 14 ++++---------- 5 files changed, 32 insertions(+), 29 deletions(-) diff --git a/lib/classifier.c b/lib/classifier.c index 16c451da1b30..4a4aacfd6208 100644 --- a/lib/classifier.c +++ b/lib/classifier.c @@ -701,9 +701,9 @@ classifier_insert(struct classifier *cls, const struct cls_rule *rule, * * Does nothing if 'rule' has been already removed, or was never inserted. * - * Returns the removed rule, or NULL, if it was already removed. + * Returns true on success, or false, if it was already removed. */ -const struct cls_rule * +bool classifier_remove(struct classifier *cls, const struct cls_rule *cls_rule) { struct cls_match *rule, *prev, *next, *head; @@ -716,7 +716,7 @@ classifier_remove(struct classifier *cls, const struct cls_rule *cls_rule) rule = get_cls_match_protected(cls_rule); if (!rule) { - return NULL; + return false; } /* Mark as removed. */ ovsrcu_set(&CONST_CAST(struct cls_rule *, cls_rule)->cls_match, NULL); @@ -820,7 +820,20 @@ check_priority: ovsrcu_postpone(cls_match_free_cb, rule); cls->n_rules--; - return cls_rule; + return true; +} + +/* Removes 'rule' from 'cls'. It is the caller's responsibility to destroy + * 'rule' with cls_rule_destroy(), freeing the memory block in which 'rule' + * resides, etc., as necessary. + * + * Asserts that the rule must be in the classifier. + */ +void +classifier_remove_assert(struct classifier *cls, + const struct cls_rule *cls_rule) +{ + ovs_assert(classifier_remove(cls, cls_rule)); } /* Prefix tree context. Valid when 'lookup_done' is true. Can skip all diff --git a/lib/classifier.h b/lib/classifier.h index f0ea5a9cb8b2..7699d58e1b07 100644 --- a/lib/classifier.h +++ b/lib/classifier.h @@ -387,8 +387,10 @@ const struct cls_rule *classifier_replace(struct classifier *, ovs_version_t, const struct cls_conjunction *, size_t n_conjunctions); -const struct cls_rule *classifier_remove(struct classifier *, - const struct cls_rule *); +bool classifier_remove(struct classifier *, + const struct cls_rule *); +void classifier_remove_assert(struct classifier *, + const struct cls_rule *); static inline void classifier_defer(struct classifier *); static inline void classifier_publish(struct classifier *); diff --git a/lib/ovs-router.c b/lib/ovs-router.c index cd2ab15fb003..a7d55c754d16 100644 --- a/lib/ovs-router.c +++ b/lib/ovs-router.c @@ -245,19 +245,15 @@ ovs_router_insert(uint32_t mark, const struct in6_addr *ip_dst, uint8_t plen, ovs_router_insert__(mark, plen, ip_dst, plen, output_bridge, gw); } -static bool +static void __rt_entry_delete(const struct cls_rule *cr) { struct ovs_router_entry *p = ovs_router_entry_cast(cr); tnl_port_map_delete_ipdev(p->output_bridge); /* Remove it. */ - cr = classifier_remove(&cls, cr); - if (cr) { - ovsrcu_postpone(rt_entry_free, ovs_router_entry_cast(cr)); - return true; - } - return false; + classifier_remove_assert(&cls, cr); + ovsrcu_postpone(rt_entry_free, p); } static bool @@ -267,7 +263,6 @@ rt_entry_delete(uint32_t mark, uint8_t priority, const struct cls_rule *cr; struct cls_rule rule; struct match match; - bool res = false; rt_init_match(&match, mark, ip6_dst, plen); @@ -277,12 +272,12 @@ rt_entry_delete(uint32_t mark, uint8_t priority, cr = classifier_find_rule_exactly(&cls, &rule, OVS_VERSION_MAX); if (cr) { ovs_mutex_lock(&mutex); - res = __rt_entry_delete(cr); + __rt_entry_delete(cr); ovs_mutex_unlock(&mutex); } cls_rule_destroy(&rule); - return res; + return (cr != NULL); } static bool diff --git a/lib/tnl-ports.c b/lib/tnl-ports.c index 04d2b3f7c6cf..b814f7a0a50a 100644 --- a/lib/tnl-ports.c +++ b/lib/tnl-ports.c @@ -223,9 +223,8 @@ tnl_port_unref(const struct cls_rule *cr) struct tnl_port_in *p = tnl_port_cast(cr); if (cr && ovs_refcount_unref_relaxed(&p->ref_cnt) == 1) { - if (classifier_remove(&cls, cr)) { - ovsrcu_postpone(tnl_port_free, p); - } + classifier_remove_assert(&cls, cr); + ovsrcu_postpone(tnl_port_free, p); } } diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 4f17f79d286f..536636393850 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -1520,10 +1520,8 @@ ofproto_rule_delete(struct ofproto *ofproto, struct rule *rule) /* Make sure there is no postponed removal of the rule. */ ovs_assert(cls_rule_visible_in_version(&rule->cr, OVS_VERSION_MAX)); - if (!classifier_remove(&rule->ofproto->tables[rule->table_id].cls, - &rule->cr)) { - OVS_NOT_REACHED(); - } + classifier_remove_assert(&rule->ofproto->tables[rule->table_id].cls, + &rule->cr); ofproto_rule_remove__(rule->ofproto, rule); if (ofproto->ofproto_class->rule_delete) { ofproto->ofproto_class->rule_delete(rule); @@ -2885,9 +2883,7 @@ remove_rule_rcu__(struct rule *rule) struct oftable *table = &ofproto->tables[rule->table_id]; ovs_assert(!cls_rule_visible_in_version(&rule->cr, OVS_VERSION_MAX)); - if (!classifier_remove(&table->cls, &rule->cr)) { - OVS_NOT_REACHED(); - } + classifier_remove_assert(&table->cls, &rule->cr); if (ofproto->ofproto_class->rule_delete) { ofproto->ofproto_class->rule_delete(rule); } @@ -5231,9 +5227,7 @@ replace_rule_revert(struct ofproto *ofproto, } /* Remove the new rule immediately. It was never visible to lookups. */ - if (!classifier_remove(&table->cls, &new_rule->cr)) { - OVS_NOT_REACHED(); - } + classifier_remove_assert(&table->cls, &new_rule->cr); ofproto_rule_remove__(ofproto, new_rule); ofproto_rule_unref(new_rule); }