| 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: <1d01ab9b-9d19-ceff-c876-02b18ee20150@amd.com>
Date: Wed, 16 Aug 2023 10:11:03 -0700
From: Brett Creeley <bcreeley@....com>
To: Wenjun Wu <wenjun1.wu@...el.com>, intel-wired-lan@...ts.osuosl.org,
netdev@...r.kernel.org
Cc: xuejun.zhang@...el.com, madhu.chittim@...el.com, qi.z.zhang@...el.com,
anthony.l.nguyen@...el.com
Subject: Re: [PATCH iwl-next v2 3/5] iavf: Add devlink and devlink port
support
On 8/7/2023 6:57 PM, Wenjun Wu wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> From: Jun Zhang <xuejun.zhang@...el.com>
>
> To allow user to configure queue bandwidth, devlink port support
> is added to support devlink port rate API.
>
> Add devlink framework registration/unregistration on iavf driver
> initialization and remove, and devlink port of DEVLINK_PORT_FLAVOUR_VIRTUAL
> is created to be associated iavf net device.
>
> Signed-off-by: Jun Zhang <xuejun.zhang@...el.com>
> ---
> drivers/net/ethernet/intel/Kconfig | 1 +
> drivers/net/ethernet/intel/iavf/Makefile | 2 +-
> drivers/net/ethernet/intel/iavf/iavf.h | 6 ++
> .../net/ethernet/intel/iavf/iavf_devlink.c | 93 +++++++++++++++++++
> .../net/ethernet/intel/iavf/iavf_devlink.h | 17 ++++
> drivers/net/ethernet/intel/iavf/iavf_main.c | 14 +++
> 6 files changed, 132 insertions(+), 1 deletion(-)
> create mode 100644 drivers/net/ethernet/intel/iavf/iavf_devlink.c
> create mode 100644 drivers/net/ethernet/intel/iavf/iavf_devlink.h
>
> diff --git a/drivers/net/ethernet/intel/Kconfig b/drivers/net/ethernet/intel/Kconfig
> index 9bc0a9519899..f916b8ef6acb 100644
> --- a/drivers/net/ethernet/intel/Kconfig
> +++ b/drivers/net/ethernet/intel/Kconfig
> @@ -256,6 +256,7 @@ config I40EVF
> tristate "Intel(R) Ethernet Adaptive Virtual Function support"
> select IAVF
> depends on PCI_MSI
> + select NET_DEVLINK
> help
> This driver supports virtual functions for Intel XL710,
> X710, X722, XXV710, and all devices advertising support for
> diff --git a/drivers/net/ethernet/intel/iavf/Makefile b/drivers/net/ethernet/intel/iavf/Makefile
> index 9c3e45c54d01..b5d7db97ab8b 100644
> --- a/drivers/net/ethernet/intel/iavf/Makefile
> +++ b/drivers/net/ethernet/intel/iavf/Makefile
> @@ -12,5 +12,5 @@ subdir-ccflags-y += -I$(src)
> obj-$(CONFIG_IAVF) += iavf.o
>
> iavf-objs := iavf_main.o iavf_ethtool.o iavf_virtchnl.o iavf_fdir.o \
> - iavf_adv_rss.o \
> + iavf_adv_rss.o iavf_devlink.o \
> iavf_txrx.o iavf_common.o iavf_adminq.o iavf_client.o
> diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h
> index 8cbdebc5b698..519aeaec793c 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf.h
> +++ b/drivers/net/ethernet/intel/iavf/iavf.h
> @@ -33,9 +33,11 @@
> #include <net/udp.h>
> #include <net/tc_act/tc_gact.h>
> #include <net/tc_act/tc_mirred.h>
> +#include <net/devlink.h>
>
> #include "iavf_type.h"
> #include <linux/avf/virtchnl.h>
> +#include "iavf_devlink.h"
> #include "iavf_txrx.h"
> #include "iavf_fdir.h"
> #include "iavf_adv_rss.h"
> @@ -369,6 +371,10 @@ struct iavf_adapter {
> struct net_device *netdev;
> struct pci_dev *pdev;
>
> + /* devlink & port data */
Nit, this comment doesn't add anything to the reader.
> + struct devlink *devlink;
> + struct devlink_port devlink_port;
> +
> struct iavf_hw hw; /* defined in iavf_type.h */
>
> enum iavf_state_t state;
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_devlink.c b/drivers/net/ethernet/intel/iavf/iavf_devlink.c
> new file mode 100644
> index 000000000000..991d041e5922
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/iavf/iavf_devlink.c
> @@ -0,0 +1,93 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/* Copyright (C) 2023 Intel Corporation */
> +
> +#include "iavf.h"
> +#include "iavf_devlink.h"
> +
> +static const struct devlink_ops iavf_devlink_ops = {};
> +
> +/**
> + * iavf_devlink_register - Register allocated devlink instance for iavf adapter
> + * @adapter: the iavf adapter to register the devlink for.
> + *
> + * Register the devlink instance associated with this iavf adapter
Nit, seems like this is a duplicate of what you above.
> + *
> + * Return: zero on success or an error code on failure.
> + */
> +int iavf_devlink_register(struct iavf_adapter *adapter)
> +{
> + struct device *dev = &adapter->pdev->dev;
> + struct iavf_devlink *ref;
> + struct devlink *devlink;
> +
> + /* Allocate devlink instance */
Nit, unnecessary comment.
> + devlink = devlink_alloc(&iavf_devlink_ops, sizeof(struct iavf_devlink),
> + dev);
> + if (!devlink)
> + return -ENOMEM;
> +
> + /* Init iavf adapter devlink */
Nit, unnecessary comment.
> + adapter->devlink = devlink;
> + ref = devlink_priv(devlink);
> + ref->devlink_ref = adapter;
> +
> + devlink_register(devlink);
> +
> + return 0;
> +}
> +
> +/**
> + * iavf_devlink_unregister - Unregister devlink resources for iavf adapter.
> + * @adapter: the iavf adapter structure
> + *
> + * Releases resources used by devlink and cleans up associated memory.
> + */
> +void iavf_devlink_unregister(struct iavf_adapter *adapter)
> +{
> + devlink_unregister(adapter->devlink);
> + devlink_free(adapter->devlink);
Seems like you should bail out if (!adapter->devlink) since allocation
of adapter->devlink can fail in iavf_devlink_register().
> +}
> +
> +/**
> + * iavf_devlink_port_register - Register devlink port for iavf adapter
> + * @adapter: the iavf adapter to register the devlink port for.
> + *
> + * Register the devlink port instance associated with this iavf adapter
> + * before iavf adapter registers with netdevice
> + *
> + * Return: zero on success or an error code on failure.
> + */
> +int iavf_devlink_port_register(struct iavf_adapter *adapter)
> +{
> + struct device *dev = &adapter->pdev->dev;
> + struct devlink_port_attrs attrs = {};
> + int err;
> +
> + /* Create devlink port: attr/port flavour, port index */
Nit, unnecessary comment.
> + SET_NETDEV_DEVLINK_PORT(adapter->netdev, &adapter->devlink_port);
> + attrs.flavour = DEVLINK_PORT_FLAVOUR_VIRTUAL;
> + memset(&adapter->devlink_port, 0, sizeof(adapter->devlink_port));
> + devlink_port_attrs_set(&adapter->devlink_port, &attrs);
> +
> + /* Register with driver specific index (device id) */
> + err = devlink_port_register(adapter->devlink, &adapter->devlink_port,
> + adapter->hw.bus.device);
> + if (err)
> + dev_err(dev, "devlink port registration failed: %d\n", err);
> +
> + return err;
> +}
> +
> +/**
> + * iavf_devlink_port_unregister - Unregister devlink port for iavf adapter.
> + * @adapter: the iavf adapter structure
> + *
> + * Releases resources used by devlink port and registration with devlink.
> + */
> +void iavf_devlink_port_unregister(struct iavf_adapter *adapter)
> +{
> + if (!adapter->devlink_port.registered)
> + return;
> +
> + devlink_port_unregister(&adapter->devlink_port);
> +}
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_devlink.h b/drivers/net/ethernet/intel/iavf/iavf_devlink.h
> new file mode 100644
> index 000000000000..5c122278611a
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/iavf/iavf_devlink.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/* Copyright (C) 2023 Intel Corporation */
> +
> +#ifndef _IAVF_DEVLINK_H_
> +#define _IAVF_DEVLINK_H_
> +
> +/* iavf devlink structure pointing to iavf adapter */
Nit, unnecessary comment.
> +struct iavf_devlink {
> + struct iavf_adapter *devlink_ref; /* ref to iavf adapter */
> +};
> +
> +int iavf_devlink_register(struct iavf_adapter *adapter);
> +void iavf_devlink_unregister(struct iavf_adapter *adapter);
> +int iavf_devlink_port_register(struct iavf_adapter *adapter);
> +void iavf_devlink_port_unregister(struct iavf_adapter *adapter);
> +
> +#endif /* _IAVF_DEVLINK_H_ */
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
> index 7b300c86ceda..db010e68d5d2 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -2037,6 +2037,7 @@ static void iavf_finish_config(struct work_struct *work)
> iavf_free_rss(adapter);
> iavf_free_misc_irq(adapter);
> iavf_reset_interrupt_capability(adapter);
> + iavf_devlink_port_unregister(adapter);
> iavf_change_state(adapter,
> __IAVF_INIT_CONFIG_ADAPTER);
> goto out;
> @@ -2708,6 +2709,9 @@ static void iavf_init_config_adapter(struct iavf_adapter *adapter)
> if (err)
> goto err_sw_init;
>
> + if (!adapter->netdev_registered)
> + iavf_devlink_port_register(adapter);
> +
> netif_carrier_off(netdev);
> adapter->link_up = false;
> netif_tx_stop_all_queues(netdev);
> @@ -2749,6 +2753,7 @@ static void iavf_init_config_adapter(struct iavf_adapter *adapter)
> err_mem:
> iavf_free_rss(adapter);
> iavf_free_misc_irq(adapter);
> + iavf_devlink_port_unregister(adapter);
> err_sw_init:
> iavf_reset_interrupt_capability(adapter);
> err:
> @@ -4995,6 +5000,12 @@ static int iavf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> /* Setup the wait queue for indicating virtchannel events */
> init_waitqueue_head(&adapter->vc_waitqueue);
>
> + /* Register iavf adapter with devlink */
> + err = iavf_devlink_register(adapter);
> + if (err)
> + dev_err(&pdev->dev, "devlink registration failed: %d\n", err);
> +
> + /* Keep driver interface even on devlink registration failure */
> return 0;
>
> err_ioremap:
> @@ -5139,6 +5150,9 @@ static void iavf_remove(struct pci_dev *pdev)
> err);
> }
>
> + iavf_devlink_port_unregister(adapter);
> + iavf_devlink_unregister(adapter);
> +
> mutex_lock(&adapter->crit_lock);
> dev_info(&adapter->pdev->dev, "Removing device\n");
> iavf_change_state(adapter, __IAVF_REMOVE);
> --
> 2.34.1
>
>
Powered by blists - more mailing lists