ethtool: remove a redundant call to rte_eth_dev_stop()
Checks
Commit Message
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(-)
Comments
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
>
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);
> }
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);
> > }
>
>
>
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);
> > > }
> >
> >
> >
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>
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.
@@ -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);
}