Re: [patch 3/5] A-MSDU Rx aggregation support

[Johannes Berg <johannes@sipsolutions.net>]

[-- Attachment #1: Type: text/plain, Size: 1352 bytes --]

On Mon, 2007-03-26 at 04:40 -0700, mohamed wrote:

> +	ethertype = (payload[6] << 8) | payload[7];
> +
> +	if (likely((memcmp(payload, rfc1042_header, 6) == 0 &&
> +		ethertype != ETH_P_AARP && ethertype != ETH_P_IPX) ||
> +		memcmp(payload, bridge_tunnel_header, 6) == 0)) {
> +		/* remove RFC1042 or Bridge-Tunnel encapsulation and
> +		* replace EtherType */	
> +		skb_pull(skb, hdrlen + 6);
> +	} else {
> +		skb_pull(skb, hdrlen);
> +	}
> +
> +	eth = (struct ethhdr*)skb->data;

If you put the eth = stuff before ethertype = you can make this code
something like ethertype = be16_to_cpu(eth->h_proto) instead of the
hardcoded endianness conversion. And the memcmp() could be
compare_ether_addr(eth->h_dest, rfc1042_header) instead.

> +	while ((u8*)eth < skb->data + skb->len) {

I don't understand this cast of eth to u8*. That seems wrong given that
there's an ethhdr there where the first byte isn't a length byte.

> +        	unsigned int eth_len = sizeof(struct ethhdr) + 
> +						ntohs(eth->h_proto);
> +        
> +		frame = dev_alloc_skb(3000);

Maybe there's a way to get an skb that actually points into the payload?
Not sure that is desirable though we really should optimise the receive
path.. it does far too many copies already.


I'll look some things later again, have to go now.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]