[PATCHv2] linux-generic: check for event type while converting packet to event

Message ID 1436271433-24698-1-git-send-email-maxim.uvarov@linaro.org
State New
Headers show

Commit Message

Maxim Uvarov July 7, 2015, 12:17 p.m.
All examples of odp code do not check odp_schedule() output,
I.e.:
ev = odp_schedule()
pkt = odp_packet_from_event(ev)

Because of ev can be not only packet (timer, crypto operation), add
check that only packet event can be converted to packet.

Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
---
 platform/linux-generic/include/odp_packet_internal.h | 5 +++++
 platform/linux-generic/odp_crypto.c                  | 2 +-
 platform/linux-generic/odp_packet.c                  | 7 ++++++-
 3 files changed, 12 insertions(+), 2 deletions(-)

Comments

Bill Fischofer July 7, 2015, 10:25 p.m. | #1
On Tue, Jul 7, 2015 at 7:17 AM, Maxim Uvarov <maxim.uvarov@linaro.org>
wrote:

> All examples of odp code do not check odp_schedule() output,
> I.e.:
> ev = odp_schedule()
> pkt = odp_packet_from_event(ev)
>
> Because of ev can be not only packet (timer, crypto operation), add
> check that only packet event can be converted to packet.
>
> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
> ---
>  platform/linux-generic/include/odp_packet_internal.h | 5 +++++
>  platform/linux-generic/odp_crypto.c                  | 2 +-
>  platform/linux-generic/odp_packet.c                  | 7 ++++++-
>  3 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/platform/linux-generic/include/odp_packet_internal.h
> b/platform/linux-generic/include/odp_packet_internal.h
> index 8c41491..3d43b25 100644
> --- a/platform/linux-generic/include/odp_packet_internal.h
> +++ b/platform/linux-generic/include/odp_packet_internal.h
> @@ -258,6 +258,11 @@ static inline void
> _odp_packet_reset_parse(odp_packet_t pkt)
>         pkt_hdr->input_flags.all = ODP_PACKET_UNPARSED;
>  }
>
> +static inline odp_packet_t _odp_packet_from_event(odp_event_t ev)
> +{
> +       return (odp_packet_t)ev;
> +}
> +
>

Why duplicate odp_packet_from_event() like this?  The main API could be
inlined for performance if that was an issue, however since linux-generic
isn't a performance platform this seems unnecessary.


>  /* Forward declarations */
>  int _odp_packet_copy_to_packet(odp_packet_t srcpkt, uint32_t srcoffset,
>                                odp_packet_t dstpkt, uint32_t dstoffset,
> diff --git a/platform/linux-generic/odp_crypto.c
> b/platform/linux-generic/odp_crypto.c
> index d49e256..7a619e0 100644
> --- a/platform/linux-generic/odp_crypto.c
> +++ b/platform/linux-generic/odp_crypto.c
> @@ -40,7 +40,7 @@ static odp_crypto_global_t *global;
>  static
>  odp_crypto_generic_op_result_t *get_op_result_from_event(odp_event_t ev)
>  {
> -       return &(odp_packet_hdr(odp_packet_from_event(ev))->op_result);
> +       return &(odp_packet_hdr(_odp_packet_from_event(ev))->op_result);
>  }
>

Unnecessary

>
>  static
> diff --git a/platform/linux-generic/odp_packet.c
> b/platform/linux-generic/odp_packet.c
> index 668ddda..062ad7e 100644
> --- a/platform/linux-generic/odp_packet.c
> +++ b/platform/linux-generic/odp_packet.c
> @@ -77,7 +77,12 @@ odp_buffer_t _odp_packet_to_buffer(odp_packet_t pkt)
>
>  odp_packet_t odp_packet_from_event(odp_event_t ev)
>  {
> -       return (odp_packet_t)ev;
> +#ifdef ODP_DEBUG
> +       if (odp_unlikely(odp_event_type(ev) != ODP_EVENT_PACKET))
> +               ODP_ABORT("Dispatching not packet event %d\n",
> +                         odp_event_type(ev));
> +#endif
> +       return _odp_packet_from_event(ev);
>

Is this going to be used?  All of the converter functions expect correct
inputs with undefined results if this is not observed since these are
fastpath APIs.  In any event the error message here is cryptic.  A better
choice might be "Cannot convert: event %d is not a packet type" or similar.


>  }
>
>  odp_event_t odp_packet_to_event(odp_packet_t pkt)
> --
> 1.9.1
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>

Patch

diff --git a/platform/linux-generic/include/odp_packet_internal.h b/platform/linux-generic/include/odp_packet_internal.h
index 8c41491..3d43b25 100644
--- a/platform/linux-generic/include/odp_packet_internal.h
+++ b/platform/linux-generic/include/odp_packet_internal.h
@@ -258,6 +258,11 @@  static inline void _odp_packet_reset_parse(odp_packet_t pkt)
 	pkt_hdr->input_flags.all = ODP_PACKET_UNPARSED;
 }
 
+static inline odp_packet_t _odp_packet_from_event(odp_event_t ev)
+{
+	return (odp_packet_t)ev;
+}
+
 /* Forward declarations */
 int _odp_packet_copy_to_packet(odp_packet_t srcpkt, uint32_t srcoffset,
 			       odp_packet_t dstpkt, uint32_t dstoffset,
diff --git a/platform/linux-generic/odp_crypto.c b/platform/linux-generic/odp_crypto.c
index d49e256..7a619e0 100644
--- a/platform/linux-generic/odp_crypto.c
+++ b/platform/linux-generic/odp_crypto.c
@@ -40,7 +40,7 @@  static odp_crypto_global_t *global;
 static
 odp_crypto_generic_op_result_t *get_op_result_from_event(odp_event_t ev)
 {
-	return &(odp_packet_hdr(odp_packet_from_event(ev))->op_result);
+	return &(odp_packet_hdr(_odp_packet_from_event(ev))->op_result);
 }
 
 static
diff --git a/platform/linux-generic/odp_packet.c b/platform/linux-generic/odp_packet.c
index 668ddda..062ad7e 100644
--- a/platform/linux-generic/odp_packet.c
+++ b/platform/linux-generic/odp_packet.c
@@ -77,7 +77,12 @@  odp_buffer_t _odp_packet_to_buffer(odp_packet_t pkt)
 
 odp_packet_t odp_packet_from_event(odp_event_t ev)
 {
-	return (odp_packet_t)ev;
+#ifdef ODP_DEBUG
+	if (odp_unlikely(odp_event_type(ev) != ODP_EVENT_PACKET))
+		ODP_ABORT("Dispatching not packet event %d\n",
+			  odp_event_type(ev));
+#endif
+	return _odp_packet_from_event(ev);
 }
 
 odp_event_t odp_packet_to_event(odp_packet_t pkt)