From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Dumazet Subject: Re: [net-next] tcp: allow drivers to tweak TSQ logic Date: Sat, 11 Nov 2017 15:38:55 -0800 Message-ID: <1510443535.2849.147.camel@edumazet-glaptop3.roam.corp.google.com> References: <1510281664.2849.143.camel@edumazet-glaptop3.roam.corp.google.com> <1510410439.12037.10.camel@sipsolutions.net> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: David Miller , netdev , Toke =?ISO-8859-1?Q?H=F8iland-J=F8rgensen?= , Kir Kolyshkin To: Johannes Berg Return-path: Received: from mail-pf0-f193.google.com ([209.85.192.193]:52643 "EHLO mail-pf0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752203AbdKKXi6 (ORCPT ); Sat, 11 Nov 2017 18:38:58 -0500 Received: by mail-pf0-f193.google.com with SMTP id m88so2005796pfi.9 for ; Sat, 11 Nov 2017 15:38:58 -0800 (PST) In-Reply-To: <1510410439.12037.10.camel@sipsolutions.net> Sender: netdev-owner@vger.kernel.org List-ID: On Sat, 2017-11-11 at 15:27 +0100, Johannes Berg wrote: > Thanks Eric! > > > We expect wifi drivers to set this field to smaller values (tests have > > been done with values from 6 to 9) > > I suppose we should test each driver or so. > > > They would have to use following template : > > > > if (skb->sk && skb->sk->sk_pacing_shift != MY_PACING_SHIFT) > > skb->sk->sk_pacing_shift = MY_PACING_SHIFT; > > Hm. I wish we wouldn't have to do this on every skb, but perhaps it > doesn't matter that much. Yes, it does not matter, even at 40Gbit ;) > > > > u16 sk_gso_max_segs; > > + u8 sk_pacing_shift; > > I guess you tried to fill a hole, but weren't we saying that it would > be better in the same cacheline? Then again, perhaps both cachelines > are resident anyway, haven't looked at this now. Same cache line already ;) u32 sk_pacing_rate; /* 0x1c0 0x4 */ u32 sk_max_pacing_rate; /* 0x1c4 0x4 */ struct page_frag sk_frag; /* 0x1c8 0x10 */ netdev_features_t sk_route_caps; /* 0x1d8 0x8 */ netdev_features_t sk_route_nocaps; /* 0x1e0 0x8 */ int sk_gso_type; /* 0x1e8 0x4 */ unsigned int sk_gso_max_size; /* 0x1ec 0x4 */ gfp_t sk_allocation; /* 0x1f0 0x4 */ __u32 sk_txhash; /* 0x1f4 0x4 */ unsigned int __sk_flags_offset[0]; /* 0x1f8 0 */ unsigned int sk_padding:1; /* 0x1f8:0x1f 0x4 */ unsigned int sk_kern_sock:1; /* 0x1f8:0x1e 0x4 */ unsigned int sk_no_check_tx:1; /* 0x1f8:0x1d 0x4 */ unsigned int sk_no_check_rx:1; /* 0x1f8:0x1c 0x4 */ unsigned int sk_userlocks:4; /* 0x1f8:0x18 0x4 */ unsigned int sk_protocol:8; /* 0x1f8:0x10 0x4 */ unsigned int sk_type:16; /* 0x1f8: 0 0x4 */ u16 sk_gso_max_segs; /* 0x1fc 0x2 */ u8 sk_pacing_shift; /* 0x1fe 0x1 */ > > Unrelated to that, I think this is missing a documentation update since > the struct has kernel-doc comments. Yeah, I believe these kernel-doc on gigantic struct sock are useless and we should remove them, they have zero useful info.