DPDK patches and discussions
 help / color / mirror / Atom feed
From: Muhammad Bilal <m.bilal@emumba.com>
To: Sunil Kumar Kori <skori@marvell.com>
Cc: "declan.doherty@intel.com" <declan.doherty@intel.com>,
	 "tomasz.kantecki@intel.com" <tomasz.kantecki@intel.com>,
	 Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	jgrajcia@cisco.com,  vipin.varghese@intel.com
Subject: Re: [dpdk-dev] [EXT] [PATCH 1/5] examples/l2fwd-event: free resources in case of error
Date: Tue, 2 Jun 2020 17:27:01 +0500
Message-ID: <CAOFC0T3jT0+mZegRSYWLJ-b21CZwFk=G3pesOLFb-+XCmfLL+g@mail.gmail.com> (raw)
In-Reply-To: <BY5PR18MB3105DA2A2C084E4D488E1ABAB4B90@BY5PR18MB3105.namprd18.prod.outlook.com>

On Tue, May 19, 2020 at 2:35 PM Sunil Kumar Kori <skori@marvell.com> wrote:
>
> >-----Original Message-----
> >From: Muhammad Bilal <m.bilal@emumba.com>
> >Sent: Tuesday, May 19, 2020 2:25 PM
> >To: declan.doherty@intel.com; tomasz.kantecki@intel.com; Pavan Nikhilesh
> >Bhagavatula <pbhagavatula@marvell.com>; Sunil Kumar Kori
> ><skori@marvell.com>
> >Cc: dev@dpdk.org; Muhammad Bilal <m.bilal@emumba.com>
> >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 <m.bilal@emumba.com>
> >---
> > 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
>

  reply	other threads:[~2020-06-02 12:27 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-19  8:54 [dpdk-dev] " Muhammad Bilal
2020-05-19  8:54 ` [dpdk-dev] [PATCH 2/5] examples/l2fwd-jobstats: " Muhammad Bilal
2020-05-19  8:54 ` [dpdk-dev] [PATCH 3/5] examples/l2fwd-keepalive: " Muhammad Bilal
2020-05-19  8:54 ` [dpdk-dev] [PATCH 4/5] examples/l2fwd-cat: " Muhammad Bilal
2020-05-19  8:54 ` [dpdk-dev] [PATCH 5/5] examples/l2fwd-crypto: " Muhammad Bilal
2020-05-19  9:34 ` [dpdk-dev] [EXT] [PATCH 1/5] examples/l2fwd-event: " Sunil Kumar Kori
2020-06-02 12:27   ` Muhammad Bilal [this message]
2020-06-10 10:01     ` Sunil Kumar Kori
2020-06-15 12:00       ` Muhammad Bilal
2020-06-16  5:53         ` Sunil Kumar Kori
2020-06-16 10:14           ` Muhammad Bilal
2020-06-16 10:38             ` Sunil Kumar Kori
2020-06-16 11:47       ` Jakub Grajciar -X (jgrajcia - PANTHEON TECH SRO at Cisco)

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='CAOFC0T3jT0+mZegRSYWLJ-b21CZwFk=G3pesOLFb-+XCmfLL+g@mail.gmail.com' \
    --to=m.bilal@emumba.com \
    --cc=declan.doherty@intel.com \
    --cc=dev@dpdk.org \
    --cc=jgrajcia@cisco.com \
    --cc=pbhagavatula@marvell.com \
    --cc=skori@marvell.com \
    --cc=tomasz.kantecki@intel.com \
    --cc=vipin.varghese@intel.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

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git