From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jianbo Liu Subject: Re: [PATCH] test: Fix memory corruption issues which fails the link_bonding test. Date: Mon, 10 Jul 2017 15:55:57 +0800 Message-ID: References: <1499671222-8283-1-git-send-email-herbert.guan@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Cc: dev@dpdk.org, Declan Doherty To: Herbert Guan Return-path: Received: from mail-yw0-f173.google.com (mail-yw0-f173.google.com [209.85.161.173]) by dpdk.org (Postfix) with ESMTP id 0FB6558CE for ; Mon, 10 Jul 2017 09:55:58 +0200 (CEST) Received: by mail-yw0-f173.google.com with SMTP id a12so32313683ywh.3 for ; Mon, 10 Jul 2017 00:55:58 -0700 (PDT) In-Reply-To: <1499671222-8283-1-git-send-email-herbert.guan@arm.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 10 July 2017 at 15:20, Herbert Guan wrote: > There were double-free problems in some test cases of link_bonding, > which will cause a duplicated mbuf will be added into mempool. After > double-free, some new allocated mbuf will hold a same address and > thus cause the memory corruption. > > Another minor issue is that in some test cases, allocated mbuf will > not be released after test case exits. Hopefully these leaked mbuf > will be released by the next test case in its setup phase when > stopping the virtual pmd ports, while this do is a memory leak of > the exited test case. > > To fix above 2 issues, this patch will do: > 1) Release virtual pmd ports' tx queue in the clean up function > remove_slaves_and_stop_bonded_device() of each test cases. > 2) Do not release allocated mbufs for test bursts. These mbufs > will be released in remove_slaves_and_stop_bonded_device() when > test case exits. > > Signed-off-by: Herbert Guan > --- > test/test/test_link_bonding.c | 54 +++---------------------------------------- > 1 file changed, 3 insertions(+), 51 deletions(-) > > diff --git a/test/test/test_link_bonding.c b/test/test/test_link_bonding.c > index aa2a1a2..c4f5c70 100644 > --- a/test/test/test_link_bonding.c > +++ b/test/test/test_link_bonding.c > @@ -221,6 +221,8 @@ struct rte_fdir_conf fdir_conf = { > > }; > > +static void free_virtualpmd_tx_queue(void); > + > static int > configure_ethdev(uint8_t port_id, uint8_t start, uint8_t en_isr) > { > @@ -684,6 +686,7 @@ struct rte_fdir_conf fdir_conf = { > remove_slaves_and_stop_bonded_device(void) > { > /* Clean up and remove slaves from bonded device */ > + free_virtualpmd_tx_queue(); > while (test_params->bonded_slave_count > 0) > TEST_ASSERT_SUCCESS(test_remove_slave_from_bonded_device(), > "test_remove_slave_from_bonded_device failed"); > @@ -1621,9 +1624,6 @@ struct rte_fdir_conf fdir_conf = { > > /* free mbufs */ > for (i = 0; i < MAX_PKT_BURST; i++) { > - if (gen_pkt_burst[i] != NULL) > - rte_pktmbuf_free(gen_pkt_burst[i]); > - > if (rx_pkt_burst[i] != NULL) > rte_pktmbuf_free(rx_pkt_burst[i]); > } > @@ -1970,12 +1970,6 @@ struct rte_fdir_conf fdir_conf = { > for (i = 0; i < MAX_PKT_BURST; i++) { > if (rx_pkt_burst[i] != NULL) > rte_pktmbuf_free(rx_pkt_burst[i]); > - > - if (gen_pkt_burst[1][i] != NULL) > - rte_pktmbuf_free(gen_pkt_burst[1][i]); > - > - if (gen_pkt_burst[3][i] != NULL) > - rte_pktmbuf_free(gen_pkt_burst[1][i]); > } > > /* Clean up and remove slaves from bonded device */ > @@ -2547,16 +2541,6 @@ struct rte_fdir_conf fdir_conf = { > "(%d) port_stats.opackets not as expected", > test_params->slave_port_ids[3]); > > - /* free mbufs */ > - for (i = 0; i < TEST_ACTIVE_BACKUP_RX_BURST_SLAVE_COUNT; i++) { > - for (j = 0; j < MAX_PKT_BURST; j++) { > - if (pkt_burst[i][j] != NULL) { > - rte_pktmbuf_free(pkt_burst[i][j]); > - pkt_burst[i][j] = NULL; > - } > - } > - } > - > /* Clean up and remove slaves from bonded device */ > return remove_slaves_and_stop_bonded_device(); > } > @@ -3456,16 +3440,6 @@ struct rte_fdir_conf fdir_conf = { > test_params->bonded_port_id, (int)port_stats.ipackets, > burst_size * 3); > > - /* free mbufs allocate for rx testing */ > - for (i = 0; i < TEST_BALANCE_RX_BURST_SLAVE_COUNT; i++) { > - for (j = 0; j < MAX_PKT_BURST; j++) { > - if (pkt_burst[i][j] != NULL) { > - rte_pktmbuf_free(pkt_burst[i][j]); > - pkt_burst[i][j] = NULL; > - } > - } > - } > - > /* Clean up and remove slaves from bonded device */ > return remove_slaves_and_stop_bonded_device(); > } > @@ -3984,16 +3958,6 @@ struct rte_fdir_conf fdir_conf = { > "(%d) port_stats.ipackets not as expected\n", > test_params->bonded_port_id); > > - /* free mbufs allocate for rx testing */ > - for (i = 0; i < BROADCAST_LINK_STATUS_NUM_OF_SLAVES; i++) { > - for (j = 0; j < MAX_PKT_BURST; j++) { > - if (pkt_burst[i][j] != NULL) { > - rte_pktmbuf_free(pkt_burst[i][j]); > - pkt_burst[i][j] = NULL; > - } > - } > - } > - > /* Clean up and remove slaves from bonded device */ > return remove_slaves_and_stop_bonded_device(); > } > @@ -4527,18 +4491,6 @@ struct rte_fdir_conf fdir_conf = { > "(%d) port_stats.ipackets not as expected\n", > test_params->bonded_port_id); > > - /* free mbufs */ > - > - for (i = 0; i < TEST_ADAPTIVE_TRANSMIT_LOAD_BALANCING_RX_BURST_SLAVE_COUNT; i++) { > - for (j = 0; j < MAX_PKT_BURST; j++) { > - if (pkt_burst[i][j] != NULL) { > - rte_pktmbuf_free(pkt_burst[i][j]); > - pkt_burst[i][j] = NULL; > - } > - } > - } > - > - > /* Clean up and remove slaves from bonded device */ > return remove_slaves_and_stop_bonded_device(); > } > -- > 1.8.3.1 > Acked-by: Jianbo Liu