Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2019-04-23 Thread Ben Pfaff
On Tue, Apr 23, 2019 at 11:44:25AM +, aravind.sridha...@dell.com wrote:
> Currently, rule_insert() API does not have return value. There are
> some possible scenarios where rule insertions can fail at run-time
> even though the static checks during rule_construct() had passed
> previously.
> Some possible scenarios for failure of rule insertions:
> **) Rule insertions can fail dynamically in Hybrid mode (both
> Openflow and Normal switch functioning coexist) where the
> CAM space could get suddenly filled up by Normal switch
> functioning and Openflow gets devoid of available space.
> **) Some deployments could have separate independent layers for
> HW rule insertions and application layer to interact with OVS.
> HW layer could face any dynamic issue during rule handling which
> application could not have predicted/captured in
> rule-construction phase.
> Rule-insert errors for bundles are handled too in this pull-request.

Thanks.

Applied to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2019-04-23 Thread Aravind.Sridharan
Currently, rule_insert() API does not have return value. There are
some possible scenarios where rule insertions can fail at run-time
even though the static checks during rule_construct() had passed
previously.
Some possible scenarios for failure of rule insertions:
**) Rule insertions can fail dynamically in Hybrid mode (both
Openflow and Normal switch functioning coexist) where the
CAM space could get suddenly filled up by Normal switch
functioning and Openflow gets devoid of available space.
**) Some deployments could have separate independent layers for
HW rule insertions and application layer to interact with OVS.
HW layer could face any dynamic issue during rule handling which
application could not have predicted/captured in
rule-construction phase.
Rule-insert errors for bundles are handled too in this pull-request.

Testing:
---
Tested failures of rule insertions and also with bundles.

Signed-off-by: Aravind Prasad S 

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index f0d387c..110789e 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4443,7 +4443,7 @@ rule_construct(struct rule *rule_)
 return 0;
}
-static void
+static enum ofperr
rule_insert(struct rule *rule_, struct rule *old_rule_, bool forward_counts)
 OVS_REQUIRES(ofproto_mutex)
{
@@ -4473,6 +4473,8 @@ rule_insert(struct rule *rule_, struct rule *old_rule_, 
bool forward_counts)
 ovs_mutex_unlock(>stats_mutex);
 ovs_mutex_unlock(_rule->stats_mutex);
 }
+
+return 0;
}
 static void
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index eb62a8f..7907d4b 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -1329,8 +1329,8 @@ struct ofproto_class {
 struct rule *(*rule_alloc)(void);
 enum ofperr (*rule_construct)(struct rule *rule)
 /* OVS_REQUIRES(ofproto_mutex) */;
-void (*rule_insert)(struct rule *rule, struct rule *old_rule,
-bool forward_counts)
+enum ofperr (*rule_insert)(struct rule *rule, struct rule *old_rule,
+   bool forward_counts)
 /* OVS_REQUIRES(ofproto_mutex) */;
 void (*rule_delete)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex) */;
 void (*rule_destruct)(struct rule *rule);
@@ -1984,7 +1984,7 @@ enum ofperr ofproto_flow_mod_learn_start(struct 
ofproto_flow_mod *ofm)
 OVS_REQUIRES(ofproto_mutex);
void ofproto_flow_mod_learn_revert(struct ofproto_flow_mod *ofm)
 OVS_REQUIRES(ofproto_mutex);
-void ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
+enum ofperr ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
   struct ofproto *orig_ofproto)
 OVS_REQUIRES(ofproto_mutex);
void ofproto_add_flow(struct ofproto *, const struct match *, int priority,
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 3579aac..1d6fc00 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -247,10 +247,10 @@ static void replace_rule_revert(struct ofproto *, struct 
rule *old_rule,
 struct rule *new_rule)
 OVS_REQUIRES(ofproto_mutex);
-static void replace_rule_finish(struct ofproto *, struct ofproto_flow_mod *,
-const struct openflow_mod_requester *,
-struct rule *old_rule, struct rule *new_rule,
-struct ovs_list *dead_cookies)
+static enum ofperr replace_rule_finish(struct ofproto *, struct 
ofproto_flow_mod *,
+   const struct openflow_mod_requester *,
+   struct rule *old_rule, struct rule 
*new_rule,
+   struct ovs_list *dead_cookies)
 OVS_REQUIRES(ofproto_mutex);
static void delete_flows__(struct rule_collection *,
enum ofp_flow_removed_reason,
@@ -272,9 +272,9 @@ static enum ofperr ofproto_flow_mod_start(struct ofproto *,
static void ofproto_flow_mod_revert(struct ofproto *,
 struct ofproto_flow_mod *)
 OVS_REQUIRES(ofproto_mutex);
-static void ofproto_flow_mod_finish(struct ofproto *,
-struct ofproto_flow_mod *,
-const struct openflow_mod_requester *)
+static enum ofperr ofproto_flow_mod_finish(struct ofproto *,
+   struct ofproto_flow_mod *,
+   const struct openflow_mod_requester 
*)
 OVS_REQUIRES(ofproto_mutex);
static enum ofperr handle_flow_mod__(struct ofproto *,
  const struct ofputil_flow_mod *,
@@ -5115,7 +5115,7 @@ add_flow_revert(struct ofproto *ofproto, struct 
ofproto_flow_mod *ofm)
}
 /* To be called after version bump. */
-static void
+static enum ofperr
add_flow_finish(struct ofproto *ofproto, struct ofproto_flow_mod *ofm,
 const struct 

Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2019-04-23 Thread Aravind.Sridharan
Hi All,

Sorry for resending the mail, the link appears wrongly in previous mail (due to 
mingling of 'dot' with github link). 

Correct GitHub pull request link - https://github.com/openvswitch/ovs/pull/282  
 

Kindly review.

Thanks,
Aravind Prasad S

-Original Message-
From: Sridharan, Aravind 
Sent: Wednesday, April 24, 2019 1:26 AM
To: 'Ben Pfaff'; ovs-dev@openvswitch.org
Subject: RE: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

Hi All,

I have raised the pull request in Github - 
https://github.com/openvswitch/ovs/pull/282. Kindly review and let me know your 
views. 

Hi Ben,
Also, I raised the similar patch sometime back and you had requested for the 
vendor name. Had to get approval from my Organization. Hence, requesting your 
review for the changes now. 

> Hi Ben,
> 
> By "which" switches, you mean to ask to which vendor switches ?
> In general, this patch will be very useful for all Switches which 
> implement OVS for their HW implementations, specifically, will be 
> useful for Switches which had implemented ofproto and netdev provider 
> APIs for their respective HW/NPU and Kernel. And, in case of HW, the 
> possibility of dynamic rule insertion failures are usually higher. 
> Static checks during rule-construct phase are not sufficient and 
> predicting future failure predictions in construct phase may not be 
> possible.
> Hence, this patch will make OVS more flexible to handle such rule 
> insertion failures.

> So, basically, you're not willing to name any switches.  I'm tired of  
>the vendor secrecy game.  They get to see all of our code but they're 
>never willing even to name themselves.

> Sorry, I'm not going to willingly take this patch.


Thanks,
S. Aravind Prasad

On Tue, Apr 23, 2019 at 02:40:55PM +, aravind.sridha...@dell.com wrote:
> Currently, rule_insert() API does not have return value. There are 
> some possible scenarios where rule insertions can fail at run-time 
> even though the static checks during rule_construct() had passed 
> previously.
> Some possible scenarios for failure of rule insertions:
> **) Rule insertions can fail dynamically in Hybrid mode (both Openflow 
> and Normal switch functioning coexist) where the CAM space could get 
> suddenly filled up by Normal switch functioning and Openflow gets 
> devoid of available space.
> **) Some deployments could have separate independent layers for HW 
> rule insertions and application layer to interact with OVS.
> HW layer could face any dynamic issue during rule handling which 
> application could not have predicted/captured in rule-construction 
> phase.
> Rule-insert errors for bundles are handled too in this pull-request.

> It looks like you're not having much luck with posting patches.  If you  > 
> haven't already tried "git send-email", please try that; otherwise, you > can 
> always submit a pull request on Github.

Thanks,

Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2019-04-23 Thread Aravind.Sridharan
Hi All,

I have raised the pull request in Github - 
https://github.com/openvswitch/ovs/pull/282. Kindly review and let me know your 
views. 

Hi Ben,
Also, I raised the similar patch sometime back and you had requested for the 
vendor name. Had to get approval from my Organization. Hence, requesting your 
review for the changes now. 

> Hi Ben,
> 
> By "which" switches, you mean to ask to which vendor switches ?
> In general, this patch will be very useful for all Switches which
> implement OVS for their HW implementations, specifically,
> will be useful for Switches which had implemented ofproto
> and netdev provider APIs for their respective HW/NPU and
> Kernel. And, in case of HW, the possibility of dynamic rule
> insertion failures are usually higher. Static checks during
> rule-construct phase are not sufficient and predicting
> future failure predictions in construct phase may not be possible.
> Hence, this patch will make OVS more flexible to handle such
> rule insertion failures.

> So, basically, you're not willing to name any switches.  I'm tired of
> the vendor secrecy game.  They get to see all of our code but they're
>never willing even to name themselves.

> Sorry, I'm not going to willingly take this patch.


Thanks,
S. Aravind Prasad

On Tue, Apr 23, 2019 at 02:40:55PM +, aravind.sridha...@dell.com wrote:
> Currently, rule_insert() API does not have return value. There are 
> some possible scenarios where rule insertions can fail at run-time 
> even though the static checks during rule_construct() had passed 
> previously.
> Some possible scenarios for failure of rule insertions:
> **) Rule insertions can fail dynamically in Hybrid mode (both Openflow 
> and Normal switch functioning coexist) where the CAM space could get 
> suddenly filled up by Normal switch functioning and Openflow gets 
> devoid of available space.
> **) Some deployments could have separate independent layers for HW 
> rule insertions and application layer to interact with OVS.
> HW layer could face any dynamic issue during rule handling which 
> application could not have predicted/captured in rule-construction 
> phase.
> Rule-insert errors for bundles are handled too in this pull-request.

> It looks like you're not having much luck with posting patches.  If you  > 
> haven't already tried "git send-email", please try that; otherwise, you > can 
> always submit a pull request on Github.

Thanks,

Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2019-04-23 Thread Ben Pfaff
On Tue, Apr 23, 2019 at 02:40:55PM +, aravind.sridha...@dell.com wrote:
> Currently, rule_insert() API does not have return value. There are
> some possible scenarios where rule insertions can fail at run-time
> even though the static checks during rule_construct() had passed
> previously.
> Some possible scenarios for failure of rule insertions:
> **) Rule insertions can fail dynamically in Hybrid mode (both
> Openflow and Normal switch functioning coexist) where the
> CAM space could get suddenly filled up by Normal switch
> functioning and Openflow gets devoid of available space.
> **) Some deployments could have separate independent layers for
> HW rule insertions and application layer to interact with OVS.
> HW layer could face any dynamic issue during rule handling which
> application could not have predicted/captured in
> rule-construction phase.
> Rule-insert errors for bundles are handled too in this pull-request.

It looks like you're not having much luck with posting patches.  If you
haven't already tried "git send-email", please try that; otherwise, you
can always submit a pull request on Github.

Thanks,

Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2019-04-23 Thread Aravind.Sridharan
Currently, rule_insert() API does not have return value. There are
some possible scenarios where rule insertions can fail at run-time
even though the static checks during rule_construct() had passed
previously.
Some possible scenarios for failure of rule insertions:
**) Rule insertions can fail dynamically in Hybrid mode (both
Openflow and Normal switch functioning coexist) where the
CAM space could get suddenly filled up by Normal switch
functioning and Openflow gets devoid of available space.
**) Some deployments could have separate independent layers for
HW rule insertions and application layer to interact with OVS.
HW layer could face any dynamic issue during rule handling which
application could not have predicted/captured in
rule-construction phase.
Rule-insert errors for bundles are handled too in this pull-request.

Testing:
---
Tested failures of rule insertions and also with bundles.

Signed-off-by: Aravind Prasad S 

---
ofproto/ofproto-dpif.c |   4 +-
ofproto/ofproto-provider.h |   6 +--
ofproto/ofproto.c  | 109 +
3 files changed, 86 insertions(+), 33 deletions(-)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index f0d387c..110789e 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4443,7 +4443,7 @@ rule_construct(struct rule *rule_)
 return 0;
}
-static void
+static enum ofperr
rule_insert(struct rule *rule_, struct rule *old_rule_, bool forward_counts)
 OVS_REQUIRES(ofproto_mutex)
{
@@ -4473,6 +4473,8 @@ rule_insert(struct rule *rule_, struct rule *old_rule_, 
bool forward_counts)
 ovs_mutex_unlock(>stats_mutex);
 ovs_mutex_unlock(_rule->stats_mutex);
 }
+
+return 0;
}
 static void
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index eb62a8f..7907d4b 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -1329,8 +1329,8 @@ struct ofproto_class {
 struct rule *(*rule_alloc)(void);
 enum ofperr (*rule_construct)(struct rule *rule)
 /* OVS_REQUIRES(ofproto_mutex) */;
-void (*rule_insert)(struct rule *rule, struct rule *old_rule,
-bool forward_counts)
+enum ofperr (*rule_insert)(struct rule *rule, struct rule *old_rule,
+   bool forward_counts)
 /* OVS_REQUIRES(ofproto_mutex) */;
 void (*rule_delete)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex) */;
 void (*rule_destruct)(struct rule *rule);
@@ -1984,7 +1984,7 @@ enum ofperr ofproto_flow_mod_learn_start(struct 
ofproto_flow_mod *ofm)
 OVS_REQUIRES(ofproto_mutex);
void ofproto_flow_mod_learn_revert(struct ofproto_flow_mod *ofm)
 OVS_REQUIRES(ofproto_mutex);
-void ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
+enum ofperr ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
   struct ofproto *orig_ofproto)
 OVS_REQUIRES(ofproto_mutex);
