| Message ID | 20200616011459.30966-1-snelson@pensando.io |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net] ionic: no link check while resetting queues | expand |
On 6/15/20 9:14 PM, Shannon Nelson wrote: > If the driver is busy resetting queues after a change in > MTU or queue parameters, don't bother checking the link, > wait until the next watchdog cycle. > > Fixes: 987c0871e8ae ("ionic: check for linkup in watchdog") > Signed-off-by: Shannon Nelson <snelson@pensando.io> > --- > drivers/net/ethernet/pensando/ionic/ionic_lif.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c > index 9d8c969f21cb..bfadc4934702 100644 > --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c > +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c > @@ -96,7 +96,8 @@ static void ionic_link_status_check(struct ionic_lif *lif) > u16 link_status; > bool link_up; > > - if (!test_bit(IONIC_LIF_F_LINK_CHECK_REQUESTED, lif->state)) > + if (!test_bit(IONIC_LIF_F_LINK_CHECK_REQUESTED, lif->state) || > + test_bit(IONIC_LIF_F_QUEUE_RESET, lif->state)) > return; > > link_status = le16_to_cpu(lif->info->status.link_status); > Would a firmware reset bit being asserted also cause an issue here (IONIC_LIF_F_FW_RESET)? Meaning do we need to test for this bit as well?
On 6/17/20 12:41 PM, Jonathan Toppins wrote: > On 6/15/20 9:14 PM, Shannon Nelson wrote: >> If the driver is busy resetting queues after a change in >> MTU or queue parameters, don't bother checking the link, >> wait until the next watchdog cycle. >> >> Fixes: 987c0871e8ae ("ionic: check for linkup in watchdog") >> Signed-off-by: Shannon Nelson <snelson@pensando.io> >> --- >> drivers/net/ethernet/pensando/ionic/ionic_lif.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c >> index 9d8c969f21cb..bfadc4934702 100644 >> --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c >> +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c >> @@ -96,7 +96,8 @@ static void ionic_link_status_check(struct ionic_lif *lif) >> u16 link_status; >> bool link_up; >> >> - if (!test_bit(IONIC_LIF_F_LINK_CHECK_REQUESTED, lif->state)) >> + if (!test_bit(IONIC_LIF_F_LINK_CHECK_REQUESTED, lif->state) || >> + test_bit(IONIC_LIF_F_QUEUE_RESET, lif->state)) >> return; >> >> link_status = le16_to_cpu(lif->info->status.link_status); >> > Would a firmware reset bit being asserted also cause an issue here > (IONIC_LIF_F_FW_RESET)? Meaning do we need to test for this bit as well? > No, we actually want the link_status_check during the FW_RESET so that we can detect when the FW has come back up and Linked. During that time we just don't want user processes poking at us, which is why the netif_device_detach()/netif_device_attach() are used there. sln
On 6/17/20 3:53 PM, Shannon Nelson wrote: > On 6/17/20 12:41 PM, Jonathan Toppins wrote: >> On 6/15/20 9:14 PM, Shannon Nelson wrote: >>> If the driver is busy resetting queues after a change in >>> MTU or queue parameters, don't bother checking the link, >>> wait until the next watchdog cycle. >>> >>> Fixes: 987c0871e8ae ("ionic: check for linkup in watchdog") >>> Signed-off-by: Shannon Nelson <snelson@pensando.io> >>> --- >>> drivers/net/ethernet/pensando/ionic/ionic_lif.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c >>> b/drivers/net/ethernet/pensando/ionic/ionic_lif.c >>> index 9d8c969f21cb..bfadc4934702 100644 >>> --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c >>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c >>> @@ -96,7 +96,8 @@ static void ionic_link_status_check(struct >>> ionic_lif *lif) >>> u16 link_status; >>> bool link_up; >>> - if (!test_bit(IONIC_LIF_F_LINK_CHECK_REQUESTED, lif->state)) >>> + if (!test_bit(IONIC_LIF_F_LINK_CHECK_REQUESTED, lif->state) || >>> + test_bit(IONIC_LIF_F_QUEUE_RESET, lif->state)) >>> return; >>> link_status = le16_to_cpu(lif->info->status.link_status); >>> >> Would a firmware reset bit being asserted also cause an issue here >> (IONIC_LIF_F_FW_RESET)? Meaning do we need to test for this bit as well? >> > > No, we actually want the link_status_check during the FW_RESET so that > we can detect when the FW has come back up and Linked. During that time > we just don't want user processes poking at us, which is why the > netif_device_detach()/netif_device_attach() are used there. > > sln > Ah ok, I missed that. Thanks. Acked-by: Jonathan Toppins <jtoppins@redhat.com>
From: Shannon Nelson <snelson@pensando.io> Date: Mon, 15 Jun 2020 18:14:59 -0700 > If the driver is busy resetting queues after a change in > MTU or queue parameters, don't bother checking the link, > wait until the next watchdog cycle. > > Fixes: 987c0871e8ae ("ionic: check for linkup in watchdog") > Signed-off-by: Shannon Nelson <snelson@pensando.io> Applied and queued up for v5.7 -stable, thanks.
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c index 9d8c969f21cb..bfadc4934702 100644 --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c @@ -96,7 +96,8 @@ static void ionic_link_status_check(struct ionic_lif *lif) u16 link_status; bool link_up; - if (!test_bit(IONIC_LIF_F_LINK_CHECK_REQUESTED, lif->state)) + if (!test_bit(IONIC_LIF_F_LINK_CHECK_REQUESTED, lif->state) || + test_bit(IONIC_LIF_F_QUEUE_RESET, lif->state)) return; link_status = le16_to_cpu(lif->info->status.link_status);
If the driver is busy resetting queues after a change in MTU or queue parameters, don't bother checking the link, wait until the next watchdog cycle. Fixes: 987c0871e8ae ("ionic: check for linkup in watchdog") Signed-off-by: Shannon Nelson <snelson@pensando.io> --- drivers/net/ethernet/pensando/ionic/ionic_lif.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)