From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.5 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_PASS,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 550C2C282D7 for ; Wed, 30 Jan 2019 23:22:24 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id EF32320881 for ; Wed, 30 Jan 2019 23:22:23 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=resnulli-us.20150623.gappssmtp.com header.i=@resnulli-us.20150623.gappssmtp.com header.b="JnUQVUEp" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727586AbfA3XWX (ORCPT ); Wed, 30 Jan 2019 18:22:23 -0500 Received: from mail-wm1-f65.google.com ([209.85.128.65]:51396 "EHLO mail-wm1-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727056AbfA3XWW (ORCPT ); Wed, 30 Jan 2019 18:22:22 -0500 Received: by mail-wm1-f65.google.com with SMTP id b11so600603wmj.1 for ; Wed, 30 Jan 2019 15:22:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=resnulli-us.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=IWVYFlCa6Zc+c4FrLMNpS5BVFapzGRxkoyEZ2iDDw2U=; b=JnUQVUEpORujwABYS4ufI34OH1URDycvy9eep8aVfWBz6H0erKu/5QGJKZlzBhqRvC +3xfdupybHziTIGjquJFycvCwQ0pMQNukgVXApIJ2El1q5EPLyRmYRGIbTLj8670uakT inTKvVJStwYvB0v+rpWy+jE2DJ3GKAamd3dbRDcJ0hoIZnzwUlGiejZSSU4ebXv3m+6Q zlbplkyjUMP5HRgfBXlJlIFzKbiTMEglQXUzkWBHGVtlk52daGikyc9dQXBpANK3NQY0 ivqGCJGdPXdkfQTIoqz61mOMKDPh5FMTwhabUce7N3EKjL/TEXNfe1Kxwv6X6NhKGMYq RqSA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=IWVYFlCa6Zc+c4FrLMNpS5BVFapzGRxkoyEZ2iDDw2U=; b=SfMR/Bq1zq2obl8iIL97ehCWqxYxa+YVBOlt6VNTl6tneZPUig1RNlBgE1OwjNznvp UPowbXmGQD/pJVWlccd1CRX/4cvtveNW5XYBGxT3jaq4YKA+yY4MxIdzi2hmL4RREy3w 6IyetpHBjjAJz80+GOS9txzEZxiVx5TUrUlk9CKNuOssDknIqA4ZPhvAG+AqhApUex90 ACmD/1iUIGJnwFV2cIW0BHJBi/DzmLn6z+LgDAuUs4aD+9K0kliRVy7XLrE+Nv2JIAXT lrwFWk1dmKV5CyPywYWnzB7W9QR+BBsgIcFFr4T8USNc6XJDTCvwjOX7x4frKUzKo0Go ntbg== X-Gm-Message-State: AJcUukcTWVTHcLlkhkttEFFx2bxYLeEUrCqse4zv86gAmQKU74n531I9 fESN30HQqo2ur8X2Ib1Gg/TUhHk9H0o= X-Google-Smtp-Source: ALg8bN6fdkkTMzU31ui6381OXYgha06mKGPaaofImiFR3I9Oe9Nptrrhsbbh4Q+MBG/YdTPbvWLf4g== X-Received: by 2002:a1c:282:: with SMTP id 124mr26485631wmc.113.1548886886294; Wed, 30 Jan 2019 14:21:26 -0800 (PST) Received: from localhost (jirka.pirko.cz. [84.16.102.26]) by smtp.gmail.com with ESMTPSA id g9sm3604244wmg.44.2019.01.30.14.21.25 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 30 Jan 2019 14:21:25 -0800 (PST) Date: Wed, 30 Jan 2019 23:12:25 +0100 From: Jiri Pirko To: Jakub Kicinski Cc: davem@davemloft.net, netdev@vger.kernel.org, oss-drivers@netronome.com, andrew@lunn.ch, f.fainelli@gmail.com, mkubecek@suse.cz, eugenem@fb.com, jonathan.lemon@gmail.com Subject: Re: [PATCH net-next v2 7/7] ethtool: add compat for devlink info Message-ID: <20190130221225.GE349@nanopsycho.orion> References: <20190130190513.25718-1-jakub.kicinski@netronome.com> <20190130190513.25718-8-jakub.kicinski@netronome.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190130190513.25718-8-jakub.kicinski@netronome.com> User-Agent: Mutt/1.10.1 (2018-07-13) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Wed, Jan 30, 2019 at 08:05:13PM CET, jakub.kicinski@netronome.com wrote: >If driver did not fill the fw_version field, try to call into >the new devlink get_info op and collect the versions that way. >We assume ethtool was always reporting running versions. > >Signed-off-by: Jakub Kicinski >--- > include/net/devlink.h | 7 ++++++ > net/core/devlink.c | 52 ++++++++++++++++++++++++++++++++++++++++++- > net/core/ethtool.c | 7 ++++++ > 3 files changed, 65 insertions(+), 1 deletion(-) > >diff --git a/include/net/devlink.h b/include/net/devlink.h >index c678ed0cb099..b4750e93303a 100644 >--- a/include/net/devlink.h >+++ b/include/net/devlink.h >@@ -640,6 +640,8 @@ int devlink_info_report_version(struct devlink_info_req *req, > enum devlink_version_type type, > const char *version_name, > const char *version_value); >+void devlink_compat_running_versions(struct net_device *dev, >+ char *buf, size_t len); > > #else > >@@ -957,6 +959,11 @@ devlink_info_report_version(struct devlink_info_req *req, > { > return 0; > } >+ >+static inline void >+devlink_compat_running_versions(struct net_device *dev, char *buf, size_t len) >+{ >+} > #endif > > #endif /* _NET_DEVLINK_H_ */ >diff --git a/net/core/devlink.c b/net/core/devlink.c >index e2027d3a5e40..5313e5918ee2 100644 >--- a/net/core/devlink.c >+++ b/net/core/devlink.c >@@ -3715,12 +3715,18 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb, > } > > struct devlink_info_req { >+ bool compat; > struct sk_buff *msg; >+ /* For compat call */ >+ char *buf; union? >+ size_t len; > }; > > int devlink_info_report_driver_name(struct devlink_info_req *req, > const char *name) > { >+ if (req->compat) >+ return 0; > return nla_put_string(req->msg, DEVLINK_ATTR_INFO_DRV_NAME, name); > } > EXPORT_SYMBOL_GPL(devlink_info_report_driver_name); >@@ -3728,6 +3734,8 @@ EXPORT_SYMBOL_GPL(devlink_info_report_driver_name); > int devlink_info_report_serial_number(struct devlink_info_req *req, > const char *sn) > { >+ if (req->compat) >+ return 0; > return nla_put_string(req->msg, DEVLINK_ATTR_INFO_SERIAL_NUMBER, sn); > } > EXPORT_SYMBOL_GPL(devlink_info_report_serial_number); >@@ -3743,7 +3751,15 @@ int devlink_info_report_version(struct devlink_info_req *req, > [DEVLINK_VERSION_RUNNING] = DEVLINK_ATTR_INFO_VERSION_RUNNING, > }; > struct nlattr *nest; >- int err; >+ int len, err; >+ >+ if (req->compat) { >+ if (type == DEVLINK_VERSION_RUNNING) { >+ len = strlcpy(req->buf, version_value, req->len); If you have multiple running versions, shouldn't you concat them into a single string for compat? >+ req->len = max_t(size_t, 0, req->len - len); >+ } >+ return 0; >+ } > > if (type >= ARRAY_SIZE(type2attr) || !type2attr[type]) > return -EINVAL; >@@ -3789,6 +3805,7 @@ devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink, > if (devlink_nl_put_handle(msg, devlink)) > goto err_cancel_msg; > >+ memset(&req, 0, sizeof(req)); > req.msg = msg; > err = devlink->ops->info_get(devlink, &req, extack); > if (err) >@@ -5263,6 +5280,39 @@ int devlink_region_snapshot_create(struct devlink_region *region, u64 data_len, > } > EXPORT_SYMBOL_GPL(devlink_region_snapshot_create); > >+void devlink_compat_running_versions(struct net_device *dev, Why "versionS?" >+ char *buf, size_t len) >+{ >+ struct devlink_port *devlink_port; >+ struct devlink_info_req req; >+ struct devlink *devlink; >+ bool found = false; >+ >+ mutex_lock(&devlink_mutex); >+ list_for_each_entry(devlink, &devlink_list, list) { >+ mutex_lock(&devlink->lock); >+ list_for_each_entry(devlink_port, &devlink->port_list, list) { >+ if (devlink_port->type == DEVLINK_PORT_TYPE_ETH || >+ devlink_port->type_dev == dev) { >+ mutex_unlock(&devlink->lock); >+ found = true; >+ goto out; >+ } >+ } >+ mutex_unlock(&devlink->lock); >+ } >+out: >+ if (found && devlink->ops->info_get) { >+ memset(&req, 0, sizeof(req)); >+ req.compat = true; >+ req.buf = buf; >+ req.len = len; >+ >+ devlink->ops->info_get(devlink, &req, NULL); I don't really like this compat bool and check in "put" functions. I would rather like to run info_get() in case of both compat and non-compat in the same way. Then for compat just pickup the info which is needed and put to buf. I don't see problem in using netlink skb for that direcly. >+ } >+ mutex_unlock(&devlink_mutex); >+} >+ > static int __init devlink_module_init(void) > { > return genl_register_family(&devlink_nl_family); >diff --git a/net/core/ethtool.c b/net/core/ethtool.c >index 158264f7cfaf..176b17d11f08 100644 >--- a/net/core/ethtool.c >+++ b/net/core/ethtool.c >@@ -27,6 +27,7 @@ > #include > #include > #include >+#include > #include > > /* >@@ -803,6 +804,12 @@ static noinline_for_stack int ethtool_get_drvinfo(struct net_device *dev, > if (ops->get_eeprom_len) > info.eedump_len = ops->get_eeprom_len(dev); > >+ rtnl_unlock(); >+ if (!info.fw_version[0]) >+ devlink_compat_running_versions(dev, info.fw_version, >+ ARRAY_SIZE(info.fw_version)); ARRAY_SIZE looks odd here. "sizeof()" would be better I think. >+ rtnl_lock(); >+ > if (copy_to_user(useraddr, &info, sizeof(info))) > return -EFAULT; > return 0; >-- >2.19.2 >