On Tue, Feb 13, 2018 at 02:16:08PM +0000, Van Haaren, Harry wrote: > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Thomas Monjalon > > Sent: Tuesday, February 13, 2018 1:51 PM > > To: Burakov, Anatoly <anatoly.bura...@intel.com> > > Cc: firstname.lastname@example.org; Tan, Jianfeng <jianfeng....@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....@intel.com > > > > Signed-off-by: Anatoly Burakov <anatoly.bura...@intel.com> > > > > > > Reviewed-by: Jianfeng Tan <jianfeng....@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