| 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 | expand |
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
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.
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);
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(-)