On Mon, 22 Oct 2007 18:15:30 -0700 (PDT) David Miller <davem@xxxxxxxxxxxxx> wrote: > From: Pierre Ossman <drzeus@xxxxxxxxx> > Date: Mon, 22 Oct 2007 19:05:32 +0200 > > > + /* > > + * The IP stack is littered with silly assumptions on alignment, > > + * so we need to do a bit of layering violation here and make > > + * assumptions about the size of the headers between us and the > > + * IP stack. > > + */ > > + skb_reserve(skb, NET_IP_ALIGN); > > + > > Please remove this comment. > > Where else could we possibly ensure that post-link-level headers are > aligned properly, taking into account DMA engine restrictions and > whatnot, other than in the driver itself? > If the assumption was that this driver was just below post-link-level, then fine. But this driver has a header of 4 bytes. The problem is one level up, and even has variable headers at that point. > It's not about IP, even though the macro has "IP" in it's name. It's > meant to ensure reasonable alignment after the link-level header, > regardless of upper-level protocol. So your layering violation > commentary is simply rediculious and frankly starting to annoy the > crap out of me. > > As you were also shown, all of THIS is fully documented in a huge > comment above the definition of NET_IP_ALIGN. > And digging through every header file is the norm when trying to figure out the broad details of writing a network driver? I looked at both of if_sdio's sister drivers, if_usb and if_cs; only one had the reserve call and it didn't have a single comment (and not even the right macro). if_cs now has a call, but with the bare minimum of a comment. I also tried looking through Documentation/, but all I could find were driver specific docs. > I'm sorry for flaming you so much, but you are showing zero respect > for existing practice in the kernel networking and you behave as if > you're the only person in the world who has to deal with these issues. > You accuse us openly of mis-architecting the Linux networking with > these so-called "layering violations" and having maintained the stack > for more than 10 years I start to take things like that quite > personally. It _is_ a layering violation, with the upside of a performance gain. You're free to make that trade-off, but that doesn't mean I have to like it. If you feel offended by that, then I'm sorry. It was never my intention to piss anyone off. Feel free to remove the comment. I don't see this discussion going anywhere productive. /Pierre
Attachment:
signature.asc
Description: PGP signature
