On 11/28/2014 07:28 AM, Bill Fischofer wrote:
The real issue here is expected frequency. With the exception of
read/write operations on polled queues, most pktio operations are
low-frequency initialization-time calls. As such, doing additional
validation in these routines carries no significant performance penalty.
so it's ok for sign-off, right?
Maxim.
On Thu, Nov 27, 2014 at 1:11 PM, Stuart Haslam <[email protected]
<mailto:[email protected]>> wrote:
On Thu, Nov 27, 2014 at 05:09:06PM +0000, Ola Liljedahl wrote:
> On 27 November 2014 at 17:56, Stuart Haslam
<[email protected] <mailto:[email protected]>> wrote:
> > Attempts to enq to a pktin queue or deq from a pktout queue are
> > programming errors, so abort.
> Are there more cases of such programming errors where we should
> specify the API semantics to be undefined instead of having to
always
> detect the error and return some error code? Undefined semantics
gives
> us the possibility to optimize for performance and not even try to
> detect such errors. Default for any ODP implementation should
still be
> to attempt to detect programming errors and to abort the program
> execution (e.g. call ODP_ABORT). The user will get a core dump they
> can debug and find the source of their programming errors.
>
> -- Ola
>
Pretty much every API gives the caller an opportunity to screw up by
passing invalid handles etc. The semantics of parameter validation
aren't defined per API but the architecture documents say this;
(from api_guide_lines.dox)
--
validations that can be performed statically at compile time or at
little to no runtime cost SHOULD be considered, APIs MAY choose to
leave behavior as undefined when presented with invalid
parameters in
the interest of runtime efficiency
--
Which makes sense.
In terms of what the linux-generic implementation actually does, the
majority of the APIs either do no validation or use ODP_ASSERT(). The
pktio APIs seem to be the odd-ones-out as most validate parameters and
return -1 on error without printing anything, so I think these
should be
changed to ODP_ASSERT()s.
--
Stuart.
> >
> > Signed-off-by: Stuart Haslam <[email protected]
<mailto:[email protected]>>
> > ---
> > (This code contribution is provided under the terms of
agreement LES-LTM-21309)
> >
> > platform/linux-generic/odp_packet_io.c | 22
++++++++++++++++++----
> > 1 file changed, 18 insertions(+), 4 deletions(-)
> >
> > diff --git a/platform/linux-generic/odp_packet_io.c
b/platform/linux-generic/odp_packet_io.c
> > index f527e45..161f2ce 100644
> > --- a/platform/linux-generic/odp_packet_io.c
> > +++ b/platform/linux-generic/odp_packet_io.c
> > @@ -384,6 +384,9 @@ int pktout_enqueue(queue_entry_t *qentry,
odp_buffer_hdr_t *buf_hdr)
> > odp_buffer_hdr_t *pktout_dequeue(queue_entry_t *qentry)
> > {
> > (void)qentry;
> > +
> > + ODP_ABORT("attempted dequeue from a pktout queue");
> > +
> > return NULL;
> > }
> >
> > @@ -408,13 +411,19 @@ int pktout_deq_multi(queue_entry_t
*qentry, odp_buffer_hdr_t *buf_hdr[],
> > (void)buf_hdr;
> > (void)num;
> >
> > + ODP_ABORT("attempted dequeue from a pktout queue");
> > +
> > return 0;
> > }
> >
> > int pktin_enqueue(queue_entry_t *qentry, odp_buffer_hdr_t
*buf_hdr)
> > {
> > - /* Use default action */
> > - return queue_enq(qentry, buf_hdr);
> > + (void)qentry;
> > + (void)buf_hdr;
> > +
> > + ODP_ABORT("attempted enqueue to a pktin queue");
> > +
> > + return -1;
> > }
> >
> > odp_buffer_hdr_t *pktin_dequeue(queue_entry_t *qentry)
> > @@ -445,8 +454,13 @@ odp_buffer_hdr_t
*pktin_dequeue(queue_entry_t *qentry)
> >
> > int pktin_enq_multi(queue_entry_t *qentry, odp_buffer_hdr_t
*buf_hdr[], int num)
> > {
> > - /* Use default action */
> > - return queue_enq_multi(qentry, buf_hdr, num);
> > + (void)qentry;
> > + (void)buf_hdr;
> > + (void)num;
> > +
> > + ODP_ABORT("attempted enqueue to a pktin queue");
> > +
> > + return 0;
> > }
> >
> > int pktin_deq_multi(queue_entry_t *qentry, odp_buffer_hdr_t
*buf_hdr[], int num)
> > --
> > 2.1.1
> >
> >
> >
> > _______________________________________________
> > lng-odp mailing list
> > [email protected] <mailto:[email protected]>
> > http://lists.linaro.org/mailman/listinfo/lng-odp
>
--
Stuart.
_______________________________________________
lng-odp mailing list
[email protected] <mailto:[email protected]>
http://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp
