On Sunday 26 November 2006 8:26 pm, Nikita V. Youshchenko wrote: > > > On Sunday 26 November 2006 5:28 am, Nikita V. Youshchenko wrote: > > > When AX88xxx hardware packs several incoming frames, it puts second > > > and subsequent frames with 2-byte alignment. This may cause situations > > > when IP layer gets IP packet not aligned at 32-bit word boundary, > > > which in turn may lead to kernel crashes on some hardware, and serious > > > slowdown on other hardware. > > > > The number "2" is magic. You should rework this patch to use > > NET_IP_ALIGN instead ... > > Hmm... > Actually, frame should be aligned at 4n+2, so here are two magic numbers (4 > and 2). Should 4 be replaced with something as well?
AFAIK the IP layer is fine with the 4-byte alignment provided if you use NET_IP_ALIGN. > > and remove the #ifdefs. > > I believe that code should really be different between different > architectures - because: > - on architectures that can't do hardware unaligned access, data move is > required, > - on architectures that can do hardware unaligned access, data move is > unnecessary slowdown. NET_IP_ALIGN is an arch-overridable value, suitable for achieving the top priority goal: an always-correct driver. Remember the basic rule that applies to such micro-optimizations: don't even think about it before you have measured macro-level wins. Maintainability is more important, especially in what you've already pointed out is an uncommon case. > And this was discussed in the mailing list several days ago. Yet the right answer wasn't given. > Why removing #ifdef's in such a situation? Arch-specific #ifdeffery is virtually NEVER ok. In this (uncommon) case you can obviously express _correct_ logic with no #ifdeffery. But if you start collecting arch-specific #ifdefs, you're going to begin an unmaintainable pile of them ... remember that for example on ARM, _some_ CPUs can do unaligned access, while others can't; so it's not a "per arch" thing, but a per-implementation-of-arch thing. - Dave > > Also, please cc your two ASIX patches to the maintainer of > > that module, he may have other comments ... > > ok > > Nikita > ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
