[RFC] bnx2: use netif_carrier_off to prevent tx timeout

Touching ->trans_start make netdev watchdog timeouts only less probable.
Use netif_carrier_off to prevent timeout, lately we take care of tuning
carrier on.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
Patch was not tested!

 drivers/net/bnx2.c |   12 +++---------
 1 files changed, 3 insertions(+), 9 deletions(-)

Stanislaw Gruszka wrote:

> Touching ->trans_start make netdev watchdog timeouts only
> less probable.
> Use netif_carrier_off to prevent timeout, lately we take care
> of tuning
> carrier on.
>
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
> Patch was not tested!
>
>  drivers/net/bnx2.c |   12 +++---------
>  1 files changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
> index 667f419..44fc392 100644
> --- a/drivers/net/bnx2.c
> +++ b/drivers/net/bnx2.c
> @@ -656,17 +656,9 @@ bnx2_netif_stop(struct bnx2 *bp, bool stop_cnic)
>       if (stop_cnic)
>               bnx2_cnic_stop(bp);
>       if (netif_running(bp->dev)) {
> -             int i;
> -
>               bnx2_napi_disable(bp);
>               netif_tx_disable(bp->dev);
> -             /* prevent tx timeout */
> -             for (i = 0; i <  bp->dev->num_tx_queues; i++) {
> -                     struct netdev_queue *txq;
> -
> -                     txq = netdev_get_tx_queue(bp->dev, i);
> -                     txq->trans_start = jiffies;
> -             }
> +             netif_carrier_off(bp->dev);
>       }
>       bnx2_disable_int_sync(bp);
>  }
> @@ -6346,6 +6338,8 @@ bnx2_vlan_rx_register(struct net_device
> *dev, struct vlan_group *vlgrp)
>       if (bp->flags & BNX2_FLAG_CAN_KEEP_VLAN)
>               bnx2_fw_sync(bp,
> BNX2_DRV_MSG_CODE_KEEP_VLAN_UPDATE, 0, 1);
>
> +     if (bp->link_up)
> +             netif_carrier_on(bp->dev);

Thanks Stanislaw, I think it is better to turn carrier on in
bnx2_netif_start().  We can use the start_cnic parameter to
decide if we need to call carrier_on().  I'll test this out.
Thanks.

>       bnx2_netif_start(bp, false);
>  }
>  #endif
> --
> 1.5.5.6
>
>
>
>

--
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

On Wed, 12 May 2010 06:31:52 -0700
"Michael Chan" <mchan@broadcom.com> wrote:

> > @@ -6346,6 +6338,8 @@ bnx2_vlan_rx_register(struct net_device
> > *dev, struct vlan_group *vlgrp)
> >       if (bp->flags & BNX2_FLAG_CAN_KEEP_VLAN)
> >               bnx2_fw_sync(bp,
> > BNX2_DRV_MSG_CODE_KEEP_VLAN_UPDATE, 0, 1);
> >
> > +     if (bp->link_up)
> > +             netif_carrier_on(bp->dev);
> 
> Thanks Stanislaw, I think it is better to turn carrier on in
> bnx2_netif_start().  We can use the start_cnic parameter to
> decide if we need to call carrier_on().

IIRC in most cases we set carrier status in bnx2_init_nic()
(based on what we get from PHY) called before bnx2_netif_start().
One exception is bnx2_vlan_rx_register() function.

Thanks
Stanislaw
--
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

Stanislaw Gruszka wrote:

> On Wed, 12 May 2010 06:31:52 -0700
> "Michael Chan" <mchan@broadcom.com> wrote:
>
> > > @@ -6346,6 +6338,8 @@ bnx2_vlan_rx_register(struct net_device
> > > *dev, struct vlan_group *vlgrp)
> > >       if (bp->flags & BNX2_FLAG_CAN_KEEP_VLAN)
> > >               bnx2_fw_sync(bp,
> > > BNX2_DRV_MSG_CODE_KEEP_VLAN_UPDATE, 0, 1);
> > >
> > > +     if (bp->link_up)
> > > +             netif_carrier_on(bp->dev);
> >
> > Thanks Stanislaw, I think it is better to turn carrier on in
> > bnx2_netif_start().  We can use the start_cnic parameter to
> > decide if we need to call carrier_on().
>
> IIRC in most cases we set carrier status in bnx2_init_nic()
> (based on what we get from PHY) called before bnx2_netif_start().
> One exception is bnx2_vlan_rx_register() function.
>

Yes, if we reset the chip and reset the PHY, we'll always get a
link up event and we'll set carrier.  We need to take care of
the cases where the chip is not reset or the phy is not reset.
I think it is better if the caller of bnx2_netif_start() does
not have to worry about setting the carrier.

--
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