| 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
| ||
|
Message-ID: <34c52e04-00aa-4507-9150-0230ce760c9a@gmail.com>
Date: Wed, 28 Aug 2024 10:49:06 -0700
From: Mohsin Bashir <mohsin.bashr@...il.com>
To: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>,
netdev@...r.kernel.org
Cc: netdev@...r.kernel.org, alexanderduyck@...com, kuba@...nel.org,
andrew@...n.ch, davem@...emloft.net, edumazet@...gle.com, pabeni@...hat.com,
kernel-team@...a.com, sanmanpradhan@...a.com, sdf@...ichev.me,
jdamato@...tly.com
Subject: Re: [PATCH net-next v2 1/2] eth: fbnic: Add ethtool support for fbnic
On 8/27/24 11:29 PM, Michal Swiatkowski wrote:
> On Tue, Aug 27, 2024 at 01:59:03PM -0700, Mohsin Bashir wrote:
>> Add ethtool ops support and enable 'get_drvinfo' for fbnic. The driver
>> provides firmware version information while the driver name and bus
>> information is provided by ethtool_get_drvinfo().
>>
>> Signed-off-by: Mohsin Bashir <mohsin.bashr@...il.com>
>> ---
>> v2:
>> - Update the emptiness check for firmware version commit string
>> - Rebase to the latest
>>
>> v1: https://lore.kernel.org/netdev/20240807002445.3833895-1-mohsin.bashr@gmail.com
> Correct link:
> https://lore.kernel.org/netdev/20240822184944.3882360-1-mohsin.bashr@gmail.com/
Thank you for pointing this out.
>
>> ---
>> drivers/net/ethernet/meta/fbnic/Makefile | 1 +
>> drivers/net/ethernet/meta/fbnic/fbnic.h | 3 +++
>> .../net/ethernet/meta/fbnic/fbnic_ethtool.c | 26 +++++++++++++++++++
>> drivers/net/ethernet/meta/fbnic/fbnic_fw.c | 13 ++++++++++
>> drivers/net/ethernet/meta/fbnic/fbnic_fw.h | 6 ++---
>> .../net/ethernet/meta/fbnic/fbnic_netdev.c | 2 ++
>> .../net/ethernet/meta/fbnic/fbnic_netdev.h | 1 +
>> 7 files changed, 49 insertions(+), 3 deletions(-)
>> create mode 100644 drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
>>
>> diff --git a/drivers/net/ethernet/meta/fbnic/Makefile b/drivers/net/ethernet/meta/fbnic/Makefile
>> index 9373b558fdc9..37cfc34a5118 100644
>> --- a/drivers/net/ethernet/meta/fbnic/Makefile
>> +++ b/drivers/net/ethernet/meta/fbnic/Makefile
>> @@ -8,6 +8,7 @@
>> obj-$(CONFIG_FBNIC) += fbnic.o
>>
>> fbnic-y := fbnic_devlink.o \
>> + fbnic_ethtool.o \
>> fbnic_fw.o \
>> fbnic_irq.o \
>> fbnic_mac.o \
>> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic.h b/drivers/net/ethernet/meta/fbnic/fbnic.h
>> index ad2689bfd6cb..28d970f81bfc 100644
>> --- a/drivers/net/ethernet/meta/fbnic/fbnic.h
>> +++ b/drivers/net/ethernet/meta/fbnic/fbnic.h
>> @@ -132,6 +132,9 @@ void fbnic_free_irq(struct fbnic_dev *dev, int nr, void *data);
>> void fbnic_free_irqs(struct fbnic_dev *fbd);
>> int fbnic_alloc_irqs(struct fbnic_dev *fbd);
>>
>> +void fbnic_get_fw_ver_commit_str(struct fbnic_dev *fbd, char *fw_version,
>> + const size_t str_sz);
>> +
>> enum fbnic_boards {
>> fbnic_board_asic
>> };
>> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
>> new file mode 100644
>> index 000000000000..7064dfc9f5b0
>> --- /dev/null
>> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
>> @@ -0,0 +1,26 @@
>> +#include <linux/ethtool.h>
>> +#include <linux/netdevice.h>
>> +#include <linux/pci.h>
>> +
>> +#include "fbnic.h"
>> +#include "fbnic_netdev.h"
>> +#include "fbnic_tlv.h"
>> +
>> +static void
>> +fbnic_get_drvinfo(struct net_device *netdev, struct ethtool_drvinfo *drvinfo)
>> +{
>> + struct fbnic_net *fbn = netdev_priv(netdev);
>> + struct fbnic_dev *fbd = fbn->fbd;
>> +
>> + fbnic_get_fw_ver_commit_str(fbd, drvinfo->fw_version,
>> + sizeof(drvinfo->fw_version));
>> +}
>> +
>> +static const struct ethtool_ops fbnic_ethtool_ops = {
>> + .get_drvinfo = fbnic_get_drvinfo,
>> +};
>> +
>> +void fbnic_set_ethtool_ops(struct net_device *dev)
>> +{
>> + dev->ethtool_ops = &fbnic_ethtool_ops;
>> +}
>> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_fw.c b/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
>> index 0c6e1b4c119b..8f7a2a19ddf8 100644
>> --- a/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
>> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
>> @@ -789,3 +789,16 @@ void fbnic_mbx_flush_tx(struct fbnic_dev *fbd)
>> count += (tx_mbx->head - head) % FBNIC_IPC_MBX_DESC_LEN;
>> } while (count < FBNIC_IPC_MBX_DESC_LEN && --attempts);
>> }
>> +
>> +void fbnic_get_fw_ver_commit_str(struct fbnic_dev *fbd, char *fw_version,
>> + const size_t str_sz)
> Don't you need a prototype for this function?
We do have the prototype in fbnic.h
>> +{
>> + struct fbnic_fw_ver *mgmt = &fbd->fw_cap.running.mgmt;
>> + const char *delim = "";
>> +
>> + if (mgmt->commit[0])
>> + delim = "_";
>> +
>> + fbnic_mk_full_fw_ver_str(mgmt->version, delim, mgmt->commit,
>> + fw_version, str_sz);
>> +}
>> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_fw.h b/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
>> index c65bca613665..221faf8c6756 100644
>> --- a/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
>> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
>> @@ -53,10 +53,10 @@ int fbnic_fw_xmit_ownership_msg(struct fbnic_dev *fbd, bool take_ownership);
>> int fbnic_fw_init_heartbeat(struct fbnic_dev *fbd, bool poll);
>> void fbnic_fw_check_heartbeat(struct fbnic_dev *fbd);
>>
>> -#define fbnic_mk_full_fw_ver_str(_rev_id, _delim, _commit, _str) \
>> +#define fbnic_mk_full_fw_ver_str(_rev_id, _delim, _commit, _str, _str_sz) \
>> do { \
>> const u32 __rev_id = _rev_id; \
>> - snprintf(_str, sizeof(_str), "%02lu.%02lu.%02lu-%03lu%s%s", \
>> + snprintf(_str, _str_sz, "%02lu.%02lu.%02lu-%03lu%s%s", \
>> FIELD_GET(FBNIC_FW_CAP_RESP_VERSION_MAJOR, __rev_id), \
>> FIELD_GET(FBNIC_FW_CAP_RESP_VERSION_MINOR, __rev_id), \
>> FIELD_GET(FBNIC_FW_CAP_RESP_VERSION_PATCH, __rev_id), \
>> @@ -65,7 +65,7 @@ do { \
>> } while (0)
>>
>> #define fbnic_mk_fw_ver_str(_rev_id, _str) \
>> - fbnic_mk_full_fw_ver_str(_rev_id, "", "", _str)
>> + fbnic_mk_full_fw_ver_str(_rev_id, "", "", _str, sizeof(_str))
>>
>> #define FW_HEARTBEAT_PERIOD (10 * HZ)
>>
>> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
>> index 571374361259..a400616a24d4 100644
>> --- a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
>> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
>> @@ -521,6 +521,8 @@ struct net_device *fbnic_netdev_alloc(struct fbnic_dev *fbd)
>> netdev->netdev_ops = &fbnic_netdev_ops;
>> netdev->stat_ops = &fbnic_stat_ops;
>>
>> + fbnic_set_ethtool_ops(netdev);
>> +
>> fbn = netdev_priv(netdev);
>>
>> fbn->netdev = netdev;
>> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
>> index 60199e634468..6c27da09a612 100644
>> --- a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
>> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
>> @@ -58,6 +58,7 @@ int fbnic_netdev_register(struct net_device *netdev);
>> void fbnic_netdev_unregister(struct net_device *netdev);
>> void fbnic_reset_queues(struct fbnic_net *fbn,
>> unsigned int tx, unsigned int rx);
>> +void fbnic_set_ethtool_ops(struct net_device *dev);
> Isn't it cleaner to have it in sth like fbnic_ethtool.h file? Probably
> you will have more functions there in the future.
>
> Thanks,
> Michal
It would definitely be cleaner if have a large number of functions which
isn't the case hence, did not add fbnic_ethtool.h
>
>>
>> void __fbnic_set_rx_mode(struct net_device *netdev);
>> void fbnic_clear_rx_mode(struct net_device *netdev);
>> --
>> 2.43.5
>>
Powered by blists - more mailing lists