Netdev List
 help / color / mirror / Atom feed
* [PATCH 07/28] bnxt_en: Add interface to support RDMA driver.
From: Selvin Xavier @ 2016-12-05  6:38 UTC (permalink / raw)
  To: dledford; +Cc: linux-rdma, Michael Chan, David Miller, netdev, Selvin Xavier
In-Reply-To: <1480919912-1079-1-git-send-email-selvin.xavier@broadcom.com>

From: Michael Chan <michael.chan@broadcom.com>

Since the network driver and RDMA driver operate on the same PCI function,
we need to create an interface to allow the RDMA driver to share resources
with the network driver.

1. Create a new bnxt_en_dev struct which will be returned by
bnxt_ulp_probe() upon success.  After that, all calls from the RDMA driver
to bnxt_en will pass a pointer to this struct.

2. This struct contains additional function pointers to register, request
msix, send fw messages, register for async events.

3. If the RDMA driver wants to enable RDMA on the function, it needs to
call the function pointer bnxt_register_device().  A ulp_ops structure
is passed for upcalls from bnxt_en to the RDMA driver.

4. MSIX is requested by calling the function pointer bnxt_request_msix().

5. The RMDA driver can call firmware APIs using the bnxt_send_fw_msg()
function pointer.

6. 1 stats context is reserved when the RDMA driver registers.  MSIX
and completion rings are reserved when the RDMA driver requests for
MSIX.

7. When the RDMA driver calls bnxt_unregister_device(), all RDMA resources
will be cleaned up.

Cc: David Miller <davem@davemloft.net>
Cc: <netdev@vger.kernel.org>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/Makefile   |   2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt.c     |  34 ++-
 drivers/net/ethernet/broadcom/bnxt/bnxt.h     |   6 +
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 288 ++++++++++++++++++++++++++
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h |  91 ++++++++
 5 files changed, 417 insertions(+), 4 deletions(-)
 create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
 create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h

diff --git a/drivers/net/ethernet/broadcom/bnxt/Makefile b/drivers/net/ethernet/broadcom/bnxt/Makefile
index b233a86..6082ed1 100644
--- a/drivers/net/ethernet/broadcom/bnxt/Makefile
+++ b/drivers/net/ethernet/broadcom/bnxt/Makefile
@@ -1,3 +1,3 @@
 obj-$(CONFIG_BNXT) += bnxt_en.o
 
-bnxt_en-y := bnxt.o bnxt_sriov.o bnxt_ethtool.o bnxt_dcb.o
+bnxt_en-y := bnxt.o bnxt_sriov.o bnxt_ethtool.o bnxt_dcb.o bnxt_ulp.o
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index c26735ea..19f26b8 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -52,6 +52,7 @@
 
 #include "bnxt_hsi.h"
 #include "bnxt.h"
+#include "bnxt_ulp.h"
 #include "bnxt_sriov.h"
 #include "bnxt_ethtool.h"
 #include "bnxt_dcb.h"
@@ -1528,12 +1529,11 @@ static int bnxt_async_event_process(struct bnxt *bp,
 		set_bit(BNXT_RESET_TASK_SILENT_SP_EVENT, &bp->sp_event);
 		break;
 	default:
-		netdev_err(bp->dev, "unhandled ASYNC event (id 0x%x)\n",
-			   event_id);
 		goto async_event_process_exit;
 	}
 	schedule_work(&bp->sp_task);
 async_event_process_exit:
+	bnxt_ulp_async_events(bp, cmpl);
 	return 0;
 }
 
@@ -3547,7 +3547,7 @@ static int bnxt_hwrm_vnic_ctx_alloc(struct bnxt *bp, u16 vnic_id, u16 ctx_idx)
 	return rc;
 }
 
-static int bnxt_hwrm_vnic_cfg(struct bnxt *bp, u16 vnic_id)
+int bnxt_hwrm_vnic_cfg(struct bnxt *bp, u16 vnic_id)
 {
 	unsigned int ring = 0, grp_idx;
 	struct bnxt_vnic_info *vnic = &bp->vnic_info[vnic_id];
@@ -3595,6 +3595,9 @@ static int bnxt_hwrm_vnic_cfg(struct bnxt *bp, u16 vnic_id)
 #endif
 	if ((bp->flags & BNXT_FLAG_STRIP_VLAN) || def_vlan)
 		req.flags |= cpu_to_le32(VNIC_CFG_REQ_FLAGS_VLAN_STRIP_MODE);
+	if (!vnic_id && bnxt_ulp_registered(bp->edev, BNXT_ROCE_ULP))
+		req.flags |=
+			cpu_to_le32(VNIC_CFG_REQ_FLAGS_ROCE_DUAL_VNIC_MODE);
 
 	return hwrm_send_message(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
 }
@@ -4842,6 +4845,16 @@ unsigned int bnxt_get_max_func_stat_ctxs(struct bnxt *bp)
 #endif
 }
 
+void bnxt_set_max_func_stat_ctxs(struct bnxt *bp, unsigned int max)
+{
+	if (BNXT_PF(bp))
+		bp->pf.max_stat_ctxs = max;
+#if defined(CONFIG_BNXT_SRIOV)
+	else
+		bp->vf.max_stat_ctxs = max;
+#endif
+}
+
 unsigned int bnxt_get_max_func_cp_rings(struct bnxt *bp)
 {
 	if (BNXT_PF(bp))
@@ -4851,6 +4864,16 @@ unsigned int bnxt_get_max_func_cp_rings(struct bnxt *bp)
 #endif
 }
 
