Improve packet-in debuggability within pinctrl module
by printing basic details about each received packet-in
message, those messages will be printed to the logs only
when DBG log level is enabled.

Also, add two coverage counters that will indicate the total
packet-in messages that were received and the number of times
that the pinctrl main thread was notified to handle a change
in the local DBs, those counters can be used by the user as
an indicator to enable the DBG logs level and see more details
about the received packet-in in the logs.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1821965
Signed-off-by: Mohammad Heib <[email protected]>
---
 controller/pinctrl.c  | 39 ++++++++++++++++++++++++++++++++++++++
 include/ovn/actions.h |  1 +
 lib/actions.c         | 44 +++++++++++++++++++++++++++++++++++++++++++
 tests/ovn.at          |  8 ++++++++
 4 files changed, 92 insertions(+)

diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 0d443c150..4ce16ac74 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -364,6 +364,8 @@ COVERAGE_DEFINE(pinctrl_drop_put_mac_binding);
 COVERAGE_DEFINE(pinctrl_drop_buffered_packets_map);
 COVERAGE_DEFINE(pinctrl_drop_controller_event);
 COVERAGE_DEFINE(pinctrl_drop_put_vport_binding);
+COVERAGE_DEFINE(pinctrl_notify_main_thread);
+COVERAGE_DEFINE(pinctrl_total_pin_pkts);
 
 struct empty_lb_backends_event {
     struct hmap_node hmap_node;
@@ -3268,6 +3270,41 @@ process_packet_in(struct rconn *swconn, const struct 
ofp_header *msg)
                      ntohl(ah->opcode));
         break;
     }
+
+
+    if (VLOG_IS_DBG_ENABLED()) {
+        struct ds pin_str = DS_EMPTY_INITIALIZER;
+        char * opc_str = ovnact_op_to_string(ntohl(ah->opcode));
+
+        ds_put_format(&pin_str,
+                        "pinctrl received  packet-in | opcode=%s",
+                        opc_str);
+
+        ds_put_format(&pin_str, "| OF_Table_ID=%u", pin.table_id);
+        ds_put_format(&pin_str, "| OF_Cookie_ID=0x%"PRIx64,
+                        ntohll(pin.cookie));
+
+        if (pin.flow_metadata.flow.in_port.ofp_port) {
+            ds_put_format(&pin_str, "| in-port=%u",
+                            pin.flow_metadata.flow.in_port.ofp_port);
+        }
+
+        ds_put_format(&pin_str, "| src-mac="ETH_ADDR_FMT",",
+                        ETH_ADDR_ARGS(headers.dl_src));
+        ds_put_format(&pin_str, " dst-mac="ETH_ADDR_FMT,
+                        ETH_ADDR_ARGS(headers.dl_dst));
+        if (headers.dl_type != htons(ETH_TYPE_IPV6)) {
+            ds_put_format(&pin_str, "| src-ip="IP_FMT",",
+                            IP_ARGS(headers.nw_src));
+            ds_put_format(&pin_str, " dst-ip="IP_FMT,
+                            IP_ARGS(headers.nw_dst));
+        }
+
+        VLOG_DBG("%s \n", ds_cstr(&pin_str));
+        ds_destroy(&pin_str);
+        free(opc_str);
+    }
+
 }
 
 /* Called with in the pinctrl_handler thread context. */
@@ -3285,6 +3322,7 @@ pinctrl_recv(struct rconn *swconn, const struct 
ofp_header *oh,
         config.miss_send_len = UINT16_MAX;
         set_switch_config(swconn, &config);
     } else if (type == OFPTYPE_PACKET_IN) {
+        COVERAGE_INC(pinctrl_total_pin_pkts);
         process_packet_in(swconn, oh);
     } else {
         if (VLOG_IS_DBG_ENABLED()) {
@@ -3309,6 +3347,7 @@ notify_pinctrl_handler(void)
 static void
 notify_pinctrl_main(void)
 {
+    COVERAGE_INC(pinctrl_notify_main_thread);
     seq_change(pinctrl_main_seq);
 }
 
diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index ede5eb93c..cdef5fb03 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -806,5 +806,6 @@ void ovnacts_encode(const struct ovnact[], size_t 
ovnacts_len,
                     struct ofpbuf *ofpacts);
 
 void ovnacts_free(struct ovnact[], size_t ovnacts_len);
+char *ovnact_op_to_string(uint32_t);
 
 #endif /* ovn/actions.h */
diff --git a/lib/actions.c b/lib/actions.c
index 6b9a426ae..da00ee349 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -4315,3 +4315,47 @@ ovnacts_free(struct ovnact *ovnacts, size_t ovnacts_len)
         }
     }
 }
