linux-kernel - Re: via-rhine: Problem with lost link after a while

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Fri, 13 Apr 2012 08:16:51 +0200
From:	Bjarke Istrup Pedersen <gurligebis@...too.org>
To:	Francois Romieu <romieu@...zoreil.com>
Cc:	linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
	Svenning Sørensen <sss@...omea.com>,
	Andreas Mohr <andi@...as.de>
Subject: Re: via-rhine: Problem with lost link after a while

11. apr. 2012 00.55 skrev Francois Romieu <romieu@...zoreil.com>:
> Bjarke Istrup Pedersen <gurligebis@...too.org> :
>> 10. apr. 2012 22.42 skrev Francois Romieu <romieu@...zoreil.com>:
> [...]
>> > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;h=3f8c91a7398b9266fbe7abcbe4bd5dffef907643
> [...]
>> Great, I'll try a 3.4-rc2 kernel, and see how it runs.
>>
>> The thread I was talking about earlier is here:
>> http://lists.soekris.com/pipermail/soekris-tech/2012-April/018318.html
>> Is there any of the changes he has there, that makes sense in the new
>> driver you wrote ?
>
> (I did not write a new driver)
>
> Regarding Svenning's patch:
> - the wmb in alloc_rbufs may help rhine_reset_task().
> - one should probably add one in rhine_rx() as well.
> - rhine_start_tx() is supposed to stop queueing when there is no room left.
>  I'm curious to know if the "Tx descriptor busy" test triggered.
> - the rmb() in rhine_tx() will not make a difference for a single core but
>  it's a good reminder that I should not have forgotten to propagate the
>  xmit / Tx completion fix back from the r8169 driver to the via-rhine one
>  (sigh)
>
> mmiowb is probably missing. I doubt it hits hard right now.
>
> I have not checked if MMIO flushes are missing. Actually I need some sleep.

I've been testing v3.4-rc2 with this patch applied for the past few
days, with alot of trafic across, both incoming and outgoing, with VPN
which is normally what can trigger problems, and it seems rock solid
:o)

Do you need me to do any specific testing of this patch?

/Bjarke

> diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c
> index fcfa01f..dfa9fc0 100644
> --- a/drivers/net/ethernet/via/via-rhine.c
> +++ b/drivers/net/ethernet/via/via-rhine.c
> @@ -1163,6 +1163,7 @@ static void alloc_rbufs(struct net_device *dev)
>                                       PCI_DMA_FROMDEVICE);
>
>                rp->rx_ring[i].addr = cpu_to_le32(rp->rx_skbuff_dma[i]);
> +               wmb();
>                rp->rx_ring[i].rx_status = cpu_to_le32(DescOwn);
>        }
>        rp->dirty_rx = (unsigned int)(i - RX_RING_SIZE);
> @@ -1709,8 +1710,13 @@ static netdev_tx_t rhine_start_tx(struct sk_buff *skb,
>               ioaddr + ChipCmd1);
>        IOSYNC;
>
> -       if (rp->cur_tx == rp->dirty_tx + TX_QUEUE_LEN)
> +       if (rp->cur_tx == rp->dirty_tx + TX_QUEUE_LEN) {
> +               smp_wmb();
>                netif_stop_queue(dev);
> +               smp_mb();
> +               if (rp->cur_tx != rp->dirty_tx + TX_QUEUE_LEN)
> +                       netif_wake_queue(dev);
> +       }
>
>        netif_dbg(rp, tx_queued, dev, "Transmit frame #%d queued in slot %d\n",
>                  rp->cur_tx - 1, entry);
> @@ -1759,6 +1765,7 @@ static void rhine_tx(struct net_device *dev)
>        struct rhine_private *rp = netdev_priv(dev);
>        int txstatus = 0, entry = rp->dirty_tx % TX_RING_SIZE;
>
> +       smp_rmb();
>        /* find and cleanup dirty tx descriptors */
>        while (rp->dirty_tx != rp->cur_tx) {
>                txstatus = le32_to_cpu(rp->tx_ring[entry].tx_status);
> @@ -1806,8 +1813,12 @@ static void rhine_tx(struct net_device *dev)
>                rp->tx_skbuff[entry] = NULL;
>                entry = (++rp->dirty_tx) % TX_RING_SIZE;
>        }
> -       if ((rp->cur_tx - rp->dirty_tx) < TX_QUEUE_LEN - 4)
> +
> +       smp_mb();
> +       if (netif_queue_stopped(dev) &&
> +           (rp->cur_tx - rp->dirty_tx) < TX_QUEUE_LEN - 4) {
>                netif_wake_queue(dev);
> +       }
>  }
>
>  /**
> @@ -1947,6 +1958,7 @@ static int rhine_rx(struct net_device *dev, int limit)
>                                               PCI_DMA_FROMDEVICE);
>                        rp->rx_ring[entry].addr = cpu_to_le32(rp->rx_skbuff_dma[entry]);
>                }
> +               wmb();
>                rp->rx_ring[entry].rx_status = cpu_to_le32(DescOwn);
>        }
>
> --
> Ueimor
>
> Will code drivers for food.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/