linux-kernel - Re: [PATCH 1/2] net: ethernet: sxgbe: remove private tx queue lock

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]

Message-ID: <20161219100215.GA6296@amd>
Date:   Mon, 19 Dec 2016 11:02:15 +0100
From:   Pavel Machek <pavel@....cz>
To:     Francois Romieu <romieu@...zoreil.com>
Cc:     Lino Sanfilippo <LinoSanfilippo@....de>, bh74.an@...sung.com,
        ks.giri@...sung.com, vipul.pandya@...sung.com,
        peppe.cavallaro@...com, alexandre.torgue@...com,
        davem@...emloft.net, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org
Subject: Re: [PATCH 1/2] net: ethernet: sxgbe: remove private tx queue lock

Hi!

> Lino, have you considered via-rhine.c since its "move work from irq to
> workqueue context" changes that started in
> 7ab87ff4c770eed71e3777936299292739fcd0fe [*] ?
> 
> It's a shameless plug - originated in r8169.c - but it should be rather
> close to what the sxgbe and friends require. Thought / opinion ?
> 
> [*] Including fixes/changes in:
> - 3a5a883a8a663b930908cae4abe5ec913b9b2fd2
> - e1efa87241272104d6a12c8b9fcdc4f62634d447
> - 810f19bcb862f8889b27e0c9d9eceac9593925dd
> - e45af497950a89459a0c4b13ffd91e1729fffef4
> - a926592f5e4e900f3fa903298c4619a131e60963
> - 559bcac35facfed49ab4f408e162971612dcfdf3

Considering the memory barriers... is something like this neccessary
in the via-rhine?

AFAICT... we need a barrier after making sure that descriptor is no
longer owned by DMA (to make sure we don't get stale data in rest of
descriptor)... and we need a barrier before giving the descriptor to
the dma, to make sure DMA engine sees the complete update....?

Thanks,
								Pavel

diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c
index ba5c542..3806e72 100644
--- a/drivers/net/ethernet/via/via-rhine.c
+++ b/drivers/net/ethernet/via/via-rhine.c
@@ -1952,6 +1952,7 @@ static void rhine_tx(struct net_device *dev)
 			  entry, txstatus);
 		if (txstatus & DescOwn)
 			break;
+		dma_rmb();
 		skb = rp->tx_skbuff[entry];
 		if (txstatus & 0x8000) {
 			netif_dbg(rp, tx_done, dev,
@@ -2061,6 +2062,7 @@ static int rhine_rx(struct net_device *dev, int limit)
 
 		if (desc_status & DescOwn)
 			break;
+		dma_rmb();
 
 		netif_dbg(rp, rx_status, dev, "%s() status %08x\n", __func__,
 			  desc_status);
@@ -2146,6 +2148,7 @@ static int rhine_rx(struct net_device *dev, int limit)
 			u64_stats_update_end(&rp->rx_stats.syncp);
 		}
 give_descriptor_to_nic:
+		dma_wmb();
 		desc->rx_status = cpu_to_le32(DescOwn);
 		entry = (++rp->cur_rx) % RX_RING_SIZE;
 	}

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Download attachment "signature.asc" of type "application/pgp-signature" (182 bytes)