On Wed, Dec 8, 2021 at 9:00 AM Mohammad Heib <[email protected]> wrote:
>
> 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]>
Thanks for v4.
Applied to the main branch.
Numan
> ---
> 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
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
