| Message ID | 20240325174828.530932-1-mmichels@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v2] acl-log: Properly log the "pass" verdict. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
On Mon, Mar 25, 2024 at 6:48 PM Mark Michelson <mmichels@redhat.com> wrote: > The "pass" verdict was not explicitly defined in the list of verdicts > for ACL logging. This resulted in logs saying "Syntax error at `pass' > unknown verdict." > > This change adds the "pass" verdict explicitly so that it shows up as a > proper log in ovn-controller. > > Reported-at: https://issues.redhat.com/browse/FDP-442 > Signed-off-by: Mark Michelson <mmichels@redhat.com> > --- > lib/acl-log.c | 4 +++- > lib/acl-log.h | 1 + > lib/actions.c | 2 ++ > tests/ovn.at | 3 +++ > 4 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/lib/acl-log.c b/lib/acl-log.c > index 9530dd763..b3eb4bbd0 100644 > --- a/lib/acl-log.c > +++ b/lib/acl-log.c > @@ -34,7 +34,9 @@ log_verdict_to_string(uint8_t verdict) > return "drop"; > } else if (verdict == LOG_VERDICT_REJECT) { > return "reject"; > - } else { > + } else if (verdict == LOG_VERDICT_PASS) { > + return "pass"; > + } else { > return "<unknown>"; > } > } > diff --git a/lib/acl-log.h b/lib/acl-log.h > index da7fa2f02..3973a8e0b 100644 > --- a/lib/acl-log.h > +++ b/lib/acl-log.h > @@ -33,6 +33,7 @@ enum log_verdict { > LOG_VERDICT_ALLOW, > LOG_VERDICT_DROP, > LOG_VERDICT_REJECT, > + LOG_VERDICT_PASS, > LOG_VERDICT_UNKNOWN = UINT8_MAX > }; > > diff --git a/lib/actions.c b/lib/actions.c > index 71615fc53..29584a189 100644 > --- a/lib/actions.c > +++ b/lib/actions.c > @@ -3596,6 +3596,8 @@ parse_log_arg(struct action_context *ctx, struct > ovnact_log *log) > log->verdict = LOG_VERDICT_REJECT; > } else if (lexer_match_id(ctx->lexer, "allow")) { > log->verdict = LOG_VERDICT_ALLOW; > + } else if (lexer_match_id(ctx->lexer, "pass")) { > + log->verdict = LOG_VERDICT_PASS; > } else { > lexer_syntax_error(ctx->lexer, "unknown verdict"); > return; > diff --git a/tests/ovn.at b/tests/ovn.at > index 4d0c7ad53..f272749aa 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -1847,6 +1847,9 @@ log(name="test1", verdict=drop, severity=info, > meter="meter1"); > 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=pass); > + formats as log(verdict=pass, severity=info); > + encodes as controller(userdata=00.00.00.07.00.00.00.00.03.06) > log(verdict=bad_verdict, severity=info); > Syntax error at `bad_verdict' unknown verdict. > log(verdict=drop, severity=bad_severity); > -- > 2.44.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Looks good to me, thanks. Acked-by: Ales Musil <amusil@redhat.com>
Thanks for the review Ales. I merged this to main and all branches down to 23.06. On 3/26/24 02:26, Ales Musil wrote: > > > On Mon, Mar 25, 2024 at 6:48 PM Mark Michelson <mmichels@redhat.com > <mailto:mmichels@redhat.com>> wrote: > > The "pass" verdict was not explicitly defined in the list of verdicts > for ACL logging. This resulted in logs saying "Syntax error at `pass' > unknown verdict." > > This change adds the "pass" verdict explicitly so that it shows up as a > proper log in ovn-controller. > > Reported-at: https://issues.redhat.com/browse/FDP-442 > <https://issues.redhat.com/browse/FDP-442> > Signed-off-by: Mark Michelson <mmichels@redhat.com > <mailto:mmichels@redhat.com>> > --- > lib/acl-log.c | 4 +++- > lib/acl-log.h | 1 + > lib/actions.c | 2 ++ > tests/ovn.at <http://ovn.at> | 3 +++ > 4 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/lib/acl-log.c b/lib/acl-log.c > index 9530dd763..b3eb4bbd0 100644 > --- a/lib/acl-log.c > +++ b/lib/acl-log.c > @@ -34,7 +34,9 @@ log_verdict_to_string(uint8_t verdict) > return "drop"; > } else if (verdict == LOG_VERDICT_REJECT) { > return "reject"; > - } else { > + } else if (verdict == LOG_VERDICT_PASS) { > + return "pass"; > + } else { > return "<unknown>"; > } > } > diff --git a/lib/acl-log.h b/lib/acl-log.h > index da7fa2f02..3973a8e0b 100644 > --- a/lib/acl-log.h > +++ b/lib/acl-log.h > @@ -33,6 +33,7 @@ enum log_verdict { > LOG_VERDICT_ALLOW, > LOG_VERDICT_DROP, > LOG_VERDICT_REJECT, > + LOG_VERDICT_PASS, > LOG_VERDICT_UNKNOWN = UINT8_MAX > }; > > diff --git a/lib/actions.c b/lib/actions.c > index 71615fc53..29584a189 100644 > --- a/lib/actions.c > +++ b/lib/actions.c > @@ -3596,6 +3596,8 @@ parse_log_arg(struct action_context *ctx, > struct ovnact_log *log) > log->verdict = LOG_VERDICT_REJECT; > } else if (lexer_match_id(ctx->lexer, "allow")) { > log->verdict = LOG_VERDICT_ALLOW; > + } else if (lexer_match_id(ctx->lexer, "pass")) { > + log->verdict = LOG_VERDICT_PASS; > } else { > lexer_syntax_error(ctx->lexer, "unknown verdict"); > return; > diff --git a/tests/ovn.at <http://ovn.at> b/tests/ovn.at <http://ovn.at> > index 4d0c7ad53..f272749aa 100644 > --- a/tests/ovn.at <http://ovn.at> > +++ b/tests/ovn.at <http://ovn.at> > @@ -1847,6 +1847,9 @@ log(name="test1", verdict=drop, severity=info, > meter="meter1"); > 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=pass); > + formats as log(verdict=pass, severity=info); > + encodes as controller(userdata=00.00.00.07.00.00.00.00.03.06) > log(verdict=bad_verdict, severity=info); > Syntax error at `bad_verdict' unknown verdict. > log(verdict=drop, severity=bad_severity); > -- > 2.44.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org <mailto:dev@openvswitch.org> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > <https://mail.openvswitch.org/mailman/listinfo/ovs-dev> > > > > Looks good to me, thanks. > > Acked-by: Ales Musil <amusil@redhat.com <mailto:amusil@redhat.com>> > > -- > > Ales Musil > > Senior Software Engineer - OVN Core > > Red Hat EMEA <https://www.redhat.com> > > amusil@redhat.com <mailto:amusil@redhat.com> > > <https://red.ht/sig> >
diff --git a/lib/acl-log.c b/lib/acl-log.c index 9530dd763..b3eb4bbd0 100644 --- a/lib/acl-log.c +++ b/lib/acl-log.c @@ -34,7 +34,9 @@ log_verdict_to_string(uint8_t verdict) return "drop"; } else if (verdict == LOG_VERDICT_REJECT) { return "reject"; - } else { + } else if (verdict == LOG_VERDICT_PASS) { + return "pass"; + } else { return "<unknown>"; } } diff --git a/lib/acl-log.h b/lib/acl-log.h index da7fa2f02..3973a8e0b 100644 --- a/lib/acl-log.h +++ b/lib/acl-log.h @@ -33,6 +33,7 @@ enum log_verdict { LOG_VERDICT_ALLOW, LOG_VERDICT_DROP, LOG_VERDICT_REJECT, + LOG_VERDICT_PASS, LOG_VERDICT_UNKNOWN = UINT8_MAX }; diff --git a/lib/actions.c b/lib/actions.c index 71615fc53..29584a189 100644 --- a/lib/actions.c +++ b/lib/actions.c @@ -3596,6 +3596,8 @@ parse_log_arg(struct action_context *ctx, struct ovnact_log *log) log->verdict = LOG_VERDICT_REJECT; } else if (lexer_match_id(ctx->lexer, "allow")) { log->verdict = LOG_VERDICT_ALLOW; + } else if (lexer_match_id(ctx->lexer, "pass")) { + log->verdict = LOG_VERDICT_PASS; } else { lexer_syntax_error(ctx->lexer, "unknown verdict"); return; diff --git a/tests/ovn.at b/tests/ovn.at index 4d0c7ad53..f272749aa 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -1847,6 +1847,9 @@ log(name="test1", verdict=drop, severity=info, meter="meter1"); 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=pass); + formats as log(verdict=pass, severity=info); + encodes as controller(userdata=00.00.00.07.00.00.00.00.03.06) log(verdict=bad_verdict, severity=info); Syntax error at `bad_verdict' unknown verdict. log(verdict=drop, severity=bad_severity);
The "pass" verdict was not explicitly defined in the list of verdicts for ACL logging. This resulted in logs saying "Syntax error at `pass' unknown verdict." This change adds the "pass" verdict explicitly so that it shows up as a proper log in ovn-controller. Reported-at: https://issues.redhat.com/browse/FDP-442 Signed-off-by: Mark Michelson <mmichels@redhat.com> --- lib/acl-log.c | 4 +++- lib/acl-log.h | 1 + lib/actions.c | 2 ++ tests/ovn.at | 3 +++ 4 files changed, 9 insertions(+), 1 deletion(-)