RE: [patch net-next 1/7] skbuff: Add the offload_mr_fwd_mark field

hello Jiri and Yotam,

On Thu, 2017-09-28 at 19:34 +0200, Jiri Pirko wrote:

quoted

From: Yotam Gigi <redacted>

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.