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.k...@linaro.org>
Thank you for your review. I'll send a v2 today.
Regards,
Olivier
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
