On Tue, Jun 02, 2020 at 03:53:07PM +0530, Harman Kalra wrote: > On Mon, Jun 01, 2020 at 01:50:26PM +0100, Burakov, Anatoly wrote: > > On 30-May-20 11:02 AM, Harman Kalra wrote: > > > On Fri, May 29, 2020 at 03:19:45PM +0100, Burakov, Anatoly wrote: > > > > External Email > > > > > > > > ---------------------------------------------------------------------- > > > > On 29-May-20 2:19 PM, Harman Kalra wrote: > > > > > > > > > > if (ret < 0) > > > > > > rte_exit(EXIT_FAILURE, "Invalid L3FWD parameters\n"); > > > > > > - if (app_mode != APP_MODE_TELEMETRY && init_power_library()) > > > > > > + if (app_mode == APP_MODE_DEFAULT) > > > > > > + app_mode = APP_MODE_LEGACY; > > > > > > + > > > > > > + /* only legacy and empty poll mode rely on power library */ > > > > > > + if ((app_mode == APP_MODE_LEGACY || app_mode == > > > > > > APP_MODE_EMPTY_POLL) && > > > > > > + init_power_library()) > > > > > > rte_exit(EXIT_FAILURE, "init_power_library failed\n"); > > > > > Hi, > > > > > > > > > > Rather than just exiting from here can we have a else condition to > > > > > automatically enter into the "interrupt only" mode. > > > > > Please correct me if I am missing something. > > > > > > > > Hi, > > > > > > > > Thanks for your review. I don't think silently proceeding is a good > > > > idea. If > > > > the user wants interrupt-only mode, they should request it. Silently > > > > falling > > > > back to interrupt-only mode will create an illusion of successful > > > > initialization and set the wrong expectation for how the application > > > > will > > > > behave. > > > > > > > > > > Hi, > > > > > > Thanks for the explanation which even I also believe is logically perfect. > > > > > > But since l3fwd-power is an old application and has many users around > > > which are currently using this app in interrupt only mode without giving > > > an extra argument. But suddenly they will start getting failure messages > > > with > > > the new patchset. > > > > > > My only intent with else condition was backward compatibility. > > > Or may be we can have more descriptive failure message, something like > > > "init_power_library failed, check manual for other modes". > > > > > > Thanks > > > Harman > > > > > > > > > > I think we can compormise on an informative log message suggesting to use > > interrupt mode. I'm not keen on reverting to previous buggy behavior :) > > > Hi > > I am not insisting to revert to previous behavior, I am just trying to > highlight some probable issues that many users might face as its an old > application. > Since many arm based soc might not be supporting frequency scaling, can > we add the following check as soon as the application starts, probe the > platform if it supports frequency scaling, if not automatically set the > mode to interrupt mode, something like: > if (access("/sys/devices/system/cpu/cpu0/cpufreq/scaling_governor", > F_OK)) > app_mode = APP_MODE_INTERRUPT;
Sorry, no direct check in application but we can introduce a new API in power library: bool rte_is_freq_scaling() { return access("/sys/devices/system/cpu/cpu0/cpufreq/scaling_governor", F_OK); } and in the application we can use "rte_is_freq_scaling()" at the start. > > > Thanks > Harman > > > > > -- > > > > Thanks, > > > > Anatoly > > > > > > -- > > Thanks, > > Anatoly