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.
On Thu, Nov 27, 2014 at 1:11 PM, Stuart Haslam <[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]> > 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]> > > > --- > > > (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] > > > http://lists.linaro.org/mailman/listinfo/lng-odp > > > > -- > Stuart. > > > _______________________________________________ > 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
