Hi Thomas From: Thomas Monjalon, December 29, 2017 > Hi Matan, > > Please find some review details below. > > As this patch is needed for the notification of new ports, I will re-send them > in a patchset, with the minor modifications described below. > > 04/12/2017 16:43, Matan Azrad: > > --- a/lib/librte_ether/rte_ethdev.c > > +++ b/lib/librte_ether/rte_ethdev.c > > +RTE_INIT(eth_dev_init_cb_lists); > [...] > > +static void > > +eth_dev_init_cb_lists(void) > > RTE_INIT macro can be used in function definition without prior declaration. > > This function should be moved just before the callback register/unregister > functions. >
OK, nice. > > @@ -2827,37 +2837,59 @@ > > + uint32_t next_port; > > + uint32_t last_port; > > A port id should be uint16_t. > Yes, I know but please note that we use next_port variable in the while statement and it can be rolled in case the max value of port id is the max value of uint16_t type. This is the reason I defined it as uint32_t type. > > --- a/lib/librte_ether/rte_ethdev.h > > +++ b/lib/librte_ether/rte_ethdev.h > > /** > > - * Register a callback function for specific port id. > > + * Register a callback function for port id event. > [...] > > - * Unregister a callback function for specific port id. > > + * Unregister a callback function for port id event. > > "port event" would be more appropriate than "port id event". While this change is relevant even before this patch, it is fine to add it here. Thanks.
