Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions
> > 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
> 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
> 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
> 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 rul
Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions
> 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 c
Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions
> 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
Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions
> 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
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 *)
>
Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions
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 of
Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions
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 *)
>
Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions
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 *)
>
Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions
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 *)
>
Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions
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 *)
>
Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions
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 of
Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions
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,
&g
[ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions
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)
OVS_RE
Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions
> 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_rul
[ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions
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)
OVS_RE
Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions"
>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. > > >
Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions"
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"
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
[ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions"
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
Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions
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] Handle rule insert failures
---
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 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 rule *new_rule = rule_collection_rules(>new_rules)[0];
struct ovs_list dead_cookies = OVS_LIST_INITIALIZER(_cookies);
+enum ofperr error = 0;
+
+error = replace_rule_finish(ofproto, ofm, req, old_rule, new_rule,
+_cookies);
+if (error) {
+return error;
+}
-replace_rule_finish(ofproto, ofm, req, old_rule, new_rule, _cookies);
learned_cookies_flush(ofproto, _cookies);
if (old_rule) {
@@ -4878,6 +4886,8 @@ add_flow_finish(struct ofproto *ofproto, struct
ofproto_flow_mod *ofm,
/* Send Vacancy Events for OF1.4+. */
send_table_status(ofproto, new_rule->table_id);
}
+
+return error;
}
[ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions
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
[ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions
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;
Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions/deletions
> 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
[ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions/deletions
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-
