[ovs-dev] ofproto: Fix OVS crash when reverting old flows in bundle commit
diff mbox series

Message ID 1529193030-3870-1-git-send-email-vishal.deep.ajmera@ericsson.com
State Accepted
Headers show
Series
  • [ovs-dev] ofproto: Fix OVS crash when reverting old flows in bundle commit
Related show

Commit Message

Vishal Deep Ajmera June 16, 2018, 11:50 p.m. UTC
During bundle commit flows which are added in bundle are applied
to ofproto in-order. In case if a flow cannot be added (e.g. flow
action is go-to group id which does not exist), OVS tries to
revert back all previous flows which were successfully applied
from the same bundle. This is possible since OVS maintains list
of old flows which were replaced by flows from the bundle.

While reinserting old flows ovs asserts due to check on rule
state != RULE_INITIALIZED.  This will work only for new flows, but
for old flow the rule state will be RULE_REMOVED. This is causing
an assert and OVS crash.

The ovs assert check should be modified to != RULE_INSERTED to prevent
any existing rule being re-inserted and allow new rules and old rules
(in case of revert) to get inserted.

Signed-off-by: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com>
---
 ofproto/ofproto.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Vishal Deep Ajmera June 18, 2018, 3:34 p.m. UTC | #1
Hi Ben,

If the patch looks ok, can we this patch merged to master ? Steps to reproduce the issue was shared on ovs-discuss list.

Warm Regards,
Vishal Ajmera
Ben Pfaff June 18, 2018, 7:50 p.m. UTC | #2
On Sun, Jun 17, 2018 at 05:20:30AM +0530, Vishal Deep Ajmera wrote:
> During bundle commit flows which are added in bundle are applied
> to ofproto in-order. In case if a flow cannot be added (e.g. flow
> action is go-to group id which does not exist), OVS tries to
> revert back all previous flows which were successfully applied
> from the same bundle. This is possible since OVS maintains list
> of old flows which were replaced by flows from the bundle.
> 
> While reinserting old flows ovs asserts due to check on rule
> state != RULE_INITIALIZED.  This will work only for new flows, but
> for old flow the rule state will be RULE_REMOVED. This is causing
> an assert and OVS crash.
> 
> The ovs assert check should be modified to != RULE_INSERTED to prevent
> any existing rule being re-inserted and allow new rules and old rules
> (in case of revert) to get inserted.
> 
> Signed-off-by: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com>

Thanks, applied to master, backported as far as branch-2.7.

I added the reproduction example to the commit message.

Patch
diff mbox series

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 829ccd8..f946e27 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -8648,7 +8648,7 @@  ofproto_rule_insert__(struct ofproto *ofproto, struct rule *rule)
     const struct rule_actions *actions = rule_get_actions(rule);
 
     /* A rule may not be reinserted. */
-    ovs_assert(rule->state == RULE_INITIALIZED);
+    ovs_assert(rule->state != RULE_INSERTED);
 
     if (rule->hard_timeout || rule->idle_timeout) {
         ovs_list_insert(&ofproto->expirable, &rule->expirable);