| Message ID | 20180731060319.27558-1-jpettit@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] ovn: Clean up log() action parsing errors. | expand |
Bleep bloop. Greetings Justin Pettit, I am a robot and I have tried out your patch.
Thanks for your contribution.
I encountered some error that I wasn't expecting. See the details below.
build:
depbase=`echo ovn/controller/lflow.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\
gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Wno-null-pointer-arithmetic -Werror -Werror -g -O2 -MT ovn/controller/lflow.o -MD -MP -MF $depbase.Tpo -c -o ovn/controller/lflow.o ovn/controller/lflow.c &&\
mv -f $depbase.Tpo $depbase.Po
depbase=`echo ovn/controller/lport.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\
gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Wno-null-pointer-arithmetic -Werror -Werror -g -O2 -MT ovn/controller/lport.o -MD -MP -MF $depbase.Tpo -c -o ovn/controller/lport.o ovn/controller/lport.c &&\
mv -f $depbase.Tpo $depbase.Po
depbase=`echo ovn/controller/ofctrl.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\
gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Wno-null-pointer-arithmetic -Werror -Werror -g -O2 -MT ovn/controller/ofctrl.o -MD -MP -MF $depbase.Tpo -c -o ovn/controller/ofctrl.o ovn/controller/ofctrl.c &&\
mv -f $depbase.Tpo $depbase.Po
ovn/controller/ofctrl.c: In function ‘ofctrl_put’:
ovn/controller/ofctrl.c:1086:9: error: missing initializer for field ‘flags’ of ‘struct ofputil_meter_config’ [-Werror=missing-field-initializers]
};
^
In file included from ovn/controller/ofctrl.c:35:0:
./include/openvswitch/ofp-meter.h:53:14: note: ‘flags’ declared here
uint16_t flags;
^
ovn/controller/ofctrl.c: At top level:
cc1: error: unrecognized command line option "-Wno-null-pointer-arithmetic" [-Werror]
cc1: all warnings being treated as errors
make[2]: *** [ovn/controller/ofctrl.o] Error 1
make[2]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace'
make: *** [all] Error 2
Please check this out. If you feel there has been an error, please email aconole@bytheb.org
Thanks,
0-day Robot
The 0-day Robot needs to lay off the pipe. Acked-by: Mark Michelson <mmichels@redhat.com> On 07/31/2018 02:03 AM, Justin Pettit wrote: > This also add some OVN action parsing tests. > > Suggested-by: Ben Pfaff <blp@ovn.org> > Signed-off-by: Justin Pettit <jpettit@ovn.org> > --- > ovn/lib/actions.c | 9 ++++++++- > tests/ovn.at | 19 +++++++++++++++++++ > 2 files changed, 27 insertions(+), 1 deletion(-) > > diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c > index 2126fd57551e..bc1a20040cff 100644 > --- a/ovn/lib/actions.c > +++ b/ovn/lib/actions.c > @@ -2071,7 +2071,8 @@ parse_log_arg(struct action_context *ctx, struct ovnact_log *log) > } else if (lexer_match_id(ctx->lexer, "allow")) { > log->verdict = LOG_VERDICT_ALLOW; > } else { > - lexer_syntax_error(ctx->lexer, "unknown acl verdict"); > + lexer_syntax_error(ctx->lexer, "unknown verdict"); > + return; > } > } else if (lexer_match_id(ctx->lexer, "name")) { > if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) { > @@ -2103,6 +2104,9 @@ parse_log_arg(struct action_context *ctx, struct ovnact_log *log) > log->severity = severity; > lexer_get(ctx->lexer); > return; > + } else { > + lexer_syntax_error(ctx->lexer, "unknown severity"); > + return; > } > } > lexer_syntax_error(ctx->lexer, "expecting severity"); > @@ -2142,6 +2146,9 @@ parse_LOG(struct action_context *ctx) > lexer_match(ctx->lexer, LEX_T_COMMA); > } > } > + if (log->verdict == LOG_VERDICT_UNKNOWN) { > + lexer_syntax_error(ctx->lexer, "expecting verdict"); > + } > } > > static void > diff --git a/tests/ovn.at b/tests/ovn.at > index df80b98d64b4..163f5c590516 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -1224,6 +1224,25 @@ set_meter(100, 1000, ); > set_meter(4294967295, 4294967295); > encodes as meter:3 > > +# log > +log(verdict=allow, severity=warning); > + encodes as controller(userdata=00.00.00.07.00.00.00.00.00.04) > +log(name="test1", verdict=drop, severity=info); > + encodes as controller(userdata=00.00.00.07.00.00.00.00.01.06.74.65.73.74.31) > +log(verdict=drop, severity=info, meter="meter1"); > + encodes as controller(userdata=00.00.00.07.00.00.00.00.01.06,meter_id=4) > +log(name="test1", verdict=drop, severity=info, meter="meter1"); > + encodes as controller(userdata=00.00.00.07.00.00.00.00.01.06.74.65.73.74.31,meter_id=4) > +log(verdict=drop); > + formats as log(verdict=drop, severity=info); > + encodes as controller(userdata=00.00.00.07.00.00.00.00.01.06) > +log(verdict=bad_verdict, severity=info); > + Syntax error at `bad_verdict' unknown verdict. > +log(verdict=drop, severity=bad_severity); > + Syntax error at `bad_severity' unknown severity. > +log(severity=notice); > + Syntax error at `;' expecting verdict. > + > # put_nd_ra_opts > reg1[0] = put_nd_ra_opts(addr_mode = "slaac", mtu = 1500, prefix = aef0::/64, slla = ae:01:02:03:04:05); > encodes as controller(userdata=00.00.00.08.00.00.00.00.00.01.de.10.00.00.00.40.86.00.00.00.ff.00.ff.ff.00.00.00.00.00.00.00.00.05.01.00.00.00.00.05.dc.03.04.40.c0.ff.ff.ff.ff.ff.ff.ff.ff.00.00.00.00.ae.f0.00.00.00.00.00.00.00.00.00.00.00.00.00.00.01.01.ae.01.02.03.04.05,pause) >
> On Jul 31, 2018, at 5:21 AM, Mark Michelson <mmichels@redhat.com> wrote: > > The 0-day Robot needs to lay off the pipe. Yeah, I'm not sure what was happening there. I emailed Aaron about it. > Acked-by: Mark Michelson <mmichels@redhat.com> Thanks! I pushed this to master and branch-2.10. --Justin
On Tue, Jul 31, 2018 at 09:13:37AM -0700, Justin Pettit wrote: > > > On Jul 31, 2018, at 5:21 AM, Mark Michelson <mmichels@redhat.com> wrote: > > > > The 0-day Robot needs to lay off the pipe. > > Yeah, I'm not sure what was happening there. I emailed Aaron about it. It's just a GCC warning. Fix: https://patchwork.ozlabs.org/patch/951704/
> On Jul 31, 2018, at 9:58 AM, Ben Pfaff <blp@ovn.org> wrote: > > On Tue, Jul 31, 2018 at 09:13:37AM -0700, Justin Pettit wrote: >> >>> On Jul 31, 2018, at 5:21 AM, Mark Michelson <mmichels@redhat.com> wrote: >>> >>> The 0-day Robot needs to lay off the pipe. >> >> Yeah, I'm not sure what was happening there. I emailed Aaron about it. > > It's just a GCC warning. Fix: > https://patchwork.ozlabs.org/patch/951704/ Interesting. I don't see it with gcc 7.3.0. Thanks for the fix. --Justin
On Tue, Jul 31, 2018 at 10:00:42AM -0700, Justin Pettit wrote: > > > On Jul 31, 2018, at 9:58 AM, Ben Pfaff <blp@ovn.org> wrote: > > > > On Tue, Jul 31, 2018 at 09:13:37AM -0700, Justin Pettit wrote: > >> > >>> On Jul 31, 2018, at 5:21 AM, Mark Michelson <mmichels@redhat.com> wrote: > >>> > >>> The 0-day Robot needs to lay off the pipe. > >> > >> Yeah, I'm not sure what was happening there. I emailed Aaron about it. > > > > It's just a GCC warning. Fix: > > https://patchwork.ozlabs.org/patch/951704/ > > Interesting. I don't see it with gcc 7.3.0. Thanks for the fix. Older GCC had some weird warnings.
Justin Pettit <jpettit@ovn.org> writes: >> On Jul 31, 2018, at 5:21 AM, Mark Michelson <mmichels@redhat.com> wrote: >> >> The 0-day Robot needs to lay off the pipe. > > Yeah, I'm not sure what was happening there. I emailed Aaron about it. Sorry about that. I'll work on protecting the list for when the robot indulges in compile-altering substances. It's a young bot - experimentation is to be expected. >> Acked-by: Mark Michelson <mmichels@redhat.com> > > Thanks! I pushed this to master and branch-2.10. > > --Justin > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On Tue, Jul 31, 2018 at 04:25:32PM -0400, Aaron Conole wrote: > Justin Pettit <jpettit@ovn.org> writes: > > >> On Jul 31, 2018, at 5:21 AM, Mark Michelson <mmichels@redhat.com> wrote: > >> > >> The 0-day Robot needs to lay off the pipe. > > > > Yeah, I'm not sure what was happening there. I emailed Aaron about it. > > Sorry about that. I'll work on protecting the list for when the robot > indulges in compile-altering substances. It's a young bot - > experimentation is to be expected. For what it's worth, I appreciate the bot. It needs a little training but that's OK. (And I still giggle a little every time I see the "Bleep, bloop" greeting.)
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c index 2126fd57551e..bc1a20040cff 100644 --- a/ovn/lib/actions.c +++ b/ovn/lib/actions.c @@ -2071,7 +2071,8 @@ parse_log_arg(struct action_context *ctx, struct ovnact_log *log) } else if (lexer_match_id(ctx->lexer, "allow")) { log->verdict = LOG_VERDICT_ALLOW; } else { - lexer_syntax_error(ctx->lexer, "unknown acl verdict"); + lexer_syntax_error(ctx->lexer, "unknown verdict"); + return; } } else if (lexer_match_id(ctx->lexer, "name")) { if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) { @@ -2103,6 +2104,9 @@ parse_log_arg(struct action_context *ctx, struct ovnact_log *log) log->severity = severity; lexer_get(ctx->lexer); return; + } else { + lexer_syntax_error(ctx->lexer, "unknown severity"); + return; } } lexer_syntax_error(ctx->lexer, "expecting severity"); @@ -2142,6 +2146,9 @@ parse_LOG(struct action_context *ctx) lexer_match(ctx->lexer, LEX_T_COMMA); } } + if (log->verdict == LOG_VERDICT_UNKNOWN) { + lexer_syntax_error(ctx->lexer, "expecting verdict"); + } } static void diff --git a/tests/ovn.at b/tests/ovn.at index df80b98d64b4..163f5c590516 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -1224,6 +1224,25 @@ set_meter(100, 1000, ); set_meter(4294967295, 4294967295); encodes as meter:3 +# log +log(verdict=allow, severity=warning); + encodes as controller(userdata=00.00.00.07.00.00.00.00.00.04) +log(name="test1", verdict=drop, severity=info); + encodes as controller(userdata=00.00.00.07.00.00.00.00.01.06.74.65.73.74.31) +log(verdict=drop, severity=info, meter="meter1"); + encodes as controller(userdata=00.00.00.07.00.00.00.00.01.06,meter_id=4) +log(name="test1", verdict=drop, severity=info, meter="meter1"); + encodes as controller(userdata=00.00.00.07.00.00.00.00.01.06.74.65.73.74.31,meter_id=4) +log(verdict=drop); + formats as log(verdict=drop, severity=info); + encodes as controller(userdata=00.00.00.07.00.00.00.00.01.06) +log(verdict=bad_verdict, severity=info); + Syntax error at `bad_verdict' unknown verdict. +log(verdict=drop, severity=bad_severity); + Syntax error at `bad_severity' unknown severity. +log(severity=notice); + Syntax error at `;' expecting verdict. + # put_nd_ra_opts reg1[0] = put_nd_ra_opts(addr_mode = "slaac", mtu = 1500, prefix = aef0::/64, slla = ae:01:02:03:04:05); encodes as controller(userdata=00.00.00.08.00.00.00.00.00.01.de.10.00.00.00.40.86.00.00.00.ff.00.ff.ff.00.00.00.00.00.00.00.00.05.01.00.00.00.00.05.dc.03.04.40.c0.ff.ff.ff.ff.ff.ff.ff.ff.00.00.00.00.ae.f0.00.00.00.00.00.00.00.00.00.00.00.00.00.00.01.01.ae.01.02.03.04.05,pause)
This also add some OVN action parsing tests. Suggested-by: Ben Pfaff <blp@ovn.org> Signed-off-by: Justin Pettit <jpettit@ovn.org> --- ovn/lib/actions.c | 9 ++++++++- tests/ovn.at | 19 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-)