[API-NEXT,PATCHv2,1/3] api: packet reference count support

Message ID 1441975993-4687-2-git-send-email-maxim.uvarov@linaro.org
State New
Headers show

Commit Message

Maxim Uvarov Sept. 11, 2015, 12:53 p.m.
Add api for packet reference count support. Which is useful in case:
 - multicast support
 - TCP retransmission
 - traffic generator

Introduced new call: newpkt = odp_packet_create_ref(pkt) which creates
reference to original packet, but handles for reference packet and original
are different.

Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
---
 include/odp/api/packet.h                             |  9 +++++++++
 platform/linux-generic/include/odp_buffer_internal.h |  2 ++
 platform/linux-generic/odp_packet.c                  | 20 ++++++++++++++++++++
 3 files changed, 31 insertions(+)

Comments

Bill Fischofer Sept. 11, 2015, 10:57 p.m. | #1
I think this needs more design work.  There's no restriction that a packet
can only have a single reference, and if multiple references are created
then the free occurs only when the last reference is freed and presumably
that could be in any order.  Implementations MAY impose a maximum number of
references, but if they do then we need some sort of odp_config.h
conventions surrounding that.

I think we should put this on the agenda for Monday's ARCH call and
possibly also bring it up in the Tuesday public call as it's an important
topic for future work.

On Fri, Sep 11, 2015 at 7:53 AM, Maxim Uvarov <maxim.uvarov@linaro.org>
wrote:

