Re: [ovs-dev] [PATCH v2.12] OVN: Combine conjunctions with identical matches into one flow.
Applied, thanks for the reminder. On Wed, Feb 26, 2020 at 05:11:07PM +0100, Maciej Jozefczyk wrote: > Hello, > > Can I ask you to verify this patch and apply it on branch-2.12, please? > We recently are having issues with Openstack gates because this patch is > missing in branch-2.12 > > Thanks in advance! > Maciej > > On Mon, Oct 28, 2019 at 11:10 PM 0-day Robot wrote: > > > Bleep bloop. Greetings Mark Michelson, I am a robot and I have tried out > > your patch. > > Thanks for your contribution. > > > > I encountered some error that I wasn't expecting. See the details below. > > > > > > git-am: > > Failed to merge in the changes. > > Patch failed at 0001 OVN: Combine conjunctions with identical matches into > > one flow. > > The copy of the patch that failed is found in: > > > > > > /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch > > When you have resolved this problem, run "git am --resolved". > > If you prefer to skip this patch, run "git am --skip" instead. > > To restore the original branch and stop patching, run "git am --abort". > > > > > > Please check this out. If you feel there has been an error, please email > > acon...@redhat.com > > > > Thanks, > > 0-day Robot > > ___ > > dev mailing list > > d...@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > > -- > Best regards, > Maciej Józefczyk > ___ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2.12] OVN: Combine conjunctions with identical matches into one flow.
Hello, Can I ask you to verify this patch and apply it on branch-2.12, please? We recently are having issues with Openstack gates because this patch is missing in branch-2.12 Thanks in advance! Maciej On Mon, Oct 28, 2019 at 11:10 PM 0-day Robot wrote: > Bleep bloop. Greetings Mark Michelson, I am a robot and I have tried out > your patch. > Thanks for your contribution. > > I encountered some error that I wasn't expecting. See the details below. > > > git-am: > Failed to merge in the changes. > Patch failed at 0001 OVN: Combine conjunctions with identical matches into > one flow. > The copy of the patch that failed is found in: > > > /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch > When you have resolved this problem, run "git am --resolved". > If you prefer to skip this patch, run "git am --skip" instead. > To restore the original branch and stop patching, run "git am --abort". > > > Please check this out. If you feel there has been an error, please email > acon...@redhat.com > > Thanks, > 0-day Robot > ___ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > -- Best regards, Maciej Józefczyk ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2.12] OVN: Combine conjunctions with identical matches into one flow.
Bleep bloop. Greetings Mark Michelson, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. git-am: Failed to merge in the changes. Patch failed at 0001 OVN: Combine conjunctions with identical matches into one flow. The copy of the patch that failed is found in: /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch When you have resolved this problem, run "git am --resolved". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2.12] OVN: Combine conjunctions with identical matches into one flow.
This is a backport of commit e659bab31a916d540411c93ca7125011b2e82b5c
from OVN master.
Conjunctive matches have an issue where it is possible to install
multiple flows that have identical matches. This results in ambiguity,
and can lead to features (such as ACLs) not functioning properly.
This change fixes the problem by combining conjunctions with identical
matches into a single flow. As an example, in the past we may have had
something like:
nw_dst=10.0.0.1 actions=conjunction(2,1/2)
nw_dst=10.0.0.1 actions=conjunction(3,1/2)
This commit changes this into
nw_dst=10.0.0.1 actions=conjunction(2,1/2),conjunction(3,1/2)
This way, there is only a single flow with the proscribed match, and
there is no ambiguity.
Signed-off-by: Mark Michelson
Acked-by: Numan Siddique
---
ovn/controller/lflow.c | 5 ++--
ovn/controller/ofctrl.c | 74 ++---
ovn/controller/ofctrl.h | 6
tests/ovn.at| 17 +---
4 files changed, 80 insertions(+), 22 deletions(-)
diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index 1aafafb33..09c2c78f1 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -735,8 +735,9 @@ consider_logical_flow(
dst->clause = src->clause;
dst->n_clauses = src->n_clauses;
}
-ofctrl_add_flow(flow_table, ptable, lflow->priority, 0, &m->match,
-&conj, &lflow->header_.uuid);
+
+ofctrl_add_or_append_flow(flow_table, ptable, lflow->priority, 0,
+ &m->match, &conj, &lflow->header_.uuid);
ofpbuf_uninit(&conj);
}
}
diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c
index 0fcaa72c3..f7211c5e0 100644
--- a/ovn/controller/ofctrl.c
+++ b/ovn/controller/ofctrl.c
@@ -69,6 +69,11 @@ struct ovn_flow {
uint64_t cookie;
};
+static struct ovn_flow *ovn_flow_alloc(uint8_t table_id, uint16_t priority,
+ uint64_t cookie,
+ const struct match *match,
+ const struct ofpbuf *actions,
+ const struct uuid *sb_uuid);
static uint32_t ovn_flow_match_hash(const struct ovn_flow *);
static struct ovn_flow *ovn_flow_lookup(struct hmap *flow_table,
const struct ovn_flow *target,
@@ -657,16 +662,8 @@ ofctrl_check_and_add_flow(struct ovn_desired_flow_table
*flow_table,
const struct uuid *sb_uuid,
bool log_duplicate_flow)
{
-struct ovn_flow *f = xmalloc(sizeof *f);
-f->table_id = table_id;
-f->priority = priority;
-minimatch_init(&f->match, match);
-f->ofpacts = xmemdup(actions->data, actions->size);
-f->ofpacts_len = actions->size;
-f->sb_uuid = *sb_uuid;
-f->match_hmap_node.hash = ovn_flow_match_hash(f);
-f->uuid_hindex_node.hash = uuid_hash(&f->sb_uuid);
-f->cookie = cookie;
+struct ovn_flow *f = ovn_flow_alloc(table_id, priority, cookie, match,
+actions, sb_uuid);
ovn_flow_log(f, "ofctrl_add_flow");
@@ -721,9 +718,66 @@ ofctrl_add_flow(struct ovn_desired_flow_table
*desired_flows,
ofctrl_check_and_add_flow(desired_flows, table_id, priority, cookie,
match, actions, sb_uuid, true);
}
+
+void
+ofctrl_add_or_append_flow(struct ovn_desired_flow_table *desired_flows,
+ uint8_t table_id, uint16_t priority, uint64_t cookie,
+ const struct match *match,
+ const struct ofpbuf *actions,
+ const struct uuid *sb_uuid)
+{
+struct ovn_flow *f = ovn_flow_alloc(table_id, priority, cookie, match,
+actions, sb_uuid);
+
+ovn_flow_log(f, "ofctrl_add_or_append_flow");
+
+struct ovn_flow *existing;
+existing = ovn_flow_lookup(&desired_flows->match_flow_table, f, false);
+if (existing) {
+/* There's already a flow with this particular match. Append the
+ * action to that flow rather than adding a new flow
+ */
+uint64_t compound_stub[64 / 8];
+struct ofpbuf compound;
+ofpbuf_use_stub(&compound, compound_stub, sizeof(compound_stub));
+ofpbuf_put(&compound, existing->ofpacts, existing->ofpacts_len);
+ofpbuf_put(&compound, f->ofpacts, f->ofpacts_len);
+
+free(existing->ofpacts);
+existing->ofpacts = xmemdup(compound.data, compound.size);
+existing->ofpacts_len = compound.size;
+
+ofpbuf_uninit(&compound);
+ovn_flow_destroy(f);
+} else {
+hmap_insert(&desired_flows->match_flow_table, &f->match_hmap_node,
+f->match_hmap_node.hash);
+hindex_insert(&desired_flows->uuid_flow_table, &f->uuid_hindex_node,
+