void ofproto_add_flow(struct ofproto *, const struct match *, int priority,
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 3579aac..1d6fc00 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -247,10 +247,10 @@ static void replace_rule_revert(struct ofproto *, struct 
rule *old_rule,
 struct rule *new_rule)
 OVS_REQUIRES(ofproto_mutex);
-static void replace_rule_finish(struct ofproto *, struct ofproto_flow_mod *,
-const struct openflow_mod_requester *,
-struct rule *old_rule, struct rule *new_rule,
-struct ovs_list *dead_cookies)
+static enum ofperr replace_rule_finish(struct ofproto *, struct 
ofproto_flow_mod *,
+   const struct openflow_mod_requester *,
+   struct rule *old_rule, struct rule 
*new_rule,
+   struct ovs_list *dead_cookies)
 OVS_REQUIRES(ofproto_mutex);
static void delete_flows__(struct rule_collection *,
enum ofp_flow_removed_reason,
@@ -272,9 +272,9 @@ static enum ofperr ofproto_flow_mod_start(struct ofproto *,
static void ofproto_flow_mod_revert(struct ofproto *,
 struct ofproto_flow_mod *)
 OVS_REQUIRES(ofproto_mutex);
-static void ofproto_flow_mod_finish(struct ofproto *,
-struct ofproto_flow_mod *,
-const struct openflow_mod_requester *)
+static enum ofperr ofproto_flow_mod_finish(struct ofproto *,
+   struct ofproto_flow_mod *,
+   const struct openflow_mod_requester 
*)
 OVS_REQUIRES(ofproto_mutex);
static enum ofperr handle_flow_mod__(struct ofproto *,
  const struct ofputil_flow_mod *,
@@ -5115,7 +5115,7 @@ add_flow_revert(struct ofproto *ofproto, 

[ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2019-04-23 Thread Aravind.Sridharan
Description
---
Currently, rule_insert() API does not have return value. There are
some possible scenarios where rule insertions can fail at run-time
even though the static checks during rule_construct() had passed
previously.
Some possible scenarios for failure of rule insertions:
**) Rule insertions can fail dynamically in Hybrid mode (both
Openflow and Normal switch functioning coexist) where the
CAM space could get suddenly filled up by Normal switch
functioning and Openflow gets devoid of available space.
**) Some deployments could have separate independent layers for
HW rule insertions and application layer to interact with OVS.
HW layer could face any dynamic issue during rule handling which
application could not have predicted/captured in
rule-construction phase.
Rule-insert errors for bundles are handled too in this pull-request.

Testing:
---
Tested with failures of rule insertions and also with bundles.

Signed-off-by: Aravind Prasad S 

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index f0d387c..110789e 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4443,7 +4443,7 @@ rule_construct(struct rule *rule_)
 return 0;
}
-static void
+static enum ofperr
rule_insert(struct rule *rule_, struct rule *old_rule_, bool forward_counts)
 OVS_REQUIRES(ofproto_mutex)
{
@@ -4473,6 +4473,8 @@ rule_insert(struct rule *rule_, struct rule *old_rule_, 
bool forward_counts)
 ovs_mutex_unlock(>stats_mutex);
 ovs_mutex_unlock(_rule->stats_mutex);
 }
+
+return 0;
}
 static void
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index eb62a8f..7907d4b 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -1329,8 +1329,8 @@ struct ofproto_class {
 struct rule *(*rule_alloc)(void);
 enum ofperr (*rule_construct)(struct rule *rule)
 /* OVS_REQUIRES(ofproto_mutex) */;
-void (*rule_insert)(struct rule *rule, struct rule *old_rule,
-bool forward_counts)
+enum ofperr (*rule_insert)(struct rule *rule, struct rule *old_rule,
+   bool forward_counts)
 /* OVS_REQUIRES(ofproto_mutex) */;
 void (*rule_delete)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex) */;
 void (*rule_destruct)(struct rule *rule);
@@ -1984,7 +1984,7 @@ enum ofperr ofproto_flow_mod_learn_start(struct 
ofproto_flow_mod *ofm)
 OVS_REQUIRES(ofproto_mutex);
void ofproto_flow_mod_learn_revert(struct ofproto_flow_mod *ofm)
 OVS_REQUIRES(ofproto_mutex);
-void ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
+enum ofperr ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
   struct ofproto *orig_ofproto)
 OVS_REQUIRES(ofproto_mutex);
void ofproto_add_flow(struct ofproto *, const struct match *, int priority,
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 3579aac..1d6fc00 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -247,10 +247,10 @@ static void replace_rule_revert(struct ofproto *, struct 
rule *old_rule,
 struct rule *new_rule)
 OVS_REQUIRES(ofproto_mutex);
-static void replace_rule_finish(struct ofproto *, struct ofproto_flow_mod *,
-const struct openflow_mod_requester *,
-struct rule *old_rule, struct rule *new_rule,
-struct ovs_list *dead_cookies)
+static enum ofperr replace_rule_finish(struct ofproto *, struct 
ofproto_flow_mod *,
+   const struct openflow_mod_requester *,
+   struct rule *old_rule, struct rule 
*new_rule,
+   struct ovs_list *dead_cookies)
 OVS_REQUIRES(ofproto_mutex);
static void delete_flows__(struct rule_collection *,
enum ofp_flow_removed_reason,
@@ -272,9 +272,9 @@ static enum ofperr ofproto_flow_mod_start(struct ofproto *,
static void ofproto_flow_mod_revert(struct ofproto *,
 struct ofproto_flow_mod *)
 OVS_REQUIRES(ofproto_mutex);
-static void ofproto_flow_mod_finish(struct ofproto *,
-struct ofproto_flow_mod *,
-const struct openflow_mod_requester *)
+static enum ofperr ofproto_flow_mod_finish(struct ofproto *,
+   struct ofproto_flow_mod *,
+   const struct openflow_mod_requester 
*)
 OVS_REQUIRES(ofproto_mutex);
static enum ofperr handle_flow_mod__(struct ofproto *,
  const struct ofputil_flow_mod *,
@@ -5115,7 +5115,7 @@ add_flow_revert(struct ofproto *ofproto, struct 
ofproto_flow_mod *ofm)
}
 /* To be called after version bump. */
-static void
+static enum ofperr
add_flow_finish(struct ofproto *ofproto, struct ofproto_flow_mod *ofm,
   

[ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2019-04-23 Thread Aravind.Sridharan
Currently, rule_insert() API does not have return value. There are
some possible scenarios where rule insertions can fail at run-time
even though the static checks during rule_construct() had passed
previously.
Some possible scenarios for failure of rule insertions:
**) Rule insertions can fail dynamically in Hybrid mode (both
Openflow and Normal switch functioning coexist) where the
CAM space could get suddenly filled up by Normal switch
functioning and Openflow gets devoid of available space.
**) Some deployments could have separate independent layers for
HW rule insertions and application layer to interact with OVS.
HW layer could face any dynamic issue during rule handling which
application could not have predicted/captured in
rule-construction phase.
Rule-insert errors for bundles are handled too in this pull-request.

Testing:
---
Tested failures of rule insertions and also with bundles.

Signed-off-by: Aravind Prasad S 

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index f0d387c..110789e 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4443,7 +4443,7 @@ rule_construct(struct rule *rule_)
 return 0;
}
-static void
+static enum ofperr
rule_insert(struct rule *rule_, struct rule *old_rule_, bool forward_counts)
 OVS_REQUIRES(ofproto_mutex)
{
@@ -4473,6 +4473,8 @@ rule_insert(struct rule *rule_, struct rule *old_rule_, 
bool forward_counts)
 ovs_mutex_unlock(>stats_mutex);
 ovs_mutex_unlock(_rule->stats_mutex);
 }
+
+return 0;
}
 static void
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index eb62a8f..7907d4b 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -1329,8 +1329,8 @@ struct ofproto_class {
 struct rule *(*rule_alloc)(void);
 enum ofperr (*rule_construct)(struct rule *rule)
 /* OVS_REQUIRES(ofproto_mutex) */;
-void (*rule_insert)(struct rule *rule, struct rule *old_rule,
-bool forward_counts)
+enum ofperr (*rule_insert)(struct rule *rule, struct rule *old_rule,
+   bool forward_counts)
 /* OVS_REQUIRES(ofproto_mutex) */;
 void (*rule_delete)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex) */;
 void (*rule_destruct)(struct rule *rule);
@@ -1984,7 +1984,7 @@ enum ofperr ofproto_flow_mod_learn_start(struct 
ofproto_flow_mod *ofm)
 OVS_REQUIRES(ofproto_mutex);
void ofproto_flow_mod_learn_revert(struct ofproto_flow_mod *ofm)
 OVS_REQUIRES(ofproto_mutex);
-void ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
+enum ofperr ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
   struct ofproto *orig_ofproto)
 OVS_REQUIRES(ofproto_mutex);
void ofproto_add_flow(struct ofproto *, const struct match *, int priority,
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 3579aac..1d6fc00 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -247,10 +247,10 @@ static void replace_rule_revert(struct ofproto *, struct 
rule *old_rule,
 struct rule *new_rule)
 OVS_REQUIRES(ofproto_mutex);
-static void replace_rule_finish(struct ofproto *, struct ofproto_flow_mod *,
-const struct openflow_mod_requester *,
-struct rule *old_rule, struct rule *new_rule,
-struct ovs_list *dead_cookies)
+static enum ofperr replace_rule_finish(struct ofproto *, struct 
ofproto_flow_mod *,
+   const struct openflow_mod_requester *,
+   struct rule *old_rule, struct rule 
*new_rule,
+   struct ovs_list *dead_cookies)
 OVS_REQUIRES(ofproto_mutex);
static void delete_flows__(struct rule_collection *,
enum ofp_flow_removed_reason,
@@ -272,9 +272,9 @@ static enum ofperr ofproto_flow_mod_start(struct ofproto *,
static void ofproto_flow_mod_revert(struct ofproto *,
 struct ofproto_flow_mod *)
 OVS_REQUIRES(ofproto_mutex);
-static void ofproto_flow_mod_finish(struct ofproto *,
-struct ofproto_flow_mod *,
-const struct openflow_mod_requester *)
+static enum ofperr ofproto_flow_mod_finish(struct ofproto *,
+   struct ofproto_flow_mod *,
+   const struct openflow_mod_requester 
*)
 OVS_REQUIRES(ofproto_mutex);
static enum ofperr handle_flow_mod__(struct ofproto *,
  const struct ofputil_flow_mod *,
@@ -5115,7 +5115,7 @@ add_flow_revert(struct ofproto *ofproto, struct 
ofproto_flow_mod *ofm)
}
 /* To be called after version bump. */
-static void
+static enum ofperr
add_flow_finish(struct ofproto *ofproto, struct ofproto_flow_mod *ofm,
 const struct 

Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2018-08-06 Thread Ben Pfaff
On Wed, Aug 01, 2018 at 07:29:06AM +0530, Aravind Prasad wrote:
>  > > Currently, rule_insert() API does not have return value. There are
> some possible
> > > scenarios where rule insertions can fail at run-time even though the
> static
> > > checks during rule_construct() had passed previously.
> > > Some possible scenarios for failure of rule insertions:
> > > **) Rule insertions can fail dynamically in Hybrid mode (both Openflow
> and
> > > Normal switch functioning coexist) where the CAM space could get
> suddenly
> > > filled up by Normal switch functioning and Openflow gets devoid of
> > > available space.
> > > **) Some deployments could have separate independent layers for HW rule
> > > insertions and application layer to interact with OVS. HW layer
> > > could face any dynamic issue during rule handling which application
> could
> > > not have predicted/captured in rule-construction phase.
> > > Rule-insert errors for bundles are handled too in this pull-request.
> > >
> > > Signed-off-by: Aravind Prasad S 
> >
> > >Which switches does this help?
> >
> > Hi Ben, These type of errors are possible in actual Hardware
> > implementations of OVS. It is possible that ofproto and netdev providers
> be
> > implemented for an actual HW/NPU. Usually, in such cases, in the rule
> > construct phase, all the static checks like verifying the qualifiers/
> > actions, CAM full checks could be done and the other related
> verifications.
> > But during the rule insert phase, it is possible that the rule insertion
> > may get failed in HW (runtime errors, HW errors and so on). Also, since HW
> > switches may support Hybrid mode (coexistence of Normal and Openflow
> > functioning), the possibility of this issue could be even more. Further,
> > when rule-insertion fails, it results in a stale state where the
> Controller
> > and Switch Flow-DB differ. Hence, we need a way to rollback for
> rule-insert
> > phase also. Kindly let me know your views. And sorry for re-sending the
> > review requests many times over. Will avoid in future.
> 
> > >Which switches does this help?
> 
> Hi Ben,
> 
> By "which" switches, you mean to ask to which vendor switches ?
> In general, this patch will be very useful for all Switches which
> implement OVS for their HW implementations, specifically,
> will be useful for Switches which had implemented ofproto
> and netdev provider APIs for their respective HW/NPU and
> Kernel. And, in case of HW, the possibility of dynamic rule
> insertion failures are usually higher. Static checks during
> rule-construct phase are not sufficient and predicting
> future failure predictions in construct phase may not be possible.
> Hence, this patch will make OVS more flexible to handle such
> rule insertion failures.

So, basically, you're not willing to name any switches.  I'm tired of
the vendor secrecy game.  They get to see all of our code but they're
never willing even to name themselves.

Sorry, I'm not going to willingly take this patch.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2018-07-31 Thread Aravind Prasad
 > > Currently, rule_insert() API does not have return value. There are
some possible
> > scenarios where rule insertions can fail at run-time even though the
static
> > checks during rule_construct() had passed previously.
> > Some possible scenarios for failure of rule insertions:
> > **) Rule insertions can fail dynamically in Hybrid mode (both Openflow
and
> > Normal switch functioning coexist) where the CAM space could get
suddenly
> > filled up by Normal switch functioning and Openflow gets devoid of
> > available space.
> > **) Some deployments could have separate independent layers for HW rule
> > insertions and application layer to interact with OVS. HW layer
> > could face any dynamic issue during rule handling which application
could
> > not have predicted/captured in rule-construction phase.
> > Rule-insert errors for bundles are handled too in this pull-request.
> >
> > Signed-off-by: Aravind Prasad S 
>
> >Which switches does this help?
>
> Hi Ben, These type of errors are possible in actual Hardware
> implementations of OVS. It is possible that ofproto and netdev providers
be
> implemented for an actual HW/NPU. Usually, in such cases, in the rule
> construct phase, all the static checks like verifying the qualifiers/
> actions, CAM full checks could be done and the other related
verifications.
> But during the rule insert phase, it is possible that the rule insertion
> may get failed in HW (runtime errors, HW errors and so on). Also, since HW
> switches may support Hybrid mode (coexistence of Normal and Openflow
> functioning), the possibility of this issue could be even more. Further,
> when rule-insertion fails, it results in a stale state where the
Controller
> and Switch Flow-DB differ. Hence, we need a way to rollback for
rule-insert
> phase also. Kindly let me know your views. And sorry for re-sending the
> review requests many times over. Will avoid in future.

> >Which switches does this help?

Hi Ben,

By "which" switches, you mean to ask to which vendor switches ?
In general, this patch will be very useful for all Switches which
implement OVS for their HW implementations, specifically,
will be useful for Switches which had implemented ofproto
and netdev provider APIs for their respective HW/NPU and
Kernel. And, in case of HW, the possibility of dynamic rule
insertion failures are usually higher. Static checks during
rule-construct phase are not sufficient and predicting
future failure predictions in construct phase may not be possible.
Hence, this patch will make OVS more flexible to handle such
rule insertion failures.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2018-07-31 Thread Ben Pfaff
On Sat, Jul 21, 2018 at 09:22:06AM +0530, Aravind Prasad wrote:
> > Currently, rule_insert() API does not have return value. There are some 
> > possible
> > scenarios where rule insertions can fail at run-time even though the static
> > checks during rule_construct() had passed previously.
> > Some possible scenarios for failure of rule insertions:
> > **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
> > Normal switch functioning coexist) where the CAM space could get suddenly
> > filled up by Normal switch functioning and Openflow gets devoid of
> > available space.
> > **) Some deployments could have separate independent layers for HW rule
> > insertions and application layer to interact with OVS. HW layer
> > could face any dynamic issue during rule handling which application could
> > not have predicted/captured in rule-construction phase.
> > Rule-insert errors for bundles are handled too in this pull-request.
> >
> > Signed-off-by: Aravind Prasad S 
> 
> >Which switches does this help?
> 
> Hi Ben, These type of errors are possible in actual Hardware
> implementations of OVS. It is possible that ofproto and netdev providers be
> implemented for an actual HW/NPU. Usually, in such cases, in the rule
> construct phase, all the static checks like verifying the qualifiers/
> actions, CAM full checks could be done and the other related verifications.
> But during the rule insert phase, it is possible that the rule insertion
> may get failed in HW (runtime errors, HW errors and so on). Also, since HW
> switches may support Hybrid mode (coexistence of Normal and Openflow
> functioning), the possibility of this issue could be even more. Further,
> when rule-insertion fails, it results in a stale state where the Controller
> and Switch Flow-DB differ. Hence, we need a way to rollback for rule-insert
> phase also. Kindly let me know your views. And sorry for re-sending the
> review requests many times over. Will avoid in future.

Which switches does this help?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2018-07-30 Thread Aravind Prasad
 > Currently, rule_insert() API does not have return value. There are some
possible
> scenarios where rule insertions can fail at run-time even though the
static
> checks during rule_construct() had passed previously.
> Some possible scenarios for failure of rule insertions:
> **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
> Normal switch functioning coexist) where the CAM space could get suddenly
> filled up by Normal switch functioning and Openflow gets devoid of
> available space.
> **) Some deployments could have separate independent layers for HW rule
> insertions and application layer to interact with OVS. HW layer
> could face any dynamic issue during rule handling which application could
> not have predicted/captured in rule-construction phase.
> Rule-insert errors for bundles are handled too in this pull-request.
>
> Signed-off-by: Aravind Prasad S 

> Which switches does this help?
> Hi Ben,
> These type of errors are possible in actual Hardware implementations
> of OVS. It is possible that ofproto and netdev providers be
> implemented for an actual HW/NPU. Usually, in such cases, in the rule
> construct phase, all the static checks like verifying the qualifiers/
> actions, CAM full checks could be done and the other related
> verifications.
> But during the rule insert phase, it is possible that
> the rule insertion may get failed in HW (runtime errors,
> HW errors and so on). Also, since HW switches may support Hybrid
> mode (coexistence of Normal and Openflow functioning), the
> possibility of this issue could be even more. Further, when
> rule-insertion fails, it results in a stale state where the
> Controller and Switch Flow-DB differ.
> Hence, we need a way to rollback for rule-insert phase also.
> Kindly let me know your views.


Hi Ben/Aaron/All,

In general, this patch will be very useful for all HW implementations
of OVS since the possibility of dynamic rule insertion failures
are high in such deployments. Static checks during
rule-construct phase are not sufficient and future failure predictions
in construct phase may not be possible. Hence, this patch will make
OVS more flexible to handle such failures.

Kindly review and let me know your views.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2018-07-28 Thread Aravind Prasad
 > Currently, rule_insert() API does not have return value. There are some
possible
> scenarios where rule insertions can fail at run-time even though the
static
> checks during rule_construct() had passed previously.
> Some possible scenarios for failure of rule insertions:
> **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
> Normal switch functioning coexist) where the CAM space could get suddenly
> filled up by Normal switch functioning and Openflow gets devoid of
> available space.
> **) Some deployments could have separate independent layers for HW rule
> insertions and application layer to interact with OVS. HW layer
> could face any dynamic issue during rule handling which application could
> not have predicted/captured in rule-construction phase.
> Rule-insert errors for bundles are handled too in this pull-request.
>
> Signed-off-by: Aravind Prasad S 

> Which switches does this help?
> Hi Ben,
> These type of errors are possible in actual Hardware implementations
> of OVS. It is possible that ofproto and netdev providers be
> implemented for an actual HW/NPU. Usually, in such cases, in the rule
> construct phase, all the static checks like verifying the qualifiers/
> actions, CAM full checks could be done and the other related
> verifications.
> But during the rule insert phase, it is possible that
> the rule insertion may get failed in HW (runtime errors,
> HW errors and so on). Also, since HW switches may support Hybrid
> mode (coexistence of Normal and Openflow functioning), the
> possibility of this issue could be even more. Further, when
> rule-insertion fails, it results in a stale state where the
> Controller and Switch Flow-DB differ.
> Hence, we need a way to rollback for rule-insert phase also.
> Kindly let me know your views.


Hi Ben/All,
Kindly review and let me know your views.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2018-07-25 Thread Aravind Prasad
> Currently, rule_insert() API does not have return value. There are some
possible
> scenarios where rule insertions can fail at run-time even though the
static
> checks during rule_construct() had passed previously.
> Some possible scenarios for failure of rule insertions:
> **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
> Normal switch functioning coexist) where the CAM space could get suddenly
> filled up by Normal switch functioning and Openflow gets devoid of
> available space.
> **) Some deployments could have separate independent layers for HW rule
> insertions and application layer to interact with OVS. HW layer
> could face any dynamic issue during rule handling which application could
> not have predicted/captured in rule-construction phase.
> Rule-insert errors for bundles are handled too in this pull-request.
>
> Signed-off-by: Aravind Prasad S 

> Which switches does this help?
> Hi Ben,
> These type of errors are possible in actual Hardware implementations
> of OVS. It is possible that ofproto and netdev providers be
> implemented for an actual HW/NPU. Usually, in such cases, in the rule
> construct phase, all the static checks like verifying the qualifiers/
> actions, CAM full checks could be done and the other related
> verifications.
> But during the rule insert phase, it is possible that
> the rule insertion may get failed in HW (runtime errors,
> HW errors and so on). Also, since HW switches may support Hybrid
> mode (coexistence of Normal and Openflow functioning), the
> possibility of this issue could be even more. Further, when
> rule-insertion fails, it results in a stale state where the
> Controller and Switch Flow-DB differ.
> Hence, we need a way to rollback for rule-insert phase also.

> Kindly let me know your views.


Hi Ben/All,
Kindly review and let me know your views.
Sorry for the previous junk mail.

On Sat, Jul 21, 2018 at 9:22 AM, Aravind Prasad  wrote:

