Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references

[Olivier MATZ <olivier.matz@6wind.com>]

Hi Bruce,

On 02/18/2015 10:35 AM, Bruce Richardson wrote:
> On Wed, Feb 18, 2015 at 10:16:56AM +0100, Olivier MATZ wrote:
>> Hi Sergio,
>>
>> On 02/16/2015 05:08 PM, Sergio Gonzalez Monroy wrote:
>>> This patch removes all references to RTE_MBUF_REFCNT, setting the refcnt
>>> field in the mbuf struct permanently.
>>>
>>> Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
>>
>> I think removing the refcount compile option goes in the right
>> direction. However, activating the refcount will break the applications
>> that reserve a private zone in mbufs. This is due to the macros
>> RTE_MBUF_TO_BADDR() and RTE_MBUF_FROM_BADDR() that suppose that
>> the beginning of the mbuf is 128 bytes (sizeof mbuf) before the
>> data buffer.
>>
>
> While I understand how the macros make certain assumptions, how does activating
> the refcnt specifically lead to the problems you describe? Could you explain
> that part in a bit more detail?

Indeed, you are right, removing the refcount option does not break
the applications if they do not use it. So there is probably no need
to fix that problem in this patch series.

However we should consider this problem as the refcount (which can
not be deactivated now) is not compatible with the private mbuf data.

By the way, once we are on this, shouldn't we consider removing the
RTE_MBUF_REFCNT_ATOMIC compile option?


Regards,
Olivier



>
> Thanks,
> /Bruce
>
>> For RTE_MBUF_TO_BADDR(), it's relatively easy to replace it. The
>> mbuf pool could store the size of the private size like it's done
>> for mbp_priv->mbuf_data_room_size. Using rte_mempool_from_obj(m)
>> or m->pool, we can retrieve the mbuf pool and this value, then
>> compute the buffer address.
>>
>> For RTE_MBUF_FROM_BADDR(), it's more complex. We could ensure that
>> a backpointer to the mbuf is always located before the data buffer,
>> but it looks difficult to do.
>>
>> Another idea would be to add a field in indirect mbufs that stores
>> the pointer to the "parent" mbuf.
>>
>> Regards,
>> Olivier
>>