app/testpmd: avoid exit without resource release

PATCH In review [email protected] 5 participants 14 messages Dec 24, 2020 – Jan 29, 2021

lore

Share & tools

lore

[email protected]

author

2020-12-24 03:58 UTC #0 lore raw

PATCH 1/1 app/testpmd: avoid exit without resource release

From: YU DAPENG <[email protected]>

In interactive mode, if testpmd exit by calling rte_exit without cmdline resource release, terminal will not echo keyboard input. So add code to just show error message, but not exit testpmd when unexpected happens on starting packet forwarding in interactive mode. User can type "quit" to exit testpmd later.

Fixes 5a8fb55c48ab ("app/testpmd: support unidirectional configuration")

Cc [email protected]

Signed-off-by YU DAPENG <[email protected]>

---
 app/test-pmd/testpmd.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

app/test-pmd/testpmd.c +9 -1

index 33fc0fddf..0071c7235 100644
@@ -2194,10 +2194,18 @@ start_packet_forwarding(int with_tx_first)
 
 	if ((strcmp(cur_fwd_eng->fwd_mode_name, "rxonly") != 0 &&
 		strcmp(cur_fwd_eng->fwd_mode_name, "txonly") != 0) &&
-		(!nb_rxq || !nb_txq))
+		(!nb_rxq || !nb_txq)) {
+#ifdef RTE_LIB_CMDLINE
+		if (interactive == 1) {
+			printf("Either rxq or txq are 0, cannot use %s fwd mode\n",
+				cur_fwd_eng->fwd_mode_name);
+			return;
+		}
+#endif
 		rte_exit(EXIT_FAILURE,
 			"Either rxq or txq are 0, cannot use %s fwd mode\n",
 			cur_fwd_eng->fwd_mode_name);
+	}
 
 	if (all_ports_started() == 0) {
 		printf("Not all ports were started\n");

Signature · 1 line

2.27.0

Stephen Hemminger

2020-12-25 03:04 UTC #1 lore raw

Quoted 8 lines · On Thu, 24 Dec 2020 11:57:48 +0800 [email protected] wrote:

From: YU DAPENG <[email protected]>
In interactive mode, if testpmd exit by calling rte_exit without cmdline resource release, terminal will not echo keyboard input. So add code to just show error message, but not exit testpmd when unexpected happens on starting packet forwarding in interactive mode. User can type "quit" to exit testpmd later.

Quoted 4 lines

Fixes 5a8fb55c48ab ("app/testpmd: support unidirectional configuration")
Cc [email protected]
Signed-off-by YU DAPENG <[email protected]>

Sounds like a more generic problem with rte_exit and librte_cmdline. Would it better to fix it in librte_cmdline by adding an atexit() handler.

Yu, DapengX

author

2020-12-25 05:09 UTC #2 lore raw

Hi Stephen,

Do you mean this solution?
1. support atexit() in librte_eal, other component can use it to register a function to be called when rte_exit() is called.
2. in librte_cmdline, use atexit() to register a function to release resource

I am looking forward to more comments from other maintainers, since this solution will modify librte_eal and librte_cmdline, but not just testpmd app.

-----Original Message-----
From: Stephen Hemminger [mailto:[email protected]]
Sent: Friday, December 25, 2020 11:03 AM
To: Yu, DapengX <[email protected]>
Cc: Lu, Wenzhuo <[email protected]>; Xing, Beilei <[email protected]>; Iremonger, Bernard <[email protected]>; [email protected]; [email protected]
Subject: Re: [dpdk-dev] [PATCH] app/testpmd: avoid exit without resource release

Quoted 8 lines · On Thu, 24 Dec 2020 11:57:48 +0800 [email protected] wrote:

From: YU DAPENG <[email protected]>
In interactive mode, if testpmd exit by calling rte_exit without
cmdline resource release, terminal will not echo keyboard input. So
add code to just show error message, but not exit testpmd when
unexpected happens on starting packet forwarding in interactive mode. User can type "quit" to exit testpmd later.

Quoted 5 lines

Fixes 5a8fb55c48ab ("app/testpmd: support unidirectional
configuration")
Cc: [email protected]
Signed-off-by YU DAPENG <[email protected]>

Sounds like a more generic problem with rte_exit and librte_cmdline. Would it better to fix it in librte_cmdline by adding an atexit() handler.

Yu, DapengX

author

2021-01-15 03:28 UTC #3 lore raw

Hi Wenzhuo, Beilei, Bernard

I need testpmd app maintainers' comment :
Do you prefer the simple solution in this patch or Stephen's comprehensive and generic solution(which will modify librte_cmdline, testpmd (and maybe rte_exit()))?
So I can continue resolving the defect.

Thanks!

-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Yu, DapengX
Sent: Friday, December 25, 2020 1:09 PM
To: Stephen Hemminger <[email protected]>
Cc: Lu, Wenzhuo <[email protected]>; Xing, Beilei <[email protected]>; Iremonger, Bernard <[email protected]>; [email protected]; [email protected]
Subject: Re: [dpdk-dev] [PATCH] app/testpmd: avoid exit without resource release

Hi Stephen,

I am looking forward to more comments from other maintainers, since this solution will modify librte_eal and librte_cmdline, but not just testpmd app.

Quoted 8 lines · On Thu, 24 Dec 2020 11:57:48 +0800 [email protected] wrote:

From: YU DAPENG <[email protected]>
In interactive mode, if testpmd exit by calling rte_exit without
cmdline resource release, terminal will not echo keyboard input. So
add code to just show error message, but not exit testpmd when
unexpected happens on starting packet forwarding in interactive mode. User can type "quit" to exit testpmd later.

Quoted 5 lines

Fixes 5a8fb55c48ab ("app/testpmd: support unidirectional
configuration")
Cc: [email protected]
Signed-off-by YU DAPENG <[email protected]>

Sounds like a more generic problem with rte_exit and librte_cmdline. Would it better to fix it in librte_cmdline by adding an atexit() handler.

Xing, Beilei

2021-01-15 05:50 UTC #4 lore raw

Quoted 10 lines

-----Original Message-----
From: Yu, DapengX <[email protected]>
Sent: Friday, January 15, 2021 11:29 AM
To: Lu, Wenzhuo <[email protected]>; Xing, Beilei <[email protected]>; Iremonger, Bernard <[email protected]>
Cc: Yu, DapengX <[email protected]>; Stephen Hemminger <[email protected]>; [email protected]; [email protected]
Subject: RE: [dpdk-dev] [PATCH] app/testpmd: avoid exit without resource release

Quoted 6 lines

Hi Wenzhuo, Beilei, Bernard
I need testpmd app maintainers' comment :
Do you prefer the simple solution in this patch or Stephen's comprehensive
and generic solution(which will modify librte_cmdline, testpmd (and maybe rte_exit()))?

The patch just workarounds a specific case, the issue exits in many cases, such as Rxonly mode and Rxq is 0. So It's better to investigate how to solve the generic problem according to Stephen's comments.

Quoted 56 lines

So I can continue resolving the defect.
Thanks!
-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Yu, DapengX
Sent: Friday, December 25, 2020 1:09 PM
To: Stephen Hemminger <[email protected]>
Cc: Lu, Wenzhuo <[email protected]>; Xing, Beilei <[email protected]>; Iremonger, Bernard <[email protected]>; [email protected]; [email protected]
Subject: Re: [dpdk-dev] [PATCH] app/testpmd: avoid exit without resource release
Hi Stephen,
Do you mean this solution?
1. support atexit() in librte_eal, other component can use it to register a function to be called when rte_exit() is called.
2. in librte_cmdline, use atexit() to register a function to release resource
I am looking forward to more comments from other maintainers, since this solution will modify librte_eal and librte_cmdline, but not just testpmd app.
-----Original Message-----
From: Stephen Hemminger [mailto:[email protected]]
Sent: Friday, December 25, 2020 11:03 AM
To: Yu, DapengX <[email protected]>
Cc: Lu, Wenzhuo <[email protected]>; Xing, Beilei <[email protected]>; Iremonger, Bernard <[email protected]>; [email protected]; [email protected]
Subject: Re: [dpdk-dev] [PATCH] app/testpmd: avoid exit without resource release
Quoted 6 lines · On Thu, 24 Dec 2020 11:57:48 +0800 [email protected] wrote:
From: YU DAPENG <[email protected]>
In interactive mode, if testpmd exit by calling rte_exit without cmdline resource release, terminal will not echo keyboard input. So add code to just show error message, but not exit testpmd when unexpected happens on starting packet forwarding in interactive mode. User
can type "quit"
Quoted 7 lines
to exit testpmd later.
Fixes 5a8fb55c48ab ("app/testpmd: support unidirectional
configuration")
Cc: [email protected]
Signed-off-by YU DAPENG <[email protected]>
Sounds like a more generic problem with rte_exit and librte_cmdline. Would it better to fix it in librte_cmdline by adding an atexit() handler.

Yu, DapengX

author

2021-01-15 06:05 UTC #5 lore raw

Hi Beilei,

Thanks for your comments, I will prepare patch v2.

-----Original Message-----
From: Xing, Beilei
Sent: Friday, January 15, 2021 1:50 PM
To: Yu, DapengX <[email protected]>; Lu, Wenzhuo <[email protected]>; Iremonger, Bernard <[email protected]>
Cc: Stephen Hemminger <[email protected]>; [email protected]; [email protected]
Subject: RE: [dpdk-dev] [PATCH] app/testpmd: avoid exit without resource release

Quoted 11 lines

-----Original Message-----
From: Yu, DapengX <[email protected]>
Sent: Friday, January 15, 2021 11:29 AM
To: Lu, Wenzhuo <[email protected]>; Xing, Beilei <[email protected]>; Iremonger, Bernard <[email protected]>
Cc: Yu, DapengX <[email protected]>; Stephen Hemminger <[email protected]>; [email protected]; [email protected]
Subject: RE: [dpdk-dev] [PATCH] app/testpmd: avoid exit without resource release

Quoted 6 lines

Hi Wenzhuo, Beilei, Bernard
I need testpmd app maintainers' comment :
Do you prefer the simple solution in this patch or Stephen's
comprehensive and generic solution(which will modify librte_cmdline, testpmd (and maybe rte_exit()))?

Quoted 58 lines

So I can continue resolving the defect.
Thanks!
-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Yu, DapengX
Sent: Friday, December 25, 2020 1:09 PM
To: Stephen Hemminger <[email protected]>
Cc: Lu, Wenzhuo <[email protected]>; Xing, Beilei <[email protected]>; Iremonger, Bernard <[email protected]>; [email protected]; [email protected]
Subject: Re: [dpdk-dev] [PATCH] app/testpmd: avoid exit without resource release
Hi Stephen,
Do you mean this solution?
1. support atexit() in librte_eal, other component can use it to register a function to be called when rte_exit() is called.
2. in librte_cmdline, use atexit() to register a function to release resource
I am looking forward to more comments from other maintainers, since this solution will modify librte_eal and librte_cmdline, but not just testpmd app.
-----Original Message-----
From: Stephen Hemminger [mailto:[email protected]]
Sent: Friday, December 25, 2020 11:03 AM
To: Yu, DapengX <[email protected]>
Cc: Lu, Wenzhuo <[email protected]>; Xing, Beilei <[email protected]>; Iremonger, Bernard <[email protected]>; [email protected]; [email protected]
Subject: Re: [dpdk-dev] [PATCH] app/testpmd: avoid exit without resource release
Quoted 7 lines · On Thu, 24 Dec 2020 11:57:48 +0800 [email protected] wrote:
From: YU DAPENG <[email protected]>
In interactive mode, if testpmd exit by calling rte_exit without cmdline resource release, terminal will not echo keyboard input. So add code to just show error message, but not exit testpmd when unexpected happens on starting packet forwarding in interactive mode. User
can type "quit"
Quoted 7 lines
to exit testpmd later.
Fixes 5a8fb55c48ab ("app/testpmd: support unidirectional
configuration")
Cc: [email protected]
Signed-off-by YU DAPENG <[email protected]>
Sounds like a more generic problem with rte_exit and librte_cmdline. Would it better to fix it in librte_cmdline by adding an atexit() handler.

Li, Xiaoyun

2021-01-15 09:21 UTC #6 lore raw

In reply to Xing, Beilei

Hi
Actually, you can just type "stty echo" to solve the problem you met.

But anyway, if you want to fix it in DPDK
I think you misunderstood stephan's solution.
You don't need to implement atexit() yourself in eal. It's a common function like exit() in rte_exit(). Then when exit() is called. Each function registered by atexit() will be called in stack order.

The simplest way to solve this problem is to register a function with atexit() in prompt() in each app which uses cmdline. Then when exit() in rte_exit() is called, the registered clean function will be called directly.

Registering in prompt() is because that prompt() is the function which starts cmdline in each app.

@Stephen Hemminger What do you think of this about atexit() in prompt() in each app?
I understand it's cmdline and rte_exit problem. But atexit needs a function without parameters.
And the cleanup for cmdline is cmdline_stdin_exit(struct cmdline *cl) which needs cmdline as the parameter to restore original terminal setting. Setting a global value will be too ugly to me.

BRs
Xiaoyun

Quoted 91 lines

-----Original Message-----
From: stable <[email protected]> On Behalf Of Xing, Beilei
Sent: Friday, January 15, 2021 13:50
To: Yu, DapengX <[email protected]>; Lu, Wenzhuo <[email protected]>; Iremonger, Bernard <[email protected]>
Cc: Stephen Hemminger <[email protected]>; [email protected]; [email protected]
Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH] app/testpmd: avoid exit without resource release
Quoted 11 lines
-----Original Message-----
From: Yu, DapengX <[email protected]>
Sent: Friday, January 15, 2021 11:29 AM
To: Lu, Wenzhuo <[email protected]>; Xing, Beilei <[email protected]>; Iremonger, Bernard <[email protected]>
Cc: Yu, DapengX <[email protected]>; Stephen Hemminger <[email protected]>; [email protected]; [email protected]
Subject: RE: [dpdk-dev] [PATCH] app/testpmd: avoid exit without resource release
Quoted 6 lines
Hi Wenzhuo, Beilei, Bernard
I need testpmd app maintainers' comment :
Do you prefer the simple solution in this patch or Stephen's
comprehensive and generic solution(which will modify librte_cmdline, testpmd (and maybe rte_exit()))?
The patch just workarounds a specific case, the issue exits in many cases, such as Rxonly mode and Rxq is 0.
So It's better to investigate how to solve the generic problem according to Stephen's comments.
Quoted 58 lines
So I can continue resolving the defect.
Thanks!
-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Yu, DapengX
Sent: Friday, December 25, 2020 1:09 PM
To: Stephen Hemminger <[email protected]>
Cc: Lu, Wenzhuo <[email protected]>; Xing, Beilei <[email protected]>; Iremonger, Bernard <[email protected]>; [email protected]; [email protected]
Subject: Re: [dpdk-dev] [PATCH] app/testpmd: avoid exit without resource release
Hi Stephen,
Do you mean this solution?
1. support atexit() in librte_eal, other component can use it to register a function to be called when rte_exit() is called.
2. in librte_cmdline, use atexit() to register a function to release resource
I am looking forward to more comments from other maintainers, since this solution will modify librte_eal and librte_cmdline, but not just testpmd app.
-----Original Message-----
From: Stephen Hemminger [mailto:[email protected]]
Sent: Friday, December 25, 2020 11:03 AM
To: Yu, DapengX <[email protected]>
Cc: Lu, Wenzhuo <[email protected]>; Xing, Beilei <[email protected]>; Iremonger, Bernard <[email protected]>; [email protected]; [email protected]
Subject: Re: [dpdk-dev] [PATCH] app/testpmd: avoid exit without resource release
Quoted 7 lines · On Thu, 24 Dec 2020 11:57:48 +0800 [email protected] wrote:
From: YU DAPENG <[email protected]>
In interactive mode, if testpmd exit by calling rte_exit without cmdline resource release, terminal will not echo keyboard input. So add code to just show error message, but not exit testpmd when unexpected happens on starting packet forwarding in interactive mode. User
can type "quit"
Quoted 7 lines
to exit testpmd later.
Fixes 5a8fb55c48ab ("app/testpmd: support unidirectional
configuration")
Cc: [email protected]
Signed-off-by YU DAPENG <[email protected]>
Sounds like a more generic problem with rte_exit and librte_cmdline. Would it better to fix it in librte_cmdline by adding an atexit() handler.

Yu, DapengX

author

2021-01-18 11:47 UTC #7 lore raw

@Stephen Hemminger
Xiaoyun proposed a solution which may match your idea, but there are some concerns:
1. a global variable for original terminal setting has to be defined.
2. in each app which uses cmdline, a function has to be registered with atexit() in prompt() to restore original terminal setting.

@Stephen Hemminger, can you give some feedback on these concerns? Thanks!

-----Original Message-----
From: Li, Xiaoyun
Sent: Friday, January 15, 2021 5:22 PM
To: Xing, Beilei <[email protected]>; Yu, DapengX <[email protected]>; Lu, Wenzhuo <[email protected]>; Iremonger, Bernard <[email protected]>; Stephen Hemminger <[email protected]>
Cc: [email protected]; [email protected]
Subject: RE: [dpdk-dev] [PATCH] app/testpmd: avoid exit without resource release

Hi
Actually, you can just type "stty echo" to solve the problem you met.

But anyway, if you want to fix it in DPDK I think you misunderstood stephan's solution. You don't need to implement atexit() yourself in eal. It's a common function like exit() in rte_exit(). Then when exit() is called. Each function registered by atexit() will be called in stack order.

Registering in prompt() is because that prompt() is the function which starts cmdline in each app.

BRs
Xiaoyun

Quoted 92 lines

-----Original Message-----
From: stable <[email protected]> On Behalf Of Xing, Beilei
Sent: Friday, January 15, 2021 13:50
To: Yu, DapengX <[email protected]>; Lu, Wenzhuo <[email protected]>; Iremonger, Bernard <[email protected]>
Cc: Stephen Hemminger <[email protected]>; [email protected]; [email protected]
Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH] app/testpmd: avoid exit without resource release
Quoted 11 lines
-----Original Message-----
From: Yu, DapengX <[email protected]>
Sent: Friday, January 15, 2021 11:29 AM
To: Lu, Wenzhuo <[email protected]>; Xing, Beilei <[email protected]>; Iremonger, Bernard <[email protected]>
Cc: Yu, DapengX <[email protected]>; Stephen Hemminger <[email protected]>; [email protected]; [email protected]
Subject: RE: [dpdk-dev] [PATCH] app/testpmd: avoid exit without resource release
Quoted 6 lines
Hi Wenzhuo, Beilei, Bernard
I need testpmd app maintainers' comment :
Do you prefer the simple solution in this patch or Stephen's
comprehensive and generic solution(which will modify librte_cmdline, testpmd (and maybe rte_exit()))?
The patch just workarounds a specific case, the issue exits in many cases, such as Rxonly mode and Rxq is 0.
So It's better to investigate how to solve the generic problem according to Stephen's comments.
Quoted 58 lines
So I can continue resolving the defect.
Thanks!
-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Yu, DapengX
Sent: Friday, December 25, 2020 1:09 PM
To: Stephen Hemminger <[email protected]>
Cc: Lu, Wenzhuo <[email protected]>; Xing, Beilei <[email protected]>; Iremonger, Bernard <[email protected]>; [email protected]; [email protected]
Subject: Re: [dpdk-dev] [PATCH] app/testpmd: avoid exit without resource release
Hi Stephen,
Do you mean this solution?
1. support atexit() in librte_eal, other component can use it to register a function to be called when rte_exit() is called.
2. in librte_cmdline, use atexit() to register a function to release resource
I am looking forward to more comments from other maintainers, since this solution will modify librte_eal and librte_cmdline, but not just testpmd app.
-----Original Message-----
From: Stephen Hemminger [mailto:[email protected]]
Sent: Friday, December 25, 2020 11:03 AM
To: Yu, DapengX <[email protected]>
Cc: Lu, Wenzhuo <[email protected]>; Xing, Beilei <[email protected]>; Iremonger, Bernard <[email protected]>; [email protected]; [email protected]
Subject: Re: [dpdk-dev] [PATCH] app/testpmd: avoid exit without resource release
Quoted 7 lines · On Thu, 24 Dec 2020 11:57:48 +0800 [email protected] wrote:
From: YU DAPENG <[email protected]>
In interactive mode, if testpmd exit by calling rte_exit without cmdline resource release, terminal will not echo keyboard input. So add code to just show error message, but not exit testpmd when unexpected happens on starting packet forwarding in interactive mode. User
can type "quit"
Quoted 7 lines
to exit testpmd later.
Fixes 5a8fb55c48ab ("app/testpmd: support unidirectional
configuration")
Cc: [email protected]
Signed-off-by YU DAPENG <[email protected]>
Sounds like a more generic problem with rte_exit and librte_cmdline. Would it better to fix it in librte_cmdline by adding an atexit() handler.

[email protected]

author

2021-01-25 03:30 UTC #8 lore raw

PATCH v2 1/1 app/testpmd: avoid exit without terminal restore

In reply to [email protected]

From: Dapeng Yu <[email protected]>

In interactive mode, if testpmd exit by calling rte_exit without restore terminal attributes, terminal will not echo keyboard input.

register a function with atexit() in prompt(), when exit() in rte_exit() is called, the registered function restores terminal attributes.

Fixes 5a8fb55c48ab ("app/testpmd: support unidirectional configuration")

Cc [email protected]

Signed-off-by Dapeng Yu <[email protected]>

---
 app/test-pmd/cmdline.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

app/test-pmd/cmdline.c +11 -2

index 89034c8b7..f7e18ba3d 100644
@@ -17116,6 +17116,7 @@ cmdline_read_from_file(const char *filename)
 void
 prompt(void)
 {
+	int ret;
 	/* initialize non-constant commands */
 	cmd_set_fwd_mode_init();
 	cmd_set_fwd_retry_mode_init();

@@ -17123,15 +17124,23 @@ prompt(void)
 	testpmd_cl = cmdline_stdin_new(main_ctx, "testpmd> ");
 	if (testpmd_cl == NULL)
 		return;
+
+	ret = atexit(prompt_exit);
+	if (ret != 0)
+		printf("Cannot set exit function for cmdline\n");
+
 	cmdline_interact(testpmd_cl);
-	cmdline_stdin_exit(testpmd_cl);
+	if (ret != 0)
+		cmdline_stdin_exit(testpmd_cl);
 }
 
 void
 prompt_exit(void)
 {
-	if (testpmd_cl != NULL)
+	if (testpmd_cl != NULL) {
 		cmdline_quit(testpmd_cl);
+		cmdline_stdin_exit(testpmd_cl);
+	}
 }
 
 static void

Signature · 1 line

2.27.0

Li, Xiaoyun

2021-01-26 06:33 UTC #9 lore raw

Re: [dpdk-dev] [PATCH v2] app/testpmd: avoid exit without terminal restore

Quoted 39 lines

-----Original Message-----
From: [email protected] <[email protected]>
Sent: Monday, January 25, 2021 11:30
To: Xing, Beilei <[email protected]>; Li, Xiaoyun <[email protected]>; Lu, Wenzhuo <[email protected]>; Iremonger, Bernard <[email protected]>; [email protected]
Cc: [email protected]; Yu, DapengX <[email protected]>; [email protected]
Subject: [PATCH v2] app/testpmd: avoid exit without terminal restore
From: Dapeng Yu <[email protected]>
In interactive mode, if testpmd exit by calling rte_exit without restore terminal attributes, terminal will not echo keyboard input.
register a function with atexit() in prompt(), when exit() in rte_exit() is called, the registered function restores terminal attributes.
Fixes 5a8fb55c48ab ("app/testpmd: support unidirectional configuration")
Cc [email protected]
Signed-off-by Dapeng Yu <[email protected]>
---
 app/test-pmd/cmdline.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
app/test-pmd/cmdline.c index renamed from app/test-pmd/cmdline.c +0 -0
89034c8b7..f7e18ba3d 100644
app/test-pmd/cmdline.c +0 -0
@@ -17116,6 +17116,7 @@ cmdline_read_from_file(const char *filename)
void
prompt(void)
{
app/test-pmd/cmdline.c +1 -0
+	int ret;
 	/* initialize non-constant commands */
 	cmd_set_fwd_mode_init();
 	cmd_set_fwd_retry_mode_init();
@@ -17123,15 +17124,23 @@ prompt(void)
 	testpmd_cl = cmdline_stdin_new(main_ctx, "testpmd> ");

app/test-pmd/cmdline.c prompt(void) · line 17133

Quoted 10 lines

app/test-pmd/cmdline.c +6 -1

 	if (testpmd_cl == NULL)
 		return;
+
+	ret = atexit(prompt_exit);
+	if (ret != 0)
+		printf("Cannot set exit function for cmdline\n");
+
 	cmdline_interact(testpmd_cl);
-	cmdline_stdin_exit(testpmd_cl);
+	if (ret != 0)

Why do you need to check ret here?
Even if registers prompt_exit failed, when users type "quit" and then break from cmdline_interact(), cmdline_stdin_exit() should be called to restore terminal settings.

Quoted 16 lines

app/test-pmd/cmdline.c +4 -2

+		cmdline_stdin_exit(testpmd_cl);
 }
 
 void
 prompt_exit(void)
 {
-	if (testpmd_cl != NULL)
+	if (testpmd_cl != NULL) {
 		cmdline_quit(testpmd_cl);
+		cmdline_stdin_exit(testpmd_cl);
+	}
 }
 
 static void
--

2.27.0

Yu, DapengX

author

2021-01-26 07:13 UTC #10 lore raw

Re: [dpdk-dev] [PATCH v2] app/testpmd: avoid exit without terminal restore

Hi Xiaoyun,

For this situation: registering prompt_exit succeed, and user type "quit" command to exit testpmd.

If the second call to cmdline_stdin_exit is not excluded from the code conditionally, the cmdline_stdin_exit will be called twice;

1. the first time: in prompt()

    #0  cmdline_stdin_exit (cl=0x3638470) at ../lib/librte_cmdline/cmdline_socket.c:56
    #1  0x000000000051fb62 in prompt () at ../app/test-pmd/cmdline.c:17134
    #2  0x00000000005a4d0f in main (argc=2, argv=0x7fffffffda70) at ../app/test-pmd/testpmd.c:3849
2. the second time: in exit() .
    #0  cmdline_stdin_exit (cl=0x3638470) at ../lib/librte_cmdline/cmdline_socket.c:56
    #1  0x000000000051fb95 in prompt_exit () at ../app/test-pmd/cmdline.c:17142
    #2  0x00007ffff630fe9c in __run_exit_handlers () from /lib64/libc.so.6
    #3  0x00007ffff630ffd0 in exit () from /lib64/libc.so.6
    #4  0x00007ffff62f96aa in __libc_start_main () from /lib64/libc.so.6
    #5  0x00000000004ead2e in _start ()

-----Original Message-----
From: Li, Xiaoyun
Sent: Tuesday, January 26, 2021 2:33 PM
To: Yu, DapengX <[email protected]>; Xing, Beilei <[email protected]>; Lu, Wenzhuo <[email protected]>; Iremonger, Bernard <[email protected]>; [email protected]
Cc: [email protected]; Yu, DapengX <[email protected]>; [email protected]
Subject: RE: [PATCH v2] app/testpmd: avoid exit without terminal restore

Quoted 39 lines

-----Original Message-----
From: [email protected] <[email protected]>
Sent: Monday, January 25, 2021 11:30
To: Xing, Beilei <[email protected]>; Li, Xiaoyun <[email protected]>; Lu, Wenzhuo <[email protected]>; Iremonger, Bernard <[email protected]>; [email protected]
Cc: [email protected]; Yu, DapengX <[email protected]>; [email protected]
Subject: [PATCH v2] app/testpmd: avoid exit without terminal restore
From: Dapeng Yu <[email protected]>
In interactive mode, if testpmd exit by calling rte_exit without restore terminal attributes, terminal will not echo keyboard input.
register a function with atexit() in prompt(), when exit() in rte_exit() is called, the registered function restores terminal attributes.
Fixes 5a8fb55c48ab ("app/testpmd: support unidirectional
configuration")
Cc: [email protected]
Signed-off-by Dapeng Yu <[email protected]>
---
 app/test-pmd/cmdline.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
app/test-pmd/cmdline.c index renamed from app/test-pmd/cmdline.c +0 -0
89034c8b7..f7e18ba3d 100644
app/test-pmd/cmdline.c +0 -0
@@ -17116,6 +17116,7 @@ cmdline_read_from_file(const char *filename) 
void
prompt(void)
{
+int ret;
/* initialize non-constant commands */ cmd_set_fwd_mode_init();
cmd_set_fwd_retry_mode_init(); @@ -17123,15 +17124,23 @@ prompt(void) testpmd_cl = cmdline_stdin_new(main_ctx, "testpmd> "); if (testpmd_cl == NULL) return;

app/test-pmd/cmdline.c prompt(void) · line 17133

Quoted 8 lines

app/test-pmd/cmdline.c +6 -1

+
+ret = atexit(prompt_exit);
+if (ret != 0)
+printf("Cannot set exit function for cmdline\n");
+
 cmdline_interact(testpmd_cl);
-cmdline_stdin_exit(testpmd_cl);
+if (ret != 0)

Quoted 16 lines

app/test-pmd/cmdline.c +4 -2

+cmdline_stdin_exit(testpmd_cl);
 }
 
 void
 prompt_exit(void)
 {
-if (testpmd_cl != NULL)
+if (testpmd_cl != NULL) {
 cmdline_quit(testpmd_cl);
+cmdline_stdin_exit(testpmd_cl);
+}
 }
 
 static void
--

2.27.0

Yu, DapengX

author

2021-01-26 07:44 UTC #11 lore raw

Re: [dpdk-dev] [PATCH v2] app/testpmd: avoid exit without terminal restore

In reply to Li, Xiaoyun

@Xiaoyun,
Add one more explanation.
even if registers prompt_exit failed, when users type "quit" and then break from cmdline_interact(), the ret check will not prevent cmdline_stdin_exit() to be called to restore terminal settings in this patch.

@Beilei
Can you give comment on this patch v2, so I can move on to resolve this issue related with this patch.

-----Original Message-----
From: Yu, DapengX
Sent: Tuesday, January 26, 2021 3:13 PM
To: Li, Xiaoyun <[email protected]>; Xing, Beilei <[email protected]>; Lu, Wenzhuo <[email protected]>; Iremonger, Bernard <[email protected]>; [email protected]
Cc: [email protected]; [email protected]
Subject: RE: [PATCH v2] app/testpmd: avoid exit without terminal restore

Hi Xiaoyun,

For this situation: registering prompt_exit succeed, and user type "quit" command to exit testpmd.

If the second call to cmdline_stdin_exit is not excluded from the code conditionally, the cmdline_stdin_exit will be called twice;

1. the first time: in prompt()

    #0  cmdline_stdin_exit (cl=0x3638470) at ../lib/librte_cmdline/cmdline_socket.c:56
    #1  0x000000000051fb62 in prompt () at ../app/test-pmd/cmdline.c:17134
    #2  0x00000000005a4d0f in main (argc=2, argv=0x7fffffffda70) at ../app/test-pmd/testpmd.c:3849 2. the second time: in exit() .
    #0  cmdline_stdin_exit (cl=0x3638470) at ../lib/librte_cmdline/cmdline_socket.c:56
    #1  0x000000000051fb95 in prompt_exit () at ../app/test-pmd/cmdline.c:17142
    #2  0x00007ffff630fe9c in __run_exit_handlers () from /lib64/libc.so.6
    #3  0x00007ffff630ffd0 in exit () from /lib64/libc.so.6
    #4  0x00007ffff62f96aa in __libc_start_main () from /lib64/libc.so.6
    #5  0x00000000004ead2e in _start ()

Quoted 39 lines

-----Original Message-----
From: [email protected] <[email protected]>
Sent: Monday, January 25, 2021 11:30
To: Xing, Beilei <[email protected]>; Li, Xiaoyun <[email protected]>; Lu, Wenzhuo <[email protected]>; Iremonger, Bernard <[email protected]>; [email protected]
Cc: [email protected]; Yu, DapengX <[email protected]>; [email protected]
Subject: [PATCH v2] app/testpmd: avoid exit without terminal restore
From: Dapeng Yu <[email protected]>
In interactive mode, if testpmd exit by calling rte_exit without restore terminal attributes, terminal will not echo keyboard input.
register a function with atexit() in prompt(), when exit() in rte_exit() is called, the registered function restores terminal attributes.
Fixes 5a8fb55c48ab ("app/testpmd: support unidirectional
configuration")
Cc: [email protected]
Signed-off-by Dapeng Yu <[email protected]>
---
 app/test-pmd/cmdline.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
app/test-pmd/cmdline.c index renamed from app/test-pmd/cmdline.c +0 -0
89034c8b7..f7e18ba3d 100644
app/test-pmd/cmdline.c +0 -0
@@ -17116,6 +17116,7 @@ cmdline_read_from_file(const char *filename) 
void
prompt(void)
{
+int ret;
/* initialize non-constant commands */ cmd_set_fwd_mode_init();
cmd_set_fwd_retry_mode_init(); @@ -17123,15 +17124,23 @@ prompt(void) testpmd_cl = cmdline_stdin_new(main_ctx, "testpmd> "); if (testpmd_cl == NULL) return;

app/test-pmd/cmdline.c prompt(void) · line 17133

Quoted 8 lines

app/test-pmd/cmdline.c +6 -1

+
+ret = atexit(prompt_exit);
+if (ret != 0)
+printf("Cannot set exit function for cmdline\n");
+
 cmdline_interact(testpmd_cl);
-cmdline_stdin_exit(testpmd_cl);
+if (ret != 0)

Quoted 16 lines

app/test-pmd/cmdline.c +4 -2

+cmdline_stdin_exit(testpmd_cl);
 }
 
 void
 prompt_exit(void)
 {
-if (testpmd_cl != NULL)
+if (testpmd_cl != NULL) {
 cmdline_quit(testpmd_cl);
+cmdline_stdin_exit(testpmd_cl);
+}
 }
 
 static void
--

2.27.0

Li, Xiaoyun

Acked-by

2021-01-26 08:24 UTC #12 lore raw

Re: [dpdk-dev] [PATCH v2] app/testpmd: avoid exit without terminal restore

Acked-by Xiaoyun Li <[email protected]>

Quoted 133 lines

-----Original Message-----
From: Yu, DapengX <[email protected]>
Sent: Tuesday, January 26, 2021 15:44
To: Li, Xiaoyun <[email protected]>; Xing, Beilei <[email protected]>; Lu, Wenzhuo <[email protected]>; Iremonger, Bernard <[email protected]>; [email protected]
Cc: [email protected]; [email protected]
Subject: RE: [PATCH v2] app/testpmd: avoid exit without terminal restore
@Xiaoyun,
Add one more explanation.
even if registers prompt_exit failed, when users type "quit" and then break from cmdline_interact(), the ret check will not prevent cmdline_stdin_exit() to be called to restore terminal settings in this patch.
@Beilei
Can you give comment on this patch v2, so I can move on to resolve this issue related with this patch.
-----Original Message-----
From: Yu, DapengX
Sent: Tuesday, January 26, 2021 3:13 PM
To: Li, Xiaoyun <[email protected]>; Xing, Beilei <[email protected]>; Lu, Wenzhuo <[email protected]>; Iremonger, Bernard <[email protected]>; [email protected]
Cc: [email protected]; [email protected]
Subject: RE: [PATCH v2] app/testpmd: avoid exit without terminal restore
Hi Xiaoyun,
For this situation: registering prompt_exit succeed, and user type "quit" command to exit testpmd.
If the second call to cmdline_stdin_exit is not excluded from the code conditionally, the cmdline_stdin_exit will be called twice;
1. the first time: in prompt()
    #0  cmdline_stdin_exit (cl=0x3638470)
at ../lib/librte_cmdline/cmdline_socket.c:56
    #1  0x000000000051fb62 in prompt () at ../app/test-pmd/cmdline.c:17134
    #2  0x00000000005a4d0f in main (argc=2, argv=0x7fffffffda70) at ../app/test-
pmd/testpmd.c:3849 2. the second time: in exit() .
    #0  cmdline_stdin_exit (cl=0x3638470)
at ../lib/librte_cmdline/cmdline_socket.c:56
    #1  0x000000000051fb95 in prompt_exit () at ../app/test-
pmd/cmdline.c:17142
    #2  0x00007ffff630fe9c in __run_exit_handlers () from /lib64/libc.so.6
    #3  0x00007ffff630ffd0 in exit () from /lib64/libc.so.6
    #4  0x00007ffff62f96aa in __libc_start_main () from /lib64/libc.so.6
    #5  0x00000000004ead2e in _start ()
-----Original Message-----
From: Li, Xiaoyun
Sent: Tuesday, January 26, 2021 2:33 PM
To: Yu, DapengX <[email protected]>; Xing, Beilei <[email protected]>; Lu, Wenzhuo <[email protected]>; Iremonger, Bernard <[email protected]>; [email protected]
Cc: [email protected]; Yu, DapengX <[email protected]>; [email protected]
Subject: RE: [PATCH v2] app/testpmd: avoid exit without terminal restore
Hi
Quoted 39 lines
-----Original Message-----
From: [email protected] <[email protected]>
Sent: Monday, January 25, 2021 11:30
To: Xing, Beilei <[email protected]>; Li, Xiaoyun <[email protected]>; Lu, Wenzhuo <[email protected]>; Iremonger, Bernard <[email protected]>; [email protected]
Cc: [email protected]; Yu, DapengX <[email protected]>; [email protected]
Subject: [PATCH v2] app/testpmd: avoid exit without terminal restore
From: Dapeng Yu <[email protected]>
In interactive mode, if testpmd exit by calling rte_exit without restore terminal attributes, terminal will not echo keyboard input.
register a function with atexit() in prompt(), when exit() in rte_exit() is called, the registered function restores terminal attributes.
Fixes 5a8fb55c48ab ("app/testpmd: support unidirectional
configuration")
Cc: [email protected]
Signed-off-by Dapeng Yu <[email protected]>
---
 app/test-pmd/cmdline.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
app/test-pmd/cmdline.c index renamed from app/test-pmd/cmdline.c +0 -0
89034c8b7..f7e18ba3d 100644
app/test-pmd/cmdline.c +0 -0
@@ -17116,6 +17116,7 @@ cmdline_read_from_file(const char *filename)
void
prompt(void)
{
+int ret;
/* initialize non-constant commands */ cmd_set_fwd_mode_init();
cmd_set_fwd_retry_mode_init(); @@ -17123,15 +17124,23 @@ prompt(void) testpmd_cl = cmdline_stdin_new(main_ctx, "testpmd> "); if (testpmd_cl == NULL) return;
app/test-pmd/cmdline.c prompt(void) · line 17133
Quoted 8 lines
app/test-pmd/cmdline.c +6 -1
+
+ret = atexit(prompt_exit);
+if (ret != 0)
+printf("Cannot set exit function for cmdline\n");
+
 cmdline_interact(testpmd_cl);
-cmdline_stdin_exit(testpmd_cl);
+if (ret != 0)
Why do you need to check ret here?
Even if registers prompt_exit failed, when users type "quit" and then break from cmdline_interact(), cmdline_stdin_exit() should be called to restore terminal settings.
Quoted 16 lines
app/test-pmd/cmdline.c +4 -2
+cmdline_stdin_exit(testpmd_cl);
 }
 
 void
 prompt_exit(void)
 {
-if (testpmd_cl != NULL)
+if (testpmd_cl != NULL) {
 cmdline_quit(testpmd_cl);
+cmdline_stdin_exit(testpmd_cl);
+}
 }
 
 static void
--
2.27.0

Ferruh Yigit

Applied

2021-01-29 10:47 UTC #13 lore raw

Re: [dpdk-dev] [dpdk-stable] [PATCH v2] app/testpmd: avoid exit without terminal restore

On 1/26/2021 8:24 AM, Li, Xiaoyun wrote:

<...>

Quoted 9 lines

Quoted 9 lines
Quoted 9 lines
From: Dapeng Yu <[email protected]>
In interactive mode, if testpmd exit by calling rte_exit without restore terminal attributes, terminal will not echo keyboard input.
register a function with atexit() in prompt(), when exit() in rte_exit() is called, the registered function restores terminal attributes.

Quoted 8 lines

Quoted 5 lines
Quoted 5 lines
Fixes 5a8fb55c48ab ("app/testpmd: support unidirectional
configuration")
Cc: [email protected]
Signed-off-by Dapeng Yu <[email protected]>
Acked-by Xiaoyun Li <[email protected]>

Applied to dpdk-next-net/main, thanks.