On 8/30/24 03:20, Jakub Kicinski wrote:>> +/* Initialize the context fetching the relevant device and
+ * acquiring a reference to it.
+ */
+static int net_shaper_ctx_init(const struct genl_info *info, int type,
+                              struct net_shaper_nl_ctx *ctx)
+{
+       struct net *ns = genl_info_net(info);
+       struct net_device *dev;
+       int ifindex;
+
+       memset(ctx, 0, sizeof(*ctx));
+       if (GENL_REQ_ATTR_CHECK(info, type))
+               return -EINVAL;
+
+       ifindex = nla_get_u32(info->attrs[type]);

Let's limit the 'binding' thing to just driver call sites, we can
redo the rest easily later. This line and next pretends to take
"arbitrary" type but clearly wants a ifindex/netdev, right?

There is a misunderstanding. This helper will be used in a following patch (7/12) with a different 'type' argument: NET_SHAPER_A_BINDING_IFINDEX. I've put a note in the commit message, but was unintentionally dropped in one of the recent refactors. I'll add that note back.

I hope you are ok with the struct net_shaper_binding * argument to most helpers? does not add complexity, will help to support devlink objects and swapping back and forth from/to struct net_device* can't be automated.

[...]
Maybe send a patch like this, to avoid having to allocate this space,
and special casing dump vs doit:

diff --git a/include/net/genetlink.h b/include/net/genetlink.h
index 9ab49bfeae78..7658f0885178 100644
--- a/include/net/genetlink.h
+++ b/include/net/genetlink.h
@@ -124,7 +124,8 @@ struct genl_family {
   * @genlhdr: generic netlink message header
   * @attrs: netlink attributes
   * @_net: network namespace
- * @user_ptr: user pointers
+ * @ctx: storage space for the use by the family
+ * @user_ptr: user pointers (deprecated, use ctx instead)
   * @extack: extended ACK report struct
   */
  struct genl_info {
@@ -135,7 +136,10 @@ struct genl_info {
        struct genlmsghdr *     genlhdr;
        struct nlattr **        attrs;
        possible_net_t          _net;
-       void *                  user_ptr[2];
+       union {
+               u8              ctx[48];
+               void *          user_ptr[2];
+       };
        struct netlink_ext_ack *extack;
  };

Makes sense. Plus likely:

#define NETLINK_CTX_SIZE 48

and use such define above and in linux/netlink.h

Thanks,

Paolo

Reply via email to