[2/3] mbuf: change free_cb interface to adapt to GSO case
Checks
Commit Message
From: Yi Yang <yangyi01@inspur.com>
In GSO case, segmented mbufs are attached to original
mbuf which can't be freed when it is external. The issue
is free_cb doesn't know original mbuf and doesn't free
it when refcnt of shinfo is 0.
Original mbuf can be freed by rte_pktmbuf_free segmented
mbufs or by rte_pktmbuf_free original mbuf. Two kind of
cases should have different behaviors. free_cb won't
explicitly call rte_pktmbuf_free to free original mbuf
if it is freed by rte_pktmbuf_free original mbuf, but it
has to call rte_pktmbuf_free to free original mbuf if it
is freed by rte_pktmbuf_free segmented mbufs.
In order to fix this issue, free_cb interface has to been
changed, __rte_pktmbuf_free_extbuf must deliver called
mbuf pointer to free_cb, argument opaque can be defined
as a custom struct by user, it can includes original mbuf
pointer, user-defined free_cb can compare caller mbuf with
mbuf in opaque struct, free_cb should free original mbuf
if they are not same, this corresponds to rte_pktmbuf_free
segmented mbufs case, otherwise, free_cb won't free original
mbuf because the caller explicitly called rte_pktmbuf_free
to free it.
Here is pseduo code to show two kind of cases.
case 1. rte_pktmbuf_free segmented mbufs
nb_tx = rte_gso_segment(original_mbuf, /* original mbuf */
&gso_ctx,
/* segmented mbuf */
(struct rte_mbuf **)&gso_mbufs,
MAX_GSO_MBUFS);
rte_eth_tx_burst(dev->port_id, qid, gso_mbufs, nb_tx);
case 2. rte_pktmbuf_free original mbuf
rte_eth_tx_burst(dev->port_id, qid, original_mbuf, 1);
Here are user defined free_cb and opaque.
struct shinfo_arg {
void *buf;
struct rte_mbuf *mbuf;
};
custom_free_cb(struct rte_mbuf *caller_m, void *opaque)
{
struct shinfo_arg *arg = (struct shinfo_arg *)opaque;
rte_free(arg->buf);
if (caller_m != arg->mbuf)
rte_pktmbuf_free(arg->mbuf);
rte_free(arg);
}
struct shinfo_arg *arg = (struct shinfo_arg *)
rte_malloc(NULL, sizeof(struct shinfo_arg),
RTE_CACHE_LINE_SIZE);
arg->buf = buf;
arg->mbuf = pkt;
shinfo->free_cb = custom_free_cb;
shinfo->fcb_opaque = arg;
Signed-off-by: Yi Yang <yangyi01@inspur.com>
---
app/test-compress-perf/comp_perf_test_common.c | 2 +-
app/test/test_compressdev.c | 2 +-
app/test/test_mbuf.c | 2 +-
drivers/net/mlx5/mlx5_rxtx.c | 2 +-
drivers/net/mlx5/mlx5_rxtx.h | 2 +-
drivers/net/netvsc/hn_rxtx.c | 3 ++-
lib/librte_mbuf/rte_mbuf.c | 4 ++--
lib/librte_mbuf/rte_mbuf.h | 2 +-
lib/librte_mbuf/rte_mbuf_core.h | 2 +-
lib/librte_vhost/virtio_net.c | 2 +-
10 files changed, 12 insertions(+), 11 deletions(-)
Comments
30/07/2020 11:56, yang_y_yi@163.com:
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -1193,7 +1193,7 @@ static inline void rte_pktmbuf_attach(struct rte_mbuf *mi, struct rte_mbuf *m)
> RTE_ASSERT(m->shinfo != NULL);
>
> if (rte_mbuf_ext_refcnt_update(m->shinfo, -1) == 0)
> - m->shinfo->free_cb(m->buf_addr, m->shinfo->fcb_opaque);
> + m->shinfo->free_cb(m, m->shinfo->fcb_opaque);
> }
>
> /**
> diff --git a/lib/librte_mbuf/rte_mbuf_core.h b/lib/librte_mbuf/rte_mbuf_core.h
> index 8cd7137..d194429 100644
> --- a/lib/librte_mbuf/rte_mbuf_core.h
> +++ b/lib/librte_mbuf/rte_mbuf_core.h
> @@ -671,7 +671,7 @@ struct rte_mbuf {
> /**
> * Function typedef of callback to free externally attached buffer.
> */
> -typedef void (*rte_mbuf_extbuf_free_callback_t)(void *addr, void *opaque);
> +typedef void (*rte_mbuf_extbuf_free_callback_t)(struct rte_mbuf *, void *);
I think a deprecation notice is required in 20.08
in order to apply such change in 20.11.
Please use --cc-cmd devtools/get-maintainer.sh when sending a patch.
30/07/2020 12:39, Yi Yang (杨燚)-云服务集团:
> Thomas, do you mean I need to change the below files to add deprecation notice about free_cb?
No it should be a separate patch for announing the deprecation in 20.08.
Below patches cannot be applied in 20.08.
>
> doc/guides/rel_notes/deprecation.rst
> doc/guides/rel_notes/release_20_08.rst
>
> I'll use --cc-cmd devtools/get-maintainer.sh to send a new version including deprecation notice you mentioned.
>
> -----邮件原件-----
> 发件人: Thomas Monjalon [mailto:thomas@monjalon.net]
> 发送时间: 2020年7月30日 18:17
> 收件人: yang_y_yi@163.com
> 抄送: dev@dpdk.org; jiayu.hu@intel.com; Yi Yang (杨燚)-云服务集团 <yangyi01@inspur.com>; yang_y_yi@163.com; olivier.matz@6wind.com
> 主题: Re: [PATCH 2/3] mbuf: change free_cb interface to adapt to GSO case
>
> 30/07/2020 11:56, yang_y_yi@163.com:
> > --- a/lib/librte_mbuf/rte_mbuf.h
> > +++ b/lib/librte_mbuf/rte_mbuf.h
> > @@ -1193,7 +1193,7 @@ static inline void rte_pktmbuf_attach(struct rte_mbuf *mi, struct rte_mbuf *m)
> > RTE_ASSERT(m->shinfo != NULL);
> >
> > if (rte_mbuf_ext_refcnt_update(m->shinfo, -1) == 0)
> > - m->shinfo->free_cb(m->buf_addr, m->shinfo->fcb_opaque);
> > + m->shinfo->free_cb(m, m->shinfo->fcb_opaque);
> > }
> >
> > /**
> > diff --git a/lib/librte_mbuf/rte_mbuf_core.h
> > b/lib/librte_mbuf/rte_mbuf_core.h index 8cd7137..d194429 100644
> > --- a/lib/librte_mbuf/rte_mbuf_core.h
> > +++ b/lib/librte_mbuf/rte_mbuf_core.h
> > @@ -671,7 +671,7 @@ struct rte_mbuf {
> > /**
> > * Function typedef of callback to free externally attached buffer.
> > */
> > -typedef void (*rte_mbuf_extbuf_free_callback_t)(void *addr, void
> > *opaque);
> > +typedef void (*rte_mbuf_extbuf_free_callback_t)(struct rte_mbuf *,
> > +void *);
>
> I think a deprecation notice is required in 20.08 in order to apply such change in 20.11.
>
> Please use --cc-cmd devtools/get-maintainer.sh when sending a patch.
>
>
>
Got it. Thanks a lot.
在 2020-07-30 18:42:38，"Thomas Monjalon" <thomas@monjalon.net> 写道：
>30/07/2020 12:39, Yi Yang (杨燚)-云服务集团:
>> Thomas, do you mean I need to change the below files to add deprecation notice about free_cb?
>
>No it should be a separate patch for announing the deprecation in 20.08.
>
>Below patches cannot be applied in 20.08.
>
>
>>
>> doc/guides/rel_notes/deprecation.rst
>> doc/guides/rel_notes/release_20_08.rst
>>
>> I'll use --cc-cmd devtools/get-maintainer.sh to send a new version including deprecation notice you mentioned.
>>
>> -----邮件原件-----
>> 发件人: Thomas Monjalon [mailto:thomas@monjalon.net]
>> 发送时间: 2020年7月30日 18:17
>> 收件人: yang_y_yi@163.com
>> 抄送: dev@dpdk.org; jiayu.hu@intel.com; Yi Yang (杨燚)-云服务集团 <yangyi01@inspur.com>; yang_y_yi@163.com; olivier.matz@6wind.com
>> 主题: Re: [PATCH 2/3] mbuf: change free_cb interface to adapt to GSO case
>>
>> 30/07/2020 11:56, yang_y_yi@163.com:
>> > --- a/lib/librte_mbuf/rte_mbuf.h
>> > +++ b/lib/librte_mbuf/rte_mbuf.h
>> > @@ -1193,7 +1193,7 @@ static inline void rte_pktmbuf_attach(struct rte_mbuf *mi, struct rte_mbuf *m)
>> > RTE_ASSERT(m->shinfo != NULL);
>> >
>> > if (rte_mbuf_ext_refcnt_update(m->shinfo, -1) == 0)
>> > - m->shinfo->free_cb(m->buf_addr, m->shinfo->fcb_opaque);
>> > + m->shinfo->free_cb(m, m->shinfo->fcb_opaque);
>> > }
>> >
>> > /**
>> > diff --git a/lib/librte_mbuf/rte_mbuf_core.h
>> > b/lib/librte_mbuf/rte_mbuf_core.h index 8cd7137..d194429 100644
>> > --- a/lib/librte_mbuf/rte_mbuf_core.h
>> > +++ b/lib/librte_mbuf/rte_mbuf_core.h
>> > @@ -671,7 +671,7 @@ struct rte_mbuf {
>> > /**
>> > * Function typedef of callback to free externally attached buffer.
>> > */
>> > -typedef void (*rte_mbuf_extbuf_free_callback_t)(void *addr, void
>> > *opaque);
>> > +typedef void (*rte_mbuf_extbuf_free_callback_t)(struct rte_mbuf *,
>> > +void *);
>>
>> I think a deprecation notice is required in 20.08 in order to apply such change in 20.11.
>>
>> Please use --cc-cmd devtools/get-maintainer.sh when sending a patch.
>>
>>
>>
>
>
>
>
Unfortunately, git send-email in windows can't work with -cc-cmd, I just tried it but maintainers weren't added to cc list. Here is my get-maintainer.sh. Anybody knows why it can't work.
$ ./get-maintainer.sh dpdk-patches/0000-cover-letter.patch
yangyi01@yangyi0100 MINGW64 ~
$ ./get-maintainer.sh dpdk-patches/0001-gso-fix-refcnt-update-issue-for-external-mbuf.patch
Jiayu Hu <jiayu.hu@intel.com>
Mark Kavanagh <mark.b.kavanagh@intel.com>
Konstantin Ananyev <konstantin.ananyev@intel.com>
yangyi01@yangyi0100 MINGW64 ~
$ ./get-maintainer.sh dpdk-patches/0002-mbuf-change-free_cb-interface-to-adapt-to-GSO-case.patch
Fiona Trahe <fiona.trahe@intel.com>
Ashish Gupta <ashish.gupta@marvell.com>
Olivier Matz <olivier.matz@6wind.com>
Matan Azrad <matan@mellanox.com>
Shahaf Shuler <shahafs@mellanox.com>
Viacheslav Ovsiienko <viacheslavo@mellanox.com>
Stephen Hemminger <sthemmin@microsoft.com>
K. Y. Srinivasan <kys@microsoft.com>
Haiyang Zhang <haiyangz@microsoft.com>
Long Li <longli@microsoft.com>
Maxime Coquelin <maxime.coquelin@redhat.com>
Zhihong Wang <zhihong.wang@intel.com>
yangyi01@yangyi0100 MINGW64 ~
$ ./get-maintainer.sh dpdk-patches/0003-vhost-use-new-free_cb-interface-to-fix-mbuf-free-iss.patch
Maxime Coquelin <maxime.coquelin@redhat.com>
Zhihong Wang <zhihong.wang@intel.com>
yangyi01@yangyi0100 MINGW64 ~
在 2020-07-30 18:42:38，"Thomas Monjalon" <thomas@monjalon.net> 写道：
>30/07/2020 12:39, Yi Yang (杨燚)-云服务集团:
>> Thomas, do you mean I need to change the below files to add deprecation notice about free_cb?
>
>No it should be a separate patch for announing the deprecation in 20.08.
>
>Below patches cannot be applied in 20.08.
>
>
>>
>> doc/guides/rel_notes/deprecation.rst
>> doc/guides/rel_notes/release_20_08.rst
>>
>> I'll use --cc-cmd devtools/get-maintainer.sh to send a new version including deprecation notice you mentioned.
>>
>> -----邮件原件-----
>> 发件人: Thomas Monjalon [mailto:thomas@monjalon.net]
>> 发送时间: 2020年7月30日 18:17
>> 收件人: yang_y_yi@163.com
>> 抄送: dev@dpdk.org; jiayu.hu@intel.com; Yi Yang (杨燚)-云服务集团 <yangyi01@inspur.com>; yang_y_yi@163.com; olivier.matz@6wind.com
>> 主题: Re: [PATCH 2/3] mbuf: change free_cb interface to adapt to GSO case
>>
>> 30/07/2020 11:56, yang_y_yi@163.com:
>> > --- a/lib/librte_mbuf/rte_mbuf.h
>> > +++ b/lib/librte_mbuf/rte_mbuf.h
>> > @@ -1193,7 +1193,7 @@ static inline void rte_pktmbuf_attach(struct rte_mbuf *mi, struct rte_mbuf *m)
>> > RTE_ASSERT(m->shinfo != NULL);
>> >
>> > if (rte_mbuf_ext_refcnt_update(m->shinfo, -1) == 0)
>> > - m->shinfo->free_cb(m->buf_addr, m->shinfo->fcb_opaque);
>> > + m->shinfo->free_cb(m, m->shinfo->fcb_opaque);
>> > }
>> >
>> > /**
>> > diff --git a/lib/librte_mbuf/rte_mbuf_core.h
>> > b/lib/librte_mbuf/rte_mbuf_core.h index 8cd7137..d194429 100644
>> > --- a/lib/librte_mbuf/rte_mbuf_core.h
>> > +++ b/lib/librte_mbuf/rte_mbuf_core.h
>> > @@ -671,7 +671,7 @@ struct rte_mbuf {
>> > /**
>> > * Function typedef of callback to free externally attached buffer.
>> > */
>> > -typedef void (*rte_mbuf_extbuf_free_callback_t)(void *addr, void
>> > *opaque);
>> > +typedef void (*rte_mbuf_extbuf_free_callback_t)(struct rte_mbuf *,
>> > +void *);
>>
>> I think a deprecation notice is required in 20.08 in order to apply such change in 20.11.
>>
>> Please use --cc-cmd devtools/get-maintainer.sh when sending a patch.
>>
>>
>>
>
>
>
>
@@ -115,7 +115,7 @@ struct cperf_buffer_info {
}
static void
-comp_perf_extbuf_free_cb(void *addr __rte_unused, void *opaque __rte_unused)
+comp_perf_extbuf_free_cb(struct rte_mbuf *m __rte_unused, void *opaque __rte_unused)
{
}
@@ -778,7 +778,7 @@ struct test_private_arrays {
}
static void
-extbuf_free_callback(void *addr __rte_unused, void *opaque __rte_unused)
+extbuf_free_callback(struct rte_mbuf *m __rte_unused, void *opaque __rte_unused)
{
}
@@ -2300,7 +2300,7 @@ struct test_case {
/* Define a free call back function to be used for external buffer */
static void
-ext_buf_free_callback_fn(void *addr __rte_unused, void *opaque)
+ext_buf_free_callback_fn(struct rte_mbuf *caller_m __rte_unused, void *opaque)
{
void *ext_buf_addr = opaque;
@@ -1622,7 +1622,7 @@ enum mlx5_txcmp_code {
}
void
-mlx5_mprq_buf_free_cb(void *addr __rte_unused, void *opaque)
+mlx5_mprq_buf_free_cb(struct rte_mbuf *caller_m __rte_unused, void *opaque)
{
struct mlx5_mprq_buf *buf = opaque;
@@ -499,7 +499,7 @@ struct mlx5_txq_ctrl *mlx5_txq_hairpin_new
uint16_t mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n);
void mlx5_rxq_initialize(struct mlx5_rxq_data *rxq);
__rte_noinline int mlx5_rx_err_handle(struct mlx5_rxq_data *rxq, uint8_t vec);
-void mlx5_mprq_buf_free_cb(void *addr, void *opaque);
+void mlx5_mprq_buf_free_cb(struct rte_mbuf *caller_m, void *opaque);
void mlx5_mprq_buf_free(struct mlx5_mprq_buf *buf);
uint16_t mlx5_rx_burst_mprq(void *dpdk_rxq, struct rte_mbuf **pkts,
uint16_t pkts_n);
@@ -519,7 +519,8 @@ int hn_dev_tx_descriptor_status(void *arg, uint16_t offset)
return 0;
}
-static void hn_rx_buf_free_cb(void *buf __rte_unused, void *opaque)
+static void hn_rx_buf_free_cb(struct rte_mbuf *caller_m __rte_unused,
+ void *opaque)
{
struct hn_rx_bufinfo *rxb = opaque;
struct hn_data *hv = rxb->hv;
@@ -118,11 +118,11 @@
* buffer.
*/
static void
-rte_pktmbuf_free_pinned_extmem(void *addr, void *opaque)
+rte_pktmbuf_free_pinned_extmem(struct rte_mbuf *caller_m, void *opaque)
{
struct rte_mbuf *m = opaque;
- RTE_SET_USED(addr);
+ RTE_SET_USED(caller_m);
RTE_ASSERT(RTE_MBUF_HAS_EXTBUF(m));
RTE_ASSERT(RTE_MBUF_HAS_PINNED_EXTBUF(m));
RTE_ASSERT(m->shinfo->fcb_opaque == m);
@@ -1193,7 +1193,7 @@ static inline void rte_pktmbuf_attach(struct rte_mbuf *mi, struct rte_mbuf *m)
RTE_ASSERT(m->shinfo != NULL);
if (rte_mbuf_ext_refcnt_update(m->shinfo, -1) == 0)
- m->shinfo->free_cb(m->buf_addr, m->shinfo->fcb_opaque);
+ m->shinfo->free_cb(m, m->shinfo->fcb_opaque);
}
/**
@@ -671,7 +671,7 @@ struct rte_mbuf {
/**
* Function typedef of callback to free externally attached buffer.
*/
-typedef void (*rte_mbuf_extbuf_free_callback_t)(void *addr, void *opaque);
+typedef void (*rte_mbuf_extbuf_free_callback_t)(struct rte_mbuf *, void *);
/**
* Shared data at the end of an external buffer.
@@ -2137,7 +2137,7 @@ uint16_t rte_vhost_poll_enqueue_completed(int vid, uint16_t queue_id,
}
static void
-virtio_dev_extbuf_free(void *addr __rte_unused, void *opaque)
+virtio_dev_extbuf_free(struct rte_mbuf * caller_m __rte_unused, void *opaque)
{
rte_free(opaque);
}