>
> > Currently, rule_insert() API does not have return value. There are some 
> > possible
> > scenarios where rule insertions can fail at run-time even though the static
> > checks during rule_construct() had passed previously.
> > Some possible scenarios for failure of rule insertions:
> > **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
> > Normal switch functioning coexist) where the CAM space could get suddenly
> > filled up by Normal switch functioning and Openflow gets devoid of
> > available space.
> > **) Some deployments could have separate independent layers for HW rule
> > insertions and application layer to interact with OVS. HW layer
> > could face any dynamic issue during rule handling which application could
> > not have predicted/captured in rule-construction phase.
> > Rule-insert errors for bundles are handled too in this pull-request.
> >
> > Signed-off-by: Aravind Prasad S 
>
> >Which switches does this help?
>
> Hi Ben, These type of errors are possible in actual Hardware
> implementations of OVS. It is possible that ofproto and netdev providers be
> implemented for an actual HW/NPU. Usually, in such cases, in the rule
> construct phase, all the static checks like verifying the qualifiers/
> actions, CAM full checks could be done and the other related verifications.
> But during the rule insert phase, it is possible that the rule insertion
> may get failed in HW (runtime errors, HW errors and so on). Also, since HW
> switches may support Hybrid mode (coexistence of Normal and Openflow
> functioning), the possibility of this issue could be even more. Further,
> when rule-insertion fails, it results in a stale state where the Controller
> and Switch Flow-DB differ. Hence, we need a way to rollback for rule-insert
> phase also. Kindly let me know your views. And sorry for re-sending the
> review requests many times over. Will avoid in future.
>
>
> On Fri, Jul 20, 2018 at 10:20 PM, Ben Pfaff  wrote:
>
> On Thu, Jul 12, 2018 at 06:04:47PM +, Aravind Prasad S wrote:
>> > Currently, rule_insert() API does not have return value. There are some
>> possible
>> > scenarios where rule insertions can fail at run-time even though the
>> static
>> > checks during rule_construct() had passed previously.
>> > Some possible scenarios for failure of rule insertions:
>> > **) Rule insertions can fail dynamically in Hybrid mode (both Openflow
>> and
>> > Normal switch functioning coexist) where the CAM space could get
>> suddenly
>> > filled up by Normal switch functioning and Openflow gets devoid of
>> > available space.
>> > **) Some deployments could have separate independent layers for HW rule
>> > insertions and application layer to interact with OVS. HW layer
>> > could face any dynamic issue during rule handling which application
>> could
>> > not have predicted/captured in rule-construction phase.
>> > Rule-insert errors for bundles are handled too in this pull-request.
>> >
>> > Signed-off-by: Aravind Prasad S 
>>
>> Which switches does this help?
>>

Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2018-07-25 Thread Aravind Prasad
> Currently, rule_insert() API does not have return value. There are some
possible
> scenarios where rule insertions can fail at run-time even though the
static
> checks during rule_construct() had passed previously.
> Some possible scenarios for failure of rule insertions:
> **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
> Normal switch functioning coexist) where the CAM space could get suddenly
> filled up by Normal switch functioning and Openflow gets devoid of
> available space.
> **) Some deployments could have separate independent layers for HW rule
> insertions and application layer to interact with OVS. HW layer
> could face any dynamic issue during rule handling which application could
> not have predicted/captured in rule-construction phase.
> Rule-insert errors for bundles are handled too in this pull-request.
>
> Signed-off-by: Aravind Prasad S 

> Which switches does this help?
> Hi Ben,
> These type of errors are possible in actual Hardware implementations
> of OVS. It is possible that ofproto and netdev providers be
> implemented for an actual HW/NPU. Usually, in such cases, in the rule
> construct phase, all the static checks like verifying the qualifiers/
> actions, CAM full checks could be done and the other related
> verifications.
> But during the rule insert phase, it is possible that
> the rule insertion may get failed in HW (runtime errors,
> HW errors and so on). Also, since HW switches may support Hybrid
> mode (coexistence of Normal and Openflow functioning), the
> possibility of this issue could be even more. Further, when
> rule-insertion fails, it results in a stale state where the
> Controller and Switch Flow-DB differ.
> Hence, we need a way to rollback for rule-insert phase also.

> Kindly let me know your views.


Hi Ben/All,
Kindly review and let me know your views.

On Tue, Jul 24, 2018 at 5:15 AM, Aravind Prasad  wrote:

> > Currently, rule_insert() API does not have return value. There are some
> possible
> > scenarios where rule insertions can fail at run-time even though the
> static
> > checks during rule_construct() had passed previously.
> > Some possible scenarios for failure of rule insertions:
> > **) Rule insertions can fail dynamically in Hybrid mode (both Openflow
> and
> > Normal switch functioning coexist) where the CAM space could get suddenly
> > filled up by Normal switch functioning and Openflow gets devoid of
> > available space.
> > **) Some deployments could have separate independent layers for HW rule
> > insertions and application layer to interact with OVS. HW layer
> > could face any dynamic issue during rule handling which application could
> > not have predicted/captured in rule-construction phase.
> > Rule-insert errors for bundles are handled too in this pull-request.
> >
> > Signed-off-by: Aravind Prasad S 
>
> >Which switches does this help?
>
> >Hi Ben,
> >These type of errors are possible in actual Hardware implementations
> >of OVS. It is possible that ofproto and netdev providers be
> >implemented for an actual HW/NPU. Usually, in such cases, in the rule
> >construct phase, all the static checks like verifying the qualifiers/
> >actions, CAM full checks could be done and the other related
> >verifications.
> >But during the rule insert phase, it is possible that
> >the rule insertion may get failed in HW (runtime errors,
> >HW errors and so on). Also, since HW switches may support Hybrid
> >mode (coexistence of Normal and Openflow functioning), the
> >possibility of this issue could be even more. Further, when
> >rule-insertion fails, it results in a stale state where the
> >Controller and Switch Flow-DB differ.
> >Hence, we need a way to rollback for rule-insert phase also.
> >Kindly let me know your views.
> >And sorry for re-sending the review requests many times over.
> >Will avoid in future.
>
> Hi Ben/All,
>
> Kindly let me know your views.
>
>
> On Sat, Jul 21, 2018 at 9:22 AM, Aravind Prasad 
> wrote:
>
>>
>> > Currently, rule_insert() API does not have return value. There are some 
>> > possible
>> > scenarios where rule insertions can fail at run-time even though the static
>> > checks during rule_construct() had passed previously.
>> > Some possible scenarios for failure of rule insertions:
>> > **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
>> > Normal switch functioning coexist) where the CAM space could get suddenly
>> > filled up by Normal switch functioning and Openflow gets devoid of
>> > available space.
>> > **) Some deployments could have separate independent layers for HW rule
>> > insertions and application layer to interact with OVS. HW layer
>> > could face any dynamic issue during rule handling which application could
>> > not have predicted/captured in rule-construction phase.
>> > Rule-insert errors for bundles are handled too in this pull-request.
>> >
>> > Signed-off-by: Aravind Prasad S 
>>
>> >Which switches does this help?
>>
>> Hi Ben, These type 

Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2018-07-23 Thread Aravind Prasad
> Currently, rule_insert() API does not have return value. There are some
possible
> scenarios where rule insertions can fail at run-time even though the
static
> checks during rule_construct() had passed previously.
> Some possible scenarios for failure of rule insertions:
> **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
> Normal switch functioning coexist) where the CAM space could get suddenly
> filled up by Normal switch functioning and Openflow gets devoid of
> available space.
> **) Some deployments could have separate independent layers for HW rule
> insertions and application layer to interact with OVS. HW layer
> could face any dynamic issue during rule handling which application could
> not have predicted/captured in rule-construction phase.
> Rule-insert errors for bundles are handled too in this pull-request.
>
> Signed-off-by: Aravind Prasad S 

>Which switches does this help?

>Hi Ben,
>These type of errors are possible in actual Hardware implementations
>of OVS. It is possible that ofproto and netdev providers be
>implemented for an actual HW/NPU. Usually, in such cases, in the rule
>construct phase, all the static checks like verifying the qualifiers/
>actions, CAM full checks could be done and the other related
>verifications.
>But during the rule insert phase, it is possible that
>the rule insertion may get failed in HW (runtime errors,
>HW errors and so on). Also, since HW switches may support Hybrid
>mode (coexistence of Normal and Openflow functioning), the
>possibility of this issue could be even more. Further, when
>rule-insertion fails, it results in a stale state where the
>Controller and Switch Flow-DB differ.
>Hence, we need a way to rollback for rule-insert phase also.
>Kindly let me know your views.
>And sorry for re-sending the review requests many times over.
>Will avoid in future.

Hi Ben/All,

Kindly let me know your views.


On Sat, Jul 21, 2018 at 9:22 AM, Aravind Prasad  wrote:

>
> > Currently, rule_insert() API does not have return value. There are some 
> > possible
> > scenarios where rule insertions can fail at run-time even though the static
> > checks during rule_construct() had passed previously.
> > Some possible scenarios for failure of rule insertions:
> > **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
> > Normal switch functioning coexist) where the CAM space could get suddenly
> > filled up by Normal switch functioning and Openflow gets devoid of
> > available space.
> > **) Some deployments could have separate independent layers for HW rule
> > insertions and application layer to interact with OVS. HW layer
> > could face any dynamic issue during rule handling which application could
> > not have predicted/captured in rule-construction phase.
> > Rule-insert errors for bundles are handled too in this pull-request.
> >
> > Signed-off-by: Aravind Prasad S 
>
> >Which switches does this help?
>
> Hi Ben, These type of errors are possible in actual Hardware
> implementations of OVS. It is possible that ofproto and netdev providers be
> implemented for an actual HW/NPU. Usually, in such cases, in the rule
> construct phase, all the static checks like verifying the qualifiers/
> actions, CAM full checks could be done and the other related verifications.
> But during the rule insert phase, it is possible that the rule insertion
> may get failed in HW (runtime errors, HW errors and so on). Also, since HW
> switches may support Hybrid mode (coexistence of Normal and Openflow
> functioning), the possibility of this issue could be even more. Further,
> when rule-insertion fails, it results in a stale state where the Controller
> and Switch Flow-DB differ. Hence, we need a way to rollback for rule-insert
> phase also. Kindly let me know your views. And sorry for re-sending the
> review requests many times over. Will avoid in future.
>
>
> On Fri, Jul 20, 2018 at 10:20 PM, Ben Pfaff  wrote:
>
> On Thu, Jul 12, 2018 at 06:04:47PM +, Aravind Prasad S wrote:
>> > Currently, rule_insert() API does not have return value. There are some
>> possible
>> > scenarios where rule insertions can fail at run-time even though the
>> static
>> > checks during rule_construct() had passed previously.
>> > Some possible scenarios for failure of rule insertions:
>> > **) Rule insertions can fail dynamically in Hybrid mode (both Openflow
>> and
>> > Normal switch functioning coexist) where the CAM space could get
>> suddenly
>> > filled up by Normal switch functioning and Openflow gets devoid of
>> > available space.
>> > **) Some deployments could have separate independent layers for HW rule
>> > insertions and application layer to interact with OVS. HW layer
>> > could face any dynamic issue during rule handling which application
>> could
>> > not have predicted/captured in rule-construction phase.
>> > Rule-insert errors for bundles are handled too in this pull-request.
>> >
>> > Signed-off-by: Aravind Prasad S 
>>
>> Which 

Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2018-07-20 Thread Aravind Prasad
> Currently, rule_insert() API does not have return value. There are some 
> possible
> scenarios where rule insertions can fail at run-time even though the static
> checks during rule_construct() had passed previously.
> Some possible scenarios for failure of rule insertions:
> **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
> Normal switch functioning coexist) where the CAM space could get suddenly
> filled up by Normal switch functioning and Openflow gets devoid of
> available space.
> **) Some deployments could have separate independent layers for HW rule
> insertions and application layer to interact with OVS. HW layer
> could face any dynamic issue during rule handling which application could
> not have predicted/captured in rule-construction phase.
> Rule-insert errors for bundles are handled too in this pull-request.
>
> Signed-off-by: Aravind Prasad S 

>Which switches does this help?

Hi Ben, These type of errors are possible in actual Hardware
implementations of OVS. It is possible that ofproto and netdev providers be
implemented for an actual HW/NPU. Usually, in such cases, in the rule
construct phase, all the static checks like verifying the qualifiers/
actions, CAM full checks could be done and the other related verifications.
But during the rule insert phase, it is possible that the rule insertion
may get failed in HW (runtime errors, HW errors and so on). Also, since HW
switches may support Hybrid mode (coexistence of Normal and Openflow
functioning), the possibility of this issue could be even more. Further,
when rule-insertion fails, it results in a stale state where the Controller
and Switch Flow-DB differ. Hence, we need a way to rollback for rule-insert
phase also. Kindly let me know your views. And sorry for re-sending the
review requests many times over. Will avoid in future.

On Fri, Jul 20, 2018 at 10:20 PM, Ben Pfaff  wrote:

On Thu, Jul 12, 2018 at 06:04:47PM +, Aravind Prasad S wrote:
> > Currently, rule_insert() API does not have return value. There are some
> possible
> > scenarios where rule insertions can fail at run-time even though the
> static
> > checks during rule_construct() had passed previously.
> > Some possible scenarios for failure of rule insertions:
> > **) Rule insertions can fail dynamically in Hybrid mode (both Openflow
> and
> > Normal switch functioning coexist) where the CAM space could get suddenly
> > filled up by Normal switch functioning and Openflow gets devoid of
> > available space.
> > **) Some deployments could have separate independent layers for HW rule
> > insertions and application layer to interact with OVS. HW layer
> > could face any dynamic issue during rule handling which application could
> > not have predicted/captured in rule-construction phase.
> > Rule-insert errors for bundles are handled too in this pull-request.
> >
> > Signed-off-by: Aravind Prasad S 
>
> Which switches does this help?
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2018-07-20 Thread Ben Pfaff
It is super annoying to send a nagging message every day.  Do not do it.

