Re: [dpdk-stable] [dpdk-dev] [PATCH] app/testpmd: avoid exit without resource release

["Yu, DapengX" <dapengx.yu@intel.com>]

@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 <beilei.xing@intel.com>; Yu, DapengX <dapengx.yu@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Iremonger, Bernard <bernard.iremonger@intel.com>; Stephen Hemminger <stephen@networkplumber.org>
Cc: dev@dpdk.org; stable@dpdk.org
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.

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

> -----Original Message-----
> From: stable <stable-bounces@dpdk.org> On Behalf Of Xing, Beilei
> Sent: Friday, January 15, 2021 13:50
> To: Yu, DapengX <dapengx.yu@intel.com>; Lu, Wenzhuo 
> <wenzhuo.lu@intel.com>; Iremonger, Bernard 
> <bernard.iremonger@intel.com>
> Cc: Stephen Hemminger <stephen@networkplumber.org>; dev@dpdk.org; 
> stable@dpdk.org
> Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH] app/testpmd: avoid exit 
> without resource release
> 
> > -----Original Message-----
> > From: Yu, DapengX <dapengx.yu@intel.com>
> > Sent: Friday, January 15, 2021 11:29 AM
> > To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Xing, Beilei 
> > <beilei.xing@intel.com>; Iremonger, Bernard 
> > <bernard.iremonger@intel.com>
> > Cc: Yu, DapengX <dapengx.yu@intel.com>; Stephen Hemminger 
> > <stephen@networkplumber.org>; dev@dpdk.org; stable@dpdk.org
> > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: avoid exit without 
> > resource release
> >
> > 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.
> 
> > So I can continue resolving the defect.
> >
> > Thanks!
> >
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Yu, DapengX
> > Sent: Friday, December 25, 2020 1:09 PM
> > To: Stephen Hemminger <stephen@networkplumber.org>
> > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Xing, Beilei 
> > <beilei.xing@intel.com>; Iremonger, Bernard 
> > <bernard.iremonger@intel.com>; dev@dpdk.org; stable@dpdk.org
> > 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:stephen@networkplumber.org]
> > Sent: Friday, December 25, 2020 11:03 AM
> > To: Yu, DapengX <dapengx.yu@intel.com>
> > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Xing, Beilei 
> > <beilei.xing@intel.com>; Iremonger, Bernard 
> > <bernard.iremonger@intel.com>; dev@dpdk.org; stable@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: avoid exit without 
> > resource release
> >
> > On Thu, 24 Dec 2020 11:57:48 +0800
> > dapengx.yu@intel.com wrote:
> >
> > > From: YU DAPENG <dapengx.yu@intel.com>
> > >
> > > 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: stable@dpdk.org
> > >
> > > Signed-off-by: YU DAPENG <dapengx.yu@intel.com>
> >
> > 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.
> >