From patchwork Fri Mar 22 10:26:01 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: solomon X-Patchwork-Id: 1061057 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; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="Jgwn5W/D"; 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 44Qfwm18w3z9sPP for ; Fri, 22 Mar 2019 21:27:19 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 312F11501; Fri, 22 Mar 2019 10:27:16 +0000 (UTC) X-Original-To: ovs-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 13CD814FE for ; Fri, 22 Mar 2019 10:26:08 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from mail-pg1-f195.google.com (mail-pg1-f195.google.com [209.85.215.195]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 87C9E148 for ; Fri, 22 Mar 2019 10:26:07 +0000 (UTC) Received: by mail-pg1-f195.google.com with SMTP id a22so1198164pgg.13 for ; Fri, 22 Mar 2019 03:26:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:subject:message-id:date:user-agent:mime-version :content-language:content-transfer-encoding; bh=JzAJ2OQ/i/VEk2OYqTkuJOQ8C8kaTODY+RIqLWA9ETg=; b=Jgwn5W/DaxQKC2FREjg0FXAnCrygNQFjFEci9FPPrSxsD0RlWI9ZSFgkvqXj7xpMy8 w+G5p7sn+rUXy8lZsk6GQ4OSTZsEYcPLshx2UzVrlGl8w+4yOJigKRUw3bGev0lJRFWC ZlQCv8HgIud1fmy2qWtZKQDC5cR3g/IlWaSXkY/H1ZdLcYf43FJ7boPztbmTtCsDvoEt Yv7lGhNeZ4IHguWYZIq5x+F7A5LCH8VvsghAKnv6Rr0vs1BQpkIUa0GswBPLowx9zE2o cIdlnajGi/KaLzw6aB9xHHbjVcMnWOC0Cb1ahoHCcuK3QDhTws5fIJuhLLejxi+E6AJI RfYg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:message-id:date:user-agent :mime-version:content-language:content-transfer-encoding; bh=JzAJ2OQ/i/VEk2OYqTkuJOQ8C8kaTODY+RIqLWA9ETg=; b=AbbusV3afxgW42meRCMvvNNXMcj8PjEyh1hkmH1HLH9bL8u/0GkAOPQw62pP2Q6oU/ m8pNF0QhNl0XKU7jFlLw1K7frRzE8hQl8b1jd/I7iSE471/MahEYOCBYQsWCqtPaf/kC a3MLaziu+PVAcTdRchW+rCbzDFZwBly6uThoWVbQ57G/Ue1hQSpkWg5NZEgOkw5jM7B1 cGl/oo8n6Hef5MVWlZwaIRRVudLlkdjP9aL4zEweFkXn90Cm7eih5PCtJXEXVPzsDoP0 eX7WZjUU/MYG90KEHbuFpUthqqgsdZLJHvKg3XL8QIh+VARGGqrOJMxlVLx6SikFAa0W Anjw== X-Gm-Message-State: APjAAAW9+2AzyrvtQTANDpAFsdAIe+EYBrZM1MZaZkT4pzysaXSPG7oB KveC85PSvXGHJpadZeQfoYP6zV73 X-Google-Smtp-Source: APXvYqwNeePr0pXOrGBvPxuFwwYa3C0qQ0OUCU92CP/TxGAnurf1ArNIj7/JNBRE3uDxh7kQekVgzA== X-Received: by 2002:a65:6091:: with SMTP id t17mr8081840pgu.328.1553250367000; Fri, 22 Mar 2019 03:26:07 -0700 (PDT) Received: from solomondeMacBook-Pro.local ([47.74.212.146]) by smtp.gmail.com with ESMTPSA id g13sm11877204pfm.159.2019.03.22.03.26.04 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 22 Mar 2019 03:26:05 -0700 (PDT) From: solomon X-Google-Original-From: solomon To: Ben Pfaff , ovs-dev@openvswitch.org Message-ID: <29ba1253-32cd-f33e-0c79-e16c17758924@gmail.com> Date: Fri, 22 Mar 2019 18:26:01 +0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:60.0) Gecko/20100101 Thunderbird/60.6.0 MIME-Version: 1.0 Content-Language: en-US X-Spam-Status: No, score=-1.7 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_ENVFROM_END_DIGIT,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 1/2] ovs: dump bucket id when doing dump-group-stats 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: , Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org The number of the buckets that are dumped start from 0 and is incremented. If the bucket in the group is adjusted, such as delete or added, the order will be disrupted. Then we can't know which bucket the output value corresponds to. So, also dump the bucket id when doing dump-group-stats command. Signed-off-by: solomon --- include/openflow/openflow-1.1.h | 4 +++- include/openvswitch/ofp-group.h | 8 +++++++- lib/ofp-group.c | 11 ++++++++--- ofproto/ofproto-dpif.c | 3 ++- tests/ofproto.at | 17 +++++++++++++++++ 5 files changed, 37 insertions(+), 6 deletions(-) diff --git a/include/openflow/openflow-1.1.h b/include/openflow/openflow-1.1.h index a29db8f3e..91b4dfd1b 100644 --- a/include/openflow/openflow-1.1.h +++ b/include/openflow/openflow-1.1.h @@ -531,10 +531,12 @@ OFP_ASSERT(sizeof(struct ofp11_group_stats_request) == 8); /* Used in group stats replies. */ struct ofp11_bucket_counter { + ovs_be32 bucket_id; + uint8_t pad2[4]; ovs_be64 packet_count; /* Number of packets processed by bucket. */ ovs_be64 byte_count; /* Number of bytes processed by bucket. */ }; -OFP_ASSERT(sizeof(struct ofp11_bucket_counter) == 16); +OFP_ASSERT(sizeof(struct ofp11_bucket_counter) == 24); /* Body of reply to OFPST11_GROUP request */ struct ofp11_group_stats { diff --git a/include/openvswitch/ofp-group.h b/include/openvswitch/ofp-group.h index cd7af0ebf..db36a2d3e 100644 --- a/include/openvswitch/ofp-group.h +++ b/include/openvswitch/ofp-group.h @@ -132,6 +132,12 @@ char *parse_ofp_group_mod_str(struct ofputil_group_mod *, int command, enum ofputil_protocol *usable_protocols) OVS_WARN_UNUSED_RESULT; +struct ofputil_bucket_counter { + uint32_t bucket_id; + uint64_t packet_count; /* Number of packets processed by bucket. */ + uint64_t byte_count; /* Number of bytes processed by bucket. */ +}; + /* Group stats reply, independent of protocol. */ struct ofputil_group_stats { uint32_t group_id; /* Group identifier. */ @@ -141,7 +147,7 @@ struct ofputil_group_stats { uint32_t duration_sec; /* UINT32_MAX if unknown. */ uint32_t duration_nsec; uint32_t n_buckets; - struct bucket_counter *bucket_stats; + struct ofputil_bucket_counter *bucket_stats; }; struct ofpbuf *ofputil_encode_group_stats_request(enum ofp_version, diff --git a/lib/ofp-group.c b/lib/ofp-group.c index 6857164f0..42e2f58dd 100644 --- a/lib/ofp-group.c +++ b/lib/ofp-group.c @@ -311,6 +311,7 @@ ofputil_group_bucket_counters_to_ofp11(const struct ofputil_group_stats *gs, int i; for (i = 0; i < gs->n_buckets; i++) { + bucket_cnts[i].bucket_id = htonl(gs->bucket_stats[i].bucket_id); bucket_cnts[i].packet_count = htonll(gs->bucket_stats[i].packet_count); bucket_cnts[i].byte_count = htonll(gs->bucket_stats[i].byte_count); } @@ -567,6 +568,7 @@ ofputil_decode_group_stats_reply(struct ofpbuf *msg, gs->bucket_stats = xmalloc(gs->n_buckets * sizeof *gs->bucket_stats); for (i = 0; i < gs->n_buckets; i++) { + gs->bucket_stats[i].bucket_id = ntohl(obc[i].bucket_id); gs->bucket_stats[i].packet_count = ntohll(obc[i].packet_count); gs->bucket_stats[i].byte_count = ntohll(obc[i].byte_count); } @@ -625,9 +627,12 @@ ofputil_group_stats_format(struct ds *s, const struct ofp_header *oh) for (uint32_t bucket_i = 0; bucket_i < gs.n_buckets; bucket_i++) { if (gs.bucket_stats[bucket_i].packet_count != UINT64_MAX) { - ds_put_format(s, ",bucket%"PRIu32":", bucket_i); - ds_put_format(s, "packet_count=%"PRIu64",", gs.bucket_stats[bucket_i].packet_count); - ds_put_format(s, "byte_count=%"PRIu64"", gs.bucket_stats[bucket_i].byte_count); + ds_put_format(s, ",bucket%"PRIu32":", + gs.bucket_stats[bucket_i].bucket_id); + ds_put_format(s, "packet_count=%"PRIu64",", + gs.bucket_stats[bucket_i].packet_count); + ds_put_format(s, "byte_count=%"PRIu64"", + gs.bucket_stats[bucket_i].byte_count); } } diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 0da0d0818..f48712e68 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -4955,9 +4955,10 @@ group_get_stats(const struct ofgroup *group_, struct ofputil_group_stats *ogs) ogs->packet_count = group->packet_count; ogs->byte_count = group->byte_count; - struct bucket_counter *bucket_stats = ogs->bucket_stats; + struct ofputil_bucket_counter *bucket_stats = ogs->bucket_stats; struct ofputil_bucket *bucket; LIST_FOR_EACH (bucket, list_node, &group->up.buckets) { + bucket_stats->bucket_id = bucket->bucket_id; bucket_stats->packet_count = bucket->stats.packet_count; bucket_stats->byte_count = bucket->stats.byte_count; bucket_stats++; diff --git a/tests/ofproto.at b/tests/ofproto.at index c5cebd9fe..da201c5f1 100644 --- a/tests/ofproto.at +++ b/tests/ofproto.at @@ -1273,6 +1273,23 @@ OFPST_GROUP reply (OF1.5): OVS_VSWITCHD_STOP AT_CLEANUP +dnl This is really bare-bones. +dnl It at least checks request and reply serialization and deserialization. +AT_SETUP([ofproto - group stats to check bucket id (OpenFlow 1.5)]) +OVS_VSWITCHD_START +AT_DATA([groups.txt], [dnl +group_id=1234,type=select,selection_method=hash bucket=bucket_id=1,weight:100,actions=output:10 +]) +AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn add-groups br0 groups.txt]) +AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn insert-buckets br0 'group_id=1234, command_bucket_id=last, bucket=bucket_id=8,weight:100,actions=output:10']) +AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn dump-group-stats br0 group_id=1234], [0], [stdout]) +AT_CHECK([strip_xids < stdout | sed 's/duration=[[0-9.]]*s/duration=?s/' | sort], [0], [dnl + group_id=1234,duration=?s,ref_count=0,packet_count=0,byte_count=0,bucket1:packet_count=0,byte_count=0,bucket8:packet_count=0,byte_count=0 +OFPST_GROUP reply (OF1.5): +]) +OVS_VSWITCHD_STOP +AT_CLEANUP + dnl This found a use-after-free error in bridge destruction in the dnl presence of groups. AT_SETUP([ofproto - group add then bridge delete (OpenFlow 1.3)])