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.