On 09/04/15 16:11, Bill Fischofer wrote:
I'm not sure what the benefit of the proposed hack would be, since
you're returning an opaque value.
Still, your app can print that number into the log, so if the developer
looks up the particular platform implementation, he can get an idea what
the problem might be. Or not, if the platform implementation decides to
just return a static value (e.g. 1). Still the chance would be given.
Of course if we decide later on to return standard error values here,
that door would be still open.
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]
<mailto:[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]>
<mailto:[email protected]
<mailto:[email protected]>__>> wrote:
On 8 April 2015 at 13:10, Bill Fischofer
<[email protected] <mailto:[email protected]>
<mailto:bill.fischofer@linaro.__org
<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]>
<mailto:[email protected]
<mailto:[email protected]>__>> wrote:
On 8 April 2015 at 12:45, Zoltan Kiss
<[email protected]
<mailto:[email protected]> <mailto:[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]>
<mailto:[email protected].__org
<mailto:[email protected]>>
https://lists.linaro.org/____mailman/listinfo/lng-odp
<https://lists.linaro.org/__mailman/listinfo/lng-odp>
<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]>
<mailto:[email protected].__org
<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]
https://lists.linaro.org/mailman/listinfo/lng-odp