On Fri, Jul 13, 2018 at 08:03:22AM +0530, Aravind Prasad wrote:
> Hi  Aaron/Ben/All,
> If possible, Kindly review the patch and let me know
> your views.
> 
> On Thu, Jul 12, 2018 at 11:34 PM, Aravind Prasad S 
> wrote:
> 
> > Currently, rule_insert() API does not have return value. There are some
> > possible
> > scenarios where rule insertions can fail at run-time even though the static
> > checks during rule_construct() had passed previously.
> > Some possible scenarios for failure of rule insertions:
> > **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
> > Normal switch functioning coexist) where the CAM space could get suddenly
> > filled up by Normal switch functioning and Openflow gets devoid of
> > available space.
> > **) Some deployments could have separate independent layers for HW rule
> > insertions and application layer to interact with OVS. HW layer
> > could face any dynamic issue during rule handling which application could
> > not have predicted/captured in rule-construction phase.
> > Rule-insert errors for bundles are handled too in this pull-request.
> >
> > Signed-off-by: Aravind Prasad S 
> > ---
> >  ofproto/ofproto-dpif.c |   4 +-
> >  ofproto/ofproto-provider.h |   6 +--
> >  ofproto/ofproto.c  | 105 ++
> > ---
> >  3 files changed, 85 insertions(+), 30 deletions(-)
> >
> > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> > index ad1e8af..d1678ed 100644
> > --- a/ofproto/ofproto-dpif.c
> > +++ b/ofproto/ofproto-dpif.c
> > @@ -4443,7 +4443,7 @@ rule_construct(struct rule *rule_)
> >  return 0;
> >  }
> >
> > -static void
> > +static enum ofperr
> >  rule_insert(struct rule *rule_, struct rule *old_rule_, bool
> > forward_counts)
> >  OVS_REQUIRES(ofproto_mutex)
> >  {
> > @@ -4473,6 +4473,8 @@ rule_insert(struct rule *rule_, struct rule
> > *old_rule_, bool forward_counts)
> >  ovs_mutex_unlock(>stats_mutex);
> >  ovs_mutex_unlock(_rule->stats_mutex);
> >  }
> > +
> > +return 0;
> >  }
> >
> >  static void
> > diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> > index 2b77b89..3f3d110 100644
> > --- a/ofproto/ofproto-provider.h
> > +++ b/ofproto/ofproto-provider.h
> > @@ -1297,8 +1297,8 @@ struct ofproto_class {
> >  struct rule *(*rule_alloc)(void);
> >  enum ofperr (*rule_construct)(struct rule *rule)
> >  /* OVS_REQUIRES(ofproto_mutex) */;
> > -void (*rule_insert)(struct rule *rule, struct rule *old_rule,
> > -bool forward_counts)
> > +enum ofperr (*rule_insert)(struct rule *rule, struct rule *old_rule,
> > +   bool forward_counts)
> >  /* OVS_REQUIRES(ofproto_mutex) */;
> >  void (*rule_delete)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex)
> > */;
> >  void (*rule_destruct)(struct rule *rule);
> > @@ -1952,7 +1952,7 @@ enum ofperr ofproto_flow_mod_learn_start(struct
> > ofproto_flow_mod *ofm)
> >  OVS_REQUIRES(ofproto_mutex);
> >  void ofproto_flow_mod_learn_revert(struct ofproto_flow_mod *ofm)
> >  OVS_REQUIRES(ofproto_mutex);
> > -void ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
> > +enum ofperr ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
> >struct ofproto *orig_ofproto)
> >  OVS_REQUIRES(ofproto_mutex);
> >  void ofproto_add_flow(struct ofproto *, const struct match *, int
> > priority,
> > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> > index f946e27..cb09ee6 100644
> > --- a/ofproto/ofproto.c
> > +++ b/ofproto/ofproto.c
> > @@ -245,10 +245,12 @@ static void replace_rule_revert(struct ofproto *,
> > struct rule *old_rule,
> >  struct rule *new_rule)
> >  OVS_REQUIRES(ofproto_mutex);
> >
> > -static void replace_rule_finish(struct ofproto *, struct ofproto_flow_mod
> > *,
> > -const struct openflow_mod_requester *,
> > -struct rule *old_rule, struct rule
> > *new_rule,
> > -struct ovs_list *dead_cookies)
> > +static enum ofperr replace_rule_finish(struct ofproto *,
> > +   struct ofproto_flow_mod *,
> > +   const struct
> > openflow_mod_requester *,
> > +   struct rule *old_rule,
> > +   struct rule *new_rule,
> > +   struct ovs_list *dead_cookies)
> >  OVS_REQUIRES(ofproto_mutex);
> >  static void delete_flows__(struct rule_collection *,
> > enum ofp_flow_removed_reason,
> > @@ -270,7 +272,7 @@ static enum ofperr ofproto_flow_mod_start(struct
> > ofproto *,
> >  static void ofproto_flow_mod_revert(struct ofproto *,
> > 

Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2018-07-20 Thread Ben Pfaff
On Thu, Jul 12, 2018 at 06:04:47PM +, Aravind Prasad S wrote:
> Currently, rule_insert() API does not have return value. There are some 
> possible
> scenarios where rule insertions can fail at run-time even though the static
> checks during rule_construct() had passed previously.
> Some possible scenarios for failure of rule insertions:
> **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
> Normal switch functioning coexist) where the CAM space could get suddenly
> filled up by Normal switch functioning and Openflow gets devoid of
> available space.
> **) Some deployments could have separate independent layers for HW rule
> insertions and application layer to interact with OVS. HW layer
> could face any dynamic issue during rule handling which application could
> not have predicted/captured in rule-construction phase.
> Rule-insert errors for bundles are handled too in this pull-request.
> 
> Signed-off-by: Aravind Prasad S 

Which switches does this help?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2018-07-19 Thread Aravind Prasad
Hi Ben/Aaron/All,

Kindly review the patch and let me know your views.

On Thu, Jul 12, 2018 at 11:34 PM, Aravind Prasad S 
wrote:

> Currently, rule_insert() API does not have return value. There are some
> possible
> scenarios where rule insertions can fail at run-time even though the static
> checks during rule_construct() had passed previously.
> Some possible scenarios for failure of rule insertions:
> **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
> Normal switch functioning coexist) where the CAM space could get suddenly
> filled up by Normal switch functioning and Openflow gets devoid of
> available space.
> **) Some deployments could have separate independent layers for HW rule
> insertions and application layer to interact with OVS. HW layer
> could face any dynamic issue during rule handling which application could
> not have predicted/captured in rule-construction phase.
> Rule-insert errors for bundles are handled too in this pull-request.
>
> Signed-off-by: Aravind Prasad S 
> ---
>  ofproto/ofproto-dpif.c |   4 +-
>  ofproto/ofproto-provider.h |   6 +--
>  ofproto/ofproto.c  | 105 ++
> ---
>  3 files changed, 85 insertions(+), 30 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index ad1e8af..d1678ed 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -4443,7 +4443,7 @@ rule_construct(struct rule *rule_)
>  return 0;
>  }
>
> -static void
> +static enum ofperr
>  rule_insert(struct rule *rule_, struct rule *old_rule_, bool
> forward_counts)
>  OVS_REQUIRES(ofproto_mutex)
>  {
> @@ -4473,6 +4473,8 @@ rule_insert(struct rule *rule_, struct rule
> *old_rule_, bool forward_counts)
>  ovs_mutex_unlock(>stats_mutex);
>  ovs_mutex_unlock(_rule->stats_mutex);
>  }
> +
> +return 0;
>  }
>
>  static void
> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> index 2b77b89..3f3d110 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -1297,8 +1297,8 @@ struct ofproto_class {
>  struct rule *(*rule_alloc)(void);
>  enum ofperr (*rule_construct)(struct rule *rule)
>  /* OVS_REQUIRES(ofproto_mutex) */;
> -void (*rule_insert)(struct rule *rule, struct rule *old_rule,
> -bool forward_counts)
> +enum ofperr (*rule_insert)(struct rule *rule, struct rule *old_rule,
> +   bool forward_counts)
>  /* OVS_REQUIRES(ofproto_mutex) */;
>  void (*rule_delete)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex)
> */;
>  void (*rule_destruct)(struct rule *rule);
> @@ -1952,7 +1952,7 @@ enum ofperr ofproto_flow_mod_learn_start(struct
> ofproto_flow_mod *ofm)
>  OVS_REQUIRES(ofproto_mutex);
>  void ofproto_flow_mod_learn_revert(struct ofproto_flow_mod *ofm)
>  OVS_REQUIRES(ofproto_mutex);
> -void ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
> +enum ofperr ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
>struct ofproto *orig_ofproto)
>  OVS_REQUIRES(ofproto_mutex);
>  void ofproto_add_flow(struct ofproto *, const struct match *, int
> priority,
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index f946e27..cb09ee6 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -245,10 +245,12 @@ static void replace_rule_revert(struct ofproto *,
> struct rule *old_rule,
>  struct rule *new_rule)
>  OVS_REQUIRES(ofproto_mutex);
>
> -static void replace_rule_finish(struct ofproto *, struct ofproto_flow_mod
> *,
> -const struct openflow_mod_requester *,
> -struct rule *old_rule, struct rule
> *new_rule,
> -struct ovs_list *dead_cookies)
> +static enum ofperr replace_rule_finish(struct ofproto *,
> +   struct ofproto_flow_mod *,
> +   const struct
> openflow_mod_requester *,
> +   struct rule *old_rule,
> +   struct rule *new_rule,
> +   struct ovs_list *dead_cookies)
>  OVS_REQUIRES(ofproto_mutex);
>  static void delete_flows__(struct rule_collection *,
> enum ofp_flow_removed_reason,
> @@ -270,7 +272,7 @@ static enum ofperr ofproto_flow_mod_start(struct
> ofproto *,
>  static void ofproto_flow_mod_revert(struct ofproto *,
>  struct ofproto_flow_mod *)
>  OVS_REQUIRES(ofproto_mutex);
> -static void ofproto_flow_mod_finish(struct ofproto *,
> +static enum ofperr ofproto_flow_mod_finish(struct ofproto *,
>  struct ofproto_flow_mod *,
>  const struct openflow_mod_requester *)
>  

Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2018-07-17 Thread Aravind Prasad
Hi Aaron/Ben/All,

If possible, Kindly review the patch and let me know your views.



On Thu, Jul 12, 2018 at 11:34 PM, Aravind Prasad S 
wrote:

> Currently, rule_insert() API does not have return value. There are some
> possible
> scenarios where rule insertions can fail at run-time even though the static
> checks during rule_construct() had passed previously.
> Some possible scenarios for failure of rule insertions:
> **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
> Normal switch functioning coexist) where the CAM space could get suddenly
> filled up by Normal switch functioning and Openflow gets devoid of
> available space.
> **) Some deployments could have separate independent layers for HW rule
> insertions and application layer to interact with OVS. HW layer
> could face any dynamic issue during rule handling which application could
> not have predicted/captured in rule-construction phase.
> Rule-insert errors for bundles are handled too in this pull-request.
>
> Signed-off-by: Aravind Prasad S 
> ---
>  ofproto/ofproto-dpif.c |   4 +-
>  ofproto/ofproto-provider.h |   6 +--
>  ofproto/ofproto.c  | 105 ++
> ---
>  3 files changed, 85 insertions(+), 30 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index ad1e8af..d1678ed 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -4443,7 +4443,7 @@ rule_construct(struct rule *rule_)
>  return 0;
>  }
>
> -static void
> +static enum ofperr
>  rule_insert(struct rule *rule_, struct rule *old_rule_, bool
> forward_counts)
>  OVS_REQUIRES(ofproto_mutex)
>  {
> @@ -4473,6 +4473,8 @@ rule_insert(struct rule *rule_, struct rule
> *old_rule_, bool forward_counts)
>  ovs_mutex_unlock(>stats_mutex);
>  ovs_mutex_unlock(_rule->stats_mutex);
>  }
> +
> +return 0;
>  }
>
>  static void
> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> index 2b77b89..3f3d110 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -1297,8 +1297,8 @@ struct ofproto_class {
>  struct rule *(*rule_alloc)(void);
>  enum ofperr (*rule_construct)(struct rule *rule)
>  /* OVS_REQUIRES(ofproto_mutex) */;
> -void (*rule_insert)(struct rule *rule, struct rule *old_rule,
> -bool forward_counts)
> +enum ofperr (*rule_insert)(struct rule *rule, struct rule *old_rule,
> +   bool forward_counts)
>  /* OVS_REQUIRES(ofproto_mutex) */;
>  void (*rule_delete)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex)
> */;
>  void (*rule_destruct)(struct rule *rule);
> @@ -1952,7 +1952,7 @@ enum ofperr ofproto_flow_mod_learn_start(struct
> ofproto_flow_mod *ofm)
>  OVS_REQUIRES(ofproto_mutex);
>  void ofproto_flow_mod_learn_revert(struct ofproto_flow_mod *ofm)
>  OVS_REQUIRES(ofproto_mutex);
> -void ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
> +enum ofperr ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
>struct ofproto *orig_ofproto)
>  OVS_REQUIRES(ofproto_mutex);
>  void ofproto_add_flow(struct ofproto *, const struct match *, int
> priority,
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index f946e27..cb09ee6 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -245,10 +245,12 @@ static void replace_rule_revert(struct ofproto *,
> struct rule *old_rule,
>  struct rule *new_rule)
>  OVS_REQUIRES(ofproto_mutex);
>
> -static void replace_rule_finish(struct ofproto *, struct ofproto_flow_mod
> *,
> -const struct openflow_mod_requester *,
> -struct rule *old_rule, struct rule
> *new_rule,
> -struct ovs_list *dead_cookies)
> +static enum ofperr replace_rule_finish(struct ofproto *,
> +   struct ofproto_flow_mod *,
> +   const struct
> openflow_mod_requester *,
> +   struct rule *old_rule,
> +   struct rule *new_rule,
> +   struct ovs_list *dead_cookies)
>  OVS_REQUIRES(ofproto_mutex);
>  static void delete_flows__(struct rule_collection *,
> enum ofp_flow_removed_reason,
> @@ -270,7 +272,7 @@ static enum ofperr ofproto_flow_mod_start(struct
> ofproto *,
>  static void ofproto_flow_mod_revert(struct ofproto *,
>  struct ofproto_flow_mod *)
>  OVS_REQUIRES(ofproto_mutex);
> -static void ofproto_flow_mod_finish(struct ofproto *,
> +static enum ofperr ofproto_flow_mod_finish(struct ofproto *,
>  struct ofproto_flow_mod *,
>  const struct openflow_mod_requester *)
>  

Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2018-07-16 Thread Aravind Prasad
 Hi Ben/Aaron/All,

Kindly review the patch and let me know your views.

On Thu, Jul 12, 2018 at 11:34 PM, Aravind Prasad S 
wrote:

> Currently, rule_insert() API does not have return value. There are some
> possible
> scenarios where rule insertions can fail at run-time even though the static
> checks during rule_construct() had passed previously.
> Some possible scenarios for failure of rule insertions:
> **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
> Normal switch functioning coexist) where the CAM space could get suddenly
> filled up by Normal switch functioning and Openflow gets devoid of
> available space.
> **) Some deployments could have separate independent layers for HW rule
> insertions and application layer to interact with OVS. HW layer
> could face any dynamic issue during rule handling which application could
> not have predicted/captured in rule-construction phase.
> Rule-insert errors for bundles are handled too in this pull-request.
>
> Signed-off-by: Aravind Prasad S 
> ---
>  ofproto/ofproto-dpif.c |   4 +-
>  ofproto/ofproto-provider.h |   6 +--
>  ofproto/ofproto.c  | 105 ++
> ---
>  3 files changed, 85 insertions(+), 30 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index ad1e8af..d1678ed 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -4443,7 +4443,7 @@ rule_construct(struct rule *rule_)
>  return 0;
>  }
>
> -static void
> +static enum ofperr
>  rule_insert(struct rule *rule_, struct rule *old_rule_, bool
> forward_counts)
>  OVS_REQUIRES(ofproto_mutex)
>  {
> @@ -4473,6 +4473,8 @@ rule_insert(struct rule *rule_, struct rule
> *old_rule_, bool forward_counts)
>  ovs_mutex_unlock(>stats_mutex);
>  ovs_mutex_unlock(_rule->stats_mutex);
>  }
> +
> +return 0;
>  }
>
>  static void
> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> index 2b77b89..3f3d110 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -1297,8 +1297,8 @@ struct ofproto_class {
>  struct rule *(*rule_alloc)(void);
>  enum ofperr (*rule_construct)(struct rule *rule)
>  /* OVS_REQUIRES(ofproto_mutex) */;
> -void (*rule_insert)(struct rule *rule, struct rule *old_rule,
> -bool forward_counts)
> +enum ofperr (*rule_insert)(struct rule *rule, struct rule *old_rule,
> +   bool forward_counts)
>  /* OVS_REQUIRES(ofproto_mutex) */;
>  void (*rule_delete)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex)
> */;
>  void (*rule_destruct)(struct rule *rule);
> @@ -1952,7 +1952,7 @@ enum ofperr ofproto_flow_mod_learn_start(struct
> ofproto_flow_mod *ofm)
>  OVS_REQUIRES(ofproto_mutex);
>  void ofproto_flow_mod_learn_revert(struct ofproto_flow_mod *ofm)
>  OVS_REQUIRES(ofproto_mutex);
> -void ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
> +enum ofperr ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
>struct ofproto *orig_ofproto)
>  OVS_REQUIRES(ofproto_mutex);
>  void ofproto_add_flow(struct ofproto *, const struct match *, int
> priority,
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index f946e27..cb09ee6 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -245,10 +245,12 @@ static void replace_rule_revert(struct ofproto *,
> struct rule *old_rule,
>  struct rule *new_rule)
>  OVS_REQUIRES(ofproto_mutex);
>
> -static void replace_rule_finish(struct ofproto *, struct ofproto_flow_mod
> *,
> -const struct openflow_mod_requester *,
> -struct rule *old_rule, struct rule
> *new_rule,
> -struct ovs_list *dead_cookies)
> +static enum ofperr replace_rule_finish(struct ofproto *,
> +   struct ofproto_flow_mod *,
> +   const struct
> openflow_mod_requester *,
> +   struct rule *old_rule,
> +   struct rule *new_rule,
> +   struct ovs_list *dead_cookies)
>  OVS_REQUIRES(ofproto_mutex);
>  static void delete_flows__(struct rule_collection *,
> enum ofp_flow_removed_reason,
> @@ -270,7 +272,7 @@ static enum ofperr ofproto_flow_mod_start(struct
> ofproto *,
>  static void ofproto_flow_mod_revert(struct ofproto *,
>  struct ofproto_flow_mod *)
>  OVS_REQUIRES(ofproto_mutex);
> -static void ofproto_flow_mod_finish(struct ofproto *,
> +static enum ofperr ofproto_flow_mod_finish(struct ofproto *,
>  struct ofproto_flow_mod *,
>  const struct openflow_mod_requester *)
>  

Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2018-07-15 Thread Aravind Prasad
Hi Aaron/Ben/All,

Kindly review the patch and let me know your views.


On Thu, Jul 12, 2018 at 11:34 PM, Aravind Prasad S 
wrote:

> Currently, rule_insert() API does not have return value. There are some
> possible
> scenarios where rule insertions can fail at run-time even though the static
> checks during rule_construct() had passed previously.
> Some possible scenarios for failure of rule insertions:
> **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
> Normal switch functioning coexist) where the CAM space could get suddenly
> filled up by Normal switch functioning and Openflow gets devoid of
> available space.
> **) Some deployments could have separate independent layers for HW rule
> insertions and application layer to interact with OVS. HW layer
> could face any dynamic issue during rule handling which application could
> not have predicted/captured in rule-construction phase.
> Rule-insert errors for bundles are handled too in this pull-request.
>
> Signed-off-by: Aravind Prasad S 
> ---
>  ofproto/ofproto-dpif.c |   4 +-
>  ofproto/ofproto-provider.h |   6 +--
>  ofproto/ofproto.c  | 105 ++
> ---
>  3 files changed, 85 insertions(+), 30 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index ad1e8af..d1678ed 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -4443,7 +4443,7 @@ rule_construct(struct rule *rule_)
>  return 0;
>  }
>
> -static void
> +static enum ofperr
>  rule_insert(struct rule *rule_, struct rule *old_rule_, bool
> forward_counts)
>  OVS_REQUIRES(ofproto_mutex)
>  {
> @@ -4473,6 +4473,8 @@ rule_insert(struct rule *rule_, struct rule
> *old_rule_, bool forward_counts)
>  ovs_mutex_unlock(>stats_mutex);
>  ovs_mutex_unlock(_rule->stats_mutex);
>  }
> +
> +return 0;
>  }
>
>  static void
> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> index 2b77b89..3f3d110 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -1297,8 +1297,8 @@ struct ofproto_class {
>  struct rule *(*rule_alloc)(void);
>  enum ofperr (*rule_construct)(struct rule *rule)
>  /* OVS_REQUIRES(ofproto_mutex) */;
> -void (*rule_insert)(struct rule *rule, struct rule *old_rule,
> -bool forward_counts)
> +enum ofperr (*rule_insert)(struct rule *rule, struct rule *old_rule,
> +   bool forward_counts)
>  /* OVS_REQUIRES(ofproto_mutex) */;
>  void (*rule_delete)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex)
> */;
>  void (*rule_destruct)(struct rule *rule);
> @@ -1952,7 +1952,7 @@ enum ofperr ofproto_flow_mod_learn_start(struct
> ofproto_flow_mod *ofm)
>  OVS_REQUIRES(ofproto_mutex);
>  void ofproto_flow_mod_learn_revert(struct ofproto_flow_mod *ofm)
>  OVS_REQUIRES(ofproto_mutex);
> -void ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
> +enum ofperr ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
>struct ofproto *orig_ofproto)
>  OVS_REQUIRES(ofproto_mutex);
>  void ofproto_add_flow(struct ofproto *, const struct match *, int
> priority,
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index f946e27..cb09ee6 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -245,10 +245,12 @@ static void replace_rule_revert(struct ofproto *,
> struct rule *old_rule,
>  struct rule *new_rule)
>  OVS_REQUIRES(ofproto_mutex);
>
> -static void replace_rule_finish(struct ofproto *, struct ofproto_flow_mod
> *,
> -const struct openflow_mod_requester *,
> -struct rule *old_rule, struct rule
> *new_rule,
> -struct ovs_list *dead_cookies)
> +static enum ofperr replace_rule_finish(struct ofproto *,
> +   struct ofproto_flow_mod *,
> +   const struct
> openflow_mod_requester *,
> +   struct rule *old_rule,
> +   struct rule *new_rule,
> +   struct ovs_list *dead_cookies)
>  OVS_REQUIRES(ofproto_mutex);
>  static void delete_flows__(struct rule_collection *,
> enum ofp_flow_removed_reason,
> @@ -270,7 +272,7 @@ static enum ofperr ofproto_flow_mod_start(struct
> ofproto *,
>  static void ofproto_flow_mod_revert(struct ofproto *,
>  struct ofproto_flow_mod *)
>  OVS_REQUIRES(ofproto_mutex);
> -static void ofproto_flow_mod_finish(struct ofproto *,
> +static enum ofperr ofproto_flow_mod_finish(struct ofproto *,
>  struct ofproto_flow_mod *,
>  const struct openflow_mod_requester *)
>  

Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2018-07-15 Thread Aravind Prasad
Hi Ben/Aaron,All,

Kindly review the patch and let me know your views.

On Thu, Jul 12, 2018 at 11:34 PM, Aravind Prasad S 
wrote:

> Currently, rule_insert() API does not have return value. There are some
> possible
> scenarios where rule insertions can fail at run-time even though the static
> checks during rule_construct() had passed previously.
> Some possible scenarios for failure of rule insertions:
> **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
> Normal switch functioning coexist) where the CAM space could get suddenly
> filled up by Normal switch functioning and Openflow gets devoid of
> available space.
> **) Some deployments could have separate independent layers for HW rule
> insertions and application layer to interact with OVS. HW layer
> could face any dynamic issue during rule handling which application could
> not have predicted/captured in rule-construction phase.
> Rule-insert errors for bundles are handled too in this pull-request.
>
> Signed-off-by: Aravind Prasad S 
> ---
>  ofproto/ofproto-dpif.c |   4 +-
>  ofproto/ofproto-provider.h |   6 +--
>  ofproto/ofproto.c  | 105 ++
> ---
>  3 files changed, 85 insertions(+), 30 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index ad1e8af..d1678ed 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -4443,7 +4443,7 @@ rule_construct(struct rule *rule_)
>  return 0;
>  }
>
> -static void
> +static enum ofperr
>  rule_insert(struct rule *rule_, struct rule *old_rule_, bool
> forward_counts)
>  OVS_REQUIRES(ofproto_mutex)
>  {
> @@ -4473,6 +4473,8 @@ rule_insert(struct rule *rule_, struct rule
> *old_rule_, bool forward_counts)
>  ovs_mutex_unlock(>stats_mutex);
>  ovs_mutex_unlock(_rule->stats_mutex);
>  }
> +
> +return 0;
>  }
>
>  static void
> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> index 2b77b89..3f3d110 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -1297,8 +1297,8 @@ struct ofproto_class {
>  struct rule *(*rule_alloc)(void);
>  enum ofperr (*rule_construct)(struct rule *rule)
>  /* OVS_REQUIRES(ofproto_mutex) */;
> -void (*rule_insert)(struct rule *rule, struct rule *old_rule,
> -bool forward_counts)
> +enum ofperr (*rule_insert)(struct rule *rule, struct rule *old_rule,
> +   bool forward_counts)
>  /* OVS_REQUIRES(ofproto_mutex) */;
>  void (*rule_delete)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex)
> */;
>  void (*rule_destruct)(struct rule *rule);
> @@ -1952,7 +1952,7 @@ enum ofperr ofproto_flow_mod_learn_start(struct
> ofproto_flow_mod *ofm)
>  OVS_REQUIRES(ofproto_mutex);
>  void ofproto_flow_mod_learn_revert(struct ofproto_flow_mod *ofm)
>  OVS_REQUIRES(ofproto_mutex);
> -void ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
> +enum ofperr ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
>struct ofproto *orig_ofproto)
>  OVS_REQUIRES(ofproto_mutex);
>  void ofproto_add_flow(struct ofproto *, const struct match *, int
> priority,
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index f946e27..cb09ee6 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -245,10 +245,12 @@ static void replace_rule_revert(struct ofproto *,
> struct rule *old_rule,
>  struct rule *new_rule)
>  OVS_REQUIRES(ofproto_mutex);
>
> -static void replace_rule_finish(struct ofproto *, struct ofproto_flow_mod
> *,
> -const struct openflow_mod_requester *,
> -struct rule *old_rule, struct rule
> *new_rule,
> -struct ovs_list *dead_cookies)
> +static enum ofperr replace_rule_finish(struct ofproto *,
> +   struct ofproto_flow_mod *,
> +   const struct
> openflow_mod_requester *,
> +   struct rule *old_rule,
> +   struct rule *new_rule,
> +   struct ovs_list *dead_cookies)
>  OVS_REQUIRES(ofproto_mutex);
>  static void delete_flows__(struct rule_collection *,
> enum ofp_flow_removed_reason,
> @@ -270,7 +272,7 @@ static enum ofperr ofproto_flow_mod_start(struct
> ofproto *,
>  static void ofproto_flow_mod_revert(struct ofproto *,
>  struct ofproto_flow_mod *)
>  OVS_REQUIRES(ofproto_mutex);
> -static void ofproto_flow_mod_finish(struct ofproto *,
> +static enum ofperr ofproto_flow_mod_finish(struct ofproto *,
>  struct ofproto_flow_mod *,
>  const struct openflow_mod_requester *)
>  

Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2018-07-13 Thread Aravind Prasad
Hi Aaron/Ben/All,

Kindly review the patch and let me know your views.


On Thu, Jul 12, 2018 at 11:34 PM, Aravind Prasad S 
wrote:

> Currently, rule_insert() API does not have return value. There are some
> possible
> scenarios where rule insertions can fail at run-time even though the static
> checks during rule_construct() had passed previously.
> Some possible scenarios for failure of rule insertions:
> **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
> Normal switch functioning coexist) where the CAM space could get suddenly
> filled up by Normal switch functioning and Openflow gets devoid of
> available space.
> **) Some deployments could have separate independent layers for HW rule
> insertions and application layer to interact with OVS. HW layer
> could face any dynamic issue during rule handling which application could
> not have predicted/captured in rule-construction phase.
> Rule-insert errors for bundles are handled too in this pull-request.
>
> Signed-off-by: Aravind Prasad S 
> ---
>  ofproto/ofproto-dpif.c |   4 +-
>  ofproto/ofproto-provider.h |   6 +--
>  ofproto/ofproto.c  | 105 ++
> ---
>  3 files changed, 85 insertions(+), 30 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index ad1e8af..d1678ed 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -4443,7 +4443,7 @@ rule_construct(struct rule *rule_)
>  return 0;
>  }
>
> -static void
> +static enum ofperr
>  rule_insert(struct rule *rule_, struct rule *old_rule_, bool
> forward_counts)
>  OVS_REQUIRES(ofproto_mutex)
>  {
> @@ -4473,6 +4473,8 @@ rule_insert(struct rule *rule_, struct rule
> *old_rule_, bool forward_counts)
>  ovs_mutex_unlock(>stats_mutex);
>  ovs_mutex_unlock(_rule->stats_mutex);
>  }
> +
> +return 0;
>  }
>
>  static void
> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> index 2b77b89..3f3d110 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -1297,8 +1297,8 @@ struct ofproto_class {
>  struct rule *(*rule_alloc)(void);
>  enum ofperr (*rule_construct)(struct rule *rule)
>  /* OVS_REQUIRES(ofproto_mutex) */;
> -void (*rule_insert)(struct rule *rule, struct rule *old_rule,
> -bool forward_counts)
> +enum ofperr (*rule_insert)(struct rule *rule, struct rule *old_rule,
> +   bool forward_counts)
>  /* OVS_REQUIRES(ofproto_mutex) */;
>  void (*rule_delete)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex)
> */;
>  void (*rule_destruct)(struct rule *rule);
> @@ -1952,7 +1952,7 @@ enum ofperr ofproto_flow_mod_learn_start(struct
> ofproto_flow_mod *ofm)
>  OVS_REQUIRES(ofproto_mutex);
>  void ofproto_flow_mod_learn_revert(struct ofproto_flow_mod *ofm)
>  OVS_REQUIRES(ofproto_mutex);
> -void ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
> +enum ofperr ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
>struct ofproto *orig_ofproto)
>  OVS_REQUIRES(ofproto_mutex);
>  void ofproto_add_flow(struct ofproto *, const struct match *, int
> priority,
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index f946e27..cb09ee6 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -245,10 +245,12 @@ static void replace_rule_revert(struct ofproto *,
> struct rule *old_rule,
>  struct rule *new_rule)
>  OVS_REQUIRES(ofproto_mutex);
>
> -static void replace_rule_finish(struct ofproto *, struct ofproto_flow_mod
> *,
> -const struct openflow_mod_requester *,
> -struct rule *old_rule, struct rule
> *new_rule,
> -struct ovs_list *dead_cookies)
> +static enum ofperr replace_rule_finish(struct ofproto *,
> +   struct ofproto_flow_mod *,
> +   const struct
> openflow_mod_requester *,
> +   struct rule *old_rule,
> +   struct rule *new_rule,
> +   struct ovs_list *dead_cookies)
>  OVS_REQUIRES(ofproto_mutex);
>  static void delete_flows__(struct rule_collection *,
> enum ofp_flow_removed_reason,
> @@ -270,7 +272,7 @@ static enum ofperr ofproto_flow_mod_start(struct
> ofproto *,
>  static void ofproto_flow_mod_revert(struct ofproto *,
>  struct ofproto_flow_mod *)
>  OVS_REQUIRES(ofproto_mutex);
> -static void ofproto_flow_mod_finish(struct ofproto *,
> +static enum ofperr ofproto_flow_mod_finish(struct ofproto *,
>  struct ofproto_flow_mod *,
>  const struct openflow_mod_requester *)
>  

Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2018-07-12 Thread Aravind Prasad
Hi  Aaron/Ben/All,
If possible, Kindly review the patch and let me know
your views.

On Thu, Jul 12, 2018 at 11:34 PM, Aravind Prasad S 
wrote:

> Currently, rule_insert() API does not have return value. There are some
> possible
> scenarios where rule insertions can fail at run-time even though the static
> checks during rule_construct() had passed previously.
> Some possible scenarios for failure of rule insertions:
> **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
> Normal switch functioning coexist) where the CAM space could get suddenly
> filled up by Normal switch functioning and Openflow gets devoid of
> available space.
> **) Some deployments could have separate independent layers for HW rule
> insertions and application layer to interact with OVS. HW layer
> could face any dynamic issue during rule handling which application could
> not have predicted/captured in rule-construction phase.
> Rule-insert errors for bundles are handled too in this pull-request.
>
> Signed-off-by: Aravind Prasad S 
> ---
>  ofproto/ofproto-dpif.c |   4 +-
>  ofproto/ofproto-provider.h |   6 +--
>  ofproto/ofproto.c  | 105 ++
> ---
>  3 files changed, 85 insertions(+), 30 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index ad1e8af..d1678ed 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -4443,7 +4443,7 @@ rule_construct(struct rule *rule_)
>  return 0;
>  }
>
> -static void
> +static enum ofperr
>  rule_insert(struct rule *rule_, struct rule *old_rule_, bool
> forward_counts)
>  OVS_REQUIRES(ofproto_mutex)
>  {
> @@ -4473,6 +4473,8 @@ rule_insert(struct rule *rule_, struct rule
> *old_rule_, bool forward_counts)
>  ovs_mutex_unlock(>stats_mutex);
>  ovs_mutex_unlock(_rule->stats_mutex);
>  }
> +
> +return 0;
>  }
>
>  static void
> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> index 2b77b89..3f3d110 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -1297,8 +1297,8 @@ struct ofproto_class {
>  struct rule *(*rule_alloc)(void);
>  enum ofperr (*rule_construct)(struct rule *rule)
>  /* OVS_REQUIRES(ofproto_mutex) */;
> -void (*rule_insert)(struct rule *rule, struct rule *old_rule,
> -bool forward_counts)
> +enum ofperr (*rule_insert)(struct rule *rule, struct rule *old_rule,
> +   bool forward_counts)
>  /* OVS_REQUIRES(ofproto_mutex) */;
>  void (*rule_delete)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex)
> */;
>  void (*rule_destruct)(struct rule *rule);
> @@ -1952,7 +1952,7 @@ enum ofperr ofproto_flow_mod_learn_start(struct
> ofproto_flow_mod *ofm)
>  OVS_REQUIRES(ofproto_mutex);
>  void ofproto_flow_mod_learn_revert(struct ofproto_flow_mod *ofm)
>  OVS_REQUIRES(ofproto_mutex);
> -void ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
> +enum ofperr ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
>struct ofproto *orig_ofproto)
>  OVS_REQUIRES(ofproto_mutex);
>  void ofproto_add_flow(struct ofproto *, const struct match *, int
> priority,
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index f946e27..cb09ee6 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -245,10 +245,12 @@ static void replace_rule_revert(struct ofproto *,
> struct rule *old_rule,
>  struct rule *new_rule)
>  OVS_REQUIRES(ofproto_mutex);
>
> -static void replace_rule_finish(struct ofproto *, struct ofproto_flow_mod
> *,
> -const struct openflow_mod_requester *,
> -struct rule *old_rule, struct rule
> *new_rule,
> -struct ovs_list *dead_cookies)
> +static enum ofperr replace_rule_finish(struct ofproto *,
> +   struct ofproto_flow_mod *,
> +   const struct
> openflow_mod_requester *,
> +   struct rule *old_rule,
> +   struct rule *new_rule,
> +   struct ovs_list *dead_cookies)
>  OVS_REQUIRES(ofproto_mutex);
>  static void delete_flows__(struct rule_collection *,
> enum ofp_flow_removed_reason,
> @@ -270,7 +272,7 @@ static enum ofperr ofproto_flow_mod_start(struct
> ofproto *,
>  static void ofproto_flow_mod_revert(struct ofproto *,
>  struct ofproto_flow_mod *)
>  OVS_REQUIRES(ofproto_mutex);
> -static void ofproto_flow_mod_finish(struct ofproto *,
> +static enum ofperr ofproto_flow_mod_finish(struct ofproto *,
>  struct ofproto_flow_mod *,
>  const struct openflow_mod_requester *)
>  

Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2018-07-12 Thread Aravind Prasad
Hi Aaron,

Thanks a lot for the review.
Submitted the patch with comments addressed as below.

> /doesnot/doesn't/  or /doesnot/does not/
> Or maybe even just remove that sentence.
> :)

Addressed. Changed to "does not"

>  This second if looks strange.  I think it should be removed.

Addressed.

> I don't know if there are any implications to passing 0 for an ofperr.
> I don't see an issue in this patch, but I'm not sure if any callers in
> the future may be affected.  There was previously a 0 value used for
> OFPBIC_BAD_EXP_TYPE - I don't even know if it would matter now.  Maybe
> it's better to have these return an integer type and then it might make
> the interface clearer.  WDYT?

As far as i had walk thro'd the code, error=0 is considered the
success value and non-zero values as failure. Hence, the same was
followed.
Also, I feel your suggestion for using a integer type for success is
valid but has to be handled as a separate cleanup patch which fixes
this behavior across the entire codebase.

Thanks,
Aravind Prasad S


On Thu, Jul 12, 2018 at 8:22 PM, Aaron Conole  wrote:

> Aravind Prasad S  writes:
>
> > Currently, rule_insert() API doesnot have return value. There are some
> possible
>
> /doesnot/doesn't/  or /doesnot/does not/
>
> Or maybe even just remove that sentence.
>
> :)
>
> > scenarios where rule insertions can fail at run-time even though the
> static
> > checks during rule_construct() had passed previously.
> > Some possible scenarios for failure of rule insertions:
> > **) Rule insertions can fail dynamically in Hybrid mode (both Openflow
> and
> > Normal switch functioning coexist) where the CAM space could get suddenly
> > filled up by Normal switch functioning and Openflow gets devoid of
> > available space.
> > **) Some deployments could have separate independent layers for HW rule
> > insertions and application layer to interact with OVS. HW layer
> > could face any dynamic issue during rule handling which application could
> > not have predicted/captured in rule-construction phase.
> > Rule-insert errors for bundles are also handled in this patch.
> >
> > Signed-off-by: Aravind Prasad S 
> > ---
> >  ofproto/ofproto-dpif.c |   4 +-
> >  ofproto/ofproto-provider.h |   6 +--
> >  ofproto/ofproto.c  | 100 ++
> +--
> >  3 files changed, 84 insertions(+), 26 deletions(-)
> >
> > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> > index ad1e8af..d1678ed 100644
> > --- a/ofproto/ofproto-dpif.c
> > +++ b/ofproto/ofproto-dpif.c
> > @@ -4443,7 +4443,7 @@ rule_construct(struct rule *rule_)
> >  return 0;
> >  }
> >
> > -static void
> > +static enum ofperr
> >  rule_insert(struct rule *rule_, struct rule *old_rule_, bool
> forward_counts)
> >  OVS_REQUIRES(ofproto_mutex)
> >  {
> > @@ -4473,6 +4473,8 @@ rule_insert(struct rule *rule_, struct rule
> *old_rule_, bool forward_counts)
> >  ovs_mutex_unlock(>stats_mutex);
> >  ovs_mutex_unlock(_rule->stats_mutex);
> >  }
> > +
> > +return 0;
> >  }
> >
> >  static void
> > diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> > index 2b77b89..3f3d110 100644
> > --- a/ofproto/ofproto-provider.h
> > +++ b/ofproto/ofproto-provider.h
> > @@ -1297,8 +1297,8 @@ struct ofproto_class {
> >  struct rule *(*rule_alloc)(void);
> >  enum ofperr (*rule_construct)(struct rule *rule)
> >  /* OVS_REQUIRES(ofproto_mutex) */;
> > -void (*rule_insert)(struct rule *rule, struct rule *old_rule,
> > -bool forward_counts)
> > +enum ofperr (*rule_insert)(struct rule *rule, struct rule *old_rule,
> > +   bool forward_counts)
> >  /* OVS_REQUIRES(ofproto_mutex) */;
> >  void (*rule_delete)(struct rule *rule) /*
> OVS_REQUIRES(ofproto_mutex) */;
> >  void (*rule_destruct)(struct rule *rule);
> > @@ -1952,7 +1952,7 @@ enum ofperr ofproto_flow_mod_learn_start(struct
> ofproto_flow_mod *ofm)
> >  OVS_REQUIRES(ofproto_mutex);
> >  void ofproto_flow_mod_learn_revert(struct ofproto_flow_mod *ofm)
> >  OVS_REQUIRES(ofproto_mutex);
> > -void ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
> > +enum ofperr ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
> >struct ofproto *orig_ofproto)
> >  OVS_REQUIRES(ofproto_mutex);
> >  void ofproto_add_flow(struct ofproto *, const struct match *, int
> priority,
> > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> > index f946e27..03597dc 100644
> > --- a/ofproto/ofproto.c
> > +++ b/ofproto/ofproto.c
> > @@ -245,10 +245,12 @@ static void replace_rule_revert(struct ofproto *,
> struct rule *old_rule,
> >  struct rule *new_rule)
> >  OVS_REQUIRES(ofproto_mutex);
> >
> > -static void replace_rule_finish(struct ofproto *, struct
> ofproto_flow_mod *,
> > -const struct openflow_mod_requester *,

[ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2018-07-12 Thread Aravind Prasad S
Currently, rule_insert() API does not have return value. There are some possible
scenarios where rule insertions can fail at run-time even though the static
checks during rule_construct() had passed previously.
Some possible scenarios for failure of rule insertions:
**) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
Normal switch functioning coexist) where the CAM space could get suddenly
filled up by Normal switch functioning and Openflow gets devoid of
available space.
**) Some deployments could have separate independent layers for HW rule
insertions and application layer to interact with OVS. HW layer
could face any dynamic issue during rule handling which application could
not have predicted/captured in rule-construction phase.
Rule-insert errors for bundles are handled too in this pull-request.

