Re: [dpdk-dev] [PATCH] eal: fix rte_errno values for IPC API

[Bruce Richardson <bruce.richardson@intel.com>]

On Tue, Feb 13, 2018 at 02:16:08PM +0000, Van Haaren, Harry wrote:
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> > Sent: Tuesday, February 13, 2018 1:51 PM
> > To: Burakov, Anatoly <anatoly.burakov@intel.com>
> > Cc: dev@dpdk.org; Tan, Jianfeng <jianfeng.tan@intel.com>
> > Subject: Re: [dpdk-dev] [PATCH] eal: fix rte_errno values for IPC API
> > 
> > > > rte_errno values should not be negative.
> > > >
> > > > Fixes: bacaa2754017 ("eal: add channel for multi-process communication")
> > > > Fixes: 783b6e54971d ("eal: add synchronous multi-process communication")
> > > > Cc: jianfeng.tan@intel.com
> > > > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > >
> > > Reviewed-by: Jianfeng Tan <jianfeng.tan@intel.com>
> > >
> > > Thanks for fixing this.
> > 
> > Applied, thanks
> > 
> > There are a lot of similar issues:
> > 
> > git grep -l 'rte_errno = -E' | sed 's,[^/]*$,,' | sort -u
> > 
> > 	drivers/event/opdl/
> > 	drivers/event/sw/
> <snip>
> > 	lib/librte_eventdev/
> 
> 
> I just checked the eventdev.h port_link() docs, which indicate negative return values.
> Perhaps the header is wrong too - but the PMDs adhere to the library header in this case.
> 
> Is there a requirement for rte_errno to be positive?
> It looks to be declared as per-lcore signed int in rte_errno.h +20
>
I think I wrote that part of the documentation, and it never crossed my
mind that people would set rte_errno to negative values, given how
errno from system calls are always positive. However, I think this
omission should be rectified, and we should enforce having rte_errno
values as positive.

> Either-way, if we want to change the PMDs, we should change the Eventdev APIs,
> which means API breakage, and application changes to handle changed return values.
> 
> Sound like more work than it is worth it to me?

I would view it as restoring sanity (or balance to the force if you
prefer! :-) ), so I'd definitely be ok with an ABI break to do that.

/Bruce