Hi Olivier,
On 25/03/15 17:04, Olivier MATZ wrote:
Hi Zoltan,
On 03/24/2015 06:42 PM, Zoltan Kiss wrote:
Hi,
I've noticed in lib/netdev-dpdk.c that __rte_pktmbuf_init() stores the
packet metadata right after "struct rte_mbuf", and before the buffer
data:
/* start of buffer is just after mbuf structure */
m->buf_addr = (char *)m + sizeof(struct dp_packet);
(struct dp_packet has the rte_mbuf as first member if DPDK enabled)
However, lib/librte_mbuf/rte_mbuf.h seems to codify that the buffer
should start right after the rte_mbuf:
/**
* Given the buf_addr returns the pointer to corresponding mbuf.
*/
#define RTE_MBUF_FROM_BADDR(ba) (((struct rte_mbuf *)(ba)) - 1)
/**
* Given the pointer to mbuf returns an address where it's buf_addr
* should point to.
*/
#define RTE_MBUF_TO_BADDR(mb) (((struct rte_mbuf *)(mb)) + 1)
These macros are used for attaching/detaching mbuf's to each other. This
is the way the code retrieves the direct buffer from an indirect one,
and vica versa. I think if we want to keep the metadata feature (which I
guess is quite important), we need to add a pointer to rte_mbuf, which
helps the direct and indirect structs to find each other. Something like:
struct rte_mbuf *attach; /**< Points to the other buffer if this
one
is (in)direct. Otherwise NULL. */
What do you think?
I've just sent a patch that should fix this issue.
http://dpdk.org/ml/archives/dev/2015-March/015722.html
Let me know if you have any comment on it.
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"
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 17ba791..4ced6d3 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -268,7 +268,7 @@ struct rte_mbuf {
uint16_t data_len; /**< Amount of data in segment buffer. */
uint32_t pkt_len; /**< Total pkt len: sum of all segments. */
uint16_t vlan_tci; /**< VLAN Tag Control Identifier (CPU order)
*/
- uint16_t reserved;
+ uint16_t priv_size; /**< size of the application private data */
union {
uint32_t rss; /**< RSS hash result if RSS enabled */
struct {
@@ -320,15 +320,38 @@ struct rte_mbuf {
} __rte_cache_aligned;
/**
- * 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.
@@ -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.
- m->buf_physaddr = rte_mempool_virt2phy(mp, m) + sizeof (*m);
+
+ m->buf_physaddr = rte_mempool_virt2phy(mp, m) + sizeof (*m) +
+ m->priv_size;
m->buf_addr = buf;
m->buf_len = (uint16_t)buf_len;
The rest of the series looks good,
Reviewed-by: Zoltan Kiss <zoltan.k...@linaro.org>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
