* [dpdk-dev] [PATCH] test: Fix memory corruption issues which fails the link_bonding test. @ 2017-07-10 7:20 Herbert Guan 2017-07-10 7:55 ` Jianbo Liu ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Herbert Guan @ 2017-07-10 7:20 UTC (permalink / raw) To: dev, declan.doherty, jianbo.liu; +Cc: Herbert Guan 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 <herbert.guan@arm.com> --- 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] test: Fix memory corruption issues which fails the link_bonding test. 2017-07-10 7:20 [dpdk-dev] [PATCH] test: Fix memory corruption issues which fails the link_bonding test Herbert Guan @ 2017-07-10 7:55 ` Jianbo Liu 2017-07-10 11:13 ` [dpdk-dev] [PATCH v2] " Herbert Guan 2017-07-12 9:59 ` [dpdk-dev] [PATCH] " Declan Doherty 2 siblings, 0 replies; 9+ messages in thread From: Jianbo Liu @ 2017-07-10 7:55 UTC (permalink / raw) To: Herbert Guan; +Cc: dev, Declan Doherty On 10 July 2017 at 15:20, Herbert Guan <herbert.guan@arm.com> 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 <herbert.guan@arm.com> > --- > 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 <jianbo.liu@linaro.org> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [dpdk-dev] [PATCH v2] test: Fix memory corruption issues which fails the link_bonding test. 2017-07-10 7:20 [dpdk-dev] [PATCH] test: Fix memory corruption issues which fails the link_bonding test Herbert Guan 2017-07-10 7:55 ` Jianbo Liu @ 2017-07-10 11:13 ` Herbert Guan 2017-07-12 10:23 ` Declan Doherty 2017-07-18 16:13 ` Ferruh Yigit 2017-07-12 9:59 ` [dpdk-dev] [PATCH] " Declan Doherty 2 siblings, 2 replies; 9+ messages in thread From: Herbert Guan @ 2017-07-10 11:13 UTC (permalink / raw) To: dev, declan.doherty, jianbo.liu; +Cc: Herbert Guan Patch V2: fix build warnings by deleting unused variables. There were double-free problems in some test cases, 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 <herbert.guan@arm.com> --- test/test/test_link_bonding.c | 64 ++++++------------------------------------- 1 file changed, 9 insertions(+), 55 deletions(-) diff --git a/test/test/test_link_bonding.c b/test/test/test_link_bonding.c index aa2a1a2..140d864 100644 --- a/test/test/test_link_bonding.c +++ b/test/test/test_link_bonding.c @@ -221,6 +221,10 @@ 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 +688,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 +1626,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 +1972,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 */ @@ -2414,7 +2410,7 @@ struct rte_fdir_conf fdir_conf = { uint8_t slaves[RTE_MAX_ETHPORTS]; - int i, j, burst_size, slave_count, primary_port; + int i, burst_size, slave_count, primary_port; burst_size = 21; @@ -2547,16 +2543,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(); } @@ -3318,7 +3304,7 @@ struct rte_fdir_conf fdir_conf = { uint8_t slaves[RTE_MAX_ETHPORTS]; - int i, j, burst_size, slave_count; + int i, burst_size, slave_count; memset(pkt_burst, 0, sizeof(pkt_burst)); @@ -3456,16 +3442,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(); } @@ -3887,7 +3863,7 @@ struct rte_fdir_conf fdir_conf = { uint8_t slaves[RTE_MAX_ETHPORTS]; - int i, j, burst_size, slave_count; + int i, burst_size, slave_count; memset(pkt_burst, 0, sizeof(pkt_burst)); @@ -3984,16 +3960,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(); } @@ -4409,7 +4375,7 @@ struct rte_fdir_conf fdir_conf = { uint8_t slaves[RTE_MAX_ETHPORTS]; - int i, j, burst_size, slave_count, primary_port; + int i, burst_size, slave_count, primary_port; burst_size = 21; @@ -4527,18 +4493,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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2] test: Fix memory corruption issues which fails the link_bonding test. 2017-07-10 11:13 ` [dpdk-dev] [PATCH v2] " Herbert Guan @ 2017-07-12 10:23 ` Declan Doherty 2017-07-31 15:25 ` Ferruh Yigit 2017-07-18 16:13 ` Ferruh Yigit 1 sibling, 1 reply; 9+ messages in thread From: Declan Doherty @ 2017-07-12 10:23 UTC (permalink / raw) To: Herbert Guan, dev, jianbo.liu On 10/07/17 12:13, Herbert Guan wrote: > Patch V2: fix build warnings by deleting unused variables. > > There were double-free problems in some test cases, 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 <herbert.guan@arm.com> > --- ... > Acked-by: Declan Doherty <declan.doherty@intel.com> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2] test: Fix memory corruption issues which fails the link_bonding test. 2017-07-12 10:23 ` Declan Doherty @ 2017-07-31 15:25 ` Ferruh Yigit 0 siblings, 0 replies; 9+ messages in thread From: Ferruh Yigit @ 2017-07-31 15:25 UTC (permalink / raw) To: Declan Doherty, Herbert Guan, dev, jianbo.liu On 7/12/2017 11:23 AM, Declan Doherty wrote: > On 10/07/17 12:13, Herbert Guan wrote: >> Patch V2: fix build warnings by deleting unused variables. >> >> There were double-free problems in some test cases, 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 <herbert.guan@arm.com> >> --- > ... >> > > Acked-by: Declan Doherty <declan.doherty@intel.com> Applied to dpdk-next-net/master, thanks. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2] test: Fix memory corruption issues which fails the link_bonding test. 2017-07-10 11:13 ` [dpdk-dev] [PATCH v2] " Herbert Guan 2017-07-12 10:23 ` Declan Doherty @ 2017-07-18 16:13 ` Ferruh Yigit 2017-07-19 1:54 ` Herbert Guan 1 sibling, 1 reply; 9+ messages in thread From: Ferruh Yigit @ 2017-07-18 16:13 UTC (permalink / raw) To: Herbert Guan, dev, declan.doherty, jianbo.liu On 7/10/2017 12:13 PM, Herbert Guan wrote: > Patch V2: fix build warnings by deleting unused variables. > > There were double-free problems in some test cases, 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 <herbert.guan@arm.com> Updated unit test causing segfault, before and after patch, am I doing something wrong? I will postpone this patch until unit test can be verified. Thanks, ferruh ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2] test: Fix memory corruption issues which fails the link_bonding test. 2017-07-18 16:13 ` Ferruh Yigit @ 2017-07-19 1:54 ` Herbert Guan 0 siblings, 0 replies; 9+ messages in thread From: Herbert Guan @ 2017-07-19 1:54 UTC (permalink / raw) To: Ferruh Yigit, dev, declan.doherty, jianbo.liu It's caused by some recent merges. Applying this patch to 17.05 is okay. I've identified the place of segfault, but there're still some other problems to this test case. Thanks, Herbert -----Original Message----- From: Ferruh Yigit [mailto:ferruh.yigit@intel.com] Sent: Wednesday, July 19, 2017 0:14 To: Herbert Guan <Herbert.Guan@arm.com>; dev@dpdk.org; declan.doherty@intel.com; jianbo.liu@linaro.org Subject: Re: [dpdk-dev] [PATCH v2] test: Fix memory corruption issues which fails the link_bonding test. On 7/10/2017 12:13 PM, Herbert Guan wrote: > Patch V2: fix build warnings by deleting unused variables. > > There were double-free problems in some test cases, 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 <herbert.guan@arm.com> Updated unit test causing segfault, before and after patch, am I doing something wrong? I will postpone this patch until unit test can be verified. Thanks, ferruh IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] test: Fix memory corruption issues which fails the link_bonding test. 2017-07-10 7:20 [dpdk-dev] [PATCH] test: Fix memory corruption issues which fails the link_bonding test Herbert Guan 2017-07-10 7:55 ` Jianbo Liu 2017-07-10 11:13 ` [dpdk-dev] [PATCH v2] " Herbert Guan @ 2017-07-12 9:59 ` Declan Doherty 2017-07-12 10:16 ` Declan Doherty 2 siblings, 1 reply; 9+ messages in thread From: Declan Doherty @ 2017-07-12 9:59 UTC (permalink / raw) To: Herbert Guan, dev, jianbo.liu On 10/07/2017 8:20 AM, 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 <herbert.guan@arm.com> > --- ... > Hey Herbert, I'm seeing compilation warnings for unused variables when I apply this patch, otherwise these changes look good. CC test_link_bonding.o /home/declan/Development/dpdk-org/master/test/test/test_link_bonding.c:2407:9: error: unused variable 'j' [-Werror,-Wunused-variable] int i, j, burst_size, slave_count, primary_port; ^ /home/declan/Development/dpdk-org/master/test/test/test_link_bonding.c:3301:9: error: unused variable 'j' [-Werror,-Wunused-variable] int i, j, burst_size, slave_count; ^ /home/declan/Development/dpdk-org/master/test/test/test_link_bonding.c:3860:9: error: unused variable 'j' [-Werror,-Wunused-variable] int i, j, burst_size, slave_count; ^ /home/declan/Development/dpdk-org/master/test/test/test_link_bonding.c:4372:9: error: unused variable 'j' [-Werror,-Wunused-variable] int i, j, burst_size, slave_count, primary_port; ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] test: Fix memory corruption issues which fails the link_bonding test. 2017-07-12 9:59 ` [dpdk-dev] [PATCH] " Declan Doherty @ 2017-07-12 10:16 ` Declan Doherty 0 siblings, 0 replies; 9+ messages in thread From: Declan Doherty @ 2017-07-12 10:16 UTC (permalink / raw) To: Herbert Guan, dev, jianbo.liu On 12/07/17 10:59, Declan Doherty wrote: > On 10/07/2017 8:20 AM, Herbert Guan wrote: ... > > Hey Herbert, > > I'm seeing compilation warnings for unused variables when I apply this > patch, otherwise these changes look good. > > CC test_link_bonding.o > /home/declan/Development/dpdk-org/master/test/test/test_link_bonding.c:2407:9: > error: unused variable 'j' > [-Werror,-Wunused-variable] > int i, j, burst_size, slave_count, primary_port; > ^ > /home/declan/Development/dpdk-org/master/test/test/test_link_bonding.c:3301:9: > error: unused variable 'j' > [-Werror,-Wunused-variable] > int i, j, burst_size, slave_count; > ^ > /home/declan/Development/dpdk-org/master/test/test/test_link_bonding.c:3860:9: > error: unused variable 'j' > [-Werror,-Wunused-variable] > int i, j, burst_size, slave_count; > ^ > /home/declan/Development/dpdk-org/master/test/test/test_link_bonding.c:4372:9: > error: unused variable 'j' > [-Werror,-Wunused-variable] > int i, j, burst_size, slave_count, primary_port; > Apologies, my outlook mail filter missed your v2 patch, so you can ignore this. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-07-31 15:25 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-07-10 7:20 [dpdk-dev] [PATCH] test: Fix memory corruption issues which fails the link_bonding test Herbert Guan 2017-07-10 7:55 ` Jianbo Liu 2017-07-10 11:13 ` [dpdk-dev] [PATCH v2] " Herbert Guan 2017-07-12 10:23 ` Declan Doherty 2017-07-31 15:25 ` Ferruh Yigit 2017-07-18 16:13 ` Ferruh Yigit 2017-07-19 1:54 ` Herbert Guan 2017-07-12 9:59 ` [dpdk-dev] [PATCH] " Declan Doherty 2017-07-12 10:16 ` Declan Doherty
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).