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

Matan Azrad matan at mellanox.com
Sat Dec 30 20:04:28 CET 2017


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.


More information about the dev mailing list