Re: [PATCH v18 2/8] eal: add thread attributes

[Tyler Retzlaff <roretzla@linux.microsoft.com>]

On Fri, Feb 04, 2022 at 07:21:10PM +0000, Ananyev, Konstantin wrote:
> > Implement thread attributes for:
> > * thread affinity
> > * thread priority
> > Implement functions for managing thread attributes.
> > 
> > Priority is represented through an enum that allows for two levels:
> > 	- RTE_THREAD_PRIORITY_NORMAL
> > 	- RTE_THREAD_PRIORITY_REALTIME_CRITICAL
> > 
> > Affinity is described by the rte_cpuset_t type.
> > 
> > An rte_thread_attr_t object can be set to the default values
> > by calling rte_thread_attr_init().
> > 
> > Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
> > ---
> >  lib/eal/common/rte_thread.c  | 46 ++++++++++++++++++
> >  lib/eal/include/rte_thread.h | 91 ++++++++++++++++++++++++++++++++++++
> >  lib/eal/version.map          |  4 ++
> >  lib/eal/windows/rte_thread.c | 44 +++++++++++++++++
> >  4 files changed, 185 insertions(+)
> > 
> > diff --git a/lib/eal/common/rte_thread.c b/lib/eal/common/rte_thread.c
> > index 92a7451b0a..27ad1c7eb0 100644
> > --- a/lib/eal/common/rte_thread.c
> > +++ b/lib/eal/common/rte_thread.c
> > @@ -9,6 +9,7 @@
> >  #include <string.h>
> > 
> >  #include <rte_common.h>
> > +#include <rte_debug.h>
> >  #include <rte_errno.h>
> >  #include <rte_log.h>
> >  #include <rte_thread.h>
> > @@ -33,6 +34,51 @@ rte_thread_equal(rte_thread_t t1, rte_thread_t t2)
> >  	return pthread_equal((pthread_t)t1.opaque_id, (pthread_t)t2.opaque_id);
> >  }
> > 
> > +int
> > +rte_thread_attr_init(rte_thread_attr_t *attr)
> > +{
> > +	RTE_VERIFY(attr != NULL);
> 
> As a generic one, here and everywhere:
> Please don't use RTE_VERIFY() for checking input function parameters.
> We don't want to panic in case of just invalid parameter from user.

i ask this question again. what useful thing will the user application
do when handling -EINVAL or rte_errno = EINVAL is returned for
incorrectly supplied parameters?

again, there should be a mechanism that allows a policy for how these
non-recoverable errors are handled rather than defaulting to tossing 
it over the fence and expecting the application to do something 
sensible when the only thing it could do is conclusively more
complicated than having passed the correct parameters in the first place.

more often then not application programmers will ignore superfluous
return values from functions like this resulting in the bug remaining
longer and the state / reason being lost.

please reconsider.