Signed-off-by: Aravind Prasad S 
---
 ofproto/ofproto-dpif.c |   4 +-
 ofproto/ofproto-provider.h |   6 +--
 ofproto/ofproto.c  | 105 ++---
 3 files changed, 85 insertions(+), 30 deletions(-)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index ad1e8af..d1678ed 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4443,7 +4443,7 @@ rule_construct(struct rule *rule_)
 return 0;
 }
 
-static void
+static enum ofperr
 rule_insert(struct rule *rule_, struct rule *old_rule_, bool forward_counts)
 OVS_REQUIRES(ofproto_mutex)
 {
@@ -4473,6 +4473,8 @@ rule_insert(struct rule *rule_, struct rule *old_rule_, 
bool forward_counts)
 ovs_mutex_unlock(>stats_mutex);
 ovs_mutex_unlock(_rule->stats_mutex);
 }
+
+return 0;
 }
 
 static void
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 2b77b89..3f3d110 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -1297,8 +1297,8 @@ struct ofproto_class {
 struct rule *(*rule_alloc)(void);
 enum ofperr (*rule_construct)(struct rule *rule)
 /* OVS_REQUIRES(ofproto_mutex) */;
-void (*rule_insert)(struct rule *rule, struct rule *old_rule,
-bool forward_counts)
+enum ofperr (*rule_insert)(struct rule *rule, struct rule *old_rule,
+   bool forward_counts)
 /* OVS_REQUIRES(ofproto_mutex) */;
 void (*rule_delete)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex) */;
 void (*rule_destruct)(struct rule *rule);
@@ -1952,7 +1952,7 @@ enum ofperr ofproto_flow_mod_learn_start(struct 
ofproto_flow_mod *ofm)
 OVS_REQUIRES(ofproto_mutex);
 void ofproto_flow_mod_learn_revert(struct ofproto_flow_mod *ofm)
 OVS_REQUIRES(ofproto_mutex);