> Add api for packet reference count support. Which is useful in case:
>  - multicast support
>  - TCP retransmission
>  - traffic generator
>
> Introduced new call: newpkt = odp_packet_create_ref(pkt) which creates
> reference to original packet, but handles for reference packet and original
> are different.
>
> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
> ---
>  include/odp/api/packet.h                             |  9 +++++++++
>  platform/linux-generic/include/odp_buffer_internal.h |  2 ++
>  platform/linux-generic/odp_packet.c                  | 20
> ++++++++++++++++++++
>  3 files changed, 31 insertions(+)
>
> diff --git a/include/odp/api/packet.h b/include/odp/api/packet.h
> index 5d46b7b..f9745fb 100644
> --- a/include/odp/api/packet.h
> +++ b/include/odp/api/packet.h
> @@ -125,6 +125,15 @@ odp_packet_t odp_packet_from_event(odp_event_t ev);
>   */
>  odp_event_t odp_packet_to_event(odp_packet_t pkt);
>
> +/**
> + * Create reference for packet handle
> + *
> + * @param pkt  Packet handle
> + *
> + * @return New packet handle
> + */
> +odp_packet_t odp_packet_create_ref(odp_packet_t pkt);
> +
>  /*
>   *
>   * Pointers and lengths
> diff --git a/platform/linux-generic/include/odp_buffer_internal.h
> b/platform/linux-generic/include/odp_buffer_internal.h
> index 4cacca1..e079c6b 100644
> --- a/platform/linux-generic/include/odp_buffer_internal.h
> +++ b/platform/linux-generic/include/odp_buffer_internal.h
> @@ -79,6 +79,7 @@ typedef union odp_buffer_bits_t {
>                 uint32_t     u32;
>                 struct {
>  #if ODP_BYTE_ORDER == ODP_BIG_ENDIAN
> +                       uint32_t ref:1; /* pkt is reference for other
> packet */
>                         uint32_t pool_id:ODP_BUFFER_POOL_BITS;
>                         uint32_t index:ODP_BUFFER_INDEX_BITS;
>                         uint32_t seg:ODP_BUFFER_SEG_BITS;
> @@ -86,6 +87,7 @@ typedef union odp_buffer_bits_t {
>                         uint32_t seg:ODP_BUFFER_SEG_BITS;
>                         uint32_t index:ODP_BUFFER_INDEX_BITS;
>                         uint32_t pool_id:ODP_BUFFER_POOL_BITS;
> +                       uint32_t ref:1; /* pkt is reference for other
> packet */
>  #endif
>                 };
>
> diff --git a/platform/linux-generic/odp_packet.c
> b/platform/linux-generic/odp_packet.c
> index 209a6e6..2d5a618 100644
> --- a/platform/linux-generic/odp_packet.c
> +++ b/platform/linux-generic/odp_packet.c
> @@ -49,6 +49,13 @@ odp_packet_t odp_packet_alloc(odp_pool_t pool_hdl,
> uint32_t len)
>
>  void odp_packet_free(odp_packet_t pkt)
>  {
> +       odp_buffer_t buf = _odp_packet_to_buffer(pkt);
> +       odp_buffer_bits_t handle;
> +
> +       handle.handle = buf;
> +       if (handle.ref)
> +               return;
> +
>         odp_buffer_free((odp_buffer_t)pkt);
>  }
>
> @@ -85,6 +92,19 @@ odp_event_t odp_packet_to_event(odp_packet_t pkt)
>         return (odp_event_t)pkt;
>  }
>
> +odp_packet_t odp_packet_create_ref(odp_packet_t pkt)
> +{
> +       odp_buffer_t buf = _odp_packet_to_buffer(pkt);
> +       odp_buffer_bits_t handle;
> +
> +       handle.handle = buf;
> +       if (handle.ref)
> +               ODP_ABORT("packet is reference already\n");
> +       handle.ref = 1;
> +
> +       return _odp_packet_from_buffer(handle.handle);
> +}
> +
>  /*
>   *
>   * Pointers and lengths
> --
> 1.9.1
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
Maxim Uvarov Sept. 14, 2015, 9:01 a.m. | #2
On 09/12/15 01:57, Bill Fischofer wrote:
> I think this needs more design work.  There's no restriction that a 
> packet can only have a single reference, and if multiple references 
> are created then the free occurs only when the last reference is freed 
> and presumably that could be in any order.  Implementations MAY impose 
> a maximum number of references, but if they do then we need some sort 
> of odp_config.h conventions surrounding that.
>
> I think we should put this on the agenda for Monday's ARCH call and 
> possibly also bring it up in the Tuesday public call as it's an 
> important topic for future work.
>

Yes,  current implementation considers that preference packet used and 
freed before original packet. Will send v3 to fix that.

I used 1 bit for reference packet for linux-generic. But it can be 
several bits configured somewhere in config.h Will add that to v3 also.

Also I'm not sure if we need reference packet for reference packet. 
Probably we need it because of app developer should not know if he uses 
reference packet or original.

Maxim.

> On Fri, Sep 11, 2015 at 7:53 AM, Maxim Uvarov <maxim.uvarov@linaro.org 
> <mailto:maxim.uvarov@linaro.org>> wrote:
>
>     Add api for packet reference count support. Which is useful in case:
>      - multicast support
>      - TCP retransmission
>      - traffic generator
>
>     Introduced new call: newpkt = odp_packet_create_ref(pkt) which creates
>     reference to original packet, but handles for reference packet and
>     original
>     are different.
>
>     Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org
>     <mailto:maxim.uvarov@linaro.org>>
>     ---
>      include/odp/api/packet.h      |  9 +++++++++
>      platform/linux-generic/include/odp_buffer_internal.h | 2 ++
>      platform/linux-generic/odp_packet.c                  | 20
>     ++++++++++++++++++++
>      3 files changed, 31 insertions(+)
>
>     diff --git a/include/odp/api/packet.h b/include/odp/api/packet.h
>     index 5d46b7b..f9745fb 100644
>     --- a/include/odp/api/packet.h
>     +++ b/include/odp/api/packet.h
>     @@ -125,6 +125,15 @@ odp_packet_t
>     odp_packet_from_event(odp_event_t ev);
>       */
>      odp_event_t odp_packet_to_event(odp_packet_t pkt);
>
>     +/**
>     + * Create reference for packet handle
>     + *
>     + * @param pkt  Packet handle
>     + *
>     + * @return New packet handle
>     + */
>     +odp_packet_t odp_packet_create_ref(odp_packet_t pkt);
>     +
>      /*
>       *
>       * Pointers and lengths
>     diff --git a/platform/linux-generic/include/odp_buffer_internal.h
>     b/platform/linux-generic/include/odp_buffer_internal.h
>     index 4cacca1..e079c6b 100644
>     --- a/platform/linux-generic/include/odp_buffer_internal.h
>     +++ b/platform/linux-generic/include/odp_buffer_internal.h
>     @@ -79,6 +79,7 @@ typedef union odp_buffer_bits_t {
>                     uint32_t     u32;
>                     struct {
>      #if ODP_BYTE_ORDER == ODP_BIG_ENDIAN
>     +                       uint32_t ref:1; /* pkt is reference for
>     other packet */
>                             uint32_t pool_id:ODP_BUFFER_POOL_BITS;
>                             uint32_t index:ODP_BUFFER_INDEX_BITS;
>                             uint32_t seg:ODP_BUFFER_SEG_BITS;
>     @@ -86,6 +87,7 @@ typedef union odp_buffer_bits_t {
>                             uint32_t seg:ODP_BUFFER_SEG_BITS;
>                             uint32_t index:ODP_BUFFER_INDEX_BITS;
>                             uint32_t pool_id:ODP_BUFFER_POOL_BITS;
>     +                       uint32_t ref:1; /* pkt is reference for
>     other packet */
>      #endif
>                     };
>
>     diff --git a/platform/linux-generic/odp_packet.c
>     b/platform/linux-generic/odp_packet.c
>     index 209a6e6..2d5a618 100644
>     --- a/platform/linux-generic/odp_packet.c
>     +++ b/platform/linux-generic/odp_packet.c
>     @@ -49,6 +49,13 @@ odp_packet_t odp_packet_alloc(odp_pool_t
>     pool_hdl, uint32_t len)
>
>      void odp_packet_free(odp_packet_t pkt)
>      {
>     +       odp_buffer_t buf = _odp_packet_to_buffer(pkt);
>     +       odp_buffer_bits_t handle;
>     +
>     +       handle.handle = buf;
>     +       if (handle.ref)
>     +               return;
>     +
>             odp_buffer_free((odp_buffer_t)pkt);
>      }
>
>     @@ -85,6 +92,19 @@ odp_event_t odp_packet_to_event(odp_packet_t pkt)
>             return (odp_event_t)pkt;
>      }
>
>     +odp_packet_t odp_packet_create_ref(odp_packet_t pkt)
>     +{
>     +       odp_buffer_t buf = _odp_packet_to_buffer(pkt);
>     +       odp_buffer_bits_t handle;
>     +
>     +       handle.handle = buf;
>     +       if (handle.ref)
>     +               ODP_ABORT("packet is reference already\n");
>     +       handle.ref = 1;
>     +
>     +       return _odp_packet_from_buffer(handle.handle);
>     +}
>     +
>      /*
>       *
>       * Pointers and lengths
>     --
>     1.9.1
>
>     _______________________________________________
>     lng-odp mailing list
>     lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org>
>     https://lists.linaro.org/mailman/listinfo/lng-odp
>
>

Patch

diff --git a/include/odp/api/packet.h b/include/odp/api/packet.h
index 5d46b7b..f9745fb 100644
--- a/include/odp/api/packet.h
+++ b/include/odp/api/packet.h
@@ -125,6 +125,15 @@  odp_packet_t odp_packet_from_event(odp_event_t ev);
  */
 odp_event_t odp_packet_to_event(odp_packet_t pkt);
 
