diff mbox

[dpdk-dev,v2,9/9] examples/l3fwd-power: fix not stop and close device

Message ID 1482996643-113253-10-git-send-email-jianfeng.tan@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Yuanhan Liu
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel compilation fail Compilation issues

Commit Message

Jianfeng Tan Dec. 29, 2016, 7:30 a.m. UTC 
  As it gets killed, in SIGINT signal handler, device is not stopped
and closed. In virtio's case, vector assignment in the KVM is not
deassigned.

This patch will invoke dev_stop() and dev_close() in signal handler.

Fixes: d7937e2e3d12 ("power: initial import")

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 examples/l3fwd-power/main.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Yuanhan Liu Dec. 30, 2016, 6:44 a.m. UTC | #1 
On Thu, Dec 29, 2016 at 07:30:43AM +0000, Jianfeng Tan wrote:
> As it gets killed, in SIGINT signal handler, device is not stopped
> and closed. In virtio's case, vector assignment in the KVM is not
> deassigned.

What wrong could happen then?

> This patch will invoke dev_stop() and dev_close() in signal handler.

I will just say, it may workaround the bug you encountered, particulary,
for this example only. If some people want to use the virtio interrupt
somewhere at another app, he also has to do similar thing.

Is there a more clean way to handle such case in the driver?

	--yliu
Jianfeng Tan Dec. 30, 2016, 6:56 a.m. UTC | #2 
> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Friday, December 30, 2016 2:45 PM
> To: Tan, Jianfeng
> Cc: dev@dpdk.org; stephen@networkplumber.org
> Subject: Re: [PATCH v2 9/9] examples/l3fwd-power: fix not stop and close
> device
> 
> On Thu, Dec 29, 2016 at 07:30:43AM +0000, Jianfeng Tan wrote:
> > As it gets killed, in SIGINT signal handler, device is not stopped
> > and closed. In virtio's case, vector assignment in the KVM is not
> > deassigned.
> 
> What wrong could happen then?

Actually, no strange things happen so far. My purpose for this patch is to verify that irqfd is deassigned.
And as calling dev_stop() and dev_close() is a good practice, so I use the word "fix" here.

> 
> > This patch will invoke dev_stop() and dev_close() in signal handler.
> 
> I will just say, it may workaround the bug you encountered, particulary,
> for this example only.

There's no bug to work around.

> If some people want to use the virtio interrupt
> somewhere at another app, he also has to do similar thing.

Vfio-pci is the best place to put device into original state, but is it really necessary to do that? If another program takes over that device, it will be re-initialized.

> 
> Is there a more clean way to handle such case in the driver?

Let's do with the necessity firstly.

Thanks,
Jianfeng

> 
> 	--yliu
diff mbox

Patch

diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index 44843ec..63e1796 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -379,6 +379,7 @@  static void
 signal_exit_now(int sigtype)
 {
 	unsigned lcore_id;
+	unsigned int portid, nb_ports;
 	int ret;
 
 	if (sigtype == SIGINT) {
@@ -393,6 +394,15 @@  signal_exit_now(int sigtype)
 					"library de-initialization failed on "
 							"core%u\n", lcore_id);
 		}
+
+		nb_ports = rte_eth_dev_count();
+		for (portid = 0; portid < nb_ports; portid++) {
+			if ((enabled_port_mask & (1 << portid)) == 0)
+				continue;
+
+			rte_eth_dev_stop(portid);
+			rte_eth_dev_close(portid);
+		}
 	}
 
 	rte_exit(EXIT_SUCCESS, "User forced exit\n");