-void ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
+enum ofperr ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
   struct ofproto *orig_ofproto)
 OVS_REQUIRES(ofproto_mutex);
 void ofproto_add_flow(struct ofproto *, const struct match *, int priority,
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index f946e27..cb09ee6 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -245,10 +245,12 @@ static void replace_rule_revert(struct ofproto *, struct 
rule *old_rule,
 struct rule *new_rule)
 OVS_REQUIRES(ofproto_mutex);
 
-static void replace_rule_finish(struct ofproto *, struct ofproto_flow_mod *,
-const struct openflow_mod_requester *,
-struct rule *old_rule, struct rule *new_rule,
-struct ovs_list *dead_cookies)
+static enum ofperr replace_rule_finish(struct ofproto *,
+   struct ofproto_flow_mod *,
+   const struct openflow_mod_requester *,
+   struct rule *old_rule,
+   struct rule *new_rule,
+   struct ovs_list *dead_cookies)
 OVS_REQUIRES(ofproto_mutex);
 static void delete_flows__(struct rule_collection *,
enum ofp_flow_removed_reason,
@@ -270,7 +272,7 @@ static enum ofperr ofproto_flow_mod_start(struct ofproto *,
 static void ofproto_flow_mod_revert(struct ofproto *,
 struct ofproto_flow_mod *)
 OVS_REQUIRES(ofproto_mutex);
-static void ofproto_flow_mod_finish(struct ofproto *,
+static enum ofperr ofproto_flow_mod_finish(struct ofproto *,
 struct ofproto_flow_mod *,
 const struct openflow_mod_requester *)
 OVS_REQUIRES(ofproto_mutex);
@@ -4855,7 +4857,7 @@ add_flow_revert(struct ofproto *ofproto, struct 
ofproto_flow_mod *ofm)
 }
 
 /* To be called after version bump. */
-static void
+static enum ofperr
 add_flow_finish(struct ofproto *ofproto, struct ofproto_flow_mod *ofm,
 const struct openflow_mod_requester *req)
 

Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2018-07-12 Thread Aaron Conole
Aravind Prasad S  writes:

> Currently, rule_insert() API doesnot have return value. There are some 
> possible

/doesnot/doesn't/  or /doesnot/does not/

Or maybe even just remove that sentence.

:)

> scenarios where rule insertions can fail at run-time even though the static 
> checks during rule_construct() had passed previously.
> Some possible scenarios for failure of rule insertions:
> **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
> Normal switch functioning coexist) where the CAM space could get suddenly
> filled up by Normal switch functioning and Openflow gets devoid of
> available space.
> **) Some deployments could have separate independent layers for HW rule
> insertions and application layer to interact with OVS. HW layer
> could face any dynamic issue during rule handling which application could
> not have predicted/captured in rule-construction phase.
> Rule-insert errors for bundles are also handled in this patch.
>
> Signed-off-by: Aravind Prasad S 
> ---
>  ofproto/ofproto-dpif.c |   4 +-
>  ofproto/ofproto-provider.h |   6 +--
>  ofproto/ofproto.c  | 100 
> +++--
>  3 files changed, 84 insertions(+), 26 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index ad1e8af..d1678ed 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -4443,7 +4443,7 @@ rule_construct(struct rule *rule_)
>  return 0;
>  }
>  
> -static void
> +static enum ofperr
>  rule_insert(struct rule *rule_, struct rule *old_rule_, bool forward_counts)
>  OVS_REQUIRES(ofproto_mutex)
>  {
> @@ -4473,6 +4473,8 @@ rule_insert(struct rule *rule_, struct rule *old_rule_, 
> bool forward_counts)
>  ovs_mutex_unlock(>stats_mutex);
>  ovs_mutex_unlock(_rule->stats_mutex);
>  }
> +
> +return 0;
>  }
>  
>  static void
> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> index 2b77b89..3f3d110 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -1297,8 +1297,8 @@ struct ofproto_class {
>  struct rule *(*rule_alloc)(void);
>  enum ofperr (*rule_construct)(struct rule *rule)
>  /* OVS_REQUIRES(ofproto_mutex) */;
> -void (*rule_insert)(struct rule *rule, struct rule *old_rule,
> -bool forward_counts)
> +enum ofperr (*rule_insert)(struct rule *rule, struct rule *old_rule,
> +   bool forward_counts)
>  /* OVS_REQUIRES(ofproto_mutex) */;
>  void (*rule_delete)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex) */;
>  void (*rule_destruct)(struct rule *rule);
> @@ -1952,7 +1952,7 @@ enum ofperr ofproto_flow_mod_learn_start(struct 
> ofproto_flow_mod *ofm)
>  OVS_REQUIRES(ofproto_mutex);
>  void ofproto_flow_mod_learn_revert(struct ofproto_flow_mod *ofm)
>  OVS_REQUIRES(ofproto_mutex);
> -void ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
> +enum ofperr ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
>struct ofproto *orig_ofproto)
>  OVS_REQUIRES(ofproto_mutex);
>  void ofproto_add_flow(struct ofproto *, const struct match *, int priority,
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index f946e27..03597dc 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -245,10 +245,12 @@ static void replace_rule_revert(struct ofproto *, 
> struct rule *old_rule,
>  struct rule *new_rule)
>  OVS_REQUIRES(ofproto_mutex);
>  
> -static void replace_rule_finish(struct ofproto *, struct ofproto_flow_mod *,
> -const struct openflow_mod_requester *,
> -struct rule *old_rule, struct rule *new_rule,
> -struct ovs_list *dead_cookies)
> +static enum ofperr replace_rule_finish(struct ofproto *,
> +   struct ofproto_flow_mod *,
> +   const struct openflow_mod_requester *,
> +   struct rule *old_rule,
> +   struct rule *new_rule,
> +   struct ovs_list *dead_cookies)
>  OVS_REQUIRES(ofproto_mutex);
>  static void delete_flows__(struct rule_collection *,
> enum ofp_flow_removed_reason,
> @@ -270,7 +272,7 @@ static enum ofperr ofproto_flow_mod_start(struct ofproto 
> *,
>  static void ofproto_flow_mod_revert(struct ofproto *,
>  struct ofproto_flow_mod *)
>  OVS_REQUIRES(ofproto_mutex);
> -static void ofproto_flow_mod_finish(struct ofproto *,
> +static enum ofperr ofproto_flow_mod_finish(struct ofproto *,
>  struct ofproto_flow_mod *,
>  const struct openflow_mod_requester *)
>  OVS_REQUIRES(ofproto_mutex);
> @@ 

Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2018-07-12 Thread Aravind Prasad
 > Currently, rule_insert() API doesnot have return value. There are some
possible
> scenarios where rule insertions can fail at run-time even though the
static
> checks during rule_construct() had passed previously.
> Some possible scenarios for failure of rule insertions:
> **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
> Normal switch functioning coexist) where the CAM space could get suddenly
> filled up by Normal switch functioning and Openflow gets devoid of
> available space.
> **) Some deployments could have separate independent layers for HW rule
> insertions and application layer to interact with OVS. HW layer
> could face any dynamic issue during rule handling which application could
> not have predicted/captured in rule-construction phase.
> Rule-insert errors for bundles are also handled in this patch.

Hi Ben/All,
Sorry for re-sending the patch. Handled rule-insertion failures in
bundle-scenario too with this patch. Thought it could be better
if the entire set of changes are provided in a single patch for
review.
Kindly review and let me know your views.

Thanks,
Aravind Prasad S

On Thu, Jul 12, 2018 at 7:57 PM, Aravind Prasad S 
wrote:

> Currently, rule_insert() API doesnot have return value. There are some
> possible
> scenarios where rule insertions can fail at run-time even though the
> static
> checks during rule_construct() had passed previously.
> Some possible scenarios for failure of rule insertions:
> **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
> Normal switch functioning coexist) where the CAM space could get suddenly
> filled up by Normal switch functioning and Openflow gets devoid of
> available space.
> **) Some deployments could have separate independent layers for HW rule
> insertions and application layer to interact with OVS. HW layer
> could face any dynamic issue during rule handling which application could
> not have predicted/captured in rule-construction phase.
> Rule-insert errors for bundles are also handled in this patch.
>
> Signed-off-by: Aravind Prasad S 
> ---
>  ofproto/ofproto-dpif.c |   4 +-
>  ofproto/ofproto-provider.h |   6 +--
>  ofproto/ofproto.c  | 100 ++
> +--
>  3 files changed, 84 insertions(+), 26 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index ad1e8af..d1678ed 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -4443,7 +4443,7 @@ rule_construct(struct rule *rule_)
>  return 0;
>  }
>
> -static void
> +static enum ofperr
>  rule_insert(struct rule *rule_, struct rule *old_rule_, bool
> forward_counts)
>  OVS_REQUIRES(ofproto_mutex)
>  {
> @@ -4473,6 +4473,8 @@ rule_insert(struct rule *rule_, struct rule
> *old_rule_, bool forward_counts)
>  ovs_mutex_unlock(>stats_mutex);
>  ovs_mutex_unlock(_rule->stats_mutex);
>  }
> +
> +return 0;
>  }
>
>  static void
> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> index 2b77b89..3f3d110 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -1297,8 +1297,8 @@ struct ofproto_class {
>  struct rule *(*rule_alloc)(void);
>  enum ofperr (*rule_construct)(struct rule *rule)
>  /* OVS_REQUIRES(ofproto_mutex) */;
> -void (*rule_insert)(struct rule *rule, struct rule *old_rule,
> -bool forward_counts)
> +enum ofperr (*rule_insert)(struct rule *rule, struct rule *old_rule,
> +   bool forward_counts)
>  /* OVS_REQUIRES(ofproto_mutex) */;
>  void (*rule_delete)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex)
> */;
>  void (*rule_destruct)(struct rule *rule);
> @@ -1952,7 +1952,7 @@ enum ofperr ofproto_flow_mod_learn_start(struct
> ofproto_flow_mod *ofm)
>  OVS_REQUIRES(ofproto_mutex);
>  void ofproto_flow_mod_learn_revert(struct ofproto_flow_mod *ofm)
>  OVS_REQUIRES(ofproto_mutex);
> -void ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
> +enum ofperr ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
>struct ofproto *orig_ofproto)
>  OVS_REQUIRES(ofproto_mutex);
>  void ofproto_add_flow(struct ofproto *, const struct match *, int
> priority,
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index f946e27..03597dc 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -245,10 +245,12 @@ static void replace_rule_revert(struct ofproto *,
> struct rule *old_rule,
>  struct rule *new_rule)
>  OVS_REQUIRES(ofproto_mutex);
>
> -static void replace_rule_finish(struct ofproto *, struct ofproto_flow_mod
> *,
> -const struct openflow_mod_requester *,
> -struct rule *old_rule, struct rule
> *new_rule,
> -struct ovs_list *dead_cookies)
> +static enum ofperr 

[ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2018-07-12 Thread Aravind Prasad S
Currently, rule_insert() API doesnot have return value. There are some possible
scenarios where rule insertions can fail at run-time even though the static 
checks during rule_construct() had passed previously.
Some possible scenarios for failure of rule insertions:
**) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
Normal switch functioning coexist) where the CAM space could get suddenly
filled up by Normal switch functioning and Openflow gets devoid of
available space.
**) Some deployments could have separate independent layers for HW rule
insertions and application layer to interact with OVS. HW layer
could face any dynamic issue during rule handling which application could
not have predicted/captured in rule-construction phase.
Rule-insert errors for bundles are also handled in this patch.

Signed-off-by: Aravind Prasad S 
---
 ofproto/ofproto-dpif.c |   4 +-
 ofproto/ofproto-provider.h |   6 +--
 ofproto/ofproto.c  | 100 +++--
 3 files changed, 84 insertions(+), 26 deletions(-)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index ad1e8af..d1678ed 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4443,7 +4443,7 @@ rule_construct(struct rule *rule_)
 return 0;
 }
 
-static void
+static enum ofperr
 rule_insert(struct rule *rule_, struct rule *old_rule_, bool forward_counts)
 OVS_REQUIRES(ofproto_mutex)
 {
@@ -4473,6 +4473,8 @@ rule_insert(struct rule *rule_, struct rule *old_rule_, 
bool forward_counts)
 ovs_mutex_unlock(>stats_mutex);
 ovs_mutex_unlock(_rule->stats_mutex);
 }
+
+return 0;
 }
 
 static void
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 2b77b89..3f3d110 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -1297,8 +1297,8 @@ struct ofproto_class {
 struct rule *(*rule_alloc)(void);
 enum ofperr (*rule_construct)(struct rule *rule)
 /* OVS_REQUIRES(ofproto_mutex) */;
-void (*rule_insert)(struct rule *rule, struct rule *old_rule,
-bool forward_counts)
+enum ofperr (*rule_insert)(struct rule *rule, struct rule *old_rule,
+   bool forward_counts)
 /* OVS_REQUIRES(ofproto_mutex) */;
 void (*rule_delete)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex) */;
 void (*rule_destruct)(struct rule *rule);
@@ -1952,7 +1952,7 @@ enum ofperr ofproto_flow_mod_learn_start(struct 
ofproto_flow_mod *ofm)
 OVS_REQUIRES(ofproto_mutex);
 void ofproto_flow_mod_learn_revert(struct ofproto_flow_mod *ofm)
 OVS_REQUIRES(ofproto_mutex);
-void ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
+enum ofperr ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
   struct ofproto *orig_ofproto)
 OVS_REQUIRES(ofproto_mutex);
 void ofproto_add_flow(struct ofproto *, const struct match *, int priority,
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index f946e27..03597dc 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -245,10 +245,12 @@ static void replace_rule_revert(struct ofproto *, struct 
rule *old_rule,
 struct rule *new_rule)
 OVS_REQUIRES(ofproto_mutex);
 
-static void replace_rule_finish(struct ofproto *, struct ofproto_flow_mod *,
-const struct openflow_mod_requester *,
-struct rule *old_rule, struct rule *new_rule,
-struct ovs_list *dead_cookies)
+static enum ofperr replace_rule_finish(struct ofproto *,
+   struct ofproto_flow_mod *,
+   const struct openflow_mod_requester *,
+   struct rule *old_rule,
+   struct rule *new_rule,
+   struct ovs_list *dead_cookies)
 OVS_REQUIRES(ofproto_mutex);
 static void delete_flows__(struct rule_collection *,
enum ofp_flow_removed_reason,
@@ -270,7 +272,7 @@ static enum ofperr ofproto_flow_mod_start(struct ofproto *,
 static void ofproto_flow_mod_revert(struct ofproto *,
 struct ofproto_flow_mod *)
 OVS_REQUIRES(ofproto_mutex);
-static void ofproto_flow_mod_finish(struct ofproto *,
+static enum ofperr ofproto_flow_mod_finish(struct ofproto *,
 struct ofproto_flow_mod *,
 const struct openflow_mod_requester *)
 OVS_REQUIRES(ofproto_mutex);
@@ -4855,7 +4857,7 @@ add_flow_revert(struct ofproto *ofproto, struct 
ofproto_flow_mod *ofm)
 }
 
 /* To be called after version bump. */
-static void
+static enum ofperr
 add_flow_finish(struct ofproto *ofproto, struct ofproto_flow_mod *ofm,
 const struct openflow_mod_requester *req)
 

Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions"

2018-07-11 Thread Aravind Prasad
 >Hi Ben/All,
>If possible, Kindly hold reviewing this patch for now. Expecting an
approval from my Org. Sorry for the inconvenience >caused and thanks for
the support.
>Will get back and intimate for the review as soon as possible after the
approval (expecting it to take not more than a week).  >And thanks again
for understanding.

>>OK.

Hi Ben/All,
The patch changes are available for review. Thanks a lot for the continued
support.
Kindly review the patch and let me know your views.
Hi Ben,
In response to your query, as I had previously suggested, we could have
Hardware
implementations with ofproto and netdev providers. Only static checks could
be
possible in Rule-construct phase. The Rule-insert API call could
potentially fail
due to any of dynamic HW failures, runtime errors and so on. And incase of
Hybrid mode implementations(coexistence of normal and Openflow Switch
functioning), the possibility of rule-insertion failures could be higher.
Kindly let me know your views.

Thanks,
Aravind Prasad S

On Tue, Jul 10, 2018 at 10:02 PM Ben Pfaff  wrote:

> OK.
>
> On Tue, Jul 10, 2018 at 02:58:47PM +0530, Aravind Prasad wrote:
> > Hi Ben/All,
> >
> > If possible, Kindly hold reviewing this patch for now. Expecting an
> > approval from my Org. Sorry for the inconvenience caused and thanks for
> the
> > support.
> >
> > Will get back and intimate for the review as soon as possible after the
> > approval (expecting it to take not more than a week).  And thanks again
> for
> > understanding.
> >
> > Thanks,
> > Aravind Prasad S
> >
> > On Tue, Jul 10, 2018 at 7:06 AM Aravind Prasad 
> wrote:
> >
> > >
> > > Currently, rule_insert() API doesnot have return value. There are some
> > > possible
> > > > scenarios where rule insertions can fail at run-time even though the
> > > static
> > > > checks during rule_construct() had passed previously.
> > > > Some possible scenarios for failure of rule insertions:
> > > > **) Rule insertions can fail dynamically in Hybrid mode (both
> Openflow
> > > and
> > > > Normal switch functioning coexist) where the CAM space could get
> suddenly
> > > > filled up by Normal switch functioning and Openflow gets devoid of
> > > > available space.
> > > > **) Some deployments could have separate independent layers for HW
> rule
> > > > insertions and application layer to interact with OVS. HW layer
> > > > could face any dynamic issue during rule handling which application
> could
> > > > not have predicted/captured in rule-construction phase.
> > > > Rule-insert errors for bundles are not handled in this pull-request.
> > > > Will be handled in upcoming pull request.
> > >
> > > >> I don't think that ofproto-dpif can ever see such a failure.  Are
> you
> > > >> planning to submit an ofproto provider that exercises this behavior?
> > >
> > > Hi Ben,
> > >
> > > These type of errors are possible in actual Hardware implementations.
> > > It is possible that ofproto and netdev providers could be implemented
> > > for a actual HW.
> > > Usually, in such cases, in the rule construct phase, all the static
> > > checks like verifying the qualifiers and actions could be done and the
> > > other related verifications.
> > > But during the rule insert phase, it is possible that the rule
> insertion
> > > may get failed in HW (runtime errors, HW errors and so on).
> > > Hence, we need a way to rollback for rule-insert phase also.
> > > Kindly let me know your views.
> > >
> > > Thanks,
> > > Aravind Prasad S
> > >
> > >
> > > On Tue, Jul 10, 2018 at 3:45 AM Ben Pfaff  wrote:
> > >
> > >> On Mon, Jul 09, 2018 at 01:02:08PM +0530, Aravind Prasad S wrote:
> > >> > Currently, rule_insert() API doesnot have return value. There are
> some
> > >> possible
> > >> > scenarios where rule insertions can fail at run-time even though the
> > >> static
> > >> > checks during rule_construct() had passed previously.
> > >> > Some possible scenarios for failure of rule insertions:
> > >> > **) Rule insertions can fail dynamically in Hybrid mode (both
> Openflow
> > >> and
> > >> > Normal switch functioning coexist) where the CAM space could get
> > >> suddenly
> > >> > filled up by Normal switch functioning and Openflow gets devoid of
> > >> > available space.
> > >> > **) Some deployments could have separate independent layers for HW
> rule
> > >> > insertions and application layer to interact with OVS. HW layer
> > >> > could face any dynamic issue during rule handling which application
> > >> could
> > >> > not have predicted/captured in rule-construction phase.
> > >> > Rule-insert errors for bundles are not handled in this pull-request.
> > >> > Will be handled in upcoming pull request.
> > >> >
> > >> > Signed-off-by: Aravind Prasad S 
> > >>
> > >> I don't think that ofproto-dpif can ever see such a failure.  Are you
> > >> planning to submit an ofproto provider that exercises this behavior?
> > >>
> > >> Thanks,
> > >>
> > >> Ben.
> > >>
> > >
>
___
dev mailing 

Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions"

