Hi Konstantin,
On 03/27/2015 10:07 AM, Olivier MATZ wrote:
>> I think that to support ability to setup priv_size on a mempool basis,
>> and reserve private space between struct rte_mbuf and rte_mbuf. buf_addr,
>> we need to:
>>
>> 1. Store priv_size both inside the mempool and inside the mbuf.
>>
>> 2. rte_pktmbuf_attach() should change the value of priv_size to the
>> priv_size of the direct mbuf we are going to attach to:
>> rte_pktmbuf_attach(struct rte_mbuf *mi, struct rte_mbuf *md) {...
>> mi->priv_size = md->priv_size; ...}
>>
>> 3. rte_pktmbuf_detach() should restore original value of mbuf's priv_size:
>> rte_pktmbuf_detach(struct rte_mbuf *m)
>> {
>> ...
>> m->priv_size = rte_mempool_get_privsize(m->pool);
>> m->buf _addr= rte_mbuf_to_baddr(m);
>> ...
>> }
>>
>> Also I think we need to provide a way to specify priv_size for all mbufs of
>> the mepool at init time:
>> - either force people to specify it at rte_mempool_create() time (probably
>> use init_arg for that),
>> - or provide separate function that could be called straight after
>> rte_mempool_create() , that
>> would setup priv_size for the pool and for all its mbufs.
>> - or some sort of combination of these 2 approaches - introduce a wrapper
>> function
>> (rte_mbuf_pool_create() or something) that would take priv_size as
>> parameter,
>> create a new mempool and then setup priv_size.
I though a bit more about this solution, and I realized that doing
mi->priv_size = md->priv_size in rte_pktmbuf_attach() is probably not
a good idea, as there is no garantee that the size of mi is large enough
to store the priv of md.
Having the same priv_size for mi and md is maybe a good constraint.
I can add this in the API comments and assertions in the code to
check this condition, what do you think?
> Introducing rte_mbuf_pool_create() seems a good idea to me, it
> would hide 'rte_pktmbuf_pool_private' from the user and force
> to initialize all the required fields (mbuf_data_room_size only
> today, and maybe mbuf_priv_size).
>
> The API would be:
>
> struct rte_mempool *
> rte_mbuf_pool_create(const char *name, unsigned n, unsigned elt_size,
> unsigned cache_size, size_t mbuf_priv_size,
> rte_mempool_obj_ctor_t *obj_init, void *obj_init_arg,
> int socket_id, unsigned flags)
>
> I can give it a try and send a patch for this.
About this, it is not required anymore for this patch series if we
agree with my comment above.
I'll send a separate patch for that. It's probably a good occasion
to get rid of the pointer casted into an integer for
mbuf_data_room_size.
Regards,
Olivier
