Re: [PATCH v2 5/6] mwifiex: do not aggregate tcp ack in usb tx aggregation queue

[Kalle Valo <kvalo@codeaurora.org>]

Xinming Hu <huxinming820@gmail.com> writes:

> From: Xinming Hu <huxm@marvell.com>
>
> Tcp ack should be send as soon to avoid throuput drop during receive tcp
> traffic.
>
> Signed-off-by: Xinming Hu <huxm@marvell.com>
> Signed-off-by: Cathy Luo <cluo@marvell.com>
> Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>

[...]

> +static bool is_piggyback_tcp_ack(struct sk_buff *skb)
> +{
> +	struct ethhdr *ethh = NULL;
> +	struct iphdr  *iph = NULL;
> +	struct tcphdr *tcph = NULL;
> +
> +	ethh = (struct ethhdr *)skb->data;
> +	if (ntohs(ethh->h_proto) != ETH_P_IP)
> +		return false;
> +
> +	iph = (struct iphdr *)((u8 *)ethh + sizeof(struct ethhdr));
> +	if (iph->protocol != IPPROTO_TCP)
> +		return false;
> +
> +	tcph = (struct tcphdr *)((u8 *)iph + iph->ihl * 4);
> +	/* Piggyback ack without payload*/
> +	if (*((u8 *)tcph + 13) == 0x10 &&
> +	    ntohs(iph->tot_len) <= (iph->ihl + tcph->doff) * 4) {
> +		return true;
> +	}

It's rather ugly to use magic values (13 and 0x10) like that. Can't you
use some of the existing defines? At least I see TCP_FLAG_ACK and struct
tcphdr::ack being available, so you should even have choises what to
use.

-- 
Kalle Valo