Re: [PATCH v2] b44: fix misalignment and wasted space in rx handling

[Felix Fietkau <nbd@openwrt.org>]

Michael Buesch wrote:
> On Friday 09 January 2009 13:39:57 Felix Fietkau wrote:
>> --- a/drivers/net/b44.c
>> +++ b/drivers/net/b44.c
>> @@ -73,8 +73,8 @@
>>  	  (BP)->tx_cons - (BP)->tx_prod - TX_RING_GAP(BP))
>>  #define NEXT_TX(N)		(((N) + 1) & (B44_TX_RING_SIZE - 1))
>>  
>> -#define RX_PKT_OFFSET		30
>> -#define RX_PKT_BUF_SZ		(1536 + RX_PKT_OFFSET + 64)
>> +#define RX_PKT_OFFSET		(RX_HEADER_LEN + 2)
>> +#define RX_PKT_BUF_SZ		(1536 + RX_PKT_OFFSET)
>>  
>>  /* minimum number of free TX descriptors required to wake up TX process */
>>  #define B44_TX_WAKEUP_THRESH		(B44_TX_RING_SIZE / 4)
>> @@ -682,7 +682,6 @@ static int b44_alloc_rx_skb(struct b44 *
>>  	}
>>  
>>  	rh = (struct rx_header *) skb->data;
>> -	skb_reserve(skb, RX_PKT_OFFSET);
> 
> Looks correct.
> 
>>  	rh->len = 0;
>>  	rh->flags = 0;
>> @@ -693,13 +692,13 @@ static int b44_alloc_rx_skb(struct b44 *
>>  	if (src_map != NULL)
>>  		src_map->skb = NULL;
>>  
>> -	ctrl  = (DESC_CTRL_LEN & (RX_PKT_BUF_SZ - RX_PKT_OFFSET));
>> +	ctrl = (DESC_CTRL_LEN & RX_PKT_BUF_SZ);
> 
> Are you sure this is right? b43 has the same DMA engine, and we
> subtract the offset from the buffer size there. So we end up with
> the descriptor control field telling the space available for the frame payload.
Quote from a Datasheet for the BCM5365 that I found online:

"This field is the length, in bytes, of the data buffer associated with this
descriptor. A descriptor with a BufCount value greater than 0x1000 causes a
descriptor protocol error."

I checked the public hnddma.c code and it also appears to not subtract
the packet offset either, so I think my version is correct.

- Felix