From: Gagandeep Singh <G.Singh@nxp.com>
To: Ferruh Yigit <ferruh.yigit@amd.com>, "dev@dpdk.org" <dev@dpdk.org>
Subject: RE: [PATCH 07/15] net/dpaa2: use internal mempool for SG table
Date: Thu, 6 Oct 2022 08:49:29 +0000 [thread overview]
Message-ID: <AS8PR04MB819829E48296B0500F865764E15C9@AS8PR04MB8198.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <8a537bed-d720-bf74-b11f-291d70dc8bb5@amd.com>
Hi
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Wednesday, October 5, 2022 7:51 PM
> To: Gagandeep Singh <G.Singh@nxp.com>; dev@dpdk.org
> Subject: Re: [PATCH 07/15] net/dpaa2: use internal mempool for SG table
>
> On 9/28/2022 6:25 AM, Gagandeep Singh wrote:
> > Creating and using driver's mempool for allocating the SG table memory
> > required for FD creation instead of relying on user mempool.
> >
>
> As far as I can see this is in the Tx path, can you please explain why
> driver need an internal pktmbuf pool?
>
> And shouldn't mempool needs to be freed on driver close and release.
>
> Same comment applies for dpaa version of the patch (12/15).
>
In TX path, in case of SG packets, driver need additional SG table memory to store the data
of all segments to pass to HW. Earlier it was dependent on the user's pool to allocate memory
which is wrong. so allocating the new pool in the driver for this purpose.
We are creating only one pool for all the DPAA net devices on first DPAA net device probe, so it can
be free only on last device probe. So will it be worth to free the pool on last device probe as
memory will be released on process kill as well?
> > Signed-off-by: Gagandeep Singh <g.singh@nxp.com>
> > ---
> > drivers/net/dpaa2/dpaa2_ethdev.c | 14 ++++++++++++++
> > drivers/net/dpaa2/dpaa2_ethdev.h | 9 +++++++++
> > drivers/net/dpaa2/dpaa2_rxtx.c | 13 ++++++-------
> > 3 files changed, 29 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c
> b/drivers/net/dpaa2/dpaa2_ethdev.c
> > index 37a8b43114..c7aae70300 100644
> > --- a/drivers/net/dpaa2/dpaa2_ethdev.c
> > +++ b/drivers/net/dpaa2/dpaa2_ethdev.c
> > @@ -78,6 +78,8 @@ bool dpaa2_enable_err_queue;
> > #define MAX_NB_RX_DESC 11264
> > int total_nb_rx_desc;
> >
> > +struct rte_mempool *dpaa2_tx_sg_pool;
> > +
> > struct rte_dpaa2_xstats_name_off {
> > char name[RTE_ETH_XSTATS_NAME_SIZE];
> > uint8_t page_id; /* dpni statistics page id */
> > @@ -2907,6 +2909,18 @@ rte_dpaa2_probe(struct rte_dpaa2_driver
> *dpaa2_drv,
> > /* Invoke PMD device initialization function */
> > diag = dpaa2_dev_init(eth_dev);
> > if (diag == 0) {
> > + if (!dpaa2_tx_sg_pool) {
> > + dpaa2_tx_sg_pool =
> > +
> rte_pktmbuf_pool_create("dpaa2_mbuf_tx_sg_pool",
> > + DPAA2_POOL_SIZE,
> > + DPAA2_POOL_CACHE_SIZE, 0,
> > + DPAA2_MAX_SGS * sizeof(struct qbman_sge),
> > + rte_socket_id());
> > + if (dpaa2_tx_sg_pool == NULL) {
> > + DPAA2_PMD_ERR("SG pool creation
> failed\n");
> > + return -ENOMEM;
> > + }
> > + }
> > rte_eth_dev_probing_finish(eth_dev);
> > return 0;
> > }
> > diff --git a/drivers/net/dpaa2/dpaa2_ethdev.h
> b/drivers/net/dpaa2/dpaa2_ethdev.h
> > index 32ae762e4a..872dced517 100644
> > --- a/drivers/net/dpaa2/dpaa2_ethdev.h
> > +++ b/drivers/net/dpaa2/dpaa2_ethdev.h
> > @@ -121,6 +121,15 @@
> > #define DPAA2_PKT_TYPE_VLAN_1 0x0160
> > #define DPAA2_PKT_TYPE_VLAN_2 0x0260
> >
> > +/* Global pool used by driver for SG list TX */
> > +extern struct rte_mempool *dpaa2_tx_sg_pool;
> > +/* Maximum SG segments */
> > +#define DPAA2_MAX_SGS 128
> > +/* SG pool size */
> > +#define DPAA2_POOL_SIZE 2048
> > +/* SG pool cache size */
> > +#define DPAA2_POOL_CACHE_SIZE 256
> > +
> > /* enable timestamp in mbuf*/
> > extern bool dpaa2_enable_ts[];
> > extern uint64_t dpaa2_timestamp_rx_dynflag;
> > diff --git a/drivers/net/dpaa2/dpaa2_rxtx.c
> b/drivers/net/dpaa2/dpaa2_rxtx.c
> > index bc0e49b0d4..dcd86c4056 100644
> > --- a/drivers/net/dpaa2/dpaa2_rxtx.c
> > +++ b/drivers/net/dpaa2/dpaa2_rxtx.c
> > @@ -403,7 +403,7 @@ eth_fd_to_mbuf(const struct qbman_fd *fd,
> > static int __rte_noinline __rte_hot
> > eth_mbuf_to_sg_fd(struct rte_mbuf *mbuf,
> > struct qbman_fd *fd,
> > - struct rte_mempool *mp, uint16_t bpid)
> > + uint16_t bpid)
> > {
> > struct rte_mbuf *cur_seg = mbuf, *prev_seg, *mi, *temp;
> > struct qbman_sge *sgt, *sge = NULL;
> > @@ -433,12 +433,12 @@ eth_mbuf_to_sg_fd(struct rte_mbuf *mbuf,
> > }
> > DPAA2_SET_FD_OFFSET(fd, offset);
> > } else {
> > - temp = rte_pktmbuf_alloc(mp);
> > + temp = rte_pktmbuf_alloc(dpaa2_tx_sg_pool);
> > if (temp == NULL) {
> > DPAA2_PMD_DP_DEBUG("No memory to allocate
> S/G table\n");
> > return -ENOMEM;
> > }
> > - DPAA2_SET_ONLY_FD_BPID(fd, bpid);
> > + DPAA2_SET_ONLY_FD_BPID(fd,
> mempool_to_bpid(dpaa2_tx_sg_pool));
> > DPAA2_SET_FD_OFFSET(fd, temp->data_off);
> > #ifdef RTE_LIBRTE_MEMPOOL_DEBUG
> > rte_mempool_check_cookies(rte_mempool_from_obj((void
> *)temp),
> > @@ -1321,9 +1321,10 @@ dpaa2_dev_tx(void *queue, struct rte_mbuf
> **bufs, uint16_t nb_pkts)
> >
> > if (unlikely(RTE_MBUF_HAS_EXTBUF(*bufs))) {
> > if (unlikely((*bufs)->nb_segs > 1)) {
> > + mp = (*bufs)->pool;
> > if (eth_mbuf_to_sg_fd(*bufs,
> > &fd_arr[loop],
> > - mp, 0))
> > +
> mempool_to_bpid(mp)))
> > goto send_n_return;
> > } else {
> > eth_mbuf_to_fd(*bufs,
> > @@ -1372,7 +1373,7 @@ dpaa2_dev_tx(void *queue, struct rte_mbuf
> **bufs, uint16_t nb_pkts)
> > if (unlikely((*bufs)->nb_segs > 1)) {
> > if (eth_mbuf_to_sg_fd(*bufs,
> > &fd_arr[loop],
> > - mp, bpid))
> > + bpid))
> > goto send_n_return;
> > } else {
> > eth_mbuf_to_fd(*bufs,
> > @@ -1646,7 +1647,6 @@ dpaa2_dev_tx_multi_txq_ordered(void **queue,
> > if (unlikely((*bufs)->nb_segs > 1)) {
> > if (eth_mbuf_to_sg_fd(*bufs,
> > &fd_arr[loop],
> > - mp,
> > bpid))
> > goto send_frames;
> > } else {
> > @@ -1810,7 +1810,6 @@ dpaa2_dev_tx_ordered(void *queue, struct
> rte_mbuf **bufs, uint16_t nb_pkts)
> > if (unlikely((*bufs)->nb_segs > 1)) {
> > if (eth_mbuf_to_sg_fd(*bufs,
> > &fd_arr[loop],
> > - mp,
> > bpid))
> > goto send_n_return;
> > } else {
next prev parent reply other threads:[~2022-10-06 8:49 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-28 5:25 [PATCH 00/15] DPAA and DPAA2 driver changes Gagandeep Singh
2022-09-28 5:25 ` [PATCH 01/15] bus/dpaa: use non-block mode for FD open Gagandeep Singh
2022-09-28 5:25 ` [PATCH 02/15] net/enetfec: fix restart issue Gagandeep Singh
2022-09-28 5:25 ` [PATCH 03/15] net/enetfec: fix buffer leak issue Gagandeep Singh
2022-09-28 5:25 ` [PATCH 04/15] net/dpaa2: fix dpdmux configuration for error behaviour Gagandeep Singh
2022-09-28 5:25 ` [PATCH 05/15] net/dpaa2: check free enqueue descriptors before Tx Gagandeep Singh
2022-10-05 14:30 ` Ferruh Yigit
2022-09-28 5:25 ` [PATCH 06/15] net/dpaa: support ESP packet type in packet parsing Gagandeep Singh
2022-10-05 14:20 ` Ferruh Yigit
2022-10-06 8:48 ` Gagandeep Singh
2022-09-28 5:25 ` [PATCH 07/15] net/dpaa2: use internal mempool for SG table Gagandeep Singh
2022-10-05 14:20 ` Ferruh Yigit
2022-10-06 8:49 ` Gagandeep Singh [this message]
2022-10-06 9:38 ` Ferruh Yigit
2022-10-06 11:18 ` Gagandeep Singh
2022-09-28 5:25 ` [PATCH 08/15] net/dpaa2: fix buffer free on transmit SG packets Gagandeep Singh
2022-10-06 7:48 ` Ferruh Yigit
2022-09-28 5:25 ` [PATCH 09/15] bus/fslmc: add timeout in MC send command API Gagandeep Singh
2022-09-28 5:25 ` [PATCH 10/15] net/dpaa: fix Jumbo packet Rx in case of VSP Gagandeep Singh
2022-09-28 5:25 ` [PATCH 11/15] bus/dpaa: pass interface name as a string instead of pointer Gagandeep Singh
2022-10-05 14:21 ` Ferruh Yigit
2022-10-06 8:51 ` Gagandeep Singh
2022-10-06 9:39 ` Ferruh Yigit
2022-10-06 11:18 ` Gagandeep Singh
2022-09-28 5:25 ` [PATCH 12/15] net/dpaa: use internal mempool for SG table Gagandeep Singh
2022-09-28 5:25 ` [PATCH 13/15] bus/dpaa: mempool ops registration change Gagandeep Singh
2022-09-28 5:25 ` [PATCH 14/15] net/dpaa: fix buffer free on transmit SG packets Gagandeep Singh
2022-09-28 5:25 ` [PATCH 15/15] net/dpaa: fix buffer free in slow path Gagandeep Singh
2022-10-05 14:21 ` Ferruh Yigit
2022-10-06 8:51 ` Gagandeep Singh
2022-10-06 9:42 ` Ferruh Yigit
2022-10-06 11:19 ` Gagandeep Singh
2022-10-05 14:27 ` [PATCH 00/15] DPAA and DPAA2 driver changes Ferruh Yigit
2022-10-06 11:15 ` Hemant Agrawal
2022-10-07 3:27 ` [PATCH v2 00/16] " Gagandeep Singh
2022-10-07 3:27 ` [PATCH v2 01/16] bus/dpaa: use non-block mode for FD open Gagandeep Singh
2022-10-07 3:27 ` [PATCH v2 02/16] net/enetfec: fix restart issue Gagandeep Singh
2022-10-07 3:27 ` [PATCH v2 03/16] net/enetfec: fix buffer leak issue Gagandeep Singh
2022-10-07 3:27 ` [PATCH v2 04/16] net/dpaa2: fix dpdmux configuration for error behaviour Gagandeep Singh
2022-10-07 3:27 ` [PATCH v2 05/16] net/dpaa2: check free enqueue descriptors before Tx Gagandeep Singh
2022-10-07 3:27 ` [PATCH v2 06/16] net/dpaa: support ESP packet type in packet parsing Gagandeep Singh
2022-10-07 3:27 ` [PATCH v2 07/16] net/dpaa2: use internal mempool for SG table Gagandeep Singh
2022-10-07 3:27 ` [PATCH v2 08/16] net/dpaa2: fix buffer free on transmit SG packets Gagandeep Singh
2022-10-07 3:27 ` [PATCH v2 09/16] bus/fslmc: add timeout in MC send command API Gagandeep Singh
2022-10-07 3:27 ` [PATCH v2 10/16] net/dpaa: fix Jumbo packet Rx in case of VSP Gagandeep Singh
2022-10-07 3:27 ` [PATCH v2 11/16] doc: add kernel version compatible information Gagandeep Singh
2022-10-07 3:27 ` [PATCH v2 12/16] bus/dpaa: pass interface name as a string instead of pointer Gagandeep Singh
2022-10-07 3:27 ` [PATCH v2 13/16] net/dpaa: use internal mempool for SG table Gagandeep Singh
2022-10-07 3:27 ` [PATCH v2 14/16] bus/dpaa: mempool ops registration change Gagandeep Singh
2022-10-07 3:27 ` [PATCH v2 15/16] net/dpaa: fix buffer free on transmit SG packets Gagandeep Singh
2022-10-07 3:27 ` [PATCH v2 16/16] net/dpaa: fix buffer free in slow path Gagandeep Singh
2022-10-07 15:22 ` [PATCH v2 00/16] DPAA and DPAA2 driver changes Ferruh Yigit
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=AS8PR04MB819829E48296B0500F865764E15C9@AS8PR04MB8198.eurprd04.prod.outlook.com \
--to=g.singh@nxp.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@amd.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).