From patchwork Sun Jan 12 22:10:49 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1221887 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.137; helo=fraxinus.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=ovn.org Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 47wrXF0FrWz9s4Y for ; Mon, 13 Jan 2020 09:11:07 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 78BDD853D9; Sun, 12 Jan 2020 22:11:05 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id FQPp9gePOugJ; Sun, 12 Jan 2020 22:11:04 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id 59E4B83F22; Sun, 12 Jan 2020 22:11:04 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 3B2DFC1D85; Sun, 12 Jan 2020 22:11:04 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 85F21C077D for ; Sun, 12 Jan 2020 22:11:02 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 6EA88851C2 for ; Sun, 12 Jan 2020 22:11:02 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id F11NPwK6NZfZ for ; Sun, 12 Jan 2020 22:11:01 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net [217.70.183.201]) by fraxinus.osuosl.org (Postfix) with ESMTPS id CC9BA83F22 for ; Sun, 12 Jan 2020 22:11:00 +0000 (UTC) X-Originating-IP: 73.241.94.255 Received: from localhost.localdomain.localdomain (unknown [73.241.94.255]) (Authenticated sender: hzhou@ovn.org) by relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 208641BF203; Sun, 12 Jan 2020 22:10:56 +0000 (UTC) From: Han Zhou To: dev@openvswitch.org Date: Sun, 12 Jan 2020 14:10:49 -0800 Message-Id: <1578867049-117521-1-git-send-email-hzhou@ovn.org> X-Mailer: git-send-email 2.1.0 Cc: Han Zhou Subject: [ovs-dev] [PATCH ovn] ofctrl.c: Update installed OVS flow cookie when lflow is changed. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" When an old lflow is replaced by a new lflow, if the OVS flows translated by the old and new lflows have same match, ofctrl will update existing OVS flow instead of deleting and adding a new one. However, when updating the existing flows, the cookie was not updated by current implementation, which results in discrepency between lflows and OVS flows, making debugging difficult and confuses tools such as ovn-trace. This patch fixes it. Note: since command OFPFC_MODIFY_STRICT in FLOW_MOD message doesn't support updating flow cookie after OpenFlow 1.1, this patch changes to use OFPFC_ADD command, which effectively modifies existing flow if a match is found. Signed-off-by: Han Zhou --- controller/ofctrl.c | 12 ++++++++++-- tests/ovn.at | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/controller/ofctrl.c b/controller/ofctrl.c index 10edd84..6f2c530 100644 --- a/controller/ofctrl.c +++ b/controller/ofctrl.c @@ -828,6 +828,7 @@ ovn_flow_to_string(const struct ovn_flow *f) { struct ds s = DS_EMPTY_INITIALIZER; ds_put_format(&s, "sb_uuid="UUID_FMT", ", UUID_ARGS(&f->sb_uuid)); + ds_put_format(&s, "cookie=%"PRIx64", ", f->cookie); ds_put_format(&s, "table_id=%"PRIu8", ", f->table_id); ds_put_format(&s, "priority=%"PRIu16", ", f->priority); minimatch_format(&f->match, NULL, NULL, &s, OFP_DEFAULT_PRIORITY); @@ -1176,7 +1177,8 @@ ofctrl_put(struct ovn_desired_flow_table *flow_table, i->sb_uuid = d->sb_uuid; } if (!ofpacts_equal(i->ofpacts, i->ofpacts_len, - d->ofpacts, d->ofpacts_len)) { + d->ofpacts, d->ofpacts_len) || + i->cookie != d->cookie) { /* Update actions in installed flow. */ struct ofputil_flow_mod fm = { .match = i->match, @@ -1184,8 +1186,14 @@ ofctrl_put(struct ovn_desired_flow_table *flow_table, .table_id = i->table_id, .ofpacts = d->ofpacts, .ofpacts_len = d->ofpacts_len, - .command = OFPFC_MODIFY_STRICT, + .command = OFPFC_ADD, }; + /* Update cookie if it is changed. */ + if (i->cookie != d->cookie) { + fm.modify_cookie = true; + fm.new_cookie = htonll(d->cookie); + i->cookie = d->cookie; + } add_flow_mod(&fm, &msgs); ovn_flow_log(i, "updating installed"); diff --git a/tests/ovn.at b/tests/ovn.at index 411b768..adb677c 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -17338,3 +17338,39 @@ OVS_WAIT_UNTIL([ OVN_CLEANUP([hv1]) AT_CLEANUP + +AT_SETUP([ovn -- trace when flow cookie updated]) +AT_KEYWORDS([cookie]) +ovn_start + +net_add n1 +sim_add hv1 +as hv1 +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 +ovs-vsctl add-port br-int vif1 -- \ + set interface vif1 external-ids:iface-id=lp1 ofport-request=1 + +ovn-nbctl ls-add lsw0 +ovn-nbctl lsp-add lsw0 lp1 +ovn-nbctl lsp-set-addresses lp1 "f0:00:00:00:00:01 10.0.0.1" +ovn-nbctl acl-add lsw0 from-lport 1000 'eth.type == 0x1234' drop + +ovn-nbctl --wait=hv --timeout=3 sync + +# Trace with --ovs should see ovs flow related to the ACL +AT_CHECK([ovn-trace --ovs lsw0 'inport == "lp1" && eth.type == 0x1234' | grep "dl_type=0x1234" | grep "cookie"], [0], [ignore]) + +# Replace the ACL with same match but different action. +ovn-nbctl acl-del lsw0 -- \ + acl-add lsw0 from-lport 1000 'eth.type == 0x1234' allow + +ovn-nbctl --wait=hv --timeout=3 sync + +# Trace with --ovs should still see the ovs flow related to the ACL, which +# means the OVS flow is updated with new cookie corresponding to the new lflow. +AT_CHECK([ovn-trace --ovs lsw0 'inport == "lp1" && eth.type == 0x1234' | grep "dl_type=0x1234 actions="], [0], [ignore]) + +OVN_CLEANUP([hv1]) + +AT_CLEANUP