[{"id":1770536,"web_url":"http://patchwork.ozlabs.org/comment/1770536/","msgid":"","list_archive_url":null,"date":"2017-09-18T21:56:35","subject":"Re: [ovs-dev] [PATCH] ofproto-dpif-upcall: Transition ukey on\n\tdp_ops error.","submitter":{"id":69140,"url":"http://patchwork.ozlabs.org/api/people/69140/","name":"Gregory Rose","email":"gvrose8192@gmail.com"},"content":"On 09/06/2017 03:12 PM, Joe Stringer wrote:\n> In most situations, we don't expect that a flow we've successfully\n> dumped, which we intend to delete, cannot be deleted. However, to make\n> this code more resilient to ensure that ukeys *will* transition in all\n> cases (including an error at this stage), grab the lock and transition\n> this ukey forward to the evicted state, effectively treating a failure\n> to delete as \"this flow is already gone\".\n> \n> If we subsequently find out that it wasn't deleted, then that's ok - we\n> will re-dump, and validate at that stage, which should lead to creating\n> a new ukey or deleting the datapath flow when that happens.\n> \n> Signed-off-by: Joe Stringer \n> ---\n> ofproto/ofproto-dpif-upcall.c | 5 +++++\n> 1 file changed, 5 insertions(+)\n> \n> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c\n> index 4a71bbe258df..bd324fbb6323 100644\n> --- a/ofproto/ofproto-dpif-upcall.c\n> +++ b/ofproto/ofproto-dpif-upcall.c\n> @@ -2227,6 +2227,11 @@ push_dp_ops(struct udpif *udpif, struct ukey_op *ops, size_t n_ops)\n> \n> if (op->dop.error) {\n> /* flow_del error, 'stats' is unusable. */\n> + if (op->ukey) {\n> + ovs_mutex_lock(&op->ukey->mutex);\n> + transition_ukey(op->ukey, UKEY_EVICTED);\n> + ovs_mutex_unlock(&op->ukey->mutex);\n> + }\n> continue;\n> }\n> \n> \n\nCompile tested only - I didn't see of any good way to force the error\n\nCode looks good to me.\n\nReviewed-by: Greg Rose ","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=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"Rre8ZGDa\"; dkim-atps=neutral"],"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 3xx0G32M4mz9s7G\n\tfor ;\n\tTue, 19 Sep 2017 07:56:43 +1000 (AEST)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id 0685CAAE;\n\tMon, 18 Sep 2017 21:56:40 +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 E58FAAA5\n\tfor ; Mon, 18 Sep 2017 21:56:38 +0000 (UTC)","from mail-pg0-f66.google.com (mail-pg0-f66.google.com\n\t[74.125.83.66])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id 85CBB271\n\tfor ; Mon, 18 Sep 2017 21:56:38 +0000 (UTC)","by mail-pg0-f66.google.com with SMTP id u18so810533pgo.1\n\tfor ; Mon, 18 Sep 2017 14:56:38 -0700 (PDT)","from gizo.bigblue.kilchis.com (67-5-132-83.ptld.qwest.net.\n\t[67.5.132.83]) by smtp.gmail.com with ESMTPSA id\n\t20sm417033pft.109.2017.09.18.14.56.35\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tMon, 18 Sep 2017 14:56:36 -0700 (PDT)"],"X-Greylist":"whitelisted by SQLgrey-1.7.6","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=subject:to:references:from:message-id:date:user-agent:mime-version\n\t:in-reply-to:content-language:content-transfer-encoding;\n\tbh=YNN8Q42xEBKuVvWlQWKe/7OgwB+JC4S1QuUMBGXGMpM=;\n\tb=Rre8ZGDa9gnLnx2UhPYK78UtYJOFhNNm19YIj6l42FCZCYXiEWlLomkcdpnQEFY+mr\n\tv7CXRMoUlCfLFdG4DN+K+UM7OuwpQcT0lWyDrI7pB85SVuCRMUEhUph1Qrbng1fThmYI\n\tnqgqa71ppp6xHFTsxHrv13RHQ1U+oX2l+6M2KxP2bnsv+CJcO1srDS5Bnjc53FzsVoje\n\tiRPOyAoukLt0M48plJPzKAKpEcS0/1ftJyZF6vENztWpp6QjSc8EIxbRu/IJxFDDYFnz\n\tVhW1xqkAUX1Qt/AieoXtmln5lyKGo5kl9zB4a5ZjgFm5dlgGnz1v0qUlQ5cmihRuIKHw\n\t1mBA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:subject:to:references:from:message-id:date\n\t:user-agent:mime-version:in-reply-to:content-language\n\t:content-transfer-encoding;\n\tbh=YNN8Q42xEBKuVvWlQWKe/7OgwB+JC4S1QuUMBGXGMpM=;\n\tb=hodyLq/yuvv1FyhsIQ3vw13Nq+lKCo7ZJlA2nI9m4+UXCSqckBPGTmDhlemYxbSrUv\n\tflzp8XMir6NjwR5DiTvufSnHDTxPNPmLp3rwEKI4VWG3JzAy3Jisv3Nn6zVTfWgnmCer\n\ty8GBOzmxeaZIH2XBPG1pdpyC/6mf3GjOfN7NLnA8PlZoCGWTDfevPswE979kqlaPmCAj\n\t4IHlZ1iHWXKaFTJT/0VJW0zih88J7Lqp+3w8EQtWFpdAu+zt4Ndz1hFZWH4hNVdG9DiY\n\tpkD+VZto/a4Mgb77Vp4JMakIXG6wLPwQBsmJDqebS8CvY7bv8TU+qZBGueY8CfO4IfN6\n\tYR2A==","X-Gm-Message-State":"AHPjjUiHWSIRApyZ27S7sp1PDo1G1A+qFKdgBt+sy/qCt0WwIwuTh1X7\n\tOKWcTM5tUOx1MZDhcdQ=","X-Google-Smtp-Source":"AOwi7QC+pGZUZJZ27cB0LsNVrqELxcon2kJ8b0c024cmWZjySmMs16R451FrmRf9Q6D5FmVpz41BdQ==","X-Received":"by 10.98.192.132 with SMTP id g4mr26227pfk.34.1505771797791;\n\tMon, 18 Sep 2017 14:56:37 -0700 (PDT)","To":"Joe Stringer , dev@openvswitch.org","References":"<20170906221252.17257-1-joe@ovn.org>","From":"Greg Rose ","Message-ID":"","Date":"Mon, 18 Sep 2017 14:56:35 -0700","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.1.0","MIME-Version":"1.0","In-Reply-To":"<20170906221252.17257-1-joe@ovn.org>","Content-Language":"en-US","X-Spam-Status":"No, score=0.7 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,\n\tDKIM_VALID_AU, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM,\n\tRCVD_IN_DNSWL_NONE, \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","Subject":"Re: [ovs-dev] [PATCH] ofproto-dpif-upcall: Transition ukey on\n\tdp_ops error.","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-Transfer-Encoding":"7bit","Content-Type":"text/plain; charset=\"us-ascii\"; Format=\"flowed\"","Sender":"ovs-dev-bounces@openvswitch.org","Errors-To":"ovs-dev-bounces@openvswitch.org"}},{"id":1798307,"web_url":"http://patchwork.ozlabs.org/comment/1798307/","msgid":"<20171102220207.GV27530@ovn.org>","list_archive_url":null,"date":"2017-11-02T22:02:07","subject":"Re: [ovs-dev] [PATCH] ofproto-dpif-upcall: Transition ukey on\n\tdp_ops error.","submitter":{"id":67603,"url":"http://patchwork.ozlabs.org/api/people/67603/","name":"Ben Pfaff","email":"blp@ovn.org"},"content":"On Mon, Sep 18, 2017 at 02:56:35PM -0700, Greg Rose wrote:\n> On 09/06/2017 03:12 PM, Joe Stringer wrote:\n> >In most situations, we don't expect that a flow we've successfully\n> >dumped, which we intend to delete, cannot be deleted. However, to make\n> >this code more resilient to ensure that ukeys *will* transition in all\n> >cases (including an error at this stage), grab the lock and transition\n> >this ukey forward to the evicted state, effectively treating a failure\n> >to delete as \"this flow is already gone\".\n> >\n> >If we subsequently find out that it wasn't deleted, then that's ok - we\n> >will re-dump, and validate at that stage, which should lead to creating\n> >a new ukey or deleting the datapath flow when that happens.\n> >\n> >Signed-off-by: Joe Stringer \n> >---\n> > ofproto/ofproto-dpif-upcall.c | 5 +++++\n> > 1 file changed, 5 insertions(+)\n> >\n> >diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c\n> >index 4a71bbe258df..bd324fbb6323 100644\n> >--- a/ofproto/ofproto-dpif-upcall.c\n> >+++ b/ofproto/ofproto-dpif-upcall.c\n> >@@ -2227,6 +2227,11 @@ push_dp_ops(struct udpif *udpif, struct ukey_op *ops, size_t n_ops)\n> > if (op->dop.error) {\n> > /* flow_del error, 'stats' is unusable. */\n> >+ if (op->ukey) {\n> >+ ovs_mutex_lock(&op->ukey->mutex);\n> >+ transition_ukey(op->ukey, UKEY_EVICTED);\n> >+ ovs_mutex_unlock(&op->ukey->mutex);\n> >+ }\n> > continue;\n> > }\n> >\n> \n> Compile tested only - I didn't see of any good way to force the error\n> \n> Code looks good to me.\n> \n> Reviewed-by: Greg Rose \n\nI applied this to master, although I don't fully understand it so that\nseems like living dangerously. That's EXTREME programming.","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 3ySfFh65d2z9s7m\n\tfor ;\n\tFri, 3 Nov 2017 09:02:16 +1100 (AEDT)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id 00A21D92;\n\tThu, 2 Nov 2017 22:02:15 +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 19268D80\n\tfor ; Thu, 2 Nov 2017 22:02:14 +0000 (UTC)","from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net\n\t[217.70.183.196])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id 8FF86484\n\tfor ; Thu, 2 Nov 2017 22:02:13 +0000 (UTC)","from ovn.org (unknown [208.91.3.26])\n\t(Authenticated sender: blp@ovn.org)\n\tby relay4-d.mail.gandi.net (Postfix) with ESMTPSA id 74A4D172098;\n\tThu, 2 Nov 2017 23:02:10 +0100 (CET)"],"X-Greylist":"domain auto-whitelisted by SQLgrey-1.7.6","X-Originating-IP":"208.91.3.26","Date":"Thu, 2 Nov 2017 15:02:07 -0700","From":"Ben Pfaff ","To":"Greg Rose ","Message-ID":"<20171102220207.GV27530@ovn.org>","References":"<20170906221252.17257-1-joe@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] ofproto-dpif-upcall: Transition ukey on\n\tdp_ops error.","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":1799379,"web_url":"http://patchwork.ozlabs.org/comment/1799379/","msgid":"","list_archive_url":null,"date":"2017-11-05T20:47:48","subject":"Re: [ovs-dev] [PATCH] ofproto-dpif-upcall: Transition ukey on\n\tdp_ops error.","submitter":{"id":67727,"url":"http://patchwork.ozlabs.org/api/people/67727/","name":"Joe Stringer","email":"joe@ovn.org"},"content":"On Thu, 2 Nov 2017, 15:02 Ben Pfaff, wrote:\n\n> On Mon, Sep 18, 2017 at 02:56:35PM -0700, Greg Rose wrote:\n> > On 09/06/2017 03:12 PM, Joe Stringer wrote:\n> > >In most situations, we don't expect that a flow we've successfully\n> > >dumped, which we intend to delete, cannot be deleted. However, to make\n> > >this code more resilient to ensure that ukeys *will* transition in all\n> > >cases (including an error at this stage), grab the lock and transition\n> > >this ukey forward to the evicted state, effectively treating a failure\n> > >to delete as \"this flow is already gone\".\n> > >\n> > >If we subsequently find out that it wasn't deleted, then that's ok - we\n> > >will re-dump, and validate at that stage, which should lead to creating\n> > >a new ukey or deleting the datapath flow when that happens.\n> > >\n> > >Signed-off-by: Joe Stringer \n> > >---\n> > > ofproto/ofproto-dpif-upcall.c | 5 +++++\n> > > 1 file changed, 5 insertions(+)\n> > >\n> > >diff --git a/ofproto/ofproto-dpif-upcall.c\n> b/ofproto/ofproto-dpif-upcall.c\n> > >index 4a71bbe258df..bd324fbb6323 100644\n> > >--- a/ofproto/ofproto-dpif-upcall.c\n> > >+++ b/ofproto/ofproto-dpif-upcall.c\n> > >@@ -2227,6 +2227,11 @@ push_dp_ops(struct udpif *udpif, struct ukey_op\n> *ops, size_t n_ops)\n> > > if (op->dop.error) {\n> > > /* flow_del error, 'stats' is unusable. */\n> > >+ if (op->ukey) {\n> > >+ ovs_mutex_lock(&op->ukey->mutex);\n> > >+ transition_ukey(op->ukey, UKEY_EVICTED);\n> > >+ ovs_mutex_unlock(&op->ukey->mutex);\n> > >+ }\n> > > continue;\n> > > }\n> > >\n> >\n> > Compile tested only - I didn't see of any good way to force the error\n> >\n> > Code looks good to me.\n> >\n> > Reviewed-by: Greg Rose \n>\n> I applied this to master, although I don't fully understand it so that\n> seems like living dangerously. That's EXTREME programming.\n>\n\nHere was I, thinking that extreme programming involved at least one of a\nvolcano, parachute, or snowboard...\n\nThanks for applying this.\n\n>","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 3yVSSy4z3cz9s81\n\tfor ;\n\tMon, 6 Nov 2017 07:48:17 +1100 (AEDT)","from mail.linux-foundation.org (localhost [127.0.0.1])\n\tby mail.linuxfoundation.org (Postfix) with ESMTP id B25A2408;\n\tSun, 5 Nov 2017 20:48:13 +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 81CA73EE\n\tfor ; Sun, 5 Nov 2017 20:48:12 +0000 (UTC)","from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net\n\t[217.70.183.197])\n\tby smtp1.linuxfoundation.org (Postfix) with ESMTPS id F208D79\n\tfor ; Sun, 5 Nov 2017 20:48:11 +0000 (UTC)","from mail-lf0-f46.google.com (mail-lf0-f46.google.com\n\t[209.85.215.46]) (Authenticated sender: joe@ovn.org)\n\tby relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 53C8941C07D\n\tfor ; Sun, 5 Nov 2017 21:48:09 +0100 (CET)","by mail-lf0-f46.google.com with SMTP id e143so8379759lfg.12\n\tfor ; Sun, 05 Nov 2017 12:48:09 -0800 (PST)","by 10.46.56.17 with HTTP; Sun, 5 Nov 2017 12:47:48 -0800 (PST)"],"X-Greylist":"domain auto-whitelisted by SQLgrey-1.7.6","X-Originating-IP":"209.85.215.46","X-Gm-Message-State":"AJaThX5/Lxr9nqr12HuIQrI76DxQZmonH1B7LIEC8M16xxu7/0ot+x6Z\n\tmlPRUp6Pl1C2S0KF/+MVmxT2/r3d6XePwquGl2U=","X-Google-Smtp-Source":"ABhQp+Q8C/McIMWA6G0rT8mUUcQHSYCJDymcZfR4OXjp5rJQim/W+Qpsa6/QVFPS0iV+mRalt/R57POJo0t/1okTZ34=","X-Received":"by 10.25.233.155 with SMTP id j27mr4526930lfk.234.1509914889240; \n\tSun, 05 Nov 2017 12:48:09 -0800 (PST)","MIME-Version":"1.0","In-Reply-To":"<20171102220207.GV27530@ovn.org>","References":"<20170906221252.17257-1-joe@ovn.org>\n\t\n\t<20171102220207.GV27530@ovn.org>","From":"Joe Stringer ","Date":"Sun, 5 Nov 2017 12:47:48 -0800","X-Gmail-Original-Message-ID":"","Message-ID":"","To":"Ben Pfaff ","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":"ovs dev ","Subject":"Re: [ovs-dev] [PATCH] ofproto-dpif-upcall: Transition ukey on\n\tdp_ops error.","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"}}]