2018-07-10 Thread Ben Pfaff
OK.

On Tue, Jul 10, 2018 at 02:58:47PM +0530, Aravind Prasad wrote:
> Hi Ben/All,
> 
> If possible, Kindly hold reviewing this patch for now. Expecting an
> approval from my Org. Sorry for the inconvenience caused and thanks for the
> support.
> 
> Will get back and intimate for the review as soon as possible after the
> approval (expecting it to take not more than a week).  And thanks again for
> understanding.
> 
> Thanks,
> Aravind Prasad S
> 
> On Tue, Jul 10, 2018 at 7:06 AM Aravind Prasad  wrote:
> 
> >
> > Currently, rule_insert() API doesnot have return value. There are some
> > possible
> > > scenarios where rule insertions can fail at run-time even though the
> > static
> > > checks during rule_construct() had passed previously.
> > > Some possible scenarios for failure of rule insertions:
> > > **) Rule insertions can fail dynamically in Hybrid mode (both Openflow
> > and
> > > Normal switch functioning coexist) where the CAM space could get suddenly
> > > filled up by Normal switch functioning and Openflow gets devoid of
> > > available space.
> > > **) Some deployments could have separate independent layers for HW rule
> > > insertions and application layer to interact with OVS. HW layer
> > > could face any dynamic issue during rule handling which application could
> > > not have predicted/captured in rule-construction phase.
> > > Rule-insert errors for bundles are not handled in this pull-request.
> > > Will be handled in upcoming pull request.
> >
> > >> I don't think that ofproto-dpif can ever see such a failure.  Are you
> > >> planning to submit an ofproto provider that exercises this behavior?
> >
> > Hi Ben,
> >
> > These type of errors are possible in actual Hardware implementations.
> > It is possible that ofproto and netdev providers could be implemented
> > for a actual HW.
> > Usually, in such cases, in the rule construct phase, all the static
> > checks like verifying the qualifiers and actions could be done and the
> > other related verifications.
> > But during the rule insert phase, it is possible that the rule insertion
> > may get failed in HW (runtime errors, HW errors and so on).
> > Hence, we need a way to rollback for rule-insert phase also.
> > Kindly let me know your views.
> >
> > Thanks,
> > Aravind Prasad S
> >
> >
> > On Tue, Jul 10, 2018 at 3:45 AM Ben Pfaff  wrote:
> >
> >> On Mon, Jul 09, 2018 at 01:02:08PM +0530, Aravind Prasad S wrote:
> >> > Currently, rule_insert() API doesnot have return value. There are some
> >> possible
> >> > scenarios where rule insertions can fail at run-time even though the
> >> static
> >> > checks during rule_construct() had passed previously.
> >> > Some possible scenarios for failure of rule insertions:
> >> > **) Rule insertions can fail dynamically in Hybrid mode (both Openflow
> >> and
> >> > Normal switch functioning coexist) where the CAM space could get
> >> suddenly
> >> > filled up by Normal switch functioning and Openflow gets devoid of
> >> > available space.
> >> > **) Some deployments could have separate independent layers for HW rule
> >> > insertions and application layer to interact with OVS. HW layer
> >> > could face any dynamic issue during rule handling which application
> >> could
> >> > not have predicted/captured in rule-construction phase.
> >> > Rule-insert errors for bundles are not handled in this pull-request.
> >> > Will be handled in upcoming pull request.
> >> >
> >> > Signed-off-by: Aravind Prasad S 
> >>
> >> I don't think that ofproto-dpif can ever see such a failure.  Are you
> >> planning to submit an ofproto provider that exercises this behavior?
> >>
> >> Thanks,
> >>
> >> Ben.
> >>
> >
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions"

2018-07-10 Thread Aravind Prasad
Hi Ben/All,

If possible, Kindly hold reviewing this patch for now. Expecting an
approval from my Org. Sorry for the inconvenience caused and thanks for the
support.

Will get back and intimate for the review as soon as possible after the
approval (expecting it to take not more than a week).  And thanks again for
understanding.

Thanks,
Aravind Prasad S

On Tue, Jul 10, 2018 at 7:06 AM Aravind Prasad  wrote:

>
> Currently, rule_insert() API doesnot have return value. There are some
> possible
> > scenarios where rule insertions can fail at run-time even though the
> static
> > checks during rule_construct() had passed previously.
> > Some possible scenarios for failure of rule insertions:
> > **) Rule insertions can fail dynamically in Hybrid mode (both Openflow
> and
> > Normal switch functioning coexist) where the CAM space could get suddenly
> > filled up by Normal switch functioning and Openflow gets devoid of
> > available space.
> > **) Some deployments could have separate independent layers for HW rule
> > insertions and application layer to interact with OVS. HW layer
> > could face any dynamic issue during rule handling which application could
> > not have predicted/captured in rule-construction phase.
> > Rule-insert errors for bundles are not handled in this pull-request.
> > Will be handled in upcoming pull request.
>
> >> I don't think that ofproto-dpif can ever see such a failure.  Are you
> >> planning to submit an ofproto provider that exercises this behavior?
>
> Hi Ben,
>
> These type of errors are possible in actual Hardware implementations.
> It is possible that ofproto and netdev providers could be implemented
> for a actual HW.
> Usually, in such cases, in the rule construct phase, all the static
> checks like verifying the qualifiers and actions could be done and the
> other related verifications.
> But during the rule insert phase, it is possible that the rule insertion
> may get failed in HW (runtime errors, HW errors and so on).
> Hence, we need a way to rollback for rule-insert phase also.
> Kindly let me know your views.
>
> Thanks,
> Aravind Prasad S
>
>
> On Tue, Jul 10, 2018 at 3:45 AM Ben Pfaff  wrote:
>
>> On Mon, Jul 09, 2018 at 01:02:08PM +0530, Aravind Prasad S wrote:
>> > Currently, rule_insert() API doesnot have return value. There are some
>> possible
>> > scenarios where rule insertions can fail at run-time even though the
>> static
>> > checks during rule_construct() had passed previously.
>> > Some possible scenarios for failure of rule insertions:
>> > **) Rule insertions can fail dynamically in Hybrid mode (both Openflow
>> and
>> > Normal switch functioning coexist) where the CAM space could get
>> suddenly
>> > filled up by Normal switch functioning and Openflow gets devoid of
>> > available space.
>> > **) Some deployments could have separate independent layers for HW rule
>> > insertions and application layer to interact with OVS. HW layer
>> > could face any dynamic issue during rule handling which application
>> could
>> > not have predicted/captured in rule-construction phase.
>> > Rule-insert errors for bundles are not handled in this pull-request.
>> > Will be handled in upcoming pull request.
>> >
>> > Signed-off-by: Aravind Prasad S 
>>
>> I don't think that ofproto-dpif can ever see such a failure.  Are you
>> planning to submit an ofproto provider that exercises this behavior?
>>
>> Thanks,
>>
>> Ben.
>>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions"

2018-07-09 Thread Aravind Prasad
Currently, rule_insert() API doesnot have return value. There are some
possible
> scenarios where rule insertions can fail at run-time even though the
static
> checks during rule_construct() had passed previously.
> Some possible scenarios for failure of rule insertions:
> **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
> Normal switch functioning coexist) where the CAM space could get suddenly
> filled up by Normal switch functioning and Openflow gets devoid of
> available space.
> **) Some deployments could have separate independent layers for HW rule
> insertions and application layer to interact with OVS. HW layer
> could face any dynamic issue during rule handling which application could
> not have predicted/captured in rule-construction phase.
> Rule-insert errors for bundles are not handled in this pull-request.
> Will be handled in upcoming pull request.

>> I don't think that ofproto-dpif can ever see such a failure.  Are you
>> planning to submit an ofproto provider that exercises this behavior?

Hi Ben,

These type of errors are possible in actual Hardware implementations.
It is possible that ofproto and netdev providers could be implemented
for a actual HW.
Usually, in such cases, in the rule construct phase, all the static
checks like verifying the qualifiers and actions could be done and the
other related verifications.
But during the rule insert phase, it is possible that the rule insertion
may get failed in HW (runtime errors, HW errors and so on).
Hence, we need a way to rollback for rule-insert phase also.
Kindly let me know your views.

Thanks,
Aravind Prasad S


On Tue, Jul 10, 2018 at 3:45 AM Ben Pfaff  wrote:

> On Mon, Jul 09, 2018 at 01:02:08PM +0530, Aravind Prasad S wrote:
> > Currently, rule_insert() API doesnot have return value. There are some
> possible
> > scenarios where rule insertions can fail at run-time even though the
> static
> > checks during rule_construct() had passed previously.
> > Some possible scenarios for failure of rule insertions:
> > **) Rule insertions can fail dynamically in Hybrid mode (both Openflow
> and
> > Normal switch functioning coexist) where the CAM space could get suddenly
> > filled up by Normal switch functioning and Openflow gets devoid of
> > available space.
> > **) Some deployments could have separate independent layers for HW rule
> > insertions and application layer to interact with OVS. HW layer
> > could face any dynamic issue during rule handling which application could
> > not have predicted/captured in rule-construction phase.
> > Rule-insert errors for bundles are not handled in this pull-request.
> > Will be handled in upcoming pull request.
> >
> > Signed-off-by: Aravind Prasad S 
>
> I don't think that ofproto-dpif can ever see such a failure.  Are you
> planning to submit an ofproto provider that exercises this behavior?
>
> Thanks,
>
> Ben.
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions"

2018-07-09 Thread Ben Pfaff
On Mon, Jul 09, 2018 at 01:02:08PM +0530, Aravind Prasad S wrote:
> Currently, rule_insert() API doesnot have return value. There are some 
> possible
> scenarios where rule insertions can fail at run-time even though the static 
> checks during rule_construct() had passed previously.
> Some possible scenarios for failure of rule insertions:
> **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
> Normal switch functioning coexist) where the CAM space could get suddenly
> filled up by Normal switch functioning and Openflow gets devoid of
> available space.
> **) Some deployments could have separate independent layers for HW rule
> insertions and application layer to interact with OVS. HW layer
> could face any dynamic issue during rule handling which application could
> not have predicted/captured in rule-construction phase.
> Rule-insert errors for bundles are not handled in this pull-request.
> Will be handled in upcoming pull request.
> 
> Signed-off-by: Aravind Prasad S 

I don't think that ofproto-dpif can ever see such a failure.  Are you
planning to submit an ofproto provider that exercises this behavior?

Thanks,

Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions"

2018-07-09 Thread Aravind Prasad S
Currently, rule_insert() API doesnot have return value. There are some possible
scenarios where rule insertions can fail at run-time even though the static 
checks during rule_construct() had passed previously.
Some possible scenarios for failure of rule insertions:
**) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
Normal switch functioning coexist) where the CAM space could get suddenly
filled up by Normal switch functioning and Openflow gets devoid of
available space.
**) Some deployments could have separate independent layers for HW rule
insertions and application layer to interact with OVS. HW layer
could face any dynamic issue during rule handling which application could
not have predicted/captured in rule-construction phase.
Rule-insert errors for bundles are not handled in this pull-request.
Will be handled in upcoming pull request.

Signed-off-by: Aravind Prasad S 
---
 ofproto/ofproto-dpif.c |  4 ++-
 ofproto/ofproto-provider.h |  6 ++--
 ofproto/ofproto.c  | 76 +-
 3 files changed, 61 insertions(+), 25 deletions(-)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index ad1e8af..d1678ed 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4443,7 +4443,7 @@ rule_construct(struct rule *rule_)
 return 0;
 }
 
-static void
+static enum ofperr
 rule_insert(struct rule *rule_, struct rule *old_rule_, bool forward_counts)
 OVS_REQUIRES(ofproto_mutex)
 {
@@ -4473,6 +4473,8 @@ rule_insert(struct rule *rule_, struct rule *old_rule_, 
bool forward_counts)
 ovs_mutex_unlock(>stats_mutex);
 ovs_mutex_unlock(_rule->stats_mutex);
 }
+
+return 0;
 }
 
 static void
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 2b77b89..3f3d110 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -1297,8 +1297,8 @@ struct ofproto_class {
 struct rule *(*rule_alloc)(void);
 enum ofperr (*rule_construct)(struct rule *rule)
 /* OVS_REQUIRES(ofproto_mutex) */;
-void (*rule_insert)(struct rule *rule, struct rule *old_rule,
-bool forward_counts)
+enum ofperr (*rule_insert)(struct rule *rule, struct rule *old_rule,
+   bool forward_counts)
 /* OVS_REQUIRES(ofproto_mutex) */;
 void (*rule_delete)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex) */;
 void (*rule_destruct)(struct rule *rule);
@@ -1952,7 +1952,7 @@ enum ofperr ofproto_flow_mod_learn_start(struct 
ofproto_flow_mod *ofm)
 OVS_REQUIRES(ofproto_mutex);
 void ofproto_flow_mod_learn_revert(struct ofproto_flow_mod *ofm)
 OVS_REQUIRES(ofproto_mutex);
-void ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
+enum ofperr ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
   struct ofproto *orig_ofproto)
 OVS_REQUIRES(ofproto_mutex);
 void ofproto_add_flow(struct ofproto *, const struct match *, int priority,
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index f946e27..81b2466 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -245,10 +245,12 @@ static void replace_rule_revert(struct ofproto *, struct 
rule *old_rule,
 struct rule *new_rule)
 OVS_REQUIRES(ofproto_mutex);
 
-static void replace_rule_finish(struct ofproto *, struct ofproto_flow_mod *,
-const struct openflow_mod_requester *,
-struct rule *old_rule, struct rule *new_rule,
-struct ovs_list *dead_cookies)
+static enum ofperr replace_rule_finish(struct ofproto *,
+   struct ofproto_flow_mod *,
+   const struct openflow_mod_requester *,
+   struct rule *old_rule,
+   struct rule *new_rule,
+   struct ovs_list *dead_cookies)
 OVS_REQUIRES(ofproto_mutex);
 static void delete_flows__(struct rule_collection *,
enum ofp_flow_removed_reason,
@@ -270,7 +272,7 @@ static enum ofperr ofproto_flow_mod_start(struct ofproto *,
 static void ofproto_flow_mod_revert(struct ofproto *,
 struct ofproto_flow_mod *)
 OVS_REQUIRES(ofproto_mutex);
-static void ofproto_flow_mod_finish(struct ofproto *,
+static enum ofperr ofproto_flow_mod_finish(struct ofproto *,
 struct ofproto_flow_mod *,
 const struct openflow_mod_requester *)
 OVS_REQUIRES(ofproto_mutex);
@@ -4855,7 +4857,7 @@ add_flow_revert(struct ofproto *ofproto, struct 
ofproto_flow_mod *ofm)
 }
 
 /* To be called after version bump. */
-static void
+static enum ofperr
 add_flow_finish(struct ofproto *ofproto, struct ofproto_flow_mod *ofm,
 const struct 

Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2018-07-08 Thread Aravind Prasad
Hi Aaron,
> Currently, rule_insert() API doesnot have return value. There are some
> possible
> scenarios where rule insertions can fail at run-time even though the
static
> checks during rule_construct() had passed previously.
>
> Some possible scenarios for failure of rule insertions:
> **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
> Normal switch functioning coexist) where the CAM space could get suddenly
> filled up by Normal switch functioning and Openflow gets devoid of
> available space.
> **) Some deployments could have separate independent layers for HW rule
> insertions and application layer to interact with OVS. HW layer
> could face any dynamic issue during rule handling which application could
> not have predicted/captured in rule-construction phase.
>
> Rule-insert errors for bundles are not handled in this pull-request.
> Will be handled in upcoming pull request.
>
> Signed-off-by: Aravind Prasad S 
> ---

>> Thanks for working on OVS.

>> As noted, the patch has some submission errors.  Please try submitting
>> again with 'git send-email' to eliminate any potential mail client
>> munging.

Thanks a lot for the info and support. Submitted the patch with git -email.
OVS group rocks :-). Very active in assistance and support too.

Thanks,
S. Aravind Prasad

On Mon, Jul 9, 2018 at 5:09 AM Aaron Conole  wrote:

> Hi Arvind,
>
> Aravind Prasad  writes:
>
> > Currently, rule_insert() API doesnot have return value. There are some
> > possible
> > scenarios where rule insertions can fail at run-time even though the
> static
> > checks during rule_construct() had passed previously.
> >
> > Some possible scenarios for failure of rule insertions:
> > **) Rule insertions can fail dynamically in Hybrid mode (both Openflow
> and
> > Normal switch functioning coexist) where the CAM space could get suddenly
> > filled up by Normal switch functioning and Openflow gets devoid of
> > available space.
> > **) Some deployments could have separate independent layers for HW rule
> > insertions and application layer to interact with OVS. HW layer
> > could face any dynamic issue during rule handling which application could
> > not have predicted/captured in rule-construction phase.
> >
> > Rule-insert errors for bundles are not handled in this pull-request.
> > Will be handled in upcoming pull request.
> >
> > Signed-off-by: Aravind Prasad S 
> > ---
>
> Thanks for working on OVS.
>
> As noted, the patch has some submission errors.  Please try submitting
> again with 'git send-email' to eliminate any potential mail client
> munging.
>
> -Aaron
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2018-07-08 Thread Aravind Prasad S
Currently, rule_insert() API doesnot have return value. There are some possible
scenarios where rule insertions can fail at run-time even though the static 
checks during rule_construct() had passed previously.
Some possible scenarios for failure of rule insertions:
**) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
Normal switch functioning coexist) where the CAM space could get suddenly
filled up by Normal switch functioning and Openflow gets devoid of
available space.
**) Some deployments could have separate independent layers for HW rule
insertions and application layer to interact with OVS. HW layer
could face any dynamic issue during rule handling which application could
not have predicted/captured in rule-construction phase.
Rule-insert errors for bundles are not handled in this pull-request.
Will be handled in upcoming pull request.

