| Message ID | 20190716171818.26327-1-blp@ovn.org |
|---|---|
| State | Accepted |
| Commit | 07468070541ccd1a0dd82292dc8cb5dfdef02dfd |
| Headers | show |
| Series | [ovs-dev] ofp-flow: Improve error message when cookie cannot be set. | expand |
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>
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.
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;
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(-)