* [Intel-wired-lan] [PATCH net-next V2 0/6] net/intel: call ndo_stop() instead of dev_close() when running offline selftest
@ 2016-02-03 8:20 ` Stefan Assmann
0 siblings, 0 replies; 31+ messages in thread
From: Stefan Assmann @ 2016-02-03 8:20 UTC (permalink / raw)
To: intel-wired-lan
Calling dev_close() causes IFF_UP to be cleared which will remove the
interfaces routes and some addresses. That's probably not what the user
intended when running the offline selftest. Besides this does not happen
if the interface is brought down before the test, so the current
behaviour is inconsistent.
Instead call the net_device_ops ndo_stop function directly and avoid
touching IFF_UP at all.
Tested this with igb, e1000e, ixgbe and i40e. All drivers now are able to
resume operation without restoring the IP address, gateway or such.
V2: rename e1000_open(), e1000_close() to e1000e_open(), e1000e_close()
to avoid name clash with e1000.
Stefan Assmann (6):
i40e: call ndo_stop() instead of dev_close() when running offline
selftest
ixgbe: call ndo_stop() instead of dev_close() when running offline
selftest
ixgbevf: call ndo_stop() instead of dev_close() when running offline
selftest
igb: call ndo_stop() instead of dev_close() when running offline
selftest
e1000: call ndo_stop() instead of dev_close() when running offline
selftest
e1000e: call ndo_stop() instead of dev_close() when running offline
selftest
drivers/net/ethernet/intel/e1000/e1000.h | 2 ++
drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 4 ++--
drivers/net/ethernet/intel/e1000/e1000_main.c | 8 ++++----
drivers/net/ethernet/intel/e1000e/e1000.h | 2 ++
drivers/net/ethernet/intel/e1000e/ethtool.c | 4 ++--
drivers/net/ethernet/intel/e1000e/netdev.c | 12 ++++++------
drivers/net/ethernet/intel/i40e/i40e.h | 2 +-
drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 4 ++--
drivers/net/ethernet/intel/i40e/i40e_main.c | 4 ----
drivers/net/ethernet/intel/igb/igb.h | 2 ++
drivers/net/ethernet/intel/igb/igb_ethtool.c | 4 ++--
drivers/net/ethernet/intel/igb/igb_main.c | 8 ++++----
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 2 ++
drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 4 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++--
drivers/net/ethernet/intel/ixgbevf/ethtool.c | 4 ++--
drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 2 ++
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 4 ++--
18 files changed, 41 insertions(+), 35 deletions(-)
--
2.5.0
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH net-next V2 1/6] i40e: call ndo_stop() instead of dev_close() when running offline selftest
2016-02-03 8:20 ` [Intel-wired-lan] " Stefan Assmann
@ 2016-02-03 8:20 ` Stefan Assmann
-1 siblings, 0 replies; 31+ messages in thread
From: Stefan Assmann @ 2016-02-03 8:20 UTC (permalink / raw)
To: intel-wired-lan; +Cc: netdev, davem, jeffrey.t.kirsher, sassmann
Calling dev_close() causes IFF_UP to be cleared which will remove the
interfaces routes and some addresses. That's probably not what the user
intended when running the offline selftest. Besides this does not happen
if the interface is brought down before the test, so the current
behaviour is inconsistent.
Instead call the net_device_ops ndo_stop function directly and avoid
touching IFF_UP at all.
Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
---
drivers/net/ethernet/intel/i40e/i40e.h | 2 +-
drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 4 ++--
drivers/net/ethernet/intel/i40e/i40e_main.c | 4 ----
3 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index 68f2204..7e16c55 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -773,6 +773,7 @@ int i40e_vlan_rx_kill_vid(struct net_device *netdev,
__always_unused __be16 proto, u16 vid);
#endif
int i40e_open(struct net_device *netdev);
+int i40e_close(struct net_device *netdev);
int i40e_vsi_open(struct i40e_vsi *vsi);
void i40e_vlan_stripping_disable(struct i40e_vsi *vsi);
int i40e_vsi_add_vlan(struct i40e_vsi *vsi, s16 vid);
@@ -785,7 +786,6 @@ bool i40e_is_vsi_in_vlan(struct i40e_vsi *vsi);
struct i40e_mac_filter *i40e_find_mac(struct i40e_vsi *vsi, u8 *macaddr,
bool is_vf, bool is_netdev);
#ifdef I40E_FCOE
-int i40e_close(struct net_device *netdev);
int i40e_setup_tc(struct net_device *netdev, u8 tc);
void i40e_netpoll(struct net_device *netdev);
int i40e_fcoe_enable(struct net_device *netdev);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 29d5833..eeca530 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -1704,7 +1704,7 @@ static void i40e_diag_test(struct net_device *netdev,
/* If the device is online then take it offline */
if (if_running)
/* indicate we're in test mode */
- dev_close(netdev);
+ i40e_close(netdev);
else
/* This reset does not affect link - if it is
* changed to a type of reset that does affect
@@ -1733,7 +1733,7 @@ static void i40e_diag_test(struct net_device *netdev,
i40e_do_reset(pf, BIT(__I40E_PF_RESET_REQUESTED));
if (if_running)
- dev_open(netdev);
+ i40e_open(netdev);
} else {
/* Online tests */
netif_info(pf, drv, netdev, "online testing starting\n");
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 8f3b53e..39a1653 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -5459,11 +5459,7 @@ static void i40e_fdir_filter_exit(struct i40e_pf *pf)
*
* Returns 0, this is not allowed to fail
**/
-#ifdef I40E_FCOE
int i40e_close(struct net_device *netdev)
-#else
-static int i40e_close(struct net_device *netdev)
-#endif
{
struct i40e_netdev_priv *np = netdev_priv(netdev);
struct i40e_vsi *vsi = np->vsi;
--
2.5.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [Intel-wired-lan] [PATCH net-next V2 1/6] i40e: call ndo_stop() instead of dev_close() when running offline selftest
@ 2016-02-03 8:20 ` Stefan Assmann
0 siblings, 0 replies; 31+ messages in thread
From: Stefan Assmann @ 2016-02-03 8:20 UTC (permalink / raw)
To: intel-wired-lan
Calling dev_close() causes IFF_UP to be cleared which will remove the
interfaces routes and some addresses. That's probably not what the user
intended when running the offline selftest. Besides this does not happen
if the interface is brought down before the test, so the current
behaviour is inconsistent.
Instead call the net_device_ops ndo_stop function directly and avoid
touching IFF_UP at all.
Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
---
drivers/net/ethernet/intel/i40e/i40e.h | 2 +-
drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 4 ++--
drivers/net/ethernet/intel/i40e/i40e_main.c | 4 ----
3 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index 68f2204..7e16c55 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -773,6 +773,7 @@ int i40e_vlan_rx_kill_vid(struct net_device *netdev,
__always_unused __be16 proto, u16 vid);
#endif
int i40e_open(struct net_device *netdev);
+int i40e_close(struct net_device *netdev);
int i40e_vsi_open(struct i40e_vsi *vsi);
void i40e_vlan_stripping_disable(struct i40e_vsi *vsi);
int i40e_vsi_add_vlan(struct i40e_vsi *vsi, s16 vid);
@@ -785,7 +786,6 @@ bool i40e_is_vsi_in_vlan(struct i40e_vsi *vsi);
struct i40e_mac_filter *i40e_find_mac(struct i40e_vsi *vsi, u8 *macaddr,
bool is_vf, bool is_netdev);
#ifdef I40E_FCOE
-int i40e_close(struct net_device *netdev);
int i40e_setup_tc(struct net_device *netdev, u8 tc);
void i40e_netpoll(struct net_device *netdev);
int i40e_fcoe_enable(struct net_device *netdev);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 29d5833..eeca530 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -1704,7 +1704,7 @@ static void i40e_diag_test(struct net_device *netdev,
/* If the device is online then take it offline */
if (if_running)
/* indicate we're in test mode */
- dev_close(netdev);
+ i40e_close(netdev);
else
/* This reset does not affect link - if it is
* changed to a type of reset that does affect
@@ -1733,7 +1733,7 @@ static void i40e_diag_test(struct net_device *netdev,
i40e_do_reset(pf, BIT(__I40E_PF_RESET_REQUESTED));
if (if_running)
- dev_open(netdev);
+ i40e_open(netdev);
} else {
/* Online tests */
netif_info(pf, drv, netdev, "online testing starting\n");
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 8f3b53e..39a1653 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -5459,11 +5459,7 @@ static void i40e_fdir_filter_exit(struct i40e_pf *pf)
*
* Returns 0, this is not allowed to fail
**/
-#ifdef I40E_FCOE
int i40e_close(struct net_device *netdev)
-#else
-static int i40e_close(struct net_device *netdev)
-#endif
{
struct i40e_netdev_priv *np = netdev_priv(netdev);
struct i40e_vsi *vsi = np->vsi;
--
2.5.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [Intel-wired-lan] [PATCH net-next V2 1/6] i40e: call ndo_stop() instead of dev_close() when running offline selftest
2016-02-03 8:20 ` [Intel-wired-lan] " Stefan Assmann
(?)
@ 2016-02-08 16:55 ` Bowers, AndrewX
-1 siblings, 0 replies; 31+ messages in thread
From: Bowers, AndrewX @ 2016-02-08 16:55 UTC (permalink / raw)
To: intel-wired-lan
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Stefan Assmann
> Sent: Wednesday, February 03, 2016 12:21 AM
> To: intel-wired-lan at lists.osuosl.org
> Cc: netdev at vger.kernel.org; davem at davemloft.net; sassmann at kpanic.de
> Subject: [Intel-wired-lan] [PATCH net-next V2 1/6] i40e: call ndo_stop()
> instead of dev_close() when running offline selftest
>
> Calling dev_close() causes IFF_UP to be cleared which will remove the
> interfaces routes and some addresses. That's probably not what the user
> intended when running the offline selftest. Besides this does not happen if
> the interface is brought down before the test, so the current behaviour is
> inconsistent.
> Instead call the net_device_ops ndo_stop function directly and avoid
> touching IFF_UP at all.
>
> Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
> ---
> drivers/net/ethernet/intel/i40e/i40e.h | 2 +-
> drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 4 ++--
> drivers/net/ethernet/intel/i40e/i40e_main.c | 4 ----
> 3 files changed, 3 insertions(+), 7 deletions(-)
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Interface did not drop addresses/routes during offline ethtool self-test.
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH net-next V2 2/6] ixgbe: call ndo_stop() instead of dev_close() when running offline selftest
2016-02-03 8:20 ` [Intel-wired-lan] " Stefan Assmann
@ 2016-02-03 8:20 ` Stefan Assmann
-1 siblings, 0 replies; 31+ messages in thread
From: Stefan Assmann @ 2016-02-03 8:20 UTC (permalink / raw)
To: intel-wired-lan; +Cc: netdev, davem, jeffrey.t.kirsher, sassmann
Calling dev_close() causes IFF_UP to be cleared which will remove the
interfaces routes and some addresses. That's probably not what the user
intended when running the offline selftest. Besides this does not happen
if the interface is brought down before the test, so the current
behaviour is inconsistent.
Instead call the net_device_ops ndo_stop function directly and avoid
touching IFF_UP at all.
Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
---
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 2 ++
drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 4 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++--
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 4b9156c..6cf1ac7 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -875,6 +875,8 @@ extern const char ixgbe_driver_version[];
extern char ixgbe_default_device_descr[];
#endif /* IXGBE_FCOE */
+int ixgbe_open(struct net_device *netdev);
+int ixgbe_close(struct net_device *netdev);
void ixgbe_up(struct ixgbe_adapter *adapter);
void ixgbe_down(struct ixgbe_adapter *adapter);
void ixgbe_reinit_locked(struct ixgbe_adapter *adapter);
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index bea96b3..2f05937 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -2053,7 +2053,7 @@ static void ixgbe_diag_test(struct net_device *netdev,
if (if_running)
/* indicate we're in test mode */
- dev_close(netdev);
+ ixgbe_close(netdev);
else
ixgbe_reset(adapter);
@@ -2091,7 +2091,7 @@ skip_loopback:
/* clear testing bit and return adapter to previous state */
clear_bit(__IXGBE_TESTING, &adapter->state);
if (if_running)
- dev_open(netdev);
+ ixgbe_open(netdev);
else if (hw->mac.ops.disable_tx_laser)
hw->mac.ops.disable_tx_laser(hw);
} else {
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index c4003a8..354be34 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -5988,7 +5988,7 @@ static int ixgbe_change_mtu(struct net_device *netdev, int new_mtu)
* handler is registered with the OS, the watchdog timer is started,
* and the stack is notified that the interface is ready.
**/
-static int ixgbe_open(struct net_device *netdev)
+int ixgbe_open(struct net_device *netdev)
{
struct ixgbe_adapter *adapter = netdev_priv(netdev);
struct ixgbe_hw *hw = &adapter->hw;
@@ -6090,7 +6090,7 @@ static void ixgbe_close_suspend(struct ixgbe_adapter *adapter)
* needs to be disabled. A global MAC reset is issued to stop the
* hardware, and all transmit and receive resources are freed.
**/
-static int ixgbe_close(struct net_device *netdev)
+int ixgbe_close(struct net_device *netdev)
{
struct ixgbe_adapter *adapter = netdev_priv(netdev);
--
2.5.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [Intel-wired-lan] [PATCH net-next V2 2/6] ixgbe: call ndo_stop() instead of dev_close() when running offline selftest
@ 2016-02-03 8:20 ` Stefan Assmann
0 siblings, 0 replies; 31+ messages in thread
From: Stefan Assmann @ 2016-02-03 8:20 UTC (permalink / raw)
To: intel-wired-lan
Calling dev_close() causes IFF_UP to be cleared which will remove the
interfaces routes and some addresses. That's probably not what the user
intended when running the offline selftest. Besides this does not happen
if the interface is brought down before the test, so the current
behaviour is inconsistent.
Instead call the net_device_ops ndo_stop function directly and avoid
touching IFF_UP at all.
Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
---
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 2 ++
drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 4 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++--
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 4b9156c..6cf1ac7 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -875,6 +875,8 @@ extern const char ixgbe_driver_version[];
extern char ixgbe_default_device_descr[];
#endif /* IXGBE_FCOE */
+int ixgbe_open(struct net_device *netdev);
+int ixgbe_close(struct net_device *netdev);
void ixgbe_up(struct ixgbe_adapter *adapter);
void ixgbe_down(struct ixgbe_adapter *adapter);
void ixgbe_reinit_locked(struct ixgbe_adapter *adapter);
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index bea96b3..2f05937 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -2053,7 +2053,7 @@ static void ixgbe_diag_test(struct net_device *netdev,
if (if_running)
/* indicate we're in test mode */
- dev_close(netdev);
+ ixgbe_close(netdev);
else
ixgbe_reset(adapter);
@@ -2091,7 +2091,7 @@ skip_loopback:
/* clear testing bit and return adapter to previous state */
clear_bit(__IXGBE_TESTING, &adapter->state);
if (if_running)
- dev_open(netdev);
+ ixgbe_open(netdev);
else if (hw->mac.ops.disable_tx_laser)
hw->mac.ops.disable_tx_laser(hw);
} else {
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index c4003a8..354be34 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -5988,7 +5988,7 @@ static int ixgbe_change_mtu(struct net_device *netdev, int new_mtu)
* handler is registered with the OS, the watchdog timer is started,
* and the stack is notified that the interface is ready.
**/
-static int ixgbe_open(struct net_device *netdev)
+int ixgbe_open(struct net_device *netdev)
{
struct ixgbe_adapter *adapter = netdev_priv(netdev);
struct ixgbe_hw *hw = &adapter->hw;
@@ -6090,7 +6090,7 @@ static void ixgbe_close_suspend(struct ixgbe_adapter *adapter)
* needs to be disabled. A global MAC reset is issued to stop the
* hardware, and all transmit and receive resources are freed.
**/
-static int ixgbe_close(struct net_device *netdev)
+int ixgbe_close(struct net_device *netdev)
{
struct ixgbe_adapter *adapter = netdev_priv(netdev);
--
2.5.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [Intel-wired-lan] [PATCH net-next V2 2/6] ixgbe: call ndo_stop() instead of dev_close() when running offline selftest
2016-02-03 8:20 ` [Intel-wired-lan] " Stefan Assmann
(?)
@ 2016-03-01 19:34 ` Bowers, AndrewX
-1 siblings, 0 replies; 31+ messages in thread
From: Bowers, AndrewX @ 2016-03-01 19:34 UTC (permalink / raw)
To: intel-wired-lan
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Stefan Assmann
> Sent: Wednesday, February 03, 2016 12:21 AM
> To: intel-wired-lan at lists.osuosl.org
> Cc: netdev at vger.kernel.org; davem at davemloft.net; sassmann at kpanic.de
> Subject: [Intel-wired-lan] [PATCH net-next V2 2/6] ixgbe: call ndo_stop()
> instead of dev_close() when running offline selftest
>
> Calling dev_close() causes IFF_UP to be cleared which will remove the
> interfaces routes and some addresses. That's probably not what the user
> intended when running the offline selftest. Besides this does not happen if
> the interface is brought down before the test, so the current behaviour is
> inconsistent.
> Instead call the net_device_ops ndo_stop function directly and avoid
> touching IFF_UP at all.
>
> Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe.h | 2 ++
> drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 4 ++--
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++--
> 3 files changed, 6 insertions(+), 4 deletions(-)
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
ethtool offline selftest does not wipe information from interface (route, etc...)
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH net-next V2 3/6] ixgbevf: call ndo_stop() instead of dev_close() when running offline selftest
2016-02-03 8:20 ` [Intel-wired-lan] " Stefan Assmann
@ 2016-02-03 8:20 ` Stefan Assmann
-1 siblings, 0 replies; 31+ messages in thread
From: Stefan Assmann @ 2016-02-03 8:20 UTC (permalink / raw)
To: intel-wired-lan; +Cc: netdev, davem, jeffrey.t.kirsher, sassmann
Calling dev_close() causes IFF_UP to be cleared which will remove the
interfaces routes and some addresses. That's probably not what the user
intended when running the offline selftest. Besides this does not happen
if the interface is brought down before the test, so the current
behaviour is inconsistent.
Instead call the net_device_ops ndo_stop function directly and avoid
touching IFF_UP at all.
Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
---
drivers/net/ethernet/intel/ixgbevf/ethtool.c | 4 ++--
drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 2 ++
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 4 ++--
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbevf/ethtool.c b/drivers/net/ethernet/intel/ixgbevf/ethtool.c
index c48aef6..d7aa4b2 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ethtool.c
@@ -680,7 +680,7 @@ static void ixgbevf_diag_test(struct net_device *netdev,
if (if_running)
/* indicate we're in test mode */
- dev_close(netdev);
+ ixgbevf_close(netdev);
else
ixgbevf_reset(adapter);
@@ -692,7 +692,7 @@ static void ixgbevf_diag_test(struct net_device *netdev,
clear_bit(__IXGBEVF_TESTING, &adapter->state);
if (if_running)
- dev_open(netdev);
+ ixgbevf_open(netdev);
} else {
hw_dbg(&adapter->hw, "online testing starting\n");
/* Online tests */
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
index 68ec7daa..991eeae 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
@@ -486,6 +486,8 @@ extern const struct ixgbe_mbx_operations ixgbevf_mbx_ops;
extern const char ixgbevf_driver_name[];
extern const char ixgbevf_driver_version[];
+int ixgbevf_open(struct net_device *netdev);
+int ixgbevf_close(struct net_device *netdev);
void ixgbevf_up(struct ixgbevf_adapter *adapter);
void ixgbevf_down(struct ixgbevf_adapter *adapter);
void ixgbevf_reinit_locked(struct ixgbevf_adapter *adapter);
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 3558f01..01f79fa 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -3122,7 +3122,7 @@ static void ixgbevf_free_all_rx_resources(struct ixgbevf_adapter *adapter)
* handler is registered with the OS, the watchdog timer is started,
* and the stack is notified that the interface is ready.
**/
-static int ixgbevf_open(struct net_device *netdev)
+int ixgbevf_open(struct net_device *netdev)
{
struct ixgbevf_adapter *adapter = netdev_priv(netdev);
struct ixgbe_hw *hw = &adapter->hw;
@@ -3205,7 +3205,7 @@ err_setup_reset:
* needs to be disabled. A global MAC reset is issued to stop the
* hardware, and all transmit and receive resources are freed.
**/
-static int ixgbevf_close(struct net_device *netdev)
+int ixgbevf_close(struct net_device *netdev)
{
struct ixgbevf_adapter *adapter = netdev_priv(netdev);
--
2.5.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [Intel-wired-lan] [PATCH net-next V2 3/6] ixgbevf: call ndo_stop() instead of dev_close() when running offline selftest
@ 2016-02-03 8:20 ` Stefan Assmann
0 siblings, 0 replies; 31+ messages in thread
From: Stefan Assmann @ 2016-02-03 8:20 UTC (permalink / raw)
To: intel-wired-lan
Calling dev_close() causes IFF_UP to be cleared which will remove the
interfaces routes and some addresses. That's probably not what the user
intended when running the offline selftest. Besides this does not happen
if the interface is brought down before the test, so the current
behaviour is inconsistent.
Instead call the net_device_ops ndo_stop function directly and avoid
touching IFF_UP at all.
Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
---
drivers/net/ethernet/intel/ixgbevf/ethtool.c | 4 ++--
drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 2 ++
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 4 ++--
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbevf/ethtool.c b/drivers/net/ethernet/intel/ixgbevf/ethtool.c
index c48aef6..d7aa4b2 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ethtool.c
@@ -680,7 +680,7 @@ static void ixgbevf_diag_test(struct net_device *netdev,
if (if_running)
/* indicate we're in test mode */
- dev_close(netdev);
+ ixgbevf_close(netdev);
else
ixgbevf_reset(adapter);
@@ -692,7 +692,7 @@ static void ixgbevf_diag_test(struct net_device *netdev,
clear_bit(__IXGBEVF_TESTING, &adapter->state);
if (if_running)
- dev_open(netdev);
+ ixgbevf_open(netdev);
} else {
hw_dbg(&adapter->hw, "online testing starting\n");
/* Online tests */
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
index 68ec7daa..991eeae 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
@@ -486,6 +486,8 @@ extern const struct ixgbe_mbx_operations ixgbevf_mbx_ops;
extern const char ixgbevf_driver_name[];
extern const char ixgbevf_driver_version[];
+int ixgbevf_open(struct net_device *netdev);
+int ixgbevf_close(struct net_device *netdev);
void ixgbevf_up(struct ixgbevf_adapter *adapter);
void ixgbevf_down(struct ixgbevf_adapter *adapter);
void ixgbevf_reinit_locked(struct ixgbevf_adapter *adapter);
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 3558f01..01f79fa 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -3122,7 +3122,7 @@ static void ixgbevf_free_all_rx_resources(struct ixgbevf_adapter *adapter)
* handler is registered with the OS, the watchdog timer is started,
* and the stack is notified that the interface is ready.
**/
-static int ixgbevf_open(struct net_device *netdev)
+int ixgbevf_open(struct net_device *netdev)
{
struct ixgbevf_adapter *adapter = netdev_priv(netdev);
struct ixgbe_hw *hw = &adapter->hw;
@@ -3205,7 +3205,7 @@ err_setup_reset:
* needs to be disabled. A global MAC reset is issued to stop the
* hardware, and all transmit and receive resources are freed.
**/
-static int ixgbevf_close(struct net_device *netdev)
+int ixgbevf_close(struct net_device *netdev)
{
struct ixgbevf_adapter *adapter = netdev_priv(netdev);
--
2.5.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [Intel-wired-lan] [PATCH net-next V2 3/6] ixgbevf: call ndo_stop() instead of dev_close() when running offline selftest
2016-02-03 8:20 ` [Intel-wired-lan] " Stefan Assmann
(?)
@ 2016-03-03 20:10 ` Bowers, AndrewX
-1 siblings, 0 replies; 31+ messages in thread
From: Bowers, AndrewX @ 2016-03-03 20:10 UTC (permalink / raw)
To: intel-wired-lan
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Stefan Assmann
> Sent: Wednesday, February 03, 2016 12:21 AM
> To: intel-wired-lan at lists.osuosl.org
> Cc: netdev at vger.kernel.org; davem at davemloft.net; sassmann at kpanic.de
> Subject: [Intel-wired-lan] [PATCH net-next V2 3/6] ixgbevf: call ndo_stop()
> instead of dev_close() when running offline selftest
>
> Calling dev_close() causes IFF_UP to be cleared which will remove the
> interfaces routes and some addresses. That's probably not what the user
> intended when running the offline selftest. Besides this does not happen if
> the interface is brought down before the test, so the current behaviour is
> inconsistent.
> Instead call the net_device_ops ndo_stop function directly and avoid
> touching IFF_UP at all.
>
> Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
> ---
> drivers/net/ethernet/intel/ixgbevf/ethtool.c | 4 ++--
> drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 2 ++
> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 4 ++--
> 3 files changed, 6 insertions(+), 4 deletions(-)
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
ethtool offline selftest does not wipe information from interface (route, etc...)
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH net-next V2 4/6] igb: call ndo_stop() instead of dev_close() when running offline selftest
2016-02-03 8:20 ` [Intel-wired-lan] " Stefan Assmann
@ 2016-02-03 8:20 ` Stefan Assmann
-1 siblings, 0 replies; 31+ messages in thread
From: Stefan Assmann @ 2016-02-03 8:20 UTC (permalink / raw)
To: intel-wired-lan; +Cc: netdev, davem, jeffrey.t.kirsher, sassmann
Calling dev_close() causes IFF_UP to be cleared which will remove the
interfaces routes and some addresses. That's probably not what the user
intended when running the offline selftest. Besides this does not happen
if the interface is brought down before the test, so the current
behaviour is inconsistent.
Instead call the net_device_ops ndo_stop function directly and avoid
touching IFF_UP at all.
Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
---
drivers/net/ethernet/intel/igb/igb.h | 2 ++
drivers/net/ethernet/intel/igb/igb_ethtool.c | 4 ++--
drivers/net/ethernet/intel/igb/igb_main.c | 8 ++++----
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index e3cb93b..5dac361 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -510,6 +510,8 @@ enum igb_boards {
extern char igb_driver_name[];
extern char igb_driver_version[];
+int igb_open(struct net_device *netdev);
+int igb_close(struct net_device *netdev);
int igb_up(struct igb_adapter *);
void igb_down(struct igb_adapter *);
void igb_reinit_locked(struct igb_adapter *);
diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index 1d329f1..7982243 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -2017,7 +2017,7 @@ static void igb_diag_test(struct net_device *netdev,
if (if_running)
/* indicate we're in test mode */
- dev_close(netdev);
+ igb_close(netdev);
else
igb_reset(adapter);
@@ -2050,7 +2050,7 @@ static void igb_diag_test(struct net_device *netdev,
clear_bit(__IGB_TESTING, &adapter->state);
if (if_running)
- dev_open(netdev);
+ igb_open(netdev);
} else {
dev_info(&adapter->pdev->dev, "online testing starting\n");
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 31e5f39..7a2ccb5 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -122,8 +122,8 @@ static void igb_setup_mrqc(struct igb_adapter *);
static int igb_probe(struct pci_dev *, const struct pci_device_id *);
static void igb_remove(struct pci_dev *pdev);
static int igb_sw_init(struct igb_adapter *);
-static int igb_open(struct net_device *);
-static int igb_close(struct net_device *);
+int igb_open(struct net_device *);
+int igb_close(struct net_device *);
static void igb_configure(struct igb_adapter *);
static void igb_configure_tx(struct igb_adapter *);
static void igb_configure_rx(struct igb_adapter *);
@@ -3132,7 +3132,7 @@ err_setup_tx:
return err;
}
-static int igb_open(struct net_device *netdev)
+int igb_open(struct net_device *netdev)
{
return __igb_open(netdev, false);
}
@@ -3169,7 +3169,7 @@ static int __igb_close(struct net_device *netdev, bool suspending)
return 0;
}
-static int igb_close(struct net_device *netdev)
+int igb_close(struct net_device *netdev)
{
return __igb_close(netdev, false);
}
--
2.5.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [Intel-wired-lan] [PATCH net-next V2 4/6] igb: call ndo_stop() instead of dev_close() when running offline selftest
@ 2016-02-03 8:20 ` Stefan Assmann
0 siblings, 0 replies; 31+ messages in thread
From: Stefan Assmann @ 2016-02-03 8:20 UTC (permalink / raw)
To: intel-wired-lan
Calling dev_close() causes IFF_UP to be cleared which will remove the
interfaces routes and some addresses. That's probably not what the user
intended when running the offline selftest. Besides this does not happen
if the interface is brought down before the test, so the current
behaviour is inconsistent.
Instead call the net_device_ops ndo_stop function directly and avoid
touching IFF_UP at all.
Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
---
drivers/net/ethernet/intel/igb/igb.h | 2 ++
drivers/net/ethernet/intel/igb/igb_ethtool.c | 4 ++--
drivers/net/ethernet/intel/igb/igb_main.c | 8 ++++----
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index e3cb93b..5dac361 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -510,6 +510,8 @@ enum igb_boards {
extern char igb_driver_name[];
extern char igb_driver_version[];
+int igb_open(struct net_device *netdev);
+int igb_close(struct net_device *netdev);
int igb_up(struct igb_adapter *);
void igb_down(struct igb_adapter *);
void igb_reinit_locked(struct igb_adapter *);
diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index 1d329f1..7982243 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -2017,7 +2017,7 @@ static void igb_diag_test(struct net_device *netdev,
if (if_running)
/* indicate we're in test mode */
- dev_close(netdev);
+ igb_close(netdev);
else
igb_reset(adapter);
@@ -2050,7 +2050,7 @@ static void igb_diag_test(struct net_device *netdev,
clear_bit(__IGB_TESTING, &adapter->state);
if (if_running)
- dev_open(netdev);
+ igb_open(netdev);
} else {
dev_info(&adapter->pdev->dev, "online testing starting\n");
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 31e5f39..7a2ccb5 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -122,8 +122,8 @@ static void igb_setup_mrqc(struct igb_adapter *);
static int igb_probe(struct pci_dev *, const struct pci_device_id *);
static void igb_remove(struct pci_dev *pdev);
static int igb_sw_init(struct igb_adapter *);
-static int igb_open(struct net_device *);
-static int igb_close(struct net_device *);
+int igb_open(struct net_device *);
+int igb_close(struct net_device *);
static void igb_configure(struct igb_adapter *);
static void igb_configure_tx(struct igb_adapter *);
static void igb_configure_rx(struct igb_adapter *);
@@ -3132,7 +3132,7 @@ err_setup_tx:
return err;
}
-static int igb_open(struct net_device *netdev)
+int igb_open(struct net_device *netdev)
{
return __igb_open(netdev, false);
}
@@ -3169,7 +3169,7 @@ static int __igb_close(struct net_device *netdev, bool suspending)
return 0;
}
-static int igb_close(struct net_device *netdev)
+int igb_close(struct net_device *netdev)
{
return __igb_close(netdev, false);
}
--
2.5.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* RE: [Intel-wired-lan] [PATCH net-next V2 4/6] igb: call ndo_stop() instead of dev_close() when running offline selftest
2016-02-03 8:20 ` [Intel-wired-lan] " Stefan Assmann
@ 2016-02-12 23:22 ` Brown, Aaron F
-1 siblings, 0 replies; 31+ messages in thread
From: Brown, Aaron F @ 2016-02-12 23:22 UTC (permalink / raw)
To: Stefan Assmann, intel-wired-lan; +Cc: netdev, davem
> From: Intel-wired-lan [intel-wired-lan-bounces@lists.osuosl.org] on behalf of Stefan Assmann [sassmann@kpanic.de]
> Sent: Wednesday, February 03, 2016 12:20 AM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; davem@davemloft.net; sassmann@kpanic.de
> Subject: [Intel-wired-lan] [PATCH net-next V2 4/6] igb: call ndo_stop() instead of dev_close() when running offline selftest
>
> Calling dev_close() causes IFF_UP to be cleared which will remove the
> interfaces routes and some addresses. That's probably not what the user
> intended when running the offline selftest. Besides this does not happen
> if the interface is brought down before the test, so the current
> behaviour is inconsistent.
> Instead call the net_device_ops ndo_stop function directly and avoid
> touching IFF_UP at all.
>
> Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
> ---
> drivers/net/ethernet/intel/igb/igb.h | 2 ++
> drivers/net/ethernet/intel/igb/igb_ethtool.c | 4 ++--
> drivers/net/ethernet/intel/igb/igb_main.c | 8 ++++----
> 3 files changed, 8 insertions(+), 6 deletions(-)
Checkpatch warns that externs should be avoided in .c files, but they pre-existed and are just being flagged due to the name changing, so...
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [Intel-wired-lan] [PATCH net-next V2 4/6] igb: call ndo_stop() instead of dev_close() when running offline selftest
@ 2016-02-12 23:22 ` Brown, Aaron F
0 siblings, 0 replies; 31+ messages in thread
From: Brown, Aaron F @ 2016-02-12 23:22 UTC (permalink / raw)
To: intel-wired-lan
> From: Intel-wired-lan [intel-wired-lan-bounces at lists.osuosl.org] on behalf of Stefan Assmann [sassmann at kpanic.de]
> Sent: Wednesday, February 03, 2016 12:20 AM
> To: intel-wired-lan at lists.osuosl.org
> Cc: netdev at vger.kernel.org; davem at davemloft.net; sassmann at kpanic.de
> Subject: [Intel-wired-lan] [PATCH net-next V2 4/6] igb: call ndo_stop() instead of dev_close() when running offline selftest
>
> Calling dev_close() causes IFF_UP to be cleared which will remove the
> interfaces routes and some addresses. That's probably not what the user
> intended when running the offline selftest. Besides this does not happen
> if the interface is brought down before the test, so the current
> behaviour is inconsistent.
> Instead call the net_device_ops ndo_stop function directly and avoid
> touching IFF_UP at all.
>
> Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
> ---
> drivers/net/ethernet/intel/igb/igb.h | 2 ++
> drivers/net/ethernet/intel/igb/igb_ethtool.c | 4 ++--
> drivers/net/ethernet/intel/igb/igb_main.c | 8 ++++----
> 3 files changed, 8 insertions(+), 6 deletions(-)
Checkpatch warns that externs should be avoided in .c files, but they pre-existed and are just being flagged due to the name changing, so...
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next V2 4/6] igb: call ndo_stop() instead of dev_close() when running offline selftest
2016-02-12 23:22 ` Brown, Aaron F
@ 2016-02-16 17:24 ` Rustad, Mark D
-1 siblings, 0 replies; 31+ messages in thread
From: Rustad, Mark D @ 2016-02-16 17:24 UTC (permalink / raw)
To: Stefan Assmann; +Cc: intel-wired-lan, netdev, davem, Brown, Aaron F
[-- Attachment #1: Type: text/plain, Size: 1479 bytes --]
Aaron F <aaron.f.brown@intel.com> wrote:
>> From: Intel-wired-lan [intel-wired-lan-bounces@lists.osuosl.org] on
>> behalf of Stefan Assmann [sassmann@kpanic.de]
>> Sent: Wednesday, February 03, 2016 12:20 AM
>> To: intel-wired-lan@lists.osuosl.org
>> Cc: netdev@vger.kernel.org; davem@davemloft.net; sassmann@kpanic.de
>> Subject: [Intel-wired-lan] [PATCH net-next V2 4/6] igb: call ndo_stop()
>> instead of dev_close() when running offline selftest
>>
>> Calling dev_close() causes IFF_UP to be cleared which will remove the
>> interfaces routes and some addresses. That's probably not what the user
>> intended when running the offline selftest. Besides this does not happen
>> if the interface is brought down before the test, so the current
>> behaviour is inconsistent.
>> Instead call the net_device_ops ndo_stop function directly and avoid
>> touching IFF_UP at all.
>>
>> Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
>> ---
>> drivers/net/ethernet/intel/igb/igb.h | 2 ++
>> drivers/net/ethernet/intel/igb/igb_ethtool.c | 4 ++--
>> drivers/net/ethernet/intel/igb/igb_main.c | 8 ++++----
>> 3 files changed, 8 insertions(+), 6 deletions(-)
>
> Checkpatch warns that externs should be avoided in .c files, but they
> pre-existed and are just being flagged due to the name changing, so...
>
> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Again, the prototypes in the .c should be deleted in favor of the ones that
were added to the .h file.
^ permalink raw reply [flat|nested] 31+ messages in thread
* [Intel-wired-lan] [PATCH net-next V2 4/6] igb: call ndo_stop() instead of dev_close() when running offline selftest
@ 2016-02-16 17:24 ` Rustad, Mark D
0 siblings, 0 replies; 31+ messages in thread
From: Rustad, Mark D @ 2016-02-16 17:24 UTC (permalink / raw)
To: intel-wired-lan
Aaron F <aaron.f.brown@intel.com> wrote:
>> From: Intel-wired-lan [intel-wired-lan-bounces at lists.osuosl.org] on
>> behalf of Stefan Assmann [sassmann at kpanic.de]
>> Sent: Wednesday, February 03, 2016 12:20 AM
>> To: intel-wired-lan at lists.osuosl.org
>> Cc: netdev at vger.kernel.org; davem at davemloft.net; sassmann at kpanic.de
>> Subject: [Intel-wired-lan] [PATCH net-next V2 4/6] igb: call ndo_stop()
>> instead of dev_close() when running offline selftest
>>
>> Calling dev_close() causes IFF_UP to be cleared which will remove the
>> interfaces routes and some addresses. That's probably not what the user
>> intended when running the offline selftest. Besides this does not happen
>> if the interface is brought down before the test, so the current
>> behaviour is inconsistent.
>> Instead call the net_device_ops ndo_stop function directly and avoid
>> touching IFF_UP at all.
>>
>> Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
>> ---
>> drivers/net/ethernet/intel/igb/igb.h | 2 ++
>> drivers/net/ethernet/intel/igb/igb_ethtool.c | 4 ++--
>> drivers/net/ethernet/intel/igb/igb_main.c | 8 ++++----
>> 3 files changed, 8 insertions(+), 6 deletions(-)
>
> Checkpatch warns that externs should be avoided in .c files, but they
> pre-existed and are just being flagged due to the name changing, so...
>
> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Again, the prototypes in the .c should be deleted in favor of the ones that
were added to the .h file.
--
Mark Rustad, Networking Division, Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 841 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20160216/c30b8ef1/attachment.asc>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH net-next V2 5/6] e1000: call ndo_stop() instead of dev_close() when running offline selftest
2016-02-03 8:20 ` [Intel-wired-lan] " Stefan Assmann
@ 2016-02-03 8:20 ` Stefan Assmann
-1 siblings, 0 replies; 31+ messages in thread
From: Stefan Assmann @ 2016-02-03 8:20 UTC (permalink / raw)
To: intel-wired-lan; +Cc: netdev, davem, jeffrey.t.kirsher, sassmann
Calling dev_close() causes IFF_UP to be cleared which will remove the
interfaces routes and some addresses. That's probably not what the user
intended when running the offline selftest. Besides this does not happen
if the interface is brought down before the test, so the current
behaviour is inconsistent.
Instead call the net_device_ops ndo_stop function directly and avoid
touching IFF_UP at all.
Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
---
drivers/net/ethernet/intel/e1000/e1000.h | 2 ++
drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 4 ++--
drivers/net/ethernet/intel/e1000/e1000_main.c | 8 ++++----
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000/e1000.h b/drivers/net/ethernet/intel/e1000/e1000.h
index 98fe5a2..d7bdea7 100644
--- a/drivers/net/ethernet/intel/e1000/e1000.h
+++ b/drivers/net/ethernet/intel/e1000/e1000.h
@@ -358,6 +358,8 @@ struct net_device *e1000_get_hw_dev(struct e1000_hw *hw);
extern char e1000_driver_name[];
extern const char e1000_driver_version[];
+int e1000_open(struct net_device *netdev);
+int e1000_close(struct net_device *netdev);
int e1000_up(struct e1000_adapter *adapter);
void e1000_down(struct e1000_adapter *adapter);
void e1000_reinit_locked(struct e1000_adapter *adapter);
diff --git a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
index 83e557c..975eeb8 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
@@ -1553,7 +1553,7 @@ static void e1000_diag_test(struct net_device *netdev,
if (if_running)
/* indicate we're in test mode */
- dev_close(netdev);
+ e1000_close(netdev);
else
e1000_reset(adapter);
@@ -1582,7 +1582,7 @@ static void e1000_diag_test(struct net_device *netdev,
e1000_reset(adapter);
clear_bit(__E1000_TESTING, &adapter->flags);
if (if_running)
- dev_open(netdev);
+ e1000_open(netdev);
} else {
e_info(hw, "online testing starting\n");
/* Online tests */
diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
index 3fc7bde..6de0c7d 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -114,8 +114,8 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
static void e1000_remove(struct pci_dev *pdev);
static int e1000_alloc_queues(struct e1000_adapter *adapter);
static int e1000_sw_init(struct e1000_adapter *adapter);
-static int e1000_open(struct net_device *netdev);
-static int e1000_close(struct net_device *netdev);
+int e1000_open(struct net_device *netdev);
+int e1000_close(struct net_device *netdev);
static void e1000_configure_tx(struct e1000_adapter *adapter);
static void e1000_configure_rx(struct e1000_adapter *adapter);
static void e1000_setup_rctl(struct e1000_adapter *adapter);
@@ -1360,7 +1360,7 @@ static int e1000_alloc_queues(struct e1000_adapter *adapter)
* handler is registered with the OS, the watchdog task is started,
* and the stack is notified that the interface is ready.
**/
-static int e1000_open(struct net_device *netdev)
+int e1000_open(struct net_device *netdev)
{
struct e1000_adapter *adapter = netdev_priv(netdev);
struct e1000_hw *hw = &adapter->hw;
@@ -1437,7 +1437,7 @@ err_setup_tx:
* needs to be disabled. A global MAC reset is issued to stop the
* hardware, and all transmit and receive resources are freed.
**/
-static int e1000_close(struct net_device *netdev)
+int e1000_close(struct net_device *netdev)
{
struct e1000_adapter *adapter = netdev_priv(netdev);
struct e1000_hw *hw = &adapter->hw;
--
2.5.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [Intel-wired-lan] [PATCH net-next V2 5/6] e1000: call ndo_stop() instead of dev_close() when running offline selftest
@ 2016-02-03 8:20 ` Stefan Assmann
0 siblings, 0 replies; 31+ messages in thread
From: Stefan Assmann @ 2016-02-03 8:20 UTC (permalink / raw)
To: intel-wired-lan
Calling dev_close() causes IFF_UP to be cleared which will remove the
interfaces routes and some addresses. That's probably not what the user
intended when running the offline selftest. Besides this does not happen
if the interface is brought down before the test, so the current
behaviour is inconsistent.
Instead call the net_device_ops ndo_stop function directly and avoid
touching IFF_UP at all.
Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
---
drivers/net/ethernet/intel/e1000/e1000.h | 2 ++
drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 4 ++--
drivers/net/ethernet/intel/e1000/e1000_main.c | 8 ++++----
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000/e1000.h b/drivers/net/ethernet/intel/e1000/e1000.h
index 98fe5a2..d7bdea7 100644
--- a/drivers/net/ethernet/intel/e1000/e1000.h
+++ b/drivers/net/ethernet/intel/e1000/e1000.h
@@ -358,6 +358,8 @@ struct net_device *e1000_get_hw_dev(struct e1000_hw *hw);
extern char e1000_driver_name[];
extern const char e1000_driver_version[];
+int e1000_open(struct net_device *netdev);
+int e1000_close(struct net_device *netdev);
int e1000_up(struct e1000_adapter *adapter);
void e1000_down(struct e1000_adapter *adapter);
void e1000_reinit_locked(struct e1000_adapter *adapter);
diff --git a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
index 83e557c..975eeb8 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
@@ -1553,7 +1553,7 @@ static void e1000_diag_test(struct net_device *netdev,
if (if_running)
/* indicate we're in test mode */
- dev_close(netdev);
+ e1000_close(netdev);
else
e1000_reset(adapter);
@@ -1582,7 +1582,7 @@ static void e1000_diag_test(struct net_device *netdev,
e1000_reset(adapter);
clear_bit(__E1000_TESTING, &adapter->flags);
if (if_running)
- dev_open(netdev);
+ e1000_open(netdev);
} else {
e_info(hw, "online testing starting\n");
/* Online tests */
diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
index 3fc7bde..6de0c7d 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -114,8 +114,8 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
static void e1000_remove(struct pci_dev *pdev);
static int e1000_alloc_queues(struct e1000_adapter *adapter);
static int e1000_sw_init(struct e1000_adapter *adapter);
-static int e1000_open(struct net_device *netdev);
-static int e1000_close(struct net_device *netdev);
+int e1000_open(struct net_device *netdev);
+int e1000_close(struct net_device *netdev);
static void e1000_configure_tx(struct e1000_adapter *adapter);
static void e1000_configure_rx(struct e1000_adapter *adapter);
static void e1000_setup_rctl(struct e1000_adapter *adapter);
@@ -1360,7 +1360,7 @@ static int e1000_alloc_queues(struct e1000_adapter *adapter)
* handler is registered with the OS, the watchdog task is started,
* and the stack is notified that the interface is ready.
**/
-static int e1000_open(struct net_device *netdev)
+int e1000_open(struct net_device *netdev)
{
struct e1000_adapter *adapter = netdev_priv(netdev);
struct e1000_hw *hw = &adapter->hw;
@@ -1437,7 +1437,7 @@ err_setup_tx:
* needs to be disabled. A global MAC reset is issued to stop the
* hardware, and all transmit and receive resources are freed.
**/
-static int e1000_close(struct net_device *netdev)
+int e1000_close(struct net_device *netdev)
{
struct e1000_adapter *adapter = netdev_priv(netdev);
struct e1000_hw *hw = &adapter->hw;
--
2.5.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* RE: [Intel-wired-lan] [PATCH net-next V2 5/6] e1000: call ndo_stop() instead of dev_close() when running offline selftest
2016-02-03 8:20 ` [Intel-wired-lan] " Stefan Assmann
@ 2016-02-12 23:20 ` Brown, Aaron F
-1 siblings, 0 replies; 31+ messages in thread
From: Brown, Aaron F @ 2016-02-12 23:20 UTC (permalink / raw)
To: Stefan Assmann, intel-wired-lan; +Cc: netdev, davem
> From: Intel-wired-lan [intel-wired-lan-bounces@lists.osuosl.org] on behalf of Stefan Assmann [sassmann@kpanic.de]
> Sent: Wednesday, February 03, 2016 12:20 AM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; davem@davemloft.net; sassmann@kpanic.de
> Subject: [Intel-wired-lan] [PATCH net-next V2 5/6] e1000: call ndo_stop() instead of dev_close() when running offline selftest
>
> Calling dev_close() causes IFF_UP to be cleared which will remove the
> interfaces routes and some addresses. That's probably not what the user
> intended when running the offline selftest. Besides this does not happen
> if the interface is brought down before the test, so the current
> behaviour is inconsistent.
> Instead call the net_device_ops ndo_stop function directly and avoid
> touching IFF_UP at all.
>
> Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
> ---
> drivers/net/ethernet/intel/e1000/e1000.h | 2 ++
> drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 4 ++--
> drivers/net/ethernet/intel/e1000/e1000_main.c | 8 ++++----
> 3 files changed, 8 insertions(+), 6 deletions(-)
Checkpatch warns that externs should be avoided in .c files, but they pre-existed and are just being flagged due to the name changing, so...
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [Intel-wired-lan] [PATCH net-next V2 5/6] e1000: call ndo_stop() instead of dev_close() when running offline selftest
@ 2016-02-12 23:20 ` Brown, Aaron F
0 siblings, 0 replies; 31+ messages in thread
From: Brown, Aaron F @ 2016-02-12 23:20 UTC (permalink / raw)
To: intel-wired-lan
> From: Intel-wired-lan [intel-wired-lan-bounces at lists.osuosl.org] on behalf of Stefan Assmann [sassmann at kpanic.de]
> Sent: Wednesday, February 03, 2016 12:20 AM
> To: intel-wired-lan at lists.osuosl.org
> Cc: netdev at vger.kernel.org; davem at davemloft.net; sassmann at kpanic.de
> Subject: [Intel-wired-lan] [PATCH net-next V2 5/6] e1000: call ndo_stop() instead of dev_close() when running offline selftest
>
> Calling dev_close() causes IFF_UP to be cleared which will remove the
> interfaces routes and some addresses. That's probably not what the user
> intended when running the offline selftest. Besides this does not happen
> if the interface is brought down before the test, so the current
> behaviour is inconsistent.
> Instead call the net_device_ops ndo_stop function directly and avoid
> touching IFF_UP at all.
>
> Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
> ---
> drivers/net/ethernet/intel/e1000/e1000.h | 2 ++
> drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 4 ++--
> drivers/net/ethernet/intel/e1000/e1000_main.c | 8 ++++----
> 3 files changed, 8 insertions(+), 6 deletions(-)
Checkpatch warns that externs should be avoided in .c files, but they pre-existed and are just being flagged due to the name changing, so...
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next V2 5/6] e1000: call ndo_stop() instead of dev_close() when running offline selftest
2016-02-12 23:20 ` Brown, Aaron F
@ 2016-02-16 17:23 ` Rustad, Mark D
-1 siblings, 0 replies; 31+ messages in thread
From: Rustad, Mark D @ 2016-02-16 17:23 UTC (permalink / raw)
To: Stefan Assmann; +Cc: intel-wired-lan, netdev, davem, Brown, Aaron F
[-- Attachment #1: Type: text/plain, Size: 1520 bytes --]
Aaron F <aaron.f.brown@intel.com> wrote:
>> From: Intel-wired-lan [intel-wired-lan-bounces@lists.osuosl.org] on
>> behalf of Stefan Assmann [sassmann@kpanic.de]
>> Sent: Wednesday, February 03, 2016 12:20 AM
>> To: intel-wired-lan@lists.osuosl.org
>> Cc: netdev@vger.kernel.org; davem@davemloft.net; sassmann@kpanic.de
>> Subject: [Intel-wired-lan] [PATCH net-next V2 5/6] e1000: call
>> ndo_stop() instead of dev_close() when running offline selftest
>>
>> Calling dev_close() causes IFF_UP to be cleared which will remove the
>> interfaces routes and some addresses. That's probably not what the user
>> intended when running the offline selftest. Besides this does not happen
>> if the interface is brought down before the test, so the current
>> behaviour is inconsistent.
>> Instead call the net_device_ops ndo_stop function directly and avoid
>> touching IFF_UP at all.
>>
>> Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
>> ---
>> drivers/net/ethernet/intel/e1000/e1000.h | 2 ++
>> drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 4 ++--
>> drivers/net/ethernet/intel/e1000/e1000_main.c | 8 ++++----
>> 3 files changed, 8 insertions(+), 6 deletions(-)
>
> Checkpatch warns that externs should be avoided in .c files, but they
> pre-existed and are just being flagged due to the name changing, so...
>
> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Actually, it is the forward declarations in the .c that should be deleted.
The prototypes should only exist in the .h file.
^ permalink raw reply [flat|nested] 31+ messages in thread
* [Intel-wired-lan] [PATCH net-next V2 5/6] e1000: call ndo_stop() instead of dev_close() when running offline selftest
@ 2016-02-16 17:23 ` Rustad, Mark D
0 siblings, 0 replies; 31+ messages in thread
From: Rustad, Mark D @ 2016-02-16 17:23 UTC (permalink / raw)
To: intel-wired-lan
Aaron F <aaron.f.brown@intel.com> wrote:
>> From: Intel-wired-lan [intel-wired-lan-bounces at lists.osuosl.org] on
>> behalf of Stefan Assmann [sassmann at kpanic.de]
>> Sent: Wednesday, February 03, 2016 12:20 AM
>> To: intel-wired-lan at lists.osuosl.org
>> Cc: netdev at vger.kernel.org; davem at davemloft.net; sassmann at kpanic.de
>> Subject: [Intel-wired-lan] [PATCH net-next V2 5/6] e1000: call
>> ndo_stop() instead of dev_close() when running offline selftest
>>
>> Calling dev_close() causes IFF_UP to be cleared which will remove the
>> interfaces routes and some addresses. That's probably not what the user
>> intended when running the offline selftest. Besides this does not happen
>> if the interface is brought down before the test, so the current
>> behaviour is inconsistent.
>> Instead call the net_device_ops ndo_stop function directly and avoid
>> touching IFF_UP at all.
>>
>> Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
>> ---
>> drivers/net/ethernet/intel/e1000/e1000.h | 2 ++
>> drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 4 ++--
>> drivers/net/ethernet/intel/e1000/e1000_main.c | 8 ++++----
>> 3 files changed, 8 insertions(+), 6 deletions(-)
>
> Checkpatch warns that externs should be avoided in .c files, but they
> pre-existed and are just being flagged due to the name changing, so...
>
> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Actually, it is the forward declarations in the .c that should be deleted.
The prototypes should only exist in the .h file.
--
Mark Rustad, Networking Division, Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 841 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20160216/1434b2f9/attachment.asc>
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [Intel-wired-lan] [PATCH net-next V2 5/6] e1000: call ndo_stop() instead of dev_close() when running offline selftest
2016-02-16 17:23 ` Rustad, Mark D
@ 2016-02-16 20:02 ` Brown, Aaron F
-1 siblings, 0 replies; 31+ messages in thread
From: Brown, Aaron F @ 2016-02-16 20:02 UTC (permalink / raw)
To: Rustad, Mark D, Stefan Assmann; +Cc: intel-wired-lan, netdev, davem
> From: Rustad, Mark D
> Sent: Tuesday, February 16, 2016 9:23 AM
> To: Stefan Assmann
> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; davem@davemloft.net; Brown, Aaron F
> Subject: Re: [Intel-wired-lan] [PATCH net-next V2 5/6] e1000: call ndo_stop() instead of dev_close() when running offline selftest
>
> > Checkpatch warns that externs should be avoided in .c files, but they
> > pre-existed and are just being flagged due to the name changing, so...
> >
> > Tested-by: Aaron Brown <aaron.f.brown@intel.com>
>
> Actually, it is the forward declarations in the .c that should be deleted.
> The prototypes should only exist in the .h file.
Thanks Mark, I completely missed that the patch added it to both files, too busy reviving older hardware to extend to run the diags against a larger set of parts I guess :")
^ permalink raw reply [flat|nested] 31+ messages in thread
* [Intel-wired-lan] [PATCH net-next V2 5/6] e1000: call ndo_stop() instead of dev_close() when running offline selftest
@ 2016-02-16 20:02 ` Brown, Aaron F
0 siblings, 0 replies; 31+ messages in thread
From: Brown, Aaron F @ 2016-02-16 20:02 UTC (permalink / raw)
To: intel-wired-lan
> From: Rustad, Mark D
> Sent: Tuesday, February 16, 2016 9:23 AM
> To: Stefan Assmann
> Cc: intel-wired-lan at lists.osuosl.org; netdev at vger.kernel.org; davem at davemloft.net; Brown, Aaron F
> Subject: Re: [Intel-wired-lan] [PATCH net-next V2 5/6] e1000: call ndo_stop() instead of dev_close() when running offline selftest
>
> > Checkpatch warns that externs should be avoided in .c files, but they
> > pre-existed and are just being flagged due to the name changing, so...
> >
> > Tested-by: Aaron Brown <aaron.f.brown@intel.com>
>
> Actually, it is the forward declarations in the .c that should be deleted.
> The prototypes should only exist in the .h file.
Thanks Mark, I completely missed that the patch added it to both files, too busy reviving older hardware to extend to run the diags against a larger set of parts I guess :")
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH net-next V2 6/6] e1000e: call ndo_stop() instead of dev_close() when running offline selftest
2016-02-03 8:20 ` [Intel-wired-lan] " Stefan Assmann
@ 2016-02-03 8:20 ` Stefan Assmann
-1 siblings, 0 replies; 31+ messages in thread
From: Stefan Assmann @ 2016-02-03 8:20 UTC (permalink / raw)
To: intel-wired-lan; +Cc: netdev, davem, jeffrey.t.kirsher, sassmann
Calling dev_close() causes IFF_UP to be cleared which will remove the
interfaces routes and some addresses. That's probably not what the user
intended when running the offline selftest. Besides this does not happen
if the interface is brought down before the test, so the current
behaviour is inconsistent.
Instead call the net_device_ops ndo_stop function directly and avoid
touching IFF_UP at all.
V2: rename e1000_open(), e1000_close() to e1000e_open(), e1000e_close()
to avoid name clash with e1000.
Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
---
drivers/net/ethernet/intel/e1000e/e1000.h | 2 ++
drivers/net/ethernet/intel/e1000e/ethtool.c | 4 ++--
drivers/net/ethernet/intel/e1000e/netdev.c | 12 ++++++------
3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h b/drivers/net/ethernet/intel/e1000e/e1000.h
index 1dc293b..52eb641 100644
--- a/drivers/net/ethernet/intel/e1000e/e1000.h
+++ b/drivers/net/ethernet/intel/e1000e/e1000.h
@@ -480,6 +480,8 @@ extern const char e1000e_driver_version[];
void e1000e_check_options(struct e1000_adapter *adapter);
void e1000e_set_ethtool_ops(struct net_device *netdev);
+int e1000e_open(struct net_device *netdev);
+int e1000e_close(struct net_device *netdev);
void e1000e_up(struct e1000_adapter *adapter);
void e1000e_down(struct e1000_adapter *adapter, bool reset);
void e1000e_reinit_locked(struct e1000_adapter *adapter);
diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c
index 6cab1f3..1e3973a 100644
--- a/drivers/net/ethernet/intel/e1000e/ethtool.c
+++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
@@ -1816,7 +1816,7 @@ static void e1000_diag_test(struct net_device *netdev,
if (if_running)
/* indicate we're in test mode */
- dev_close(netdev);
+ e1000e_close(netdev);
if (e1000_reg_test(adapter, &data[0]))
eth_test->flags |= ETH_TEST_FL_FAILED;
@@ -1849,7 +1849,7 @@ static void e1000_diag_test(struct net_device *netdev,
clear_bit(__E1000_TESTING, &adapter->state);
if (if_running)
- dev_open(netdev);
+ e1000e_open(netdev);
} else {
/* Online tests */
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index c71ba1b..02449a0 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -4495,7 +4495,7 @@ static int e1000_test_msi(struct e1000_adapter *adapter)
}
/**
- * e1000_open - Called when a network interface is made active
+ * e1000e_open - Called when a network interface is made active
* @netdev: network interface device structure
*
* Returns 0 on success, negative value on failure
@@ -4506,7 +4506,7 @@ static int e1000_test_msi(struct e1000_adapter *adapter)
* handler is registered with the OS, the watchdog timer is started,
* and the stack is notified that the interface is ready.
**/
-static int e1000_open(struct net_device *netdev)
+int e1000e_open(struct net_device *netdev)
{
struct e1000_adapter *adapter = netdev_priv(netdev);
struct e1000_hw *hw = &adapter->hw;
@@ -4604,7 +4604,7 @@ err_setup_tx:
}
/**
- * e1000_close - Disables a network interface
+ * e1000e_close - Disables a network interface
* @netdev: network interface device structure
*
* Returns 0, this is not allowed to fail
@@ -4614,7 +4614,7 @@ err_setup_tx:
* needs to be disabled. A global MAC reset is issued to stop the
* hardware, and all transmit and receive resources are freed.
**/
-static int e1000_close(struct net_device *netdev)
+int e1000e_close(struct net_device *netdev)
{
struct e1000_adapter *adapter = netdev_priv(netdev);
struct pci_dev *pdev = adapter->pdev;
@@ -6920,8 +6920,8 @@ static int e1000_set_features(struct net_device *netdev,
}
static const struct net_device_ops e1000e_netdev_ops = {
- .ndo_open = e1000_open,
- .ndo_stop = e1000_close,
+ .ndo_open = e1000e_open,
+ .ndo_stop = e1000e_close,
.ndo_start_xmit = e1000_xmit_frame,
.ndo_get_stats64 = e1000e_get_stats64,
.ndo_set_rx_mode = e1000e_set_rx_mode,
--
2.5.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [Intel-wired-lan] [PATCH net-next V2 6/6] e1000e: call ndo_stop() instead of dev_close() when running offline selftest
@ 2016-02-03 8:20 ` Stefan Assmann
0 siblings, 0 replies; 31+ messages in thread
From: Stefan Assmann @ 2016-02-03 8:20 UTC (permalink / raw)
To: intel-wired-lan
Calling dev_close() causes IFF_UP to be cleared which will remove the
interfaces routes and some addresses. That's probably not what the user
intended when running the offline selftest. Besides this does not happen
if the interface is brought down before the test, so the current
behaviour is inconsistent.
Instead call the net_device_ops ndo_stop function directly and avoid
touching IFF_UP at all.
V2: rename e1000_open(), e1000_close() to e1000e_open(), e1000e_close()
to avoid name clash with e1000.
Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
---
drivers/net/ethernet/intel/e1000e/e1000.h | 2 ++
drivers/net/ethernet/intel/e1000e/ethtool.c | 4 ++--
drivers/net/ethernet/intel/e1000e/netdev.c | 12 ++++++------
3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h b/drivers/net/ethernet/intel/e1000e/e1000.h
index 1dc293b..52eb641 100644
--- a/drivers/net/ethernet/intel/e1000e/e1000.h
+++ b/drivers/net/ethernet/intel/e1000e/e1000.h
@@ -480,6 +480,8 @@ extern const char e1000e_driver_version[];
void e1000e_check_options(struct e1000_adapter *adapter);
void e1000e_set_ethtool_ops(struct net_device *netdev);
+int e1000e_open(struct net_device *netdev);
+int e1000e_close(struct net_device *netdev);
void e1000e_up(struct e1000_adapter *adapter);
void e1000e_down(struct e1000_adapter *adapter, bool reset);
void e1000e_reinit_locked(struct e1000_adapter *adapter);
diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c
index 6cab1f3..1e3973a 100644
--- a/drivers/net/ethernet/intel/e1000e/ethtool.c
+++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
@@ -1816,7 +1816,7 @@ static void e1000_diag_test(struct net_device *netdev,
if (if_running)
/* indicate we're in test mode */
- dev_close(netdev);
+ e1000e_close(netdev);
if (e1000_reg_test(adapter, &data[0]))
eth_test->flags |= ETH_TEST_FL_FAILED;
@@ -1849,7 +1849,7 @@ static void e1000_diag_test(struct net_device *netdev,
clear_bit(__E1000_TESTING, &adapter->state);
if (if_running)
- dev_open(netdev);
+ e1000e_open(netdev);
} else {
/* Online tests */
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index c71ba1b..02449a0 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -4495,7 +4495,7 @@ static int e1000_test_msi(struct e1000_adapter *adapter)
}
/**
- * e1000_open - Called when a network interface is made active
+ * e1000e_open - Called when a network interface is made active
* @netdev: network interface device structure
*
* Returns 0 on success, negative value on failure
@@ -4506,7 +4506,7 @@ static int e1000_test_msi(struct e1000_adapter *adapter)
* handler is registered with the OS, the watchdog timer is started,
* and the stack is notified that the interface is ready.
**/
-static int e1000_open(struct net_device *netdev)
+int e1000e_open(struct net_device *netdev)
{
struct e1000_adapter *adapter = netdev_priv(netdev);
struct e1000_hw *hw = &adapter->hw;
@@ -4604,7 +4604,7 @@ err_setup_tx:
}
/**
- * e1000_close - Disables a network interface
+ * e1000e_close - Disables a network interface
* @netdev: network interface device structure
*
* Returns 0, this is not allowed to fail
@@ -4614,7 +4614,7 @@ err_setup_tx:
* needs to be disabled. A global MAC reset is issued to stop the
* hardware, and all transmit and receive resources are freed.
**/
-static int e1000_close(struct net_device *netdev)
+int e1000e_close(struct net_device *netdev)
{
struct e1000_adapter *adapter = netdev_priv(netdev);
struct pci_dev *pdev = adapter->pdev;
@@ -6920,8 +6920,8 @@ static int e1000_set_features(struct net_device *netdev,
}
static const struct net_device_ops e1000e_netdev_ops = {
- .ndo_open = e1000_open,
- .ndo_stop = e1000_close,
+ .ndo_open = e1000e_open,
+ .ndo_stop = e1000e_close,
.ndo_start_xmit = e1000_xmit_frame,
.ndo_get_stats64 = e1000e_get_stats64,
.ndo_set_rx_mode = e1000e_set_rx_mode,
--
2.5.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* RE: [Intel-wired-lan] [PATCH net-next V2 6/6] e1000e: call ndo_stop() instead of dev_close() when running offline selftest
2016-02-03 8:20 ` [Intel-wired-lan] " Stefan Assmann
@ 2016-02-09 12:30 ` Avargil, Raanan
-1 siblings, 0 replies; 31+ messages in thread
From: Avargil, Raanan @ 2016-02-09 12:30 UTC (permalink / raw)
To: Stefan Assmann, intel-wired-lan; +Cc: netdev, davem
Acked-by: Raanan Avargil <Raanan.avargil@intel.com>
--
Regards,
Raanan
-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On Behalf Of Stefan Assmann
Sent: Wednesday, February 03, 2016 10:21
To: intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org; davem@davemloft.net; sassmann@kpanic.de
Subject: [Intel-wired-lan] [PATCH net-next V2 6/6] e1000e: call ndo_stop() instead of dev_close() when running offline selftest
Importance: High
Calling dev_close() causes IFF_UP to be cleared which will remove the interfaces routes and some addresses. That's probably not what the user intended when running the offline selftest. Besides this does not happen if the interface is brought down before the test, so the current behaviour is inconsistent.
Instead call the net_device_ops ndo_stop function directly and avoid touching IFF_UP at all.
V2: rename e1000_open(), e1000_close() to e1000e_open(), e1000e_close() to avoid name clash with e1000.
Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
---
drivers/net/ethernet/intel/e1000e/e1000.h | 2 ++
drivers/net/ethernet/intel/e1000e/ethtool.c | 4 ++-- drivers/net/ethernet/intel/e1000e/netdev.c | 12 ++++++------
3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h b/drivers/net/ethernet/intel/e1000e/e1000.h
index 1dc293b..52eb641 100644
--- a/drivers/net/ethernet/intel/e1000e/e1000.h
+++ b/drivers/net/ethernet/intel/e1000e/e1000.h
@@ -480,6 +480,8 @@ extern const char e1000e_driver_version[]; void e1000e_check_options(struct e1000_adapter *adapter); void e1000e_set_ethtool_ops(struct net_device *netdev);
+int e1000e_open(struct net_device *netdev); int e1000e_close(struct
+net_device *netdev);
void e1000e_up(struct e1000_adapter *adapter); void e1000e_down(struct e1000_adapter *adapter, bool reset); void e1000e_reinit_locked(struct e1000_adapter *adapter); diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c
index 6cab1f3..1e3973a 100644
--- a/drivers/net/ethernet/intel/e1000e/ethtool.c
+++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
@@ -1816,7 +1816,7 @@ static void e1000_diag_test(struct net_device *netdev,
if (if_running)
/* indicate we're in test mode */
- dev_close(netdev);
+ e1000e_close(netdev);
if (e1000_reg_test(adapter, &data[0]))
eth_test->flags |= ETH_TEST_FL_FAILED; @@ -1849,7 +1849,7 @@ static void e1000_diag_test(struct net_device *netdev,
clear_bit(__E1000_TESTING, &adapter->state);
if (if_running)
- dev_open(netdev);
+ e1000e_open(netdev);
} else {
/* Online tests */
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index c71ba1b..02449a0 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -4495,7 +4495,7 @@ static int e1000_test_msi(struct e1000_adapter *adapter) }
/**
- * e1000_open - Called when a network interface is made active
+ * e1000e_open - Called when a network interface is made active
* @netdev: network interface device structure
*
* Returns 0 on success, negative value on failure @@ -4506,7 +4506,7 @@ static int e1000_test_msi(struct e1000_adapter *adapter)
* handler is registered with the OS, the watchdog timer is started,
* and the stack is notified that the interface is ready.
**/
-static int e1000_open(struct net_device *netdev)
+int e1000e_open(struct net_device *netdev)
{
struct e1000_adapter *adapter = netdev_priv(netdev);
struct e1000_hw *hw = &adapter->hw;
@@ -4604,7 +4604,7 @@ err_setup_tx:
}
/**
- * e1000_close - Disables a network interface
+ * e1000e_close - Disables a network interface
* @netdev: network interface device structure
*
* Returns 0, this is not allowed to fail @@ -4614,7 +4614,7 @@ err_setup_tx:
* needs to be disabled. A global MAC reset is issued to stop the
* hardware, and all transmit and receive resources are freed.
**/
-static int e1000_close(struct net_device *netdev)
+int e1000e_close(struct net_device *netdev)
{
struct e1000_adapter *adapter = netdev_priv(netdev);
struct pci_dev *pdev = adapter->pdev;
@@ -6920,8 +6920,8 @@ static int e1000_set_features(struct net_device *netdev, }
static const struct net_device_ops e1000e_netdev_ops = {
- .ndo_open = e1000_open,
- .ndo_stop = e1000_close,
+ .ndo_open = e1000e_open,
+ .ndo_stop = e1000e_close,
.ndo_start_xmit = e1000_xmit_frame,
.ndo_get_stats64 = e1000e_get_stats64,
.ndo_set_rx_mode = e1000e_set_rx_mode,
^ permalink raw reply related [flat|nested] 31+ messages in thread* [Intel-wired-lan] [PATCH net-next V2 6/6] e1000e: call ndo_stop() instead of dev_close() when running offline selftest
@ 2016-02-09 12:30 ` Avargil, Raanan
0 siblings, 0 replies; 31+ messages in thread
From: Avargil, Raanan @ 2016-02-09 12:30 UTC (permalink / raw)
To: intel-wired-lan
Acked-by: Raanan Avargil <Raanan.avargil@intel.com>
--
Regards,
Raanan
-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Stefan Assmann
Sent: Wednesday, February 03, 2016 10:21
To: intel-wired-lan@lists.osuosl.org
Cc: netdev at vger.kernel.org; davem at davemloft.net; sassmann at kpanic.de
Subject: [Intel-wired-lan] [PATCH net-next V2 6/6] e1000e: call ndo_stop() instead of dev_close() when running offline selftest
Importance: High
Calling dev_close() causes IFF_UP to be cleared which will remove the interfaces routes and some addresses. That's probably not what the user intended when running the offline selftest. Besides this does not happen if the interface is brought down before the test, so the current behaviour is inconsistent.
Instead call the net_device_ops ndo_stop function directly and avoid touching IFF_UP at all.
V2: rename e1000_open(), e1000_close() to e1000e_open(), e1000e_close() to avoid name clash with e1000.
Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
---
drivers/net/ethernet/intel/e1000e/e1000.h | 2 ++
drivers/net/ethernet/intel/e1000e/ethtool.c | 4 ++-- drivers/net/ethernet/intel/e1000e/netdev.c | 12 ++++++------
3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h b/drivers/net/ethernet/intel/e1000e/e1000.h
index 1dc293b..52eb641 100644
--- a/drivers/net/ethernet/intel/e1000e/e1000.h
+++ b/drivers/net/ethernet/intel/e1000e/e1000.h
@@ -480,6 +480,8 @@ extern const char e1000e_driver_version[]; void e1000e_check_options(struct e1000_adapter *adapter); void e1000e_set_ethtool_ops(struct net_device *netdev);
+int e1000e_open(struct net_device *netdev); int e1000e_close(struct
+net_device *netdev);
void e1000e_up(struct e1000_adapter *adapter); void e1000e_down(struct e1000_adapter *adapter, bool reset); void e1000e_reinit_locked(struct e1000_adapter *adapter); diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c
index 6cab1f3..1e3973a 100644
--- a/drivers/net/ethernet/intel/e1000e/ethtool.c
+++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
@@ -1816,7 +1816,7 @@ static void e1000_diag_test(struct net_device *netdev,
if (if_running)
/* indicate we're in test mode */
- dev_close(netdev);
+ e1000e_close(netdev);
if (e1000_reg_test(adapter, &data[0]))
eth_test->flags |= ETH_TEST_FL_FAILED; @@ -1849,7 +1849,7 @@ static void e1000_diag_test(struct net_device *netdev,
clear_bit(__E1000_TESTING, &adapter->state);
if (if_running)
- dev_open(netdev);
+ e1000e_open(netdev);
} else {
/* Online tests */
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index c71ba1b..02449a0 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -4495,7 +4495,7 @@ static int e1000_test_msi(struct e1000_adapter *adapter) }
/**
- * e1000_open - Called when a network interface is made active
+ * e1000e_open - Called when a network interface is made active
* @netdev: network interface device structure
*
* Returns 0 on success, negative value on failure @@ -4506,7 +4506,7 @@ static int e1000_test_msi(struct e1000_adapter *adapter)
* handler is registered with the OS, the watchdog timer is started,
* and the stack is notified that the interface is ready.
**/
-static int e1000_open(struct net_device *netdev)
+int e1000e_open(struct net_device *netdev)
{
struct e1000_adapter *adapter = netdev_priv(netdev);
struct e1000_hw *hw = &adapter->hw;
@@ -4604,7 +4604,7 @@ err_setup_tx:
}
/**
- * e1000_close - Disables a network interface
+ * e1000e_close - Disables a network interface
* @netdev: network interface device structure
*
* Returns 0, this is not allowed to fail @@ -4614,7 +4614,7 @@ err_setup_tx:
* needs to be disabled. A global MAC reset is issued to stop the
* hardware, and all transmit and receive resources are freed.
**/
-static int e1000_close(struct net_device *netdev)
+int e1000e_close(struct net_device *netdev)
{
struct e1000_adapter *adapter = netdev_priv(netdev);
struct pci_dev *pdev = adapter->pdev;
@@ -6920,8 +6920,8 @@ static int e1000_set_features(struct net_device *netdev, }
static const struct net_device_ops e1000e_netdev_ops = {
- .ndo_open = e1000_open,
- .ndo_stop = e1000_close,
+ .ndo_open = e1000e_open,
+ .ndo_stop = e1000e_close,
.ndo_start_xmit = e1000_xmit_frame,
.ndo_get_stats64 = e1000e_get_stats64,
.ndo_set_rx_mode = e1000e_set_rx_mode,
--
2.5.0
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan
---------------------------------------------------------------------
Intel Israel (74) Limited
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
^ permalink raw reply related [flat|nested] 31+ messages in thread
* RE: [Intel-wired-lan] [PATCH net-next V2 6/6] e1000e: call ndo_stop() instead of dev_close() when running offline selftest
2016-02-03 8:20 ` [Intel-wired-lan] " Stefan Assmann
@ 2016-02-12 23:17 ` Brown, Aaron F
-1 siblings, 0 replies; 31+ messages in thread
From: Brown, Aaron F @ 2016-02-12 23:17 UTC (permalink / raw)
To: Stefan Assmann, intel-wired-lan; +Cc: netdev, davem
> From: Intel-wired-lan [intel-wired-lan-bounces@lists.osuosl.org] on behalf of Stefan Assmann [sassmann@kpanic.de]
> Sent: Wednesday, February 03, 2016 12:20 AM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; davem@davemloft.net; sassmann@kpanic.de
> Subject: [Intel-wired-lan] [PATCH net-next V2 6/6] e1000e: call ndo_stop() instead of dev_close() when running offline selftest
>
> Calling dev_close() causes IFF_UP to be cleared which will remove the
> interfaces routes and some addresses. That's probably not what the user
> intended when running the offline selftest. Besides this does not happen
> if the interface is brought down before the test, so the current
> behaviour is inconsistent.
> Instead call the net_device_ops ndo_stop function directly and avoid
> touching IFF_UP at all.
>
> V2: rename e1000_open(), e1000_close() to e1000e_open(), e1000e_close()
> to avoid name clash with e1000.
>
> Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
> ---
> drivers/net/ethernet/intel/e1000e/e1000.h | 2 ++
> drivers/net/ethernet/intel/e1000e/ethtool.c | 4 ++--
> drivers/net/ethernet/intel/e1000e/netdev.c | 12 ++++++------
> 3 files changed, 10 insertions(+), 8 deletions(-)
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [Intel-wired-lan] [PATCH net-next V2 6/6] e1000e: call ndo_stop() instead of dev_close() when running offline selftest
@ 2016-02-12 23:17 ` Brown, Aaron F
0 siblings, 0 replies; 31+ messages in thread
From: Brown, Aaron F @ 2016-02-12 23:17 UTC (permalink / raw)
To: intel-wired-lan
> From: Intel-wired-lan [intel-wired-lan-bounces at lists.osuosl.org] on behalf of Stefan Assmann [sassmann at kpanic.de]
> Sent: Wednesday, February 03, 2016 12:20 AM
> To: intel-wired-lan at lists.osuosl.org
> Cc: netdev at vger.kernel.org; davem at davemloft.net; sassmann at kpanic.de
> Subject: [Intel-wired-lan] [PATCH net-next V2 6/6] e1000e: call ndo_stop() instead of dev_close() when running offline selftest
>
> Calling dev_close() causes IFF_UP to be cleared which will remove the
> interfaces routes and some addresses. That's probably not what the user
> intended when running the offline selftest. Besides this does not happen
> if the interface is brought down before the test, so the current
> behaviour is inconsistent.
> Instead call the net_device_ops ndo_stop function directly and avoid
> touching IFF_UP at all.
>
> V2: rename e1000_open(), e1000_close() to e1000e_open(), e1000e_close()
> to avoid name clash with e1000.
>
> Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
> ---
> drivers/net/ethernet/intel/e1000e/e1000.h | 2 ++
> drivers/net/ethernet/intel/e1000e/ethtool.c | 4 ++--
> drivers/net/ethernet/intel/e1000e/netdev.c | 12 ++++++------
> 3 files changed, 10 insertions(+), 8 deletions(-)
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
^ permalink raw reply [flat|nested] 31+ messages in thread