Re: [PATCHv2 net-next-2.6 2/2] qlcnic: Take FW dump via ethtool

On Tue, 2011-05-10 at 14:00 -0700, Anirban Chakraborty wrote:

On May 10, 2011, at 11:40 AM, Ben Hutchings wrote:

quoted

On Mon, 2011-05-09 at 18:02 -0700, Anirban Chakraborty wrote:

quoted

Driver checks if the previous dump has been cleared before taking the dump.
It doesn't take the dump if it is not cleared.

Signed-off-by: Anirban Chakraborty <redacted>
---
drivers/net/qlcnic/qlcnic_ethtool.c |   60 +++++++++++++++++++++++++++++++++++
1 files changed, 60 insertions(+), 0 deletions(-)

diff --git a/drivers/net/qlcnic/qlcnic_ethtool.c b/drivers/net/qlcnic/qlcnic_ethtool.c
index c541461..1237449 100644
--- a/drivers/net/qlcnic/qlcnic_ethtool.c
+++ b/drivers/net/qlcnic/qlcnic_ethtool.c

@@ -965,6 +965,64 @@ static void qlcnic_set_msglevel(struct net_device *netdev, u32 msglvl)

	adapter->msg_enable = msglvl;
}

+static int
+qlcnic_get_dump(struct net_device *netdev, struct ethtool_dump *dump,
+		void *buffer)
+{
+	int i, copy_sz;
+	u32 *hdr_ptr, *data;
+	struct qlcnic_adapter *adapter = netdev_priv(netdev);
+	struct qlcnic_fw_dump *fw_dump = &adapter->ahw->fw_dump;
+
+	if (dump->type == ETHTOOL_DUMP_FLAG) {
+		dump->len = fw_dump->tmpl_hdr->size + fw_dump->size;
+		dump->flag = fw_dump->tmpl_hdr->drv_cap_mask;
+		return 0;
+	}
+	if (!fw_dump->clr) {
+		netdev_info(netdev, "Dump not available\n");
+		return -EINVAL;
+	}
+	copy_sz = fw_dump->tmpl_hdr->size;
+	/* Copy template header first */
+	hdr_ptr = (u32 *) fw_dump->tmpl_hdr;
+	data = (u32 *) buffer;
+	for (i = 0; i < copy_sz/sizeof(u32); i++)
+		*data++ = cpu_to_le32(*hdr_ptr++);
+	/* Copy captured dump data */
+	memcpy(buffer + copy_sz, fw_dump->data, fw_dump->size);
+	dump->len = copy_sz + fw_dump->size;
+	dump->flag = fw_dump->tmpl_hdr->drv_cap_mask;
+	/* free dump area once the whoel dump data has been captured */
+	vfree(fw_dump->data);
+	fw_dump->size = 0;
+	fw_dump->data = NULL;
+	fw_dump->clr = 0;

This doesn't seem to be serialised with the code that captures firmware
dumps.  They need to use the same lock!

When we take the dump, we bring down the interface. So, ethtool will not get a chance to
fetch the dump data while the dump operation is in progress.

[...]

The ethtool core generally doesn't care whether the interface is
running.  Its operations are serialised only by the RTNL lock.  The work
item you added to extract a dump from the NIC does not take that lock.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.