[ovs-dev] ofp-flow: Improve error message when cookie cannot be set.
diff mbox series

Message ID 20190716171818.26327-1-blp@ovn.org
State New
Headers show
Series
  • [ovs-dev] ofp-flow: Improve error message when cookie cannot be set.
Related show

Commit Message

Ben Pfaff July 16, 2019, 5:18 p.m. UTC
The "cookie" value has two meanings in "ovs-ofctl add-flow", etc.  With
a mask, it indicates a match; without a mask, it indicates that the
cookie should be set.  In some case, the cookie cannot be set, which may
mean that the user meant to indicate a match.  The error message for this
case was poor; this improves it.

Suggested-by: "Yi Yang (杨燚)-云服务集团" <yangyi01@inspur.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
---
 lib/ofp-flow.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Greg Rose July 16, 2019, 10:36 p.m. UTC | #1
On 7/16/2019 10:18 AM, Ben Pfaff wrote:
> The "cookie" value has two meanings in "ovs-ofctl add-flow", etc.  With
> a mask, it indicates a match; without a mask, it indicates that the
> cookie should be set.  In some case, the cookie cannot be set, which may
> mean that the user meant to indicate a match.  The error message for this
> case was poor; this improves it.
>
> Suggested-by: "Yi Yang (杨燚)-云服务集团" <yangyi01@inspur.com>
> Signed-off-by: Ben Pfaff <blp@ovn.org>
> ---
>   lib/ofp-flow.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/lib/ofp-flow.c b/lib/ofp-flow.c
> index c45afd204f80..ff0396845a4e 100644
> --- a/lib/ofp-flow.c
> +++ b/lib/ofp-flow.c
> @@ -1,5 +1,5 @@
>   /*
> - * Copyright (c) 2008-2017 Nicira, Inc.
> + * Copyright (c) 2008-2017, 2019 Nicira, Inc.
>    *
>    * Licensed under the Apache License, Version 2.0 (the "License");
>    * you may not use this file except in compliance with the License.
> @@ -1660,7 +1660,9 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int command, char *string,
>                       /* No mask means that the cookie is being set. */
>                       if (command != OFPFC_ADD && command != OFPFC_MODIFY
>                           && command != OFPFC_MODIFY_STRICT) {
> -                        return xstrdup("cannot set cookie");
> +                        return xasprintf("cannot set cookie (to match on a "
> +                                         "cookie, specify a mask, e.g. "
> +                                         "cookie=%s/-1)", value);
>                       }
>                       error = str_to_be64(value, &fm->new_cookie);
>                       fm->modify_cookie = true;
More instructive and helpful error messages are a good thing.
Reviewed-by: Greg Rose <gvrose8192@gmail.com>
Ben Pfaff July 17, 2019, 6:09 p.m. UTC | #2
On Tue, Jul 16, 2019 at 03:36:07PM -0700, Gregory Rose wrote:
> 
> On 7/16/2019 10:18 AM, Ben Pfaff wrote:
> > The "cookie" value has two meanings in "ovs-ofctl add-flow", etc.  With
> > a mask, it indicates a match; without a mask, it indicates that the
> > cookie should be set.  In some case, the cookie cannot be set, which may
> > mean that the user meant to indicate a match.  The error message for this
> > case was poor; this improves it.
> > 
> > Suggested-by: "Yi Yang (杨燚)-云服务集团" <yangyi01@inspur.com>
> > Signed-off-by: Ben Pfaff <blp@ovn.org>
> > ---
> >   lib/ofp-flow.c | 6 ++++--
> >   1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/ofp-flow.c b/lib/ofp-flow.c
> > index c45afd204f80..ff0396845a4e 100644
> > --- a/lib/ofp-flow.c
> > +++ b/lib/ofp-flow.c
> > @@ -1,5 +1,5 @@
> >   /*
> > - * Copyright (c) 2008-2017 Nicira, Inc.
> > + * Copyright (c) 2008-2017, 2019 Nicira, Inc.
> >    *
> >    * Licensed under the Apache License, Version 2.0 (the "License");
> >    * you may not use this file except in compliance with the License.
> > @@ -1660,7 +1660,9 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int command, char *string,
> >                       /* No mask means that the cookie is being set. */
> >                       if (command != OFPFC_ADD && command != OFPFC_MODIFY
> >                           && command != OFPFC_MODIFY_STRICT) {
> > -                        return xstrdup("cannot set cookie");
> > +                        return xasprintf("cannot set cookie (to match on a "
> > +                                         "cookie, specify a mask, e.g. "
> > +                                         "cookie=%s/-1)", value);
> >                       }
> >                       error = str_to_be64(value, &fm->new_cookie);
> >                       fm->modify_cookie = true;
> More instructive and helpful error messages are a good thing.
> Reviewed-by: Greg Rose <gvrose8192@gmail.com>

Thanks, Greg!  I applied this to master.

Patch
diff mbox series

diff --git a/lib/ofp-flow.c b/lib/ofp-flow.c
index c45afd204f80..ff0396845a4e 100644
--- a/lib/ofp-flow.c
+++ b/lib/ofp-flow.c
@@ -1,5 +1,5 @@ 
 /*
- * Copyright (c) 2008-2017 Nicira, Inc.
+ * Copyright (c) 2008-2017, 2019 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -1660,7 +1660,9 @@  parse_ofp_str__(struct ofputil_flow_mod *fm, int command, char *string,
                     /* No mask means that the cookie is being set. */
                     if (command != OFPFC_ADD && command != OFPFC_MODIFY
                         && command != OFPFC_MODIFY_STRICT) {
-                        return xstrdup("cannot set cookie");
+                        return xasprintf("cannot set cookie (to match on a "
+                                         "cookie, specify a mask, e.g. "
+                                         "cookie=%s/-1)", value);
                     }
                     error = str_to_be64(value, &fm->new_cookie);
                     fm->modify_cookie = true;