+/**
+ * Create reference for packet handle
+ *
+ * @param pkt  Packet handle
+ *
+ * @return New packet handle
+ */
+odp_packet_t odp_packet_create_ref(odp_packet_t pkt);
+
 /*
  *
  * Pointers and lengths
diff --git a/platform/linux-generic/include/odp_buffer_internal.h b/platform/linux-generic/include/odp_buffer_internal.h
index 4cacca1..e079c6b 100644
--- a/platform/linux-generic/include/odp_buffer_internal.h
+++ b/platform/linux-generic/include/odp_buffer_internal.h
@@ -79,6 +79,7 @@  typedef union odp_buffer_bits_t {
 		uint32_t     u32;
 		struct {
 #if ODP_BYTE_ORDER == ODP_BIG_ENDIAN
+			uint32_t ref:1; /* pkt is reference for other packet */
 			uint32_t pool_id:ODP_BUFFER_POOL_BITS;
 			uint32_t index:ODP_BUFFER_INDEX_BITS;
 			uint32_t seg:ODP_BUFFER_SEG_BITS;
@@ -86,6 +87,7 @@  typedef union odp_buffer_bits_t {
 			uint32_t seg:ODP_BUFFER_SEG_BITS;
 			uint32_t index:ODP_BUFFER_INDEX_BITS;
 			uint32_t pool_id:ODP_BUFFER_POOL_BITS;
+			uint32_t ref:1; /* pkt is reference for other packet */
 #endif
 		};
 
diff --git a/platform/linux-generic/odp_packet.c b/platform/linux-generic/odp_packet.c
index 209a6e6..2d5a618 100644
--- a/platform/linux-generic/odp_packet.c
+++ b/platform/linux-generic/odp_packet.c
@@ -49,6 +49,13 @@  odp_packet_t odp_packet_alloc(odp_pool_t pool_hdl, uint32_t len)
 
 void odp_packet_free(odp_packet_t pkt)
 {
+	odp_buffer_t buf = _odp_packet_to_buffer(pkt);
+	odp_buffer_bits_t handle;
+
+	handle.handle = buf;
+	if (handle.ref)
+		return;
+
 	odp_buffer_free((odp_buffer_t)pkt);
 }
 
@@ -85,6 +92,19 @@  odp_event_t odp_packet_to_event(odp_packet_t pkt)
 	return (odp_event_t)pkt;
 }
 
+odp_packet_t odp_packet_create_ref(odp_packet_t pkt)
+{
+	odp_buffer_t buf = _odp_packet_to_buffer(pkt);
+	odp_buffer_bits_t handle;
+
+	handle.handle = buf;
+	if (handle.ref)
+		ODP_ABORT("packet is reference already\n");
+	handle.ref = 1;
+
+	return _odp_packet_from_buffer(handle.handle);
+}
+
 /*
  *
  * Pointers and lengths