lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 10 Oct 2019 20:17:43 +0200
From:   Michal Kubecek <mkubecek@...e.cz>
To:     netdev@...r.kernel.org
Cc:     Jiri Pirko <jiri@...nulli.us>, David Miller <davem@...emloft.net>,
        Jakub Kicinski <jakub.kicinski@...ronome.com>,
        Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        John Linville <linville@...driver.com>,
        Stephen Hemminger <stephen@...workplumber.org>,
        Johannes Berg <johannes@...solutions.net>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v7 13/17] ethtool: add standard notification
 handler

On Thu, Oct 10, 2019 at 05:25:59PM +0200, Jiri Pirko wrote:
> Wed, Oct 09, 2019 at 10:59:40PM CEST, mkubecek@...e.cz wrote:
> >+static void *ethnl_bcastmsg_put(struct sk_buff *skb, u8 cmd)
> >+{
> >+	return genlmsg_put(skb, 0, ++ethnl_bcast_seq, &ethtool_genl_family, 0,
> >+			   cmd);
> >+}
> >+
> >+static int ethnl_multicast(struct sk_buff *skb, struct net_device *dev)
> >+{
> >+	return genlmsg_multicast_netns(&ethtool_genl_family, dev_net(dev), skb,
> >+				       0, ETHNL_MCGRP_MONITOR, GFP_KERNEL);
> >+}
> 
> No need for these 2 helpers. Just put the code directly into
> ethnl_std_notify() and make the code easier to read.

In later patches (not submitted yet), these two will be also called by
other notification handlers.

> >+static const struct get_request_ops *ethnl_std_notify_to_ops(unsigned int cmd)
> >+{
> >+	WARN_ONCE(1, "unexpected notification type %u\n", cmd);
> >+	return NULL;
> >+}
> 
> Why this isn't a table similar to get_requests ?

It's a relic of earlier version before splitting the complex message
types when the table was rather sparse. I'll change it to a lookup table
to make it consistent with the rest of the code.

> >+
> >+/* generic notification handler */
> >+static void ethnl_std_notify(struct net_device *dev, unsigned int cmd,
> 
> Better "common" comparing to "standard", I believe.

That's similar to ethnl_std_parse(), the idea is that this is the
standard handler for notifications which are triggered without
additional data and the message is the same as reply to corresponding
"GET" request (which is generated by the standard ethnl_get_doit()
handler). Notifications for actions and notifications for SET commands
which cannot be generated this standard way will have to use their own
(nonstandard) handler.

Michal

Powered by blists - more mailing lists