+void bnxt_set_max_func_cp_rings(struct bnxt *bp, unsigned int max)
+{
+	if (BNXT_PF(bp))
+		bp->pf.max_cp_rings = max;
+#if defined(CONFIG_BNXT_SRIOV)
+	else
+		bp->vf.max_cp_rings = max;
+#endif
+}
+
 static unsigned int bnxt_get_max_func_irqs(struct bnxt *bp)
 {
 	if (BNXT_PF(bp))
@@ -6767,6 +6790,8 @@ static void bnxt_remove_one(struct pci_dev *pdev)
 	pci_iounmap(pdev, bp->bar2);
 	pci_iounmap(pdev, bp->bar1);
 	pci_iounmap(pdev, bp->bar0);
+	kfree(bp->edev);
+	bp->edev = NULL;
 	free_netdev(dev);
 
 	pci_release_regions(pdev);
@@ -6936,6 +6961,7 @@ void bnxt_restore_pf_fw_resources(struct bnxt *bp)
 {
 	ASSERT_RTNL();
 	bnxt_hwrm_func_qcaps(bp);
+	bnxt_subtract_ulp_resources(bp, BNXT_ROCE_ULP);
 }
 
 static void bnxt_parse_log_pcie_link(struct bnxt *bp)
@@ -7047,6 +7073,8 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (rc)
 		goto init_err;
 
+	bp->ulp_probe = bnxt_ulp_probe;
+
 	/* Get the MAX capabilities for this function */
 	rc = bnxt_hwrm_func_qcaps(bp);
 	if (rc) {
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index eec2415..16defe9 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -972,6 +972,9 @@ struct bnxt {
 #define BNXT_SINGLE_PF(bp)	(BNXT_PF(bp) && !BNXT_NPAR(bp))
 #define BNXT_CHIP_TYPE_NITRO_A0(bp) ((bp)->flags & BNXT_FLAG_CHIP_NITRO_A0)
 
+	struct bnxt_en_dev	*edev;
+	struct bnxt_en_dev *	(*ulp_probe)(struct net_device *);
+
 	struct bnxt_napi	**bnapi;
 
 	struct bnxt_rx_ring_info	*rx_ring;
@@ -1242,9 +1245,12 @@ int hwrm_send_message(struct bnxt *, void *, u32, int);
 int hwrm_send_message_silent(struct bnxt *, void *, u32, int);
 int bnxt_hwrm_func_rgtr_async_events(struct bnxt *bp, unsigned long *bmap,
 				     int bmap_size);
+int bnxt_hwrm_vnic_cfg(struct bnxt *bp, u16 vnic_id);
 int bnxt_hwrm_set_coal(struct bnxt *);
 unsigned int bnxt_get_max_func_stat_ctxs(struct bnxt *bp);
+void bnxt_set_max_func_stat_ctxs(struct bnxt *bp, unsigned int max);
 unsigned int bnxt_get_max_func_cp_rings(struct bnxt *bp);
+void bnxt_set_max_func_cp_rings(struct bnxt *bp, unsigned int max);
 void bnxt_set_max_func_irqs(struct bnxt *bp, unsigned int max);
 void bnxt_tx_disable(struct bnxt *bp);
 void bnxt_tx_enable(struct bnxt *bp);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
new file mode 100644
index 0000000..a018f4b
--- /dev/null
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
@@ -0,0 +1,288 @@
+/* Broadcom NetXtreme-C/E network driver.
+ *
+ * Copyright (c) 2016 Broadcom Limited
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/interrupt.h>
+#include <linux/pci.h>
+#include <linux/netdevice.h>
+#include <linux/rtnetlink.h>
+#include <linux/bitops.h>
+#include <linux/irq.h>
+#include <asm/byteorder.h>
+#include <linux/bitmap.h>
+
+#include "bnxt_hsi.h"
+#include "bnxt.h"
+#include "bnxt_ulp.h"
+
+static int bnxt_register_dev(struct bnxt_en_dev *edev, int ulp_id,
+			     struct bnxt_ulp_ops *ulp_ops, void *handle)
+{
+	struct net_device *dev = edev->net;
+	struct bnxt *bp = netdev_priv(dev);
+	struct bnxt_ulp *ulp;
+
+	ASSERT_RTNL();
+	if (ulp_id >= BNXT_MAX_ULP)
+		return -EINVAL;
+
+	ulp = &edev->ulp_tbl[ulp_id];
+	if (rcu_access_pointer(ulp->ulp_ops)) {
+		netdev_err(bp->dev, "ulp id %d already registered\n", ulp_id);
+		return -EBUSY;
+	}
+	if (ulp_id == BNXT_ROCE_ULP) {
+		unsigned int max_stat_ctxs;
+
+		max_stat_ctxs = bnxt_get_max_func_stat_ctxs(bp);
+		if (max_stat_ctxs <= BNXT_MIN_ROCE_STAT_CTXS ||
+		    bp->num_stat_ctxs == max_stat_ctxs)
+			return -ENOMEM;
+		bnxt_set_max_func_stat_ctxs(bp, max_stat_ctxs -
+					    BNXT_MIN_ROCE_STAT_CTXS);
+	}
+
+	atomic_set(&ulp->ref_count, 0);
+	ulp->handle = handle;
+	rcu_assign_pointer(ulp->ulp_ops, ulp_ops);
+
+	if (ulp_id == BNXT_ROCE_ULP) {
+		if (test_bit(BNXT_STATE_OPEN, &bp->state))
+			bnxt_hwrm_vnic_cfg(bp, 0);
+	}
+
+	return 0;
+}
+
+static int bnxt_unregister_dev(struct bnxt_en_dev *edev, int ulp_id)
+{
+	struct net_device *dev = edev->net;
+	struct bnxt *bp = netdev_priv(dev);
+	struct bnxt_ulp *ulp;
+
+	ASSERT_RTNL();
+	if (ulp_id >= BNXT_MAX_ULP)
+		return -EINVAL;
+
+	ulp = &edev->ulp_tbl[ulp_id];
+	if (!rcu_access_pointer(ulp->ulp_ops)) {
+		netdev_err(bp->dev, "ulp id %d not registered\n", ulp_id);
+		return -EINVAL;
+	}
+	if (ulp_id == BNXT_ROCE_ULP) {
+		unsigned int max_stat_ctxs;
+
+		max_stat_ctxs = bnxt_get_max_func_stat_ctxs(bp);
+		bnxt_set_max_func_stat_ctxs(bp, max_stat_ctxs + 1);
+	}
+	if (ulp->max_async_event_id)
+		bnxt_hwrm_func_rgtr_async_events(bp, NULL, 0);
+
+	RCU_INIT_POINTER(ulp->ulp_ops, NULL);
+	synchronize_rcu();
+	ulp->max_async_event_id = 0;
+	ulp->async_events_bmap = NULL;
+	return 0;
+}
+
+static int bnxt_req_msix_vecs(struct bnxt_en_dev *edev, int ulp_id,
+			      struct bnxt_msix_entry *ent, int num_msix)
+{
+	struct net_device *dev = edev->net;
+	struct bnxt *bp = netdev_priv(dev);
+	int max_idx, max_cp_rings;
+	int avail_msix, i, idx;
+
+	ASSERT_RTNL();
+	if (ulp_id != BNXT_ROCE_ULP)
+		return -EINVAL;
+
+	if (!(bp->flags & BNXT_FLAG_USING_MSIX))
+		return -ENODEV;
+
+	max_cp_rings = bnxt_get_max_func_cp_rings(bp);
+	max_idx = min_t(int, bp->total_irqs, max_cp_rings);
+	avail_msix = max_idx - bp->cp_nr_rings;
+	if (!avail_msix)
+		return -ENOMEM;
+	if (avail_msix > num_msix)
+		avail_msix = num_msix;
+
+	idx = max_idx - avail_msix;
+	for (i = 0; i < avail_msix; i++) {
+		ent[i].vector = bp->irq_tbl[idx + i].vector;
+		ent[i].ring_idx = idx + i;
+		ent[i].db_offset = (idx + i) * 0x80;
+	}
+	bnxt_set_max_func_irqs(bp, max_idx - avail_msix);
+	bnxt_set_max_func_cp_rings(bp, max_cp_rings - avail_msix);
+	edev->ulp_tbl[ulp_id].msix_requested = avail_msix;
+	return avail_msix;
+}
+
+static int bnxt_free_msix_vecs(struct bnxt_en_dev *edev, int ulp_id)
+{
+	struct net_device *dev = edev->net;
+	struct bnxt *bp = netdev_priv(dev);
+	int max_cp_rings, msix_requested;
+
+	ASSERT_RTNL();
+	if (ulp_id != BNXT_ROCE_ULP)
+		return -EINVAL;
+
+	max_cp_rings = bnxt_get_max_func_cp_rings(bp);
+	msix_requested = edev->ulp_tbl[ulp_id].msix_requested;
+	bnxt_set_max_func_cp_rings(bp, max_cp_rings + msix_requested);
+	edev->ulp_tbl[ulp_id].msix_requested = 0;
+	bnxt_set_max_func_irqs(bp, bp->total_irqs);
+	return 0;
+}
+
+void bnxt_subtract_ulp_resources(struct bnxt *bp, int ulp_id)
+{
+	ASSERT_RTNL();
+	if (bnxt_ulp_registered(bp->edev, ulp_id)) {
+		struct bnxt_en_dev *edev = bp->edev;
+		unsigned int msix_req, max;
+
+		msix_req = edev->ulp_tbl[ulp_id].msix_requested;
+		max = bnxt_get_max_func_cp_rings(bp);
+		bnxt_set_max_func_cp_rings(bp, max - msix_req);
+		max = bnxt_get_max_func_stat_ctxs(bp);
+		bnxt_set_max_func_stat_ctxs(bp, max - 1);
+	}
+}
+
+static int bnxt_send_msg(struct bnxt_en_dev *edev, int ulp_id,
+			 struct bnxt_fw_msg *fw_msg)
+{
+	struct net_device *dev = edev->net;
+	struct bnxt *bp = netdev_priv(dev);
+	struct input *req;
+	int rc;
+
+	mutex_lock(&bp->hwrm_cmd_lock);
+	req = fw_msg->msg;
+	req->resp_addr = cpu_to_le64(bp->hwrm_cmd_resp_dma_addr);
+	rc = _hwrm_send_message(bp, fw_msg->msg, fw_msg->msg_len,
+				fw_msg->timeout);
+	if (!rc) {
+		struct output *resp = bp->hwrm_cmd_resp_addr;
+		u32 len = le16_to_cpu(resp->resp_len);
+
+		if (fw_msg->resp_max_len < len)
+			len = fw_msg->resp_max_len;
+
+		memcpy(fw_msg->resp, resp, len);
+	}
+	mutex_unlock(&bp->hwrm_cmd_lock);
+	return rc;
+}
+
+void bnxt_ulp_stop(struct bnxt *bp)
+{
+	struct bnxt_en_dev *edev = bp->edev;
+	struct bnxt_ulp_ops *ops;
+	int i;
+
+	if (!edev)
+		return;
+
+	rcu_read_lock();
+	for (i = 0; i < BNXT_MAX_ULP; i++) {
+		struct bnxt_ulp *ulp = &edev->ulp_tbl[i];
+
+		ops = rcu_dereference(ulp->ulp_ops);
+		if (!ops || !ops->ulp_stop)
+			continue;
+	}
+	rcu_read_unlock();
+}
+
+void bnxt_ulp_async_events(struct bnxt *bp, struct hwrm_async_event_cmpl *cmpl)
+{
+	u16 event_id = le16_to_cpu(cmpl->event_id);
+	struct bnxt_en_dev *edev = bp->edev;
+	struct bnxt_ulp_ops *ops;
+	int i;
+
+	if (!edev)
+		return;
+
+	rcu_read_lock();
+	for (i = 0; i < BNXT_MAX_ULP; i++) {
+		struct bnxt_ulp *ulp = &edev->ulp_tbl[i];
+
+		ops = rcu_dereference(ulp->ulp_ops);
+		if (!ops || !ops->ulp_async_notifier)
+			continue;
+		if (!ulp->async_events_bmap ||
+		    event_id > ulp->max_async_event_id)
+			continue;
+
+		/* Read max_async_event_id first before testing the bitmap. */
+		smp_rmb();
+		if (test_bit(event_id, ulp->async_events_bmap))
+			ops->ulp_async_notifier(ulp->handle, cmpl);
+	}
+	rcu_read_unlock();
+}
+
+static int bnxt_register_async_events(struct bnxt_en_dev *edev, int ulp_id,
+				      unsigned long *events_bmap, u16 max_id)
+{
+	struct net_device *dev = edev->net;
+	struct bnxt *bp = netdev_priv(dev);
+	struct bnxt_ulp *ulp;
+
+	if (ulp_id >= BNXT_MAX_ULP)
+		return -EINVAL;
+
+	ulp = &edev->ulp_tbl[ulp_id];
+	ulp->async_events_bmap = events_bmap;
+	/* Make sure bnxt_ulp_async_events() sees this order */
+	smp_wmb();
+	ulp->max_async_event_id = max_id;
+	bnxt_hwrm_func_rgtr_async_events(bp, events_bmap, max_id + 1);
+	return 0;
+}
+
+static const struct bnxt_en_ops bnxt_en_ops_tbl = {
+	.bnxt_register_device	= bnxt_register_dev,
+	.bnxt_unregister_device	= bnxt_unregister_dev,
+	.bnxt_request_msix	= bnxt_req_msix_vecs,
+	.bnxt_free_msix		= bnxt_free_msix_vecs,
+	.bnxt_send_fw_msg	= bnxt_send_msg,
+	.bnxt_register_fw_async_events	= bnxt_register_async_events,
+};
+
+struct bnxt_en_dev *bnxt_ulp_probe(struct net_device *dev)
+{
+	struct bnxt *bp = netdev_priv(dev);
+	struct bnxt_en_dev *edev;
+
+	edev = bp->edev;
+	if (!edev) {
+		edev = kzalloc(sizeof(*edev), GFP_KERNEL);
+		if (!edev)
+			return ERR_PTR(-ENOMEM);
+		edev->en_ops = &bnxt_en_ops_tbl;
+		if (bp->flags & BNXT_FLAG_ROCEV1_CAP)
+			edev->flags |= BNXT_EN_FLAG_ROCEV1_CAP;
+		if (bp->flags & BNXT_FLAG_ROCEV2_CAP)
+			edev->flags |= BNXT_EN_FLAG_ROCEV2_CAP;
+		edev->net = dev;
+		edev->pdev = bp->pdev;
+		bp->edev = edev;
+	}
+	return bp->edev;
+}
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
new file mode 100644
index 0000000..1ebccd8
--- /dev/null
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
@@ -0,0 +1,91 @@
+/* Broadcom NetXtreme-C/E network driver.
+ *
+ * Copyright (c) 2016 Broadcom Limited
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation.
+ */
+
+#ifndef BNXT_ULP_H
+#define BNXT_ULP_H
+
+#define BNXT_ROCE_ULP	0
+#define BNXT_OTHER_ULP	1
+#define BNXT_MAX_ULP	2
+
+#define BNXT_MIN_ROCE_CP_RINGS	2
+#define BNXT_MIN_ROCE_STAT_CTXS	1
+
+struct hwrm_async_event_cmpl;
+struct bnxt;
+
+struct bnxt_ulp_ops {
+	/* async_notifier() cannot sleep (in BH context) */
+	void (*ulp_async_notifier)(void *, struct hwrm_async_event_cmpl *);
+	void (*ulp_stop)(void *);
+	void (*ulp_start)(void *);
+	void (*ulp_sriov_config)(void *, int);
+};
+
+struct bnxt_msix_entry {
+	u32	vector;
+	u32	ring_idx;
+	u32	db_offset;
+};
+
+struct bnxt_fw_msg {
+	void	*msg;
+	int	msg_len;
+	void	*resp;
+	int	resp_max_len;
+	int	timeout;
+};
+
+struct bnxt_ulp {
+	void		*handle;
+	struct bnxt_ulp_ops __rcu *ulp_ops;
+	unsigned long	*async_events_bmap;
+	u16		max_async_event_id;
+	u16		msix_requested;
+	atomic_t	ref_count;
+};
+
+struct bnxt_en_dev {
+	struct net_device *net;
+	struct pci_dev *pdev;
+	u32 flags;
+	#define BNXT_EN_FLAG_ROCEV1_CAP		0x1
+	#define BNXT_EN_FLAG_ROCEV2_CAP		0x2
+	#define BNXT_EN_FLAG_ROCE_CAP		(BNXT_EN_FLAG_ROCEV1_CAP | \
+						 BNXT_EN_FLAG_ROCEV2_CAP)
+	const struct bnxt_en_ops	*en_ops;
+	struct bnxt_ulp			ulp_tbl[BNXT_MAX_ULP];
+};
+
+struct bnxt_en_ops {
+	int (*bnxt_register_device)(struct bnxt_en_dev *, int,
+				    struct bnxt_ulp_ops *, void *);
+	int (*bnxt_unregister_device)(struct bnxt_en_dev *, int);
+	int (*bnxt_request_msix)(struct bnxt_en_dev *, int,
+				 struct bnxt_msix_entry *, int);
+	int (*bnxt_free_msix)(struct bnxt_en_dev *, int);
+	int (*bnxt_send_fw_msg)(struct bnxt_en_dev *, int,
+				struct bnxt_fw_msg *);
+	int (*bnxt_register_fw_async_events)(struct bnxt_en_dev *, int,
+					     unsigned long *, u16);
+};
+
+static inline bool bnxt_ulp_registered(struct bnxt_en_dev *edev, int ulp_id)
+{
+	if (edev && rcu_access_pointer(edev->ulp_tbl[ulp_id].ulp_ops))
+		return true;
+	return false;
+}
+
+void bnxt_subtract_ulp_resources(struct bnxt *bp, int ulp_id);
+void bnxt_ulp_stop(struct bnxt *bp);
+void bnxt_ulp_async_events(struct bnxt *bp, struct hwrm_async_event_cmpl *cmpl);
+struct bnxt_en_dev *bnxt_ulp_probe(struct net_device *dev);
+
+#endif
-- 
2.5.5

^ permalink raw reply related

* [PATCH 03/28] bnxt_en: Move function reset to bnxt_init_one().
From: Selvin Xavier @ 2016-12-05  6:38 UTC (permalink / raw)
  To: dledford; +Cc: linux-rdma, Michael Chan, David Miller, netdev, Selvin Xavier
In-Reply-To: <1480919912-1079-1-git-send-email-selvin.xavier@broadcom.com>

From: Michael Chan <michael.chan@broadcom.com>

Now that MSIX is enabled in bnxt_init_one(), resources may be allocated by
the RDMA driver before the network device is opened.  So we cannot do
function reset in bnxt_open() which will clear all the resources.

The proper place to do function reset now is in bnxt_init_one().
If we get AER, we'll do function reset as well.

Cc: David Miller <davem@davemloft.net>
Cc: <netdev@vger.kernel.org>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 25 ++++++-------------------
 drivers/net/ethernet/broadcom/bnxt/bnxt.h |  1 -
 2 files changed, 6 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 9178bf8..8b00ef4 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -5613,22 +5613,7 @@ int bnxt_open_nic(struct bnxt *bp, bool irq_re_init, bool link_re_init)
 static int bnxt_open(struct net_device *dev)
 {
 	struct bnxt *bp = netdev_priv(dev);
-	int rc = 0;
 
-	if (!test_bit(BNXT_STATE_FN_RST_DONE, &bp->state)) {
-		rc = bnxt_hwrm_func_reset(bp);
-		if (rc) {
-			netdev_err(bp->dev, "hwrm chip reset failure rc: %x\n",
-				   rc);
-			rc = -EBUSY;
-			return rc;
-		}
-		/* Do func_reset during the 1st PF open only to prevent killing
-		 * the VFs when the PF is brought down and up.
-		 */
-		if (BNXT_PF(bp))
-			set_bit(BNXT_STATE_FN_RST_DONE, &bp->state);
-	}
 	return __bnxt_open_nic(bp, true, true);
 }
 
@@ -7028,6 +7013,10 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (rc)
 		goto init_err;
 
+	rc = bnxt_hwrm_func_reset(bp);
+	if (rc)
+		goto init_err;
+
 	rc = bnxt_init_int_mode(bp);
 	if (rc)
 		goto init_err;
@@ -7069,7 +7058,6 @@ static pci_ers_result_t bnxt_io_error_detected(struct pci_dev *pdev,
 					       pci_channel_state_t state)
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
-	struct bnxt *bp = netdev_priv(netdev);
 
 	netdev_info(netdev, "PCI I/O error detected\n");
 
