I'm not sure what the benefit of the proposed hack would be, since you're returning an opaque value. The error flags are intentionally vague at this point because we haven't defined any standard error categories. As we do, the summary will be augmented with specific access functions for the defined categories. All part of the API evolution.
On Thu, Apr 9, 2015 at 10:01 AM, Zoltan Kiss <[email protected]> wrote: > We can say then: > > * @retval non-zero packet has errors, implementation can optionally return > a non-zero error code, STRICTLY FOR DEBUGGING PURPOSES > > I think that would clarify that you shouldn't use this non-zero value to > anything else than saving it into your logs. But any ideas to make it more > clear are welcome. > I came accross this when I found that all my DPDK packets are dropped due > to an error. Currently I have this hack in place to get the error value in > if there is anything wrong: > > diff --git a/platform/linux-generic/odp_packet_flags.c > b/platform/linux-generic/odp_packet_flags.c > index ab3d12f..439255a 100644 > --- a/platform/linux-generic/odp_packet_flags.c > +++ b/platform/linux-generic/odp_packet_flags.c > @@ -10,7 +10,7 @@ > > int odp_packet_has_error(odp_packet_t pkt) > { > - return (odp_packet_hdr(pkt)->error_flags.all != 0); > + return (odp_packet_hdr(pkt)->error_flags.all); > } > > /* Get Input Flags */ > > I think it would be useful for an app developer to have the chance to see > the underlying error code. Otherwise hacks like mine above would born. > > Zoli > > On 08/04/15 18:18, Bill Fischofer wrote: > >> The only problem with documenting the return as !0 is that that syntax >> is ambiguous. The C expression !0 evaluates to a specific value, but >> we're saying that any non-zero value is regarded as true, which is why >> it is documented as such. >> >> On Wed, Apr 8, 2015 at 12:12 PM, Mike Holmes <[email protected] >> <mailto:[email protected]>> wrote: >> >> >> >> On 8 April 2015 at 13:10, Bill Fischofer <[email protected] >> <mailto:[email protected]>> wrote: >> >> These are booleans. However we decided that ODP considers any >> non-zero value to be "true", hence the doc change. The >> linux-generic implementation always returns 1 for true and since >> 1 is a non-zero value that's completely conforming. >> >> >> Agree - it is the optional addition info aspect of !0 I thought >> might be an issue, if two states are returned I have no qualms about >> portability >> >> >> On Wed, Apr 8, 2015 at 12:07 PM, Mike Holmes >> <[email protected] <mailto:[email protected]>> wrote: >> >> >> >> On 8 April 2015 at 12:45, Zoltan Kiss >> <[email protected] <mailto:[email protected]>> >> wrote: >> >> Hi, >> >> Now it looks like: >> >> * @retval 1 packet has errors >> * @retval 0 packet has no errors >> >> I found it is better for debugging if it actually >> returns some error code. How about changing it to: >> >> * @retval 0 packet has no errors >> * @retval !0 packet has errors, implementation can >> optionally return an error code >> >> >> This looks like it was intended to be a boolean test so >> extra info might be misleading, and to be a standard people >> can rely on optional is never a good thing. >> >> Is there a minimum set of errors this api can be said will >> return an error code for ? If there is no standard set at >> all I question having anything optional becasue the api will >> have no portability at all. >> >> Taken from the hdr in question it looks like it would be >> this list >> >> struct { >> /* Bitfield flags for each detected error */ >> uint32_t app_error:1; /**< Error bit for application use */ >> uint32_t frame_len:1; /**< Frame length error */ >> uint32_t snap_len:1; /**< Snap length error */ >> uint32_t l2_chksum:1; /**< L2 checksum error, checks TBD */ >> uint32_t ip_err:1; /**< IP error, checks TBD */ >> uint32_t tcp_err:1; /**< TCP error, checks TBD */ >> uint32_t udp_err:1; /**< UDP error, checks TBD */ >> }; >> } error_flags_t; >> >> >> >> Zoli >> _________________________________________________ >> lng-odp mailing list >> [email protected] <mailto:[email protected] >> > >> https://lists.linaro.org/__mailman/listinfo/lng-odp >> <https://lists.linaro.org/mailman/listinfo/lng-odp> >> >> >> >> >> -- >> Mike Holmes >> Technical Manager - Linaro Networking Group >> Linaro.org <http://www.linaro.org/>***│ *Open source >> software for ARM SoCs >> >> __ >> >> >> >> _______________________________________________ >> lng-odp mailing list >> [email protected] <mailto:[email protected]> >> https://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> >> >> >> -- >> Mike Holmes >> Technical Manager - Linaro Networking Group >> Linaro.org <http://www.linaro.org/>***│ *Open source software for >> ARM SoCs >> >> __ >> >> >> >>
_______________________________________________ lng-odp mailing list [email protected] https://lists.linaro.org/mailman/listinfo/lng-odp
