On Fri, Feb 11, 2022 at 9:42 AM Dumitru Ceara <[email protected]> wrote: > > It's useful to differentiate between ingress and egress pipelines in the > ACL logs. To achieve this we determine the direction by interpreting the > openflow table ID when processing packets punted to pinctrl by "log()" > action. > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1992641 > Signed-off-by: Dumitru Ceara <[email protected]> > --- > v3: Simplify the patch as suggested by Numan; detect pipeline based on > openflow table id. > v2: Add the "direction" after the "verdict" and "severity" fields in the > ACL logs. > ---
Thanks for v3. I've one minor comment. I somehow feel using "from-lport" and "to-lport" instead of "IN" and "OUT" would be more appropriate. But I'm fine with the present patch too. Acked-by: Numan Siddique <[email protected]> Thanks Numan > NEWS | 2 ++ > controller/pinctrl.c | 4 ++- > lib/acl-log.c | 8 +++-- > lib/acl-log.h | 3 +- > tests/ovn.at | 68 ++++++++++++++++++++++++++++++++++--------- > utilities/ovn-trace.c | 9 ++++-- > 6 files changed, 73 insertions(+), 21 deletions(-) > > diff --git a/NEWS b/NEWS > index 194557410b..a01820dfe6 100644 > --- a/NEWS > +++ b/NEWS > @@ -1,5 +1,7 @@ > Post v21.12.0 > ------------- > + - When configured to log packtes matching ACLs, log the direction (logical > + pipeline) too. > > OVN v21.12.0 - 22 Dec 2021 > -------------------------- > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > index fd0bccdb6c..e275b5e08a 100644 > --- a/controller/pinctrl.c > +++ b/controller/pinctrl.c > @@ -3166,7 +3166,9 @@ process_packet_in(struct rconn *swconn, const struct > ofp_header *msg) > break; > > case ACTION_OPCODE_LOG: > - handle_acl_log(&headers, &userdata); > + handle_acl_log(&headers, &userdata, > + pin.table_id < OFTABLE_LOG_EGRESS_PIPELINE > + ? "IN" : "OUT"); > break; > > case ACTION_OPCODE_PUT_ND_RA_OPTS: > diff --git a/lib/acl-log.c b/lib/acl-log.c > index 220b6dc30e..9530dd7635 100644 > --- a/lib/acl-log.c > +++ b/lib/acl-log.c > @@ -76,7 +76,8 @@ log_severity_from_string(const char *name) > } > > void > -handle_acl_log(const struct flow *headers, struct ofpbuf *userdata) > +handle_acl_log(const struct flow *headers, struct ofpbuf *userdata, > + const char *direction) > { > if (!VLOG_IS_INFO_ENABLED()) { > return; > @@ -94,9 +95,10 @@ handle_acl_log(const struct flow *headers, struct ofpbuf > *userdata) > struct ds ds = DS_EMPTY_INITIALIZER; > ds_put_cstr(&ds, "name="); > json_string_escape(name_len ? name : "<unnamed>", &ds); > - ds_put_format(&ds, ", verdict=%s, severity=%s: ", > + ds_put_format(&ds, ", verdict=%s, severity=%s, direction=%s: ", > log_verdict_to_string(lph->verdict), > - log_severity_to_string(lph->severity)); > + log_severity_to_string(lph->severity), > + direction); > flow_format(&ds, headers, NULL); > > VLOG_INFO("%s", ds_cstr(&ds)); > diff --git a/lib/acl-log.h b/lib/acl-log.h > index 4f23f790dd..da7fa2f02c 100644 > --- a/lib/acl-log.h > +++ b/lib/acl-log.h > @@ -49,6 +49,7 @@ const char *log_verdict_to_string(uint8_t verdict); > const char *log_severity_to_string(uint8_t severity); > uint8_t log_severity_from_string(const char *name); > > -void handle_acl_log(const struct flow *headers, struct ofpbuf *userdata); > +void handle_acl_log(const struct flow *headers, struct ofpbuf *userdata, > + const char *direction); > > #endif /* lib/acl-log.h */ > diff --git a/tests/ovn.at b/tests/ovn.at > index 957eb7850f..cfb9818ced 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -8965,33 +8965,59 @@ ovn-nbctl lsp-set-addresses lp2 $lp2_mac > ovn-nbctl --wait=sb sync > wait_for_ports_up > > -ovn-nbctl acl-add lsw0 to-lport 1000 'tcp.dst==80' drop > -ovn-nbctl --log --severity=alert --name=drop-flow acl-add lsw0 to-lport 1000 > 'tcp.dst==81' drop > +ovn-nbctl acl-add lsw0 from-lport 1000 'tcp.dst==80' drop > +ovn-nbctl --log --severity=alert --name=drop-flow acl-add lsw0 from-lport > 1000 'tcp.dst==81' drop > + > +ovn-nbctl acl-add lsw0 to-lport 1000 'tcp.dst==180' drop > +ovn-nbctl --log --severity=alert --name=drop-flow acl-add lsw0 to-lport 1000 > 'tcp.dst==181' drop > + > +ovn-nbctl acl-add lsw0 from-lport 1000 'tcp.dst==82' allow > +ovn-nbctl --log --severity=info --name=allow-flow acl-add lsw0 from-lport > 1000 'tcp.dst==83' allow > > ovn-nbctl acl-add lsw0 to-lport 1000 'tcp.dst==82' allow > ovn-nbctl --log --severity=info --name=allow-flow acl-add lsw0 to-lport 1000 > 'tcp.dst==83' allow > > +ovn-nbctl acl-add lsw0 from-lport 1000 'tcp.dst==84' allow-related > +ovn-nbctl --log acl-add lsw0 from-lport 1000 'tcp.dst==85' allow-related > + > ovn-nbctl acl-add lsw0 to-lport 1000 'tcp.dst==84' allow-related > ovn-nbctl --log acl-add lsw0 to-lport 1000 'tcp.dst==85' allow-related > > -ovn-nbctl acl-add lsw0 to-lport 1000 'tcp.dst==86' reject > -ovn-nbctl --wait=hv --log --severity=alert --name=reject-flow acl-add lsw0 > to-lport 1000 'tcp.dst==87' reject > +ovn-nbctl acl-add lsw0 from-lport 1000 'tcp.dst==86' reject > +ovn-nbctl --log --severity=alert --name=reject-flow acl-add lsw0 from-lport > 1000 'tcp.dst==87' reject > + > +ovn-nbctl acl-add lsw0 to-lport 1000 'tcp.dst==186' reject > +ovn-nbctl --log --severity=alert --name=reject-flow acl-add lsw0 to-lport > 1000 'tcp.dst==187' reject > + > +ovn-nbctl --wait=hv sync > > ovn-sbctl dump-flows > sbflows > AT_CAPTURE_FILE([sbflows]) > > -# Send packet that should be dropped without logging. > +# Send packet that should be dropped without logging in the ingress pipeline. > packet="inport==\"lp1\" && eth.src==$lp1_mac && eth.dst==$lp2_mac && > ip4 && ip.ttl==64 && ip4.src==$lp1_ip && ip4.dst==$lp2_ip && > tcp && tcp.flags==2 && tcp.src==4360 && tcp.dst==80" > as hv ovs-appctl -t ovn-controller inject-pkt "$packet" > > -# Send packet that should be dropped with logging. > +# Send packet that should be dropped with logging in the ingress pipeline. > packet="inport==\"lp1\" && eth.src==$lp1_mac && eth.dst==$lp2_mac && > ip4 && ip.ttl==64 && ip4.src==$lp1_ip && ip4.dst==$lp2_ip && > tcp && tcp.flags==2 && tcp.src==4361 && tcp.dst==81" > as hv ovs-appctl -t ovn-controller inject-pkt "$packet" > > +# Send packet that should be dropped without logging in the eggress pipeline. > +packet="inport==\"lp1\" && eth.src==$lp1_mac && eth.dst==$lp2_mac && > + ip4 && ip.ttl==64 && ip4.src==$lp1_ip && ip4.dst==$lp2_ip && > + tcp && tcp.flags==2 && tcp.src==4360 && tcp.dst==180" > +as hv ovs-appctl -t ovn-controller inject-pkt "$packet" > + > +# Send packet that should be dropped with logging in the egress pipeline. > +packet="inport==\"lp1\" && eth.src==$lp1_mac && eth.dst==$lp2_mac && > + ip4 && ip.ttl==64 && ip4.src==$lp1_ip && ip4.dst==$lp2_ip && > + tcp && tcp.flags==2 && tcp.src==4361 && tcp.dst==181" > +as hv ovs-appctl -t ovn-controller inject-pkt "$packet" > + > # Send packet that should be allowed without logging. > packet="inport==\"lp1\" && eth.src==$lp1_mac && eth.dst==$lp2_mac && > ip4 && ip.ttl==64 && ip4.src==$lp1_ip && ip4.dst==$lp2_ip && > @@ -9016,25 +9042,41 @@ packet="inport==\"lp1\" && eth.src==$lp1_mac && > eth.dst==$lp2_mac && > tcp && tcp.flags==2 && tcp.src==4365 && tcp.dst==85" > as hv ovs-appctl -t ovn-controller inject-pkt "$packet" > > -# Send packet that should be rejected without logging. > +# Send packet that should be rejected without logging in the ingress > pipeline. > packet="inport==\"lp1\" && eth.src==$lp1_mac && eth.dst==$lp2_mac && > ip4 && ip.ttl==64 && ip4.src==$lp1_ip && ip4.dst==$lp2_ip && > tcp && tcp.flags==2 && tcp.src==4366 && tcp.dst==86" > as hv ovs-appctl -t ovn-controller inject-pkt "$packet" > > -# Send packet that should be rejected with logging. > +# Send packet that should be rejected with logging in the ingress pipeline. > packet="inport==\"lp1\" && eth.src==$lp1_mac && eth.dst==$lp2_mac && > ip4 && ip.ttl==64 && ip4.src==$lp1_ip && ip4.dst==$lp2_ip && > tcp && tcp.flags==2 && tcp.src==4367 && tcp.dst==87" > as hv ovs-appctl -t ovn-controller inject-pkt "$packet" > > -OVS_WAIT_UNTIL([ test 4 = $(grep -c 'acl_log' hv/ovn-controller.log) ]) > +# Send packet that should be rejected without logging in the egress pipeline. > +packet="inport==\"lp1\" && eth.src==$lp1_mac && eth.dst==$lp2_mac && > + ip4 && ip.ttl==64 && ip4.src==$lp1_ip && ip4.dst==$lp2_ip && > + tcp && tcp.flags==2 && tcp.src==4366 && tcp.dst==186" > +as hv ovs-appctl -t ovn-controller inject-pkt "$packet" > + > +# Send packet that should be rejected with logging in the egress pipeline. > +packet="inport==\"lp1\" && eth.src==$lp1_mac && eth.dst==$lp2_mac && > + ip4 && ip.ttl==64 && ip4.src==$lp1_ip && ip4.dst==$lp2_ip && > + tcp && tcp.flags==2 && tcp.src==4367 && tcp.dst==187" > +as hv ovs-appctl -t ovn-controller inject-pkt "$packet" > + > +OVS_WAIT_UNTIL([ test 8 = $(grep -c 'acl_log' hv/ovn-controller.log) ]) > > AT_CHECK([grep 'acl_log' hv/ovn-controller.log | sed 's/.*name=/name=/'], > [0], [dnl > -name="drop-flow", verdict=drop, severity=alert: > tcp,vlan_tci=0x0000,dl_src=f0:00:00:00:00:01,dl_dst=f0:00:00:00:00:02,nw_src=192.168.1.2,nw_dst=192.168.1.3,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=4361,tp_dst=81,tcp_flags=syn > -name="allow-flow", verdict=allow, severity=info: > tcp,vlan_tci=0x0000,dl_src=f0:00:00:00:00:01,dl_dst=f0:00:00:00:00:02,nw_src=192.168.1.2,nw_dst=192.168.1.3,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=4363,tp_dst=83,tcp_flags=syn > -name="<unnamed>", verdict=allow, severity=info: > tcp,vlan_tci=0x0000,dl_src=f0:00:00:00:00:01,dl_dst=f0:00:00:00:00:02,nw_src=192.168.1.2,nw_dst=192.168.1.3,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=4365,tp_dst=85,tcp_flags=syn > -name="reject-flow", verdict=reject, severity=alert: > tcp,vlan_tci=0x0000,dl_src=f0:00:00:00:00:01,dl_dst=f0:00:00:00:00:02,nw_src=192.168.1.2,nw_dst=192.168.1.3,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=4367,tp_dst=87,tcp_flags=syn > +name="drop-flow", verdict=drop, severity=alert, direction=IN: > tcp,vlan_tci=0x0000,dl_src=f0:00:00:00:00:01,dl_dst=f0:00:00:00:00:02,nw_src=192.168.1.2,nw_dst=192.168.1.3,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=4361,tp_dst=81,tcp_flags=syn > +name="drop-flow", verdict=drop, severity=alert, direction=OUT: > tcp,vlan_tci=0x0000,dl_src=f0:00:00:00:00:01,dl_dst=f0:00:00:00:00:02,nw_src=192.168.1.2,nw_dst=192.168.1.3,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=4361,tp_dst=181,tcp_flags=syn > +name="allow-flow", verdict=allow, severity=info, direction=IN: > tcp,vlan_tci=0x0000,dl_src=f0:00:00:00:00:01,dl_dst=f0:00:00:00:00:02,nw_src=192.168.1.2,nw_dst=192.168.1.3,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=4363,tp_dst=83,tcp_flags=syn > +name="allow-flow", verdict=allow, severity=info, direction=OUT: > tcp,vlan_tci=0x0000,dl_src=f0:00:00:00:00:01,dl_dst=f0:00:00:00:00:02,nw_src=192.168.1.2,nw_dst=192.168.1.3,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=4363,tp_dst=83,tcp_flags=syn > +name="<unnamed>", verdict=allow, severity=info, direction=IN: > tcp,vlan_tci=0x0000,dl_src=f0:00:00:00:00:01,dl_dst=f0:00:00:00:00:02,nw_src=192.168.1.2,nw_dst=192.168.1.3,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=4365,tp_dst=85,tcp_flags=syn > +name="<unnamed>", verdict=allow, severity=info, direction=OUT: > tcp,vlan_tci=0x0000,dl_src=f0:00:00:00:00:01,dl_dst=f0:00:00:00:00:02,nw_src=192.168.1.2,nw_dst=192.168.1.3,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=4365,tp_dst=85,tcp_flags=syn > +name="reject-flow", verdict=reject, severity=alert, direction=IN: > tcp,vlan_tci=0x0000,dl_src=f0:00:00:00:00:01,dl_dst=f0:00:00:00:00:02,nw_src=192.168.1.2,nw_dst=192.168.1.3,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=4367,tp_dst=87,tcp_flags=syn > +name="reject-flow", verdict=reject, severity=alert, direction=OUT: > tcp,vlan_tci=0x0000,dl_src=f0:00:00:00:00:01,dl_dst=f0:00:00:00:00:02,nw_src=192.168.1.2,nw_dst=192.168.1.3,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=4367,tp_dst=187,tcp_flags=syn > ]) > > OVN_CLEANUP([hv]) > diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c > index 0795913d3e..ece5803f29 100644 > --- a/utilities/ovn-trace.c > +++ b/utilities/ovn-trace.c > @@ -2457,12 +2457,14 @@ execute_select(const struct ovnact_select *select, > > static void > execute_log(const struct ovnact_log *log, struct flow *uflow, > - struct ovs_list *super) > + struct ovs_list *super, const char *direction) > { > char *packet_str = flow_to_string(uflow, NULL); > ovntrace_node_append(super, OVNTRACE_NODE_TRANSFORMATION, > - "LOG: ACL name=%s, verdict=%s, severity=%s, > packet=\"%s\"", > + "LOG: ACL name=%s, direction=%s, verdict=%s, " > + "severity=%s, packet=\"%s\"", > log->name ? log->name : "<unnamed>", > + direction, > log_verdict_to_string(log->verdict), > log_severity_to_string(log->severity), > packet_str); > @@ -2726,7 +2728,8 @@ trace_actions(const struct ovnact *ovnacts, size_t > ovnacts_len, > break; > > case OVNACT_LOG: > - execute_log(ovnact_get_LOG(a), uflow, super); > + execute_log(ovnact_get_LOG(a), uflow, super, > + pipeline == OVNACT_P_INGRESS ? "IN" : "OUT"); > break; > > case OVNACT_SET_METER: > -- > 2.27.0 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
