| Message ID | 20200305052319.14682-9-snelson@pensando.io |
|---|---|
| State | Superseded |
| Delegated to: | David Miller |
| Headers | show |
| Series | ionic updates | expand |
On Wed, Mar 04, 2020 at 09:23:19PM -0800, Shannon Nelson wrote: > Use the default kernel version in ethtool drv_info output > and drop the module version. > > Cc: Leon Romanovsky <leonro@mellanox.com> > Signed-off-by: Shannon Nelson <snelson@pensando.io> > --- > drivers/net/ethernet/pensando/ionic/ionic.h | 1 - > drivers/net/ethernet/pensando/ionic/ionic_ethtool.c | 1 - > drivers/net/ethernet/pensando/ionic/ionic_main.c | 6 ++---- > 3 files changed, 2 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/ethernet/pensando/ionic/ionic.h b/drivers/net/ethernet/pensando/ionic/ionic.h > index c8ff33da243a..1c720759fd80 100644 > --- a/drivers/net/ethernet/pensando/ionic/ionic.h > +++ b/drivers/net/ethernet/pensando/ionic/ionic.h > @@ -12,7 +12,6 @@ struct ionic_lif; > > #define IONIC_DRV_NAME "ionic" > #define IONIC_DRV_DESCRIPTION "Pensando Ethernet NIC Driver" > -#define IONIC_DRV_VERSION "0.20.0-k" > > #define PCI_VENDOR_ID_PENSANDO 0x1dd8 > > diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c > index acd53e27d1ec..bea9b78e0189 100644 > --- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c > +++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c > @@ -86,7 +86,6 @@ static void ionic_get_drvinfo(struct net_device *netdev, > struct ionic *ionic = lif->ionic; > > strlcpy(drvinfo->driver, IONIC_DRV_NAME, sizeof(drvinfo->driver)); > - strlcpy(drvinfo->version, IONIC_DRV_VERSION, sizeof(drvinfo->version)); > strlcpy(drvinfo->fw_version, ionic->idev.dev_info.fw_version, > sizeof(drvinfo->fw_version)); > strlcpy(drvinfo->bus_info, ionic_bus_info(ionic), > diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c > index a8e3fb73b465..e4a76e66f542 100644 > --- a/drivers/net/ethernet/pensando/ionic/ionic_main.c > +++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c > @@ -6,6 +6,7 @@ > #include <linux/module.h> > #include <linux/netdevice.h> > #include <linux/utsname.h> > +#include <linux/vermagic.h> > > #include "ionic.h" > #include "ionic_bus.h" > @@ -15,7 +16,6 @@ > MODULE_DESCRIPTION(IONIC_DRV_DESCRIPTION); > MODULE_AUTHOR("Pensando Systems, Inc"); > MODULE_LICENSE("GPL"); > -MODULE_VERSION(IONIC_DRV_VERSION); > > static const char *ionic_error_to_str(enum ionic_status_code code) > { > @@ -414,7 +414,7 @@ int ionic_identify(struct ionic *ionic) > memset(ident, 0, sizeof(*ident)); > > ident->drv.os_type = cpu_to_le32(IONIC_OS_TYPE_LINUX); > - strncpy(ident->drv.driver_ver_str, IONIC_DRV_VERSION, > + strncpy(ident->drv.driver_ver_str, UTS_RELEASE, > sizeof(ident->drv.driver_ver_str) - 1); i see that you responded to my question about usage of this string [1] and I can't say anything about netdev policy on that, but in other subsystems, the idea that driver has duplicated debug functionalities to the general kernel code is not welcomed. [1] https://lore.kernel.org/netdev/e0cbc84c-7860-abf2-a622-4035be1479dc@pensando.io > > mutex_lock(&ionic->dev_cmd_lock); > @@ -558,8 +558,6 @@ int ionic_port_reset(struct ionic *ionic) > > static int __init ionic_init_module(void) > { > - pr_info("%s %s, ver %s\n", > - IONIC_DRV_NAME, IONIC_DRV_DESCRIPTION, IONIC_DRV_VERSION); > ionic_debugfs_create(); > return ionic_bus_register_driver(); > } > -- > 2.17.1 >
On 3/4/20 10:10 PM, Leon Romanovsky wrote: > On Wed, Mar 04, 2020 at 09:23:19PM -0800, Shannon Nelson wrote: >> Use the default kernel version in ethtool drv_info output >> and drop the module version. >> >> Cc: Leon Romanovsky <leonro@mellanox.com> >> Signed-off-by: Shannon Nelson <snelson@pensando.io> >> --- >> drivers/net/ethernet/pensando/ionic/ionic.h | 1 - >> drivers/net/ethernet/pensando/ionic/ionic_ethtool.c | 1 - >> drivers/net/ethernet/pensando/ionic/ionic_main.c | 6 ++---- >> 3 files changed, 2 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/net/ethernet/pensando/ionic/ionic.h b/drivers/net/ethernet/pensando/ionic/ionic.h >> index c8ff33da243a..1c720759fd80 100644 >> --- a/drivers/net/ethernet/pensando/ionic/ionic.h >> +++ b/drivers/net/ethernet/pensando/ionic/ionic.h >> @@ -12,7 +12,6 @@ struct ionic_lif; >> >> #define IONIC_DRV_NAME "ionic" >> #define IONIC_DRV_DESCRIPTION "Pensando Ethernet NIC Driver" >> -#define IONIC_DRV_VERSION "0.20.0-k" >> >> #define PCI_VENDOR_ID_PENSANDO 0x1dd8 >> >> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c >> index acd53e27d1ec..bea9b78e0189 100644 >> --- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c >> +++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c >> @@ -86,7 +86,6 @@ static void ionic_get_drvinfo(struct net_device *netdev, >> struct ionic *ionic = lif->ionic; >> >> strlcpy(drvinfo->driver, IONIC_DRV_NAME, sizeof(drvinfo->driver)); >> - strlcpy(drvinfo->version, IONIC_DRV_VERSION, sizeof(drvinfo->version)); >> strlcpy(drvinfo->fw_version, ionic->idev.dev_info.fw_version, >> sizeof(drvinfo->fw_version)); >> strlcpy(drvinfo->bus_info, ionic_bus_info(ionic), >> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c >> index a8e3fb73b465..e4a76e66f542 100644 >> --- a/drivers/net/ethernet/pensando/ionic/ionic_main.c >> +++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c >> @@ -6,6 +6,7 @@ >> #include <linux/module.h> >> #include <linux/netdevice.h> >> #include <linux/utsname.h> >> +#include <linux/vermagic.h> >> >> #include "ionic.h" >> #include "ionic_bus.h" >> @@ -15,7 +16,6 @@ >> MODULE_DESCRIPTION(IONIC_DRV_DESCRIPTION); >> MODULE_AUTHOR("Pensando Systems, Inc"); >> MODULE_LICENSE("GPL"); >> -MODULE_VERSION(IONIC_DRV_VERSION); >> >> static const char *ionic_error_to_str(enum ionic_status_code code) >> { >> @@ -414,7 +414,7 @@ int ionic_identify(struct ionic *ionic) >> memset(ident, 0, sizeof(*ident)); >> >> ident->drv.os_type = cpu_to_le32(IONIC_OS_TYPE_LINUX); >> - strncpy(ident->drv.driver_ver_str, IONIC_DRV_VERSION, >> + strncpy(ident->drv.driver_ver_str, UTS_RELEASE, >> sizeof(ident->drv.driver_ver_str) - 1); > i see that you responded to my question about usage of this string [1] > and I can't say anything about netdev policy on that, but in other > subsystems, the idea that driver has duplicated debug functionalities > to the general kernel code is not welcomed. > > [1] https://lore.kernel.org/netdev/e0cbc84c-7860-abf2-a622-4035be1479dc@pensando.io This DSC (Distributed Services Card) is more than a simple NIC, and in several use cases is intended to be managed centrally and installed in hosts that can be handed out to customers as bare-metal machines to which the datacenter personnel cannot access. The device can be accessed through a separate management network port, similar to an ilom or cimc other similar host management gizmo. Getting a little information about the driver into the card's logfiles allows for a little better debugging context from the management side without having access to the host. Yes, we want to keep functionality duplication to a minimum, but I think this is a different case. We also want to keep customer information leakage to a minimum, which is why we were using only the individual driver version info before it was replaced with the kernel version. I'd like to keep at least some bit of driver context information available to those on the management side of this PCI device. sln >> mutex_lock(&ionic->dev_cmd_lock); >> @@ -558,8 +558,6 @@ int ionic_port_reset(struct ionic *ionic) >> >> static int __init ionic_init_module(void) >> { >> - pr_info("%s %s, ver %s\n", >> - IONIC_DRV_NAME, IONIC_DRV_DESCRIPTION, IONIC_DRV_VERSION); >> ionic_debugfs_create(); >> return ionic_bus_register_driver(); >> } >> -- >> 2.17.1 >>
On Wed, Mar 04, 2020 at 11:20:11PM -0800, Shannon Nelson wrote: > On 3/4/20 10:10 PM, Leon Romanovsky wrote: > > On Wed, Mar 04, 2020 at 09:23:19PM -0800, Shannon Nelson wrote: > > > Use the default kernel version in ethtool drv_info output > > > and drop the module version. > > > > > > Cc: Leon Romanovsky <leonro@mellanox.com> > > > Signed-off-by: Shannon Nelson <snelson@pensando.io> > > > --- > > > drivers/net/ethernet/pensando/ionic/ionic.h | 1 - > > > drivers/net/ethernet/pensando/ionic/ionic_ethtool.c | 1 - > > > drivers/net/ethernet/pensando/ionic/ionic_main.c | 6 ++---- > > > 3 files changed, 2 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/net/ethernet/pensando/ionic/ionic.h b/drivers/net/ethernet/pensando/ionic/ionic.h > > > index c8ff33da243a..1c720759fd80 100644 > > > --- a/drivers/net/ethernet/pensando/ionic/ionic.h > > > +++ b/drivers/net/ethernet/pensando/ionic/ionic.h > > > @@ -12,7 +12,6 @@ struct ionic_lif; > > > > > > #define IONIC_DRV_NAME "ionic" > > > #define IONIC_DRV_DESCRIPTION "Pensando Ethernet NIC Driver" > > > -#define IONIC_DRV_VERSION "0.20.0-k" > > > > > > #define PCI_VENDOR_ID_PENSANDO 0x1dd8 > > > > > > diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c > > > index acd53e27d1ec..bea9b78e0189 100644 > > > --- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c > > > +++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c > > > @@ -86,7 +86,6 @@ static void ionic_get_drvinfo(struct net_device *netdev, > > > struct ionic *ionic = lif->ionic; > > > > > > strlcpy(drvinfo->driver, IONIC_DRV_NAME, sizeof(drvinfo->driver)); > > > - strlcpy(drvinfo->version, IONIC_DRV_VERSION, sizeof(drvinfo->version)); > > > strlcpy(drvinfo->fw_version, ionic->idev.dev_info.fw_version, > > > sizeof(drvinfo->fw_version)); > > > strlcpy(drvinfo->bus_info, ionic_bus_info(ionic), > > > diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c > > > index a8e3fb73b465..e4a76e66f542 100644 > > > --- a/drivers/net/ethernet/pensando/ionic/ionic_main.c > > > +++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c > > > @@ -6,6 +6,7 @@ > > > #include <linux/module.h> > > > #include <linux/netdevice.h> > > > #include <linux/utsname.h> > > > +#include <linux/vermagic.h> > > > > > > #include "ionic.h" > > > #include "ionic_bus.h" > > > @@ -15,7 +16,6 @@ > > > MODULE_DESCRIPTION(IONIC_DRV_DESCRIPTION); > > > MODULE_AUTHOR("Pensando Systems, Inc"); > > > MODULE_LICENSE("GPL"); > > > -MODULE_VERSION(IONIC_DRV_VERSION); > > > > > > static const char *ionic_error_to_str(enum ionic_status_code code) > > > { > > > @@ -414,7 +414,7 @@ int ionic_identify(struct ionic *ionic) > > > memset(ident, 0, sizeof(*ident)); > > > > > > ident->drv.os_type = cpu_to_le32(IONIC_OS_TYPE_LINUX); > > > - strncpy(ident->drv.driver_ver_str, IONIC_DRV_VERSION, > > > + strncpy(ident->drv.driver_ver_str, UTS_RELEASE, > > > sizeof(ident->drv.driver_ver_str) - 1); > > i see that you responded to my question about usage of this string [1] > > and I can't say anything about netdev policy on that, but in other > > subsystems, the idea that driver has duplicated debug functionalities > > to the general kernel code is not welcomed. > > > > [1] https://lore.kernel.org/netdev/e0cbc84c-7860-abf2-a622-4035be1479dc@pensando.io > > This DSC (Distributed Services Card) is more than a simple NIC, and in > several use cases is intended to be managed centrally and installed in hosts > that can be handed out to customers as bare-metal machines to which the > datacenter personnel cannot access. The device can be accessed through a > separate management network port, similar to an ilom or cimc other similar > host management gizmo. Getting a little information about the driver into > the card's logfiles allows for a little better debugging context from the > management side without having access to the host. > > Yes, we want to keep functionality duplication to a minimum, but I think > this is a different case. We also want to keep customer information leakage > to a minimum, which is why we were using only the individual driver version > info before it was replaced with the kernel version. I'd like to keep at > least some bit of driver context information available to those on the > management side of this PCI device. Again, I'm not netdev person and say nothing about if it is ok or not. However, this string is completely unreliable, especially if user changes your driver in his bare-metal machine. Thanks > > sln > > > > mutex_lock(&ionic->dev_cmd_lock); > > > @@ -558,8 +558,6 @@ int ionic_port_reset(struct ionic *ionic) > > > > > > static int __init ionic_init_module(void) > > > { > > > - pr_info("%s %s, ver %s\n", > > > - IONIC_DRV_NAME, IONIC_DRV_DESCRIPTION, IONIC_DRV_VERSION); > > > ionic_debugfs_create(); > > > return ionic_bus_register_driver(); > > > } > > > -- > > > 2.17.1 > > > >
diff --git a/drivers/net/ethernet/pensando/ionic/ionic.h b/drivers/net/ethernet/pensando/ionic/ionic.h index c8ff33da243a..1c720759fd80 100644 --- a/drivers/net/ethernet/pensando/ionic/ionic.h +++ b/drivers/net/ethernet/pensando/ionic/ionic.h @@ -12,7 +12,6 @@ struct ionic_lif; #define IONIC_DRV_NAME "ionic" #define IONIC_DRV_DESCRIPTION "Pensando Ethernet NIC Driver" -#define IONIC_DRV_VERSION "0.20.0-k" #define PCI_VENDOR_ID_PENSANDO 0x1dd8 diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c index acd53e27d1ec..bea9b78e0189 100644 --- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c +++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c @@ -86,7 +86,6 @@ static void ionic_get_drvinfo(struct net_device *netdev, struct ionic *ionic = lif->ionic; strlcpy(drvinfo->driver, IONIC_DRV_NAME, sizeof(drvinfo->driver)); - strlcpy(drvinfo->version, IONIC_DRV_VERSION, sizeof(drvinfo->version)); strlcpy(drvinfo->fw_version, ionic->idev.dev_info.fw_version, sizeof(drvinfo->fw_version)); strlcpy(drvinfo->bus_info, ionic_bus_info(ionic), diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c index a8e3fb73b465..e4a76e66f542 100644 --- a/drivers/net/ethernet/pensando/ionic/ionic_main.c +++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c @@ -6,6 +6,7 @@ #include <linux/module.h> #include <linux/netdevice.h> #include <linux/utsname.h> +#include <linux/vermagic.h> #include "ionic.h" #include "ionic_bus.h" @@ -15,7 +16,6 @@ MODULE_DESCRIPTION(IONIC_DRV_DESCRIPTION); MODULE_AUTHOR("Pensando Systems, Inc"); MODULE_LICENSE("GPL"); -MODULE_VERSION(IONIC_DRV_VERSION); static const char *ionic_error_to_str(enum ionic_status_code code) { @@ -414,7 +414,7 @@ int ionic_identify(struct ionic *ionic) memset(ident, 0, sizeof(*ident)); ident->drv.os_type = cpu_to_le32(IONIC_OS_TYPE_LINUX); - strncpy(ident->drv.driver_ver_str, IONIC_DRV_VERSION, + strncpy(ident->drv.driver_ver_str, UTS_RELEASE, sizeof(ident->drv.driver_ver_str) - 1); mutex_lock(&ionic->dev_cmd_lock); @@ -558,8 +558,6 @@ int ionic_port_reset(struct ionic *ionic) static int __init ionic_init_module(void) { - pr_info("%s %s, ver %s\n", - IONIC_DRV_NAME, IONIC_DRV_DESCRIPTION, IONIC_DRV_VERSION); ionic_debugfs_create(); return ionic_bus_register_driver(); }
Use the default kernel version in ethtool drv_info output and drop the module version. Cc: Leon Romanovsky <leonro@mellanox.com> Signed-off-by: Shannon Nelson <snelson@pensando.io> --- drivers/net/ethernet/pensando/ionic/ionic.h | 1 - drivers/net/ethernet/pensando/ionic/ionic_ethtool.c | 1 - drivers/net/ethernet/pensando/ionic/ionic_main.c | 6 ++---- 3 files changed, 2 insertions(+), 6 deletions(-)