+
+/* Return ovn action opcode string representation.
+ * The returned memory is dynamically allocated
+ * and the caller must free it using free().
+ */
+
+char *
+ovnact_op_to_string(uint32_t ovnact_opc)
+{
+    switch (ovnact_opc) {
+#define ACTION_OPCODES                              \
+        ACTION_OPCODE(ARP)                          \
+        ACTION_OPCODE(IGMP)                         \
+        ACTION_OPCODE(PUT_ARP)                      \
+        ACTION_OPCODE(PUT_DHCP_OPTS)                \
+        ACTION_OPCODE(ND_NA)                        \
+        ACTION_OPCODE(ND_NA_ROUTER)                 \
+        ACTION_OPCODE(PUT_ND)                       \
+        ACTION_OPCODE(PUT_FDB)                      \
+        ACTION_OPCODE(PUT_DHCPV6_OPTS)              \
+        ACTION_OPCODE(DNS_LOOKUP)                   \
+        ACTION_OPCODE(LOG)                          \
+        ACTION_OPCODE(PUT_ND_RA_OPTS)               \
+        ACTION_OPCODE(ND_NS)                        \
+        ACTION_OPCODE(ICMP)                         \
+        ACTION_OPCODE(ICMP4_ERROR)                  \
+        ACTION_OPCODE(ICMP6_ERROR)                  \
+        ACTION_OPCODE(TCP_RESET)                    \
+        ACTION_OPCODE(SCTP_ABORT)                   \
+        ACTION_OPCODE(REJECT)                       \
+        ACTION_OPCODE(PUT_ICMP4_FRAG_MTU)           \
+        ACTION_OPCODE(PUT_ICMP6_FRAG_MTU)           \
+        ACTION_OPCODE(EVENT)                        \
+        ACTION_OPCODE(BIND_VPORT)                   \
+        ACTION_OPCODE(DHCP6_SERVER)                 \
+        ACTION_OPCODE(HANDLE_SVC_CHECK)             \
+        ACTION_OPCODE(BFD_MSG)
+#define ACTION_OPCODE(ENUM) \
+    case ACTION_OPCODE_##ENUM: return xstrdup(#ENUM);
+    ACTION_OPCODES
+#undef ACTION_OPCODE
+    default: return xasprintf("unrecognized(%"PRIu32")", ovnact_opc);
+    }
+}
diff --git a/tests/ovn.at b/tests/ovn.at
index a4ed03041..9ec62e321 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -18283,6 +18283,8 @@ ovs-vsctl -- add-port br-int hv1-vif3 -- \
     options:rxq_pcap=hv1/vif3-rx.pcap \
     ofport-request=3
 
+ovs-appctl -t ovn-controller vlog/set dbg
+
 sim_add hv2
 as hv2
 ovs-vsctl add-br br-phys
@@ -18473,6 +18475,8 @@ wait_row_count Port_Binding 1 logical_port=sw0-vir 
chassis=$hv1_ch_uuid
 check_row_count Port_Binding 1 logical_port=sw0-vir virtual_parent=sw0-p1
 wait_for_ports_up sw0-vir
 check ovn-nbctl --wait=hv sync
+AT_CHECK([test 2 = `cat hv1/ovn-controller.log | grep "pinctrl received  
packet-in" | \
+grep opcode=BIND_VPORT | grep OF_Table_ID=24 | wc -l`])
 
 wait_row_count Port_Binding 1 logical_port=sw0-vir6 chassis=$hv1_ch_uuid
 check_row_count Port_Binding 1 logical_port=sw0-vir6 virtual_parent=sw0-p1
@@ -21339,6 +21343,10 @@ list mac_binding], [0], [lr0-sw0
 
 AT_CHECK([test 1 = `cat hv1/ovn-controller.log | grep NXT_PACKET_IN2 | \
 grep table_id=10 | wc -l`])
+
+AT_CHECK([test 1 = `cat hv1/ovn-controller.log | grep "pinctrl received  
packet-in" | \
+grep opcode=PUT_ARP | grep OF_Table_ID=10 | wc -l`])
+
 AT_CHECK([test 1 = `as hv1 ovs-ofctl dump-flows br-int table=10 | grep arp | \
 grep controller | grep -v n_packets=0 | wc -l`])
 
-- 
2.26.3

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to