On Mon, Apr 30, 2012 at 3:49 AM, David Woodhouse <dw...@infradead.org> wrote:
> On Sun, 2012-04-22 at 14:48 -0700, Dave Täht wrote:
>>

Thank you very much for the code review!

 +
>> +-#define tcp_flag_word(tp) ( ((union tcp_word_hdr *)(tp))->words [3])
>> ++#define tcp_flag_word2(tp) ( ((union tcp_word_hdr *)(tp))->words [3])
>> ++#define tcp_flag_word(tp) ( __get_unaligned_cpu32(&(((union tcp_word_hdr 
>> *)(tp))->words [3])))
>
> Ewww. And didn't you already make that union __packed anyway?

yes, ewww. I'll note that I don't like the define in the general case.
It is used both for assignment (once!) and for access , which is why
it got broken into two macros...

>
>> +diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
>> +index 22ef5f9..bf3f773 100644
>> +--- a/net/ipv4/tcp.c
>> ++++ b/net/ipv4/tcp.c
>> +@@ -2834,7 +2834,7 @@ found:
>> +
>> +       p = *head;
>> +       th2 = tcp_hdr(p);
>> +-      tcp_flag_word(th2) |= flags & (TCP_FLAG_FIN | TCP_FLAG_PSH);
>> ++      tcp_flag_word2(th2) = tcp_flag_word(th2) | flags & (TCP_FLAG_FIN | 
>> TCP_FLAG_PSH);
>
> net/ipv4/tcp.c: In function 'tcp_gro_receive':
> net/ipv4/tcp.c:2837:82: warning: suggest parentheses around arithmetic in 
> operand of '|' [-Wparentheses]
>
> Have you posted these patches to the netdev list? And is the response
> going to be "just make sure your network devices are receiving packets
> into sensibly-aligned buffers, and none of this is necessary"?

Yes, that would be the answer I expect from the list.

There is no reason to try to push this stuff upstream.

> There's a reason a lot of Ethernet drivers pad the start of their skb by
> 2 bytes before receiving the packet...

Tell it to however wired up this chip and shipped it in qty millions.
Actually that message was already received, successor chipsets from
this manufacturer did it up right.

I am somewhat forgiving in that. Compared to some arches I've worked
with the ar71xx is very solid. (for comparison, take the ep9302, where
a working toolchain didn't arrive until 2 years after the chip had
ceased production)

But this problem is a PITA, and as demonstrated, scribbling on these
core portions of the stack improves performance by an enormous amount.

> --
> dwmw2
>
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
>



-- 
Dave Täht
SKYPE: davetaht
US Tel: 1-239-829-5608
http://www.bufferbloat.net
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Reply via email to