* [PATCH net-next 1/1] iavf: delete the iavf client interface
2023-10-23 23:08 [PATCH net-next 0/9] Intel Wired LAN Driver Updates for 2023-10-23 (iavf) Jacob Keller
@ 2023-10-23 23:08 ` Jacob Keller
2023-10-23 23:08 ` [PATCH net-next 1/9] iavf: fix comments about old bit locks Jacob Keller
` (9 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Jacob Keller @ 2023-10-23 23:08 UTC (permalink / raw)
To: netdev, David Miller, Jakub Kicinski; +Cc: Michal Schmidt, Jacob Keller
From: Michal Schmidt <mschmidt@redhat.com>
The iavf client interface was added in 2017 by commit ed0e894de7c1
("i40evf: add client interface"), but there have never been any in-tree
callers.
It's not useful for future development either. The Intel out-of-tree
iavf and irdma drivers instead use an auxiliary bus, which is a better
solution.
Remove the iavf client interface code. Also gone are the client_task
work and the client_lock mutex.
Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
drivers/net/ethernet/intel/iavf/Makefile | 2 +-
drivers/net/ethernet/intel/iavf/iavf.h | 27 -
drivers/net/ethernet/intel/iavf/iavf_client.c | 578 ------------------
drivers/net/ethernet/intel/iavf/iavf_client.h | 169 -----
drivers/net/ethernet/intel/iavf/iavf_main.c | 82 ---
.../net/ethernet/intel/iavf/iavf_virtchnl.c | 14 -
6 files changed, 1 insertion(+), 871 deletions(-)
delete mode 100644 drivers/net/ethernet/intel/iavf/iavf_client.c
delete mode 100644 drivers/net/ethernet/intel/iavf/iavf_client.h
diff --git a/drivers/net/ethernet/intel/iavf/Makefile b/drivers/net/ethernet/intel/iavf/Makefile
index 9c3e45c54d01..2d154a4e2fd7 100644
--- a/drivers/net/ethernet/intel/iavf/Makefile
+++ b/drivers/net/ethernet/intel/iavf/Makefile
@@ -13,4 +13,4 @@ obj-$(CONFIG_IAVF) += iavf.o
iavf-objs := iavf_main.o iavf_ethtool.o iavf_virtchnl.o iavf_fdir.o \
iavf_adv_rss.o \
- iavf_txrx.o iavf_common.o iavf_adminq.o iavf_client.o
+ iavf_txrx.o iavf_common.o iavf_adminq.o
diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h
index f026d0670338..e7ab89dc883a 100644
--- a/drivers/net/ethernet/intel/iavf/iavf.h
+++ b/drivers/net/ethernet/intel/iavf/iavf.h
@@ -63,7 +63,6 @@ struct iavf_vsi {
DECLARE_BITMAP(state, __IAVF_VSI_STATE_SIZE__);
int base_vector;
u16 qs_handle;
- void *priv; /* client driver data reference. */
};
/* How many Rx Buffers do we bundle into one write to the hardware ? */
@@ -256,7 +255,6 @@ struct iavf_adapter {
struct work_struct reset_task;
struct work_struct adminq_task;
struct work_struct finish_config;
- struct delayed_work client_task;
wait_queue_head_t down_waitqueue;
wait_queue_head_t reset_waitqueue;
wait_queue_head_t vc_waitqueue;
@@ -265,7 +263,6 @@ struct iavf_adapter {
int num_vlan_filters;
struct list_head mac_filter_list;
struct mutex crit_lock;
- struct mutex client_lock;
/* Lock to protect accesses to MAC and VLAN lists */
spinlock_t mac_vlan_list_lock;
char misc_vector_name[IFNAMSIZ + 9];
@@ -282,10 +279,6 @@ struct iavf_adapter {
u64 hw_csum_rx_error;
u32 rx_desc_count;
int num_msix_vectors;
- int num_rdma_msix;
- int rdma_base_vector;
- u32 client_pending;
- struct iavf_client_instance *cinst;
struct msix_entry *msix_entries;
u32 flags;
@@ -294,10 +287,6 @@ struct iavf_adapter {
#define IAVF_FLAG_RESET_PENDING BIT(4)
#define IAVF_FLAG_RESET_NEEDED BIT(5)
#define IAVF_FLAG_WB_ON_ITR_CAPABLE BIT(6)
-#define IAVF_FLAG_SERVICE_CLIENT_REQUESTED BIT(9)
-#define IAVF_FLAG_CLIENT_NEEDS_OPEN BIT(10)
-#define IAVF_FLAG_CLIENT_NEEDS_CLOSE BIT(11)
-#define IAVF_FLAG_CLIENT_NEEDS_L2_PARAMS BIT(12)
#define IAVF_FLAG_LEGACY_RX BIT(15)
#define IAVF_FLAG_REINIT_ITR_NEEDED BIT(16)
#define IAVF_FLAG_QUEUES_DISABLED BIT(17)
@@ -388,11 +377,6 @@ struct iavf_adapter {
u32 link_speed_mbps;
enum virtchnl_ops current_op;
-#define CLIENT_ALLOWED(_a) ((_a)->vf_res ? \
- (_a)->vf_res->vf_cap_flags & \
- VIRTCHNL_VF_OFFLOAD_RDMA : \
- 0)
-#define CLIENT_ENABLED(_a) ((_a)->cinst)
/* RSS by the PF should be preferred over RSS via other methods. */
#define RSS_PF(_a) ((_a)->vf_res->vf_cap_flags & \
VIRTCHNL_VF_OFFLOAD_RSS_PF)
@@ -460,12 +444,6 @@ struct iavf_adapter {
/* Ethtool Private Flags */
-/* lan device, used by client interface */
-struct iavf_device {
- struct list_head list;
- struct iavf_adapter *vf;
-};
-
/* needed by iavf_ethtool.c */
extern char iavf_driver_name[];
@@ -569,11 +547,6 @@ void iavf_virtchnl_completion(struct iavf_adapter *adapter,
int iavf_config_rss(struct iavf_adapter *adapter);
int iavf_lan_add_device(struct iavf_adapter *adapter);
int iavf_lan_del_device(struct iavf_adapter *adapter);
-void iavf_client_subtask(struct iavf_adapter *adapter);
-void iavf_notify_client_message(struct iavf_vsi *vsi, u8 *msg, u16 len);
-void iavf_notify_client_l2_params(struct iavf_vsi *vsi);
-void iavf_notify_client_open(struct iavf_vsi *vsi);
-void iavf_notify_client_close(struct iavf_vsi *vsi, bool reset);
void iavf_enable_channels(struct iavf_adapter *adapter);
void iavf_disable_channels(struct iavf_adapter *adapter);
void iavf_add_cloud_filter(struct iavf_adapter *adapter);
diff --git a/drivers/net/ethernet/intel/iavf/iavf_client.c b/drivers/net/ethernet/intel/iavf/iavf_client.c
deleted file mode 100644
index e6051b6355aa..000000000000
--- a/drivers/net/ethernet/intel/iavf/iavf_client.c
+++ /dev/null
@@ -1,578 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/* Copyright(c) 2013 - 2018 Intel Corporation. */
-
-#include <linux/list.h>
-#include <linux/errno.h>
-
-#include "iavf.h"
-#include "iavf_prototype.h"
-#include "iavf_client.h"
-
-static
-const char iavf_client_interface_version_str[] = IAVF_CLIENT_VERSION_STR;
-static struct iavf_client *vf_registered_client;
-static LIST_HEAD(iavf_devices);
-static DEFINE_MUTEX(iavf_device_mutex);
-
-static u32 iavf_client_virtchnl_send(struct iavf_info *ldev,
- struct iavf_client *client,
- u8 *msg, u16 len);
-
-static int iavf_client_setup_qvlist(struct iavf_info *ldev,
- struct iavf_client *client,
- struct iavf_qvlist_info *qvlist_info);
-
-static struct iavf_ops iavf_lan_ops = {
- .virtchnl_send = iavf_client_virtchnl_send,
- .setup_qvlist = iavf_client_setup_qvlist,
-};
-
-/**
- * iavf_client_get_params - retrieve relevant client parameters
- * @vsi: VSI with parameters
- * @params: client param struct
- **/
-static
-void iavf_client_get_params(struct iavf_vsi *vsi, struct iavf_params *params)
-{
- int i;
-
- memset(params, 0, sizeof(struct iavf_params));
- params->mtu = vsi->netdev->mtu;
- params->link_up = vsi->back->link_up;
-
- for (i = 0; i < IAVF_MAX_USER_PRIORITY; i++) {
- params->qos.prio_qos[i].tc = 0;
- params->qos.prio_qos[i].qs_handle = vsi->qs_handle;
- }
-}
-
-/**
- * iavf_notify_client_message - call the client message receive callback
- * @vsi: the VSI associated with this client
- * @msg: message buffer
- * @len: length of message
- *
- * If there is a client to this VSI, call the client
- **/
-void iavf_notify_client_message(struct iavf_vsi *vsi, u8 *msg, u16 len)
-{
- struct iavf_client_instance *cinst;
-
- if (!vsi)
- return;
-
- cinst = vsi->back->cinst;
- if (!cinst || !cinst->client || !cinst->client->ops ||
- !cinst->client->ops->virtchnl_receive) {
- dev_dbg(&vsi->back->pdev->dev,
- "Cannot locate client instance virtchnl_receive function\n");
- return;
- }
- cinst->client->ops->virtchnl_receive(&cinst->lan_info, cinst->client,
- msg, len);
-}
-
-/**
- * iavf_notify_client_l2_params - call the client notify callback
- * @vsi: the VSI with l2 param changes
- *
- * If there is a client to this VSI, call the client
- **/
-void iavf_notify_client_l2_params(struct iavf_vsi *vsi)
-{
- struct iavf_client_instance *cinst;
- struct iavf_params params;
-
- if (!vsi)
- return;
-
- cinst = vsi->back->cinst;
-
- if (!cinst || !cinst->client || !cinst->client->ops ||
- !cinst->client->ops->l2_param_change) {
- dev_dbg(&vsi->back->pdev->dev,
- "Cannot locate client instance l2_param_change function\n");
- return;
- }
- iavf_client_get_params(vsi, ¶ms);
- cinst->lan_info.params = params;
- cinst->client->ops->l2_param_change(&cinst->lan_info, cinst->client,
- ¶ms);
-}
-
-/**
- * iavf_notify_client_open - call the client open callback
- * @vsi: the VSI with netdev opened
- *
- * If there is a client to this netdev, call the client with open
- **/
-void iavf_notify_client_open(struct iavf_vsi *vsi)
-{
- struct iavf_adapter *adapter = vsi->back;
- struct iavf_client_instance *cinst = adapter->cinst;
- int ret;
-
- if (!cinst || !cinst->client || !cinst->client->ops ||
- !cinst->client->ops->open) {
- dev_dbg(&vsi->back->pdev->dev,
- "Cannot locate client instance open function\n");
- return;
- }
- if (!(test_bit(__IAVF_CLIENT_INSTANCE_OPENED, &cinst->state))) {
- ret = cinst->client->ops->open(&cinst->lan_info, cinst->client);
- if (!ret)
- set_bit(__IAVF_CLIENT_INSTANCE_OPENED, &cinst->state);
- }
-}
-
-/**
- * iavf_client_release_qvlist - send a message to the PF to release rdma qv map
- * @ldev: pointer to L2 context.
- *
- * Return 0 on success or < 0 on error
- **/
-static int iavf_client_release_qvlist(struct iavf_info *ldev)
-{
- struct iavf_adapter *adapter = ldev->vf;
- enum iavf_status err;
-
- if (adapter->aq_required)
- return -EAGAIN;
-
- err = iavf_aq_send_msg_to_pf(&adapter->hw,
- VIRTCHNL_OP_RELEASE_RDMA_IRQ_MAP,
- IAVF_SUCCESS, NULL, 0, NULL);
-
- if (err)
- dev_err(&adapter->pdev->dev,
- "Unable to send RDMA vector release message to PF, error %d, aq status %d\n",
- err, adapter->hw.aq.asq_last_status);
-
- return err;
-}
-
-/**
- * iavf_notify_client_close - call the client close callback
- * @vsi: the VSI with netdev closed
- * @reset: true when close called due to reset pending
- *
- * If there is a client to this netdev, call the client with close
- **/
-void iavf_notify_client_close(struct iavf_vsi *vsi, bool reset)
-{
- struct iavf_adapter *adapter = vsi->back;
- struct iavf_client_instance *cinst = adapter->cinst;
-
- if (!cinst || !cinst->client || !cinst->client->ops ||
- !cinst->client->ops->close) {
- dev_dbg(&vsi->back->pdev->dev,
- "Cannot locate client instance close function\n");
- return;
- }
- cinst->client->ops->close(&cinst->lan_info, cinst->client, reset);
- iavf_client_release_qvlist(&cinst->lan_info);
- clear_bit(__IAVF_CLIENT_INSTANCE_OPENED, &cinst->state);
-}
-
-/**
- * iavf_client_add_instance - add a client instance to the instance list
- * @adapter: pointer to the board struct
- *
- * Returns cinst ptr on success, NULL on failure
- **/
-static struct iavf_client_instance *
-iavf_client_add_instance(struct iavf_adapter *adapter)
-{
- struct iavf_client_instance *cinst = NULL;
- struct iavf_vsi *vsi = &adapter->vsi;
- struct netdev_hw_addr *mac = NULL;
- struct iavf_params params;
-
- if (!vf_registered_client)
- goto out;
-
- if (adapter->cinst) {
- cinst = adapter->cinst;
- goto out;
- }
-
- cinst = kzalloc(sizeof(*cinst), GFP_KERNEL);
- if (!cinst)
- goto out;
-
- cinst->lan_info.vf = (void *)adapter;
- cinst->lan_info.netdev = vsi->netdev;
- cinst->lan_info.pcidev = adapter->pdev;
- cinst->lan_info.fid = 0;
- cinst->lan_info.ftype = IAVF_CLIENT_FTYPE_VF;
- cinst->lan_info.hw_addr = adapter->hw.hw_addr;
- cinst->lan_info.ops = &iavf_lan_ops;
- cinst->lan_info.version.major = IAVF_CLIENT_VERSION_MAJOR;
- cinst->lan_info.version.minor = IAVF_CLIENT_VERSION_MINOR;
- cinst->lan_info.version.build = IAVF_CLIENT_VERSION_BUILD;
- iavf_client_get_params(vsi, ¶ms);
- cinst->lan_info.params = params;
- set_bit(__IAVF_CLIENT_INSTANCE_NONE, &cinst->state);
-
- cinst->lan_info.msix_count = adapter->num_rdma_msix;
- cinst->lan_info.msix_entries =
- &adapter->msix_entries[adapter->rdma_base_vector];
-
- mac = list_first_entry(&cinst->lan_info.netdev->dev_addrs.list,
- struct netdev_hw_addr, list);
- if (mac)
- ether_addr_copy(cinst->lan_info.lanmac, mac->addr);
- else
- dev_err(&adapter->pdev->dev, "MAC address list is empty!\n");
-
- cinst->client = vf_registered_client;
- adapter->cinst = cinst;
-out:
- return cinst;
-}
-
-/**
- * iavf_client_del_instance - removes a client instance from the list
- * @adapter: pointer to the board struct
- *
- **/
-static
-void iavf_client_del_instance(struct iavf_adapter *adapter)
-{
- kfree(adapter->cinst);
- adapter->cinst = NULL;
-}
-
-/**
- * iavf_client_subtask - client maintenance work
- * @adapter: board private structure
- **/
-void iavf_client_subtask(struct iavf_adapter *adapter)
-{
- struct iavf_client *client = vf_registered_client;
- struct iavf_client_instance *cinst;
- int ret = 0;
-
- if (adapter->state < __IAVF_DOWN)
- return;
-
- /* first check client is registered */
- if (!client)
- return;
-
- /* Add the client instance to the instance list */
- cinst = iavf_client_add_instance(adapter);
- if (!cinst)
- return;
-
- dev_info(&adapter->pdev->dev, "Added instance of Client %s\n",
- client->name);
-
- if (!test_bit(__IAVF_CLIENT_INSTANCE_OPENED, &cinst->state)) {
- /* Send an Open request to the client */
-
- if (client->ops && client->ops->open)
- ret = client->ops->open(&cinst->lan_info, client);
- if (!ret)
- set_bit(__IAVF_CLIENT_INSTANCE_OPENED,
- &cinst->state);
- else
- /* remove client instance */
- iavf_client_del_instance(adapter);
- }
-}
-
-/**
- * iavf_lan_add_device - add a lan device struct to the list of lan devices
- * @adapter: pointer to the board struct
- *
- * Returns 0 on success or none 0 on error
- **/
-int iavf_lan_add_device(struct iavf_adapter *adapter)
-{
- struct iavf_device *ldev;
- int ret = 0;
-
- mutex_lock(&iavf_device_mutex);
- list_for_each_entry(ldev, &iavf_devices, list) {
- if (ldev->vf == adapter) {
- ret = -EEXIST;
- goto out;
- }
- }
- ldev = kzalloc(sizeof(*ldev), GFP_KERNEL);
- if (!ldev) {
- ret = -ENOMEM;
- goto out;
- }
- ldev->vf = adapter;
- INIT_LIST_HEAD(&ldev->list);
- list_add(&ldev->list, &iavf_devices);
- dev_info(&adapter->pdev->dev, "Added LAN device bus=0x%02x dev=0x%02x func=0x%02x\n",
- adapter->hw.bus.bus_id, adapter->hw.bus.device,
- adapter->hw.bus.func);
-
- /* Since in some cases register may have happened before a device gets
- * added, we can schedule a subtask to go initiate the clients.
- */
- adapter->flags |= IAVF_FLAG_SERVICE_CLIENT_REQUESTED;
-
-out:
- mutex_unlock(&iavf_device_mutex);
- return ret;
-}
-
-/**
- * iavf_lan_del_device - removes a lan device from the device list
- * @adapter: pointer to the board struct
- *
- * Returns 0 on success or non-0 on error
- **/
-int iavf_lan_del_device(struct iavf_adapter *adapter)
-{
- struct iavf_device *ldev, *tmp;
- int ret = -ENODEV;
-
- mutex_lock(&iavf_device_mutex);
- list_for_each_entry_safe(ldev, tmp, &iavf_devices, list) {
- if (ldev->vf == adapter) {
- dev_info(&adapter->pdev->dev,
- "Deleted LAN device bus=0x%02x dev=0x%02x func=0x%02x\n",
- adapter->hw.bus.bus_id, adapter->hw.bus.device,
- adapter->hw.bus.func);
- list_del(&ldev->list);
- kfree(ldev);
- ret = 0;
- break;
- }
- }
-
- mutex_unlock(&iavf_device_mutex);
- return ret;
-}
-
-/**
- * iavf_client_release - release client specific resources
- * @client: pointer to the registered client
- *
- **/
-static void iavf_client_release(struct iavf_client *client)
-{
- struct iavf_client_instance *cinst;
- struct iavf_device *ldev;
- struct iavf_adapter *adapter;
-
- mutex_lock(&iavf_device_mutex);
- list_for_each_entry(ldev, &iavf_devices, list) {
- adapter = ldev->vf;
- cinst = adapter->cinst;
- if (!cinst)
- continue;
- if (test_bit(__IAVF_CLIENT_INSTANCE_OPENED, &cinst->state)) {
- if (client->ops && client->ops->close)
- client->ops->close(&cinst->lan_info, client,
- false);
- iavf_client_release_qvlist(&cinst->lan_info);
- clear_bit(__IAVF_CLIENT_INSTANCE_OPENED, &cinst->state);
-
- dev_warn(&adapter->pdev->dev,
- "Client %s instance closed\n", client->name);
- }
- /* delete the client instance */
- iavf_client_del_instance(adapter);
- dev_info(&adapter->pdev->dev, "Deleted client instance of Client %s\n",
- client->name);
- }
- mutex_unlock(&iavf_device_mutex);
-}
-
-/**
- * iavf_client_prepare - prepare client specific resources
- * @client: pointer to the registered client
- *
- **/
-static void iavf_client_prepare(struct iavf_client *client)
-{
- struct iavf_device *ldev;
- struct iavf_adapter *adapter;
-
- mutex_lock(&iavf_device_mutex);
- list_for_each_entry(ldev, &iavf_devices, list) {
- adapter = ldev->vf;
- /* Signal the watchdog to service the client */
- adapter->flags |= IAVF_FLAG_SERVICE_CLIENT_REQUESTED;
- }
- mutex_unlock(&iavf_device_mutex);
-}
-
-/**
- * iavf_client_virtchnl_send - send a message to the PF instance
- * @ldev: pointer to L2 context.
- * @client: Client pointer.
- * @msg: pointer to message buffer
- * @len: message length
- *
- * Return 0 on success or < 0 on error
- **/
-static u32 iavf_client_virtchnl_send(struct iavf_info *ldev,
- struct iavf_client *client,
- u8 *msg, u16 len)
-{
- struct iavf_adapter *adapter = ldev->vf;
- enum iavf_status err;
-
- if (adapter->aq_required)
- return -EAGAIN;
-
- err = iavf_aq_send_msg_to_pf(&adapter->hw, VIRTCHNL_OP_RDMA,
- IAVF_SUCCESS, msg, len, NULL);
- if (err)
- dev_err(&adapter->pdev->dev, "Unable to send RDMA message to PF, error %d, aq status %d\n",
- err, adapter->hw.aq.asq_last_status);
-
- return err;
-}
-
-/**
- * iavf_client_setup_qvlist - send a message to the PF to setup rdma qv map
- * @ldev: pointer to L2 context.
- * @client: Client pointer.
- * @qvlist_info: queue and vector list
- *
- * Return 0 on success or < 0 on error
- **/
-static int iavf_client_setup_qvlist(struct iavf_info *ldev,
- struct iavf_client *client,
- struct iavf_qvlist_info *qvlist_info)
-{
- struct virtchnl_rdma_qvlist_info *v_qvlist_info;
- struct iavf_adapter *adapter = ldev->vf;
- struct iavf_qv_info *qv_info;
- enum iavf_status err;
- u32 v_idx, i;
- size_t msg_size;
-
- if (adapter->aq_required)
- return -EAGAIN;
-
- /* A quick check on whether the vectors belong to the client */
- for (i = 0; i < qvlist_info->num_vectors; i++) {
- qv_info = &qvlist_info->qv_info[i];
- if (!qv_info)
- continue;
- v_idx = qv_info->v_idx;
- if ((v_idx >=
- (adapter->rdma_base_vector + adapter->num_rdma_msix)) ||
- (v_idx < adapter->rdma_base_vector))
- return -EINVAL;
- }
-
- v_qvlist_info = (struct virtchnl_rdma_qvlist_info *)qvlist_info;
- msg_size = virtchnl_struct_size(v_qvlist_info, qv_info,
- v_qvlist_info->num_vectors);
-
- adapter->client_pending |= BIT(VIRTCHNL_OP_CONFIG_RDMA_IRQ_MAP);
- err = iavf_aq_send_msg_to_pf(&adapter->hw,
- VIRTCHNL_OP_CONFIG_RDMA_IRQ_MAP, IAVF_SUCCESS,
- (u8 *)v_qvlist_info, msg_size, NULL);
-
- if (err) {
- dev_err(&adapter->pdev->dev,
- "Unable to send RDMA vector config message to PF, error %d, aq status %d\n",
- err, adapter->hw.aq.asq_last_status);
- goto out;
- }
-
- err = -EBUSY;
- for (i = 0; i < 5; i++) {
- msleep(100);
- if (!(adapter->client_pending &
- BIT(VIRTCHNL_OP_CONFIG_RDMA_IRQ_MAP))) {
- err = 0;
- break;
- }
- }
-out:
- return err;
-}
-
-/**
- * iavf_register_client - Register a iavf client driver with the L2 driver
- * @client: pointer to the iavf_client struct
- *
- * Returns 0 on success or non-0 on error
- **/
-int iavf_register_client(struct iavf_client *client)
-{
- int ret = 0;
-
- if (!client) {
- ret = -EIO;
- goto out;
- }
-
- if (strlen(client->name) == 0) {
- pr_info("iavf: Failed to register client with no name\n");
- ret = -EIO;
- goto out;
- }
-
- if (vf_registered_client) {
- pr_info("iavf: Client %s has already been registered!\n",
- client->name);
- ret = -EEXIST;
- goto out;
- }
-
- if ((client->version.major != IAVF_CLIENT_VERSION_MAJOR) ||
- (client->version.minor != IAVF_CLIENT_VERSION_MINOR)) {
- pr_info("iavf: Failed to register client %s due to mismatched client interface version\n",
- client->name);
- pr_info("Client is using version: %02d.%02d.%02d while LAN driver supports %s\n",
- client->version.major, client->version.minor,
- client->version.build,
- iavf_client_interface_version_str);
- ret = -EIO;
- goto out;
- }
-
- vf_registered_client = client;
-
- iavf_client_prepare(client);
-
- pr_info("iavf: Registered client %s with return code %d\n",
- client->name, ret);
-out:
- return ret;
-}
-EXPORT_SYMBOL(iavf_register_client);
-
-/**
- * iavf_unregister_client - Unregister a iavf client driver with the L2 driver
- * @client: pointer to the iavf_client struct
- *
- * Returns 0 on success or non-0 on error
- **/
-int iavf_unregister_client(struct iavf_client *client)
-{
- int ret = 0;
-
- /* When a unregister request comes through we would have to send
- * a close for each of the client instances that were opened.
- * client_release function is called to handle this.
- */
- iavf_client_release(client);
-
- if (vf_registered_client != client) {
- pr_info("iavf: Client %s has not been registered\n",
- client->name);
- ret = -ENODEV;
- goto out;
- }
- vf_registered_client = NULL;
- pr_info("iavf: Unregistered client %s\n", client->name);
-out:
- return ret;
-}
-EXPORT_SYMBOL(iavf_unregister_client);
diff --git a/drivers/net/ethernet/intel/iavf/iavf_client.h b/drivers/net/ethernet/intel/iavf/iavf_client.h
deleted file mode 100644
index 500269bc0f5b..000000000000
--- a/drivers/net/ethernet/intel/iavf/iavf_client.h
+++ /dev/null
@@ -1,169 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/* Copyright(c) 2013 - 2018 Intel Corporation. */
-
-#ifndef _IAVF_CLIENT_H_
-#define _IAVF_CLIENT_H_
-
-#define IAVF_CLIENT_STR_LENGTH 10
-
-/* Client interface version should be updated anytime there is a change in the
- * existing APIs or data structures.
- */
-#define IAVF_CLIENT_VERSION_MAJOR 0
-#define IAVF_CLIENT_VERSION_MINOR 01
-#define IAVF_CLIENT_VERSION_BUILD 00
-#define IAVF_CLIENT_VERSION_STR \
- __stringify(IAVF_CLIENT_VERSION_MAJOR) "." \
- __stringify(IAVF_CLIENT_VERSION_MINOR) "." \
- __stringify(IAVF_CLIENT_VERSION_BUILD)
-
-struct iavf_client_version {
- u8 major;
- u8 minor;
- u8 build;
- u8 rsvd;
-};
-
-enum iavf_client_state {
- __IAVF_CLIENT_NULL,
- __IAVF_CLIENT_REGISTERED
-};
-
-enum iavf_client_instance_state {
- __IAVF_CLIENT_INSTANCE_NONE,
- __IAVF_CLIENT_INSTANCE_OPENED,
-};
-
-struct iavf_ops;
-struct iavf_client;
-
-/* HW does not define a type value for AEQ; only for RX/TX and CEQ.
- * In order for us to keep the interface simple, SW will define a
- * unique type value for AEQ.
- */
-#define IAVF_QUEUE_TYPE_PE_AEQ 0x80
-#define IAVF_QUEUE_INVALID_IDX 0xFFFF
-
-struct iavf_qv_info {
- u32 v_idx; /* msix_vector */
- u16 ceq_idx;
- u16 aeq_idx;
- u8 itr_idx;
-};
-
-struct iavf_qvlist_info {
- u32 num_vectors;
- struct iavf_qv_info qv_info[];
-};
-
-#define IAVF_CLIENT_MSIX_ALL 0xFFFFFFFF
-
-/* set of LAN parameters useful for clients managed by LAN */
-
-/* Struct to hold per priority info */
-struct iavf_prio_qos_params {
- u16 qs_handle; /* qs handle for prio */
- u8 tc; /* TC mapped to prio */
- u8 reserved;
-};
-
-#define IAVF_CLIENT_MAX_USER_PRIORITY 8
-/* Struct to hold Client QoS */
-struct iavf_qos_params {
- struct iavf_prio_qos_params prio_qos[IAVF_CLIENT_MAX_USER_PRIORITY];
-};
-
-struct iavf_params {
- struct iavf_qos_params qos;
- u16 mtu;
- u16 link_up; /* boolean */
-};
-
-/* Structure to hold LAN device info for a client device */
-struct iavf_info {
- struct iavf_client_version version;
- u8 lanmac[6];
- struct net_device *netdev;
- struct pci_dev *pcidev;
- u8 __iomem *hw_addr;
- u8 fid; /* function id, PF id or VF id */
-#define IAVF_CLIENT_FTYPE_PF 0
-#define IAVF_CLIENT_FTYPE_VF 1
- u8 ftype; /* function type, PF or VF */
- void *vf; /* cast to iavf_adapter */
-
- /* All L2 params that could change during the life span of the device
- * and needs to be communicated to the client when they change
- */
- struct iavf_params params;
- struct iavf_ops *ops;
-
- u16 msix_count; /* number of msix vectors*/
- /* Array down below will be dynamically allocated based on msix_count */
- struct msix_entry *msix_entries;
- u16 itr_index; /* Which ITR index the PE driver is suppose to use */
-};
-
-struct iavf_ops {
- /* setup_q_vector_list enables queues with a particular vector */
- int (*setup_qvlist)(struct iavf_info *ldev, struct iavf_client *client,
- struct iavf_qvlist_info *qv_info);
-
- u32 (*virtchnl_send)(struct iavf_info *ldev, struct iavf_client *client,
- u8 *msg, u16 len);
-
- /* If the PE Engine is unresponsive, RDMA driver can request a reset.*/
- void (*request_reset)(struct iavf_info *ldev,
- struct iavf_client *client);
-};
-
-struct iavf_client_ops {
- /* Should be called from register_client() or whenever the driver is
- * ready to create a specific client instance.
- */
- int (*open)(struct iavf_info *ldev, struct iavf_client *client);
-
- /* Should be closed when netdev is unavailable or when unregister
- * call comes in. If the close happens due to a reset, set the reset
- * bit to true.
- */
- void (*close)(struct iavf_info *ldev, struct iavf_client *client,
- bool reset);
-
- /* called when some l2 managed parameters changes - mss */
- void (*l2_param_change)(struct iavf_info *ldev,
- struct iavf_client *client,
- struct iavf_params *params);
-
- /* called when a message is received from the PF */
- int (*virtchnl_receive)(struct iavf_info *ldev,
- struct iavf_client *client,
- u8 *msg, u16 len);
-};
-
-/* Client device */
-struct iavf_client_instance {
- struct list_head list;
- struct iavf_info lan_info;
- struct iavf_client *client;
- unsigned long state;
-};
-
-struct iavf_client {
- struct list_head list; /* list of registered clients */
- char name[IAVF_CLIENT_STR_LENGTH];
- struct iavf_client_version version;
- unsigned long state; /* client state */
- atomic_t ref_cnt; /* Count of all the client devices of this kind */
- u32 flags;
-#define IAVF_CLIENT_FLAGS_LAUNCH_ON_PROBE BIT(0)
-#define IAVF_TX_FLAGS_NOTIFY_OTHER_EVENTS BIT(2)
- u8 type;
-#define IAVF_CLIENT_RDMA 0
- struct iavf_client_ops *ops; /* client ops provided by the client */
-};
-
-/* used by clients */
-int iavf_register_client(struct iavf_client *client);
-int iavf_unregister_client(struct iavf_client *client);
-#endif /* _IAVF_CLIENT_H_ */
diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index 49a764ca5e36..6e27b7938b8a 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -3,7 +3,6 @@
#include "iavf.h"
#include "iavf_prototype.h"
-#include "iavf_client.h"
/* All iavf tracepoints are defined by the include below, which must
* be included exactly once across the whole kernel with
* CREATE_TRACE_POINTS defined
@@ -1286,8 +1285,6 @@ static void iavf_up_complete(struct iavf_adapter *adapter)
iavf_napi_enable_all(adapter);
adapter->aq_required |= IAVF_FLAG_AQ_ENABLE_QUEUES;
- if (CLIENT_ENABLED(adapter))
- adapter->flags |= IAVF_FLAG_CLIENT_NEEDS_OPEN;
mod_delayed_work(adapter->wq, &adapter->watchdog_task, 0);
}
@@ -2706,12 +2703,6 @@ static void iavf_init_config_adapter(struct iavf_adapter *adapter)
adapter->link_up = false;
netif_tx_stop_all_queues(netdev);
- if (CLIENT_ALLOWED(adapter)) {
- err = iavf_lan_add_device(adapter);
- if (err)
- dev_info(&pdev->dev, "Failed to add VF to client API service list: %d\n",
- err);
- }
dev_info(&pdev->dev, "MAC address: %pM\n", adapter->hw.mac.addr);
if (netdev->features & NETIF_F_GRO)
dev_info(&pdev->dev, "GRO is enabled\n");
@@ -2908,7 +2899,6 @@ static void iavf_watchdog_task(struct work_struct *work)
return;
}
- schedule_delayed_work(&adapter->client_task, msecs_to_jiffies(5));
mutex_unlock(&adapter->crit_lock);
restart_watchdog:
if (adapter->state >= __IAVF_DOWN)
@@ -3019,15 +3009,6 @@ static void iavf_reset_task(struct work_struct *work)
return;
}
- mutex_lock(&adapter->client_lock);
- if (CLIENT_ENABLED(adapter)) {
- adapter->flags &= ~(IAVF_FLAG_CLIENT_NEEDS_OPEN |
- IAVF_FLAG_CLIENT_NEEDS_CLOSE |
- IAVF_FLAG_CLIENT_NEEDS_L2_PARAMS |
- IAVF_FLAG_SERVICE_CLIENT_REQUESTED);
- cancel_delayed_work_sync(&adapter->client_task);
- iavf_notify_client_close(&adapter->vsi, true);
- }
iavf_misc_irq_disable(adapter);
if (adapter->flags & IAVF_FLAG_RESET_NEEDED) {
adapter->flags &= ~IAVF_FLAG_RESET_NEEDED;
@@ -3071,7 +3052,6 @@ static void iavf_reset_task(struct work_struct *work)
dev_err(&adapter->pdev->dev, "Reset never finished (%x)\n",
reg_val);
iavf_disable_vf(adapter);
- mutex_unlock(&adapter->client_lock);
mutex_unlock(&adapter->crit_lock);
return; /* Do not attempt to reinit. It's dead, Jim. */
}
@@ -3210,7 +3190,6 @@ static void iavf_reset_task(struct work_struct *work)
adapter->flags &= ~IAVF_FLAG_REINIT_ITR_NEEDED;
wake_up(&adapter->reset_waitqueue);
- mutex_unlock(&adapter->client_lock);
mutex_unlock(&adapter->crit_lock);
return;
@@ -3221,7 +3200,6 @@ static void iavf_reset_task(struct work_struct *work)
}
iavf_disable_vf(adapter);
- mutex_unlock(&adapter->client_lock);
mutex_unlock(&adapter->crit_lock);
dev_err(&adapter->pdev->dev, "failed to allocate resources during reinit\n");
}
@@ -3320,48 +3298,6 @@ static void iavf_adminq_task(struct work_struct *work)
iavf_misc_irq_enable(adapter);
}
-/**
- * iavf_client_task - worker thread to perform client work
- * @work: pointer to work_struct containing our data
- *
- * This task handles client interactions. Because client calls can be
- * reentrant, we can't handle them in the watchdog.
- **/
-static void iavf_client_task(struct work_struct *work)
-{
- struct iavf_adapter *adapter =
- container_of(work, struct iavf_adapter, client_task.work);
-
- /* If we can't get the client bit, just give up. We'll be rescheduled
- * later.
- */
-
- if (!mutex_trylock(&adapter->client_lock))
- return;
-
- if (adapter->flags & IAVF_FLAG_SERVICE_CLIENT_REQUESTED) {
- iavf_client_subtask(adapter);
- adapter->flags &= ~IAVF_FLAG_SERVICE_CLIENT_REQUESTED;
- goto out;
- }
- if (adapter->flags & IAVF_FLAG_CLIENT_NEEDS_L2_PARAMS) {
- iavf_notify_client_l2_params(&adapter->vsi);
- adapter->flags &= ~IAVF_FLAG_CLIENT_NEEDS_L2_PARAMS;
- goto out;
- }
- if (adapter->flags & IAVF_FLAG_CLIENT_NEEDS_CLOSE) {
- iavf_notify_client_close(&adapter->vsi, false);
- adapter->flags &= ~IAVF_FLAG_CLIENT_NEEDS_CLOSE;
- goto out;
- }
- if (adapter->flags & IAVF_FLAG_CLIENT_NEEDS_OPEN) {
- iavf_notify_client_open(&adapter->vsi);
- adapter->flags &= ~IAVF_FLAG_CLIENT_NEEDS_OPEN;
- }
-out:
- mutex_unlock(&adapter->client_lock);
-}
-
/**
* iavf_free_all_tx_resources - Free Tx Resources for All Queues
* @adapter: board private structure
@@ -4294,8 +4230,6 @@ static int iavf_close(struct net_device *netdev)
}
set_bit(__IAVF_VSI_DOWN, adapter->vsi.state);
- if (CLIENT_ENABLED(adapter))
- adapter->flags |= IAVF_FLAG_CLIENT_NEEDS_CLOSE;
/* We cannot send IAVF_FLAG_AQ_GET_OFFLOAD_VLAN_V2_CAPS before
* IAVF_FLAG_AQ_DISABLE_QUEUES because in such case there is rtnl
* deadlock with adminq_task() until iavf_close timeouts. We must send
@@ -4364,10 +4298,6 @@ static int iavf_change_mtu(struct net_device *netdev, int new_mtu)
netdev_dbg(netdev, "changing MTU from %d to %d\n",
netdev->mtu, new_mtu);
netdev->mtu = new_mtu;
- if (CLIENT_ENABLED(adapter)) {
- iavf_notify_client_l2_params(&adapter->vsi);
- adapter->flags |= IAVF_FLAG_SERVICE_CLIENT_REQUESTED;
- }
if (netif_running(netdev)) {
iavf_schedule_reset(adapter, IAVF_FLAG_RESET_NEEDED);
@@ -5011,7 +4941,6 @@ static int iavf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
* and destroy them only once in remove
*/
mutex_init(&adapter->crit_lock);
- mutex_init(&adapter->client_lock);
mutex_init(&hw->aq.asq_mutex);
mutex_init(&hw->aq.arq_mutex);
@@ -5031,7 +4960,6 @@ static int iavf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
INIT_WORK(&adapter->adminq_task, iavf_adminq_task);
INIT_WORK(&adapter->finish_config, iavf_finish_config);
INIT_DELAYED_WORK(&adapter->watchdog_task, iavf_watchdog_task);
- INIT_DELAYED_WORK(&adapter->client_task, iavf_client_task);
queue_delayed_work(adapter->wq, &adapter->watchdog_task,
msecs_to_jiffies(5 * (pdev->devfn & 0x07)));
@@ -5142,7 +5070,6 @@ static void iavf_remove(struct pci_dev *pdev)
struct iavf_mac_filter *f, *ftmp;
struct net_device *netdev;
struct iavf_hw *hw;
- int err;
netdev = adapter->netdev;
hw = &adapter->hw;
@@ -5176,13 +5103,6 @@ static void iavf_remove(struct pci_dev *pdev)
if (netdev->reg_state == NETREG_REGISTERED)
unregister_netdev(netdev);
- if (CLIENT_ALLOWED(adapter)) {
- err = iavf_lan_del_device(adapter);
- if (err)
- dev_warn(&pdev->dev, "Failed to delete client device: %d\n",
- err);
- }
-
mutex_lock(&adapter->crit_lock);
dev_info(&adapter->pdev->dev, "Removing device\n");
iavf_change_state(adapter, __IAVF_REMOVE);
@@ -5200,7 +5120,6 @@ static void iavf_remove(struct pci_dev *pdev)
cancel_work_sync(&adapter->reset_task);
cancel_delayed_work_sync(&adapter->watchdog_task);
cancel_work_sync(&adapter->adminq_task);
- cancel_delayed_work_sync(&adapter->client_task);
adapter->aq_required = 0;
adapter->flags &= ~IAVF_FLAG_REINIT_ITR_NEEDED;
@@ -5218,7 +5137,6 @@ static void iavf_remove(struct pci_dev *pdev)
/* destroy the locks only once, here */
mutex_destroy(&hw->aq.arq_mutex);
mutex_destroy(&hw->aq.asq_mutex);
- mutex_destroy(&adapter->client_lock);
mutex_unlock(&adapter->crit_lock);
mutex_destroy(&adapter->crit_lock);
diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
index 82b84a93bcc8..64c4443dbef9 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
@@ -3,7 +3,6 @@
#include "iavf.h"
#include "iavf_prototype.h"
-#include "iavf_client.h"
/**
* iavf_send_pf_msg
@@ -2309,19 +2308,6 @@ void iavf_virtchnl_completion(struct iavf_adapter *adapter,
if (v_opcode != adapter->current_op)
return;
break;
- case VIRTCHNL_OP_RDMA:
- /* Gobble zero-length replies from the PF. They indicate that
- * a previous message was received OK, and the client doesn't
- * care about that.
- */
- if (msglen && CLIENT_ENABLED(adapter))
- iavf_notify_client_message(&adapter->vsi, msg, msglen);
- break;
-
- case VIRTCHNL_OP_CONFIG_RDMA_IRQ_MAP:
- adapter->client_pending &=
- ~(BIT(VIRTCHNL_OP_CONFIG_RDMA_IRQ_MAP));
- break;
case VIRTCHNL_OP_GET_RSS_HENA_CAPS: {
struct virtchnl_rss_hena *vrh = (struct virtchnl_rss_hena *)msg;
--
2.41.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH net-next 1/9] iavf: fix comments about old bit locks
2023-10-23 23:08 [PATCH net-next 0/9] Intel Wired LAN Driver Updates for 2023-10-23 (iavf) Jacob Keller
2023-10-23 23:08 ` [PATCH net-next 1/1] iavf: delete the iavf client interface Jacob Keller
@ 2023-10-23 23:08 ` Jacob Keller
2023-10-23 23:08 ` [PATCH net-next 2/9] iavf: simplify mutex_trylock+sleep loops Jacob Keller
` (8 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Jacob Keller @ 2023-10-23 23:08 UTC (permalink / raw)
To: netdev, David Miller, Jakub Kicinski
Cc: Michal Schmidt, Wojciech Drewek, Rafal Romanowski, Jacob Keller
From: Michal Schmidt <mschmidt@redhat.com>
Bit lock __IAVF_IN_CRITICAL_TASK does not exist anymore since commit
5ac49f3c2702 ("iavf: use mutexes for locking of critical sections").
Adjust the comments accordingly.
Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
drivers/net/ethernet/intel/iavf/iavf_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index 7fb7dfe6e13f..8bfa928ab415 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -1276,7 +1276,7 @@ static void iavf_configure(struct iavf_adapter *adapter)
* iavf_up_complete - Finish the last steps of bringing up a connection
* @adapter: board private structure
*
- * Expects to be called while holding the __IAVF_IN_CRITICAL_TASK bit lock.
+ * Expects to be called while holding crit_lock.
**/
static void iavf_up_complete(struct iavf_adapter *adapter)
{
@@ -1400,7 +1400,7 @@ static void iavf_clear_adv_rss_conf(struct iavf_adapter *adapter)
* iavf_down - Shutdown the connection processing
* @adapter: board private structure
*
- * Expects to be called while holding the __IAVF_IN_CRITICAL_TASK bit lock.
+ * Expects to be called while holding crit_lock.
**/
void iavf_down(struct iavf_adapter *adapter)
{
--
2.41.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH net-next 2/9] iavf: simplify mutex_trylock+sleep loops
2023-10-23 23:08 [PATCH net-next 0/9] Intel Wired LAN Driver Updates for 2023-10-23 (iavf) Jacob Keller
2023-10-23 23:08 ` [PATCH net-next 1/1] iavf: delete the iavf client interface Jacob Keller
2023-10-23 23:08 ` [PATCH net-next 1/9] iavf: fix comments about old bit locks Jacob Keller
@ 2023-10-23 23:08 ` Jacob Keller
2023-10-23 23:08 ` [PATCH net-next 3/9] iavf: in iavf_down, don't queue watchdog_task if comms failed Jacob Keller
` (7 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Jacob Keller @ 2023-10-23 23:08 UTC (permalink / raw)
To: netdev, David Miller, Jakub Kicinski
Cc: Michal Schmidt, Wojciech Drewek, Rafal Romanowski, Jacob Keller
From: Michal Schmidt <mschmidt@redhat.com>
This pattern appears in two places in the iavf source code:
while (!mutex_trylock(...))
usleep_range(...);
That's just mutex_lock with extra steps.
The pattern is a leftover from when iavf used bit flags instead of
mutexes for locking. Commit 5ac49f3c2702 ("iavf: use mutexes for locking
of critical sections") replaced test_and_set_bit with !mutex_trylock,
preserving the pattern.
Simplify it to mutex_lock.
Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
drivers/net/ethernet/intel/iavf/iavf_main.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index 8bfa928ab415..da9cd53afc24 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -3011,8 +3011,7 @@ static void iavf_reset_task(struct work_struct *work)
return;
}
- while (!mutex_trylock(&adapter->client_lock))
- usleep_range(500, 1000);
+ mutex_lock(&adapter->client_lock);
if (CLIENT_ENABLED(adapter)) {
adapter->flags &= ~(IAVF_FLAG_CLIENT_NEEDS_OPEN |
IAVF_FLAG_CLIENT_NEEDS_CLOSE |
@@ -5064,8 +5063,7 @@ static int __maybe_unused iavf_suspend(struct device *dev_d)
netif_device_detach(netdev);
- while (!mutex_trylock(&adapter->crit_lock))
- usleep_range(500, 1000);
+ mutex_lock(&adapter->crit_lock);
if (netif_running(netdev)) {
rtnl_lock();
--
2.41.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH net-next 3/9] iavf: in iavf_down, don't queue watchdog_task if comms failed
2023-10-23 23:08 [PATCH net-next 0/9] Intel Wired LAN Driver Updates for 2023-10-23 (iavf) Jacob Keller
` (2 preceding siblings ...)
2023-10-23 23:08 ` [PATCH net-next 2/9] iavf: simplify mutex_trylock+sleep loops Jacob Keller
@ 2023-10-23 23:08 ` Jacob Keller
2023-10-23 23:08 ` [PATCH net-next 4/9] iavf: in iavf_down, disable queues when removing the driver Jacob Keller
` (6 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Jacob Keller @ 2023-10-23 23:08 UTC (permalink / raw)
To: netdev, David Miller, Jakub Kicinski
Cc: Michal Schmidt, Wojciech Drewek, Rafal Romanowski, Jacob Keller
From: Michal Schmidt <mschmidt@redhat.com>
The reason for queueing watchdog_task is to have it process the
aq_required flags that are being set here. If comms failed, there's
nothing to do, so return early.
Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
drivers/net/ethernet/intel/iavf/iavf_main.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index da9cd53afc24..b30d703e26a1 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -1420,8 +1420,10 @@ void iavf_down(struct iavf_adapter *adapter)
iavf_clear_fdir_filters(adapter);
iavf_clear_adv_rss_conf(adapter);
- if (!(adapter->flags & IAVF_FLAG_PF_COMMS_FAILED) &&
- !(test_bit(__IAVF_IN_REMOVE_TASK, &adapter->crit_section))) {
+ if (adapter->flags & IAVF_FLAG_PF_COMMS_FAILED)
+ return;
+
+ if (!test_bit(__IAVF_IN_REMOVE_TASK, &adapter->crit_section)) {
/* cancel any current operation */
adapter->current_op = VIRTCHNL_OP_UNKNOWN;
/* Schedule operations to close down the HW. Don't wait
--
2.41.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH net-next 4/9] iavf: in iavf_down, disable queues when removing the driver
2023-10-23 23:08 [PATCH net-next 0/9] Intel Wired LAN Driver Updates for 2023-10-23 (iavf) Jacob Keller
` (3 preceding siblings ...)
2023-10-23 23:08 ` [PATCH net-next 3/9] iavf: in iavf_down, don't queue watchdog_task if comms failed Jacob Keller
@ 2023-10-23 23:08 ` Jacob Keller
2023-10-24 23:42 ` Jakub Kicinski
2023-10-23 23:08 ` [PATCH net-next 5/9] iavf: fix the waiting time for initial reset Jacob Keller
` (5 subsequent siblings)
10 siblings, 1 reply; 19+ messages in thread
From: Jacob Keller @ 2023-10-23 23:08 UTC (permalink / raw)
To: netdev, David Miller, Jakub Kicinski
Cc: Michal Schmidt, Wojciech Drewek, Rafal Romanowski, Jacob Keller
From: Michal Schmidt <mschmidt@redhat.com>
In iavf_down, we're skipping the scheduling of certain operations if
the driver is being removed. However, the IAVF_FLAG_AQ_DISABLE_QUEUES
request must not be skipped in this case, because iavf_close waits
for the transition to the __IAVF_DOWN state, which happens in
iavf_virtchnl_completion after the queues are released.
Without this fix, "rmmod iavf" takes half a second per interface that's
up and prints the "Device resources not yet released" warning.
Fixes: c8de44b577eb ("iavf: do not process adminq tasks when __IAVF_IN_REMOVE_TASK is set")
Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
drivers/net/ethernet/intel/iavf/iavf_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index b30d703e26a1..7ca19dfea109 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -1440,9 +1440,9 @@ void iavf_down(struct iavf_adapter *adapter)
adapter->aq_required |= IAVF_FLAG_AQ_DEL_FDIR_FILTER;
if (!list_empty(&adapter->adv_rss_list_head))
adapter->aq_required |= IAVF_FLAG_AQ_DEL_ADV_RSS_CFG;
- adapter->aq_required |= IAVF_FLAG_AQ_DISABLE_QUEUES;
}
+ adapter->aq_required |= IAVF_FLAG_AQ_DISABLE_QUEUES;
mod_delayed_work(adapter->wq, &adapter->watchdog_task, 0);
}
--
2.41.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH net-next 4/9] iavf: in iavf_down, disable queues when removing the driver
2023-10-23 23:08 ` [PATCH net-next 4/9] iavf: in iavf_down, disable queues when removing the driver Jacob Keller
@ 2023-10-24 23:42 ` Jakub Kicinski
2023-10-25 15:24 ` Michal Schmidt
0 siblings, 1 reply; 19+ messages in thread
From: Jakub Kicinski @ 2023-10-24 23:42 UTC (permalink / raw)
To: Jacob Keller
Cc: netdev, David Miller, Michal Schmidt, Wojciech Drewek,
Rafal Romanowski
On Mon, 23 Oct 2023 16:08:21 -0700 Jacob Keller wrote:
> From: Michal Schmidt <mschmidt@redhat.com>
>
> In iavf_down, we're skipping the scheduling of certain operations if
> the driver is being removed. However, the IAVF_FLAG_AQ_DISABLE_QUEUES
> request must not be skipped in this case, because iavf_close waits
> for the transition to the __IAVF_DOWN state, which happens in
> iavf_virtchnl_completion after the queues are released.
>
> Without this fix, "rmmod iavf" takes half a second per interface that's
> up and prints the "Device resources not yet released" warning.
>
> Fixes: c8de44b577eb ("iavf: do not process adminq tasks when __IAVF_IN_REMOVE_TASK is set")
This looks like a 6.6 regression, why send it for net-next?
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH net-next 4/9] iavf: in iavf_down, disable queues when removing the driver
2023-10-24 23:42 ` Jakub Kicinski
@ 2023-10-25 15:24 ` Michal Schmidt
2023-10-25 16:25 ` Jakub Kicinski
0 siblings, 1 reply; 19+ messages in thread
From: Michal Schmidt @ 2023-10-25 15:24 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Jacob Keller, netdev, David Miller, Wojciech Drewek,
Rafal Romanowski
On Wed, Oct 25, 2023 at 1:42 AM Jakub Kicinski <kuba@kernel.org> wrote:
> On Mon, 23 Oct 2023 16:08:21 -0700 Jacob Keller wrote:
> > From: Michal Schmidt <mschmidt@redhat.com>
> >
> > In iavf_down, we're skipping the scheduling of certain operations if
> > the driver is being removed. However, the IAVF_FLAG_AQ_DISABLE_QUEUES
> > request must not be skipped in this case, because iavf_close waits
> > for the transition to the __IAVF_DOWN state, which happens in
> > iavf_virtchnl_completion after the queues are released.
> >
> > Without this fix, "rmmod iavf" takes half a second per interface that's
> > up and prints the "Device resources not yet released" warning.
> >
> > Fixes: c8de44b577eb ("iavf: do not process adminq tasks when __IAVF_IN_REMOVE_TASK is set")
>
> This looks like a 6.6 regression, why send it for net-next?
Hi Jakub,
At first I thought I had a dependency on the preceding patch in the
series, but after rethinking and retesting it, it's actually fine to
put this patch in net.git.
Can you please do that, or will you require resending?
Thanks,
Michal
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH net-next 4/9] iavf: in iavf_down, disable queues when removing the driver
2023-10-25 15:24 ` Michal Schmidt
@ 2023-10-25 16:25 ` Jakub Kicinski
2023-10-25 17:23 ` Jacob Keller
0 siblings, 1 reply; 19+ messages in thread
From: Jakub Kicinski @ 2023-10-25 16:25 UTC (permalink / raw)
To: Michal Schmidt
Cc: Jacob Keller, netdev, David Miller, Wojciech Drewek,
Rafal Romanowski
On Wed, 25 Oct 2023 17:24:59 +0200 Michal Schmidt wrote:
> > This looks like a 6.6 regression, why send it for net-next?
>
> Hi Jakub,
> At first I thought I had a dependency on the preceding patch in the
> series, but after rethinking and retesting it, it's actually fine to
> put this patch in net.git.
> Can you please do that, or will you require resending?
I'd prefer if Jake could resend just the fix for net, after re-testing
that it indeed works right. I'll make sure that it makes tomorrow's PR
from net, in case the net-next stuff would conflict.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net-next 4/9] iavf: in iavf_down, disable queues when removing the driver
2023-10-25 16:25 ` Jakub Kicinski
@ 2023-10-25 17:23 ` Jacob Keller
2023-10-25 18:32 ` Jacob Keller
0 siblings, 1 reply; 19+ messages in thread
From: Jacob Keller @ 2023-10-25 17:23 UTC (permalink / raw)
To: Jakub Kicinski, Michal Schmidt
Cc: netdev, David Miller, Wojciech Drewek, Rafal Romanowski
On 10/25/2023 9:25 AM, Jakub Kicinski wrote:
> On Wed, 25 Oct 2023 17:24:59 +0200 Michal Schmidt wrote:
>>> This looks like a 6.6 regression, why send it for net-next?
>>
>> Hi Jakub,
>> At first I thought I had a dependency on the preceding patch in the
>> series, but after rethinking and retesting it, it's actually fine to
>> put this patch in net.git.
>> Can you please do that, or will you require resending?
>
> I'd prefer if Jake could resend just the fix for net, after re-testing
> that it indeed works right. I'll make sure that it makes tomorrow's PR
> from net, in case the net-next stuff would conflict.
>
Will do, thanks.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net-next 4/9] iavf: in iavf_down, disable queues when removing the driver
2023-10-25 17:23 ` Jacob Keller
@ 2023-10-25 18:32 ` Jacob Keller
0 siblings, 0 replies; 19+ messages in thread
From: Jacob Keller @ 2023-10-25 18:32 UTC (permalink / raw)
To: Jakub Kicinski, Michal Schmidt
Cc: netdev, David Miller, Wojciech Drewek, Rafal Romanowski
On 10/25/2023 10:23 AM, Jacob Keller wrote:
>
>
> On 10/25/2023 9:25 AM, Jakub Kicinski wrote:
>> On Wed, 25 Oct 2023 17:24:59 +0200 Michal Schmidt wrote:
>>>> This looks like a 6.6 regression, why send it for net-next?
>>>
>>> Hi Jakub,
>>> At first I thought I had a dependency on the preceding patch in the
>>> series, but after rethinking and retesting it, it's actually fine to
>>> put this patch in net.git.
>>> Can you please do that, or will you require resending?
>>
>> I'd prefer if Jake could resend just the fix for net, after re-testing
>> that it indeed works right. I'll make sure that it makes tomorrow's PR
>> from net, in case the net-next stuff would conflict.
>>
>
> Will do, thanks.
>
I tested this on one of my dev systems with 128 VFs. Without the fix
applied:
$ time rmmod iavf
real 1m19.132s
user 0m0.007s
sys 0m1.974s
With the fix applied:
$ time rmmod iavf
real 0m17.951s
user 0m0.006s
sys 0m0.827s
I'll send this out to net shortly.
Thanks,
Jake
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH net-next 5/9] iavf: fix the waiting time for initial reset
2023-10-23 23:08 [PATCH net-next 0/9] Intel Wired LAN Driver Updates for 2023-10-23 (iavf) Jacob Keller
` (4 preceding siblings ...)
2023-10-23 23:08 ` [PATCH net-next 4/9] iavf: in iavf_down, disable queues when removing the driver Jacob Keller
@ 2023-10-23 23:08 ` Jacob Keller
2023-10-23 23:08 ` [PATCH net-next 6/9] iavf: rely on netdev's own registered state Jacob Keller
` (4 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Jacob Keller @ 2023-10-23 23:08 UTC (permalink / raw)
To: netdev, David Miller, Jakub Kicinski
Cc: Michal Schmidt, Wojciech Drewek, Rafal Romanowski, Jacob Keller
From: Michal Schmidt <mschmidt@redhat.com>
Every time I create VFs on ice, I receive at least one "Device is still
in reset (-16), retrying" message per VF. It recovers fine, but typical
usecases should not trigger scary-looking messages.
The waiting for reset is too short. It makes no sense to check every 10
microseconds. Typical reset waiting times are at least tens of
milliseconds and can be several seconds. I suspect the polling interval
was meant to be 10 milliseconds all along.
IAVF_RESET_WAIT_COMPLETE_COUNT is defined as 2000, so the total waiting
time could be over 20 seconds. I have seen resets take 5 seconds (with
128 VFs on ice).
The added benefit of not triggering the "Device is still in reset" path
is that we avoid going through the __IAVF_INIT_FAILED state, which would
take a full second before retrying.
Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
drivers/net/ethernet/intel/iavf/iavf_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index 7ca19dfea109..36d72f30ffce 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -4786,7 +4786,7 @@ static int iavf_check_reset_complete(struct iavf_hw *hw)
if ((rstat == VIRTCHNL_VFR_VFACTIVE) ||
(rstat == VIRTCHNL_VFR_COMPLETED))
return 0;
- usleep_range(10, 20);
+ msleep(IAVF_RESET_WAIT_MS);
}
return -EBUSY;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH net-next 6/9] iavf: rely on netdev's own registered state
2023-10-23 23:08 [PATCH net-next 0/9] Intel Wired LAN Driver Updates for 2023-10-23 (iavf) Jacob Keller
` (5 preceding siblings ...)
2023-10-23 23:08 ` [PATCH net-next 5/9] iavf: fix the waiting time for initial reset Jacob Keller
@ 2023-10-23 23:08 ` Jacob Keller
2023-10-23 23:08 ` [PATCH net-next 7/9] iavf: use unregister_netdev Jacob Keller
` (3 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Jacob Keller @ 2023-10-23 23:08 UTC (permalink / raw)
To: netdev, David Miller, Jakub Kicinski
Cc: Michal Schmidt, Wojciech Drewek, Jacob Keller
From: Michal Schmidt <mschmidt@redhat.com>
The information whether a netdev has been registered is already present
in the netdev itself. There's no need for a driver flag with the same
meaning.
Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
drivers/net/ethernet/intel/iavf/iavf.h | 1 -
drivers/net/ethernet/intel/iavf/iavf_main.c | 9 +++------
2 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h
index 44daf335e8c5..f026d0670338 100644
--- a/drivers/net/ethernet/intel/iavf/iavf.h
+++ b/drivers/net/ethernet/intel/iavf/iavf.h
@@ -377,7 +377,6 @@ struct iavf_adapter {
unsigned long crit_section;
struct delayed_work watchdog_task;
- bool netdev_registered;
bool link_up;
enum virtchnl_link_speed link_speed;
/* This is only populated if the VIRTCHNL_VF_CAP_ADV_LINK_SPEED is set
diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index 36d72f30ffce..6e93e73385b3 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -2021,7 +2021,7 @@ static void iavf_finish_config(struct work_struct *work)
mutex_lock(&adapter->crit_lock);
if ((adapter->flags & IAVF_FLAG_SETUP_NETDEV_FEATURES) &&
- adapter->netdev_registered &&
+ adapter->netdev->reg_state == NETREG_REGISTERED &&
!test_bit(__IAVF_IN_REMOVE_TASK, &adapter->crit_section)) {
netdev_update_features(adapter->netdev);
adapter->flags &= ~IAVF_FLAG_SETUP_NETDEV_FEATURES;
@@ -2029,7 +2029,7 @@ static void iavf_finish_config(struct work_struct *work)
switch (adapter->state) {
case __IAVF_DOWN:
- if (!adapter->netdev_registered) {
+ if (adapter->netdev->reg_state != NETREG_REGISTERED) {
err = register_netdevice(adapter->netdev);
if (err) {
dev_err(&adapter->pdev->dev, "Unable to register netdev (%d)\n",
@@ -2043,7 +2043,6 @@ static void iavf_finish_config(struct work_struct *work)
__IAVF_INIT_CONFIG_ADAPTER);
goto out;
}
- adapter->netdev_registered = true;
}
/* Set the real number of queues when reset occurs while
@@ -5168,10 +5167,8 @@ static void iavf_remove(struct pci_dev *pdev)
cancel_work_sync(&adapter->finish_config);
rtnl_lock();
- if (adapter->netdev_registered) {
+ if (netdev->reg_state == NETREG_REGISTERED)
unregister_netdevice(netdev);
- adapter->netdev_registered = false;
- }
rtnl_unlock();
if (CLIENT_ALLOWED(adapter)) {
--
2.41.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH net-next 7/9] iavf: use unregister_netdev
2023-10-23 23:08 [PATCH net-next 0/9] Intel Wired LAN Driver Updates for 2023-10-23 (iavf) Jacob Keller
` (6 preceding siblings ...)
2023-10-23 23:08 ` [PATCH net-next 6/9] iavf: rely on netdev's own registered state Jacob Keller
@ 2023-10-23 23:08 ` Jacob Keller
2023-10-24 23:44 ` Jakub Kicinski
2023-10-23 23:08 ` [PATCH net-next 8/9] iavf: add a common function for undoing the interrupt scheme Jacob Keller
` (2 subsequent siblings)
10 siblings, 1 reply; 19+ messages in thread
From: Jacob Keller @ 2023-10-23 23:08 UTC (permalink / raw)
To: netdev, David Miller, Jakub Kicinski
Cc: Michal Schmidt, Wojciech Drewek, Jacob Keller
From: Michal Schmidt <mschmidt@redhat.com>
Use unregister_netdev, which takes rtnl_lock for us. We don't have to
check the reg_state under rtnl_lock. There's nothing to race with. We
have just cancelled the finish_config work.
Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
drivers/net/ethernet/intel/iavf/iavf_main.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index 6e93e73385b3..85252b9dce50 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -5166,10 +5166,8 @@ static void iavf_remove(struct pci_dev *pdev)
cancel_delayed_work_sync(&adapter->watchdog_task);
cancel_work_sync(&adapter->finish_config);
- rtnl_lock();
if (netdev->reg_state == NETREG_REGISTERED)
- unregister_netdevice(netdev);
- rtnl_unlock();
+ unregister_netdev(netdev);
if (CLIENT_ALLOWED(adapter)) {
err = iavf_lan_del_device(adapter);
--
2.41.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH net-next 7/9] iavf: use unregister_netdev
2023-10-23 23:08 ` [PATCH net-next 7/9] iavf: use unregister_netdev Jacob Keller
@ 2023-10-24 23:44 ` Jakub Kicinski
0 siblings, 0 replies; 19+ messages in thread
From: Jakub Kicinski @ 2023-10-24 23:44 UTC (permalink / raw)
To: Jacob Keller; +Cc: netdev, David Miller, Michal Schmidt, Wojciech Drewek
On Mon, 23 Oct 2023 16:08:24 -0700 Jacob Keller wrote:
> Use unregister_netdev, which takes rtnl_lock for us. We don't have to
> check the reg_state under rtnl_lock. There's nothing to race with. We
> have just cancelled the finish_config work.
I can't really convince myself that its indeed the case... but either
way if something can register the netdev past the check - the code is
buggy with or without rtnl_lock, so patch seems sane.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH net-next 8/9] iavf: add a common function for undoing the interrupt scheme
2023-10-23 23:08 [PATCH net-next 0/9] Intel Wired LAN Driver Updates for 2023-10-23 (iavf) Jacob Keller
` (7 preceding siblings ...)
2023-10-23 23:08 ` [PATCH net-next 7/9] iavf: use unregister_netdev Jacob Keller
@ 2023-10-23 23:08 ` Jacob Keller
2023-10-23 23:08 ` [PATCH net-next 9/9] iavf: delete the iavf client interface Jacob Keller
2023-10-27 16:58 ` [PATCH net-next 0/9] Intel Wired LAN Driver Updates for 2023-10-23 (iavf) Jacob Keller
10 siblings, 0 replies; 19+ messages in thread
From: Jacob Keller @ 2023-10-23 23:08 UTC (permalink / raw)
To: netdev, David Miller, Jakub Kicinski
Cc: Michal Schmidt, Wojciech Drewek, Jacob Keller
From: Michal Schmidt <mschmidt@redhat.com>
Add a new function iavf_free_interrupt_scheme that does the inverse of
iavf_init_interrupt_scheme. Symmetry is nice. And there will be three
callers already.
Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
drivers/net/ethernet/intel/iavf/iavf_main.c | 26 ++++++++++++---------
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index 85252b9dce50..49a764ca5e36 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -1954,6 +1954,17 @@ static int iavf_init_interrupt_scheme(struct iavf_adapter *adapter)
return err;
}
+/**
+ * iavf_free_interrupt_scheme - Undo what iavf_init_interrupt_scheme does
+ * @adapter: board private structure
+ **/
+static void iavf_free_interrupt_scheme(struct iavf_adapter *adapter)
+{
+ iavf_free_q_vectors(adapter);
+ iavf_reset_interrupt_capability(adapter);
+ iavf_free_queues(adapter);
+}
+
/**
* iavf_free_rss - Free memory used by RSS structs
* @adapter: board private structure
@@ -1982,11 +1993,9 @@ static int iavf_reinit_interrupt_scheme(struct iavf_adapter *adapter, bool runni
if (running)
iavf_free_traffic_irqs(adapter);
iavf_free_misc_irq(adapter);
- iavf_reset_interrupt_capability(adapter);
- iavf_free_q_vectors(adapter);
- iavf_free_queues(adapter);
+ iavf_free_interrupt_scheme(adapter);
- err = iavf_init_interrupt_scheme(adapter);
+ err = iavf_init_interrupt_scheme(adapter);
if (err)
goto err;
@@ -2968,9 +2977,7 @@ static void iavf_disable_vf(struct iavf_adapter *adapter)
spin_unlock_bh(&adapter->cloud_filter_list_lock);
iavf_free_misc_irq(adapter);
- iavf_reset_interrupt_capability(adapter);
- iavf_free_q_vectors(adapter);
- iavf_free_queues(adapter);
+ iavf_free_interrupt_scheme(adapter);
memset(adapter->vf_res, 0, IAVF_VIRTCHNL_VF_RESOURCE_SIZE);
iavf_shutdown_adminq(&adapter->hw);
adapter->flags &= ~IAVF_FLAG_RESET_PENDING;
@@ -5201,9 +5208,7 @@ static void iavf_remove(struct pci_dev *pdev)
iavf_free_all_tx_resources(adapter);
iavf_free_all_rx_resources(adapter);
iavf_free_misc_irq(adapter);
-
- iavf_reset_interrupt_capability(adapter);
- iavf_free_q_vectors(adapter);
+ iavf_free_interrupt_scheme(adapter);
iavf_free_rss(adapter);
@@ -5219,7 +5224,6 @@ static void iavf_remove(struct pci_dev *pdev)
iounmap(hw->hw_addr);
pci_release_regions(pdev);
- iavf_free_queues(adapter);
kfree(adapter->vf_res);
spin_lock_bh(&adapter->mac_vlan_list_lock);
/* If we got removed before an up/down sequence, we've got a filter
--
2.41.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH net-next 9/9] iavf: delete the iavf client interface
2023-10-23 23:08 [PATCH net-next 0/9] Intel Wired LAN Driver Updates for 2023-10-23 (iavf) Jacob Keller
` (8 preceding siblings ...)
2023-10-23 23:08 ` [PATCH net-next 8/9] iavf: add a common function for undoing the interrupt scheme Jacob Keller
@ 2023-10-23 23:08 ` Jacob Keller
2023-10-27 16:58 ` [PATCH net-next 0/9] Intel Wired LAN Driver Updates for 2023-10-23 (iavf) Jacob Keller
10 siblings, 0 replies; 19+ messages in thread
From: Jacob Keller @ 2023-10-23 23:08 UTC (permalink / raw)
To: netdev, David Miller, Jakub Kicinski; +Cc: Michal Schmidt, Jacob Keller
From: Michal Schmidt <mschmidt@redhat.com>
The iavf client interface was added in 2017 by commit ed0e894de7c1
("i40evf: add client interface"), but there have never been any in-tree
callers.
It's not useful for future development either. The Intel out-of-tree
iavf and irdma drivers instead use an auxiliary bus, which is a better
solution.
Remove the iavf client interface code. Also gone are the client_task
work and the client_lock mutex.
Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
drivers/net/ethernet/intel/iavf/Makefile | 2 +-
drivers/net/ethernet/intel/iavf/iavf.h | 27 -
drivers/net/ethernet/intel/iavf/iavf_client.c | 578 ------------------
drivers/net/ethernet/intel/iavf/iavf_client.h | 169 -----
drivers/net/ethernet/intel/iavf/iavf_main.c | 82 ---
.../net/ethernet/intel/iavf/iavf_virtchnl.c | 14 -
6 files changed, 1 insertion(+), 871 deletions(-)
delete mode 100644 drivers/net/ethernet/intel/iavf/iavf_client.c
delete mode 100644 drivers/net/ethernet/intel/iavf/iavf_client.h
diff --git a/drivers/net/ethernet/intel/iavf/Makefile b/drivers/net/ethernet/intel/iavf/Makefile
index 9c3e45c54d01..2d154a4e2fd7 100644
--- a/drivers/net/ethernet/intel/iavf/Makefile
+++ b/drivers/net/ethernet/intel/iavf/Makefile
@@ -13,4 +13,4 @@ obj-$(CONFIG_IAVF) += iavf.o
iavf-objs := iavf_main.o iavf_ethtool.o iavf_virtchnl.o iavf_fdir.o \
iavf_adv_rss.o \
- iavf_txrx.o iavf_common.o iavf_adminq.o iavf_client.o
+ iavf_txrx.o iavf_common.o iavf_adminq.o
diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h
index f026d0670338..e7ab89dc883a 100644
--- a/drivers/net/ethernet/intel/iavf/iavf.h
+++ b/drivers/net/ethernet/intel/iavf/iavf.h
@@ -63,7 +63,6 @@ struct iavf_vsi {
DECLARE_BITMAP(state, __IAVF_VSI_STATE_SIZE__);
int base_vector;
u16 qs_handle;
- void *priv; /* client driver data reference. */
};
/* How many Rx Buffers do we bundle into one write to the hardware ? */
@@ -256,7 +255,6 @@ struct iavf_adapter {
struct work_struct reset_task;
struct work_struct adminq_task;
struct work_struct finish_config;
- struct delayed_work client_task;
wait_queue_head_t down_waitqueue;
wait_queue_head_t reset_waitqueue;
wait_queue_head_t vc_waitqueue;
@@ -265,7 +263,6 @@ struct iavf_adapter {
int num_vlan_filters;
struct list_head mac_filter_list;
struct mutex crit_lock;
- struct mutex client_lock;
/* Lock to protect accesses to MAC and VLAN lists */
spinlock_t mac_vlan_list_lock;
char misc_vector_name[IFNAMSIZ + 9];
@@ -282,10 +279,6 @@ struct iavf_adapter {
u64 hw_csum_rx_error;
u32 rx_desc_count;
int num_msix_vectors;
- int num_rdma_msix;
- int rdma_base_vector;
- u32 client_pending;
- struct iavf_client_instance *cinst;
struct msix_entry *msix_entries;
u32 flags;
@@ -294,10 +287,6 @@ struct iavf_adapter {
#define IAVF_FLAG_RESET_PENDING BIT(4)
#define IAVF_FLAG_RESET_NEEDED BIT(5)
#define IAVF_FLAG_WB_ON_ITR_CAPABLE BIT(6)
-#define IAVF_FLAG_SERVICE_CLIENT_REQUESTED BIT(9)
-#define IAVF_FLAG_CLIENT_NEEDS_OPEN BIT(10)
-#define IAVF_FLAG_CLIENT_NEEDS_CLOSE BIT(11)
-#define IAVF_FLAG_CLIENT_NEEDS_L2_PARAMS BIT(12)
#define IAVF_FLAG_LEGACY_RX BIT(15)
#define IAVF_FLAG_REINIT_ITR_NEEDED BIT(16)
#define IAVF_FLAG_QUEUES_DISABLED BIT(17)
@@ -388,11 +377,6 @@ struct iavf_adapter {
u32 link_speed_mbps;
enum virtchnl_ops current_op;
-#define CLIENT_ALLOWED(_a) ((_a)->vf_res ? \
- (_a)->vf_res->vf_cap_flags & \
- VIRTCHNL_VF_OFFLOAD_RDMA : \
- 0)
-#define CLIENT_ENABLED(_a) ((_a)->cinst)
/* RSS by the PF should be preferred over RSS via other methods. */
#define RSS_PF(_a) ((_a)->vf_res->vf_cap_flags & \
VIRTCHNL_VF_OFFLOAD_RSS_PF)
@@ -460,12 +444,6 @@ struct iavf_adapter {
/* Ethtool Private Flags */
-/* lan device, used by client interface */
-struct iavf_device {
- struct list_head list;
- struct iavf_adapter *vf;
-};
-
/* needed by iavf_ethtool.c */
extern char iavf_driver_name[];
@@ -569,11 +547,6 @@ void iavf_virtchnl_completion(struct iavf_adapter *adapter,
int iavf_config_rss(struct iavf_adapter *adapter);
int iavf_lan_add_device(struct iavf_adapter *adapter);
int iavf_lan_del_device(struct iavf_adapter *adapter);
-void iavf_client_subtask(struct iavf_adapter *adapter);
-void iavf_notify_client_message(struct iavf_vsi *vsi, u8 *msg, u16 len);
-void iavf_notify_client_l2_params(struct iavf_vsi *vsi);
-void iavf_notify_client_open(struct iavf_vsi *vsi);
-void iavf_notify_client_close(struct iavf_vsi *vsi, bool reset);
void iavf_enable_channels(struct iavf_adapter *adapter);
void iavf_disable_channels(struct iavf_adapter *adapter);
void iavf_add_cloud_filter(struct iavf_adapter *adapter);
diff --git a/drivers/net/ethernet/intel/iavf/iavf_client.c b/drivers/net/ethernet/intel/iavf/iavf_client.c
deleted file mode 100644
index e6051b6355aa..000000000000
--- a/drivers/net/ethernet/intel/iavf/iavf_client.c
+++ /dev/null
@@ -1,578 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/* Copyright(c) 2013 - 2018 Intel Corporation. */
-
-#include <linux/list.h>
-#include <linux/errno.h>
-
-#include "iavf.h"
-#include "iavf_prototype.h"
-#include "iavf_client.h"
-
-static
-const char iavf_client_interface_version_str[] = IAVF_CLIENT_VERSION_STR;
-static struct iavf_client *vf_registered_client;
-static LIST_HEAD(iavf_devices);
-static DEFINE_MUTEX(iavf_device_mutex);
-
-static u32 iavf_client_virtchnl_send(struct iavf_info *ldev,
- struct iavf_client *client,
- u8 *msg, u16 len);
-
-static int iavf_client_setup_qvlist(struct iavf_info *ldev,
- struct iavf_client *client,
- struct iavf_qvlist_info *qvlist_info);
-
-static struct iavf_ops iavf_lan_ops = {
- .virtchnl_send = iavf_client_virtchnl_send,
- .setup_qvlist = iavf_client_setup_qvlist,
-};
-
-/**
- * iavf_client_get_params - retrieve relevant client parameters
- * @vsi: VSI with parameters
- * @params: client param struct
- **/
-static
-void iavf_client_get_params(struct iavf_vsi *vsi, struct iavf_params *params)
-{
- int i;
-
- memset(params, 0, sizeof(struct iavf_params));
- params->mtu = vsi->netdev->mtu;
- params->link_up = vsi->back->link_up;
-
- for (i = 0; i < IAVF_MAX_USER_PRIORITY; i++) {
- params->qos.prio_qos[i].tc = 0;
- params->qos.prio_qos[i].qs_handle = vsi->qs_handle;
- }
-}
-
-/**
- * iavf_notify_client_message - call the client message receive callback
- * @vsi: the VSI associated with this client
- * @msg: message buffer
- * @len: length of message
- *
- * If there is a client to this VSI, call the client
- **/
-void iavf_notify_client_message(struct iavf_vsi *vsi, u8 *msg, u16 len)
-{
- struct iavf_client_instance *cinst;
-
- if (!vsi)
- return;
-
- cinst = vsi->back->cinst;
- if (!cinst || !cinst->client || !cinst->client->ops ||
- !cinst->client->ops->virtchnl_receive) {
- dev_dbg(&vsi->back->pdev->dev,
- "Cannot locate client instance virtchnl_receive function\n");
- return;
- }
- cinst->client->ops->virtchnl_receive(&cinst->lan_info, cinst->client,
- msg, len);
-}
-
-/**
- * iavf_notify_client_l2_params - call the client notify callback
- * @vsi: the VSI with l2 param changes
- *
- * If there is a client to this VSI, call the client
- **/
-void iavf_notify_client_l2_params(struct iavf_vsi *vsi)
-{
- struct iavf_client_instance *cinst;
- struct iavf_params params;
-
- if (!vsi)
- return;
-
- cinst = vsi->back->cinst;
-
- if (!cinst || !cinst->client || !cinst->client->ops ||
- !cinst->client->ops->l2_param_change) {
- dev_dbg(&vsi->back->pdev->dev,
- "Cannot locate client instance l2_param_change function\n");
- return;
- }
- iavf_client_get_params(vsi, ¶ms);
- cinst->lan_info.params = params;
- cinst->client->ops->l2_param_change(&cinst->lan_info, cinst->client,
- ¶ms);
-}
-
-/**
- * iavf_notify_client_open - call the client open callback
- * @vsi: the VSI with netdev opened
- *
- * If there is a client to this netdev, call the client with open
- **/
-void iavf_notify_client_open(struct iavf_vsi *vsi)
-{
- struct iavf_adapter *adapter = vsi->back;
- struct iavf_client_instance *cinst = adapter->cinst;
- int ret;
-
- if (!cinst || !cinst->client || !cinst->client->ops ||
- !cinst->client->ops->open) {
- dev_dbg(&vsi->back->pdev->dev,
- "Cannot locate client instance open function\n");
- return;
- }
- if (!(test_bit(__IAVF_CLIENT_INSTANCE_OPENED, &cinst->state))) {
- ret = cinst->client->ops->open(&cinst->lan_info, cinst->client);
- if (!ret)
- set_bit(__IAVF_CLIENT_INSTANCE_OPENED, &cinst->state);
- }
-}
-
-/**
- * iavf_client_release_qvlist - send a message to the PF to release rdma qv map
- * @ldev: pointer to L2 context.
- *
- * Return 0 on success or < 0 on error
- **/
-static int iavf_client_release_qvlist(struct iavf_info *ldev)
-{
- struct iavf_adapter *adapter = ldev->vf;
- enum iavf_status err;
-
- if (adapter->aq_required)
- return -EAGAIN;
-
- err = iavf_aq_send_msg_to_pf(&adapter->hw,
- VIRTCHNL_OP_RELEASE_RDMA_IRQ_MAP,
- IAVF_SUCCESS, NULL, 0, NULL);
-
- if (err)
- dev_err(&adapter->pdev->dev,
- "Unable to send RDMA vector release message to PF, error %d, aq status %d\n",
- err, adapter->hw.aq.asq_last_status);
-
- return err;
-}
-
-/**
- * iavf_notify_client_close - call the client close callback
- * @vsi: the VSI with netdev closed
- * @reset: true when close called due to reset pending
- *
- * If there is a client to this netdev, call the client with close
- **/
-void iavf_notify_client_close(struct iavf_vsi *vsi, bool reset)
-{
- struct iavf_adapter *adapter = vsi->back;
- struct iavf_client_instance *cinst = adapter->cinst;
-
- if (!cinst || !cinst->client || !cinst->client->ops ||
- !cinst->client->ops->close) {
- dev_dbg(&vsi->back->pdev->dev,
- "Cannot locate client instance close function\n");
- return;
- }
- cinst->client->ops->close(&cinst->lan_info, cinst->client, reset);
- iavf_client_release_qvlist(&cinst->lan_info);
- clear_bit(__IAVF_CLIENT_INSTANCE_OPENED, &cinst->state);
-}
-
-/**
- * iavf_client_add_instance - add a client instance to the instance list
- * @adapter: pointer to the board struct
- *
- * Returns cinst ptr on success, NULL on failure
- **/
-static struct iavf_client_instance *
-iavf_client_add_instance(struct iavf_adapter *adapter)
-{
- struct iavf_client_instance *cinst = NULL;
- struct iavf_vsi *vsi = &adapter->vsi;
- struct netdev_hw_addr *mac = NULL;
- struct iavf_params params;
-
- if (!vf_registered_client)
- goto out;
-
- if (adapter->cinst) {
- cinst = adapter->cinst;
- goto out;
- }
-
- cinst = kzalloc(sizeof(*cinst), GFP_KERNEL);
- if (!cinst)
- goto out;
-
- cinst->lan_info.vf = (void *)adapter;
- cinst->lan_info.netdev = vsi->netdev;
- cinst->lan_info.pcidev = adapter->pdev;
- cinst->lan_info.fid = 0;
- cinst->lan_info.ftype = IAVF_CLIENT_FTYPE_VF;
- cinst->lan_info.hw_addr = adapter->hw.hw_addr;
- cinst->lan_info.ops = &iavf_lan_ops;
- cinst->lan_info.version.major = IAVF_CLIENT_VERSION_MAJOR;
- cinst->lan_info.version.minor = IAVF_CLIENT_VERSION_MINOR;
- cinst->lan_info.version.build = IAVF_CLIENT_VERSION_BUILD;
- iavf_client_get_params(vsi, ¶ms);
- cinst->lan_info.params = params;
- set_bit(__IAVF_CLIENT_INSTANCE_NONE, &cinst->state);
-
- cinst->lan_info.msix_count = adapter->num_rdma_msix;
- cinst->lan_info.msix_entries =
- &adapter->msix_entries[adapter->rdma_base_vector];
-
- mac = list_first_entry(&cinst->lan_info.netdev->dev_addrs.list,
- struct netdev_hw_addr, list);
- if (mac)
- ether_addr_copy(cinst->lan_info.lanmac, mac->addr);
- else
- dev_err(&adapter->pdev->dev, "MAC address list is empty!\n");
-
- cinst->client = vf_registered_client;
- adapter->cinst = cinst;
-out:
- return cinst;
-}
-
-/**
- * iavf_client_del_instance - removes a client instance from the list
- * @adapter: pointer to the board struct
- *
- **/
-static
-void iavf_client_del_instance(struct iavf_adapter *adapter)
-{
- kfree(adapter->cinst);
- adapter->cinst = NULL;
-}
-
-/**
- * iavf_client_subtask - client maintenance work
- * @adapter: board private structure
- **/
-void iavf_client_subtask(struct iavf_adapter *adapter)
-{
- struct iavf_client *client = vf_registered_client;
- struct iavf_client_instance *cinst;
- int ret = 0;
-
- if (adapter->state < __IAVF_DOWN)
- return;
-
- /* first check client is registered */
- if (!client)
- return;
-
- /* Add the client instance to the instance list */
- cinst = iavf_client_add_instance(adapter);
- if (!cinst)
- return;
-
- dev_info(&adapter->pdev->dev, "Added instance of Client %s\n",
- client->name);
-
- if (!test_bit(__IAVF_CLIENT_INSTANCE_OPENED, &cinst->state)) {
- /* Send an Open request to the client */
-
- if (client->ops && client->ops->open)
- ret = client->ops->open(&cinst->lan_info, client);
- if (!ret)
- set_bit(__IAVF_CLIENT_INSTANCE_OPENED,
- &cinst->state);
- else
- /* remove client instance */
- iavf_client_del_instance(adapter);
- }
-}
-
-/**
- * iavf_lan_add_device - add a lan device struct to the list of lan devices
- * @adapter: pointer to the board struct
- *
- * Returns 0 on success or none 0 on error
- **/
-int iavf_lan_add_device(struct iavf_adapter *adapter)
-{
- struct iavf_device *ldev;
- int ret = 0;
-
- mutex_lock(&iavf_device_mutex);
- list_for_each_entry(ldev, &iavf_devices, list) {
- if (ldev->vf == adapter) {
- ret = -EEXIST;
- goto out;
- }
- }
- ldev = kzalloc(sizeof(*ldev), GFP_KERNEL);
- if (!ldev) {
- ret = -ENOMEM;
- goto out;
- }
- ldev->vf = adapter;
- INIT_LIST_HEAD(&ldev->list);
- list_add(&ldev->list, &iavf_devices);
- dev_info(&adapter->pdev->dev, "Added LAN device bus=0x%02x dev=0x%02x func=0x%02x\n",
- adapter->hw.bus.bus_id, adapter->hw.bus.device,
- adapter->hw.bus.func);
-
- /* Since in some cases register may have happened before a device gets
- * added, we can schedule a subtask to go initiate the clients.
- */
- adapter->flags |= IAVF_FLAG_SERVICE_CLIENT_REQUESTED;
-
-out:
- mutex_unlock(&iavf_device_mutex);
- return ret;
-}
-
-/**
- * iavf_lan_del_device - removes a lan device from the device list
- * @adapter: pointer to the board struct
- *
- * Returns 0 on success or non-0 on error
- **/
-int iavf_lan_del_device(struct iavf_adapter *adapter)
-{
- struct iavf_device *ldev, *tmp;
- int ret = -ENODEV;
-
- mutex_lock(&iavf_device_mutex);
- list_for_each_entry_safe(ldev, tmp, &iavf_devices, list) {
- if (ldev->vf == adapter) {
- dev_info(&adapter->pdev->dev,
- "Deleted LAN device bus=0x%02x dev=0x%02x func=0x%02x\n",
- adapter->hw.bus.bus_id, adapter->hw.bus.device,
- adapter->hw.bus.func);
- list_del(&ldev->list);
- kfree(ldev);
- ret = 0;
- break;
- }
- }
-
- mutex_unlock(&iavf_device_mutex);
- return ret;
-}
-
-/**
- * iavf_client_release - release client specific resources
- * @client: pointer to the registered client
- *
- **/
-static void iavf_client_release(struct iavf_client *client)
-{
- struct iavf_client_instance *cinst;
- struct iavf_device *ldev;
- struct iavf_adapter *adapter;
-
- mutex_lock(&iavf_device_mutex);
- list_for_each_entry(ldev, &iavf_devices, list) {
- adapter = ldev->vf;
- cinst = adapter->cinst;
- if (!cinst)
- continue;
- if (test_bit(__IAVF_CLIENT_INSTANCE_OPENED, &cinst->state)) {
- if (client->ops && client->ops->close)
- client->ops->close(&cinst->lan_info, client,
- false);
- iavf_client_release_qvlist(&cinst->lan_info);
- clear_bit(__IAVF_CLIENT_INSTANCE_OPENED, &cinst->state);
-
- dev_warn(&adapter->pdev->dev,
- "Client %s instance closed\n", client->name);
- }
- /* delete the client instance */
- iavf_client_del_instance(adapter);
- dev_info(&adapter->pdev->dev, "Deleted client instance of Client %s\n",
- client->name);
- }
- mutex_unlock(&iavf_device_mutex);
-}
-
-/**
- * iavf_client_prepare - prepare client specific resources
- * @client: pointer to the registered client
- *
- **/
-static void iavf_client_prepare(struct iavf_client *client)
-{
- struct iavf_device *ldev;
- struct iavf_adapter *adapter;
-
- mutex_lock(&iavf_device_mutex);
- list_for_each_entry(ldev, &iavf_devices, list) {
- adapter = ldev->vf;
- /* Signal the watchdog to service the client */
- adapter->flags |= IAVF_FLAG_SERVICE_CLIENT_REQUESTED;
- }
- mutex_unlock(&iavf_device_mutex);
-}
-
-/**
- * iavf_client_virtchnl_send - send a message to the PF instance
- * @ldev: pointer to L2 context.
- * @client: Client pointer.
- * @msg: pointer to message buffer
- * @len: message length
- *
- * Return 0 on success or < 0 on error
- **/
-static u32 iavf_client_virtchnl_send(struct iavf_info *ldev,
- struct iavf_client *client,
- u8 *msg, u16 len)
-{
- struct iavf_adapter *adapter = ldev->vf;
- enum iavf_status err;
-
- if (adapter->aq_required)
- return -EAGAIN;
-
- err = iavf_aq_send_msg_to_pf(&adapter->hw, VIRTCHNL_OP_RDMA,
- IAVF_SUCCESS, msg, len, NULL);
- if (err)
- dev_err(&adapter->pdev->dev, "Unable to send RDMA message to PF, error %d, aq status %d\n",
- err, adapter->hw.aq.asq_last_status);
-
- return err;
-}
-
-/**
- * iavf_client_setup_qvlist - send a message to the PF to setup rdma qv map
- * @ldev: pointer to L2 context.
- * @client: Client pointer.
- * @qvlist_info: queue and vector list
- *
- * Return 0 on success or < 0 on error
- **/
-static int iavf_client_setup_qvlist(struct iavf_info *ldev,
- struct iavf_client *client,
- struct iavf_qvlist_info *qvlist_info)
-{
- struct virtchnl_rdma_qvlist_info *v_qvlist_info;
- struct iavf_adapter *adapter = ldev->vf;
- struct iavf_qv_info *qv_info;
- enum iavf_status err;
- u32 v_idx, i;
- size_t msg_size;
-
- if (adapter->aq_required)
- return -EAGAIN;
-
- /* A quick check on whether the vectors belong to the client */
- for (i = 0; i < qvlist_info->num_vectors; i++) {
- qv_info = &qvlist_info->qv_info[i];
- if (!qv_info)
- continue;
- v_idx = qv_info->v_idx;
- if ((v_idx >=
- (adapter->rdma_base_vector + adapter->num_rdma_msix)) ||
- (v_idx < adapter->rdma_base_vector))
- return -EINVAL;
- }
-
- v_qvlist_info = (struct virtchnl_rdma_qvlist_info *)qvlist_info;
- msg_size = virtchnl_struct_size(v_qvlist_info, qv_info,
- v_qvlist_info->num_vectors);
-
- adapter->client_pending |= BIT(VIRTCHNL_OP_CONFIG_RDMA_IRQ_MAP);
- err = iavf_aq_send_msg_to_pf(&adapter->hw,
- VIRTCHNL_OP_CONFIG_RDMA_IRQ_MAP, IAVF_SUCCESS,
- (u8 *)v_qvlist_info, msg_size, NULL);
-
- if (err) {
- dev_err(&adapter->pdev->dev,
- "Unable to send RDMA vector config message to PF, error %d, aq status %d\n",
- err, adapter->hw.aq.asq_last_status);
- goto out;
- }
-
- err = -EBUSY;
- for (i = 0; i < 5; i++) {
- msleep(100);
- if (!(adapter->client_pending &
- BIT(VIRTCHNL_OP_CONFIG_RDMA_IRQ_MAP))) {
- err = 0;
- break;
- }
- }
-out:
- return err;
-}
-
-/**
- * iavf_register_client - Register a iavf client driver with the L2 driver
- * @client: pointer to the iavf_client struct
- *
- * Returns 0 on success or non-0 on error
- **/
-int iavf_register_client(struct iavf_client *client)
-{
- int ret = 0;
-
- if (!client) {
- ret = -EIO;
- goto out;
- }
-
- if (strlen(client->name) == 0) {
- pr_info("iavf: Failed to register client with no name\n");
- ret = -EIO;
- goto out;
- }
-
- if (vf_registered_client) {
- pr_info("iavf: Client %s has already been registered!\n",
- client->name);
- ret = -EEXIST;
- goto out;
- }
-
- if ((client->version.major != IAVF_CLIENT_VERSION_MAJOR) ||
- (client->version.minor != IAVF_CLIENT_VERSION_MINOR)) {
- pr_info("iavf: Failed to register client %s due to mismatched client interface version\n",
- client->name);
- pr_info("Client is using version: %02d.%02d.%02d while LAN driver supports %s\n",
- client->version.major, client->version.minor,
- client->version.build,
- iavf_client_interface_version_str);
- ret = -EIO;
- goto out;
- }
-
- vf_registered_client = client;
-
- iavf_client_prepare(client);
-
- pr_info("iavf: Registered client %s with return code %d\n",
- client->name, ret);
-out:
- return ret;
-}
-EXPORT_SYMBOL(iavf_register_client);
-
-/**
- * iavf_unregister_client - Unregister a iavf client driver with the L2 driver
- * @client: pointer to the iavf_client struct
- *
- * Returns 0 on success or non-0 on error
- **/
-int iavf_unregister_client(struct iavf_client *client)
-{
- int ret = 0;
-
- /* When a unregister request comes through we would have to send
- * a close for each of the client instances that were opened.
- * client_release function is called to handle this.
- */
- iavf_client_release(client);
-
- if (vf_registered_client != client) {
- pr_info("iavf: Client %s has not been registered\n",
- client->name);
- ret = -ENODEV;
- goto out;
- }
- vf_registered_client = NULL;
- pr_info("iavf: Unregistered client %s\n", client->name);
-out:
- return ret;
-}
-EXPORT_SYMBOL(iavf_unregister_client);
diff --git a/drivers/net/ethernet/intel/iavf/iavf_client.h b/drivers/net/ethernet/intel/iavf/iavf_client.h
deleted file mode 100644
index 500269bc0f5b..000000000000
--- a/drivers/net/ethernet/intel/iavf/iavf_client.h
+++ /dev/null
@@ -1,169 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/* Copyright(c) 2013 - 2018 Intel Corporation. */
-
-#ifndef _IAVF_CLIENT_H_
-#define _IAVF_CLIENT_H_
-
-#define IAVF_CLIENT_STR_LENGTH 10
-
-/* Client interface version should be updated anytime there is a change in the
- * existing APIs or data structures.
- */
-#define IAVF_CLIENT_VERSION_MAJOR 0
-#define IAVF_CLIENT_VERSION_MINOR 01
-#define IAVF_CLIENT_VERSION_BUILD 00
-#define IAVF_CLIENT_VERSION_STR \
- __stringify(IAVF_CLIENT_VERSION_MAJOR) "." \
- __stringify(IAVF_CLIENT_VERSION_MINOR) "." \
- __stringify(IAVF_CLIENT_VERSION_BUILD)
-
-struct iavf_client_version {
- u8 major;
- u8 minor;
- u8 build;
- u8 rsvd;
-};
-
-enum iavf_client_state {
- __IAVF_CLIENT_NULL,
- __IAVF_CLIENT_REGISTERED
-};
-
-enum iavf_client_instance_state {
- __IAVF_CLIENT_INSTANCE_NONE,
- __IAVF_CLIENT_INSTANCE_OPENED,
-};
-
-struct iavf_ops;
-struct iavf_client;
-
-/* HW does not define a type value for AEQ; only for RX/TX and CEQ.
- * In order for us to keep the interface simple, SW will define a
- * unique type value for AEQ.
- */
-#define IAVF_QUEUE_TYPE_PE_AEQ 0x80
-#define IAVF_QUEUE_INVALID_IDX 0xFFFF
-
-struct iavf_qv_info {
- u32 v_idx; /* msix_vector */
- u16 ceq_idx;
- u16 aeq_idx;
- u8 itr_idx;
-};
-
-struct iavf_qvlist_info {
- u32 num_vectors;
- struct iavf_qv_info qv_info[];
-};
-
-#define IAVF_CLIENT_MSIX_ALL 0xFFFFFFFF
-
-/* set of LAN parameters useful for clients managed by LAN */
-
-/* Struct to hold per priority info */
-struct iavf_prio_qos_params {
- u16 qs_handle; /* qs handle for prio */
- u8 tc; /* TC mapped to prio */
- u8 reserved;
-};
-
-#define IAVF_CLIENT_MAX_USER_PRIORITY 8
-/* Struct to hold Client QoS */
-struct iavf_qos_params {
- struct iavf_prio_qos_params prio_qos[IAVF_CLIENT_MAX_USER_PRIORITY];
-};
-
-struct iavf_params {
- struct iavf_qos_params qos;
- u16 mtu;
- u16 link_up; /* boolean */
-};
-
-/* Structure to hold LAN device info for a client device */
-struct iavf_info {
- struct iavf_client_version version;
- u8 lanmac[6];
- struct net_device *netdev;
- struct pci_dev *pcidev;
- u8 __iomem *hw_addr;
- u8 fid; /* function id, PF id or VF id */
-#define IAVF_CLIENT_FTYPE_PF 0
-#define IAVF_CLIENT_FTYPE_VF 1
- u8 ftype; /* function type, PF or VF */
- void *vf; /* cast to iavf_adapter */
-
- /* All L2 params that could change during the life span of the device
- * and needs to be communicated to the client when they change
- */
- struct iavf_params params;
- struct iavf_ops *ops;
-
- u16 msix_count; /* number of msix vectors*/
- /* Array down below will be dynamically allocated based on msix_count */
- struct msix_entry *msix_entries;
- u16 itr_index; /* Which ITR index the PE driver is suppose to use */
-};
-
-struct iavf_ops {
- /* setup_q_vector_list enables queues with a particular vector */
- int (*setup_qvlist)(struct iavf_info *ldev, struct iavf_client *client,
- struct iavf_qvlist_info *qv_info);
-
- u32 (*virtchnl_send)(struct iavf_info *ldev, struct iavf_client *client,
- u8 *msg, u16 len);
-
- /* If the PE Engine is unresponsive, RDMA driver can request a reset.*/
- void (*request_reset)(struct iavf_info *ldev,
- struct iavf_client *client);
-};
-
-struct iavf_client_ops {
- /* Should be called from register_client() or whenever the driver is
- * ready to create a specific client instance.
- */
- int (*open)(struct iavf_info *ldev, struct iavf_client *client);
-
- /* Should be closed when netdev is unavailable or when unregister
- * call comes in. If the close happens due to a reset, set the reset
- * bit to true.
- */
- void (*close)(struct iavf_info *ldev, struct iavf_client *client,
- bool reset);
-
- /* called when some l2 managed parameters changes - mss */
- void (*l2_param_change)(struct iavf_info *ldev,
- struct iavf_client *client,
- struct iavf_params *params);
-
- /* called when a message is received from the PF */
- int (*virtchnl_receive)(struct iavf_info *ldev,
- struct iavf_client *client,
- u8 *msg, u16 len);
-};
-
-/* Client device */
-struct iavf_client_instance {
- struct list_head list;
- struct iavf_info lan_info;
- struct iavf_client *client;
- unsigned long state;
-};
-
-struct iavf_client {
- struct list_head list; /* list of registered clients */
- char name[IAVF_CLIENT_STR_LENGTH];
- struct iavf_client_version version;
- unsigned long state; /* client state */
- atomic_t ref_cnt; /* Count of all the client devices of this kind */
- u32 flags;
-#define IAVF_CLIENT_FLAGS_LAUNCH_ON_PROBE BIT(0)
-#define IAVF_TX_FLAGS_NOTIFY_OTHER_EVENTS BIT(2)
- u8 type;
-#define IAVF_CLIENT_RDMA 0
- struct iavf_client_ops *ops; /* client ops provided by the client */
-};
-
-/* used by clients */
-int iavf_register_client(struct iavf_client *client);
-int iavf_unregister_client(struct iavf_client *client);
-#endif /* _IAVF_CLIENT_H_ */
diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index 49a764ca5e36..6e27b7938b8a 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -3,7 +3,6 @@
#include "iavf.h"
#include "iavf_prototype.h"
-#include "iavf_client.h"
/* All iavf tracepoints are defined by the include below, which must
* be included exactly once across the whole kernel with
* CREATE_TRACE_POINTS defined
@@ -1286,8 +1285,6 @@ static void iavf_up_complete(struct iavf_adapter *adapter)
iavf_napi_enable_all(adapter);
adapter->aq_required |= IAVF_FLAG_AQ_ENABLE_QUEUES;
- if (CLIENT_ENABLED(adapter))
- adapter->flags |= IAVF_FLAG_CLIENT_NEEDS_OPEN;
mod_delayed_work(adapter->wq, &adapter->watchdog_task, 0);
}
@@ -2706,12 +2703,6 @@ static void iavf_init_config_adapter(struct iavf_adapter *adapter)
adapter->link_up = false;
netif_tx_stop_all_queues(netdev);
- if (CLIENT_ALLOWED(adapter)) {
- err = iavf_lan_add_device(adapter);
- if (err)
- dev_info(&pdev->dev, "Failed to add VF to client API service list: %d\n",
- err);
- }
dev_info(&pdev->dev, "MAC address: %pM\n", adapter->hw.mac.addr);
if (netdev->features & NETIF_F_GRO)
dev_info(&pdev->dev, "GRO is enabled\n");
@@ -2908,7 +2899,6 @@ static void iavf_watchdog_task(struct work_struct *work)
return;
}
- schedule_delayed_work(&adapter->client_task, msecs_to_jiffies(5));
mutex_unlock(&adapter->crit_lock);
restart_watchdog:
if (adapter->state >= __IAVF_DOWN)
@@ -3019,15 +3009,6 @@ static void iavf_reset_task(struct work_struct *work)
return;
}
- mutex_lock(&adapter->client_lock);
- if (CLIENT_ENABLED(adapter)) {
- adapter->flags &= ~(IAVF_FLAG_CLIENT_NEEDS_OPEN |
- IAVF_FLAG_CLIENT_NEEDS_CLOSE |
- IAVF_FLAG_CLIENT_NEEDS_L2_PARAMS |
- IAVF_FLAG_SERVICE_CLIENT_REQUESTED);
- cancel_delayed_work_sync(&adapter->client_task);
- iavf_notify_client_close(&adapter->vsi, true);
- }
iavf_misc_irq_disable(adapter);
if (adapter->flags & IAVF_FLAG_RESET_NEEDED) {
adapter->flags &= ~IAVF_FLAG_RESET_NEEDED;
@@ -3071,7 +3052,6 @@ static void iavf_reset_task(struct work_struct *work)
dev_err(&adapter->pdev->dev, "Reset never finished (%x)\n",
reg_val);
iavf_disable_vf(adapter);
- mutex_unlock(&adapter->client_lock);
mutex_unlock(&adapter->crit_lock);
return; /* Do not attempt to reinit. It's dead, Jim. */
}
@@ -3210,7 +3190,6 @@ static void iavf_reset_task(struct work_struct *work)
adapter->flags &= ~IAVF_FLAG_REINIT_ITR_NEEDED;
wake_up(&adapter->reset_waitqueue);
- mutex_unlock(&adapter->client_lock);
mutex_unlock(&adapter->crit_lock);
return;
@@ -3221,7 +3200,6 @@ static void iavf_reset_task(struct work_struct *work)
}
iavf_disable_vf(adapter);
- mutex_unlock(&adapter->client_lock);
mutex_unlock(&adapter->crit_lock);
dev_err(&adapter->pdev->dev, "failed to allocate resources during reinit\n");
}
@@ -3320,48 +3298,6 @@ static void iavf_adminq_task(struct work_struct *work)
iavf_misc_irq_enable(adapter);
}
-/**
- * iavf_client_task - worker thread to perform client work
- * @work: pointer to work_struct containing our data
- *
- * This task handles client interactions. Because client calls can be
- * reentrant, we can't handle them in the watchdog.
- **/
-static void iavf_client_task(struct work_struct *work)
-{
- struct iavf_adapter *adapter =
- container_of(work, struct iavf_adapter, client_task.work);
-
- /* If we can't get the client bit, just give up. We'll be rescheduled
- * later.
- */
-
- if (!mutex_trylock(&adapter->client_lock))
- return;
-
- if (adapter->flags & IAVF_FLAG_SERVICE_CLIENT_REQUESTED) {
- iavf_client_subtask(adapter);
- adapter->flags &= ~IAVF_FLAG_SERVICE_CLIENT_REQUESTED;
- goto out;
- }
- if (adapter->flags & IAVF_FLAG_CLIENT_NEEDS_L2_PARAMS) {
- iavf_notify_client_l2_params(&adapter->vsi);
- adapter->flags &= ~IAVF_FLAG_CLIENT_NEEDS_L2_PARAMS;
- goto out;
- }
- if (adapter->flags & IAVF_FLAG_CLIENT_NEEDS_CLOSE) {
- iavf_notify_client_close(&adapter->vsi, false);
- adapter->flags &= ~IAVF_FLAG_CLIENT_NEEDS_CLOSE;
- goto out;
- }
- if (adapter->flags & IAVF_FLAG_CLIENT_NEEDS_OPEN) {
- iavf_notify_client_open(&adapter->vsi);
- adapter->flags &= ~IAVF_FLAG_CLIENT_NEEDS_OPEN;
- }
-out:
- mutex_unlock(&adapter->client_lock);
-}
-
/**
* iavf_free_all_tx_resources - Free Tx Resources for All Queues
* @adapter: board private structure
@@ -4294,8 +4230,6 @@ static int iavf_close(struct net_device *netdev)
}
set_bit(__IAVF_VSI_DOWN, adapter->vsi.state);
- if (CLIENT_ENABLED(adapter))
- adapter->flags |= IAVF_FLAG_CLIENT_NEEDS_CLOSE;
/* We cannot send IAVF_FLAG_AQ_GET_OFFLOAD_VLAN_V2_CAPS before
* IAVF_FLAG_AQ_DISABLE_QUEUES because in such case there is rtnl
* deadlock with adminq_task() until iavf_close timeouts. We must send
@@ -4364,10 +4298,6 @@ static int iavf_change_mtu(struct net_device *netdev, int new_mtu)
netdev_dbg(netdev, "changing MTU from %d to %d\n",
netdev->mtu, new_mtu);
netdev->mtu = new_mtu;
- if (CLIENT_ENABLED(adapter)) {
- iavf_notify_client_l2_params(&adapter->vsi);
- adapter->flags |= IAVF_FLAG_SERVICE_CLIENT_REQUESTED;
- }
if (netif_running(netdev)) {
iavf_schedule_reset(adapter, IAVF_FLAG_RESET_NEEDED);
@@ -5011,7 +4941,6 @@ static int iavf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
* and destroy them only once in remove
*/
mutex_init(&adapter->crit_lock);
- mutex_init(&adapter->client_lock);
mutex_init(&hw->aq.asq_mutex);
mutex_init(&hw->aq.arq_mutex);
@@ -5031,7 +4960,6 @@ static int iavf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
INIT_WORK(&adapter->adminq_task, iavf_adminq_task);
INIT_WORK(&adapter->finish_config, iavf_finish_config);
INIT_DELAYED_WORK(&adapter->watchdog_task, iavf_watchdog_task);
- INIT_DELAYED_WORK(&adapter->client_task, iavf_client_task);
queue_delayed_work(adapter->wq, &adapter->watchdog_task,
msecs_to_jiffies(5 * (pdev->devfn & 0x07)));
@@ -5142,7 +5070,6 @@ static void iavf_remove(struct pci_dev *pdev)
struct iavf_mac_filter *f, *ftmp;
struct net_device *netdev;
struct iavf_hw *hw;
- int err;
netdev = adapter->netdev;
hw = &adapter->hw;
@@ -5176,13 +5103,6 @@ static void iavf_remove(struct pci_dev *pdev)
if (netdev->reg_state == NETREG_REGISTERED)
unregister_netdev(netdev);
- if (CLIENT_ALLOWED(adapter)) {
- err = iavf_lan_del_device(adapter);
- if (err)
- dev_warn(&pdev->dev, "Failed to delete client device: %d\n",
- err);
- }
-
mutex_lock(&adapter->crit_lock);
dev_info(&adapter->pdev->dev, "Removing device\n");
iavf_change_state(adapter, __IAVF_REMOVE);
@@ -5200,7 +5120,6 @@ static void iavf_remove(struct pci_dev *pdev)
cancel_work_sync(&adapter->reset_task);
cancel_delayed_work_sync(&adapter->watchdog_task);
cancel_work_sync(&adapter->adminq_task);
- cancel_delayed_work_sync(&adapter->client_task);
adapter->aq_required = 0;
adapter->flags &= ~IAVF_FLAG_REINIT_ITR_NEEDED;
@@ -5218,7 +5137,6 @@ static void iavf_remove(struct pci_dev *pdev)
/* destroy the locks only once, here */
mutex_destroy(&hw->aq.arq_mutex);
mutex_destroy(&hw->aq.asq_mutex);
- mutex_destroy(&adapter->client_lock);
mutex_unlock(&adapter->crit_lock);
mutex_destroy(&adapter->crit_lock);
diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
index 82b84a93bcc8..64c4443dbef9 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
@@ -3,7 +3,6 @@
#include "iavf.h"
#include "iavf_prototype.h"
-#include "iavf_client.h"
/**
* iavf_send_pf_msg
@@ -2309,19 +2308,6 @@ void iavf_virtchnl_completion(struct iavf_adapter *adapter,
if (v_opcode != adapter->current_op)
return;
break;
- case VIRTCHNL_OP_RDMA:
- /* Gobble zero-length replies from the PF. They indicate that
- * a previous message was received OK, and the client doesn't
- * care about that.
- */
- if (msglen && CLIENT_ENABLED(adapter))
- iavf_notify_client_message(&adapter->vsi, msg, msglen);
- break;
-
- case VIRTCHNL_OP_CONFIG_RDMA_IRQ_MAP:
- adapter->client_pending &=
- ~(BIT(VIRTCHNL_OP_CONFIG_RDMA_IRQ_MAP));
- break;
case VIRTCHNL_OP_GET_RSS_HENA_CAPS: {
struct virtchnl_rss_hena *vrh = (struct virtchnl_rss_hena *)msg;
--
2.41.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH net-next 0/9] Intel Wired LAN Driver Updates for 2023-10-23 (iavf)
2023-10-23 23:08 [PATCH net-next 0/9] Intel Wired LAN Driver Updates for 2023-10-23 (iavf) Jacob Keller
` (9 preceding siblings ...)
2023-10-23 23:08 ` [PATCH net-next 9/9] iavf: delete the iavf client interface Jacob Keller
@ 2023-10-27 16:58 ` Jacob Keller
2023-10-27 17:41 ` Jakub Kicinski
10 siblings, 1 reply; 19+ messages in thread
From: Jacob Keller @ 2023-10-27 16:58 UTC (permalink / raw)
To: netdev, David Miller, Jakub Kicinski
On 10/23/2023 4:08 PM, Jacob Keller wrote:
> This series includes iAVF driver cleanups from Michal Schmidt.
>
> Michal removes and updates stale comments, fixes some locking anti-patterns,
> improves handling of resets when the PF is slow, avoids unnecessary
> duplication of netdev state, refactors away some duplicate code, and finally
> removes the never-actually-used client interface.
>
> Michal Schmidt (9):
> iavf: fix comments about old bit locks
> iavf: simplify mutex_trylock+sleep loops
> iavf: in iavf_down, don't queue watchdog_task if comms failed
> iavf: in iavf_down, disable queues when removing the driver
> iavf: fix the waiting time for initial reset
> iavf: rely on netdev's own registered state
> iavf: use unregister_netdev
> iavf: add a common function for undoing the interrupt scheme
> iavf: delete the iavf client interface
>
> drivers/net/ethernet/intel/iavf/Makefile | 2 +-
> drivers/net/ethernet/intel/iavf/iavf.h | 28 -
> drivers/net/ethernet/intel/iavf/iavf_client.c | 578 ------------------
> drivers/net/ethernet/intel/iavf/iavf_client.h | 169 -----
> drivers/net/ethernet/intel/iavf/iavf_main.c | 139 +----
> .../net/ethernet/intel/iavf/iavf_virtchnl.c | 14 -
> 6 files changed, 29 insertions(+), 901 deletions(-)
> delete mode 100644 drivers/net/ethernet/intel/iavf/iavf_client.c
> delete mode 100644 drivers/net/ethernet/intel/iavf/iavf_client.h
>
>
> base-commit: d6e48462e88fe7efc78b455ecde5b0ca43ec50b7
Since we pulled the fix out into net, I assume I should resend a rebased
v2 of this series?
Thanks,
Jake
^ permalink raw reply [flat|nested] 19+ messages in thread