On Monday 11 December 2006 9:21 pm, Nikita V. Youshchenko wrote:
>
> > On Monday 11 December 2006 2:48 am, Nikita V. Youshchenko wrote:
> >>
> >> > On Sunday 10 December 2006 1:58 pm, Nikita V. Youshchenko wrote:
> >> > > +static inline void ensure_alignment(struct sk_buff *skb)
> >> > > +{
> >> > > + char *pkt = skb->data;
> >> > > + int offset = NET_IP_ALIGN ? ((unsigned long)pkt -
> >> > > NET_IP_ALIGN) & 3 : 0;
> >> >
> >> > I suppose it's reasonable to expect that at most the low 2 bits must
> >> > be zero (i.e. align to max of u32), but it just _looks_ wrong ...
> >>
> >> Can't understand what do you mean by "at most the low 2 bits must be
> >> zero", sorry. Could you please explain?
> >
> > You're masking with magic number 0x03, which assumes that only the low 2
> > bits a must be zero. Oh, and if it's _already_ aligned you're still moving
> > it...
>
> I still can't get yout point, sorry.
> Maybe we are having some terminology mismatch.
>
> I'm *not* assuming that "only the low 2 bits a must be zero".
> I'm assuming that the low two bits must be same as they are in NET_IP_ALIGN
> constant.
No you're not; see the expression above. Doing what you just described
would be ((pkt & ~3) | NET_IP_ALIGN), plus/minus 4 ...
Regardless, your code is not clear; and that goes beyond the fact that
bit twiddling is often tricky.
> And I do move if and only if these bits on pkt and in NET_IP_ALIGN
> constant are different.
So you want to test for (NET_IP_ALIGN && ((pkt & 3) == NET_IP_ALIGN))...
> And this is exactly what this patch is intended to do: to make IP header
> u32-aligned, frame shoud start at address that has same 2 low
> bits as NET_IP_ALIGN has.
>
> See examples in my previous mail.
>
>
> >> And there is at least 4 bytes available before pkt (that was used for
> >> frame size word in urb space).
> >
> > Then it's worth a comment highlighting that fact. In the context of
> > that function, it's not clear.
>
> Sorry, but there *is* such a comment in the patch :).
> Please re-read the patch :)
In _that_ function, where the confusion could arise, I saw no such comment.
Can you resend the latest version of your patch?
- Dave
-------------------------------------------------------------------------
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
