On 09/11/15 10:48, Savolainen, Petri (Nokia - FI/Espoo) wrote:
-----Original Message-----
From: EXT Maxim Uvarov [mailto:maxim.uva...@linaro.org]
Sent: Thursday, September 10, 2015 7:11 PM
To: Savolainen, Petri (Nokia - FI/Espoo); EXT Bill Fischofer
Cc: LNG ODP Mailman List
Subject: Re: [lng-odp] [API-NEXT PATCH] api: packet reference count
support
On 09/10/15 15:54, Savolainen, Petri (Nokia - FI/Espoo) wrote:
-----Original Message-----
From: EXT Maxim Uvarov [mailto:maxim.uva...@linaro.org]
Sent: Thursday, September 10, 2015 2:20 PM
To: Savolainen, Petri (Nokia - FI/Espoo); EXT Bill Fischofer
Cc: LNG ODP Mailman List
Subject: Re: [lng-odp] [API-NEXT PATCH] api: packet reference count
support
On 09/10/15 12:00, Savolainen, Petri (Nokia - FI/Espoo) wrote:
The problem to solve is: how to enable multiple references to a
packet
and more generally to a packet segment. It would be better to
create
new handles, instead of increment a 32bit count. Currently, the
ownership of a handle is lost during enq/free/… operations. That
semantic is worth maintaining also with multiple references.
odp_packet_t pkt = current_packet;
odp_packet_t my_ref;
// Create new reference to a packet
my_ref = odp_packet_create_ref(pkt);
if (odp_packet_ref_count(pkt) > 1)
// reference count is now 2
foo();
odp_packet_free(pkt);
// After free, user does not own handle ‘pkt’ anymore and cannot
it.
if (odp_packet_ref_count(my_ref) > 1)
// reference count is now 1
foo();
odp_packet_free(my_ref);
// Packet is now freed and user does not have any reference to it
anymore.
I think that uses case is not close to real life. If you do in one
thread:
pkt = <get packet or alloc>;
odp_packet_refcount_set(pkt, 2);
foo(pkt);
odp_packet_free(pkt); <- ref is 1
foo(pkt);
odp_packet_free(pkt); <- ref 0, free
Then you know what pkt is on each line. Instead of having 2
reference
as
in your example my_ref and pkt, you
use just one pkt.
The point of the example is clear ownership of a reference to the
packet.
These are *bugs* currently, and API should stay clear on ownership
issues also multiple references:
// double free a packet (handle or reference)
odp_packet_free(pkt);
odp_packet_free(pkt);
// double enqueue a packet (handle or reference)
ev = odp_packet_from_event(pkt);
odp_queue_enq(queue1, ev);
odp_queue_enq(queue2, ev);
With two different handles (references) it's clear how owns the
handle and implementation can either return the same or different value
as the handle.
my_ref = odp_packet_create_ref(pkt);
ev1 = odp_packet_from_event(pkt);
odp_queue_enq(queue1, ev1);
ev2 = odp_packet_from_event(my_ref);
odp_queue_enq(queue2, ev2);
In my case it's easy to check, inside odp_packet_free(pkt):
if (!odp_packet_refcount(pkt))
ODP_ABORT("illegal free");
That will capture bugs with double enqueue when you will try to send
that packets.
If your case we should maintain somewhere array of packet handles and
packet headers,
with unclear size depending how match reference we can have. And
handles
should not
overlap with current packet handles. That looks more complicated,
right?
Just like any other ODP API, this is a question of API definition vs. freedom
of implementation. API spec defines two handles (one per reference) and thus
user must act as if those two handles are unique, BUT the implementation can
choose if it reuses the handle value or not.
For example,
typedef odp_packet_t soc_packet_descriptor_t *;
odp_packet_t odp_packet_create_ref(odp_packet_t pkt)
{
soc_packet_descriptor_t *desc = (soc_packet_descriptor_t *) pkt;
desc->ref_count++;
return pkt;
}
that variant is clear, but pkt on input and output is the same - it's
pointer to desc somewhere.
But it's the same what I proposed.
OR
odp_packet_t odp_packet_create_ref(odp_packet_t pkt)
{
soc_packet_descriptor_t *desc = ((soc_packet_descriptor_t *) pkt) &
PTR_MASK;
desc->ref_count++;
return (desc |
THIS_IS_A_REFERENCE_TO_A_PACKET_NOT_THE_ORIGINAL_PACKET_DESC);
}
That variant modifies desc pointer. And there might be possible issues:
1. it might not portable due to unable to restore original pointer to
descr (if all bits of pointer address are used.).
2. I assume some hw schedulers can unable to work with such modified
pointers. And it will be hard to implement in hw.
3. On every function which works with packet you need clear
REFERENCE_BIT before accessing packet (access to *desc),
which is additional overhead.
Maxim.
Now in case of several threads:
pkt = <get packet or alloc>;
odp_packet_refcount_set(pkt, 2);
<enqueue it to scheduler>
threads code:
pkt = odp_schedule();
foo(pkt);
odp_packet_free(pkt);
In that case you don't need to take care about real pkt and it's
ref.
Just process packets returned by schedule() and resend them or free.
The difference is reference ownership, and usage of one vs. two
(potentially different) handles.
Thread 1: Thread2:
// send to thread 2
queue_enq(queue, pkt);
pkt = schedule();
// access reference
foo(pkt); bar(pkt);
Thread 1: Thread2:
pkt2 = create_ref(pkt1);
// send to thread 2
queue_enq(queue, pkt1);
pkt1 = schedule();
// access reference
foo(pkt2); bar(pkt1);
That usage is clear. Not clear how to maintain pkt2 table of handlers
in
that case. And how fast
look up in that table will be and what is overhead for that lookup.
No need to look up. No need to explicit reference counting by application. See
above.
-Petri
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
