| Message ID | 20230921031409.514488-2-liuhangbin@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | IPv4: send notify when delete source address routes | expand |
On 9/20/23 9:14 PM, Hangbin Liu wrote: > The FIB info structure currently looks like this: > struct fib_info { > struct hlist_node fib_hash; /* 0 16 */ > [...] > u32 fib_priority; /* 80 4 */ > > /* XXX 4 bytes hole, try to pack */ > > struct dst_metrics * fib_metrics; /* 88 8 */ > int fib_nhs; /* 96 4 */ > bool fib_nh_is_v6; /* 100 1 */ > bool nh_updated; /* 101 1 */ > > /* XXX 2 bytes hole, try to pack */ 2B hole here and you want to add a single flag so another bool. I would prefer the delay to a bitfield until all holes are consumed.
On Thu, Sep 21, 2023 at 07:03:20AM -0600, David Ahern wrote: > On 9/20/23 9:14 PM, Hangbin Liu wrote: > > The FIB info structure currently looks like this: > > struct fib_info { > > struct hlist_node fib_hash; /* 0 16 */ > > [...] > > u32 fib_priority; /* 80 4 */ > > > > /* XXX 4 bytes hole, try to pack */ > > > > struct dst_metrics * fib_metrics; /* 88 8 */ > > int fib_nhs; /* 96 4 */ > > bool fib_nh_is_v6; /* 100 1 */ > > bool nh_updated; /* 101 1 */ > > > > /* XXX 2 bytes hole, try to pack */ > > 2B hole here and you want to add a single flag so another bool. I would > prefer the delay to a bitfield until all holes are consumed. > OK, just in case I didn't misunderstand. I should add a `bool pfsrc_removed` here and drop the first patch, right? Thanks Hangbin
On 9/21/23 7:29 PM, Hangbin Liu wrote: > On Thu, Sep 21, 2023 at 07:03:20AM -0600, David Ahern wrote: >> On 9/20/23 9:14 PM, Hangbin Liu wrote: >>> The FIB info structure currently looks like this: >>> struct fib_info { >>> struct hlist_node fib_hash; /* 0 16 */ >>> [...] >>> u32 fib_priority; /* 80 4 */ >>> >>> /* XXX 4 bytes hole, try to pack */ >>> >>> struct dst_metrics * fib_metrics; /* 88 8 */ >>> int fib_nhs; /* 96 4 */ >>> bool fib_nh_is_v6; /* 100 1 */ >>> bool nh_updated; /* 101 1 */ >>> >>> /* XXX 2 bytes hole, try to pack */ >> >> 2B hole here and you want to add a single flag so another bool. I would >> prefer the delay to a bitfield until all holes are consumed. >> > > OK, just in case I didn't misunderstand. I should add a `bool pfsrc_removed` > here and drop the first patch, right? yes. The bitfield has higher overhead. Let's reserve that overhead to when there are no more holes and a new field pushes the struct over 128B.
diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h index f0c13864180e..6d05469cf5da 100644 --- a/include/net/ip_fib.h +++ b/include/net/ip_fib.h @@ -152,8 +152,8 @@ struct fib_info { #define fib_rtt fib_metrics->metrics[RTAX_RTT-1] #define fib_advmss fib_metrics->metrics[RTAX_ADVMSS-1] int fib_nhs; - bool fib_nh_is_v6; - bool nh_updated; + u8 fib_nh_is_v6:1, + nh_updated:1; struct nexthop *nh; struct rcu_head rcu; struct fib_nh fib_nh[]; diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c index eafa4a033515..b2858b0a1229 100644 --- a/net/ipv4/fib_semantics.c +++ b/net/ipv4/fib_semantics.c @@ -1573,7 +1573,7 @@ struct fib_info *fib_create_info(struct fib_config *cfg, fib_info_update_nhc_saddr(net, &nexthop_nh->nh_common, fi->fib_scope); if (nexthop_nh->fib_nh_gw_family == AF_INET6) - fi->fib_nh_is_v6 = true; + fi->fib_nh_is_v6 = 1; } endfor_nexthops(fi) fib_rebalance(fi); diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c index bbff68b5b5d4..54ba53c89b3d 100644 --- a/net/ipv4/nexthop.c +++ b/net/ipv4/nexthop.c @@ -2213,12 +2213,12 @@ static void __nexthop_replace_notify(struct net *net, struct nexthop *nh, * and then walk the fib tables once */ list_for_each_entry(fi, &nh->fi_list, nh_list) - fi->nh_updated = true; + fi->nh_updated = 1; fib_info_notify_update(net, info); list_for_each_entry(fi, &nh->fi_list, nh_list) - fi->nh_updated = false; + fi->nh_updated = 0; } list_for_each_entry(f6i, &nh->f6i_list, nh_list)
The FIB info structure currently looks like this: struct fib_info { struct hlist_node fib_hash; /* 0 16 */ [...] u32 fib_priority; /* 80 4 */ /* XXX 4 bytes hole, try to pack */ struct dst_metrics * fib_metrics; /* 88 8 */ int fib_nhs; /* 96 4 */ bool fib_nh_is_v6; /* 100 1 */ bool nh_updated; /* 101 1 */ /* XXX 2 bytes hole, try to pack */ struct nexthop * nh; /* 104 8 */ struct callback_head rcu __attribute__((__aligned__(8))); /* 112 16 */ /* --- cacheline 2 boundary (128 bytes) --- */ struct fib_nh fib_nh[]; /* 128 0 */ /* size: 128, cachelines: 2, members: 21 */ /* sum members: 122, holes: 2, sum holes: 6 */ /* forced alignments: 1 */ } __attribute__((__aligned__(8))); Let's convert fib_nh_is_v6 and nh_updated to use a single bit, so that we can add other functional bits in later patch. Suggested-by: Ido Schimmel <idosch@idosch.org> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> --- include/net/ip_fib.h | 4 ++-- net/ipv4/fib_semantics.c | 2 +- net/ipv4/nexthop.c | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-)