| Message ID | 20201001000518.685243-3-kuba@kernel.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | genetlink: support per-command policy dump | expand |
On Wed, 2020-09-30 at 17:05 -0700, Jakub Kicinski wrote: > There are holes and oversized members in struct genl_family. > > Before: /* size: 104, cachelines: 2, members: 16 */ > After: /* size: 88, cachelines: 2, members: 16 */ > > The command field in struct genlmsghdr is a u8, so no point > in the operation count being 32 bit. Also operation 0 is > usually undefined, so we only need 255 entries. > > netnsok and parallel_ops are only ever initialized to true. The fundamentally are bools, so that makes sense :) > We can grow the fields as needed, compiler should warn us > if someone tries to assign larger constants. > > Signed-off-by: Jakub Kicinski <kuba@kernel.org> > --- > include/net/genetlink.h | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/include/net/genetlink.h b/include/net/genetlink.h > index a3484fd736d6..0033c76ff094 100644 > --- a/include/net/genetlink.h > +++ b/include/net/genetlink.h > @@ -48,8 +48,11 @@ struct genl_family { > char name[GENL_NAMSIZ]; > unsigned int version; > unsigned int maxattr; > - bool netnsok; > - bool parallel_ops; > + unsigned int mcgrp_offset; /* private */ In practice, we can probably also shrink that to u16, since it just gives you the offset of the multicast groups this family has in the whole table - and we'll hopefully never run more than 2**16 multicast groups across all genetlink families :) But it also doesn't matter much. Reviewed-by: Johannes Berg <johannes@sipsolutions.net> johannes
diff --git a/include/net/genetlink.h b/include/net/genetlink.h index a3484fd736d6..0033c76ff094 100644 --- a/include/net/genetlink.h +++ b/include/net/genetlink.h @@ -48,8 +48,11 @@ struct genl_family { char name[GENL_NAMSIZ]; unsigned int version; unsigned int maxattr; - bool netnsok; - bool parallel_ops; + unsigned int mcgrp_offset; /* private */ + u8 netnsok:1; + u8 parallel_ops:1; + u8 n_ops; + u8 n_mcgrps; const struct nla_policy *policy; int (*pre_doit)(const struct genl_ops *ops, struct sk_buff *skb, @@ -59,9 +62,6 @@ struct genl_family { struct genl_info *info); const struct genl_ops * ops; const struct genl_multicast_group *mcgrps; - unsigned int n_ops; - unsigned int n_mcgrps; - unsigned int mcgrp_offset; /* private */ struct module *module; };
There are holes and oversized members in struct genl_family. Before: /* size: 104, cachelines: 2, members: 16 */ After: /* size: 88, cachelines: 2, members: 16 */ The command field in struct genlmsghdr is a u8, so no point in the operation count being 32 bit. Also operation 0 is usually undefined, so we only need 255 entries. netnsok and parallel_ops are only ever initialized to true. We can grow the fields as needed, compiler should warn us if someone tries to assign larger constants. Signed-off-by: Jakub Kicinski <kuba@kernel.org> --- include/net/genetlink.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)