mbox series

[RFC,net-next,0/9] genetlink: support per-command policy dump

Message ID 20201001000518.685243-1-kuba@kernel.org
Headers show
Series genetlink: support per-command policy dump | expand

Message

Jakub Kicinski Oct. 1, 2020, 12:05 a.m. UTC
Hi!

The objective of this series is to dump ethtool policies
to be able to tell which flags are supported by the kernel.
Ethtool policies are per command.

First patch here is included for completeness - it's already
in net, but other patches won't apply cleanly without it.

The series adds new set of "light" ops which don't have all
the callbacks, and won't have the policy. Most of families
are then moved to these ops. This gives us 4096B in savings
on an allyesconfig build (not counting the growth that would
have happened when policy is added):

     text       data       bss        dec       hex
244415581  227958581  78372980  550747142  20d3bc06
244415581  227962677  78372980  550751238  20d3cc06

Next 5 patches deal the dumping per-op policy.

Jakub Kicinski (9):
  genetlink: add missing kdoc for validation flags
  genetlink: reorg struct genl_family
  genetlink: add small version of ops
  genetlink: move to smaller ops wherever possible
  genetlink: add a structure for dump state
  genetlink: use .start callback for dumppolicy
  genetlink: bring back per op policy
  genetlink: use per-op policy for CTRL_CMD_GETPOLICY
  genetlink: allow dumping command-specific policy

 drivers/block/nbd.c                      |   6 +-
 drivers/net/gtp.c                        |   6 +-
 drivers/net/ieee802154/mac802154_hwsim.c |   6 +-
 drivers/net/macsec.c                     |   6 +-
 drivers/net/team/team.c                  |   6 +-
 drivers/net/wireless/mac80211_hwsim.c    |   6 +-
 drivers/target/target_core_user.c        |   6 +-
 drivers/thermal/thermal_netlink.c        |   6 +-
 fs/dlm/netlink.c                         |   6 +-
 include/net/genetlink.h                  |  40 +++-
 include/uapi/linux/genetlink.h           |   1 +
 kernel/taskstats.c                       |   6 +-
 net/batman-adv/netlink.c                 |   6 +-
 net/core/devlink.c                       |   6 +-
 net/core/drop_monitor.c                  |   6 +-
 net/hsr/hsr_netlink.c                    |   6 +-
 net/ieee802154/netlink.c                 |   6 +-
 net/ipv4/fou.c                           |   6 +-
 net/ipv4/tcp_metrics.c                   |   6 +-
 net/l2tp/l2tp_netlink.c                  |   6 +-
 net/mptcp/pm_netlink.c                   |   6 +-
 net/ncsi/ncsi-netlink.c                  |   6 +-
 net/netfilter/ipvs/ip_vs_ctl.c           |   6 +-
 net/netlabel/netlabel_calipso.c          |   6 +-
 net/netlabel/netlabel_cipso_v4.c         |   6 +-
 net/netlabel/netlabel_mgmt.c             |   6 +-
 net/netlabel/netlabel_unlabeled.c        |   6 +-
 net/netlink/genetlink.c                  | 225 ++++++++++++++++-------
 net/openvswitch/conntrack.c              |   6 +-
 net/openvswitch/datapath.c               |  24 +--
 net/openvswitch/meter.c                  |   6 +-
 net/psample/psample.c                    |   6 +-
 net/tipc/netlink_compat.c                |   6 +-
 net/wimax/stack.c                        |   6 +-
 net/wireless/nl80211.c                   |   5 +
 35 files changed, 304 insertions(+), 171 deletions(-)

Comments

Johannes Berg Oct. 1, 2020, 7:40 a.m. UTC | #1
On Wed, 2020-09-30 at 17:05 -0700, Jakub Kicinski wrote:
> We want to add maxattr and policy back to genl_ops, to enable
> dumping per command policy to user space. This, however, would
> cause bloat for all the families with global policies. Introduce
> smaller version of ops (half the size of genl_ops). Translate
> these smaller ops into a full blown struct before use in the
> core.

LGTM. That part about the WARN_ON was even easier than I thought :)

Reviewed-by: Johannes Berg <johannes@sipsolutions.net>

johannes
Johannes Berg Oct. 1, 2020, 7:49 a.m. UTC | #2
On Wed, 2020-09-30 at 17:05 -0700, Jakub Kicinski wrote:
> The structure of ctrl_dumppolicy() is clearly split into
> init and dumping. Move the init to a .start callback
> for clarity, it's a more idiomatic netlink dump code structure.

Yep, makes sense.

Reviewed-by: Johannes Berg <johannes@sipsolutions.net>

johannes
Johannes Berg Oct. 1, 2020, 7:53 a.m. UTC | #3
On Wed, 2020-09-30 at 17:05 -0700, Jakub Kicinski wrote:
> Add policy to the struct genl_ops structure, this time
> with maxattr, so it can be used properly.
> 
> Propagate .policy and .maxattr from the family
> in genl_get_cmd() if needed, this say the rest of the

typo: "this way"

> code does not have to worry if the policy is per op
> or global.

Maybe make 'taskstats', which I munged a bit in commit 3b0f31f2b8c9
("genetlink: make policy common to family") go back to using per-command 
policy - properly this time with maxattr?


But the code here looks good to me.

Reviewed-by: Johannes Berg <johannes@sipsolutions.net>

johannes