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 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 23 Nov 2022 10:46:55 +0100 From: Jiri Pirko <jiri@...dia.com> To: Vincent Mailhol <mailhol.vincent@...adoo.fr> Cc: netdev@...r.kernel.org, "David S . Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, linux-kernel@...r.kernel.org Subject: Re: [RFC PATCH] net: devlink: devlink_nl_info_fill: populate default information Tue, Nov 22, 2022 at 04:49:34PM CET, mailhol.vincent@...adoo.fr wrote: >Some piece of information are common to the vast majority of the >devices. Examples are: > > * the driver name. > * the serial number of a USB device. > >Modify devlink_nl_info_fill() to retrieve those information so that >the drivers do not have to. Rationale: factorize code. > >Signed-off-by: Vincent Mailhol <mailhol.vincent@...adoo.fr> >--- >I am sending this as an RFC because I just started to study devlink. > >I can see a parallel with ethtool for which the core will fill >whatever it can. c.f.: >commit f20a0a0519f3 ("ethtool: doc: clarify what drivers can implement in their get_drvinfo()") >Link: https://git.kernel.org/netdev/net-next/c/f20a0a0519f3 > >I think that devlink should do the same. > >Right now, I identified two fields. If this RFC receive positive >feedback, I will iron it up and try to see if there is more that can >be filled by default. > >Thank you for your comments. >--- > net/core/devlink.c | 36 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 36 insertions(+) > >diff --git a/net/core/devlink.c b/net/core/devlink.c >index 7f789bbcbbd7..1908b360caf7 100644 >--- a/net/core/devlink.c >+++ b/net/core/devlink.c >@@ -18,6 +18,7 @@ > #include <linux/netdevice.h> > #include <linux/spinlock.h> > #include <linux/refcount.h> >+#include <linux/usb.h> > #include <linux/workqueue.h> > #include <linux/u64_stats_sync.h> > #include <linux/timekeeping.h> >@@ -6685,12 +6686,37 @@ int devlink_info_version_running_put_ext(struct devlink_info_req *req, > } > EXPORT_SYMBOL_GPL(devlink_info_version_running_put_ext); > >+static int devlink_nl_driver_info_get(struct device_driver *drv, >+ struct devlink_info_req *req) >+{ >+ if (!drv) >+ return 0; >+ >+ if (drv->name[0]) >+ return devlink_info_driver_name_put(req, drv->name); Make sure that this provides the same value for all existing drivers using devlink. >+ >+ return 0; >+} >+ >+static int devlink_nl_usb_info_get(struct usb_device *udev, >+ struct devlink_info_req *req) >+{ >+ if (!udev) >+ return 0; >+ >+ if (udev->serial[0]) >+ return devlink_info_serial_number_put(req, udev->serial); >+ >+ return 0; >+} >+ > static int > devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink, > enum devlink_command cmd, u32 portid, > u32 seq, int flags, struct netlink_ext_ack *extack) > { > struct devlink_info_req req = {}; >+ struct device *dev = devlink_to_dev(devlink); > void *hdr; > int err; > >@@ -6707,6 +6733,16 @@ devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink, > if (err) > goto err_cancel_msg; > >+ err = devlink_nl_driver_info_get(dev->driver, &req); >+ if (err) >+ goto err_cancel_msg; >+ >+ if (!strcmp(dev->parent->type->name, "usb_device")) { Comparing to string does not seem correct here. >+ err = devlink_nl_usb_info_get(to_usb_device(dev->parent), &req); As Jakub pointed out, you have to make sure that driver does not put the same attrs again. You have to introduce this functionality with removing the fill-ups in drivers atomically, in a single patch. >+ if (err) >+ goto err_cancel_msg; >+ } >+ > genlmsg_end(msg, hdr); > return 0; > >-- >2.37.4 >
Powered by blists - more mailing lists