From: Yongseok Koh <yskoh@mellanox.com>
To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
Cc: "Lu, Wenzhuo" <wenzhuo.lu@intel.com>,
"Wu, Jingjing" <jingjing.wu@intel.com>,
"olivier.matz@6wind.com" <olivier.matz@6wind.com>,
"dev@dpdk.org" <dev@dpdk.org>,
"adrien.mazarguil@6wind.com" <adrien.mazarguil@6wind.com>,
"nelio.laranjeiro@6wind.com" <nelio.laranjeiro@6wind.com>
Subject: Re: [dpdk-dev] [PATCH v3 1/2] mbuf: support attaching external buffer to mbuf
Date: Mon, 23 Apr 2018 19:04:27 -0700 [thread overview]
Message-ID: <20180424020427.GA83470@yongseok-MBP.local> (raw)
In-Reply-To: <2601191342CEEE43887BDE71AB977258AE91994F@IRSMSX102.ger.corp.intel.com>
On Mon, Apr 23, 2018 at 11:53:04AM +0000, Ananyev, Konstantin wrote:
[...]
> > @@ -693,9 +711,14 @@ rte_mbuf_to_baddr(struct rte_mbuf *md)
> > #define RTE_MBUF_INDIRECT(mb) ((mb)->ol_flags & IND_ATTACHED_MBUF)
> >
> > /**
> > + * Returns TRUE if given mbuf has external buffer, or FALSE otherwise.
> > + */
> > +#define RTE_MBUF_HAS_EXTBUF(mb) ((mb)->ol_flags & EXT_ATTACHED_MBUF)
> > +
> > +/**
> > * Returns TRUE if given mbuf is direct, or FALSE otherwise.
> > */
> > -#define RTE_MBUF_DIRECT(mb) (!RTE_MBUF_INDIRECT(mb))
> > +#define RTE_MBUF_DIRECT(mb) (!RTE_MBUF_INDIRECT(mb) && !RTE_MBUF_HAS_EXTBUF(mb))
>
> As a nit:
> RTE_MBUF_DIRECT(mb) (((mb)->ol_flags & (IND_ATTACHED_MBUF | EXT_ATTACHED_MBUF)) == 0)
It was for better readability and I expected compiler did the same.
But, if you still want this way, I can change it.
[...]
> > +static inline char * __rte_experimental
> > +rte_pktmbuf_attach_extbuf(struct rte_mbuf *m, void *buf_addr,
> > + uint16_t buf_len, rte_mbuf_extbuf_free_callback_t free_cb,
> > + void *fcb_opaque)
> > +{
> > + void *buf_end = RTE_PTR_ADD(buf_addr, buf_len);
> > + struct rte_mbuf_ext_shared_info *shinfo;
> > +
> > + shinfo = RTE_PTR_ALIGN_FLOOR(RTE_PTR_SUB(buf_end, sizeof(*shinfo)),
> > + sizeof(uintptr_t));
> > +
> > + if ((void *)shinfo <= buf_addr)
> > + return NULL;
> > +
> > + m->buf_addr = buf_addr;
> > + m->buf_iova = rte_mempool_virt2iova(buf_addr);
>
>
> That wouldn't work for arbitrary extern buffer.
> Only for the one that is an element in some other mempool.
> For arbitrary external buffer - callee has to provide PA for it plus guarantee that
> it's VA would be locked down.
> From other side - if your intention is just to use only elements of other mempools -
> No need to have free_cb(). mempool_put should do.
Of course, I didn't mean that. That was a mistake. Please refer to my reply to
Olivier.
[...]
> > /**
> > - * Detach an indirect packet mbuf.
> > + * @internal used by rte_pktmbuf_detach().
> > + *
> > + * Decrement the reference counter of the external buffer. When the
> > + * reference counter becomes 0, the buffer is freed by pre-registered
> > + * callback.
> > + */
> > +static inline void
> > +__rte_pktmbuf_free_extbuf(struct rte_mbuf *m)
> > +{
> > + struct rte_mbuf_ext_shared_info *shinfo;
> > +
> > + RTE_ASSERT(RTE_MBUF_HAS_EXTBUF(m));
> > +
> > + shinfo = rte_mbuf_ext_shinfo(m);
> > +
> > + if (rte_extbuf_refcnt_update(shinfo, -1) == 0)
> > + shinfo->free_cb(m->buf_addr, shinfo->fcb_opaque);
>
>
> I understand the reason but extra function call for each external mbuf - seems quite expensive.
> Wonder is it possible to group them somehow and amortize the cost?
Good point. I thought about it today.
Comparing to the regular mbuf, maybe three differences. a) free function isn't
inlined but a real branch. b) no help from core local cache like mempool's c) no
free_bulk func like rte_mempool_put_bulk(). But these look quite costly and
complicated for the external buffer attachment.
For example, to free it in bulk, external buffers should be grouped as the
buffers would have different callback functions. To do that, I have to make an
API to pre-register an external buffer group to prepare resources for the bulk
free. Then, buffers can't be anonymous anymore but have to be registered in
advance. If so, it would be better to use existing APIs, especially when a user
wants high throughput...
Let me know if you have better idea to implement it. Then, I'll gladly take
that. Or, we can push any improvement patch in the next releases.
Thanks,
Yongseok
next prev parent reply other threads:[~2018-04-24 2:04 UTC|newest]
Thread overview: 86+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-10 1:25 [dpdk-dev] [PATCH v1 0/6] net/mlx5: add Multi-Packet Rx support Yongseok Koh
2018-03-10 1:25 ` [dpdk-dev] [PATCH v1 1/6] mbuf: add buffer offset field for flexible indirection Yongseok Koh
2018-03-10 1:25 ` [dpdk-dev] [PATCH v1 2/6] net/mlx5: separate filling Rx flags Yongseok Koh
2018-03-10 1:25 ` [dpdk-dev] [PATCH v1 3/6] net/mlx5: add a function to rdma-core glue Yongseok Koh
2018-03-12 9:13 ` Nélio Laranjeiro
2018-03-10 1:25 ` [dpdk-dev] [PATCH v1 4/6] net/mlx5: add Multi-Packet Rx support Yongseok Koh
2018-03-12 9:20 ` Nélio Laranjeiro
2018-03-10 1:25 ` [dpdk-dev] [PATCH v1 5/6] net/mlx5: release Tx queue resource earlier than Rx Yongseok Koh
2018-03-10 1:25 ` [dpdk-dev] [PATCH v1 6/6] app/testpmd: conserve mbuf indirection flag Yongseok Koh
2018-04-02 18:50 ` [dpdk-dev] [PATCH v2 0/6] net/mlx5: add Multi-Packet Rx support Yongseok Koh
2018-04-02 18:50 ` [dpdk-dev] [PATCH v2 1/6] mbuf: add buffer offset field for flexible indirection Yongseok Koh
2018-04-03 8:26 ` Olivier Matz
2018-04-04 0:12 ` Yongseok Koh
2018-04-09 16:04 ` Olivier Matz
2018-04-10 1:59 ` Yongseok Koh
2018-04-11 0:25 ` Ananyev, Konstantin
2018-04-11 5:33 ` Yongseok Koh
2018-04-11 11:39 ` Ananyev, Konstantin
2018-04-11 14:02 ` Andrew Rybchenko
2018-04-11 17:18 ` Yongseok Koh
2018-04-11 17:08 ` Yongseok Koh
2018-04-12 16:34 ` Ananyev, Konstantin
2018-04-12 18:58 ` Yongseok Koh
2018-04-02 18:50 ` [dpdk-dev] [PATCH v2 2/6] net/mlx5: separate filling Rx flags Yongseok Koh
2018-04-02 18:50 ` [dpdk-dev] [PATCH v2 3/6] net/mlx5: add a function to rdma-core glue Yongseok Koh
2018-04-02 18:50 ` [dpdk-dev] [PATCH v2 4/6] net/mlx5: add Multi-Packet Rx support Yongseok Koh
2018-04-02 18:50 ` [dpdk-dev] [PATCH v2 5/6] net/mlx5: release Tx queue resource earlier than Rx Yongseok Koh
2018-04-02 18:50 ` [dpdk-dev] [PATCH v2 6/6] app/testpmd: conserve mbuf indirection flag Yongseok Koh
2018-04-19 1:11 ` [dpdk-dev] [PATCH v3 1/2] mbuf: support attaching external buffer to mbuf Yongseok Koh
2018-04-19 1:11 ` [dpdk-dev] [PATCH v3 2/2] app/testpmd: conserve offload flags of mbuf Yongseok Koh
2018-04-23 11:53 ` [dpdk-dev] [PATCH v3 1/2] mbuf: support attaching external buffer to mbuf Ananyev, Konstantin
2018-04-24 2:04 ` Yongseok Koh [this message]
2018-04-25 13:16 ` Ananyev, Konstantin
2018-04-25 16:44 ` Yongseok Koh
2018-04-25 18:05 ` Ananyev, Konstantin
2018-04-23 16:18 ` Olivier Matz
2018-04-24 1:29 ` Yongseok Koh
2018-04-24 15:36 ` Olivier Matz
2018-04-24 1:38 ` [dpdk-dev] [PATCH v4 " Yongseok Koh
2018-04-24 1:38 ` [dpdk-dev] [PATCH v4 2/2] app/testpmd: conserve offload flags of mbuf Yongseok Koh
2018-04-24 5:01 ` [dpdk-dev] [PATCH v4 1/2] mbuf: support attaching external buffer to mbuf Stephen Hemminger
2018-04-24 11:47 ` Yongseok Koh
2018-04-24 12:28 ` Andrew Rybchenko
2018-04-24 16:02 ` Olivier Matz
2018-04-24 18:21 ` [dpdk-dev] ***Spam*** " Andrew Rybchenko
2018-04-24 19:15 ` [dpdk-dev] " Olivier Matz
2018-04-24 20:22 ` Thomas Monjalon
2018-04-24 21:53 ` Yongseok Koh
2018-04-24 22:15 ` Thomas Monjalon
2018-04-25 8:21 ` Olivier Matz
2018-04-25 15:06 ` Stephen Hemminger
2018-04-24 23:34 ` Yongseok Koh
2018-04-25 14:45 ` Andrew Rybchenko
2018-04-25 17:40 ` Yongseok Koh
2018-04-25 8:28 ` Olivier Matz
2018-04-25 9:08 ` Yongseok Koh
2018-04-25 9:19 ` Yongseok Koh
2018-04-25 20:00 ` Olivier Matz
2018-04-25 22:54 ` Yongseok Koh
2018-04-24 22:30 ` Yongseok Koh
2018-04-25 2:53 ` [dpdk-dev] [PATCH v5 " Yongseok Koh
2018-04-25 2:53 ` [dpdk-dev] [PATCH v5 2/2] app/testpmd: conserve offload flags of mbuf Yongseok Koh
2018-04-25 13:31 ` [dpdk-dev] [PATCH v5 1/2] mbuf: support attaching external buffer to mbuf Ananyev, Konstantin
2018-04-25 17:06 ` Yongseok Koh
2018-04-25 17:23 ` Ananyev, Konstantin
2018-04-25 18:02 ` Yongseok Koh
2018-04-25 18:22 ` Yongseok Koh
2018-04-25 18:30 ` Yongseok Koh
2018-04-26 1:10 ` [dpdk-dev] [PATCH v6 " Yongseok Koh
2018-04-26 1:10 ` [dpdk-dev] [PATCH v6 2/2] app/testpmd: conserve offload flags of mbuf Yongseok Koh
2018-04-26 11:39 ` [dpdk-dev] [PATCH v6 1/2] mbuf: support attaching external buffer to mbuf Ananyev, Konstantin
2018-04-26 16:05 ` Andrew Rybchenko
2018-04-26 16:10 ` Thomas Monjalon
2018-04-26 19:42 ` Olivier Matz
2018-04-26 19:58 ` Thomas Monjalon
2018-04-26 20:07 ` Olivier Matz
2018-04-26 20:24 ` Thomas Monjalon
2018-04-26 17:18 ` Yongseok Koh
2018-04-26 19:45 ` Olivier Matz
2018-04-27 0:01 ` [dpdk-dev] [PATCH v7 " Yongseok Koh
2018-04-27 0:01 ` [dpdk-dev] [PATCH v7 2/2] app/testpmd: conserve offload flags of mbuf Yongseok Koh
2018-04-27 8:00 ` Andrew Rybchenko
2018-04-27 7:22 ` [dpdk-dev] [PATCH v7 1/2] mbuf: support attaching external buffer to mbuf Andrew Rybchenko
2018-04-27 17:22 ` [dpdk-dev] [PATCH v8 " Yongseok Koh
2018-04-27 17:22 ` [dpdk-dev] [PATCH v8 2/2] app/testpmd: conserve offload flags of mbuf Yongseok Koh
2018-04-27 18:09 ` [dpdk-dev] [PATCH v8 1/2] mbuf: support attaching external buffer to mbuf Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180424020427.GA83470@yongseok-MBP.local \
--to=yskoh@mellanox.com \
--cc=adrien.mazarguil@6wind.com \
--cc=dev@dpdk.org \
--cc=jingjing.wu@intel.com \
--cc=konstantin.ananyev@intel.com \
--cc=nelio.laranjeiro@6wind.com \
--cc=olivier.matz@6wind.com \
--cc=wenzhuo.lu@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).