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