@@ -7084,8 +7072,6 @@ static pci_ers_result_t bnxt_io_error_detected(struct pci_dev *pdev,
 	if (netif_running(netdev))
 		bnxt_close(netdev);
 
-	/* So that func_reset will be done during slot_reset */
-	clear_bit(BNXT_STATE_FN_RST_DONE, &bp->state);
 	pci_disable_device(pdev);
 	rtnl_unlock();
 
@@ -7119,7 +7105,8 @@ static pci_ers_result_t bnxt_io_slot_reset(struct pci_dev *pdev)
 	} else {
 		pci_set_master(pdev);
 
-		if (netif_running(netdev))
+		err = bnxt_hwrm_func_reset(bp);
+		if (!err && netif_running(netdev))
 			err = bnxt_open(netdev);
 
 		if (!err)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 1461355..0ee2cc4 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1021,7 +1021,6 @@ struct bnxt {
 	unsigned long		state;
 #define BNXT_STATE_OPEN		0
 #define BNXT_STATE_IN_SP_TASK	1
-#define BNXT_STATE_FN_RST_DONE	2
 
 	struct bnxt_irq	*irq_tbl;
 	int			total_irqs;
-- 
2.5.5

^ permalink raw reply related

* [PATCH 02/28] bnxt_en: Enable MSIX early in bnxt_init_one().
From: Selvin Xavier @ 2016-12-05  6:38 UTC (permalink / raw)
  To: dledford; +Cc: linux-rdma, Michael Chan, David Miller, netdev, Selvin Xavier
In-Reply-To: <1480919912-1079-1-git-send-email-selvin.xavier@broadcom.com>

From: Michael Chan <michael.chan@broadcom.com>

To better support the new RDMA driver, we need to move pci_enable_msix()
from bnxt_open() to bnxt_init_one().  This way, MSIX vectors are available
to the RDMA driver whether the network device is up or down.

Part of the existing bnxt_setup_int_mode() function is now refactored into
a new bnxt_init_int_mode().  bnxt_init_int_mode() is called during
bnxt_init_one() to enable MSIX.  The remaining logic in
bnxt_setup_int_mode() to map the IRQs to the completion rings is called
during bnxt_open().

Cc: David Miller <davem@davemloft.net>
Cc: <netdev@vger.kernel.org>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 183 +++++++++++++++++++-----------
 drivers/net/ethernet/broadcom/bnxt/bnxt.h |   1 +
 2 files changed, 115 insertions(+), 69 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 6cdfe3e..9178bf8 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -4743,6 +4743,80 @@ static int bnxt_trim_rings(struct bnxt *bp, int *rx, int *tx, int max,
 	return 0;
 }
 
+static void bnxt_setup_msix(struct bnxt *bp)
+{
+	const int len = sizeof(bp->irq_tbl[0].name);
+	struct net_device *dev = bp->dev;
+	int tcs, i;
+
+	tcs = netdev_get_num_tc(dev);
+	if (tcs > 1) {
+		bp->tx_nr_rings_per_tc = bp->tx_nr_rings / tcs;
+		if (bp->tx_nr_rings_per_tc == 0) {
+			netdev_reset_tc(dev);
+			bp->tx_nr_rings_per_tc = bp->tx_nr_rings;
+		} else {
+			int i, off, count;
+
+			bp->tx_nr_rings = bp->tx_nr_rings_per_tc * tcs;
+			for (i = 0; i < tcs; i++) {
+				count = bp->tx_nr_rings_per_tc;
+				off = i * count;
+				netdev_set_tc_queue(dev, i, count, off);
+			}
+		}
+	}
+
+	for (i = 0; i < bp->cp_nr_rings; i++) {
+		char *attr;
+
+		if (bp->flags & BNXT_FLAG_SHARED_RINGS)
+			attr = "TxRx";
+		else if (i < bp->rx_nr_rings)
+			attr = "rx";
+		else
+			attr = "tx";
+
+		snprintf(bp->irq_tbl[i].name, len, "%s-%s-%d", dev->name, attr,
+			 i);
+		bp->irq_tbl[i].handler = bnxt_msix;
+	}
+}
+
+static void bnxt_setup_inta(struct bnxt *bp)
+{
+	const int len = sizeof(bp->irq_tbl[0].name);
+
+	if (netdev_get_num_tc(bp->dev))
+		netdev_reset_tc(bp->dev);
+
+	snprintf(bp->irq_tbl[0].name, len, "%s-%s-%d", bp->dev->name, "TxRx",
+		 0);
+	bp->irq_tbl[0].handler = bnxt_inta;
+}
+
+static int bnxt_setup_int_mode(struct bnxt *bp)
+{
+	int rc;
+
+	if (bp->flags & BNXT_FLAG_USING_MSIX)
+		bnxt_setup_msix(bp);
+	else
+		bnxt_setup_inta(bp);
+
+	rc = bnxt_set_real_num_queues(bp);
+	return rc;
+}
+
+static unsigned int bnxt_get_max_func_irqs(struct bnxt *bp)
+{
+	if (BNXT_PF(bp))
+		return bp->pf.max_irqs;
+#if defined(CONFIG_BNXT_SRIOV)
+	return bp->vf.max_irqs;
+#endif
+}
+
 void bnxt_set_max_func_irqs(struct bnxt *bp, unsigned int max_irqs)
 {
 	if (BNXT_PF(bp))
@@ -4753,16 +4827,12 @@ void bnxt_set_max_func_irqs(struct bnxt *bp, unsigned int max_irqs)
 #endif
 }
 
-static int bnxt_setup_msix(struct bnxt *bp)
+static int bnxt_init_msix(struct bnxt *bp)
 {
-	struct msix_entry *msix_ent;
-	struct net_device *dev = bp->dev;
 	int i, total_vecs, rc = 0, min = 1;
-	const int len = sizeof(bp->irq_tbl[0].name);
-
-	bp->flags &= ~BNXT_FLAG_USING_MSIX;
-	total_vecs = bp->cp_nr_rings;
+	struct msix_entry *msix_ent;
 
+	total_vecs = bnxt_get_max_func_irqs(bp);
 	msix_ent = kcalloc(total_vecs, sizeof(struct msix_entry), GFP_KERNEL);
 	if (!msix_ent)
 		return -ENOMEM;
@@ -4783,8 +4853,10 @@ static int bnxt_setup_msix(struct bnxt *bp)
 
 	bp->irq_tbl = kcalloc(total_vecs, sizeof(struct bnxt_irq), GFP_KERNEL);
 	if (bp->irq_tbl) {
-		int tcs;
+		for (i = 0; i < total_vecs; i++)
+			bp->irq_tbl[i].vector = msix_ent[i].vector;
 
+		bp->total_irqs = total_vecs;
 		/* Trim rings based upon num of vectors allocated */
 		rc = bnxt_trim_rings(bp, &bp->rx_nr_rings, &bp->tx_nr_rings,
 				     total_vecs, min == 1);
@@ -4792,43 +4864,10 @@ static int bnxt_setup_msix(struct bnxt *bp)
 			goto msix_setup_exit;
 
 		bp->tx_nr_rings_per_tc = bp->tx_nr_rings;
-		tcs = netdev_get_num_tc(dev);
-		if (tcs > 1) {
-			bp->tx_nr_rings_per_tc = bp->tx_nr_rings / tcs;
-			if (bp->tx_nr_rings_per_tc == 0) {
-				netdev_reset_tc(dev);
-				bp->tx_nr_rings_per_tc = bp->tx_nr_rings;
-			} else {
-				int i, off, count;
+		bp->cp_nr_rings = (min == 1) ?
+				  max_t(int, bp->tx_nr_rings, bp->rx_nr_rings) :
+				  bp->tx_nr_rings + bp->rx_nr_rings;
 
-				bp->tx_nr_rings = bp->tx_nr_rings_per_tc * tcs;
-				for (i = 0; i < tcs; i++) {
-					count = bp->tx_nr_rings_per_tc;
-					off = i * count;
-					netdev_set_tc_queue(dev, i, count, off);
-				}
-			}
-		}
-		bp->cp_nr_rings = total_vecs;
-
-		for (i = 0; i < bp->cp_nr_rings; i++) {
-			char *attr;
-
-			bp->irq_tbl[i].vector = msix_ent[i].vector;
-			if (bp->flags & BNXT_FLAG_SHARED_RINGS)
-				attr = "TxRx";
-			else if (i < bp->rx_nr_rings)
-				attr = "rx";
-			else
-				attr = "tx";
-
-			snprintf(bp->irq_tbl[i].name, len,
-				 "%s-%s-%d", dev->name, attr, i);
-			bp->irq_tbl[i].handler = bnxt_msix;
-		}
-		rc = bnxt_set_real_num_queues(bp);
-		if (rc)
-			goto msix_setup_exit;
 	} else {
 		rc = -ENOMEM;
 		goto msix_setup_exit;
@@ -4838,52 +4877,54 @@ static int bnxt_setup_msix(struct bnxt *bp)
 	return 0;
 
 msix_setup_exit:
-	netdev_err(bp->dev, "bnxt_setup_msix err: %x\n", rc);
+	netdev_err(bp->dev, "bnxt_init_msix err: %x\n", rc);
+	kfree(bp->irq_tbl);
+	bp->irq_tbl = NULL;
 	pci_disable_msix(bp->pdev);
 	kfree(msix_ent);
 	return rc;
 }
 
-static int bnxt_setup_inta(struct bnxt *bp)
+static int bnxt_init_inta(struct bnxt *bp)
 {
-	int rc;
-	const int len = sizeof(bp->irq_tbl[0].name);
-
-	if (netdev_get_num_tc(bp->dev))
-		netdev_reset_tc(bp->dev);
-
 	bp->irq_tbl = kcalloc(1, sizeof(struct bnxt_irq), GFP_KERNEL);
-	if (!bp->irq_tbl) {
-		rc = -ENOMEM;
-		return rc;
-	}
+	if (!bp->irq_tbl)
+		return -ENOMEM;
+
+	bp->total_irqs = 1;
 	bp->rx_nr_rings = 1;
 	bp->tx_nr_rings = 1;
 	bp->cp_nr_rings = 1;
 	bp->tx_nr_rings_per_tc = bp->tx_nr_rings;
 	bp->flags |= BNXT_FLAG_SHARED_RINGS;
 	bp->irq_tbl[0].vector = bp->pdev->irq;
-	snprintf(bp->irq_tbl[0].name, len,
-		 "%s-%s-%d", bp->dev->name, "TxRx", 0);
-	bp->irq_tbl[0].handler = bnxt_inta;
-	rc = bnxt_set_real_num_queues(bp);
-	return rc;
+	return 0;
 }
 
-static int bnxt_setup_int_mode(struct bnxt *bp)
+static int bnxt_init_int_mode(struct bnxt *bp)
 {
 	int rc = 0;
 
 	if (bp->flags & BNXT_FLAG_MSIX_CAP)
-		rc = bnxt_setup_msix(bp);
+		rc = bnxt_init_msix(bp);
 
 	if (!(bp->flags & BNXT_FLAG_USING_MSIX) && BNXT_PF(bp)) {
 		/* fallback to INTA */
-		rc = bnxt_setup_inta(bp);
+		rc = bnxt_init_inta(bp);
 	}
 	return rc;
 }
 
+static void bnxt_clear_int_mode(struct bnxt *bp)
+{
+	if (bp->flags & BNXT_FLAG_USING_MSIX)
+		pci_disable_msix(bp->pdev);
+
+	kfree(bp->irq_tbl);
+	bp->irq_tbl = NULL;
+	bp->flags &= ~BNXT_FLAG_USING_MSIX;
+}
+
 static void bnxt_free_irq(struct bnxt *bp)
 {
 	struct bnxt_irq *irq;
@@ -4902,10 +4943,6 @@ static void bnxt_free_irq(struct bnxt *bp)
 			free_irq(irq->vector, bp->bnapi[i]);
 		irq->requested = 0;
 	}
-	if (bp->flags & BNXT_FLAG_USING_MSIX)
-		pci_disable_msix(bp->pdev);
-	kfree(bp->irq_tbl);
-	bp->irq_tbl = NULL;
 }
 
 static int bnxt_request_irq(struct bnxt *bp)
@@ -6695,6 +6732,7 @@ static void bnxt_remove_one(struct pci_dev *pdev)
 	cancel_work_sync(&bp->sp_task);
 	bp->sp_event = 0;
 
+	bnxt_clear_int_mode(bp);
 	bnxt_hwrm_func_drv_unrgtr(bp);
 	bnxt_free_hwrm_resources(bp);
 	bnxt_dcb_free(bp);
@@ -6990,10 +7028,14 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (rc)
 		goto init_err;
 
-	rc = register_netdev(dev);
+	rc = bnxt_init_int_mode(bp);
 	if (rc)
 		goto init_err;
 
+	rc = register_netdev(dev);
+	if (rc)
+		goto init_err_clr_int;
+
 	netdev_info(dev, "%s found at mem %lx, node addr %pM\n",
 		    board_info[ent->driver_data].name,
 		    (long)pci_resource_start(pdev, 0), dev->dev_addr);
@@ -7002,6 +7044,9 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	return 0;
 
+init_err_clr_int:
+	bnxt_clear_int_mode(bp);
+
 init_err:
 	pci_iounmap(pdev, bp->bar0);
 	pci_release_regions(pdev);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 8327d0d..1461355 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1024,6 +1024,7 @@ struct bnxt {
 #define BNXT_STATE_FN_RST_DONE	2
 
 	struct bnxt_irq	*irq_tbl;
+	int			total_irqs;
 	u8			mac_addr[ETH_ALEN];
 
 #ifdef CONFIG_BNXT_DCB
-- 
2.5.5

^ permalink raw reply related

* [PATCH 06/28] bnxt_en: Refactor the driver registration function with firmware.
From: Selvin Xavier @ 2016-12-05  6:38 UTC (permalink / raw)
  To: dledford-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Michael Chan, David Miller,
	netdev-u79uwXL29TY76Z2rM5mHXA, Selvin Xavier
In-Reply-To: <1480919912-1079-1-git-send-email-selvin.xavier-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

From: Michael Chan <michael.chan-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

The driver register function with firmware consists of passing version
information and registering for async events.  To support the RDMA driver,
the async events that we need to register may change.  Separate the
driver register function into 2 parts so that we can just update the
async events for the RDMA driver.

Cc: David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Cc: <netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Signed-off-by: Michael Chan <michael.chan-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Signed-off-by: Selvin Xavier <selvin.xavier-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 34 ++++++++++++++++++++++++++-----
 drivers/net/ethernet/broadcom/bnxt/bnxt.h |  2 ++
 2 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 7218d65..c26735ea 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -3117,27 +3117,46 @@ int hwrm_send_message_silent(struct bnxt *bp, void *msg, u32 msg_len,
 	return rc;
 }
 
-static int bnxt_hwrm_func_drv_rgtr(struct bnxt *bp)
+int bnxt_hwrm_func_rgtr_async_events(struct bnxt *bp, unsigned long *bmap,
+				     int bmap_size)
 {
 	struct hwrm_func_drv_rgtr_input req = {0};
-	int i;
 	DECLARE_BITMAP(async_events_bmap, 256);
 	u32 *events = (u32 *)async_events_bmap;
+	int i;
 
 	bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_FUNC_DRV_RGTR, -1, -1);
 
 	req.enables =
-		cpu_to_le32(FUNC_DRV_RGTR_REQ_ENABLES_OS_TYPE |
-			    FUNC_DRV_RGTR_REQ_ENABLES_VER |
-			    FUNC_DRV_RGTR_REQ_ENABLES_ASYNC_EVENT_FWD);
+		cpu_to_le32(FUNC_DRV_RGTR_REQ_ENABLES_ASYNC_EVENT_FWD);
 
 	memset(async_events_bmap, 0, sizeof(async_events_bmap));
 	for (i = 0; i < ARRAY_SIZE(bnxt_async_events_arr); i++)
 		__set_bit(bnxt_async_events_arr[i], async_events_bmap);
 
+	if (bmap && bmap_size) {
+		for (i = 0; i < bmap_size; i++) {
+			if (test_bit(i, bmap))
+				__set_bit(i, async_events_bmap);
+		}
+	}
+
 	for (i = 0; i < 8; i++)
 		req.async_event_fwd[i] |= cpu_to_le32(events[i]);
 
+	return hwrm_send_message(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
+}
+
+static int bnxt_hwrm_func_drv_rgtr(struct bnxt *bp)
+{
+	struct hwrm_func_drv_rgtr_input req = {0};
+
+	bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_FUNC_DRV_RGTR, -1, -1);
+
+	req.enables =
+		cpu_to_le32(FUNC_DRV_RGTR_REQ_ENABLES_OS_TYPE |
+			    FUNC_DRV_RGTR_REQ_ENABLES_VER);
+
 	req.os_type = cpu_to_le16(FUNC_DRV_RGTR_REQ_OS_TYPE_LINUX);
 	req.ver_maj = DRV_VER_MAJ;
 	req.ver_min = DRV_VER_MIN;
@@ -3146,6 +3165,7 @@ static int bnxt_hwrm_func_drv_rgtr(struct bnxt *bp)
 	if (BNXT_PF(bp)) {
 		DECLARE_BITMAP(vf_req_snif_bmap, 256);
 		u32 *data = (u32 *)vf_req_snif_bmap;
+		int i;
 
 		memset(vf_req_snif_bmap, 0, sizeof(vf_req_snif_bmap));
 		for (i = 0; i < ARRAY_SIZE(bnxt_vf_req_snif); i++)
@@ -7023,6 +7043,10 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (rc)
 		goto init_err;
 
+	rc = bnxt_hwrm_func_rgtr_async_events(bp, NULL, 0);
+	if (rc)
+		goto init_err;
+
 	/* Get the MAX capabilities for this function */
 	rc = bnxt_hwrm_func_qcaps(bp);
 	if (rc) {
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index d796836..eec2415 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1240,6 +1240,8 @@ void bnxt_hwrm_cmd_hdr_init(struct bnxt *, void *, u16, u16, u16);
 int _hwrm_send_message(struct bnxt *, void *, u32, int);
 int hwrm_send_message(struct bnxt *, void *, u32, int);
 int hwrm_send_message_silent(struct bnxt *, void *, u32, int);
+int bnxt_hwrm_func_rgtr_async_events(struct bnxt *bp, unsigned long *bmap,
+				     int bmap_size);
 int bnxt_hwrm_set_coal(struct bnxt *);
 unsigned int bnxt_get_max_func_stat_ctxs(struct bnxt *bp);
 unsigned int bnxt_get_max_func_cp_rings(struct bnxt *bp);
-- 
2.5.5

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 01/28] bnxt_en: Add bnxt_set_max_func_irqs().
From: Selvin Xavier @ 2016-12-05  6:38 UTC (permalink / raw)
  To: dledford-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Michael Chan, David Miller,
	netdev-u79uwXL29TY76Z2rM5mHXA, Selvin Xavier
In-Reply-To: <1480919912-1079-1-git-send-email-selvin.xavier-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

From: Michael Chan <michael.chan-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

By refactoring existing code into this new function.  The new function
will be used in subsequent patches.

Cc: David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Cc: <netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Signed-off-by: Michael Chan <michael.chan-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Signed-off-by: Selvin Xavier <selvin.xavier-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 17 +++++++++++------
 drivers/net/ethernet/broadcom/bnxt/bnxt.h |  1 +
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index e8ab5fd..6cdfe3e 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -4743,6 +4743,16 @@ static int bnxt_trim_rings(struct bnxt *bp, int *rx, int *tx, int max,
 	return 0;
 }
 
+void bnxt_set_max_func_irqs(struct bnxt *bp, unsigned int max_irqs)
+{
+	if (BNXT_PF(bp))
+		bp->pf.max_irqs = max_irqs;
+#if defined(CONFIG_BNXT_SRIOV)
+	else
+		bp->vf.max_irqs = max_irqs;
+#endif
+}
+
 static int bnxt_setup_msix(struct bnxt *bp)
 {
 	struct msix_entry *msix_ent;
@@ -6949,12 +6959,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	bnxt_set_tpa_flags(bp);
 	bnxt_set_ring_params(bp);
-	if (BNXT_PF(bp))
-		bp->pf.max_irqs = max_irqs;
-#if defined(CONFIG_BNXT_SRIOV)
-	else
-		bp->vf.max_irqs = max_irqs;
-#endif
+	bnxt_set_max_func_irqs(bp, max_irqs);
 	bnxt_set_dflt_rings(bp);
 
 	/* Default RSS hash cfg. */
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index b4abc1b..8327d0d 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1235,6 +1235,7 @@ int hwrm_send_message(struct bnxt *, void *, u32, int);
 int hwrm_send_message_silent(struct bnxt *, void *, u32, int);
 int bnxt_hwrm_set_coal(struct bnxt *);
 int bnxt_hwrm_func_qcaps(struct bnxt *);
+void bnxt_set_max_func_irqs(struct bnxt *bp, unsigned int max);
 void bnxt_tx_disable(struct bnxt *bp);
 void bnxt_tx_enable(struct bnxt *bp);
 int bnxt_hwrm_set_pause(struct bnxt *);
-- 
2.5.5

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: [PATCH V2 net 10/20] net/ena: remove redundant logic in napi callback for busy poll mode
From: Eric Dumazet @ 2016-12-05  5:45 UTC (permalink / raw)
  To: Netanel Belgazal
  Cc: linux-kernel, davem, netdev, dwmw, zorik, alex, saeed, msw,
	aliguori, nafea
In-Reply-To: <1480857578-5065-11-git-send-email-netanel@annapurnalabs.com>

On Sun, 2016-12-04 at 15:19 +0200, Netanel Belgazal wrote:
> sk_busy_loop can call the napi callback few million times a sec.
> For each call there is unmask interrupt.
> We want to reduce the number of unmasks.
> 
> Add an atomic variable that will tell the napi handler if
> it was called from irq context or not.
> Unmask the interrupt only from irq context.
> 
> A schenario where the driver left with missed unmask isn't feasible.
> when ena_intr_msix_io is called the driver have 2 options:
> 1)Before napi completes and call napi_complete_done
> 2)After calling napi_complete_done
> 
> In the former case the napi will unmask the interrupt as needed.
> In the latter case napi_complete_done will remove napi from the schedule
> list so napi will be rescheduled (by ena_intr_msix_io) and interrupt
> will be unmasked as desire in the 2nd napi call.
> 
> Signed-off-by: Netanel Belgazal <netanel@annapurnalabs.com>
> ---


This looks very complicated to me.

I guess you missed the recent patches that happened on net-next ?

2e713283751f494596655d9125c168aeb913f71d net/mlx4_en: use napi_complete_done() return value
364b6055738b4c752c30ccaaf25c624e69d76195 net: busy-poll: return busypolling status to drivers
21cb84c48ca0619181106f0f44f3802a989de024 net: busy-poll: remove need_resched() from sk_can_busy_loop()
217f6974368188fd8bd7804bf5a036aa5762c5e4 net: busy-poll: allow preemption in sk_busy_loop()

napi_complete_done() return code can be used by a driver,
no need to add yet another atomic operation in fast path.

Anyway, this looks wrong :

@@ -1186,6 +1201,7 @@ static irqreturn_t ena_intr_msix_io(int irq, void *data)
 {
        struct ena_napi *ena_napi = data;
 
+       atomic_set(&ena_napi->unmask_interrupt, 1);
        napi_schedule(&ena_napi->napi);
 
You probably wanted :

if (napi_schedule_prep(n)) {
	atomic_set(&ena_napi->unmask_interrupt, 1);
	__napi_schedule(n);
}



Please rework this napi poll using core infrastructure.

busypoll logic should be centralized, not reimplemented in different ways in a driver.

Thanks.

^ permalink raw reply

* Re: [PATCH] iproute2: ss: escape all null bytes in abstract unix domain socket
From: Isaac Boukris @ 2016-12-05  5:38 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Stephen Hemminger, David Miller, netdev
In-Reply-To: <1480721075.18162.389.camel@edumazet-glaptop3.roam.corp.google.com>

On Sat, Dec 3, 2016 at 1:24 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Fri, 2016-12-02 at 15:18 -0800, Stephen Hemminger wrote:
>>                                     name[i] = '@';
>> >
>> > ss.c: In function 'unix_show_sock':
>> > ss.c:3128:4: error: 'for' loop initial declarations are only allowed in C99 mode
>> > ss.c:3128:4: note: use option -std=c99 or -std=gnu99 to compile your code
>> > make[1]: *** [ss.o] Error 1
>> >
>> >
>> >
>>
>> Thanks, fixed by patch from Simon
>
> Right, thanks !


Thanks for notifying me. Sorry for the bug and thanks for the fix!

^ permalink raw reply

* Re: [PATCH V2 net 13/20] net/ena: change driver's default timeouts
From: Matt Wilson @ 2016-12-05  4:35 UTC (permalink / raw)
  To: Netanel Belgazal
  Cc: linux-kernel, davem, netdev, dwmw, zorik, alex, saeed, msw,
	aliguori, nafea
In-Reply-To: <1480857578-5065-14-git-send-email-netanel@annapurnalabs.com>

On Sun, Dec 04, 2016 at 03:19:31PM +0200, Netanel Belgazal wrote:

... because? (they turned out to be too aggressive, I believe.)

> Signed-off-by: Netanel Belgazal <netanel@annapurnalabs.com>
> ---
>  drivers/net/ethernet/amazon/ena/ena_com.c    | 4 ++--
>  drivers/net/ethernet/amazon/ena/ena_netdev.h | 7 ++++---
>  2 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/amazon/ena/ena_com.c b/drivers/net/ethernet/amazon/ena/ena_com.c
> index 4147d6e..a550c8a 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_com.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_com.c
> @@ -36,9 +36,9 @@
>  /*****************************************************************************/
>  
>  /* Timeout in micro-sec */
> -#define ADMIN_CMD_TIMEOUT_US (1000000)
> +#define ADMIN_CMD_TIMEOUT_US (3000000)
>  
> -#define ENA_ASYNC_QUEUE_DEPTH 4
> +#define ENA_ASYNC_QUEUE_DEPTH 16

Why is this changed at the same time?

>  #define ENA_ADMIN_QUEUE_DEPTH 32
>  
>  #define MIN_ENA_VER (((ENA_COMMON_SPEC_VERSION_MAJOR) << \
> diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.h b/drivers/net/ethernet/amazon/ena/ena_netdev.h
> index c081fd3..ed42e07 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_netdev.h
> +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.h
> @@ -39,6 +39,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/netdevice.h>
>  #include <linux/skbuff.h>
> +#include <linux/u64_stats_sync.h>

This change seems unrelated.

>  #include "ena_com.h"
>  #include "ena_eth_com.h"
> @@ -100,7 +101,7 @@
>  /* Number of queues to check for missing queues per timer service */
>  #define ENA_MONITORED_TX_QUEUES	4
>  /* Max timeout packets before device reset */
> -#define MAX_NUM_OF_TIMEOUTED_PACKETS 32
> +#define MAX_NUM_OF_TIMEOUTED_PACKETS 128
>  
>  #define ENA_TX_RING_IDX_NEXT(idx, ring_size) (((idx) + 1) & ((ring_size) - 1))
>  
> @@ -116,9 +117,9 @@
>  #define ENA_IO_IRQ_IDX(q)		(ENA_IO_IRQ_FIRST_IDX + (q))
>  
>  /* ENA device should send keep alive msg every 1 sec.
> - * We wait for 3 sec just to be on the safe side.
> + * We wait for 6 sec just to be on the safe side.
>   */
> -#define ENA_DEVICE_KALIVE_TIMEOUT	(3 * HZ)
> +#define ENA_DEVICE_KALIVE_TIMEOUT	(6 * HZ)
>  
>  #define ENA_MMIO_DISABLE_REG_READ	BIT(0)
>  

^ permalink raw reply

* Re: [PATCH V2 net 08/20] net/ena: add hardware hints capability to the driver
From: Matt Wilson @ 2016-12-05  4:31 UTC (permalink / raw)
  To: Netanel Belgazal
  Cc: linux-kernel, davem, netdev, dwmw, zorik, alex, saeed, msw,
	aliguori, nafea
In-Reply-To: <1480857578-5065-9-git-send-email-netanel@annapurnalabs.com>

On Sun, Dec 04, 2016 at 03:19:26PM +0200, Netanel Belgazal wrote:
> The ENA device can update the ena driver about the desire timeouts.
> The hardware hints are transmitted as Asynchronous event to the driver.

This is really a new feature, not a bugfix - correct? If it is a new
feature, submit it separately. If the built-in defaults need to be
changed, submit that as a bugfix.

--msw

> In case the device does not support this capability, the driver
> will use its own defines.
> 
> Signed-off-by: Netanel Belgazal <netanel@annapurnalabs.com>
> ---
>  drivers/net/ethernet/amazon/ena/ena_admin_defs.h | 31 +++++++++
>  drivers/net/ethernet/amazon/ena/ena_com.c        | 41 ++++++++---
>  drivers/net/ethernet/amazon/ena/ena_com.h        |  5 ++
>  drivers/net/ethernet/amazon/ena/ena_ethtool.c    |  1 -
>  drivers/net/ethernet/amazon/ena/ena_netdev.c     | 86 +++++++++++++++++++-----
>  drivers/net/ethernet/amazon/ena/ena_netdev.h     | 19 +++++-
>  drivers/net/ethernet/amazon/ena/ena_regs_defs.h  |  2 +
>  7 files changed, 157 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/net/ethernet/amazon/ena/ena_admin_defs.h b/drivers/net/ethernet/amazon/ena/ena_admin_defs.h
> index 6d70bf5..35ae511 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_admin_defs.h
> +++ b/drivers/net/ethernet/amazon/ena/ena_admin_defs.h
> @@ -70,6 +70,8 @@ enum ena_admin_aq_feature_id {
>  
>  	ENA_ADMIN_MAX_QUEUES_NUM		= 2,
>  
> +	ENA_ADMIN_HW_HINTS			= 3,
> +
>  	ENA_ADMIN_RSS_HASH_FUNCTION		= 10,
>  
>  	ENA_ADMIN_STATELESS_OFFLOAD_CONFIG	= 11,
> @@ -749,6 +751,31 @@ struct ena_admin_feature_rss_ind_table {
>  	struct ena_admin_rss_ind_table_entry inline_entry;
>  };
>  
> +/* When hint value is 0, driver should use it's own predefined value */
> +struct ena_admin_ena_hw_hints {
> +	/* value in ms */
> +	u16 mmio_read_timeout;
> +
> +	/* value in ms */
> +	u16 driver_watchdog_timeout;
> +
> +	/* Per packet tx completion timeout. value in ms */
> +	u16 missing_tx_completion_timeout;
> +
> +	u16 missed_tx_completion_count_threshold_to_reset;
> +
> +	/* value in ms */
> +	u16 admin_completion_tx_timeout;
> +
> +	u16 netdev_wd_timeout;
> +
> +	u16 max_tx_sgl_size;
> +
> +	u16 max_rx_sgl_size;
> +
> +	u16 reserved[8];
> +};
> +
>  struct ena_admin_get_feat_cmd {
>  	struct ena_admin_aq_common_desc aq_common_descriptor;
>  
> @@ -782,6 +809,8 @@ struct ena_admin_get_feat_resp {
>  		struct ena_admin_feature_rss_ind_table ind_table;
>  
>  		struct ena_admin_feature_intr_moder_desc intr_moderation;
> +
> +		struct ena_admin_ena_hw_hints hw_hints;
>  	} u;
>  };
>  
> @@ -857,6 +886,8 @@ enum ena_admin_aenq_notification_syndrom {
>  	ENA_ADMIN_SUSPEND	= 0,
>  
>  	ENA_ADMIN_RESUME	= 1,
> +
> +	ENA_ADMIN_UPDATE_HINTS	= 2,
>  };
>  
>  struct ena_admin_aenq_entry {
> diff --git a/drivers/net/ethernet/amazon/ena/ena_com.c b/drivers/net/ethernet/amazon/ena/ena_com.c
> index 46aad3a..f1e4f04 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_com.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_com.c
> @@ -508,15 +508,13 @@ static int ena_com_comp_status_to_errno(u8 comp_status)
>  static int ena_com_wait_and_process_admin_cq_polling(struct ena_comp_ctx *comp_ctx,
>  						     struct ena_com_admin_queue *admin_queue)
>  {
> -	unsigned long flags;
> -	u32 start_time;
> +	unsigned long flags, timeout;
>  	int ret;
>  
> -	start_time = ((u32)jiffies_to_usecs(jiffies));
> +	timeout = jiffies + usecs_to_jiffies(admin_queue->completion_timeout);
>  
>  	while (comp_ctx->status == ENA_CMD_SUBMITTED) {
> -		if ((((u32)jiffies_to_usecs(jiffies)) - start_time) >
> -		    ADMIN_CMD_TIMEOUT_US) {
> +		if (time_is_before_jiffies(timeout)) {
>  			pr_err("Wait for completion (polling) timeout\n");
>  			/* ENA didn't have any completion */
>  			spin_lock_irqsave(&admin_queue->q_lock, flags);
> @@ -560,7 +558,8 @@ static int ena_com_wait_and_process_admin_cq_interrupts(struct ena_comp_ctx *com
>  	int ret;
>  
>  	wait_for_completion_timeout(&comp_ctx->wait_event,
> -				    usecs_to_jiffies(ADMIN_CMD_TIMEOUT_US));
> +				    usecs_to_jiffies(
> +					    admin_queue->completion_timeout));
>  
>  	/* In case the command wasn't completed find out the root cause.
>  	 * There might be 2 kinds of errors
> @@ -600,12 +599,14 @@ static u32 ena_com_reg_bar_read32(struct ena_com_dev *ena_dev, u16 offset)
>  	struct ena_com_mmio_read *mmio_read = &ena_dev->mmio_read;
>  	volatile struct ena_admin_ena_mmio_req_read_less_resp *read_resp =
>  		mmio_read->read_resp;
> -	u32 mmio_read_reg, ret;
> +	u32 mmio_read_reg, timeout, ret;
>  	unsigned long flags;
>  	int i;
>  
>  	might_sleep();
>  
> +	timeout = mmio_read->reg_read_to ? : ENA_REG_READ_TIMEOUT;
> +
>  	/* If readless is disabled, perform regular read */
>  	if (!mmio_read->readless_supported)
>  		return readl(ena_dev->reg_bar + offset);
> @@ -626,14 +627,14 @@ static u32 ena_com_reg_bar_read32(struct ena_com_dev *ena_dev, u16 offset)
>  
>  	writel(mmio_read_reg, ena_dev->reg_bar + ENA_REGS_MMIO_REG_READ_OFF);
>  
> -	for (i = 0; i < ENA_REG_READ_TIMEOUT; i++) {
> +	for (i = 0; i < timeout; i++) {
>  		if (read_resp->req_id == mmio_read->seq_num)
>  			break;
>  
>  		udelay(1);
>  	}
>  
> -	if (unlikely(i == ENA_REG_READ_TIMEOUT)) {
> +	if (unlikely(i == timeout)) {
>  		pr_err("reading reg failed for timeout. expected: req id[%hu] offset[%hu] actual: req id[%hu] offset[%hu]\n",
>  		       mmio_read->seq_num, offset, read_resp->req_id,
>  		       read_resp->reg_off);
> @@ -1717,6 +1718,20 @@ int ena_com_get_dev_attr_feat(struct ena_com_dev *ena_dev,
>  	memcpy(&get_feat_ctx->offload, &get_resp.u.offload,
>  	       sizeof(get_resp.u.offload));
>  
> +	/* Driver hints isn't mandatory admin command. So in case the
> +	 * command isn't supported set driver hints to 0
> +	 */
> +	rc = ena_com_get_feature(ena_dev, &get_resp, ENA_ADMIN_HW_HINTS);
> +
> +	if (!rc)
> +		memcpy(&get_feat_ctx->hw_hints, &get_resp.u.hw_hints,
> +		       sizeof(get_resp.u.hw_hints));
> +	else if (rc == -EPERM)
> +		memset(&get_feat_ctx->hw_hints, 0x0,
> +		       sizeof(get_feat_ctx->hw_hints));
> +	else
> +		return rc;
> +
>  	return 0;
>  }
>  
> @@ -1842,6 +1857,14 @@ int ena_com_dev_reset(struct ena_com_dev *ena_dev)
>  		return rc;
>  	}
>  
> +	timeout = (cap & ENA_REGS_CAPS_ADMIN_CMD_TO_MASK) >>
> +		ENA_REGS_CAPS_ADMIN_CMD_TO_SHIFT;
> +	if (timeout)
> +		/* the resolution of timeout reg is 100ms */
> +		ena_dev->admin_queue.completion_timeout = timeout * 100000;
> +	else
> +		ena_dev->admin_queue.completion_timeout = ADMIN_CMD_TIMEOUT_US;
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/net/ethernet/amazon/ena/ena_com.h b/drivers/net/ethernet/amazon/ena/ena_com.h
> index 509d7b8..6883ee5 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_com.h
> +++ b/drivers/net/ethernet/amazon/ena/ena_com.h
> @@ -96,6 +96,8 @@
>  #define ENA_INTR_MODER_LEVEL_STRIDE			2
>  #define ENA_INTR_BYTE_COUNT_NOT_SUPPORTED		0xFFFFFF
>  
> +#define ENA_HW_HINTS_NO_TIMEOUT				0xFFFF
> +
>  enum ena_intr_moder_level {
>  	ENA_INTR_MODER_LOWEST = 0,
>  	ENA_INTR_MODER_LOW,
> @@ -232,6 +234,7 @@ struct ena_com_admin_queue {
>  	void *q_dmadev;
>  	spinlock_t q_lock; /* spinlock for the admin queue */
>  	struct ena_comp_ctx *comp_ctx;
> +	u32 completion_timeout;
>  	u16 q_depth;
>  	struct ena_com_admin_cq cq;
>  	struct ena_com_admin_sq sq;
> @@ -266,6 +269,7 @@ struct ena_com_aenq {
>  struct ena_com_mmio_read {
>  	struct ena_admin_ena_mmio_req_read_less_resp *read_resp;
>  	dma_addr_t read_resp_dma_addr;
> +	u32 reg_read_to; /* in us */
>  	u16 seq_num;
>  	bool readless_supported;
>  	/* spin lock to ensure a single outstanding read */
> @@ -335,6 +339,7 @@ struct ena_com_dev_get_features_ctx {
>  	struct ena_admin_device_attr_feature_desc dev_attr;
>  	struct ena_admin_feature_aenq_desc aenq;
>  	struct ena_admin_feature_offload_desc offload;
> +	struct ena_admin_ena_hw_hints hw_hints;
>  };
>  
>  struct ena_com_create_io_ctx {
> diff --git a/drivers/net/ethernet/amazon/ena/ena_ethtool.c b/drivers/net/ethernet/amazon/ena/ena_ethtool.c
> index 67b2338f..a1fbfc2 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_ethtool.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_ethtool.c
> @@ -80,7 +80,6 @@ static const struct ena_stats ena_stats_tx_strings[] = {
>  	ENA_STAT_TX_ENTRY(tx_poll),
>  	ENA_STAT_TX_ENTRY(doorbells),
>  	ENA_STAT_TX_ENTRY(prepare_ctx_err),
> -	ENA_STAT_TX_ENTRY(missing_tx_comp),
>  	ENA_STAT_TX_ENTRY(bad_req_id),
>  };
>  
> diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> index 962ffb5..0b718ee 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> @@ -1981,6 +1981,7 @@ static netdev_tx_t ena_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  
>  	tx_info->tx_descs = nb_hw_desc;
>  	tx_info->last_jiffies = jiffies;
> +	tx_info->print_once = 0;
>  
>  	tx_ring->next_to_use = ENA_TX_RING_IDX_NEXT(next_to_use,
>  		tx_ring->ring_size);
> @@ -2554,33 +2555,34 @@ static void check_for_missing_tx_completions(struct ena_adapter *adapter)
>  	if (!test_bit(ENA_FLAG_DEV_UP, &adapter->flags))
>  		return;
>  
> +	if (adapter->missing_tx_completion_to == ENA_HW_HINTS_NO_TIMEOUT)
> +		return;
> +
>  	budget = ENA_MONITORED_TX_QUEUES;
>  
>  	for (i = adapter->last_monitored_tx_qid; i < adapter->num_queues; i++) {
>  		tx_ring = &adapter->tx_ring[i];
>  
> +		missed_tx = 0;
> +
>  		for (j = 0; j < tx_ring->ring_size; j++) {
>  			tx_buf = &tx_ring->tx_buffer_info[j];
>  			last_jiffies = tx_buf->last_jiffies;
> -			if (unlikely(last_jiffies && time_is_before_jiffies(last_jiffies + TX_TIMEOUT))) {
> -				netif_notice(adapter, tx_err, adapter->netdev,
> -					     "Found a Tx that wasn't completed on time, qid %d, index %d.\n",
> -					     tx_ring->qid, j);
> +			if (unlikely(last_jiffies && time_is_before_jiffies(last_jiffies + adapter->missing_tx_completion_to))) {
> +				if (!tx_buf->print_once)
> +					netif_notice(adapter, tx_err, adapter->netdev,
> +						     "Found a Tx that wasn't completed on time, qid %d, index %d.\n",
> +						     tx_ring->qid, j);
>  
> -				u64_stats_update_begin(&tx_ring->syncp);
> -				missed_tx = tx_ring->tx_stats.missing_tx_comp++;
> -				u64_stats_update_end(&tx_ring->syncp);
> +				tx_buf->print_once = 1;
> +				missed_tx++;
>  
> -				/* Clear last jiffies so the lost buffer won't
> -				 * be counted twice.
> -				 */
> -				tx_buf->last_jiffies = 0;
> -
> -				if (unlikely(missed_tx > MAX_NUM_OF_TIMEOUTED_PACKETS)) {
> +				if (unlikely(missed_tx > adapter->missing_tx_completion_threshold)) {
>  					netif_err(adapter, tx_err, adapter->netdev,
>  						  "The number of lost tx completion is above the threshold (%d > %d). Reset the device\n",
> -						  missed_tx, MAX_NUM_OF_TIMEOUTED_PACKETS);
> +						  missed_tx, adapter->missing_tx_completion_threshold);
>  					set_bit(ENA_FLAG_TRIGGER_RESET, &adapter->flags);
> +					return;
>  				}
>  			}
>  		}
> @@ -2601,8 +2603,11 @@ static void check_for_missing_keep_alive(struct ena_adapter *adapter)
>  	if (!adapter->wd_state)
>  		return;
>  
> -	keep_alive_expired = round_jiffies(adapter->last_keep_alive_jiffies
> -					   + ENA_DEVICE_KALIVE_TIMEOUT);
> +	if (adapter->keep_alive_timeout == ENA_HW_HINTS_NO_TIMEOUT)
> +		return;
> +
> +	keep_alive_expired = round_jiffies(adapter->last_keep_alive_jiffies +
> +					   adapter->keep_alive_timeout);
>  	if (unlikely(time_is_before_jiffies(keep_alive_expired))) {
>  		netif_err(adapter, drv, adapter->netdev,
>  			  "Keep alive watchdog timeout.\n");
> @@ -2625,6 +2630,44 @@ static void check_for_admin_com_state(struct ena_adapter *adapter)
>  	}
>  }
>  
> +static void ena_update_hints(struct ena_adapter *adapter,
> +			     struct ena_admin_ena_hw_hints *hints)
> +{
> +	struct net_device *netdev = adapter->netdev;
> +
> +	if (hints->admin_completion_tx_timeout)
> +		adapter->ena_dev->admin_queue.completion_timeout =
> +			hints->admin_completion_tx_timeout * 1000;
> +
> +	if (hints->mmio_read_timeout)
> +		/* convert to usec */
> +		adapter->ena_dev->mmio_read.reg_read_to =
> +			hints->mmio_read_timeout * 1000;
> +
> +	if (hints->missed_tx_completion_count_threshold_to_reset)
> +		adapter->missing_tx_completion_threshold =
> +			hints->missed_tx_completion_count_threshold_to_reset;
> +
> +	if (hints->missing_tx_completion_timeout) {
> +		if (hints->missing_tx_completion_timeout == ENA_HW_HINTS_NO_TIMEOUT)
> +			adapter->missing_tx_completion_to = ENA_HW_HINTS_NO_TIMEOUT;
> +		else
> +			adapter->missing_tx_completion_to =
> +				msecs_to_jiffies(hints->missing_tx_completion_timeout);
> +	}
> +
> +	if (hints->netdev_wd_timeout)
> +		netdev->watchdog_timeo = msecs_to_jiffies(hints->netdev_wd_timeout);
> +
> +	if (hints->driver_watchdog_timeout) {
> +		if (hints->driver_watchdog_timeout == ENA_HW_HINTS_NO_TIMEOUT)
> +			adapter->keep_alive_timeout = ENA_HW_HINTS_NO_TIMEOUT;
> +		else
> +			adapter->keep_alive_timeout =
> +				msecs_to_jiffies(hints->driver_watchdog_timeout);
> +	}
> +}
> +
>  static void ena_update_host_info(struct ena_admin_host_info *host_info,
>  				 struct net_device *netdev)
>  {
> @@ -3036,6 +3079,11 @@ static int ena_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	INIT_WORK(&adapter->reset_task, ena_fw_reset_device);
>  
>  	adapter->last_keep_alive_jiffies = jiffies;
> +	adapter->keep_alive_timeout = ENA_DEVICE_KALIVE_TIMEOUT;
> +	adapter->missing_tx_completion_to = TX_TIMEOUT;
> +	adapter->missing_tx_completion_threshold = MAX_NUM_OF_TIMEOUTED_PACKETS;
> +
> +	ena_update_hints(adapter, &get_feat_ctx.hw_hints);
>  
>  	init_timer(&adapter->timer_service);
>  	adapter->timer_service.expires = round_jiffies(jiffies + HZ);
> @@ -3256,6 +3304,7 @@ static void ena_notification(void *adapter_data,
>  			     struct ena_admin_aenq_entry *aenq_e)
>  {
>  	struct ena_adapter *adapter = (struct ena_adapter *)adapter_data;
> +	struct ena_admin_ena_hw_hints *hints;
>  
>  	WARN(aenq_e->aenq_common_desc.group != ENA_ADMIN_NOTIFICATION,
>  	     "Invalid group(%x) expected %x\n",
> @@ -3273,6 +3322,11 @@ static void ena_notification(void *adapter_data,
>  	case ENA_ADMIN_RESUME:
>  		queue_work(ena_wq, &adapter->resume_io_task);
>  		break;
> +	case ENA_ADMIN_UPDATE_HINTS:
> +		hints = (struct ena_admin_ena_hw_hints *)
> +			(&aenq_e->inline_data_w4);
> +		ena_update_hints(adapter, hints);
> +		break;
>  	default:
>  		netif_err(adapter, drv, adapter->netdev,
>  			  "Invalid aenq notification link state %d\n",
> diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.h b/drivers/net/ethernet/amazon/ena/ena_netdev.h
> index f0ddc11..2897fab 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_netdev.h
> +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.h
> @@ -146,7 +146,18 @@ struct ena_tx_buffer {
>  	u32 tx_descs;
>  	/* num of buffers used by this skb */
>  	u32 num_of_bufs;
> -	/* Save the last jiffies to detect missing tx packets */
> +
> +	/* Used for detect missing tx packets to limit the number of prints */
> +	u32 print_once;
> +	/* Save the last jiffies to detect missing tx packets
> +	 *
> +	 * sets to non zero value on ena_start_xmit and set to zero on
> +	 * napi and timer_Service_routine.
> +	 *
> +	 * while this value is not protected by lock,
> +	 * a given packet is not expected to be handled by ena_start_xmit
> +	 * and by napi/timer_service at the same time.
> +	 */
>  	unsigned long last_jiffies;
>  	struct ena_com_buf bufs[ENA_PKT_MAX_BUFS];
>  } ____cacheline_aligned;
> @@ -170,7 +181,6 @@ struct ena_stats_tx {
>  	u64 napi_comp;
>  	u64 tx_poll;
>  	u64 doorbells;
> -	u64 missing_tx_comp;
>  	u64 bad_req_id;
>  };
>  
> @@ -270,6 +280,8 @@ struct ena_adapter {
>  	struct msix_entry *msix_entries;
>  	int msix_vecs;
>  
> +	u32 missing_tx_completion_threshold;
> +
>  	u32 tx_usecs, rx_usecs; /* interrupt moderation */
>  	u32 tx_frames, rx_frames; /* interrupt moderation */
>  
> @@ -283,6 +295,9 @@ struct ena_adapter {
>  
>  	u8 mac_addr[ETH_ALEN];
>  
> +	unsigned long keep_alive_timeout;
> +	unsigned long missing_tx_completion_to;
> +
>  	char name[ENA_NAME_MAX_LEN];
>  
>  	unsigned long flags;
> diff --git a/drivers/net/ethernet/amazon/ena/ena_regs_defs.h b/drivers/net/ethernet/amazon/ena/ena_regs_defs.h
> index 26097a2..c3891c5 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_regs_defs.h
> +++ b/drivers/net/ethernet/amazon/ena/ena_regs_defs.h
> @@ -78,6 +78,8 @@
>  #define ENA_REGS_CAPS_RESET_TIMEOUT_MASK		0x3e
>  #define ENA_REGS_CAPS_DMA_ADDR_WIDTH_SHIFT		8
>  #define ENA_REGS_CAPS_DMA_ADDR_WIDTH_MASK		0xff00
> +#define ENA_REGS_CAPS_ADMIN_CMD_TO_SHIFT		16
> +#define ENA_REGS_CAPS_ADMIN_CMD_TO_MASK		0xf0000
>  
>  /* aq_caps register */
>  #define ENA_REGS_AQ_CAPS_AQ_DEPTH_MASK		0xffff

^ permalink raw reply

* Re: [PATCH V2 net 06/20] net/ena: fix NULL dereference when removing the driver after device reset faild
From: Matt Wilson @ 2016-12-05  4:29 UTC (permalink / raw)
  To: Netanel Belgazal
  Cc: linux-kernel, davem, netdev, dwmw, zorik, alex, saeed, msw,
	aliguori, nafea
In-Reply-To: <1480857578-5065-7-git-send-email-netanel@annapurnalabs.com>

On Sun, Dec 04, 2016 at 03:19:24PM +0200, Netanel Belgazal wrote:
> If for some reason the device stop responding and the device reset failed
> to recover the device, the mmio register read datastructure will not be
> reinitialized.

If for some reason the device stops responding, and the device reset
fails to recover the device, the MMIO register read data structure
will not be reinitialized.

> On driver removal, the driver will also tries to reset the device
> but this time the mmio data structure will be NULL.

On driver removal, the driver will also try to reset the device, but
this time the MMIO data structure will be NULL.

> To solve this issue perform the device reset in the remove function only if
> the device is runnig.

To solve this issue, perform the device reset in the remove function
only if the device is running.

Do you have an example of the NULL pointer dereference that you can
paste in? It can be helpful for those searching for a fix for a bug
they've experienced.

--msw

> Signed-off-by: Netanel Belgazal <netanel@annapurnalabs.com>
> ---
>  drivers/net/ethernet/amazon/ena/ena_netdev.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> index 224302c..ad5f78f 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> @@ -2516,6 +2516,8 @@ static void ena_fw_reset_device(struct work_struct *work)
>  err:
>  	rtnl_unlock();
>  
> +	clear_bit(ENA_FLAG_DEVICE_RUNNING, &adapter->flags);
> +
>  	dev_err(&pdev->dev,
>  		"Reset attempt failed. Can not reset the device\n");
>  }
> @@ -3126,7 +3128,9 @@ static void ena_remove(struct pci_dev *pdev)
>  
>  	cancel_work_sync(&adapter->resume_io_task);
>  
> -	ena_com_dev_reset(ena_dev);
> +	/* Reset the device only if the device is running. */
> +	if (test_bit(ENA_FLAG_DEVICE_RUNNING, &adapter->flags))
> +		ena_com_dev_reset(ena_dev);
>  
>  	ena_free_mgmnt_irq(adapter);
>  

^ permalink raw reply

* Re: [PATCH V2 net 07/20] net/ena: refactor ena_get_stats64 to be atomic context safe
From: Matt Wilson @ 2016-12-05  4:24 UTC (permalink / raw)
  To: Netanel Belgazal
  Cc: linux-kernel, davem, netdev, dwmw, zorik, alex, saeed, msw,
	aliguori, nafea
In-Reply-To: <1480857578-5065-8-git-send-email-netanel@annapurnalabs.com>

On Sun, Dec 04, 2016 at 03:19:25PM +0200, Netanel Belgazal wrote:
> ndo_get_stat64 can be called from atomic context.
> However the current implementation sends an admin command to retrieve
> the statistics from the device.
> This admin commands uses sleep.

Suggest some comment edits:

ndo_get_stat64() can be called from atomic context, but the current
implementation sends an admin command to retrieve the statistics from
the device. This admin command can sleep.

> Refactor the implementation of ena_get_stats64 to take the
> {rx,tx}bytes/cnt from the driver's inner counters
> and to take the rx drops counter
> from the asynchronous keep alive (heart bit) event.

This patch re-factors the implementation of ena_get_stats64() to use
the {rx,tx}bytes/count from the driver's inner counters, and to obtain
the rx drop counter from the asynchronous keep alive (heart bit)
event.

--msw

> Signed-off-by: Netanel Belgazal <netanel@annapurnalabs.com>
> ---
>  drivers/net/ethernet/amazon/ena/ena_admin_defs.h |  8 ++++
>  drivers/net/ethernet/amazon/ena/ena_netdev.c     | 57 +++++++++++++++++-------
>  drivers/net/ethernet/amazon/ena/ena_netdev.h     |  1 +
>  3 files changed, 51 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/ethernet/amazon/ena/ena_admin_defs.h b/drivers/net/ethernet/amazon/ena/ena_admin_defs.h
> index f48c886..6d70bf5 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_admin_defs.h
> +++ b/drivers/net/ethernet/amazon/ena/ena_admin_defs.h
> @@ -873,6 +873,14 @@ struct ena_admin_aenq_link_change_desc {
>  	u32 flags;
>  };
>  
> +struct ena_admin_aenq_keep_alive_desc {
> +	struct ena_admin_aenq_common_desc aenq_common_desc;
> +
> +	u32 rx_drops_low;
> +
> +	u32 rx_drops_high;
> +};
> +
>  struct ena_admin_ena_mmio_req_read_less_resp {
>  	u16 req_id;
>  
> diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> index ad5f78f..962ffb5 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> @@ -2176,28 +2176,46 @@ static struct rtnl_link_stats64 *ena_get_stats64(struct net_device *netdev,
>  						 struct rtnl_link_stats64 *stats)
>  {
>  	struct ena_adapter *adapter = netdev_priv(netdev);
> -	struct ena_admin_basic_stats ena_stats;
> -	int rc;
> +	struct ena_ring *rx_ring, *tx_ring;
> +	unsigned int start;
> +	u64 rx_drops;
> +	int i;
>  
>  	if (!test_bit(ENA_FLAG_DEV_UP, &adapter->flags))
>  		return NULL;
>  
> -	rc = ena_com_get_dev_basic_stats(adapter->ena_dev, &ena_stats);
> -	if (rc)
> -		return NULL;
> +	for (i = 0; i < adapter->num_queues; i++) {
> +		u64 bytes, packets;
> +
> +		tx_ring = &adapter->tx_ring[i];
> +
> +		do {
> +			start = u64_stats_fetch_begin_irq(&tx_ring->syncp);
> +			packets = tx_ring->tx_stats.cnt;
> +			bytes = tx_ring->tx_stats.bytes;
> +		} while (u64_stats_fetch_retry_irq(&tx_ring->syncp, start));
> +
> +		stats->tx_packets += packets;
> +		stats->tx_bytes += bytes;
>  
> -	stats->tx_bytes = ((u64)ena_stats.tx_bytes_high << 32) |
> -		ena_stats.tx_bytes_low;
> -	stats->rx_bytes = ((u64)ena_stats.rx_bytes_high << 32) |
> -		ena_stats.rx_bytes_low;
> +		rx_ring = &adapter->rx_ring[i];
> +
> +		do {
> +			start = u64_stats_fetch_begin_irq(&rx_ring->syncp);
> +			packets = rx_ring->rx_stats.cnt;
> +			bytes = rx_ring->rx_stats.bytes;
> +		} while (u64_stats_fetch_retry_irq(&rx_ring->syncp, start));
>  
> -	stats->rx_packets = ((u64)ena_stats.rx_pkts_high << 32) |
> -		ena_stats.rx_pkts_low;
> -	stats->tx_packets = ((u64)ena_stats.tx_pkts_high << 32) |
> -		ena_stats.tx_pkts_low;
> +		stats->rx_packets += packets;
> +		stats->rx_bytes += bytes;
> +	}
> +
> +	do {
> +		start = u64_stats_fetch_begin_irq(&adapter->syncp);
> +		rx_drops = adapter->dev_stats.rx_drops;
> +	} while (u64_stats_fetch_retry_irq(&adapter->syncp, start));
>  
> -	stats->rx_dropped = ((u64)ena_stats.rx_drops_high << 32) |
> -		ena_stats.rx_drops_low;
> +	stats->rx_dropped = rx_drops;
>  
>  	stats->multicast = 0;
>  	stats->collisions = 0;
> @@ -3221,8 +3239,17 @@ static void ena_keep_alive_wd(void *adapter_data,
>  			      struct ena_admin_aenq_entry *aenq_e)
>  {
>  	struct ena_adapter *adapter = (struct ena_adapter *)adapter_data;
> +	struct ena_admin_aenq_keep_alive_desc *desc;
> +	u64 rx_drops;
>  
> +	desc = (struct ena_admin_aenq_keep_alive_desc *)aenq_e;
>  	adapter->last_keep_alive_jiffies = jiffies;
> +
> +	rx_drops = ((u64)desc->rx_drops_high << 32) | desc->rx_drops_low;
> +
> +	u64_stats_update_begin(&adapter->syncp);
> +	adapter->dev_stats.rx_drops = rx_drops;
> +	u64_stats_update_end(&adapter->syncp);
>  }
>  
>  static void ena_notification(void *adapter_data,
> diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.h b/drivers/net/ethernet/amazon/ena/ena_netdev.h
> index 69d7e9e..f0ddc11 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_netdev.h
> +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.h
> @@ -241,6 +241,7 @@ struct ena_stats_dev {
>  	u64 interface_up;
>  	u64 interface_down;
>  	u64 admin_q_pause;
> +	u64 rx_drops;
>  };
>  
>  enum ena_flags_t {

^ permalink raw reply

* Re: [PATCH V2 net 05/20] net/ena: fix RSS default hash configuration
From: Matt Wilson @ 2016-12-05  4:20 UTC (permalink / raw)
  To: Netanel Belgazal
  Cc: linux-kernel, davem, netdev, dwmw, zorik, alex, saeed, msw,
	aliguori, nafea
In-Reply-To: <1480857578-5065-6-git-send-email-netanel@annapurnalabs.com>

On Sun, Dec 04, 2016 at 03:19:23PM +0200, Netanel Belgazal wrote:
> ENA default hash configure IPv4_frag hash twice instead of

configure -> configures. You may want to include "erroneously". What
is the consequence of this bug?

> configure non ip packets.

configuring non-IP packets.

--msw

> Signed-off-by: Netanel Belgazal <netanel@annapurnalabs.com>
> ---
>  drivers/net/ethernet/amazon/ena/ena_com.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/amazon/ena/ena_com.c b/drivers/net/ethernet/amazon/ena/ena_com.c
> index 3066d9c..46aad3a 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_com.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_com.c
> @@ -2184,7 +2184,7 @@ int ena_com_set_default_hash_ctrl(struct ena_com_dev *ena_dev)
>  	hash_ctrl->selected_fields[ENA_ADMIN_RSS_IP4_FRAG].fields =
>  		ENA_ADMIN_RSS_L3_SA | ENA_ADMIN_RSS_L3_DA;
>  
> -	hash_ctrl->selected_fields[ENA_ADMIN_RSS_IP4_FRAG].fields =
> +	hash_ctrl->selected_fields[ENA_ADMIN_RSS_NOT_IP].fields =
>  		ENA_ADMIN_RSS_L2_DA | ENA_ADMIN_RSS_L2_SA;
>  
>  	for (i = 0; i < ENA_ADMIN_RSS_PROTO_NUM; i++) {

^ permalink raw reply

* Re: [PATCH V2 net 04/20] net/ena: fix ethtool RSS flow configuration
From: Matt Wilson @ 2016-12-05  4:18 UTC (permalink / raw)
  To: Netanel Belgazal
  Cc: linux-kernel, davem, netdev, dwmw, zorik, alex, saeed, msw,
	aliguori, nafea
In-Reply-To: <1480857578-5065-5-git-send-email-netanel@annapurnalabs.com>

On Sun, Dec 04, 2016 at 03:19:22PM +0200, Netanel Belgazal wrote:
> ena_flow_data_to_flow_hash and ena_flow_hash_to_flow_type
> treat the ena_flow_hash_to_flow_type enum as power of two values.
> 
> Change the values of ena_admin_flow_hash_fields to be power of two values.

Then I generally prefer BIT(0), BIT(1), BIT(2), etc.

Also it would be helpful to include some comments about the
consequences of the current state of the code.

--msw

> Signed-off-by: Netanel Belgazal <netanel@annapurnalabs.com>
> ---
>  drivers/net/ethernet/amazon/ena/ena_admin_defs.h | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/amazon/ena/ena_admin_defs.h b/drivers/net/ethernet/amazon/ena/ena_admin_defs.h
> index a46e749..f48c886 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_admin_defs.h
> +++ b/drivers/net/ethernet/amazon/ena/ena_admin_defs.h
> @@ -631,22 +631,22 @@ enum ena_admin_flow_hash_proto {
>  /* RSS flow hash fields */
>  enum ena_admin_flow_hash_fields {
>  	/* Ethernet Dest Addr */
> -	ENA_ADMIN_RSS_L2_DA	= 0,
> +	ENA_ADMIN_RSS_L2_DA	= 0x1,
>  
>  	/* Ethernet Src Addr */
> -	ENA_ADMIN_RSS_L2_SA	= 1,
> +	ENA_ADMIN_RSS_L2_SA	= 0x2,
>  
>  	/* ipv4/6 Dest Addr */
> -	ENA_ADMIN_RSS_L3_DA	= 2,
> +	ENA_ADMIN_RSS_L3_DA	= 0x4,
>  
>  	/* ipv4/6 Src Addr */
> -	ENA_ADMIN_RSS_L3_SA	= 5,
> +	ENA_ADMIN_RSS_L3_SA	= 0x8,
>  
>  	/* tcp/udp Dest Port */
> -	ENA_ADMIN_RSS_L4_DP	= 6,
> +	ENA_ADMIN_RSS_L4_DP	= 0x10,
>  
>  	/* tcp/udp Src Port */
> -	ENA_ADMIN_RSS_L4_SP	= 7,
> +	ENA_ADMIN_RSS_L4_SP	= 0x20,
>  };
>  
>  struct ena_admin_proto_input {

^ permalink raw reply

* Re: [PATCH V2 net 03/20] net/ena: fix queues number calculation
From: Matt Wilson @ 2016-12-05  4:11 UTC (permalink / raw)
  To: Netanel Belgazal
  Cc: linux-kernel, davem, netdev, dwmw, zorik, alex, saeed, msw,
	aliguori, nafea
In-Reply-To: <1480857578-5065-4-git-send-email-netanel@annapurnalabs.com>

On Sun, Dec 04, 2016 at 03:19:21PM +0200, Netanel Belgazal wrote:
> The ENA driver tries to open a queue per vCPU.
> To determine how many vCPUs the instance have it uses num_possible_cpus
> while it should have use num_online_cpus instead.

use () when referring to functions: num_possible_cpus(), num_online_cpus().

> Signed-off-by: Netanel Belgazal <netanel@annapurnalabs.com>

Reviewed-by: Matt Wilson <msw@amazon.com>

> ---
>  drivers/net/ethernet/amazon/ena/ena_netdev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> index 397c9bc..224302c 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> @@ -2667,7 +2667,7 @@ static int ena_calc_io_queue_num(struct pci_dev *pdev,
>  		io_sq_num = get_feat_ctx->max_queues.max_sq_num;
>  	}
>  
> -	io_queue_num = min_t(int, num_possible_cpus(), ENA_MAX_NUM_IO_QUEUES);
> +	io_queue_num = min_t(int, num_online_cpus(), ENA_MAX_NUM_IO_QUEUES);
>  	io_queue_num = min_t(int, io_queue_num, io_sq_num);
>  	io_queue_num = min_t(int, io_queue_num,
>  			     get_feat_ctx->max_queues.max_cq_num);

^ permalink raw reply

* Re: [PATCH V2 net 02/20] net/ena: fix error handling when probe fails
From: Matt Wilson @ 2016-12-05  4:09 UTC (permalink / raw)
  To: Netanel Belgazal
  Cc: linux-kernel, davem, netdev, dwmw, zorik, alex, saeed, msw,
	aliguori, nafea
In-Reply-To: <1480857578-5065-3-git-send-email-netanel@annapurnalabs.com>

On Sun, Dec 04, 2016 at 03:19:20PM +0200, Netanel Belgazal wrote:
> When driver fails in probe, it will release all resources, including
> adapter.
> In case of probe failure, ena_remove should not try to free the adapter
> resources.

Please word wrap your commit message around 75 columns.

> Signed-off-by: Netanel Belgazal <netanel@annapurnalabs.com>

Reviewed-by: Matt Wilson <msw@amazon.com>

> ---
>  drivers/net/ethernet/amazon/ena/ena_netdev.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> index 33a760e..397c9bc 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> @@ -3054,6 +3054,7 @@ static int ena_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  err_free_region:
>  	ena_release_bars(ena_dev, pdev);
>  err_free_ena_dev:
> +	pci_set_drvdata(pdev, NULL);
>  	vfree(ena_dev);
>  err_disable_device:
>  	pci_disable_device(pdev);

^ permalink raw reply

* Re: [PATCH V2 net 01/20] net/ena: remove ntuple filter support from device feature list
From: Matt Wilson @ 2016-12-05  4:08 UTC (permalink / raw)
  To: Netanel Belgazal
  Cc: linux-kernel, davem, netdev, dwmw, zorik, alex, saeed, msw,
	aliguori, nafea
In-Reply-To: <1480857578-5065-2-git-send-email-netanel@annapurnalabs.com>

On Sun, Dec 04, 2016 at 03:19:19PM +0200, Netanel Belgazal wrote:
> Remove NETIF_F_NTUPLE from netdev->features.
> The ENA device driver does not support ntuple filtering.
> 
> Signed-off-by: Netanel Belgazal <netanel@annapurnalabs.com>

Reviewed-by: Matt Wilson <msw@amazon.com>

> ---
>  drivers/net/ethernet/amazon/ena/ena_netdev.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> index bfeaec5..33a760e 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> @@ -2729,7 +2729,6 @@ static void ena_set_dev_offloads(struct ena_com_dev_get_features_ctx *feat,
>  	netdev->features =
>  		dev_features |
>  		NETIF_F_SG |
> -		NETIF_F_NTUPLE |
>  		NETIF_F_RXHASH |
>  		NETIF_F_HIGHDMA;
>  

^ permalink raw reply

* Re: "af_unix: conditionally use freezable blocking calls in read" is wrong
From: Al Viro @ 2016-12-05  3:52 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, xiyou.wangcong
In-Reply-To: <20161204.214214.1868750446620130834.davem@davemloft.net>

On Sun, Dec 04, 2016 at 09:42:14PM -0500, David Miller wrote:
> > 	I've run into that converting AF_UNIX to generic_file_splice_read();
> > I can kludge around that ("freezable unless ->msg_iter is ITER_PIPE"), but
> > that only delays trouble.
> > 
> > 	Note that the only other user of freezable_schedule_timeout() is
> > a very different story - it's a kernel thread, which *does* have a guaranteed
> > locking environment.  Making such assumptions in unix_stream_recvmsg(),
> > OTOH, is insane...
> 
> We have to otherwise Android phones drain their batteries in 10
> minutes.
> 
> I'm not going to revert this and be responsible for that.
> 
> So you have to find a way to make the freezable calls legitimate.

Oh, well...  As I said, I can kludge around that - call from
generic_file_splice_read() can be distinguished by looking at the
->msg_iter->type; it still means unpleasantness for kernel_recvmsg()
users - in effect, it can only be called with locks held if you know that
the socket is not an AF_UNIX one.

BTW, how do they deal with plain pipes?

^ permalink raw reply

* [PATCH] ixgbe: initialize u64_stats_sync structures early at ixgbe_probe
From: Liwei Song @ 2016-12-05  3:40 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: intel-wired-lan, netdev, linux-kernel, liwei.song

Fix the following CallTrace:
INFO: trying to register non-static key.
the code is fine but needs lockdep annotation.
turning off the locking correctness validator.
CPU: 71 PID: 1 Comm: swapper/0 Not tainted 4.8.8-WR9.0.0.1_standard #11
Hardware name: Intel Corporation S2600WTT/S2600WTT,
BIOS GRNDSDP1.86B.0036.R05.1407140519 07/14/2014
 00200086 00200086 eb5e1ab8 c144dd70 00000000 00000000 eb5e1af8 c10af89a
 c1d23de4 eb5e1af8 00000009 eb5d8600 eb5d8638 eb5e1af8 c10b14d8 00000009
 0000000a c1d32911 00000000 00000000 e44c826c eb5d8000 eb5e1b74 c10b214e
Call Trace:
 [<c144dd70>] dump_stack+0x5f/0x8f
 [<c10af89a>] register_lock_class+0x25a/0x4c0
 [<c10b14d8>] ? check_irq_usage+0x88/0xc0
 [<c10b214e>] __lock_acquire+0x5e/0x17a0
 [<c1abdb9b>] ? _raw_spin_unlock_irqrestore+0x3b/0x70
 [<c10cf14a>] ? rcu_read_lock_sched_held+0x8a/0x90
 [<c10b3c5f>] lock_acquire+0x9f/0x1f0
 [<c1922dcf>] ? dev_get_stats+0x5f/0x110
 [<c176e6b3>] ixgbe_get_stats64+0x113/0x320
 [<c1922dcf>] ? dev_get_stats+0x5f/0x110
 [<c1922dcf>] dev_get_stats+0x5f/0x110
 [<c1ab5415>] rtnl_fill_stats+0x40/0x105
 [<c193dd45>] rtnl_fill_ifinfo+0x4c5/0xd20
 [<c11c5115>] ? __kmalloc_node_track_caller+0x1a5/0x410
 [<c1917487>] ? __kmalloc_reserve.isra.42+0x27/0x80
 [<c191754f>] ? __alloc_skb+0x6f/0x270
 [<c1942291>] rtmsg_ifinfo_build_skb+0x71/0xd0
 [<c194230a>] rtmsg_ifinfo.part.23+0x1a/0x50
 [<c1923dad>] ? call_netdevice_notifiers_info+0x2d/0x60
 [<c194236b>] rtmsg_ifinfo+0x2b/0x40
 [<c192f997>] register_netdevice+0x3d7/0x4d0
 [<c192faa7>] register_netdev+0x17/0x30
 [<c177b83d>] ixgbe_probe+0x118d/0x1610
 [<c1498202>] local_pci_probe+0x32/0x80
 [<c1498172>] ? pci_match_device+0xd2/0x100
 [<c14991e0>] pci_device_probe+0xc0/0x110
 [<c1652cc5>] driver_probe_device+0x1c5/0x280
 [<c1498172>] ? pci_match_device+0xd2/0x100
 [<c1652e09>] __driver_attach+0x89/0x90
 [<c1652d80>] ? driver_probe_device+0x280/0x280
 [<c165114f>] bus_for_each_dev+0x4f/0x80
 [<c165269e>] driver_attach+0x1e/0x20
 [<c1652d80>] ? driver_probe_device+0x280/0x280
 [<c1652317>] bus_add_driver+0x1a7/0x220
 [<c1653a79>] driver_register+0x59/0xe0
 [<c1f897b8>] ? igb_init_module+0x49/0x49
 [<c1497b2a>] __pci_register_driver+0x4a/0x50
 [<c1f8985d>] ixgbe_init_module+0xa5/0xc4
 [<c1000485>] do_one_initcall+0x35/0x150
 [<c107e818>] ? parameq+0x18/0x70
 [<c1f395d8>] ? repair_env_string+0x12/0x51
 [<c107ead0>] ? parse_args+0x260/0x3b0
 [<c1074f73>] ? __usermodehelper_set_disable_depth+0x43/0x50
 [<c1f39e90>] kernel_init_freeable+0x19b/0x267
 [<c1f395c6>] ? set_debug_rodata+0xf/0xf
 [<c10b1e7b>] ? trace_hardirqs_on+0xb/0x10
 [<c1abdc02>] ? _raw_spin_unlock_irq+0x32/0x50
 [<c1085f0b>] ? finish_task_switch+0xab/0x1f0
 [<c1085ec9>] ? finish_task_switch+0x69/0x1f0
 [<c1ab6a30>] kernel_init+0x10/0x110
 [<c108bd65>] ? schedule_tail+0x25/0x80
 [<c1abe422>] ret_from_kernel_thread+0xe/0x24
 [<c1ab6a20>] ? rest_init+0x130/0x130

This CallTrace occurred on 32-bit kernel with CONFIG_PROVE_LOCKING
enabled.

This happens at ixgbe driver probe hardware stage, when comes to
ixgbe_get_stats64, the seqcount/seqlock still not initialize, although
this was initialize in TX/RX resources setup routin, but it was too late,
then lockdep give this Warning.

To fix this, move the u64_stats_init function to driver probe stage,
which before we get the status of seqcount and after the RX/TX ring
was finished init.

Signed-off-by: Liwei Song <liwei.song@windriver.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index fee1f29..ab1d114 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -5811,8 +5811,6 @@ int ixgbe_setup_tx_resources(struct ixgbe_ring *tx_ring)
 	if (!tx_ring->tx_buffer_info)
 		goto err;
 
-	u64_stats_init(&tx_ring->syncp);
-
 	/* round up to nearest 4K */
 	tx_ring->size = tx_ring->count * sizeof(union ixgbe_adv_tx_desc);
 	tx_ring->size = ALIGN(tx_ring->size, 4096);
@@ -5895,8 +5893,6 @@ int ixgbe_setup_rx_resources(struct ixgbe_ring *rx_ring)
 	if (!rx_ring->rx_buffer_info)
 		goto err;
 
-	u64_stats_init(&rx_ring->syncp);
-
 	/* Round up to nearest 4K */
 	rx_ring->size = rx_ring->count * sizeof(union ixgbe_adv_rx_desc);
 	rx_ring->size = ALIGN(rx_ring->size, 4096);
@@ -9686,6 +9682,12 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (err)
 		goto err_sw_init;
 
+	for (i = 0; i < adapter->num_rx_queues; i++)
+		u64_stats_init(&adapter->rx_ring[i]->syncp);
+
+	for (i = 0; i < adapter->num_tx_queues; i++)
+		u64_stats_init(&adapter->tx_ring[i]->syncp);
+
 	/* WOL not supported for all devices */
 	adapter->wol = 0;
 	hw->eeprom.ops.read(hw, 0x2c, &adapter->eeprom_cap);
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH] net: ping: check minimum size on ICMP header length
From: Lorenzo Colitti @ 2016-12-05  3:35 UTC (permalink / raw)
  To: Kees Cook
  Cc: David S. Miller, netdev@vger.kernel.org, Min Chong, Qidan He,
	Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
	Patrick McHardy, lkml
In-Reply-To: <20161203005853.GA117599@beast>

On Sat, Dec 3, 2016 at 9:58 AM, Kees Cook <keescook@chromium.org> wrote:
> -       if (len > 0xFFFF)
> +       if (len > 0xFFFF || len < icmph_len)
>                 return -EMSGSIZE;

EMSGSIZE usually means the message is too long. Maybe use EINVAL?
That's what the code will return if the passed-in ICMP header is
invalid (e.g., is not an echo request).

^ permalink raw reply

* Re: [PATCH V2 net 00/20] Increase ENA driver version to 1.1.2
From: Matt Wilson @ 2016-12-05  3:30 UTC (permalink / raw)
  To: David Miller
  Cc: netanel, linux-kernel, netdev, dwmw, zorik, alex, saeed, msw,
	aliguori, nafea
In-Reply-To: <20161204.213743.1610654756480441815.davem@davemloft.net>

On Sun, Dec 04, 2016 at 09:37:43PM -0500, David Miller wrote:
> 
> It is not appropriate to submit so many patches at one time.

Indeed, https://www.kernel.org/doc/Documentation/SubmittingPatches
recommends submitting no more than 15 or so at once.

> Please keep your patch series to no more than about a dozen
> at a time.

How about 15 from SubmittingPatches? The first 15 in the series are
all important bugfixes. Should Netanel resubmit a series with just the
bugfixes and a new cover letter? Or are you willing to consider the
first 15 of this series as posted?

> Also, group your changes logically and tie an appropriately
> descriptive cover letter.
> 
> "Increase driver version to X.Y.Z" tells the reader absolutely
> nothing.  Someone reading that Subject line in the GIT logs
> will have no idea what the overall purpose of the patch series
> is and what it accomplishes.

You're right, the cover letter subject needs to be better. There is
only one commit submitted with the subject "increase driver version to
1.1.2." - Patch 20/20. It is logically like:

commit b8b2372de9cc00d5ed667c7b8db29b6cfbf037f5
Author: Manish Chopra <manish.chopra@qlogic.com>
Date:   Wed Aug 3 04:02:04 2016 -0400

    qlcnic: Update version to 5.3.65
    
    Signed-off-by: Manish Chopra <manish.chopra@qlogic.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

[...]

commit ae33256c55d2fefcad8712e750b846461994a1af
Author: Bimmy Pujari <bimmy.pujari@intel.com>
Date:   Mon Jun 20 09:10:39 2016 -0700

    i40e/i40evf-bump version to 1.6.11
    
    Signed-off-by: Bimmy Pujari <bimmy.pujari@intel.com>
    Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
    Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

[...]

commit 5264cc63ba10ebfa0e54e3e641cce2656c7a60e8
Author: Jacob Keller <jacob.e.keller@intel.com>
Date:   Tue Jun 7 16:09:02 2016 -0700

    fm10k: bump version number
    
    Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
    Tested-by: Krishneil Singh <Krishneil.k.singh@intel.com>
    Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

[...]

commit a58a3e68037647de78e3461194239a1104f76003
Author: Michael Chan <michael.chan@broadcom.com>
Date:   Fri Jul 1 18:46:20 2016 -0400

    bnxt_en: Update firmware spec. to 1.3.0.
    
    And update driver version to 1.3.0.
    
    Signed-off-by: Michael Chan <michael.chan@broadcom.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

> You really need to describe the high level purpose of the patch set.
> Is it adding a new feature?  What is that feature?  Why are you
> adding that feature?  How is that feature implemented?  Why is
> it implemented that way?

The priority is to get bug fixes to the ENA driver in 4.9. Let's focus
on the first 15.

--msw

^ permalink raw reply

* [PATCH net] net: ep93xx_eth: Do not crash unloading module
From: Florian Fainelli @ 2016-12-05  3:22 UTC (permalink / raw)
  To: netdev; +Cc: davem, hsweeten, Florian Fainelli

When we unload the ep93xx_eth, whether we have opened the network
interface or not, we will either hit a kernel paging request error, or a
simple NULL pointer de-reference because:

- if ep93xx_open has been called, we have created a valid DMA mapping
  for ep->descs, when we call ep93xx_stop, we also call
  ep93xx_free_buffers, ep->descs now has a stale value

- if ep93xx_open has not been called, we have a NULL pointer for
  ep->descs, so performing any operation against that address just won't
  work

Fix this by adding a NULL pointer check for ep->descs which means that
ep93xx_free_buffers() was able to successfully tear down the descriptors
and free the DMA cookie as well.

Fixes: 1d22e05df818 ("[PATCH] Cirrus Logic ep93xx ethernet driver")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/cirrus/ep93xx_eth.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/cirrus/ep93xx_eth.c b/drivers/net/ethernet/cirrus/ep93xx_eth.c
index de9f7c97d916..9a161e981529 100644
--- a/drivers/net/ethernet/cirrus/ep93xx_eth.c
+++ b/drivers/net/ethernet/cirrus/ep93xx_eth.c
@@ -468,6 +468,9 @@ static void ep93xx_free_buffers(struct ep93xx_priv *ep)
 	struct device *dev = ep->dev->dev.parent;
 	int i;
 
+	if (!ep->descs)
+		return;
+
 	for (i = 0; i < RX_QUEUE_ENTRIES; i++) {
 		dma_addr_t d;
 
@@ -490,6 +493,7 @@ static void ep93xx_free_buffers(struct ep93xx_priv *ep)
 
 	dma_free_coherent(dev, sizeof(struct ep93xx_descs), ep->descs,
 							ep->descs_dma_addr);
+	ep->descs = NULL;
 }
 
 static int ep93xx_alloc_buffers(struct ep93xx_priv *ep)
-- 
2.9.3

^ permalink raw reply related

* Re: [Patch net-next] act_mirred: fix a typo in get_dev
From: Hadar Hen Zion @ 2016-12-04 13:12 UTC (permalink / raw)
  To: Cong Wang, netdev; +Cc: Jiri Pirko
In-Reply-To: <1480790161-8097-1-git-send-email-xiyou.wangcong@gmail.com>



On 12/3/2016 8:36 PM, Cong Wang wrote:
> Cc: Hadar Hen Zion <hadarh@mellanox.com>
> Cc: Jiri Pirko <jiri@mellanox.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
>   net/sched/act_mirred.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
> index bb09ba3..2d9fa6e 100644
> --- a/net/sched/act_mirred.c
> +++ b/net/sched/act_mirred.c
> @@ -321,7 +321,7 @@ static int tcf_mirred_device(const struct tc_action *a, struct net *net,
>   	int ifindex = tcf_mirred_ifindex(a);
>   
>   	*mirred_dev = __dev_get_by_index(net, ifindex);
> -	if (!mirred_dev)
> +	if (!*mirred_dev)
>   		return -EINVAL;
>   	return 0;
>   }
Thank you for this fix! good catch.
I know it's already applied.

Hadar

^ permalink raw reply

* Re: [RFC PATCH 2/2] Documentation: devictree: Add macb mdio bindings
From: Harini Katakam @ 2016-12-05  3:03 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Rob Herring, Harini Katakam, Nicolas Ferre, David Miller,
	Pawel Moll, Mark Rutland,
	ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org,
	Kumar Gala, Boris Brezillon,
	alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	michals-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org
In-Reply-To: <bfbc84ce-4975-3213-3b46-8c394c717ea9-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Hi Florian,

On Sun, Dec 4, 2016 at 4:10 AM, Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Le 12/03/16 à 13:35, Rob Herring a écrit :
>> On Mon, Nov 28, 2016 at 03:19:27PM +0530, Harini Katakam wrote:
>>> +- reg: Address and length of the register set of MAC to be used
>>> +- clock-names: Tuple listing input clock names.
>>> +    Required elements: 'pclk', 'hclk'
>>> +    Optional elements: 'tx_clk'
>>> +- clocks: Phandles to input clocks.
>
> You are also missing mandatory properties:
>
> #address-cells = <1> and #size-cells = <0>
>
> Where is patch 1? Can you make sure you have the same recipient list for
> both patches in this series so we can review both the binding and driver?
>

Thanks for review, I'll update.

I did send the cover letter, patch 1 and 2 to the same recipient list.
I can see them on the mailing list. The first patch is called:
[RFC PATCH 1/2] net: macb: Add MDIO driver for accessing multiple PHY devices
I hope you can find it.

Regards,
Harini
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC PATCH 2/2] Documentation: devictree: Add macb mdio bindings
From: Harini Katakam @ 2016-12-05  2:55 UTC (permalink / raw)
  To: Rob Herring
  Cc: Harini Katakam, Nicolas Ferre, David Miller, Pawel Moll,
	Mark Rutland,
	ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org,
	Kumar Gala, Boris Brezillon,
	alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	michals-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org
In-Reply-To: <20161203213553.f3agwvaseufglnq6@rob-hp-laptop>

Hi Rob,


Thanks for the review.
On Sun, Dec 4, 2016 at 3:05 AM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Mon, Nov 28, 2016 at 03:19:27PM +0530, Harini Katakam wrote:
<snip>
>> +Required properties:
>> +- compatible: Should be "cdns,macb-mdio"
>
> Only one version ever? This needs more specific compatible strings.
>

This is part of the Cadence MAC in a way.
I can use revision number from the Cadence spec I was working with.
But it is not necessarily specific that version.

I'll take care of the other comments in the next version.

Regards,
Harini
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: "af_unix: conditionally use freezable blocking calls in read" is wrong
From: David Miller @ 2016-12-05  2:42 UTC (permalink / raw)
  To: viro; +Cc: netdev, xiyou.wangcong
In-Reply-To: <20161204210455.GI1555@ZenIV.linux.org.uk>

From: Al Viro <viro@ZenIV.linux.org.uk>
Date: Sun, 4 Dec 2016 21:04:55 +0000

> 	Could we please kill that kludge?  "af_unix: use freezable blocking
> calls in read" had been wrong to start with; having a method make assumptions
> of that sort ("nobody will call me while holding locks I hadn't thought of")
> is asking for serious trouble.  splice is just a place where lockdep has
> caught that - we *can't* assume that nobody will ever call kernel_recvmsg()
> while holding some locks.
> 
> 	I've run into that converting AF_UNIX to generic_file_splice_read();
> I can kludge around that ("freezable unless ->msg_iter is ITER_PIPE"), but
> that only delays trouble.
> 
> 	Note that the only other user of freezable_schedule_timeout() is
> a very different story - it's a kernel thread, which *does* have a guaranteed
> locking environment.  Making such assumptions in unix_stream_recvmsg(),
> OTOH, is insane...

We have to otherwise Android phones drain their batteries in 10
minutes.

I'm not going to revert this and be responsible for that.

So you have to find a way to make the freezable calls legitimate.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox