Hi Zoltan,
On 03/25/2015 07:57 PM, Zoltan Kiss wrote:
> I have some comments for the first patch:
>
>> diff --git a/examples/vhost/main.c b/examples/vhost/main.c
>> index c3fcb80..050f3ac 100644
>> --- a/examples/vhost/main.c
>> +++ b/examples/vhost/main.c
> I've sent in a separate patch for this file, I think it's just easier to
> ditch the old copy-pasted code, see "[PATCH] examples/vhost: use library
> routines instead of local copies"
As Konstantin stated in the other thread, this modification is
wrong, sorry. I don't see why I changed this code instead of keeping
the initial logic. I'll fix that in the v2.
>> /**
>> - * Given the buf_addr returns the pointer to corresponding mbuf.
>> + * Return the mbuf owning the given data buffer address.
>> + *
>> + * @param mi
>> + * The pointer to the indirect mbuf.
>> + * @param buffer_addr
>> + * The address of the data buffer of the direct mbuf.
> You don't need this parameter, it's mi->buf_addr.
That's correct. In this case, I propose to change the name of the
function in:
struct rte_mbuf *rte_mbuf_from_indirect(struct rte_mbuf *mi)
>> @@ -744,9 +767,11 @@ static inline void rte_pktmbuf_attach(struct
>> rte_mbuf *mi, struct rte_mbuf *md)
>> static inline void rte_pktmbuf_detach(struct rte_mbuf *m)
>> {
>> const struct rte_mempool *mp = m->pool;
>> - void *buf = RTE_MBUF_TO_BADDR(m);
>> + void *buf = rte_mbuf_to_baddr(m);
>> uint32_t buf_len = mp->elt_size - sizeof(*m);
> I don't see any reason to keep buf and buf_len, just assign straight to
> m->buf_addr and *len.
> Besides that, you need to deduct m->priv_size from buf_len.
Agree. I suggest something like:
static inline void rte_pktmbuf_detach(struct rte_mbuf *m)
{
const struct rte_mempool *mp = m->pool;
void *buf = rte_mbuf_to_baddr(m);
unsigned mhdr_size = (char *)buf - (char *)m;
m->buf_addr = buf;
m->buf_physaddr = rte_mempool_virt2phy(mp, m) + mhdr_size;
m->buf_len = (uint16_t)(mp->elt_size - mhdr_size);
...
> The rest of the series looks good,
>
> Reviewed-by: Zoltan Kiss <zoltan.kiss at linaro.org>
Thank you for your review. I'll send a v2 today.
Regards,
Olivier
