Hi Kalle, > -----Original Message----- > From: Kalle Valo [mailto:kvalo@xxxxxxxxxxxxxx] > Sent: 2017年5月18日 22:33 > To: Xinming Hu > Cc: Linux Wireless; Brian Norris; Dmitry Torokhov; rajatja@xxxxxxxxxx; > Zhiyuan Yang; Cathy Luo; Xinming Hu; Ganapathi Bhat > Subject: Re: [PATCH v2 5/6] mwifiex: do not aggregate tcp ack in usb tx > aggregation queue > > Xinming Hu <huxinming820@xxxxxxxxx> writes: > > > From: Xinming Hu <huxm@xxxxxxxxxxx> > > > > Tcp ack should be send as soon to avoid throuput drop during receive > > tcp traffic. > > > > Signed-off-by: Xinming Hu <huxm@xxxxxxxxxxx> > > Signed-off-by: Cathy Luo <cluo@xxxxxxxxxxx> > > Signed-off-by: Ganapathi Bhat <gbhat@xxxxxxxxxxx> > > [...] > > > +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. > My wrong. Will resend V3 serials. Thanks for the review. Regards, Simon > -- > Kalle Valo
