> hello Jiri and Yotam, > > On Thu, 2017-09-28 at 19:34 +0200, Jiri Pirko wrote: > > From: Yotam Gigi <yot...@mellanox.com> > > > > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h > > index 19e64bf..ada8214 100644 > > --- a/include/linux/skbuff.h > > +++ b/include/linux/skbuff.h > > @@ -772,6 +772,7 @@ struct sk_buff { > > __u8 remcsum_offload:1; > > #ifdef CONFIG_NET_SWITCHDEV > > __u8 offload_fwd_mark:1; > > + __u8 offload_mr_fwd_mark:1; > > I had a look at the pahole output: > > $ make allyesconfig > $ make net/core/skbuff.o > $ pahole net/core/skbuff.o | grep -C7 tc_from_ingress > > > __u8 ipvs_property:1; /* 147: 7 1 */ > __u8 inner_protocol_type:1; /* 147: 6 1 */ > __u8 remcsum_offload:1; /* 147: 5 1 */ > __u8 offload_fwd_mark:1; /* 147: 4 1 */ > __u8 tc_skip_classify:1; /* 147: 3 1 */ > __u8 tc_at_ingress:1; /* 147: 2 1 */ > __u8 tc_redirected:1; /* 147: 1 1 */ > __u8 tc_from_ingress:1; /* 147: 0 1 */ > __u16 tc_index; /* 148 2 */ > > /* XXX 2 bytes hole, try to pack */ > > union { > __wsum csum; /* 4 */ > struct { > > apparently there are no more spare bits to use at that offset: therefore, > adding 'offload_mr_fwd_mark' before 'tc_skip_classify' will make > 'tc_from_ingress' slip at offset 148, and tc_index at offset 150. > I think you can use that 2-bytes hole below tc_index, and also move the > offload_fwd_mark bit there, as we use both when > CONFIG_NET_SWITCHDEV is > enabled. This way we will also gain one spare bit, without changing the > struct size or worsening the cacheline alignments. > > what do you think?
Your pahole output still shows a 2B hole until the following union which is 4B-aligned. While it's true tc_index moves to offset 150, the union will not move [I.e., stay at offset 152] so the layout doesn't really change [greatly] nor the size of the struct. And we have the benefit of all the bits remaining consecutive.