[PATCH] ethtool: remove a redundant call to rte_eth_dev_stop()

[PATCH] ethtool: remove a redundant call to rte_eth_dev_stop()

[Usman Tanveer]

There is a call to rte_eth_dev_stop() in rte_ethtool_net_open()
due to which user gets misleading message upon first open/start call.
It says that the
device is already stopped, which should not be the case. This patch
removes rte_eth_dev_stop() from rte_ethtool_net_open().

Signed-off-by: Usman Tanveer <usman.tanveer@emumba.com>
---
 examples/ethtool/lib/rte_ethtool.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/examples/ethtool/lib/rte_ethtool.c b/examples/ethtool/lib/rte_ethtool.c
index ffaad96498..0a000d0a7b 100644
--- a/examples/ethtool/lib/rte_ethtool.c
+++ b/examples/ethtool/lib/rte_ethtool.c
@@ -297,12 +297,6 @@ rte_ethtool_set_pauseparam(uint16_t port_id,
 int
 rte_ethtool_net_open(uint16_t port_id)
 {
-	int ret;
-
-	ret = rte_eth_dev_stop(port_id);
-	if (ret != 0)
-		return ret;
-
 	return rte_eth_dev_start(port_id);
 }
 
-- 
2.25.1

Re: [PATCH] ethtool: remove a redundant call to rte_eth_dev_stop()

[Usman Tanveer]

Hi,
I've submitted a patch on 27th May, 2022. One of the tests is failing
and I think it is not related to the patch.
Following test is failing:
- ci/github-robot: build

Can you please rerun the tests so that the patch can be submitted.

Regards,
-Usman


On Fri, May 27, 2022 at 11:42 AM Usman Tanveer <usman.tanveer@emumba.com> wrote:
>
> There is a call to rte_eth_dev_stop() in rte_ethtool_net_open()
> due to which user gets misleading message upon first open/start call.
> It says that the
> device is already stopped, which should not be the case. This patch
> removes rte_eth_dev_stop() from rte_ethtool_net_open().
>
> Signed-off-by: Usman Tanveer <usman.tanveer@emumba.com>
> ---
>  examples/ethtool/lib/rte_ethtool.c | 6 ------
>  1 file changed, 6 deletions(-)
>
> diff --git a/examples/ethtool/lib/rte_ethtool.c b/examples/ethtool/lib/rte_ethtool.c
> index ffaad96498..0a000d0a7b 100644
> --- a/examples/ethtool/lib/rte_ethtool.c
> +++ b/examples/ethtool/lib/rte_ethtool.c
> @@ -297,12 +297,6 @@ rte_ethtool_set_pauseparam(uint16_t port_id,
>  int
>  rte_ethtool_net_open(uint16_t port_id)
>  {
> -       int ret;
> -
> -       ret = rte_eth_dev_stop(port_id);
> -       if (ret != 0)
> -               return ret;
> -
>         return rte_eth_dev_start(port_id);
>  }
>
> --
> 2.25.1
>

Re: [PATCH] ethtool: remove a redundant call to rte_eth_dev_stop()

[Thomas Monjalon]

27/05/2022 08:42, Usman Tanveer:
> There is a call to rte_eth_dev_stop() in rte_ethtool_net_open()
> due to which user gets misleading message upon first open/start call.
> It says that the
> device is already stopped, which should not be the case. This patch
> removes rte_eth_dev_stop() from rte_ethtool_net_open().

Why was it there?
Any opinion? Is it safe to remove?

>  int
>  rte_ethtool_net_open(uint16_t port_id)
>  {
> -	int ret;
> -
> -	ret = rte_eth_dev_stop(port_id);
> -	if (ret != 0)
> -		return ret;
> -
>  	return rte_eth_dev_start(port_id);
>  }

Re: [PATCH] ethtool: remove a redundant call to rte_eth_dev_stop()

[Usman Tanveer]

It has been there since the file was added (2015). I'm not able to
find any purpose for this.

Although, it's misleading the messages it shows upon calling
rte_ethtool_net_open() as mentioned above. And when this function is
called even when the port is already UP, it should print a message
"device already started", but it's not like that because it first
stops the device and then starts it again.

The above change fixes both misleading messages.


On Mon, Jun 27, 2022 at 2:16 AM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 27/05/2022 08:42, Usman Tanveer:
> > There is a call to rte_eth_dev_stop() in rte_ethtool_net_open()
> > due to which user gets misleading message upon first open/start call.
> > It says that the
> > device is already stopped, which should not be the case. This patch
> > removes rte_eth_dev_stop() from rte_ethtool_net_open().
>
> Why was it there?
> Any opinion? Is it safe to remove?
>
> >  int
> >  rte_ethtool_net_open(uint16_t port_id)
> >  {
> > -     int ret;
> > -
> > -     ret = rte_eth_dev_stop(port_id);
> > -     if (ret != 0)
> > -             return ret;
> > -
> >       return rte_eth_dev_start(port_id);
> >  }
>
>
>

Re: [PATCH] ethtool: remove a redundant call to rte_eth_dev_stop()

[Usman Tanveer]

Hi,

Can you please have a look and update the status?


On Wed, Jul 6, 2022 at 2:49 PM Usman Tanveer <usman.tanveer@emumba.com> wrote:
>
> It has been there since the file was added (2015). I'm not able to
> find any purpose for this.
>
> Although, it's misleading the messages it shows upon calling
> rte_ethtool_net_open() as mentioned above. And when this function is
> called even when the port is already UP, it should print a message
> "device already started", but it's not like that because it first
> stops the device and then starts it again.
>
> The above change fixes both misleading messages.
>
>
> On Mon, Jun 27, 2022 at 2:16 AM Thomas Monjalon <thomas@monjalon.net> wrote:
> >
> > 27/05/2022 08:42, Usman Tanveer:
> > > There is a call to rte_eth_dev_stop() in rte_ethtool_net_open()
> > > due to which user gets misleading message upon first open/start call.
> > > It says that the
> > > device is already stopped, which should not be the case. This patch
> > > removes rte_eth_dev_stop() from rte_ethtool_net_open().
> >
> > Why was it there?
> > Any opinion? Is it safe to remove?
> >
> > >  int
> > >  rte_ethtool_net_open(uint16_t port_id)
> > >  {
> > > -     int ret;
> > > -
> > > -     ret = rte_eth_dev_stop(port_id);
> > > -     if (ret != 0)
> > > -             return ret;
> > > -
> > >       return rte_eth_dev_start(port_id);
> > >  }
> >
> >
> >

Re: [PATCH] ethtool: remove a redundant call to rte_eth_dev_stop()

[Stephen Hemminger]

On Thu, 18 Aug 2022 15:18:36 +0500
Usman Tanveer <usman.tanveer@emumba.com> wrote:

> Hi,
> 
> Can you please have a look and update the status?

Looks OK to me.

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

Re: [PATCH] ethtool: remove a redundant call to rte_eth_dev_stop()

[Thomas Monjalon]

04/07/2023 00:31, Stephen Hemminger:
> On Thu, 18 Aug 2022 15:18:36 +0500
> Usman Tanveer <usman.tanveer@emumba.com> wrote:
> 
> > Hi,
> > 
> > Can you please have a look and update the status?
> 
> Looks OK to me.
> 
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>

better title: examples/ethtool: remove stop before start

Applied, thanks.