On Thu, Feb 9, 2017 at 9:32 AM, Savolainen, Petri (Nokia - FI/Espoo) <petri.savolai...@nokia-bell-labs.com> wrote: > Hi, > > I still think that packet ref API should be defined so that an implementation > may fall back to doing packet copy instead of reference to shared data (if it > simply cannot handle packets by reference). Now after implementing packet ref > API as copy (in about 90 lines), only issues are with tests like this one ... > > ref = odp_packet_ref(pkt, 100); > odp_packet_free(ref); > > /* Verify that pkt no longer has a ref */ > CU_ASSERT(odp_packet_has_ref(pkt) == 0); > > ... since free(ref) would need to update also 'pkt'.
There is no need for any special action here since if the implementation did a copy under the covers then the packet would not have any references. In fact, it would be a useful feature that odp_packet_has_ref(pkt) == 0 after a odp_packet_ref() call would be a positive confirmation that the "reference" was finessed with a copy under the covers. Note that in this case odp_packet_unshared_len(pkt) would be similarly unchanged. A true reference means exactly that: the packet is sharing one or more segments with some other handle. Trying to lie about that if a copy has taken place is not only complicated but also potentially dangerously misleading. The problem, of course, is that the utility of references on such implementations is questionable since references are intended to be a high-performance zero-copy alternative to the application making packet copies itself. But if that's the best a given implementation can do then that's is sufficient to claim API conformance since the ODP spec does not make specific performance guarantees. It's the reason, for example, that odp-linux was able to get away with not having real support for ordered queues by substituting atomic queues initially. > > I think we should lesser the synchronization guarantees of > odp_packet_has_ref(), so that it returns if a packet "may have" still have > multiple references. And same thing for odp_packet_unshared_len() (may have > shared bytes). If application needs Disagree. Just be honest. "Maybe" predicates are useless. to be sure, it would need to be assured some other way (e.g. calculating itself how many references was created / freed). Most application would not be interested on the book keeping, but on the possibility of creating and using references. Again, no need to do anything special since with references every reference must be freed to avoid memory leaks anyway and this is true regardless of whether copies are being done or true segment sharing is going on. > > -Petri > > > > > > >
