diff mbox series

[ovs-dev,[PATCH,v21.12] ovn v2] acl-log: Log the direction (logical pipeline) of the matching ACL.

Message ID 20220329140745.273804-1-mmichels@redhat.com
State Not Applicable
Headers show
Series [ovs-dev,[PATCH,v21.12] ovn v2] acl-log: Log the direction (logical pipeline) of the matching ACL. | expand

Checks

Context Check Description
ovsrobot/intel-ovs-compilation fail test: fail

Commit Message

Mark Michelson March 29, 2022, 2:07 p.m. UTC 
From: Dumitru Ceara <dceara@redhat.com>

This is a backport of d7514abe117. Its original commit message is
below, including the original sign-offs and acks.

-------------------------

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
Acked-by: Numan Siddique <numans@ovn.org>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Numan Siddique <numans@ovn.org>
Signed-off-by: Mark Michelson <mmichels@redhat.com>
---
v1 -> v2:
  * Eliminated stray conflict marker in NEWS file.
---
 NEWS                  |  2 ++
 controller/pinctrl.c  |  4 ++-
 lib/acl-log.c         |  8 +++--
 lib/acl-log.h         |  3 +-
 ovs                   |  2 +-
 tests/ovn.at          | 68 ++++++++++++++++++++++++++++++++++---------
 utilities/ovn-trace.c |  9 ++++--
 7 files changed, 74 insertions(+), 22 deletions(-)
diff mbox series

Patch

diff --git a/NEWS b/NEWS
index d0570aabb..3b3104c2f 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,7 @@ 
 OVN v21.12.2 - xx xxx xxxx
 --------------------------
+  - When configured to log packets matching ACLs, log the direction (logical
+    pipeline) too.
 
 OVN v21.12.1 - 11 Mar 2022
 --------------------------
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 1b8b47523..2dd1bc7bd 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
+                       ? "from-lport" : "to-lport");
         break;
 
     case ACTION_OPCODE_PUT_ND_RA_OPTS:
diff --git a/lib/acl-log.c b/lib/acl-log.c
index 220b6dc30..9530dd763 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 4f23f790d..da7fa2f02 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/ovs b/ovs
index 498cedc48..91e1ff5dd 160000
--- a/ovs
+++ b/ovs
@@ -1 +1 @@ 
-Subproject commit 498cedc483f3239c839c55b4d9f2261b61fb6ace
+Subproject commit 91e1ff5dde396fbcc8623ac0726066e970e6de15
diff --git a/tests/ovn.at b/tests/ovn.at
index 5625f7767..426a1fbb0 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -9003,33 +9003,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 &&
@@ -9054,25 +9080,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=from-lport: 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=to-lport: 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=from-lport: 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=to-lport: 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=from-lport: 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=to-lport: 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=from-lport: 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=to-lport: 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 0795913d3..ece5803f2 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: