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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4967D5DE.2050002@openwrt.org>
Date:	Fri, 09 Jan 2009 23:55:26 +0100
From:	Felix Fietkau <nbd@...nwrt.org>
To:	Michael Buesch <mb@...sch.de>
CC:	netdev@...r.kernel.org, jgarzik@...ox.com, zambrano@...adcom.com
Subject: Re: [PATCH v2] b44: fix misalignment and wasted space in rx handling

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


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html