Re: [PATCH v8 01/11] ethdev: introduce flow engine configuration

[Thomas Monjalon <thomas@monjalon.net>]

21/02/2022 15:53, Andrew Rybchenko:
> On 2/21/22 15:53, Ori Kam wrote:
> > From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >>>> +/**
> >>>> + * @warning
> >>>> + * @b EXPERIMENTAL: this API may change without prior notice.
> >>>> + *
> >>>> + * Get information about flow engine resources.
> >>>> + *
> >>>> + * @param port_id
> >>>> + *   Port identifier of Ethernet device.
> >>>> + * @param[out] port_info
> >>>> + *   A pointer to a structure of type *rte_flow_port_info*
> >>>> + *   to be filled with the resources information of the port.
> >>>> + * @param[out] error
> >>>> + *   Perform verbose error reporting if not NULL.
> >>>> + *   PMDs initialize this structure in case of error only.
> >>>> + *
> >>>> + * @return
> >>>> + *   0 on success, a negative errno value otherwise and rte_errno is
> >>>> set.
> >>>
> >>> If I'm not mistakes we should be explicit with
> >>> negative result values menting
> >>>
> > I'm not sure, until now we didn't have any errors values defined in RTE flow.
> > I don't want to enforce PMD with the error types.
> > If PMD can say that it can give better error code or add a case that may result in
> > error, I don't want to change the API.
> > So I think we better leave the error codes out of documentation unless they are final and can only
> > be resulted from the rte_level.
> 
> It is not helpful for application. If so, application don't
> know how to interpret and handle various error codes.

Yes rte_flow error codes are not listed
(except for rte_flow_validate and indirect action).
As a consequence, the error code is mainly for debug purposes.

I am OK with being consistent and not listing error codes
in these new functions for now.
For consistency, I suggest removing error codes
from rte_flow_async_action_handle_* in this patchset.

We should have a general discussion about error codes handling later.
It may be a design decision to allow flexibility to PMDs.
If we want to provide some detailed error handling to applications,
we could list main or all kind of errors.

Anyway, this inconsistency is not new, so it should not block the patches IMHO.