Signed-off-by: Aravind Prasad S 
---
 ofproto/ofproto-dpif.c |  4 ++-
 ofproto/ofproto-provider.h |  6 ++--
 ofproto/ofproto.c  | 76 +-
 3 files changed, 61 insertions(+), 25 deletions(-)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index ad1e8af..d1678ed 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4443,7 +4443,7 @@ rule_construct(struct rule *rule_)
 return 0;
 }
 
-static void
+static enum ofperr
 rule_insert(struct rule *rule_, struct rule *old_rule_, bool forward_counts)
 OVS_REQUIRES(ofproto_mutex)
 {
@@ -4473,6 +4473,8 @@ rule_insert(struct rule *rule_, struct rule *old_rule_, 
bool forward_counts)
 ovs_mutex_unlock(>stats_mutex);
 ovs_mutex_unlock(_rule->stats_mutex);
 }
+
+return 0;
 }
 
 static void
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 2b77b89..3f3d110 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -1297,8 +1297,8 @@ struct ofproto_class {
 struct rule *(*rule_alloc)(void);
 enum ofperr (*rule_construct)(struct rule *rule)
 /* OVS_REQUIRES(ofproto_mutex) */;
-void (*rule_insert)(struct rule *rule, struct rule *old_rule,
-bool forward_counts)
+enum ofperr (*rule_insert)(struct rule *rule, struct rule *old_rule,
+   bool forward_counts)
 /* OVS_REQUIRES(ofproto_mutex) */;
 void (*rule_delete)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex) */;
 void (*rule_destruct)(struct rule *rule);
@@ -1952,7 +1952,7 @@ enum ofperr ofproto_flow_mod_learn_start(struct 
ofproto_flow_mod *ofm)
 OVS_REQUIRES(ofproto_mutex);
 void ofproto_flow_mod_learn_revert(struct ofproto_flow_mod *ofm)
 OVS_REQUIRES(ofproto_mutex);
-void ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
+enum ofperr ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
   struct ofproto *orig_ofproto)
 OVS_REQUIRES(ofproto_mutex);
 void ofproto_add_flow(struct ofproto *, const struct match *, int priority,
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index f946e27..81b2466 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -245,10 +245,12 @@ static void replace_rule_revert(struct ofproto *, struct 
rule *old_rule,
 struct rule *new_rule)
 OVS_REQUIRES(ofproto_mutex);
 
-static void replace_rule_finish(struct ofproto *, struct ofproto_flow_mod *,
-const struct openflow_mod_requester *,
-struct rule *old_rule, struct rule *new_rule,
-struct ovs_list *dead_cookies)
+static enum ofperr replace_rule_finish(struct ofproto *,
+   struct ofproto_flow_mod *,
+   const struct openflow_mod_requester *,
+   struct rule *old_rule,
+   struct rule *new_rule,
+   struct ovs_list *dead_cookies)
 OVS_REQUIRES(ofproto_mutex);
 static void delete_flows__(struct rule_collection *,
enum ofp_flow_removed_reason,
@@ -270,7 +272,7 @@ static enum ofperr ofproto_flow_mod_start(struct ofproto *,
 static void ofproto_flow_mod_revert(struct ofproto *,
 struct ofproto_flow_mod *)
 OVS_REQUIRES(ofproto_mutex);
-static void ofproto_flow_mod_finish(struct ofproto *,
+static enum ofperr ofproto_flow_mod_finish(struct ofproto *,
 struct ofproto_flow_mod *,
 const struct openflow_mod_requester *)
 OVS_REQUIRES(ofproto_mutex);
@@ -4855,7 +4857,7 @@ add_flow_revert(struct ofproto *ofproto, struct 
ofproto_flow_mod *ofm)
 }
 
 /* To be called after version bump. */
-static void
+static enum ofperr
 add_flow_finish(struct ofproto *ofproto, struct ofproto_flow_mod *ofm,
 const struct 

Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2018-07-08 Thread Aaron Conole
Hi Arvind,

Aravind Prasad  writes:

> Currently, rule_insert() API doesnot have return value. There are some
> possible
> scenarios where rule insertions can fail at run-time even though the static
> checks during rule_construct() had passed previously.
>
> Some possible scenarios for failure of rule insertions:
> **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
> Normal switch functioning coexist) where the CAM space could get suddenly
> filled up by Normal switch functioning and Openflow gets devoid of
> available space.
> **) Some deployments could have separate independent layers for HW rule
> insertions and application layer to interact with OVS. HW layer
> could face any dynamic issue during rule handling which application could
> not have predicted/captured in rule-construction phase.
>
> Rule-insert errors for bundles are not handled in this pull-request.
> Will be handled in upcoming pull request.
>
> Signed-off-by: Aravind Prasad S 
> ---

Thanks for working on OVS.

As noted, the patch has some submission errors.  Please try submitting
again with 'git send-email' to eliminate any potential mail client
munging.

-Aaron
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions

2018-07-08 Thread Aravind Prasad
Currently, rule_insert() API doesnot have return value. There are some
possible
scenarios where rule insertions can fail at run-time even though the static
checks during rule_construct() had passed previously.

Some possible scenarios for failure of rule insertions:
**) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
Normal switch functioning coexist) where the CAM space could get suddenly
filled up by Normal switch functioning and Openflow gets devoid of
available space.
**) Some deployments could have separate independent layers for HW rule
insertions and application layer to interact with OVS. HW layer
could face any dynamic issue during rule handling which application could
not have predicted/captured in rule-construction phase.

Rule-insert errors for bundles are not handled in this pull-request.
Will be handled in upcoming pull request.

Signed-off-by: Aravind Prasad S 
---

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index ad1e8af..d1678ed 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4443,7 +4443,7 @@ rule_construct(struct rule *rule_)
 return 0;
 }

-static void
+static enum ofperr
 rule_insert(struct rule *rule_, struct rule *old_rule_, bool
forward_counts)
 OVS_REQUIRES(ofproto_mutex)
 {
@@ -4473,6 +4473,8 @@ rule_insert(struct rule *rule_, struct rule
*old_rule_, bool forward_counts)
 ovs_mutex_unlock(>stats_mutex);
 ovs_mutex_unlock(_rule->stats_mutex);
 }
+
+return 0;
 }

 static void
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 2b77b89..3f3d110 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -1297,8 +1297,8 @@ struct ofproto_class {
 struct rule *(*rule_alloc)(void);
 enum ofperr (*rule_construct)(struct rule *rule)
 /* OVS_REQUIRES(ofproto_mutex) */;
-void (*rule_insert)(struct rule *rule, struct rule *old_rule,
-bool forward_counts)
+enum ofperr (*rule_insert)(struct rule *rule, struct rule *old_rule,
+   bool forward_counts)
 /* OVS_REQUIRES(ofproto_mutex) */;
 void (*rule_delete)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex)
*/;
 void (*rule_destruct)(struct rule *rule);
@@ -1952,7 +1952,7 @@ enum ofperr ofproto_flow_mod_learn_start(struct
ofproto_flow_mod *ofm)
 OVS_REQUIRES(ofproto_mutex);
 void ofproto_flow_mod_learn_revert(struct ofproto_flow_mod *ofm)
 OVS_REQUIRES(ofproto_mutex);
-void ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
+enum ofperr ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
   struct ofproto *orig_ofproto)
 OVS_REQUIRES(ofproto_mutex);
 void ofproto_add_flow(struct ofproto *, const struct match *, int priority,
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index f946e27..81b2466 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -245,10 +245,12 @@ static void replace_rule_revert(struct ofproto *,
struct rule *old_rule,
 struct rule *new_rule)
 OVS_REQUIRES(ofproto_mutex);

-static void replace_rule_finish(struct ofproto *, struct ofproto_flow_mod
*,
-const struct openflow_mod_requester *,
-struct rule *old_rule, struct rule
*new_rule,
-struct ovs_list *dead_cookies)
+static enum ofperr replace_rule_finish(struct ofproto *,
+   struct ofproto_flow_mod *,
+   const struct openflow_mod_requester
*,
+   struct rule *old_rule,
+   struct rule *new_rule,
+   struct ovs_list *dead_cookies)
 OVS_REQUIRES(ofproto_mutex);
 static void delete_flows__(struct rule_collection *,
enum ofp_flow_removed_reason,
@@ -270,7 +272,7 @@ static enum ofperr ofproto_flow_mod_start(struct
ofproto *,
 static void ofproto_flow_mod_revert(struct ofproto *,
 struct ofproto_flow_mod *)
 OVS_REQUIRES(ofproto_mutex);
-static void ofproto_flow_mod_finish(struct ofproto *,
+static enum ofperr ofproto_flow_mod_finish(struct ofproto *,
 struct ofproto_flow_mod *,
 const struct openflow_mod_requester *)
 OVS_REQUIRES(ofproto_mutex);
@@ -4855,7 +4857,7 @@ add_flow_revert(struct ofproto *ofproto, struct
ofproto_flow_mod *ofm)
 }

 /* To be called after version bump. */
-static void
+static enum ofperr
 add_flow_finish(struct ofproto *ofproto, struct ofproto_flow_mod *ofm,
 const struct openflow_mod_requester *req)
 OVS_REQUIRES(ofproto_mutex)
@@ -4864,8 +4866,14 @@ add_flow_finish(struct ofproto *ofproto, struct
ofproto_flow_mod *ofm,
 ? rule_collection_rules(>old_rules)[0] : NULL;
 struct 

Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions/deletions

2018-06-17 Thread Aravind Prasad
 > Currently, rule_insert() and rule_delete() ofproto provider APIs do not
>
>
> have return values. There are some possible scenarios where rule
insertions
>
> and deletions can fail at run-time even though the static checks during
>
> rule_construct() had passed previously.
>
>
>
> Some possible scenarios for failure of rule insertions and deletions:
>
>
>
> **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
>
>
> Normal switch functioning coexist) where the CAM space could get suddenly
>
>
> filled up by Normal switch functioning and Openflow gets devoid of
>
>
> available space.
>
>
>
> **) Some deployments could have separate independent layers for HW rule
>
>
> insertions/deletions and application layer to interact with OVS. HW layer
>
>
> could face any dynamic issue during rule handling which application could
>
>
> not have predicted/captured in rule-construction phase.
>
>
>
>
>
> This patch is the first step to introduce error reporting for rule
>
>
> insertions/deletions from Client back to OVS.
>
>
> Signed-off-by: Aravind Prasad S 

>>Thanks for working to improve OVS.

>>Please don't triple-space your commit message.

>>This commit doesn't actually do anything to handle failures.  It only
reports them.  To be acceptable in OVS, it would have to actually handle
them.

>>(Also, for error reporting purposes, it's not acceptable to just print a
number.)

Hi Ben,

Thanks for the review. Since the changes to delete the erroneous flow could
be a lot more, thought , it would be better to push the changes
incrementally.
Anyways, thanks again and will get back with the necessary changes.

Thanks,
S. Aravind Prasad

On Sat, Jun 16, 2018 at 9:19 PM, Ben Pfaff  wrote:

> On Sat, Jun 16, 2018 at 06:14:38PM +0530, Aravind Prasad wrote:
> > Currently, rule_insert() and rule_delete() ofproto provider APIs do not
> >
> >
> > have return values. There are some possible scenarios where rule
> insertions
> >
> > and deletions can fail at run-time even though the static checks during
> >
> > rule_construct() had passed previously.
> >
> >
> >
> > Some possible scenarios for failure of rule insertions and deletions:
> >
> >
> >
> > **) Rule insertions can fail dynamically in Hybrid mode (both Openflow
> and
> >
> >
> > Normal switch functioning coexist) where the CAM space could get suddenly
> >
> >
> > filled up by Normal switch functioning and Openflow gets devoid of
> >
> >
> > available space.
> >
> >
> >
> > **) Some deployments could have separate independent layers for HW rule
> >
> >
> > insertions/deletions and application layer to interact with OVS. HW layer
> >
> >
> > could face any dynamic issue during rule handling which application could
> >
> >
> > not have predicted/captured in rule-construction phase.
> >
> >
> >
> >
> >
> > This patch is the first step to introduce error reporting for rule
> >
> >
> > insertions/deletions from Client back to OVS.
> >
> >
> > Signed-off-by: Aravind Prasad S 
>
> Thanks for working to improve OVS.
>
> Please don't triple-space your commit message.
>
> This commit doesn't actually do anything to handle failures.  It only
> reports them.  To be acceptable in OVS, it would have to actually handle
> them.
>
> (Also, for error reporting purposes, it's not acceptable to just print a
> number.)
>
> Thanks,
>
> Ben.
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions/deletions

2018-06-16 Thread Ben Pfaff
On Sat, Jun 16, 2018 at 06:14:38PM +0530, Aravind Prasad wrote:
> Currently, rule_insert() and rule_delete() ofproto provider APIs do not
> 
> 
> have return values. There are some possible scenarios where rule insertions
> 
> and deletions can fail at run-time even though the static checks during
> 
> rule_construct() had passed previously.
> 
> 
> 
> Some possible scenarios for failure of rule insertions and deletions:
> 
> 
> 
> **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
> 
> 
> Normal switch functioning coexist) where the CAM space could get suddenly
> 
> 
> filled up by Normal switch functioning and Openflow gets devoid of
> 
> 
> available space.
> 
> 
> 
> **) Some deployments could have separate independent layers for HW rule
> 
> 
> insertions/deletions and application layer to interact with OVS. HW layer
> 
> 
> could face any dynamic issue during rule handling which application could
> 
> 
> not have predicted/captured in rule-construction phase.
> 
> 
> 
> 
> 
> This patch is the first step to introduce error reporting for rule
> 
> 
> insertions/deletions from Client back to OVS.
> 
> 
> Signed-off-by: Aravind Prasad S 

Thanks for working to improve OVS.

Please don't triple-space your commit message.

This commit doesn't actually do anything to handle failures.  It only
reports them.  To be acceptable in OVS, it would have to actually handle
them.

(Also, for error reporting purposes, it's not acceptable to just print a
number.)

Thanks,

Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions/deletions

2018-06-16 Thread Aravind Prasad
Currently, rule_insert() and rule_delete() ofproto provider APIs do not


have return values. There are some possible scenarios where rule insertions

and deletions can fail at run-time even though the static checks during

rule_construct() had passed previously.



Some possible scenarios for failure of rule insertions and deletions:



**) Rule insertions can fail dynamically in Hybrid mode (both Openflow and


Normal switch functioning coexist) where the CAM space could get suddenly


filled up by Normal switch functioning and Openflow gets devoid of


available space.



**) Some deployments could have separate independent layers for HW rule


insertions/deletions and application layer to interact with OVS. HW layer


could face any dynamic issue during rule handling which application could


not have predicted/captured in rule-construction phase.





This patch is the first step to introduce error reporting for rule


insertions/deletions from Client back to OVS.


Signed-off-by: Aravind Prasad S 

---

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c

index ca4582c..ca485b3 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -,7 +,7 @@ rule_construct(struct rule *rule_)
 return 0;
 }

-static void
+static enum ofperr
 rule_insert(struct rule *rule_, struct rule *old_rule_, bool forward_counts)
 OVS_REQUIRES(ofproto_mutex)
 {
@@ -4474,6 +4474,8 @@ rule_insert(struct rule *rule_, struct rule
*old_rule_, bool forward_counts)
 ovs_mutex_unlock(>stats_mutex);
 ovs_mutex_unlock(_rule->stats_mutex);
 }
+
+return 0;
 }

 static void
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 2b77b89..8c7ed4e 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -1297,10 +1297,11 @@ struct ofproto_class {
 struct rule *(*rule_alloc)(void);
 enum ofperr (*rule_construct)(struct rule *rule)
 /* OVS_REQUIRES(ofproto_mutex) */;
-void (*rule_insert)(struct rule *rule, struct rule *old_rule,
-bool forward_counts)
+enum ofperr (*rule_insert)(struct rule *rule, struct rule *old_rule,
+bool forward_counts)
+/* OVS_REQUIRES(ofproto_mutex) */;
+enum ofperr (*rule_delete)(struct rule *rule)
 /* OVS_REQUIRES(ofproto_mutex) */;
-void (*rule_delete)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex) */;
 void (*rule_destruct)(struct rule *rule);
 void (*rule_dealloc)(struct rule *rule);

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 829ccd8..f6cea33 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1522,6 +1522,8 @@ void
 ofproto_rule_delete(struct ofproto *ofproto, struct rule *rule)
 OVS_EXCLUDED(ofproto_mutex)
 {
+int error = 0;
+
 /* This skips the ofmonitor and flow-removed notifications because the
  * switch is being deleted and any OpenFlow channels have been or soon will
  * be killed. */
@@ -1535,7 +1537,10 @@ ofproto_rule_delete(struct ofproto *ofproto,
struct rule *rule)
  >cr);
 ofproto_rule_remove__(rule->ofproto, rule);
 if (ofproto->ofproto_class->rule_delete) {
-ofproto->ofproto_class->rule_delete(rule);
+error = ofproto->ofproto_class->rule_delete(rule);
+if (error) {
+VLOG_INFO("Rule deletion failed, error = %u", error);
+}
 }

 /* This may not be the last reference to the rule. */
@@ -2882,11 +2887,15 @@ remove_rule_rcu__(struct rule *rule)
 {
 struct ofproto *ofproto = rule->ofproto;
 struct oftable *table = >tables[rule->table_id];
+int error = 0;

 ovs_assert(!cls_rule_visible_in_version(>cr, OVS_VERSION_MAX));
 classifier_remove_assert(>cls, >cr);
 if (ofproto->ofproto_class->rule_delete) {
-ofproto->ofproto_class->rule_delete(rule);
+error = ofproto->ofproto_class->rule_delete(rule);
+if (error) {
+VLOG_INFO("Rule deletion failed, error = %u", error);
+}
 }
 ofproto_rule_unref(rule);
 }
@@ -5252,6 +5261,7 @@ replace_rule_finish(struct ofproto *ofproto,
struct ofproto_flow_mod *ofm,
 OVS_REQUIRES(ofproto_mutex)
 {
 struct rule *replaced_rule;
+int error = 0;

 replaced_rule = (old_rule && old_rule->removed_reason != OFPRR_EVICTION)
 ? old_rule : NULL;
@@ -5261,8 +5271,12 @@ replace_rule_finish(struct ofproto *ofproto,
struct ofproto_flow_mod *ofm,
  * link the packet and byte counts from the old rule to the new one if
  * 'modify_keep_counts' is 'true'.  The 'replaced_rule' will be deleted
  * right after this call. */
-ofproto->ofproto_class->rule_insert(new_rule, replaced_rule,
-ofm->modify_keep_counts);
+error = ofproto->ofproto_class->rule_insert(new_rule, replaced_rule,
+