Re: [dpdk-dev] [PATCH v2] ethdev: allow all ports event registration

[Matan Azrad <matan@mellanox.com>]

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.