From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 6E4B9A04FD; Tue, 2 Jun 2020 14:27:16 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 72D391BF7F; Tue, 2 Jun 2020 14:27:15 +0200 (CEST) Received: from mail-il1-f177.google.com (mail-il1-f177.google.com [209.85.166.177]) by dpdk.org (Postfix) with ESMTP id C85E61BF70 for ; Tue, 2 Jun 2020 14:27:13 +0200 (CEST) Received: by mail-il1-f177.google.com with SMTP id r2so12657999ila.4 for ; Tue, 02 Jun 2020 05:27:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=emumba-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Ro2wjoqrHJ7w7F9uvC+lhMhKMaEAHmpKiRYzgHOD/uc=; b=H3UEzRNyWNZ6RHTcaWuDX1EzOAZIrls0yUvblcZn3UvOCjNqpLrAz61xtkbetU9tR9 oGvpvDMnRhqgcd2jUSl/sZgoZwD/FpzEJUUd5l/35K02dKeMJrmbkXdUBvNVfuzWBM6+ K6bPbHgv1Qm5QPyl3eqE4ERqmao3yfgxtv0fo6Ev7BvZwhSPHmnuNYm9QETPHwHtBeKz 1LU36QIL14E/OaUw55J1B2MXveefWGFJt321s1Q7U/3kggTsWzoH+fyrQwXzFJTwdBB8 vHSgN1leC/Y4IpqtOOWiNUfNbDFlZsdaqZV0ymDuIfPabjcXTAndqCRG/Ma3Q0FtfGpU SrZQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Ro2wjoqrHJ7w7F9uvC+lhMhKMaEAHmpKiRYzgHOD/uc=; b=HAW93Nk0Ssv25YB6aCTtnzgqmvI1XLzbm/mMPf+SB086eKgEtJCCwLLgn5xAmq7KIY yhaOwFMfndDXPbNmJ7kcrfmviJQg5NqCbdXO4fGlN5aEaOB+yWkh1TOdHVm3ODaYESkO V92sXBI4x1mhyMomv1pHcZ6dKbD35l74FeHvVU2mU894i5fLwMA+HyCUXoYP1eClyXf2 SWE629pXkJkfkTmiW88/EOxtD6kL5E1SyapykHrKoaIYcUmzFm4Kf7kol5oNKo8egwF+ f+2ZxOAuACcns91UoKXbt87zbWJ7XVC7DbEVqfjg+OJE9KFJWFHTA6WEzRBfHYDme0Kl 7Ajg== X-Gm-Message-State: AOAM533nCbhTJ+uAtILVRWKkKBhJ5vO2vNLftBRQgwe+vOysWraGVMKg 0cxwKbzgFxq5yq4D87me4BkwCaVC10XuKwnJSy6s9Q== X-Google-Smtp-Source: ABdhPJx+l+4l1mYcTv8qbFBpPb+gJT4uu8PWfEILAokbxWmNkXHscEQ5zlwET7k+ZOnhVHIPE98JJpQrp7WQ5Hj+e24= X-Received: by 2002:a92:5dd2:: with SMTP id e79mr23387517ilg.94.1591100832863; Tue, 02 Jun 2020 05:27:12 -0700 (PDT) MIME-Version: 1.0 References: <20200519085444.4562-1-m.bilal@emumba.com> In-Reply-To: From: Muhammad Bilal Date: Tue, 2 Jun 2020 17:27:01 +0500 Message-ID: To: Sunil Kumar Kori Cc: "declan.doherty@intel.com" , "tomasz.kantecki@intel.com" , Pavan Nikhilesh Bhagavatula , "dev@dpdk.org" , jgrajcia@cisco.com, vipin.varghese@intel.com Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [EXT] [PATCH 1/5] examples/l2fwd-event: free resources in case of error X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list 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 Tue, May 19, 2020 at 2:35 PM Sunil Kumar Kori wrote: > > >-----Original Message----- > >From: Muhammad Bilal > >Sent: Tuesday, May 19, 2020 2:25 PM > >To: declan.doherty@intel.com; tomasz.kantecki@intel.com; Pavan Nikhilesh > >Bhagavatula ; Sunil Kumar Kori > > > >Cc: dev@dpdk.org; Muhammad Bilal > >Subject: [EXT] [PATCH 1/5] examples/l2fwd-event: free resources in case of > >error > > > >External Email > > > >---------------------------------------------------------------------- > >Freeing the resources and call rte_eal_cleanup in case of error exit. > >Signed-off-by: Muhammad Bilal > >--- > > examples/l2fwd-event/main.c | 43 ++++++++++++++++++++++++++++++------- > > 1 file changed, 35 insertions(+), 8 deletions(-) > > > >diff --git a/examples/l2fwd-event/main.c b/examples/l2fwd-event/main.c > >index 9593ef11e..442a664e9 100644 > >--- a/examples/l2fwd-event/main.c > >+++ b/examples/l2fwd-event/main.c > >@@ -556,13 +556,26 @@ signal_handler(int signum) > > } > > } > > > >+static void > >+stop_and_close_eth_dev(uint16_t portid) { > >+ RTE_ETH_FOREACH_DEV(portid) { > >+ printf("Closing port %d...", portid); > >+ rte_eth_dev_stop(portid); > >+ rte_eth_dev_close(portid); > >+ printf(" Done\n"); > >+ } > >+ rte_eal_cleanup(); > >+} > >+ > > int > > main(int argc, char **argv) > > { > > struct l2fwd_resources *rsrc; > > uint16_t nb_ports_available = 0; > > uint32_t nb_ports_in_mask = 0; > >- uint16_t port_id, last_port; > >+ uint16_t port_id = 0; > >+ uint16_t last_port; > > uint32_t nb_mbufs; > > uint16_t nb_ports; > > int i, ret; > >@@ -581,20 +594,26 @@ main(int argc, char **argv) > > > > /* parse application arguments (after the EAL ones) */ > > ret = l2fwd_event_parse_args(argc, argv, rsrc); > >- if (ret < 0) > >+ if (ret < 0) { > >+ stop_and_close_eth_dev(port_id); > > rte_panic("Invalid L2FWD arguments\n"); > >+ } > > Yes you are right we should use rte_exit instead of rte_panic, as rte_exit internally calls rte_eal_cleanup function. > IMO, up to this point only eal_init is done so rte_eal_cleanup will be sufficient for this. > Also another way to handle this, use rte_exit instead rte_panic. rte_exit internally calls > rte_eal_cleanup. Refer l2fwd. > > Also I think, it is better to release the relevant resources on error. Here I'm solving the problem reported in bugzilla id 437. The bug was that if we use --vdev=net_memif with l2fwd application (and with its other variants) then a socket is created by memif PMD, after rte_eal_init function has run successfully. And if an error occurs then the application exits without freeing the resources (socket). On running the application 2nd time we get an error of "socket already exists". As in the previous version of patch " http://patches.dpdk.org/patch/70081/ " it was recommended to clean the resources when an error occurs. Here only using rte_eal_cleanup() is not solving the problem as using memif PMD is creating a socket and it's only cleaned when rte_eth_dev_close(portid) function is called. so that's why using it along with rte_exit or rte_panic. > > > printf("MAC updating %s\n", rsrc->mac_updating ? "enabled" : > > "disabled"); > > > > nb_ports = rte_eth_dev_count_avail(); > >- if (nb_ports == 0) > >+ if (nb_ports == 0) { > >+ stop_and_close_eth_dev(port_id); > > rte_panic("No Ethernet ports - bye\n"); > >+ } > > > Same as above. > > > /* check port mask to possible port mask */ > >- if (rsrc->enabled_port_mask & ~((1 << nb_ports) - 1)) > >+ if (rsrc->enabled_port_mask & ~((1 << nb_ports) - 1)) { > >+ stop_and_close_eth_dev(port_id); > > rte_panic("Invalid portmask; possible (0x%x)\n", > > (1 << nb_ports) - 1); > >+ } > > > Same as above. > > > if (!rsrc->port_pairs) { > > last_port = 0; > >@@ -621,8 +640,10 @@ main(int argc, char **argv) > > rsrc->dst_ports[last_port] = last_port; > > } > > } else { > >- if (check_port_pair_config(rsrc) < 0) > >+ if (check_port_pair_config(rsrc) < 0) { > >+ stop_and_close_eth_dev(port_id); > > rte_panic("Invalid port pair config\n"); > >+ } > > } > > > > nb_mbufs = RTE_MAX(nb_ports * (RTE_TEST_RX_DESC_DEFAULT + > >@@ -634,12 +655,16 @@ main(int argc, char **argv) > > rsrc->pktmbuf_pool = rte_pktmbuf_pool_create("mbuf_pool", > > nb_mbufs, MEMPOOL_CACHE_SIZE, 0, > > RTE_MBUF_DEFAULT_BUF_SIZE, rte_socket_id()); > >- if (rsrc->pktmbuf_pool == NULL) > >+ if (rsrc->pktmbuf_pool == NULL) { > >+ stop_and_close_eth_dev(port_id); > > rte_panic("Cannot init mbuf pool\n"); > >+ } > > > > nb_ports_available = l2fwd_event_init_ports(rsrc); > >- if (!nb_ports_available) > >+ if (!nb_ports_available) { > >+ stop_and_close_eth_dev(port_id); > > rte_panic("All available ports are disabled. Please set > >portmask.\n"); > >+ } > > > > /* Configure eventdev parameters if required */ > > if (rsrc->event_mode) > >@@ -659,9 +684,11 @@ main(int argc, char **argv) > > continue; > > > > ret = rte_eth_dev_start(port_id); > >- if (ret < 0) > >+ if (ret < 0) { > >+ stop_and_close_eth_dev(port_id); > > rte_panic("rte_eth_dev_start:err=%d, port=%u\n", > >ret, > > port_id); > >+ } > > } > > > > if (rsrc->event_mode) > >-- > >2.17.1 >