Hi Thomas,
> -----Original Message-----
> From: Thomas Monjalon [mailto:tho...@monjalon.net]
> Sent: Wednesday, October 24, 2018 2:41 PM
> To: Iremonger, Bernard <bernard.iremon...@intel.com>; Wu, Jingjing
> <jingjing...@intel.com>; Lu, Wenzhuo <wenzhuo...@intel.com>
> Cc: dev@dpdk.org; ophi...@mellanox.com; wis...@mellanox.com; Yigit,
> Ferruh <ferruh.yi...@intel.com>; arybche...@solarflare.com
> Subject: [PATCH 4/5] app/testpmd: move ethdev events registration
>
> The callback for ethdev events was registered on port start, so it was missing
> some events.
>
> It is now registered at the beginning of the main function.
>
> Signed-off-by: Thomas Monjalon <tho...@monjalon.net>
> ---
> app/test-pmd/testpmd.c | 72 ++++++++++++++++++++++++------------------
> 1 file changed, 42 insertions(+), 30 deletions(-)
>
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> 72b91adf5..6eb416e7a 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -345,6 +345,21 @@ uint8_t rmv_interrupt = 1; /* enabled by default */
>
> uint8_t hot_plug = 0; /**< hotplug disabled by default. */
>
> +/* Pretty printing of ethdev events */
> +static const char * const eth_event_desc[] = {
> + [RTE_ETH_EVENT_UNKNOWN] = "unknown",
> + [RTE_ETH_EVENT_INTR_LSC] = "LSC",
How about replacing "LSC" with "interrupt link status change"
> + [RTE_ETH_EVENT_QUEUE_STATE] = "queue state",
> + [RTE_ETH_EVENT_INTR_RESET] = "interrupt reset",
> + [RTE_ETH_EVENT_VF_MBOX] = "VF mbox",
> + [RTE_ETH_EVENT_IPSEC] = "IPsec",
> + [RTE_ETH_EVENT_MACSEC] = "MACsec",
> + [RTE_ETH_EVENT_INTR_RMV] = "device removal",
How about replacing "device removal" with "interrupt device removal"
> + [RTE_ETH_EVENT_NEW] = "device probed",
> + [RTE_ETH_EVENT_DESTROY] = "device released",
> + [RTE_ETH_EVENT_MAX] = NULL,
> +};
> +
> /*
> * Display or mask ether events
> * Default to all events except VF_MBOX @@ -1934,7 +1949,6 @@
> start_port(portid_t pid)
> queueid_t qi;
> struct rte_port *port;
> struct ether_addr mac_addr;
> - enum rte_eth_event_type event_type;
>
> if (port_id_is_invalid(pid, ENABLED_WARN))
> return 0;
> @@ -2090,20 +2104,6 @@ start_port(portid_t pid)
> need_check_link_status = 1;
> }
>
> - for (event_type = RTE_ETH_EVENT_UNKNOWN;
> - event_type < RTE_ETH_EVENT_MAX;
> - event_type++) {
> - diag = rte_eth_dev_callback_register(RTE_ETH_ALL,
> - event_type,
> - eth_event_callback,
> - NULL);
> - if (diag) {
> - printf("Failed to setup even callback for event %d\n",
> - event_type);
> - return -1;
> - }
> - }
> -
> if (need_check_link_status == 1 && !no_link_check)
> check_all_ports_link_status(RTE_PORT_ALL);
> else if (need_check_link_status == 0)
> @@ -2522,20 +2522,6 @@ static int
> eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void
> *param,
> void *ret_param)
> {
> - static const char * const event_desc[] = {
> - [RTE_ETH_EVENT_UNKNOWN] = "Unknown",
> - [RTE_ETH_EVENT_INTR_LSC] = "LSC",
> - [RTE_ETH_EVENT_QUEUE_STATE] = "Queue state",
> - [RTE_ETH_EVENT_INTR_RESET] = "Interrupt reset",
> - [RTE_ETH_EVENT_VF_MBOX] = "VF Mbox",
> - [RTE_ETH_EVENT_IPSEC] = "IPsec",
> - [RTE_ETH_EVENT_MACSEC] = "MACsec",
> - [RTE_ETH_EVENT_INTR_RMV] = "device removal",
> - [RTE_ETH_EVENT_NEW] = "device probed",
> - [RTE_ETH_EVENT_DESTROY] = "device released",
> - [RTE_ETH_EVENT_MAX] = NULL,
> - };
> -
> RTE_SET_USED(param);
> RTE_SET_USED(ret_param);
>
> @@ -2545,7 +2531,7 @@ eth_event_callback(portid_t port_id, enum
> rte_eth_event_type type, void *param,
> fflush(stderr);
> } else if (event_print_mask & (UINT32_C(1) << type)) {
> printf("\nPort %" PRIu16 ": %s event\n", port_id,
> - event_desc[type]);
> + eth_event_desc[type]);
> fflush(stdout);
> }
>
> @@ -2564,6 +2550,28 @@ eth_event_callback(portid_t port_id, enum
> rte_eth_event_type type, void *param,
> return 0;
> }
>
> +static int
> +register_eth_event_callback(void)
> +{
> + int ret;
> + enum rte_eth_event_type event;
> +
> + for (event = RTE_ETH_EVENT_UNKNOWN;
> + event < RTE_ETH_EVENT_MAX; event++) {
> + ret = rte_eth_dev_callback_register(RTE_ETH_ALL,
> + event,
> + eth_event_callback,
> + NULL);
> + if (ret != 0) {
> + TESTPMD_LOG(ERR, "Failed to register callback for "
> + "%s event\n", eth_event_desc[event]);
> + return -1;
> + }
> + }
> +
> + return 0;
> +}
> +
> /* This function is used by the interrupt thread */ static void
> eth_dev_event_callback(const char *device_name, enum rte_dev_event_type
> type, @@ -3048,6 +3056,10 @@ main(int argc, char** argv)
> rte_panic("Cannot register log type");
> rte_log_set_level(testpmd_logtype, RTE_LOG_DEBUG);
>
> + ret = register_eth_event_callback();
> + if (ret != 0)
> + rte_panic("Cannot register for ethdev events");
> +
> #ifdef RTE_LIBRTE_PDUMP
> /* initialize packet capture framework */
> rte_pdump_init(NULL);
> --
> 2.19.0
Regards,
Bernard
