* [PATCH] vmxnet3: set carrier state properly on probe
@ 2013-01-25 20:54 Neil Horman
2013-01-29 17:54 ` Ben Hutchings
2013-01-29 18:36 ` [PATCH v2] " Neil Horman
0 siblings, 2 replies; 12+ messages in thread
From: Neil Horman @ 2013-01-25 20:54 UTC (permalink / raw)
To: netdev; +Cc: Neil Horman, David S. Miller, VMware, Inc.
vmxnet3 fails to set netif_carrier_off on probe, meaning that when an interface
is opened the __LINK_STATE_NOCARRIER bit is already cleared, and so
/sys/class/net/<ifname>/operstate remains in the unknown state. Correct this by
setting netif_carrier_off on probe, like other drivers do.
Also, while we're at it, lets remove the netif_carrier_ok checks from the
link_state_update function, as that check is atomically contained within the
netif_carrier_[on|off] functions anyway
Tested successfully by myself
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: "David S. Miller" <davem@davemloft.net>
CC: "VMware, Inc." <pv-drivers@vmware.com>
---
drivers/net/vmxnet3/vmxnet3_drv.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index b1c90f8..66c26a9 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -150,8 +150,7 @@ vmxnet3_check_link(struct vmxnet3_adapter *adapter, bool affectTxQueue)
if (ret & 1) { /* Link is up. */
netdev_info(adapter->netdev, "NIC Link is Up %d Mbps\n",
adapter->link_speed);
- if (!netif_carrier_ok(adapter->netdev))
- netif_carrier_on(adapter->netdev);
+ netif_carrier_on(adapter->netdev);
if (affectTxQueue) {
for (i = 0; i < adapter->num_tx_queues; i++)
@@ -160,8 +159,7 @@ vmxnet3_check_link(struct vmxnet3_adapter *adapter, bool affectTxQueue)
}
} else {
netdev_info(adapter->netdev, "NIC Link is Down\n");
- if (netif_carrier_ok(adapter->netdev))
- netif_carrier_off(adapter->netdev);
+ netif_carrier_off(adapter->netdev);
if (affectTxQueue) {
for (i = 0; i < adapter->num_tx_queues; i++)
@@ -3067,6 +3065,7 @@ vmxnet3_probe_device(struct pci_dev *pdev,
goto err_register;
}
+ netif_carrier_off(netdev);
vmxnet3_check_link(adapter, false);
return 0;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] vmxnet3: set carrier state properly on probe
2013-01-25 20:54 [PATCH] vmxnet3: set carrier state properly on probe Neil Horman
@ 2013-01-29 17:54 ` Ben Hutchings
2013-01-29 18:54 ` [Pv-drivers] " Dmitry Torokhov
2013-01-29 18:36 ` [PATCH v2] " Neil Horman
1 sibling, 1 reply; 12+ messages in thread
From: Ben Hutchings @ 2013-01-29 17:54 UTC (permalink / raw)
To: Neil Horman; +Cc: netdev, David S. Miller, VMware, Inc.
On Fri, 2013-01-25 at 15:54 -0500, Neil Horman wrote:
> vmxnet3 fails to set netif_carrier_off on probe, meaning that when an interface
> is opened the __LINK_STATE_NOCARRIER bit is already cleared, and so
> /sys/class/net/<ifname>/operstate remains in the unknown state. Correct this by
> setting netif_carrier_off on probe, like other drivers do.
>
> Also, while we're at it, lets remove the netif_carrier_ok checks from the
> link_state_update function, as that check is atomically contained within the
> netif_carrier_[on|off] functions anyway
>
> Tested successfully by myself
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: "David S. Miller" <davem@davemloft.net>
> CC: "VMware, Inc." <pv-drivers@vmware.com>
> ---
> drivers/net/vmxnet3/vmxnet3_drv.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
> index b1c90f8..66c26a9 100644
> --- a/drivers/net/vmxnet3/vmxnet3_drv.c
> +++ b/drivers/net/vmxnet3/vmxnet3_drv.c
[...]
> @@ -3067,6 +3065,7 @@ vmxnet3_probe_device(struct pci_dev *pdev,
> goto err_register;
> }
>
> + netif_carrier_off(netdev);
> vmxnet3_check_link(adapter, false);
> return 0;
>
You should do this before calling register_netdev(), otherwise the link
state change can race with vmxnet3_open().
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2] vmxnet3: set carrier state properly on probe
2013-01-25 20:54 [PATCH] vmxnet3: set carrier state properly on probe Neil Horman
2013-01-29 17:54 ` Ben Hutchings
@ 2013-01-29 18:36 ` Neil Horman
2013-01-29 19:54 ` [Pv-drivers] " Dmitry Torokhov
2013-01-29 20:44 ` David Miller
1 sibling, 2 replies; 12+ messages in thread
From: Neil Horman @ 2013-01-29 18:36 UTC (permalink / raw)
To: netdev; +Cc: Neil Horman, David S. Miller, VMware, Inc., Ben Hutchings
vmxnet3 fails to set netif_carrier_off on probe, meaning that when an interface
is opened the __LINK_STATE_NOCARRIER bit is already cleared, and so
/sys/class/net/<ifname>/operstate remains in the unknown state. Correct this by
setting netif_carrier_off on probe, like other drivers do.
Also, while we're at it, lets remove the netif_carrier_ok checks from the
link_state_update function, as that check is atomically contained within the
netif_carrier_[on|off] functions anyway
Tested successfully by myself
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: "David S. Miller" <davem@davemloft.net>
CC: "VMware, Inc." <pv-drivers@vmware.com>
CC: Ben Hutchings <bhutchings@solarflare.com>
---
Change notes:
v2) Moved netif_carrier_off above register_netdev to prevent race with dev_open
as per Ben H.
---
drivers/net/vmxnet3/vmxnet3_drv.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index b1c90f8..ffb97b2 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -150,8 +150,7 @@ vmxnet3_check_link(struct vmxnet3_adapter *adapter, bool affectTxQueue)
if (ret & 1) { /* Link is up. */
netdev_info(adapter->netdev, "NIC Link is Up %d Mbps\n",
adapter->link_speed);
- if (!netif_carrier_ok(adapter->netdev))
- netif_carrier_on(adapter->netdev);
+ netif_carrier_on(adapter->netdev);
if (affectTxQueue) {
for (i = 0; i < adapter->num_tx_queues; i++)
@@ -160,8 +159,7 @@ vmxnet3_check_link(struct vmxnet3_adapter *adapter, bool affectTxQueue)
}
} else {
netdev_info(adapter->netdev, "NIC Link is Down\n");
- if (netif_carrier_ok(adapter->netdev))
- netif_carrier_off(adapter->netdev);
+ netif_carrier_off(adapter->netdev);
if (affectTxQueue) {
for (i = 0; i < adapter->num_tx_queues; i++)
@@ -3060,6 +3058,7 @@ vmxnet3_probe_device(struct pci_dev *pdev,
netif_set_real_num_tx_queues(adapter->netdev, adapter->num_tx_queues);
netif_set_real_num_rx_queues(adapter->netdev, adapter->num_rx_queues);
+ netif_carrier_off(netdev);
err = register_netdev(netdev);
if (err) {
--
1.7.11.7
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Pv-drivers] [PATCH] vmxnet3: set carrier state properly on probe
2013-01-29 17:54 ` Ben Hutchings
@ 2013-01-29 18:54 ` Dmitry Torokhov
2013-01-29 19:09 ` Neil Horman
0 siblings, 1 reply; 12+ messages in thread
From: Dmitry Torokhov @ 2013-01-29 18:54 UTC (permalink / raw)
To: pv-drivers; +Cc: Ben Hutchings, Neil Horman, netdev, David S. Miller
On Tuesday, January 29, 2013 05:54:11 PM Ben Hutchings wrote:
> On Fri, 2013-01-25 at 15:54 -0500, Neil Horman wrote:
> > vmxnet3 fails to set netif_carrier_off on probe, meaning that when an
> > interface is opened the __LINK_STATE_NOCARRIER bit is already cleared,
> > and so /sys/class/net/<ifname>/operstate remains in the unknown state.
> > Correct this by setting netif_carrier_off on probe, like other drivers
> > do.
> >
> > Also, while we're at it, lets remove the netif_carrier_ok checks from the
> > link_state_update function, as that check is atomically contained within
> > the netif_carrier_[on|off] functions anyway
> >
> > Tested successfully by myself
> >
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > CC: "David S. Miller" <davem@davemloft.net>
> > CC: "VMware, Inc." <pv-drivers@vmware.com>
> > ---
> >
> > drivers/net/vmxnet3/vmxnet3_drv.c | 7 +++----
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c
> > b/drivers/net/vmxnet3/vmxnet3_drv.c index b1c90f8..66c26a9 100644
> > --- a/drivers/net/vmxnet3/vmxnet3_drv.c
> > +++ b/drivers/net/vmxnet3/vmxnet3_drv.c
>
> [...]
>
> > @@ -3067,6 +3065,7 @@ vmxnet3_probe_device(struct pci_dev *pdev,
> >
> > goto err_register;
> >
> > }
> >
> > + netif_carrier_off(netdev);
> >
> > vmxnet3_check_link(adapter, false);
> > return 0;
>
> You should do this before calling register_netdev(), otherwise the link
> state change can race with vmxnet3_open().
Hmm, is it safe to do before the network device is registered? The rest
of the drivers (e1000, tg3, etc) seem to be doing netif_carrier_off()
in open() rather than probe() and then do link checks.
Thanks,
Dmitry
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Pv-drivers] [PATCH] vmxnet3: set carrier state properly on probe
2013-01-29 18:54 ` [Pv-drivers] " Dmitry Torokhov
@ 2013-01-29 19:09 ` Neil Horman
2013-01-29 19:17 ` Ben Hutchings
0 siblings, 1 reply; 12+ messages in thread
From: Neil Horman @ 2013-01-29 19:09 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: pv-drivers, Ben Hutchings, netdev, David S. Miller
On Tue, Jan 29, 2013 at 10:54:28AM -0800, Dmitry Torokhov wrote:
> On Tuesday, January 29, 2013 05:54:11 PM Ben Hutchings wrote:
> > On Fri, 2013-01-25 at 15:54 -0500, Neil Horman wrote:
> > > vmxnet3 fails to set netif_carrier_off on probe, meaning that when an
> > > interface is opened the __LINK_STATE_NOCARRIER bit is already cleared,
> > > and so /sys/class/net/<ifname>/operstate remains in the unknown state.
> > > Correct this by setting netif_carrier_off on probe, like other drivers
> > > do.
> > >
> > > Also, while we're at it, lets remove the netif_carrier_ok checks from the
> > > link_state_update function, as that check is atomically contained within
> > > the netif_carrier_[on|off] functions anyway
> > >
> > > Tested successfully by myself
> > >
> > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > > CC: "David S. Miller" <davem@davemloft.net>
> > > CC: "VMware, Inc." <pv-drivers@vmware.com>
> > > ---
> > >
> > > drivers/net/vmxnet3/vmxnet3_drv.c | 7 +++----
> > > 1 file changed, 3 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c
> > > b/drivers/net/vmxnet3/vmxnet3_drv.c index b1c90f8..66c26a9 100644
> > > --- a/drivers/net/vmxnet3/vmxnet3_drv.c
> > > +++ b/drivers/net/vmxnet3/vmxnet3_drv.c
> >
> > [...]
> >
> > > @@ -3067,6 +3065,7 @@ vmxnet3_probe_device(struct pci_dev *pdev,
> > >
> > > goto err_register;
> > >
> > > }
> > >
> > > + netif_carrier_off(netdev);
> > >
> > > vmxnet3_check_link(adapter, false);
> > > return 0;
> >
> > You should do this before calling register_netdev(), otherwise the link
> > state change can race with vmxnet3_open().
>
> Hmm, is it safe to do before the network device is registered? The rest
> of the drivers (e1000, tg3, etc) seem to be doing netif_carrier_off()
> in open() rather than probe() and then do link checks.
>
It appears to be safe to me, at least for the netif_carrier_off case, as theres
a check to effecitvely just set the NOCARRIER bit if the device is in
UNREGISTERED state.
Neil
> Thanks,
> Dmitry
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Pv-drivers] [PATCH] vmxnet3: set carrier state properly on probe
2013-01-29 19:09 ` Neil Horman
@ 2013-01-29 19:17 ` Ben Hutchings
2013-01-29 19:53 ` Dmitry Torokhov
0 siblings, 1 reply; 12+ messages in thread
From: Ben Hutchings @ 2013-01-29 19:17 UTC (permalink / raw)
To: Neil Horman; +Cc: Dmitry Torokhov, pv-drivers, netdev, David S. Miller
On Tue, 2013-01-29 at 14:09 -0500, Neil Horman wrote:
> On Tue, Jan 29, 2013 at 10:54:28AM -0800, Dmitry Torokhov wrote:
> > On Tuesday, January 29, 2013 05:54:11 PM Ben Hutchings wrote:
> > > On Fri, 2013-01-25 at 15:54 -0500, Neil Horman wrote:
> > > > vmxnet3 fails to set netif_carrier_off on probe, meaning that when an
> > > > interface is opened the __LINK_STATE_NOCARRIER bit is already cleared,
> > > > and so /sys/class/net/<ifname>/operstate remains in the unknown state.
> > > > Correct this by setting netif_carrier_off on probe, like other drivers
> > > > do.
> > > >
> > > > Also, while we're at it, lets remove the netif_carrier_ok checks from the
> > > > link_state_update function, as that check is atomically contained within
> > > > the netif_carrier_[on|off] functions anyway
> > > >
> > > > Tested successfully by myself
> > > >
> > > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > > > CC: "David S. Miller" <davem@davemloft.net>
> > > > CC: "VMware, Inc." <pv-drivers@vmware.com>
> > > > ---
> > > >
> > > > drivers/net/vmxnet3/vmxnet3_drv.c | 7 +++----
> > > > 1 file changed, 3 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c
> > > > b/drivers/net/vmxnet3/vmxnet3_drv.c index b1c90f8..66c26a9 100644
> > > > --- a/drivers/net/vmxnet3/vmxnet3_drv.c
> > > > +++ b/drivers/net/vmxnet3/vmxnet3_drv.c
> > >
> > > [...]
> > >
> > > > @@ -3067,6 +3065,7 @@ vmxnet3_probe_device(struct pci_dev *pdev,
> > > >
> > > > goto err_register;
> > > >
> > > > }
> > > >
> > > > + netif_carrier_off(netdev);
> > > >
> > > > vmxnet3_check_link(adapter, false);
> > > > return 0;
> > >
> > > You should do this before calling register_netdev(), otherwise the link
> > > state change can race with vmxnet3_open().
> >
> > Hmm, is it safe to do before the network device is registered? The rest
> > of the drivers (e1000, tg3, etc) seem to be doing netif_carrier_off()
> > in open() rather than probe() and then do link checks.
> >
> It appears to be safe to me, at least for the netif_carrier_off case, as theres
> a check to effecitvely just set the NOCARRIER bit if the device is in
> UNREGISTERED state.
Right, I made this work properly a little while ago (commit
8f4cccbbd92f). Previous best practice was to call netif_carrier_off()
after registration, but userland could still see the wrong operstate
because linkwatch updates it asynchronously.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Pv-drivers] [PATCH] vmxnet3: set carrier state properly on probe
2013-01-29 19:17 ` Ben Hutchings
@ 2013-01-29 19:53 ` Dmitry Torokhov
0 siblings, 0 replies; 12+ messages in thread
From: Dmitry Torokhov @ 2013-01-29 19:53 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Neil Horman, pv-drivers, netdev, David S. Miller
On Tuesday, January 29, 2013 07:17:10 PM Ben Hutchings wrote:
> On Tue, 2013-01-29 at 14:09 -0500, Neil Horman wrote:
> > On Tue, Jan 29, 2013 at 10:54:28AM -0800, Dmitry Torokhov wrote:
> > > On Tuesday, January 29, 2013 05:54:11 PM Ben Hutchings wrote:
> > > > On Fri, 2013-01-25 at 15:54 -0500, Neil Horman wrote:
> > > > > vmxnet3 fails to set netif_carrier_off on probe, meaning that when
> > > > > an
> > > > > interface is opened the __LINK_STATE_NOCARRIER bit is already
> > > > > cleared,
> > > > > and so /sys/class/net/<ifname>/operstate remains in the unknown
> > > > > state.
> > > > > Correct this by setting netif_carrier_off on probe, like other
> > > > > drivers
> > > > > do.
> > > > >
> > > > > Also, while we're at it, lets remove the netif_carrier_ok checks
> > > > > from the
> > > > > link_state_update function, as that check is atomically contained
> > > > > within
> > > > > the netif_carrier_[on|off] functions anyway
> > > > >
> > > > > Tested successfully by myself
> > > > >
> > > > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > > > > CC: "David S. Miller" <davem@davemloft.net>
> > > > > CC: "VMware, Inc." <pv-drivers@vmware.com>
> > > > > ---
> > > > >
> > > > > drivers/net/vmxnet3/vmxnet3_drv.c | 7 +++----
> > > > > 1 file changed, 3 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c
> > > > > b/drivers/net/vmxnet3/vmxnet3_drv.c index b1c90f8..66c26a9 100644
> > > > > --- a/drivers/net/vmxnet3/vmxnet3_drv.c
> > > > > +++ b/drivers/net/vmxnet3/vmxnet3_drv.c
> > > >
> > > > [...]
> > > >
> > > > > @@ -3067,6 +3065,7 @@ vmxnet3_probe_device(struct pci_dev *pdev,
> > > > >
> > > > > goto err_register;
> > > > >
> > > > > }
> > > > >
> > > > > + netif_carrier_off(netdev);
> > > > >
> > > > > vmxnet3_check_link(adapter, false);
> > > > > return 0;
> > > >
> > > > You should do this before calling register_netdev(), otherwise the
> > > > link
> > > > state change can race with vmxnet3_open().
> > >
> > > Hmm, is it safe to do before the network device is registered? The rest
> > > of the drivers (e1000, tg3, etc) seem to be doing netif_carrier_off()
> > > in open() rather than probe() and then do link checks.
> >
> > It appears to be safe to me, at least for the netif_carrier_off case, as
> > theres a check to effecitvely just set the NOCARRIER bit if the device is
> > in UNREGISTERED state.
>
> Right, I made this work properly a little while ago (commit
> 8f4cccbbd92f). Previous best practice was to call netif_carrier_off()
> after registration, but userland could still see the wrong operstate
> because linkwatch updates it asynchronously.
Ah, OK I see. Thank you for the explanation.
Thanks,
Dmitry
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Pv-drivers] [PATCH v2] vmxnet3: set carrier state properly on probe
2013-01-29 18:36 ` [PATCH v2] " Neil Horman
@ 2013-01-29 19:54 ` Dmitry Torokhov
2013-01-29 20:44 ` David Miller
1 sibling, 0 replies; 12+ messages in thread
From: Dmitry Torokhov @ 2013-01-29 19:54 UTC (permalink / raw)
To: pv-drivers; +Cc: Neil Horman, netdev, Ben Hutchings, David S. Miller
On Tuesday, January 29, 2013 01:36:51 PM Neil Horman wrote:
> vmxnet3 fails to set netif_carrier_off on probe, meaning that when an
> interface is opened the __LINK_STATE_NOCARRIER bit is already cleared, and
> so /sys/class/net/<ifname>/operstate remains in the unknown state. Correct
> this by setting netif_carrier_off on probe, like other drivers do.
>
> Also, while we're at it, lets remove the netif_carrier_ok checks from the
> link_state_update function, as that check is atomically contained within the
> netif_carrier_[on|off] functions anyway
>
> Tested successfully by myself
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: "David S. Miller" <davem@davemloft.net>
> CC: "VMware, Inc." <pv-drivers@vmware.com>
Acked-by: Dmitry Torokhov <dtor@vmware.com>
> CC: Ben Hutchings <bhutchings@solarflare.com>
>
> ---
> Change notes:
>
> v2) Moved netif_carrier_off above register_netdev to prevent race with
> dev_open as per Ben H.
> ---
> drivers/net/vmxnet3/vmxnet3_drv.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c
> b/drivers/net/vmxnet3/vmxnet3_drv.c index b1c90f8..ffb97b2 100644
> --- a/drivers/net/vmxnet3/vmxnet3_drv.c
> +++ b/drivers/net/vmxnet3/vmxnet3_drv.c
> @@ -150,8 +150,7 @@ vmxnet3_check_link(struct vmxnet3_adapter *adapter, bool
> affectTxQueue) if (ret & 1) { /* Link is up. */
> netdev_info(adapter->netdev, "NIC Link is Up %d Mbps\n",
> adapter->link_speed);
> - if (!netif_carrier_ok(adapter->netdev))
> - netif_carrier_on(adapter->netdev);
> + netif_carrier_on(adapter->netdev);
>
> if (affectTxQueue) {
> for (i = 0; i < adapter->num_tx_queues; i++)
> @@ -160,8 +159,7 @@ vmxnet3_check_link(struct vmxnet3_adapter *adapter, bool
> affectTxQueue) }
> } else {
> netdev_info(adapter->netdev, "NIC Link is Down\n");
> - if (netif_carrier_ok(adapter->netdev))
> - netif_carrier_off(adapter->netdev);
> + netif_carrier_off(adapter->netdev);
>
> if (affectTxQueue) {
> for (i = 0; i < adapter->num_tx_queues; i++)
> @@ -3060,6 +3058,7 @@ vmxnet3_probe_device(struct pci_dev *pdev,
> netif_set_real_num_tx_queues(adapter->netdev, adapter->num_tx_queues);
> netif_set_real_num_rx_queues(adapter->netdev, adapter->num_rx_queues);
>
> + netif_carrier_off(netdev);
> err = register_netdev(netdev);
>
> if (err) {
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] vmxnet3: set carrier state properly on probe
2013-01-29 18:36 ` [PATCH v2] " Neil Horman
2013-01-29 19:54 ` [Pv-drivers] " Dmitry Torokhov
@ 2013-01-29 20:44 ` David Miller
2013-01-29 21:08 ` Neil Horman
2013-01-29 21:15 ` [PATCH v3] " Neil Horman
1 sibling, 2 replies; 12+ messages in thread
From: David Miller @ 2013-01-29 20:44 UTC (permalink / raw)
To: nhorman; +Cc: netdev, pv-drivers, bhutchings
From: Neil Horman <nhorman@tuxdriver.com>
Date: Tue, 29 Jan 2013 13:36:51 -0500
> vmxnet3 fails to set netif_carrier_off on probe, meaning that when an interface
> is opened the __LINK_STATE_NOCARRIER bit is already cleared, and so
> /sys/class/net/<ifname>/operstate remains in the unknown state. Correct this by
> setting netif_carrier_off on probe, like other drivers do.
>
> Also, while we're at it, lets remove the netif_carrier_ok checks from the
> link_state_update function, as that check is atomically contained within the
> netif_carrier_[on|off] functions anyway
>
> Tested successfully by myself
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
This doesn't apply to the current 'net' tree. Please respin.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] vmxnet3: set carrier state properly on probe
2013-01-29 20:44 ` David Miller
@ 2013-01-29 21:08 ` Neil Horman
2013-01-29 21:15 ` [PATCH v3] " Neil Horman
1 sibling, 0 replies; 12+ messages in thread
From: Neil Horman @ 2013-01-29 21:08 UTC (permalink / raw)
To: David Miller; +Cc: netdev, pv-drivers, bhutchings
On Tue, Jan 29, 2013 at 03:44:30PM -0500, David Miller wrote:
> From: Neil Horman <nhorman@tuxdriver.com>
> Date: Tue, 29 Jan 2013 13:36:51 -0500
>
> > vmxnet3 fails to set netif_carrier_off on probe, meaning that when an interface
> > is opened the __LINK_STATE_NOCARRIER bit is already cleared, and so
> > /sys/class/net/<ifname>/operstate remains in the unknown state. Correct this by
> > setting netif_carrier_off on probe, like other drivers do.
> >
> > Also, while we're at it, lets remove the netif_carrier_ok checks from the
> > link_state_update function, as that check is atomically contained within the
> > netif_carrier_[on|off] functions anyway
> >
> > Tested successfully by myself
> >
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
>
> This doesn't apply to the current 'net' tree. Please respin.
I based it off net-next. I'll respin for net
Neil
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3] vmxnet3: set carrier state properly on probe
2013-01-29 20:44 ` David Miller
2013-01-29 21:08 ` Neil Horman
@ 2013-01-29 21:15 ` Neil Horman
2013-01-29 21:29 ` David Miller
1 sibling, 1 reply; 12+ messages in thread
From: Neil Horman @ 2013-01-29 21:15 UTC (permalink / raw)
To: netdev; +Cc: Neil Horman, David S. Miller, VMware, Inc., Ben Hutchings
vmxnet3 fails to set netif_carrier_off on probe, meaning that when an interface
is opened the __LINK_STATE_NOCARRIER bit is already cleared, and so
/sys/class/net/<ifname>/operstate remains in the unknown state. Correct this by
setting netif_carrier_off on probe, like other drivers do.
Also, while we're at it, lets remove the netif_carrier_ok checks from the
link_state_update function, as that check is atomically contained within the
netif_carrier_[on|off] functions anyway
Tested successfully by myself
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: "David S. Miller" <davem@davemloft.net>
CC: "VMware, Inc." <pv-drivers@vmware.com>
CC: Ben Hutchings <bhutchings@solarflare.com>
---
Change notes:
v2) Moved netif_carrier_off above register_netdev to prevent race with dev_open
as per Ben H.
v3) Respin for net tree
---
drivers/net/vmxnet3/vmxnet3_drv.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index dc8913c..12c6440 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -154,8 +154,7 @@ vmxnet3_check_link(struct vmxnet3_adapter *adapter, bool affectTxQueue)
if (ret & 1) { /* Link is up. */
printk(KERN_INFO "%s: NIC Link is Up %d Mbps\n",
adapter->netdev->name, adapter->link_speed);
- if (!netif_carrier_ok(adapter->netdev))
- netif_carrier_on(adapter->netdev);
+ netif_carrier_on(adapter->netdev);
if (affectTxQueue) {
for (i = 0; i < adapter->num_tx_queues; i++)
@@ -165,8 +164,7 @@ vmxnet3_check_link(struct vmxnet3_adapter *adapter, bool affectTxQueue)
} else {
printk(KERN_INFO "%s: NIC Link is Down\n",
adapter->netdev->name);
- if (netif_carrier_ok(adapter->netdev))
- netif_carrier_off(adapter->netdev);
+ netif_carrier_off(adapter->netdev);
if (affectTxQueue) {
for (i = 0; i < adapter->num_tx_queues; i++)
@@ -3061,6 +3059,7 @@ vmxnet3_probe_device(struct pci_dev *pdev,
netif_set_real_num_tx_queues(adapter->netdev, adapter->num_tx_queues);
netif_set_real_num_rx_queues(adapter->netdev, adapter->num_rx_queues);
+ netif_carrier_off(netdev);
err = register_netdev(netdev);
if (err) {
--
1.7.11.7
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v3] vmxnet3: set carrier state properly on probe
2013-01-29 21:15 ` [PATCH v3] " Neil Horman
@ 2013-01-29 21:29 ` David Miller
0 siblings, 0 replies; 12+ messages in thread
From: David Miller @ 2013-01-29 21:29 UTC (permalink / raw)
To: nhorman; +Cc: netdev, pv-drivers, bhutchings
From: Neil Horman <nhorman@tuxdriver.com>
Date: Tue, 29 Jan 2013 16:15:45 -0500
> vmxnet3 fails to set netif_carrier_off on probe, meaning that when an interface
> is opened the __LINK_STATE_NOCARRIER bit is already cleared, and so
> /sys/class/net/<ifname>/operstate remains in the unknown state. Correct this by
> setting netif_carrier_off on probe, like other drivers do.
>
> Also, while we're at it, lets remove the netif_carrier_ok checks from the
> link_state_update function, as that check is atomically contained within the
> netif_carrier_[on|off] functions anyway
>
> Tested successfully by myself
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
Applied, thanks Neil.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-01-29 21:29 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-25 20:54 [PATCH] vmxnet3: set carrier state properly on probe Neil Horman
2013-01-29 17:54 ` Ben Hutchings
2013-01-29 18:54 ` [Pv-drivers] " Dmitry Torokhov
2013-01-29 19:09 ` Neil Horman
2013-01-29 19:17 ` Ben Hutchings
2013-01-29 19:53 ` Dmitry Torokhov
2013-01-29 18:36 ` [PATCH v2] " Neil Horman
2013-01-29 19:54 ` [Pv-drivers] " Dmitry Torokhov
2013-01-29 20:44 ` David Miller
2013-01-29 21:08 ` Neil Horman
2013-01-29 21:15 ` [PATCH v3] " Neil Horman
2013-01-29 21:29 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).