From: Jakub Kicinski
> Sent: 05 November 2020 22:47
> 
> On Wed,  4 Nov 2020 16:48:57 +0100 Andrew Lunn wrote:
> > -   buf = (char*)((u32)skb->data & ~0x3);
> > -   len = (skb->len + 3 + ((u32)skb->data & 3)) & ~0x3;
> > -   cmdA = (((u32)skb->data & 0x3) << 16) |
> > +   offset = (unsigned long)skb->data & 3;
> > +   buf = skb->data - offset;
> > +   len = skb->len + offset;
> > +   cmdA = (offset << 16) |
> 
> The len calculation is wrong, you trusted people on the mailing list
> too much ;)

I misread what the comment-free convoluted code was doing....

Clearly it is rounding the base address down to a multiple of 4
and passing the offset in cmdA.
This is fine - but quite why the (I assume) hardware doesn't do
this itself and just document that it does a 32bit read is
another matter - the logic will be much the same and I doubt
anything modern is that pushed for space.

However rounding the length up to a multiple of 4 is buggy.
If this is an ethernet chipset it must (honest) be able to
send frames that don't end on a 4 byte boundary.
So rounding up the length is very dubious.

        David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

Reply via email to