[{"id":1760467,"web_url":"http://patchwork.ozlabs.org/comment/1760467/","msgid":"","list_archive_url":null,"date":"2017-08-30T20:25:38","subject":"Re: [ovs-dev] [PATCH 2/2] monitor: Simplify calculation of\n\tcond->conditional.","submitter":{"id":67699,"url":"http://patchwork.ozlabs.org/api/people/67699/","name":"Andy Zhou","email":"azhou@ovn.org"},"content":"On Wed, Aug 30, 2017 at 9:33 AM, Ben Pfaff wrote:\n> This removes n_true_cnd from struct ovsdb_monitor_session_condition.\n> It was an \"optimization\" that is not part of any inner loop, but\n> make the code harder to reason about.\n>\n> Signed-off-by: Ben Pfaff \n\nThanks for the simplification.\nAcked-by: Andy Zhou ","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","ovs-dev@mail.linuxfoundation.org"],"Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","Received":["from mail.linuxfoundation.org (mail.linuxfoundation.org\n\t[140.211.169.12])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xjH8g6PRXz9sNw\n\tfor ;\n\tThu, 31 Aug 2017 06:26:27 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id A81D0AAB;\n\tWed, 30 Aug 2017 20:26:25 +0000 (UTC)","from smtp1.linuxfoundation.org (smtp1.linux-foundation.org\n\t[172.17.192.35])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPS id 05792890\n\tfor ; Wed, 30 Aug 2017 20:26:24 +0000 (UTC)","from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id 6E459489\n\tfor ; Wed, 30 Aug 2017 20:26:23 +0000 (UTC)","from mail-pg0-f51.google.com (mail-pg0-f51.google.com\n\t[74.125.83.51]) (Authenticated sender: azhou@ovn.org)\n\tby relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 8F797C5A51\n\tfor ; Wed, 30 Aug 2017 22:26:20 +0200 (CEST)","by mail-pg0-f51.google.com with SMTP id 83so22811605pgb.4\n\tfor ; Wed, 30 Aug 2017 13:26:20 -0700 (PDT)","by 10.100.163.195 with HTTP; Wed, 30 Aug 2017 13:25:38 -0700 (PDT)"],"X-Greylist":"domain auto-whitelisted by SQLgrey-1.7.6","X-Originating-IP":"74.125.83.51","X-Gm-Message-State":"AHYfb5jnuYotqClBbU38/SySFv+x3LFOQLUecslvZs/GaprXF6hOFtlT\n\tjcULSCEsWC5o096WIRNLZr441vbTtw==","X-Received":"by 10.98.224.66 with SMTP id f63mr2857611pfh.76.1504124779127;\n\tWed, 30 Aug 2017 13:26:19 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<20170830163314.1411-2-blp@ovn.org>","References":"<20170830163314.1411-1-blp@ovn.org>\n\t<20170830163314.1411-2-blp@ovn.org>","From":"Andy Zhou ","Date":"Wed, 30 Aug 2017 13:25:38 -0700","X-Gmail-Original-Message-ID":"","Message-ID":"","To":"Ben Pfaff ","X-Spam-Status":"No, score=-0.2 required=5.0 tests=RCVD_IN_DNSWL_LOW,\n\tRCVD_IN_SORBS_SPAM autolearn=disabled version=3.3.1","X-Spam-Checker-Version":"SpamAssassin 3.3.1 (2010-03-16) on\n\tsmtp1.linux-foundation.org","Cc":"\"\" ","Subject":"Re: [ovs-dev] [PATCH 2/2] monitor: Simplify calculation of\n\tcond->conditional.","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.12","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}},{"id":1760681,"web_url":"http://patchwork.ozlabs.org/comment/1760681/","msgid":"","list_archive_url":null,"date":"2017-08-31T06:50:08","subject":"Re: [ovs-dev] [PATCH 2/2] monitor: Simplify calculation\n\tofcond->conditional.","submitter":{"id":2836,"url":"http://patchwork.ozlabs.org/api/people/2836/","name":"Liran Schour","email":"lirans@il.ibm.com"},"content":"ovs-dev-bounces@openvswitch.org wrote on 30/08/2017 07:33:14 PM:\n\n> This removes n_true_cnd from struct ovsdb_monitor_session_condition.\n> It was an \"optimization\" that is not part of any inner loop, but\n> make the code harder to reason about.\n> \n> Signed-off-by: Ben Pfaff \n> ---\n> ovsdb/monitor.c | 28 ++++++++++++----------------\n> 1 file changed, 12 insertions(+), 16 deletions(-)\n> \n> diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c\n> index 7a5c2f905560..c0f9c557ab67 100644\n> --- a/ovsdb/monitor.c\n> +++ b/ovsdb/monitor.c\n> @@ -46,8 +46,7 @@ static struct hmap ovsdb_monitors = \n> HMAP_INITIALIZER(&ovsdb_monitors);\n> \n> /* Keep state of session's conditions */\n> struct ovsdb_monitor_session_condition {\n> - bool conditional;\n> - size_t n_true_cnd;\n> + bool conditional; /* True iff every table's condition is \ntrue. */\n> struct shash tables; /* Contains\n> * \"struct \n> ovsdb_monitor_table_condition *\"s. */\n> };\n> @@ -583,8 +582,17 @@ static inline void\n> ovsdb_monitor_session_condition_set_mode(\n> struct \n> ovsdb_monitor_session_condition *cond)\n> {\n> - cond->conditional = shash_count(&cond->tables) !=\n> - cond->n_true_cnd;\n> + struct shash_node *node;\n> +\n> + SHASH_FOR_EACH (node, &cond->tables) {\n> + struct ovsdb_monitor_table_condition *mtc = node->data;\n> +\n> + if (!ovsdb_condition_is_true(&mtc->new_condition)) {\n> + cond->conditional = true;\n> + return;\n> + }\n> + }\n> + cond->conditional = false;\n> }\n> \n> /* Returnes an empty allocated session's condition state holder */\n> @@ -649,9 +657,6 @@ ovsdb_monitor_table_condition_create(\n> shash_add(&condition->tables, table->schema->name, mtc);\n> /* On session startup old == new condition */\n> ovsdb_condition_clone(&mtc->new_condition, &mtc->old_condition);\n> - if (ovsdb_condition_is_true(&mtc->old_condition)) {\n> - condition->n_true_cnd++;\n> - }\n> ovsdb_monitor_session_condition_set_mode(condition);\n> \n> return NULL;\n> @@ -722,15 +727,6 @@ ovsdb_monitor_table_condition_updated(struct \n> ovsdb_monitor_table *mt,\n> /* If conditional monitoring - set old condition to new \ncondition */\n> if (ovsdb_condition_cmp_3way(&mtc->old_condition,\n> &mtc->new_condition)) {\n> - if (ovsdb_condition_is_true(&mtc->new_condition)) {\n> - if (!ovsdb_condition_is_true(&mtc->old_condition)) {\n> - condition->n_true_cnd++;\n> - }\n> - } else {\n> - if (ovsdb_condition_is_true(&mtc->old_condition)) {\n> - condition->n_true_cnd--;\n> - }\n> - }\n> ovsdb_condition_destroy(&mtc->old_condition);\n> ovsdb_condition_clone(&mtc->old_condition, \n&mtc->new_condition);\n> ovsdb_monitor_session_condition_set_mode(condition);\n> -- \n> 2.10.2\n> \n\nThanks for the cleanup. It is a forgotten leftover from a failed effort of \n\"optimization\" :)\n\nAcked-by: Liran Schour ","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","ovs-dev@mail.linuxfoundation.org"],"Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","Received":["from mail.linuxfoundation.org (mail.linuxfoundation.org\n\t[140.211.169.12])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xjY0T4pfSz9s7F\n\tfor ;\n\tThu, 31 Aug 2017 16:50:17 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id 0A6F2890;\n\tThu, 31 Aug 2017 06:50:16 +0000 (UTC)","from smtp1.linuxfoundation.org (smtp1.linux-foundation.org\n\t[172.17.192.35])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPS id 3CE3E504\n\tfor ; Thu, 31 Aug 2017 06:50:14 +0000 (UTC)","from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com\n\t[148.163.156.1])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id CECF787\n\tfor ; Thu, 31 Aug 2017 06:50:13 +0000 (UTC)","from pps.filterd (m0098396.ppops.net [127.0.0.1])\n\tby mx0a-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id\n\tv7V6nmMm132922\n\tfor ; Thu, 31 Aug 2017 02:50:13 -0400","from smtp.notes.na.collabserv.com (smtp.notes.na.collabserv.com\n\t[192.155.248.67])\n\tby mx0a-001b2d01.pphosted.com with ESMTP id 2cpc4g5wgm-1\n\t(version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=NOT)\n\tfor ; Thu, 31 Aug 2017 02:50:13 -0400","from localhost\n\tby smtp.notes.na.collabserv.com with smtp.notes.na.collabserv.com\n\tESMTP for from ;\n\tThu, 31 Aug 2017 06:50:10 -0000","from us1a3-smtp07.a3.dal06.isc4sb.com (10.146.103.14)\n\tby smtp.notes.na.collabserv.com (10.106.227.16) with\n\tsmtp.notes.na.collabserv.com ESMTP; Thu, 31 Aug 2017 06:50:08 -0000","from us1a3-mail116.a3.dal06.isc4sb.com ([10.146.45.125])\n\tby us1a3-smtp07.a3.dal06.isc4sb.com\n\twith ESMTP id 2017083106500852-110947 ;\n\tThu, 31 Aug 2017 06:50:08 +0000 "],"X-Greylist":"domain auto-whitelisted by SQLgrey-1.7.6","In-Reply-To":"<20170830163314.1411-2-blp@ovn.org>","To":"Ben Pfaff ","From":"\"Liran Schour\" ","Date":"Thu, 31 Aug 2017 09:50:08 +0300","References":"<20170830163314.1411-1-blp@ovn.org>\n\t<20170830163314.1411-2-blp@ovn.org>","MIME-Version":"1.0","X-KeepSent":"CF2B4A6A:1CA1F7A3-C225818D:00256448;\n type=4; name=$KeepSent","X-Mailer":"IBM Notes Release 9.0.1FP7 August 18, 2016","X-LLNOutbound":"False","X-Disclaimed":"16163","X-TNEFEvaluated":"1","x-cbid":"17083106-0327-0000-0000-000002C4336D","X-IBM-SpamModules-Scores":"FL=0; FP=0; FZ=0; HX=0; KW=0; PH=0; SC=0.49; ST=0; \n\tTS=0; UL=0; ISC=; MB=0.001326","X-IBM-SpamModules-Versions":"BY=3.00007640; HX=3.00000241; KW=3.00000007;\n\tPH=3.00000004; SC=3.00000226; SDB=6.00910086; UDB=6.00456504;\n\tIPR=6.00690366; \n\tBA=6.00005562; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009;\n\tZB=6.00000000; \n\tZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00016938;\n\tXFM=3.00000015; UTC=2017-08-31 06:50:10","X-IBM-AV-DETECTION":"SAVI=unsuspicious REMOTE=unsuspicious XFE=unused","X-IBM-AV-VERSION":"SAVI=2017-08-31 01:20:30 - 6.00007258","x-cbparentid":"17083106-0328-0000-0000-00001658646A","Message-Id":"","X-Proofpoint-Virus-Version":"vendor=fsecure engine=2.50.10432:, ,\n\tdefinitions=2017-08-31_01:, , signatures=0","X-Proofpoint-Spam-Reason":"safe","X-Spam-Status":"No, score=-0.2 required=5.0 tests=HTML_MESSAGE,\n\tRCVD_IN_DNSWL_LOW, RCVD_IN_SORBS_SPAM autolearn=disabled version=3.3.1","X-Spam-Checker-Version":"SpamAssassin 3.3.1 (2010-03-16) on\n\tsmtp1.linux-foundation.org","X-Content-Filtered-By":"Mailman/MimeDel 2.1.12","Cc":"dev@openvswitch.org, ovs-dev-bounces@openvswitch.org","Subject":"Re: [ovs-dev] [PATCH 2/2] monitor: Simplify calculation\n\tofcond->conditional.","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.12","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}},{"id":1761092,"web_url":"http://patchwork.ozlabs.org/comment/1761092/","msgid":"<20170831145632.GK6175@ovn.org>","list_archive_url":null,"date":"2017-08-31T14:56:32","subject":"Re: [ovs-dev] [PATCH 2/2] monitor: Simplify calculation\n\tofcond->conditional.","submitter":{"id":67603,"url":"http://patchwork.ozlabs.org/api/people/67603/","name":"Ben Pfaff","email":"blp@ovn.org"},"content":"On Thu, Aug 31, 2017 at 09:50:08AM +0300, Liran Schour wrote:\n> ovs-dev-bounces@openvswitch.org wrote on 30/08/2017 07:33:14 PM:\n> \n> > This removes n_true_cnd from struct ovsdb_monitor_session_condition.\n> > It was an \"optimization\" that is not part of any inner loop, but\n> > make the code harder to reason about.\n> > \n> > Signed-off-by: Ben Pfaff \n> > ---\n> > ovsdb/monitor.c | 28 ++++++++++++----------------\n> > 1 file changed, 12 insertions(+), 16 deletions(-)\n> > \n> > diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c\n> > index 7a5c2f905560..c0f9c557ab67 100644\n> > --- a/ovsdb/monitor.c\n> > +++ b/ovsdb/monitor.c\n> > @@ -46,8 +46,7 @@ static struct hmap ovsdb_monitors = \n> > HMAP_INITIALIZER(&ovsdb_monitors);\n> > \n> > /* Keep state of session's conditions */\n> > struct ovsdb_monitor_session_condition {\n> > - bool conditional;\n> > - size_t n_true_cnd;\n> > + bool conditional; /* True iff every table's condition is \n> true. */\n> > struct shash tables; /* Contains\n> > * \"struct \n> > ovsdb_monitor_table_condition *\"s. */\n> > };\n> > @@ -583,8 +582,17 @@ static inline void\n> > ovsdb_monitor_session_condition_set_mode(\n> > struct \n> > ovsdb_monitor_session_condition *cond)\n> > {\n> > - cond->conditional = shash_count(&cond->tables) !=\n> > - cond->n_true_cnd;\n> > + struct shash_node *node;\n> > +\n> > + SHASH_FOR_EACH (node, &cond->tables) {\n> > + struct ovsdb_monitor_table_condition *mtc = node->data;\n> > +\n> > + if (!ovsdb_condition_is_true(&mtc->new_condition)) {\n> > + cond->conditional = true;\n> > + return;\n> > + }\n> > + }\n> > + cond->conditional = false;\n> > }\n> > \n> > /* Returnes an empty allocated session's condition state holder */\n> > @@ -649,9 +657,6 @@ ovsdb_monitor_table_condition_create(\n> > shash_add(&condition->tables, table->schema->name, mtc);\n> > /* On session startup old == new condition */\n> > ovsdb_condition_clone(&mtc->new_condition, &mtc->old_condition);\n> > - if (ovsdb_condition_is_true(&mtc->old_condition)) {\n> > - condition->n_true_cnd++;\n> > - }\n> > ovsdb_monitor_session_condition_set_mode(condition);\n> > \n> > return NULL;\n> > @@ -722,15 +727,6 @@ ovsdb_monitor_table_condition_updated(struct \n> > ovsdb_monitor_table *mt,\n> > /* If conditional monitoring - set old condition to new \n> condition */\n> > if (ovsdb_condition_cmp_3way(&mtc->old_condition,\n> > &mtc->new_condition)) {\n> > - if (ovsdb_condition_is_true(&mtc->new_condition)) {\n> > - if (!ovsdb_condition_is_true(&mtc->old_condition)) {\n> > - condition->n_true_cnd++;\n> > - }\n> > - } else {\n> > - if (ovsdb_condition_is_true(&mtc->old_condition)) {\n> > - condition->n_true_cnd--;\n> > - }\n> > - }\n> > ovsdb_condition_destroy(&mtc->old_condition);\n> > ovsdb_condition_clone(&mtc->old_condition, \n> &mtc->new_condition);\n> > ovsdb_monitor_session_condition_set_mode(condition);\n> > -- \n> > 2.10.2\n> > \n> \n> Thanks for the cleanup. It is a forgotten leftover from a failed effort of \n> \"optimization\" :)\n> \n> Acked-by: Liran Schour \n\nThanks Andy and Liran. I applied this to master.","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","ovs-dev@mail.linuxfoundation.org"],"Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=openvswitch.org\n\t(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;\n\tenvelope-from=ovs-dev-bounces@openvswitch.org;\n\treceiver=)","Received":["from mail.linuxfoundation.org (mail.linuxfoundation.org\n\t[140.211.169.12])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xjlnz3V0Xz9s75\n\tfor ;\n\tFri, 1 Sep 2017 00:56:55 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id 38AD6AF4;\n\tThu, 31 Aug 2017 14:56:43 +0000 (UTC)","from smtp1.linuxfoundation.org (smtp1.linux-foundation.org\n\t[172.17.192.35])\n\tby mail.linuxfoundation.org (Postfix) with ESMTPS id BBBB1A84\n\tfor ; Thu, 31 Aug 2017 14:56:41 +0000 (UTC)","from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id 5994A42E\n\tfor ; Thu, 31 Aug 2017 14:56:41 +0000 (UTC)","from ovn.org (173-228-112-34.dsl.dynamic.fusionbroadband.com\n\t[173.228.112.34]) (Authenticated sender: blp@ovn.org)\n\tby relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 6FB6BC5A69;\n\tThu, 31 Aug 2017 16:56:36 +0200 (CEST)"],"X-Greylist":"domain auto-whitelisted by SQLgrey-1.7.6","X-Originating-IP":"173.228.112.34","Date":"Thu, 31 Aug 2017 07:56:32 -0700","From":"Ben Pfaff ","To":"Liran Schour , Andy Zhou ","Message-ID":"<20170831145632.GK6175@ovn.org>","References":"<20170830163314.1411-1-blp@ovn.org>\n\t<20170830163314.1411-2-blp@ovn.org>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","User-Agent":"Mutt/1.5.23 (2014-03-12)","X-Spam-Status":"No, score=-0.7 required=5.0 tests=RCVD_IN_DNSWL_LOW\n\tautolearn=disabled version=3.3.1","X-Spam-Checker-Version":"SpamAssassin 3.3.1 (2010-03-16) on\n\tsmtp1.linux-foundation.org","Cc":"dev@openvswitch.org","Subject":"Re: [ovs-dev] [PATCH 2/2] monitor: Simplify calculation\n\tofcond->conditional.","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.12","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}}]