netdev - Re: [PATCH] fix e100 rx path on ARM (was [PATCH] e100 rx: or s and el bits)

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: <d79bd7b3b3b89b941e70abf73e684a7b@bga.com>
Date:	Thu, 24 May 2007 09:25:33 -0500
From:	Milton Miller <miltonm@....com>
To:	David Acker <dacker@...net.com>
Cc:	Jeff Garzik <jgarzik@...ox.com>,
	"Kok, Auke" <auke-jan.h.kok@...el.com>,
	e1000-devel@...ts.sourceforge.net,
	Jeff Kirsher <jeffrey.t.kirsher@...el.com>,
	John Ronciak <john.ronciak@...el.com>,
	Jesse Brandeburg <jesse.brandeburg@...el.com>,
	Scott Feldman <sfeldma@...ox.com>, netdev@...r.kernel.org
Subject: Re: [PATCH] fix e100 rx path on ARM (was [PATCH] e100 rx: or s and el bits)

On May 24, 2007, at 7:51 AM, David Acker wrote:
> Milton Miller wrote:
>> Comments?  Questions?
> This sounds pretty reasonable.  I will take a stab at coding this up 
> today; I always think better looking at code.

Thanks.

By the way, find_mark_el should probably get passed
the old fill point.   The EL it will be clearing will
either be on that rx or the previous one (or there is
none for the initial alloc_list).

>> Do we need to continue with the stop-before-last plan?  As
>> long as we set size to 0 with EL, we shoud be able to change
>> the link, sync, set size 0, sync, and then set size.
> Perhaps not.  I can take a try at coding it without it.  It would 
> certainly make the driver easier to follow.
>
>> We still need the "advance at least 2 if not stopped" check when
>> deciding to move the EL.  This would break if the hardware
>> in the dma path can read the multiple cache lines of the
>> RFD out of order, so it may not be safe (if some pci host
>> decided to prefetch data, and got the second line ahead of
>> the first and didn't discard prefetch until pci bus
>> disconnect).  Actually, I am afraid I know hardware that
>> might do that.
> Hmm, me too.
>

I think we should be conservative here, and keep the stop
before the last and modify the last that we currently have.
I know the above prefetch hardware exists on a system that
can plug e100 cards.  While prefetching multiple lines is
based on the pci fetch code (read vs read line vs read
multiple), its just too close to failing.

The reason this behavior won't break us with the two packet
version is we always have written the unaligned (and hence
not atomic) write of the link field before we even expose
the RFD to the hardware.   Since data is aligned to two
bytes, the write to the EL bit and the write to length will
be atomic.  In addition, if the hardware reorders the loads
and gets length with EL, it will just RNR after filling
in the packet and we will handle it just fine.

Perhaps we should comment that we rely on 2 byte alignment,
in case some crazy architecture sets ETH_HDR_ALIGN to an
odd value.  (It's arch dependent to allow trade offs of
partial cache line writes from IO vs processing unaligned
TCP and IP headers by the cpu).  I wouldn't do BUG_ON
but wouldn't object to BUILD_BUG_ON if someone thinks
it's needed.

The difference is a additional pointer follow in the new
find_mark_el() from the new fill point to the mark point
(and the availability of one more RFD for a packet).
Either way we need to check its ->prev for being the
being current EL when hardware is not stopped.

milton

-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html