All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH for-next V2 0/4] RDMA: Cleanups and improvements
@ 2019-07-31 20:24 Kamal Heib
  2019-07-31 20:24 ` [PATCH for-next V2 1/4] RDMA: Introduce ib_port_phys_state enum Kamal Heib
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Kamal Heib @ 2019-07-31 20:24 UTC (permalink / raw)
  To: linux-rdma
  Cc: Jason Gunthorpe, Doug Ledford, Potnuri Bharat Teja,
	Selvin Xavier, Gal Pressman, Leon Romanovsky, Michal Kalderon,
	Christian Benvenuti, Moni Shoua, Bernard Metzler, Shiraz Saleem,
	Kamal Heib

This series includes few cleanups and improvements, the first patch
introduce a new enum for describing the physical state values and use it
instead of using the magic numbers, patch 2-4 add support for a common
query port for iWARP drivers and remove the common code from the iWARP
drivers.

Changes from v1 :
- Patch #3:
-- Delete __ prefix.
-- Add missing dev_put(netdev);
-- Initilize gid to {}.
-- Return error code directly.

Kamal Heib (4):
  RDMA: Introduce ib_port_phys_state enum
  RDMA/cxgb3: Use ib_device_set_netdev()
  RDMA/core: Add common iWARP query port
  RDMA/{cxgb3, cxgb4, i40iw}: Remove common code

 drivers/infiniband/core/device.c             | 87 ++++++++++++++++----
 drivers/infiniband/core/sysfs.c              | 24 ++++--
 drivers/infiniband/hw/bnxt_re/ib_verbs.c     |  4 +-
 drivers/infiniband/hw/cxgb3/iwch_provider.c  | 45 +++++-----
 drivers/infiniband/hw/cxgb4/provider.c       | 24 ------
 drivers/infiniband/hw/efa/efa_verbs.c        |  2 +-
 drivers/infiniband/hw/i40iw/i40iw_verbs.c    | 11 ---
 drivers/infiniband/hw/mlx5/main.c            |  4 +-
 drivers/infiniband/hw/ocrdma/ocrdma_verbs.c  |  4 +-
 drivers/infiniband/hw/qedr/verbs.c           |  4 +-
 drivers/infiniband/hw/usnic/usnic_ib_verbs.c |  7 +-
 drivers/infiniband/sw/rxe/rxe.h              |  4 -
 drivers/infiniband/sw/rxe/rxe_param.h        |  2 +-
 drivers/infiniband/sw/rxe/rxe_verbs.c        |  6 +-
 drivers/infiniband/sw/siw/siw_verbs.c        |  3 +-
 include/rdma/ib_verbs.h                      | 10 +++
 16 files changed, 136 insertions(+), 105 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH for-next V2 1/4] RDMA: Introduce ib_port_phys_state enum
  2019-07-31 20:24 [PATCH for-next V2 0/4] RDMA: Cleanups and improvements Kamal Heib
@ 2019-07-31 20:24 ` Kamal Heib
  2019-07-31 20:32   ` Andrew Boyer
  2019-07-31 20:24 ` [PATCH for-next V2 2/4] RDMA/cxgb3: Use ib_device_set_netdev() Kamal Heib
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Kamal Heib @ 2019-07-31 20:24 UTC (permalink / raw)
  To: linux-rdma
  Cc: Jason Gunthorpe, Doug Ledford, Potnuri Bharat Teja,
	Selvin Xavier, Gal Pressman, Leon Romanovsky, Michal Kalderon,
	Christian Benvenuti, Moni Shoua, Bernard Metzler, Shiraz Saleem,
	Kamal Heib

In order to improve readability, add ib_port_phys_state enum to replace
the use of magic numbers.

Signed-off-by: Kamal Heib <kamalheib1@gmail.com>
---
 drivers/infiniband/core/sysfs.c              | 24 +++++++++++++-------
 drivers/infiniband/hw/bnxt_re/ib_verbs.c     |  4 ++--
 drivers/infiniband/hw/efa/efa_verbs.c        |  2 +-
 drivers/infiniband/hw/mlx5/main.c            |  4 ++--
 drivers/infiniband/hw/ocrdma/ocrdma_verbs.c  |  4 ++--
 drivers/infiniband/hw/qedr/verbs.c           |  4 ++--
 drivers/infiniband/hw/usnic/usnic_ib_verbs.c |  7 +++---
 drivers/infiniband/sw/rxe/rxe.h              |  4 ----
 drivers/infiniband/sw/rxe/rxe_param.h        |  2 +-
 drivers/infiniband/sw/rxe/rxe_verbs.c        |  6 ++---
 drivers/infiniband/sw/siw/siw_verbs.c        |  3 ++-
 include/rdma/ib_verbs.h                      | 10 ++++++++
 12 files changed, 45 insertions(+), 29 deletions(-)

diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c
index b477295a96c2..46722e04f6e1 100644
--- a/drivers/infiniband/core/sysfs.c
+++ b/drivers/infiniband/core/sysfs.c
@@ -301,14 +301,22 @@ static ssize_t phys_state_show(struct ib_port *p, struct port_attribute *unused,
 		return ret;
 
 	switch (attr.phys_state) {
-	case 1:  return sprintf(buf, "1: Sleep\n");
-	case 2:  return sprintf(buf, "2: Polling\n");
-	case 3:  return sprintf(buf, "3: Disabled\n");
-	case 4:  return sprintf(buf, "4: PortConfigurationTraining\n");
-	case 5:  return sprintf(buf, "5: LinkUp\n");
-	case 6:  return sprintf(buf, "6: LinkErrorRecovery\n");
-	case 7:  return sprintf(buf, "7: Phy Test\n");
-	default: return sprintf(buf, "%d: <unknown>\n", attr.phys_state);
+	case IB_PORT_PHYS_STATE_SLEEP:
+		return sprintf(buf, "1: Sleep\n");
+	case IB_PORT_PHYS_STATE_POLLING:
+		return sprintf(buf, "2: Polling\n");
+	case IB_PORT_PHYS_STATE_DISABLED:
+		return sprintf(buf, "3: Disabled\n");
+	case IB_PORT_PHYS_STATE_PORT_CONFIGURATION_TRAINING:
+		return sprintf(buf, "4: PortConfigurationTraining\n");
+	case IB_PORT_PHYS_STATE_LINK_UP:
+		return sprintf(buf, "5: LinkUp\n");
+	case IB_PORT_PHYS_STATE_LINK_ERROR_RECOVERY:
+		return sprintf(buf, "6: LinkErrorRecovery\n");
+	case IB_PORT_PHYS_STATE_PHY_TEST:
+		return sprintf(buf, "7: Phy Test\n");
+	default:
+		return sprintf(buf, "%d: <unknown>\n", attr.phys_state);
 	}
 }
 
diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
index a91653aabf38..ca6306c24881 100644
--- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
+++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
@@ -220,10 +220,10 @@ int bnxt_re_query_port(struct ib_device *ibdev, u8 port_num,
 
 	if (netif_running(rdev->netdev) && netif_carrier_ok(rdev->netdev)) {
 		port_attr->state = IB_PORT_ACTIVE;
-		port_attr->phys_state = 5;
+		port_attr->phys_state = IB_PORT_PHYS_STATE_LINK_UP;
 	} else {
 		port_attr->state = IB_PORT_DOWN;
-		port_attr->phys_state = 3;
+		port_attr->phys_state = IB_PORT_PHYS_STATE_DISABLED;
 	}
 	port_attr->max_mtu = IB_MTU_4096;
 	port_attr->active_mtu = iboe_get_mtu(rdev->netdev->mtu);
diff --git a/drivers/infiniband/hw/efa/efa_verbs.c b/drivers/infiniband/hw/efa/efa_verbs.c
index df77bc312a25..f36071a92f97 100644
--- a/drivers/infiniband/hw/efa/efa_verbs.c
+++ b/drivers/infiniband/hw/efa/efa_verbs.c
@@ -306,7 +306,7 @@ int efa_query_port(struct ib_device *ibdev, u8 port,
 	props->lmc = 1;
 
 	props->state = IB_PORT_ACTIVE;
-	props->phys_state = 5;
+	props->phys_state = IB_PORT_PHYS_STATE_LINK_UP;
 	props->gid_tbl_len = 1;
 	props->pkey_tbl_len = 1;
 	props->active_speed = IB_SPEED_EDR;
diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index c2a5780cb394..bc4d7dca170f 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -535,7 +535,7 @@ static int mlx5_query_port_roce(struct ib_device *device, u8 port_num,
 	props->max_msg_sz       = 1 << MLX5_CAP_GEN(dev->mdev, log_max_msg);
 	props->pkey_tbl_len     = 1;
 	props->state            = IB_PORT_DOWN;
-	props->phys_state       = 3;
+	props->phys_state       = IB_PORT_PHYS_STATE_DISABLED;
 
 	mlx5_query_nic_vport_qkey_viol_cntr(mdev, &qkey_viol_cntr);
 	props->qkey_viol_cntr = qkey_viol_cntr;
@@ -561,7 +561,7 @@ static int mlx5_query_port_roce(struct ib_device *device, u8 port_num,
 
 	if (netif_running(ndev) && netif_carrier_ok(ndev)) {
 		props->state      = IB_PORT_ACTIVE;
-		props->phys_state = 5;
+		props->phys_state = IB_PORT_PHYS_STATE_LINK_UP;
 	}
 
 	ndev_ib_mtu = iboe_get_mtu(ndev->mtu);
diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c b/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
index bccc11378109..e8267e590772 100644
--- a/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
+++ b/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
@@ -163,10 +163,10 @@ int ocrdma_query_port(struct ib_device *ibdev,
 	netdev = dev->nic_info.netdev;
 	if (netif_running(netdev) && netif_oper_up(netdev)) {
 		port_state = IB_PORT_ACTIVE;
-		props->phys_state = 5;
+		props->phys_state = IB_PORT_PHYS_STATE_LINK_UP;
 	} else {
 		port_state = IB_PORT_DOWN;
-		props->phys_state = 3;
+		props->phys_state = IB_PORT_PHYS_STATE_DISABLED;
 	}
 	props->max_mtu = IB_MTU_4096;
 	props->active_mtu = iboe_get_mtu(netdev->mtu);
diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c
index 27d90a84ea01..1373312aec58 100644
--- a/drivers/infiniband/hw/qedr/verbs.c
+++ b/drivers/infiniband/hw/qedr/verbs.c
@@ -221,10 +221,10 @@ int qedr_query_port(struct ib_device *ibdev, u8 port, struct ib_port_attr *attr)
 	/* *attr being zeroed by the caller, avoid zeroing it here */
 	if (rdma_port->port_state == QED_RDMA_PORT_UP) {
 		attr->state = IB_PORT_ACTIVE;
-		attr->phys_state = 5;
+		attr->phys_state = IB_PORT_PHYS_STATE_LINK_UP;
 	} else {
 		attr->state = IB_PORT_DOWN;
-		attr->phys_state = 3;
+		attr->phys_state = IB_PORT_PHYS_STATE_DISABLED;
 	}
 	attr->max_mtu = IB_MTU_4096;
 	attr->active_mtu = iboe_get_mtu(dev->ndev->mtu);
diff --git a/drivers/infiniband/hw/usnic/usnic_ib_verbs.c b/drivers/infiniband/hw/usnic/usnic_ib_verbs.c
index eeb07b245ef9..4f8f1d3eb559 100644
--- a/drivers/infiniband/hw/usnic/usnic_ib_verbs.c
+++ b/drivers/infiniband/hw/usnic/usnic_ib_verbs.c
@@ -356,13 +356,14 @@ int usnic_ib_query_port(struct ib_device *ibdev, u8 port,
 
 	if (!us_ibdev->ufdev->link_up) {
 		props->state = IB_PORT_DOWN;
-		props->phys_state = 3;
+		props->phys_state = IB_PORT_PHYS_STATE_DISABLED;
 	} else if (!us_ibdev->ufdev->inaddr) {
 		props->state = IB_PORT_INIT;
-		props->phys_state = 4;
+		props->phys_state =
+			IB_PORT_PHYS_STATE_PORT_CONFIGURATION_TRAINING;
 	} else {
 		props->state = IB_PORT_ACTIVE;
-		props->phys_state = 5;
+		props->phys_state = IB_PORT_PHYS_STATE_LINK_UP;
 	}
 
 	props->port_cap_flags = 0;
diff --git a/drivers/infiniband/sw/rxe/rxe.h b/drivers/infiniband/sw/rxe/rxe.h
index ecf6e659c0da..fb07eed9e402 100644
--- a/drivers/infiniband/sw/rxe/rxe.h
+++ b/drivers/infiniband/sw/rxe/rxe.h
@@ -65,10 +65,6 @@
  */
 #define RXE_UVERBS_ABI_VERSION		2
 
-#define RDMA_LINK_PHYS_STATE_LINK_UP	(5)
-#define RDMA_LINK_PHYS_STATE_DISABLED	(3)
-#define RDMA_LINK_PHYS_STATE_POLLING	(2)
-
 #define RXE_ROCE_V2_SPORT		(0xc000)
 
 static inline u32 rxe_crc32(struct rxe_dev *rxe,
diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h
index 1abed47ca221..fe5207386700 100644
--- a/drivers/infiniband/sw/rxe/rxe_param.h
+++ b/drivers/infiniband/sw/rxe/rxe_param.h
@@ -154,7 +154,7 @@ enum rxe_port_param {
 	RXE_PORT_ACTIVE_WIDTH		= IB_WIDTH_1X,
 	RXE_PORT_ACTIVE_SPEED		= 1,
 	RXE_PORT_PKEY_TBL_LEN		= 64,
-	RXE_PORT_PHYS_STATE		= 2,
+	RXE_PORT_PHYS_STATE		= IB_PORT_PHYS_STATE_POLLING,
 	RXE_PORT_SUBNET_PREFIX		= 0xfe80000000000000ULL,
 };
 
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
index 4ebdfcf4d33e..623129f27f5a 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.c
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
@@ -69,11 +69,11 @@ static int rxe_query_port(struct ib_device *dev,
 			      &attr->active_width);
 
 	if (attr->state == IB_PORT_ACTIVE)
-		attr->phys_state = RDMA_LINK_PHYS_STATE_LINK_UP;
+		attr->phys_state = IB_PORT_PHYS_STATE_LINK_UP;
 	else if (dev_get_flags(rxe->ndev) & IFF_UP)
-		attr->phys_state = RDMA_LINK_PHYS_STATE_POLLING;
+		attr->phys_state = IB_PORT_PHYS_STATE_POLLING;
 	else
-		attr->phys_state = RDMA_LINK_PHYS_STATE_DISABLED;
+		attr->phys_state = IB_PORT_PHYS_STATE_DISABLED;
 
 	mutex_unlock(&rxe->usdev_lock);
 
diff --git a/drivers/infiniband/sw/siw/siw_verbs.c b/drivers/infiniband/sw/siw/siw_verbs.c
index 32dc79d0e898..404e7ca4b30c 100644
--- a/drivers/infiniband/sw/siw/siw_verbs.c
+++ b/drivers/infiniband/sw/siw/siw_verbs.c
@@ -206,7 +206,8 @@ int siw_query_port(struct ib_device *base_dev, u8 port,
 	attr->gid_tbl_len = 1;
 	attr->max_msg_sz = -1;
 	attr->max_mtu = ib_mtu_int_to_enum(sdev->netdev->mtu);
-	attr->phys_state = sdev->state == IB_PORT_ACTIVE ? 5 : 3;
+	attr->phys_state = sdev->state == IB_PORT_ACTIVE ?
+		IB_PORT_PHYS_STATE_LINK_UP : IB_PORT_PHYS_STATE_DISABLED;
 	attr->pkey_tbl_len = 1;
 	attr->port_cap_flags = IB_PORT_CM_SUP | IB_PORT_DEVICE_MGMT_SUP;
 	attr->state = sdev->state;
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index c5f8a9f17063..27fe844cff42 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -451,6 +451,16 @@ enum ib_port_state {
 	IB_PORT_ACTIVE_DEFER	= 5
 };
 
+enum ib_port_phys_state {
+	IB_PORT_PHYS_STATE_SLEEP = 1,
+	IB_PORT_PHYS_STATE_POLLING = 2,
+	IB_PORT_PHYS_STATE_DISABLED = 3,
+	IB_PORT_PHYS_STATE_PORT_CONFIGURATION_TRAINING = 4,
+	IB_PORT_PHYS_STATE_LINK_UP = 5,
+	IB_PORT_PHYS_STATE_LINK_ERROR_RECOVERY = 6,
+	IB_PORT_PHYS_STATE_PHY_TEST = 7,
+};
+
 enum ib_port_width {
 	IB_WIDTH_1X	= 1,
 	IB_WIDTH_2X	= 16,
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH for-next V2 2/4] RDMA/cxgb3: Use ib_device_set_netdev()
  2019-07-31 20:24 [PATCH for-next V2 0/4] RDMA: Cleanups and improvements Kamal Heib
  2019-07-31 20:24 ` [PATCH for-next V2 1/4] RDMA: Introduce ib_port_phys_state enum Kamal Heib
@ 2019-07-31 20:24 ` Kamal Heib
  2019-07-31 20:24 ` [PATCH for-next V2 3/4] RDMA/core: Add common iWARP query port Kamal Heib
  2019-07-31 20:24 ` [PATCH for-next V2 4/4] RDMA/{cxgb3, cxgb4, i40iw}: Remove common code Kamal Heib
  3 siblings, 0 replies; 10+ messages in thread
From: Kamal Heib @ 2019-07-31 20:24 UTC (permalink / raw)
  To: linux-rdma
  Cc: Jason Gunthorpe, Doug Ledford, Potnuri Bharat Teja,
	Selvin Xavier, Gal Pressman, Leon Romanovsky, Michal Kalderon,
	Christian Benvenuti, Moni Shoua, Bernard Metzler, Shiraz Saleem,
	Kamal Heib

This change is required to associate the cxgb3 ib_dev with the
underlying net_device, so in the upcoming patch we can call
ib_device_get_netdev().

Signed-off-by: Kamal Heib <kamalheib1@gmail.com>
---
 drivers/infiniband/hw/cxgb3/iwch_provider.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/infiniband/hw/cxgb3/iwch_provider.c b/drivers/infiniband/hw/cxgb3/iwch_provider.c
index e775c1a1a450..5848e4727b2e 100644
--- a/drivers/infiniband/hw/cxgb3/iwch_provider.c
+++ b/drivers/infiniband/hw/cxgb3/iwch_provider.c
@@ -1273,8 +1273,24 @@ static const struct ib_device_ops iwch_dev_ops = {
 	INIT_RDMA_OBJ_SIZE(ib_ucontext, iwch_ucontext, ibucontext),
 };
 
+static int set_netdevs(struct ib_device *ib_dev, struct cxio_rdev *rdev)
+{
+	int ret;
+	int i;
+
+	for (i = 0; i < rdev->port_info.nports; i++) {
+		ret = ib_device_set_netdev(ib_dev, rdev->port_info.lldevs[i],
+					   i + 1);
+		if (ret)
+			return ret;
+	}
+	return 0;
+}
+
 int iwch_register_device(struct iwch_dev *dev)
 {
+	int err;
+
 	pr_debug("%s iwch_dev %p\n", __func__, dev);
 	memset(&dev->ibdev.node_guid, 0, sizeof(dev->ibdev.node_guid));
 	memcpy(&dev->ibdev.node_guid, dev->rdev.t3cdev_p->lldev->dev_addr, 6);
@@ -1315,6 +1331,10 @@ int iwch_register_device(struct iwch_dev *dev)
 
 	rdma_set_device_sysfs_group(&dev->ibdev, &iwch_attr_group);
 	ib_set_device_ops(&dev->ibdev, &iwch_dev_ops);
+	err = set_netdevs(&dev->ibdev, &dev->rdev);
+	if (err)
+		return err;
+
 	return ib_register_device(&dev->ibdev, "cxgb3_%d");
 }
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH for-next V2 3/4] RDMA/core: Add common iWARP query port
  2019-07-31 20:24 [PATCH for-next V2 0/4] RDMA: Cleanups and improvements Kamal Heib
  2019-07-31 20:24 ` [PATCH for-next V2 1/4] RDMA: Introduce ib_port_phys_state enum Kamal Heib
  2019-07-31 20:24 ` [PATCH for-next V2 2/4] RDMA/cxgb3: Use ib_device_set_netdev() Kamal Heib
@ 2019-07-31 20:24 ` Kamal Heib
  2019-08-01 11:10   ` Michal Kalderon
  2019-07-31 20:24 ` [PATCH for-next V2 4/4] RDMA/{cxgb3, cxgb4, i40iw}: Remove common code Kamal Heib
  3 siblings, 1 reply; 10+ messages in thread
From: Kamal Heib @ 2019-07-31 20:24 UTC (permalink / raw)
  To: linux-rdma
  Cc: Jason Gunthorpe, Doug Ledford, Potnuri Bharat Teja,
	Selvin Xavier, Gal Pressman, Leon Romanovsky, Michal Kalderon,
	Christian Benvenuti, Moni Shoua, Bernard Metzler, Shiraz Saleem,
	Kamal Heib

Add support for a common iWARP query port function, the new function
includes a common code that is used by the iWARP devices to update the
port attributes like max_mtu, active_mtu, state, and phys_state, the
function also includes a call for the driver-specific query_port callback
to query the device-specific port attributes.

Signed-off-by: Kamal Heib <kamalheib1@gmail.com>
---
 drivers/infiniband/core/device.c | 87 ++++++++++++++++++++++++++------
 1 file changed, 71 insertions(+), 16 deletions(-)

diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 9773145dee09..860c08ca49e7 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -1940,31 +1940,64 @@ void ib_dispatch_event(struct ib_event *event)
 }
 EXPORT_SYMBOL(ib_dispatch_event);
 
-/**
- * ib_query_port - Query IB port attributes
- * @device:Device to query
- * @port_num:Port number to query
- * @port_attr:Port attributes
- *
- * ib_query_port() returns the attributes of a port through the
- * @port_attr pointer.
- */
-int ib_query_port(struct ib_device *device,
-		  u8 port_num,
-		  struct ib_port_attr *port_attr)
+static int iw_query_port(struct ib_device *device,
+			   u8 port_num,
+			   struct ib_port_attr *port_attr)
 {
-	union ib_gid gid;
+	struct in_device *inetdev;
+	struct net_device *netdev;
 	int err;
 
-	if (!rdma_is_port_valid(device, port_num))
-		return -EINVAL;
+	memset(port_attr, 0, sizeof(*port_attr));
+
+	netdev = ib_device_get_netdev(device, port_num);
+	if (!netdev)
+		return -ENODEV;
+
+	dev_put(netdev);
+
+	port_attr->max_mtu = IB_MTU_4096;
+	port_attr->active_mtu = ib_mtu_int_to_enum(netdev->mtu);
+
+	if (!netif_carrier_ok(netdev)) {
+		port_attr->state = IB_PORT_DOWN;
+		port_attr->phys_state = IB_PORT_PHYS_STATE_DISABLED;
+	} else {
+		inetdev = in_dev_get(netdev);
+
+		if (inetdev && inetdev->ifa_list) {
+			port_attr->state = IB_PORT_ACTIVE;
+			port_attr->phys_state = IB_PORT_PHYS_STATE_LINK_UP;
+			in_dev_put(inetdev);
+		} else {
+			port_attr->state = IB_PORT_INIT;
+			port_attr->phys_state =
+				IB_PORT_PHYS_STATE_PORT_CONFIGURATION_TRAINING;
+		}
+	}
+
+	err = device->ops.query_port(device, port_num, port_attr);
+	if (err)
+		return err;
+
+	return 0;
+}
+
+static int __ib_query_port(struct ib_device *device,
+			   u8 port_num,
+			   struct ib_port_attr *port_attr)
+{
+	union ib_gid gid = {};
+	int err;
 
 	memset(port_attr, 0, sizeof(*port_attr));
+
 	err = device->ops.query_port(device, port_num, port_attr);
 	if (err || port_attr->subnet_prefix)
 		return err;
 
-	if (rdma_port_get_link_layer(device, port_num) != IB_LINK_LAYER_INFINIBAND)
+	if (rdma_port_get_link_layer(device, port_num) !=
+	    IB_LINK_LAYER_INFINIBAND)
 		return 0;
 
 	err = device->ops.query_gid(device, port_num, 0, &gid);
@@ -1974,6 +2007,28 @@ int ib_query_port(struct ib_device *device,
 	port_attr->subnet_prefix = be64_to_cpu(gid.global.subnet_prefix);
 	return 0;
 }
+
+/**
+ * ib_query_port - Query IB port attributes
+ * @device:Device to query
+ * @port_num:Port number to query
+ * @port_attr:Port attributes
+ *
+ * ib_query_port() returns the attributes of a port through the
+ * @port_attr pointer.
+ */
+int ib_query_port(struct ib_device *device,
+		  u8 port_num,
+		  struct ib_port_attr *port_attr)
+{
+	if (!rdma_is_port_valid(device, port_num))
+		return -EINVAL;
+
+	if (rdma_node_get_transport(device->node_type) == RDMA_TRANSPORT_IWARP)
+		return iw_query_port(device, port_num, port_attr);
+	else
+		return __ib_query_port(device, port_num, port_attr);
+}
 EXPORT_SYMBOL(ib_query_port);
 
 static void add_ndev_hash(struct ib_port_data *pdata)
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH for-next V2 4/4] RDMA/{cxgb3, cxgb4, i40iw}: Remove common code
  2019-07-31 20:24 [PATCH for-next V2 0/4] RDMA: Cleanups and improvements Kamal Heib
                   ` (2 preceding siblings ...)
  2019-07-31 20:24 ` [PATCH for-next V2 3/4] RDMA/core: Add common iWARP query port Kamal Heib
@ 2019-07-31 20:24 ` Kamal Heib
  2019-08-01  8:49   ` Potnuri Bharat Teja
  3 siblings, 1 reply; 10+ messages in thread
From: Kamal Heib @ 2019-07-31 20:24 UTC (permalink / raw)
  To: linux-rdma
  Cc: Jason Gunthorpe, Doug Ledford, Potnuri Bharat Teja,
	Selvin Xavier, Gal Pressman, Leon Romanovsky, Michal Kalderon,
	Christian Benvenuti, Moni Shoua, Bernard Metzler, Shiraz Saleem,
	Kamal Heib

Now that we have a common iWARP query port function we can remove the
common code from the iWARP drivers.

Signed-off-by: Kamal Heib <kamalheib1@gmail.com>
---
 drivers/infiniband/hw/cxgb3/iwch_provider.c | 25 ---------------------
 drivers/infiniband/hw/cxgb4/provider.c      | 24 --------------------
 drivers/infiniband/hw/i40iw/i40iw_verbs.c   | 11 ---------
 3 files changed, 60 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb3/iwch_provider.c b/drivers/infiniband/hw/cxgb3/iwch_provider.c
index 5848e4727b2e..dcf02ec02810 100644
--- a/drivers/infiniband/hw/cxgb3/iwch_provider.c
+++ b/drivers/infiniband/hw/cxgb3/iwch_provider.c
@@ -991,33 +991,8 @@ static int iwch_query_device(struct ib_device *ibdev, struct ib_device_attr *pro
 static int iwch_query_port(struct ib_device *ibdev,
 			   u8 port, struct ib_port_attr *props)
 {
-	struct iwch_dev *dev;
-	struct net_device *netdev;
-	struct in_device *inetdev;
-
 	pr_debug("%s ibdev %p\n", __func__, ibdev);
 
-	dev = to_iwch_dev(ibdev);
-	netdev = dev->rdev.port_info.lldevs[port-1];
-
-	/* props being zeroed by the caller, avoid zeroing it here */
-	props->max_mtu = IB_MTU_4096;
-	props->active_mtu = ib_mtu_int_to_enum(netdev->mtu);
-
-	if (!netif_carrier_ok(netdev))
-		props->state = IB_PORT_DOWN;
-	else {
-		inetdev = in_dev_get(netdev);
-		if (inetdev) {
-			if (inetdev->ifa_list)
-				props->state = IB_PORT_ACTIVE;
-			else
-				props->state = IB_PORT_INIT;
-			in_dev_put(inetdev);
-		} else
-			props->state = IB_PORT_INIT;
-	}
-
 	props->port_cap_flags =
 	    IB_PORT_CM_SUP |
 	    IB_PORT_SNMP_TUNNEL_SUP |
diff --git a/drivers/infiniband/hw/cxgb4/provider.c b/drivers/infiniband/hw/cxgb4/provider.c
index 5e59c5708729..d373ac0fe2cb 100644
--- a/drivers/infiniband/hw/cxgb4/provider.c
+++ b/drivers/infiniband/hw/cxgb4/provider.c
@@ -305,32 +305,8 @@ static int c4iw_query_device(struct ib_device *ibdev, struct ib_device_attr *pro
 static int c4iw_query_port(struct ib_device *ibdev, u8 port,
 			   struct ib_port_attr *props)
 {
-	struct c4iw_dev *dev;
-	struct net_device *netdev;
-	struct in_device *inetdev;
-
 	pr_debug("ibdev %p\n", ibdev);
 
-	dev = to_c4iw_dev(ibdev);
-	netdev = dev->rdev.lldi.ports[port-1];
-	/* props being zeroed by the caller, avoid zeroing it here */
-	props->max_mtu = IB_MTU_4096;
-	props->active_mtu = ib_mtu_int_to_enum(netdev->mtu);
-
-	if (!netif_carrier_ok(netdev))
-		props->state = IB_PORT_DOWN;
-	else {
-		inetdev = in_dev_get(netdev);
-		if (inetdev) {
-			if (inetdev->ifa_list)
-				props->state = IB_PORT_ACTIVE;
-			else
-				props->state = IB_PORT_INIT;
-			in_dev_put(inetdev);
-		} else
-			props->state = IB_PORT_INIT;
-	}
-
 	props->port_cap_flags =
 	    IB_PORT_CM_SUP |
 	    IB_PORT_SNMP_TUNNEL_SUP |
diff --git a/drivers/infiniband/hw/i40iw/i40iw_verbs.c b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
index d169a8031375..8056930bbe2c 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_verbs.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
@@ -97,18 +97,7 @@ static int i40iw_query_port(struct ib_device *ibdev,
 			    u8 port,
 			    struct ib_port_attr *props)
 {
-	struct i40iw_device *iwdev = to_iwdev(ibdev);
-	struct net_device *netdev = iwdev->netdev;
-
-	/* props being zeroed by the caller, avoid zeroing it here */
-	props->max_mtu = IB_MTU_4096;
-	props->active_mtu = ib_mtu_int_to_enum(netdev->mtu);
-
 	props->lid = 1;
-	if (netif_carrier_ok(iwdev->netdev))
-		props->state = IB_PORT_ACTIVE;
-	else
-		props->state = IB_PORT_DOWN;
 	props->port_cap_flags = IB_PORT_CM_SUP | IB_PORT_REINIT_SUP |
 		IB_PORT_VENDOR_CLASS_SUP | IB_PORT_BOOT_MGMT_SUP;
 	props->gid_tbl_len = 1;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH for-next V2 1/4] RDMA: Introduce ib_port_phys_state enum
  2019-07-31 20:24 ` [PATCH for-next V2 1/4] RDMA: Introduce ib_port_phys_state enum Kamal Heib
@ 2019-07-31 20:32   ` Andrew Boyer
  2019-08-01  8:35     ` Devesh Sharma
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Boyer @ 2019-07-31 20:32 UTC (permalink / raw)
  To: linux-rdma

Thanks! I meant to do this two years ago.

Reviewed-by: Andrew Boyer <aboyer@tobark.org>

> On Jul 31, 2019, at 4:24 PM, Kamal Heib <kamalheib1@gmail.com> wrote:
> 
> In order to improve readability, add ib_port_phys_state enum to replace
> the use of magic numbers.
> 
> Signed-off-by: Kamal Heib <kamalheib1@gmail.com>
> ---
> drivers/infiniband/core/sysfs.c              | 24 +++++++++++++-------
> drivers/infiniband/hw/bnxt_re/ib_verbs.c     |  4 ++--
> drivers/infiniband/hw/efa/efa_verbs.c        |  2 +-
> drivers/infiniband/hw/mlx5/main.c            |  4 ++--
> drivers/infiniband/hw/ocrdma/ocrdma_verbs.c  |  4 ++--
> drivers/infiniband/hw/qedr/verbs.c           |  4 ++--
> drivers/infiniband/hw/usnic/usnic_ib_verbs.c |  7 +++---
> drivers/infiniband/sw/rxe/rxe.h              |  4 ----
> drivers/infiniband/sw/rxe/rxe_param.h        |  2 +-
> drivers/infiniband/sw/rxe/rxe_verbs.c        |  6 ++---
> drivers/infiniband/sw/siw/siw_verbs.c        |  3 ++-
> include/rdma/ib_verbs.h                      | 10 ++++++++
> 12 files changed, 45 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c
> index b477295a96c2..46722e04f6e1 100644
> --- a/drivers/infiniband/core/sysfs.c
> +++ b/drivers/infiniband/core/sysfs.c
> @@ -301,14 +301,22 @@ static ssize_t phys_state_show(struct ib_port *p, struct port_attribute *unused,
> 		return ret;
> 
> 	switch (attr.phys_state) {
> -	case 1:  return sprintf(buf, "1: Sleep\n");
> -	case 2:  return sprintf(buf, "2: Polling\n");
> -	case 3:  return sprintf(buf, "3: Disabled\n");
> -	case 4:  return sprintf(buf, "4: PortConfigurationTraining\n");
> -	case 5:  return sprintf(buf, "5: LinkUp\n");
> -	case 6:  return sprintf(buf, "6: LinkErrorRecovery\n");
> -	case 7:  return sprintf(buf, "7: Phy Test\n");
> -	default: return sprintf(buf, "%d: <unknown>\n", attr.phys_state);
> +	case IB_PORT_PHYS_STATE_SLEEP:
> +		return sprintf(buf, "1: Sleep\n");
> +	case IB_PORT_PHYS_STATE_POLLING:
> +		return sprintf(buf, "2: Polling\n");
> +	case IB_PORT_PHYS_STATE_DISABLED:
> +		return sprintf(buf, "3: Disabled\n");
> +	case IB_PORT_PHYS_STATE_PORT_CONFIGURATION_TRAINING:
> +		return sprintf(buf, "4: PortConfigurationTraining\n");
> +	case IB_PORT_PHYS_STATE_LINK_UP:
> +		return sprintf(buf, "5: LinkUp\n");
> +	case IB_PORT_PHYS_STATE_LINK_ERROR_RECOVERY:
> +		return sprintf(buf, "6: LinkErrorRecovery\n");
> +	case IB_PORT_PHYS_STATE_PHY_TEST:
> +		return sprintf(buf, "7: Phy Test\n");
> +	default:
> +		return sprintf(buf, "%d: <unknown>\n", attr.phys_state);
> 	}
> }
> 
> diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> index a91653aabf38..ca6306c24881 100644
> --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> @@ -220,10 +220,10 @@ int bnxt_re_query_port(struct ib_device *ibdev, u8 port_num,
> 
> 	if (netif_running(rdev->netdev) && netif_carrier_ok(rdev->netdev)) {
> 		port_attr->state = IB_PORT_ACTIVE;
> -		port_attr->phys_state = 5;
> +		port_attr->phys_state = IB_PORT_PHYS_STATE_LINK_UP;
> 	} else {
> 		port_attr->state = IB_PORT_DOWN;
> -		port_attr->phys_state = 3;
> +		port_attr->phys_state = IB_PORT_PHYS_STATE_DISABLED;
> 	}
> 	port_attr->max_mtu = IB_MTU_4096;
> 	port_attr->active_mtu = iboe_get_mtu(rdev->netdev->mtu);
> diff --git a/drivers/infiniband/hw/efa/efa_verbs.c b/drivers/infiniband/hw/efa/efa_verbs.c
> index df77bc312a25..f36071a92f97 100644
> --- a/drivers/infiniband/hw/efa/efa_verbs.c
> +++ b/drivers/infiniband/hw/efa/efa_verbs.c
> @@ -306,7 +306,7 @@ int efa_query_port(struct ib_device *ibdev, u8 port,
> 	props->lmc = 1;
> 
> 	props->state = IB_PORT_ACTIVE;
> -	props->phys_state = 5;
> +	props->phys_state = IB_PORT_PHYS_STATE_LINK_UP;
> 	props->gid_tbl_len = 1;
> 	props->pkey_tbl_len = 1;
> 	props->active_speed = IB_SPEED_EDR;
> diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
> index c2a5780cb394..bc4d7dca170f 100644
> --- a/drivers/infiniband/hw/mlx5/main.c
> +++ b/drivers/infiniband/hw/mlx5/main.c
> @@ -535,7 +535,7 @@ static int mlx5_query_port_roce(struct ib_device *device, u8 port_num,
> 	props->max_msg_sz       = 1 << MLX5_CAP_GEN(dev->mdev, log_max_msg);
> 	props->pkey_tbl_len     = 1;
> 	props->state            = IB_PORT_DOWN;
> -	props->phys_state       = 3;
> +	props->phys_state       = IB_PORT_PHYS_STATE_DISABLED;
> 
> 	mlx5_query_nic_vport_qkey_viol_cntr(mdev, &qkey_viol_cntr);
> 	props->qkey_viol_cntr = qkey_viol_cntr;
> @@ -561,7 +561,7 @@ static int mlx5_query_port_roce(struct ib_device *device, u8 port_num,
> 
> 	if (netif_running(ndev) && netif_carrier_ok(ndev)) {
> 		props->state      = IB_PORT_ACTIVE;
> -		props->phys_state = 5;
> +		props->phys_state = IB_PORT_PHYS_STATE_LINK_UP;
> 	}
> 
> 	ndev_ib_mtu = iboe_get_mtu(ndev->mtu);
> diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c b/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
> index bccc11378109..e8267e590772 100644
> --- a/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
> +++ b/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
> @@ -163,10 +163,10 @@ int ocrdma_query_port(struct ib_device *ibdev,
> 	netdev = dev->nic_info.netdev;
> 	if (netif_running(netdev) && netif_oper_up(netdev)) {
> 		port_state = IB_PORT_ACTIVE;
> -		props->phys_state = 5;
> +		props->phys_state = IB_PORT_PHYS_STATE_LINK_UP;
> 	} else {
> 		port_state = IB_PORT_DOWN;
> -		props->phys_state = 3;
> +		props->phys_state = IB_PORT_PHYS_STATE_DISABLED;
> 	}
> 	props->max_mtu = IB_MTU_4096;
> 	props->active_mtu = iboe_get_mtu(netdev->mtu);
> diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c
> index 27d90a84ea01..1373312aec58 100644
> --- a/drivers/infiniband/hw/qedr/verbs.c
> +++ b/drivers/infiniband/hw/qedr/verbs.c
> @@ -221,10 +221,10 @@ int qedr_query_port(struct ib_device *ibdev, u8 port, struct ib_port_attr *attr)
> 	/* *attr being zeroed by the caller, avoid zeroing it here */
> 	if (rdma_port->port_state == QED_RDMA_PORT_UP) {
> 		attr->state = IB_PORT_ACTIVE;
> -		attr->phys_state = 5;
> +		attr->phys_state = IB_PORT_PHYS_STATE_LINK_UP;
> 	} else {
> 		attr->state = IB_PORT_DOWN;
> -		attr->phys_state = 3;
> +		attr->phys_state = IB_PORT_PHYS_STATE_DISABLED;
> 	}
> 	attr->max_mtu = IB_MTU_4096;
> 	attr->active_mtu = iboe_get_mtu(dev->ndev->mtu);
> diff --git a/drivers/infiniband/hw/usnic/usnic_ib_verbs.c b/drivers/infiniband/hw/usnic/usnic_ib_verbs.c
> index eeb07b245ef9..4f8f1d3eb559 100644
> --- a/drivers/infiniband/hw/usnic/usnic_ib_verbs.c
> +++ b/drivers/infiniband/hw/usnic/usnic_ib_verbs.c
> @@ -356,13 +356,14 @@ int usnic_ib_query_port(struct ib_device *ibdev, u8 port,
> 
> 	if (!us_ibdev->ufdev->link_up) {
> 		props->state = IB_PORT_DOWN;
> -		props->phys_state = 3;
> +		props->phys_state = IB_PORT_PHYS_STATE_DISABLED;
> 	} else if (!us_ibdev->ufdev->inaddr) {
> 		props->state = IB_PORT_INIT;
> -		props->phys_state = 4;
> +		props->phys_state =
> +			IB_PORT_PHYS_STATE_PORT_CONFIGURATION_TRAINING;
> 	} else {
> 		props->state = IB_PORT_ACTIVE;
> -		props->phys_state = 5;
> +		props->phys_state = IB_PORT_PHYS_STATE_LINK_UP;
> 	}
> 
> 	props->port_cap_flags = 0;
> diff --git a/drivers/infiniband/sw/rxe/rxe.h b/drivers/infiniband/sw/rxe/rxe.h
> index ecf6e659c0da..fb07eed9e402 100644
> --- a/drivers/infiniband/sw/rxe/rxe.h
> +++ b/drivers/infiniband/sw/rxe/rxe.h
> @@ -65,10 +65,6 @@
>  */
> #define RXE_UVERBS_ABI_VERSION		2
> 
> -#define RDMA_LINK_PHYS_STATE_LINK_UP	(5)
> -#define RDMA_LINK_PHYS_STATE_DISABLED	(3)
> -#define RDMA_LINK_PHYS_STATE_POLLING	(2)
> -
> #define RXE_ROCE_V2_SPORT		(0xc000)
> 
> static inline u32 rxe_crc32(struct rxe_dev *rxe,
> diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h
> index 1abed47ca221..fe5207386700 100644
> --- a/drivers/infiniband/sw/rxe/rxe_param.h
> +++ b/drivers/infiniband/sw/rxe/rxe_param.h
> @@ -154,7 +154,7 @@ enum rxe_port_param {
> 	RXE_PORT_ACTIVE_WIDTH		= IB_WIDTH_1X,
> 	RXE_PORT_ACTIVE_SPEED		= 1,
> 	RXE_PORT_PKEY_TBL_LEN		= 64,
> -	RXE_PORT_PHYS_STATE		= 2,
> +	RXE_PORT_PHYS_STATE		= IB_PORT_PHYS_STATE_POLLING,
> 	RXE_PORT_SUBNET_PREFIX		= 0xfe80000000000000ULL,
> };
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
> index 4ebdfcf4d33e..623129f27f5a 100644
> --- a/drivers/infiniband/sw/rxe/rxe_verbs.c
> +++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
> @@ -69,11 +69,11 @@ static int rxe_query_port(struct ib_device *dev,
> 			      &attr->active_width);
> 
> 	if (attr->state == IB_PORT_ACTIVE)
> -		attr->phys_state = RDMA_LINK_PHYS_STATE_LINK_UP;
> +		attr->phys_state = IB_PORT_PHYS_STATE_LINK_UP;
> 	else if (dev_get_flags(rxe->ndev) & IFF_UP)
> -		attr->phys_state = RDMA_LINK_PHYS_STATE_POLLING;
> +		attr->phys_state = IB_PORT_PHYS_STATE_POLLING;
> 	else
> -		attr->phys_state = RDMA_LINK_PHYS_STATE_DISABLED;
> +		attr->phys_state = IB_PORT_PHYS_STATE_DISABLED;
> 
> 	mutex_unlock(&rxe->usdev_lock);
> 
> diff --git a/drivers/infiniband/sw/siw/siw_verbs.c b/drivers/infiniband/sw/siw/siw_verbs.c
> index 32dc79d0e898..404e7ca4b30c 100644
> --- a/drivers/infiniband/sw/siw/siw_verbs.c
> +++ b/drivers/infiniband/sw/siw/siw_verbs.c
> @@ -206,7 +206,8 @@ int siw_query_port(struct ib_device *base_dev, u8 port,
> 	attr->gid_tbl_len = 1;
> 	attr->max_msg_sz = -1;
> 	attr->max_mtu = ib_mtu_int_to_enum(sdev->netdev->mtu);
> -	attr->phys_state = sdev->state == IB_PORT_ACTIVE ? 5 : 3;
> +	attr->phys_state = sdev->state == IB_PORT_ACTIVE ?
> +		IB_PORT_PHYS_STATE_LINK_UP : IB_PORT_PHYS_STATE_DISABLED;
> 	attr->pkey_tbl_len = 1;
> 	attr->port_cap_flags = IB_PORT_CM_SUP | IB_PORT_DEVICE_MGMT_SUP;
> 	attr->state = sdev->state;
> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index c5f8a9f17063..27fe844cff42 100644
> --- a/include/rdma/ib_verbs.h
> +++ b/include/rdma/ib_verbs.h
> @@ -451,6 +451,16 @@ enum ib_port_state {
> 	IB_PORT_ACTIVE_DEFER	= 5
> };
> 
> +enum ib_port_phys_state {
> +	IB_PORT_PHYS_STATE_SLEEP = 1,
> +	IB_PORT_PHYS_STATE_POLLING = 2,
> +	IB_PORT_PHYS_STATE_DISABLED = 3,
> +	IB_PORT_PHYS_STATE_PORT_CONFIGURATION_TRAINING = 4,
> +	IB_PORT_PHYS_STATE_LINK_UP = 5,
> +	IB_PORT_PHYS_STATE_LINK_ERROR_RECOVERY = 6,
> +	IB_PORT_PHYS_STATE_PHY_TEST = 7,
> +};
> +
> enum ib_port_width {
> 	IB_WIDTH_1X	= 1,
> 	IB_WIDTH_2X	= 16,
> -- 
> 2.20.1
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH for-next V2 1/4] RDMA: Introduce ib_port_phys_state enum
  2019-07-31 20:32   ` Andrew Boyer
@ 2019-08-01  8:35     ` Devesh Sharma
  0 siblings, 0 replies; 10+ messages in thread
From: Devesh Sharma @ 2019-08-01  8:35 UTC (permalink / raw)
  To: linux-rdma

On Thu, Aug 1, 2019 at 2:02 AM Andrew Boyer <aboyer@pensando.io> wrote:
>
> Thanks! I meant to do this two years ago.
>
> Reviewed-by: Andrew Boyer <aboyer@tobark.org>
>
> > On Jul 31, 2019, at 4:24 PM, Kamal Heib <kamalheib1@gmail.com> wrote:
> >
> > In order to improve readability, add ib_port_phys_state enum to replace
> > the use of magic numbers.
> >
> > Signed-off-by: Kamal Heib <kamalheib1@gmail.com>
> > ---
> > drivers/infiniband/core/sysfs.c              | 24 +++++++++++++-------
> > drivers/infiniband/hw/bnxt_re/ib_verbs.c     |  4 ++--
> > drivers/infiniband/hw/efa/efa_verbs.c        |  2 +-
> > drivers/infiniband/hw/mlx5/main.c            |  4 ++--
> > drivers/infiniband/hw/ocrdma/ocrdma_verbs.c  |  4 ++--
> > drivers/infiniband/hw/qedr/verbs.c           |  4 ++--
> > drivers/infiniband/hw/usnic/usnic_ib_verbs.c |  7 +++---
> > drivers/infiniband/sw/rxe/rxe.h              |  4 ----
> > drivers/infiniband/sw/rxe/rxe_param.h        |  2 +-
> > drivers/infiniband/sw/rxe/rxe_verbs.c        |  6 ++---
> > drivers/infiniband/sw/siw/siw_verbs.c        |  3 ++-
> > include/rdma/ib_verbs.h                      | 10 ++++++++
> > 12 files changed, 45 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c
> > index b477295a96c2..46722e04f6e1 100644
> > --- a/drivers/infiniband/core/sysfs.c
> > +++ b/drivers/infiniband/core/sysfs.c
> > @@ -301,14 +301,22 @@ static ssize_t phys_state_show(struct ib_port *p, struct port_attribute *unused,
> >               return ret;
> >
> >       switch (attr.phys_state) {
> > -     case 1:  return sprintf(buf, "1: Sleep\n");
> > -     case 2:  return sprintf(buf, "2: Polling\n");
> > -     case 3:  return sprintf(buf, "3: Disabled\n");
> > -     case 4:  return sprintf(buf, "4: PortConfigurationTraining\n");
> > -     case 5:  return sprintf(buf, "5: LinkUp\n");
> > -     case 6:  return sprintf(buf, "6: LinkErrorRecovery\n");
> > -     case 7:  return sprintf(buf, "7: Phy Test\n");
> > -     default: return sprintf(buf, "%d: <unknown>\n", attr.phys_state);
> > +     case IB_PORT_PHYS_STATE_SLEEP:
> > +             return sprintf(buf, "1: Sleep\n");
> > +     case IB_PORT_PHYS_STATE_POLLING:
> > +             return sprintf(buf, "2: Polling\n");
> > +     case IB_PORT_PHYS_STATE_DISABLED:
> > +             return sprintf(buf, "3: Disabled\n");
> > +     case IB_PORT_PHYS_STATE_PORT_CONFIGURATION_TRAINING:
> > +             return sprintf(buf, "4: PortConfigurationTraining\n");
> > +     case IB_PORT_PHYS_STATE_LINK_UP:
> > +             return sprintf(buf, "5: LinkUp\n");
> > +     case IB_PORT_PHYS_STATE_LINK_ERROR_RECOVERY:
> > +             return sprintf(buf, "6: LinkErrorRecovery\n");
> > +     case IB_PORT_PHYS_STATE_PHY_TEST:
> > +             return sprintf(buf, "7: Phy Test\n");
> > +     default:
> > +             return sprintf(buf, "%d: <unknown>\n", attr.phys_state);
> >       }
> > }
> >
> > diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > index a91653aabf38..ca6306c24881 100644
> > --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
> > @@ -220,10 +220,10 @@ int bnxt_re_query_port(struct ib_device *ibdev, u8 port_num,
> >
> >       if (netif_running(rdev->netdev) && netif_carrier_ok(rdev->netdev)) {
> >               port_attr->state = IB_PORT_ACTIVE;
> > -             port_attr->phys_state = 5;
> > +             port_attr->phys_state = IB_PORT_PHYS_STATE_LINK_UP;
> >       } else {
> >               port_attr->state = IB_PORT_DOWN;
> > -             port_attr->phys_state = 3;
> > +             port_attr->phys_state = IB_PORT_PHYS_STATE_DISABLED;
> >       }
> >       port_attr->max_mtu = IB_MTU_4096;
> >       port_attr->active_mtu = iboe_get_mtu(rdev->netdev->mtu);
> > diff --git a/drivers/infiniband/hw/efa/efa_verbs.c b/drivers/infiniband/hw/efa/efa_verbs.c
> > index df77bc312a25..f36071a92f97 100644
> > --- a/drivers/infiniband/hw/efa/efa_verbs.c
> > +++ b/drivers/infiniband/hw/efa/efa_verbs.c
> > @@ -306,7 +306,7 @@ int efa_query_port(struct ib_device *ibdev, u8 port,
> >       props->lmc = 1;
> >
> >       props->state = IB_PORT_ACTIVE;
> > -     props->phys_state = 5;
> > +     props->phys_state = IB_PORT_PHYS_STATE_LINK_UP;
> >       props->gid_tbl_len = 1;
> >       props->pkey_tbl_len = 1;
> >       props->active_speed = IB_SPEED_EDR;
> > diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
> > index c2a5780cb394..bc4d7dca170f 100644
> > --- a/drivers/infiniband/hw/mlx5/main.c
> > +++ b/drivers/infiniband/hw/mlx5/main.c
> > @@ -535,7 +535,7 @@ static int mlx5_query_port_roce(struct ib_device *device, u8 port_num,
> >       props->max_msg_sz       = 1 << MLX5_CAP_GEN(dev->mdev, log_max_msg);
> >       props->pkey_tbl_len     = 1;
> >       props->state            = IB_PORT_DOWN;
> > -     props->phys_state       = 3;
> > +     props->phys_state       = IB_PORT_PHYS_STATE_DISABLED;
> >
> >       mlx5_query_nic_vport_qkey_viol_cntr(mdev, &qkey_viol_cntr);
> >       props->qkey_viol_cntr = qkey_viol_cntr;
> > @@ -561,7 +561,7 @@ static int mlx5_query_port_roce(struct ib_device *device, u8 port_num,
> >
> >       if (netif_running(ndev) && netif_carrier_ok(ndev)) {
> >               props->state      = IB_PORT_ACTIVE;
> > -             props->phys_state = 5;
> > +             props->phys_state = IB_PORT_PHYS_STATE_LINK_UP;
> >       }
> >
> >       ndev_ib_mtu = iboe_get_mtu(ndev->mtu);
> > diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c b/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
> > index bccc11378109..e8267e590772 100644
> > --- a/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
> > +++ b/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
> > @@ -163,10 +163,10 @@ int ocrdma_query_port(struct ib_device *ibdev,
> >       netdev = dev->nic_info.netdev;
> >       if (netif_running(netdev) && netif_oper_up(netdev)) {
> >               port_state = IB_PORT_ACTIVE;
> > -             props->phys_state = 5;
> > +             props->phys_state = IB_PORT_PHYS_STATE_LINK_UP;
> >       } else {
> >               port_state = IB_PORT_DOWN;
> > -             props->phys_state = 3;
> > +             props->phys_state = IB_PORT_PHYS_STATE_DISABLED;
> >       }
> >       props->max_mtu = IB_MTU_4096;
> >       props->active_mtu = iboe_get_mtu(netdev->mtu);
> > diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c
> > index 27d90a84ea01..1373312aec58 100644
> > --- a/drivers/infiniband/hw/qedr/verbs.c
> > +++ b/drivers/infiniband/hw/qedr/verbs.c
> > @@ -221,10 +221,10 @@ int qedr_query_port(struct ib_device *ibdev, u8 port, struct ib_port_attr *attr)
> >       /* *attr being zeroed by the caller, avoid zeroing it here */
> >       if (rdma_port->port_state == QED_RDMA_PORT_UP) {
> >               attr->state = IB_PORT_ACTIVE;
> > -             attr->phys_state = 5;
> > +             attr->phys_state = IB_PORT_PHYS_STATE_LINK_UP;
> >       } else {
> >               attr->state = IB_PORT_DOWN;
> > -             attr->phys_state = 3;
> > +             attr->phys_state = IB_PORT_PHYS_STATE_DISABLED;
> >       }
> >       attr->max_mtu = IB_MTU_4096;
> >       attr->active_mtu = iboe_get_mtu(dev->ndev->mtu);
> > diff --git a/drivers/infiniband/hw/usnic/usnic_ib_verbs.c b/drivers/infiniband/hw/usnic/usnic_ib_verbs.c
> > index eeb07b245ef9..4f8f1d3eb559 100644
> > --- a/drivers/infiniband/hw/usnic/usnic_ib_verbs.c
> > +++ b/drivers/infiniband/hw/usnic/usnic_ib_verbs.c
> > @@ -356,13 +356,14 @@ int usnic_ib_query_port(struct ib_device *ibdev, u8 port,
> >
> >       if (!us_ibdev->ufdev->link_up) {
> >               props->state = IB_PORT_DOWN;
> > -             props->phys_state = 3;
> > +             props->phys_state = IB_PORT_PHYS_STATE_DISABLED;
> >       } else if (!us_ibdev->ufdev->inaddr) {
> >               props->state = IB_PORT_INIT;
> > -             props->phys_state = 4;
> > +             props->phys_state =
> > +                     IB_PORT_PHYS_STATE_PORT_CONFIGURATION_TRAINING;
> >       } else {
> >               props->state = IB_PORT_ACTIVE;
> > -             props->phys_state = 5;
> > +             props->phys_state = IB_PORT_PHYS_STATE_LINK_UP;
> >       }
> >
> >       props->port_cap_flags = 0;
> > diff --git a/drivers/infiniband/sw/rxe/rxe.h b/drivers/infiniband/sw/rxe/rxe.h
> > index ecf6e659c0da..fb07eed9e402 100644
> > --- a/drivers/infiniband/sw/rxe/rxe.h
> > +++ b/drivers/infiniband/sw/rxe/rxe.h
> > @@ -65,10 +65,6 @@
> >  */
> > #define RXE_UVERBS_ABI_VERSION                2
> >
> > -#define RDMA_LINK_PHYS_STATE_LINK_UP (5)
> > -#define RDMA_LINK_PHYS_STATE_DISABLED        (3)
> > -#define RDMA_LINK_PHYS_STATE_POLLING (2)
> > -
> > #define RXE_ROCE_V2_SPORT             (0xc000)
> >
> > static inline u32 rxe_crc32(struct rxe_dev *rxe,
> > diff --git a/drivers/infiniband/sw/rxe/rxe_param.h b/drivers/infiniband/sw/rxe/rxe_param.h
> > index 1abed47ca221..fe5207386700 100644
> > --- a/drivers/infiniband/sw/rxe/rxe_param.h
> > +++ b/drivers/infiniband/sw/rxe/rxe_param.h
> > @@ -154,7 +154,7 @@ enum rxe_port_param {
> >       RXE_PORT_ACTIVE_WIDTH           = IB_WIDTH_1X,
> >       RXE_PORT_ACTIVE_SPEED           = 1,
> >       RXE_PORT_PKEY_TBL_LEN           = 64,
> > -     RXE_PORT_PHYS_STATE             = 2,
> > +     RXE_PORT_PHYS_STATE             = IB_PORT_PHYS_STATE_POLLING,
> >       RXE_PORT_SUBNET_PREFIX          = 0xfe80000000000000ULL,
> > };
> >
> > diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
> > index 4ebdfcf4d33e..623129f27f5a 100644
> > --- a/drivers/infiniband/sw/rxe/rxe_verbs.c
> > +++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
> > @@ -69,11 +69,11 @@ static int rxe_query_port(struct ib_device *dev,
> >                             &attr->active_width);
> >
> >       if (attr->state == IB_PORT_ACTIVE)
> > -             attr->phys_state = RDMA_LINK_PHYS_STATE_LINK_UP;
> > +             attr->phys_state = IB_PORT_PHYS_STATE_LINK_UP;
> >       else if (dev_get_flags(rxe->ndev) & IFF_UP)
> > -             attr->phys_state = RDMA_LINK_PHYS_STATE_POLLING;
> > +             attr->phys_state = IB_PORT_PHYS_STATE_POLLING;
> >       else
> > -             attr->phys_state = RDMA_LINK_PHYS_STATE_DISABLED;
> > +             attr->phys_state = IB_PORT_PHYS_STATE_DISABLED;
> >
> >       mutex_unlock(&rxe->usdev_lock);
> >
> > diff --git a/drivers/infiniband/sw/siw/siw_verbs.c b/drivers/infiniband/sw/siw/siw_verbs.c
> > index 32dc79d0e898..404e7ca4b30c 100644
> > --- a/drivers/infiniband/sw/siw/siw_verbs.c
> > +++ b/drivers/infiniband/sw/siw/siw_verbs.c
> > @@ -206,7 +206,8 @@ int siw_query_port(struct ib_device *base_dev, u8 port,
> >       attr->gid_tbl_len = 1;
> >       attr->max_msg_sz = -1;
> >       attr->max_mtu = ib_mtu_int_to_enum(sdev->netdev->mtu);
> > -     attr->phys_state = sdev->state == IB_PORT_ACTIVE ? 5 : 3;
> > +     attr->phys_state = sdev->state == IB_PORT_ACTIVE ?
> > +             IB_PORT_PHYS_STATE_LINK_UP : IB_PORT_PHYS_STATE_DISABLED;
> >       attr->pkey_tbl_len = 1;
> >       attr->port_cap_flags = IB_PORT_CM_SUP | IB_PORT_DEVICE_MGMT_SUP;
> >       attr->state = sdev->state;
> > diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> > index c5f8a9f17063..27fe844cff42 100644
> > --- a/include/rdma/ib_verbs.h
> > +++ b/include/rdma/ib_verbs.h
> > @@ -451,6 +451,16 @@ enum ib_port_state {
> >       IB_PORT_ACTIVE_DEFER    = 5
> > };
> >
> > +enum ib_port_phys_state {
> > +     IB_PORT_PHYS_STATE_SLEEP = 1,
> > +     IB_PORT_PHYS_STATE_POLLING = 2,
> > +     IB_PORT_PHYS_STATE_DISABLED = 3,
> > +     IB_PORT_PHYS_STATE_PORT_CONFIGURATION_TRAINING = 4,
> > +     IB_PORT_PHYS_STATE_LINK_UP = 5,
> > +     IB_PORT_PHYS_STATE_LINK_ERROR_RECOVERY = 6,
> > +     IB_PORT_PHYS_STATE_PHY_TEST = 7,
> > +};
> > +
> > enum ib_port_width {
> >       IB_WIDTH_1X     = 1,
> >       IB_WIDTH_2X     = 16,
> > --
> > 2.20.1
> >
>

Ack'ing for bnxt_re

Acked-By: Devesh Sharma <devesh.sharma@broadcom.com>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH for-next V2 4/4] RDMA/{cxgb3, cxgb4, i40iw}: Remove common code
  2019-07-31 20:24 ` [PATCH for-next V2 4/4] RDMA/{cxgb3, cxgb4, i40iw}: Remove common code Kamal Heib
@ 2019-08-01  8:49   ` Potnuri Bharat Teja
  0 siblings, 0 replies; 10+ messages in thread
From: Potnuri Bharat Teja @ 2019-08-01  8:49 UTC (permalink / raw)
  To: Kamal Heib
  Cc: linux-rdma, Jason Gunthorpe, Doug Ledford, Selvin Xavier,
	Gal Pressman, Leon Romanovsky, Michal Kalderon,
	Christian Benvenuti, Moni Shoua, Bernard Metzler, Shiraz Saleem

On Thursday, August 08/01/19, 2019 at 01:54:59 +0530, Kamal Heib wrote:
> Now that we have a common iWARP query port function we can remove the
> common code from the iWARP drivers.
> 
> Signed-off-by: Kamal Heib <kamalheib1@gmail.com>
> ---
Thanks for the changes! Ack for cxgb4.
Acked-by: Potnuri Bharat Teja <bharat@chelsio.com>


>  drivers/infiniband/hw/cxgb3/iwch_provider.c | 25 ---------------------
>  drivers/infiniband/hw/cxgb4/provider.c      | 24 --------------------
>  drivers/infiniband/hw/i40iw/i40iw_verbs.c   | 11 ---------
>  3 files changed, 60 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/cxgb3/iwch_provider.c b/drivers/infiniband/hw/cxgb3/iwch_provider.c
> index 5848e4727b2e..dcf02ec02810 100644
> --- a/drivers/infiniband/hw/cxgb3/iwch_provider.c
> +++ b/drivers/infiniband/hw/cxgb3/iwch_provider.c
> @@ -991,33 +991,8 @@ static int iwch_query_device(struct ib_device *ibdev, struct ib_device_attr *pro
>  static int iwch_query_port(struct ib_device *ibdev,
>  			   u8 port, struct ib_port_attr *props)
>  {
> -	struct iwch_dev *dev;
> -	struct net_device *netdev;
> -	struct in_device *inetdev;
> -
>  	pr_debug("%s ibdev %p\n", __func__, ibdev);
>  
> -	dev = to_iwch_dev(ibdev);
> -	netdev = dev->rdev.port_info.lldevs[port-1];
> -
> -	/* props being zeroed by the caller, avoid zeroing it here */
> -	props->max_mtu = IB_MTU_4096;
> -	props->active_mtu = ib_mtu_int_to_enum(netdev->mtu);
> -
> -	if (!netif_carrier_ok(netdev))
> -		props->state = IB_PORT_DOWN;
> -	else {
> -		inetdev = in_dev_get(netdev);
> -		if (inetdev) {
> -			if (inetdev->ifa_list)
> -				props->state = IB_PORT_ACTIVE;
> -			else
> -				props->state = IB_PORT_INIT;
> -			in_dev_put(inetdev);
> -		} else
> -			props->state = IB_PORT_INIT;
> -	}
> -
>  	props->port_cap_flags =
>  	    IB_PORT_CM_SUP |
>  	    IB_PORT_SNMP_TUNNEL_SUP |
> diff --git a/drivers/infiniband/hw/cxgb4/provider.c b/drivers/infiniband/hw/cxgb4/provider.c
> index 5e59c5708729..d373ac0fe2cb 100644
> --- a/drivers/infiniband/hw/cxgb4/provider.c
> +++ b/drivers/infiniband/hw/cxgb4/provider.c
> @@ -305,32 +305,8 @@ static int c4iw_query_device(struct ib_device *ibdev, struct ib_device_attr *pro
>  static int c4iw_query_port(struct ib_device *ibdev, u8 port,
>  			   struct ib_port_attr *props)
>  {
> -	struct c4iw_dev *dev;
> -	struct net_device *netdev;
> -	struct in_device *inetdev;
> -
>  	pr_debug("ibdev %p\n", ibdev);
>  
> -	dev = to_c4iw_dev(ibdev);
> -	netdev = dev->rdev.lldi.ports[port-1];
> -	/* props being zeroed by the caller, avoid zeroing it here */
> -	props->max_mtu = IB_MTU_4096;
> -	props->active_mtu = ib_mtu_int_to_enum(netdev->mtu);
> -
> -	if (!netif_carrier_ok(netdev))
> -		props->state = IB_PORT_DOWN;
> -	else {
> -		inetdev = in_dev_get(netdev);
> -		if (inetdev) {
> -			if (inetdev->ifa_list)
> -				props->state = IB_PORT_ACTIVE;
> -			else
> -				props->state = IB_PORT_INIT;
> -			in_dev_put(inetdev);
> -		} else
> -			props->state = IB_PORT_INIT;
> -	}
> -
>  	props->port_cap_flags =
>  	    IB_PORT_CM_SUP |
>  	    IB_PORT_SNMP_TUNNEL_SUP |
> diff --git a/drivers/infiniband/hw/i40iw/i40iw_verbs.c b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
> index d169a8031375..8056930bbe2c 100644
> --- a/drivers/infiniband/hw/i40iw/i40iw_verbs.c
> +++ b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
> @@ -97,18 +97,7 @@ static int i40iw_query_port(struct ib_device *ibdev,
>  			    u8 port,
>  			    struct ib_port_attr *props)
>  {
> -	struct i40iw_device *iwdev = to_iwdev(ibdev);
> -	struct net_device *netdev = iwdev->netdev;
> -
> -	/* props being zeroed by the caller, avoid zeroing it here */
> -	props->max_mtu = IB_MTU_4096;
> -	props->active_mtu = ib_mtu_int_to_enum(netdev->mtu);
> -
>  	props->lid = 1;
> -	if (netif_carrier_ok(iwdev->netdev))
> -		props->state = IB_PORT_ACTIVE;
> -	else
> -		props->state = IB_PORT_DOWN;
>  	props->port_cap_flags = IB_PORT_CM_SUP | IB_PORT_REINIT_SUP |
>  		IB_PORT_VENDOR_CLASS_SUP | IB_PORT_BOOT_MGMT_SUP;
>  	props->gid_tbl_len = 1;
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH for-next V2 3/4] RDMA/core: Add common iWARP query port
  2019-07-31 20:24 ` [PATCH for-next V2 3/4] RDMA/core: Add common iWARP query port Kamal Heib
@ 2019-08-01 11:10   ` Michal Kalderon
  2019-08-01 13:22     ` Kamal Heib
  0 siblings, 1 reply; 10+ messages in thread
From: Michal Kalderon @ 2019-08-01 11:10 UTC (permalink / raw)
  To: Kamal Heib, linux-rdma
  Cc: Jason Gunthorpe, Doug Ledford, Potnuri Bharat Teja,
	Selvin Xavier, Gal Pressman, Leon Romanovsky,
	Christian Benvenuti, Moni Shoua, Bernard Metzler, Shiraz Saleem

> From: linux-rdma-owner@vger.kernel.org <linux-rdma-
> owner@vger.kernel.org> On Behalf Of Kamal Heib
> 
> Add support for a common iWARP query port function, the new function
> includes a common code that is used by the iWARP devices to update the
> port attributes like max_mtu, active_mtu, state, and phys_state, the
> function also includes a call for the driver-specific query_port callback to
> query the device-specific port attributes.
> 
Thanks, the qedr is also a iWARP device but it has most of the code common with
The RoCE part, so we'll need to split the code earlier between the protocols. 
However, why not make the code for port-state and mtu common for Both iWARP + RoCE? 
 
> Signed-off-by: Kamal Heib <kamalheib1@gmail.com>
> ---
>  drivers/infiniband/core/device.c | 87 ++++++++++++++++++++++++++------
>  1 file changed, 71 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/infiniband/core/device.c
> b/drivers/infiniband/core/device.c
> index 9773145dee09..860c08ca49e7 100644
> --- a/drivers/infiniband/core/device.c
> +++ b/drivers/infiniband/core/device.c
> @@ -1940,31 +1940,64 @@ void ib_dispatch_event(struct ib_event *event)
> }  EXPORT_SYMBOL(ib_dispatch_event);
> 
> -/**
> - * ib_query_port - Query IB port attributes
> - * @device:Device to query
> - * @port_num:Port number to query
> - * @port_attr:Port attributes
> - *
> - * ib_query_port() returns the attributes of a port through the
> - * @port_attr pointer.
> - */
> -int ib_query_port(struct ib_device *device,
> -		  u8 port_num,
> -		  struct ib_port_attr *port_attr)
> +static int iw_query_port(struct ib_device *device,
> +			   u8 port_num,
> +			   struct ib_port_attr *port_attr)
>  {
> -	union ib_gid gid;
> +	struct in_device *inetdev;
> +	struct net_device *netdev;
>  	int err;
> 
> -	if (!rdma_is_port_valid(device, port_num))
> -		return -EINVAL;
> +	memset(port_attr, 0, sizeof(*port_attr));
> +
> +	netdev = ib_device_get_netdev(device, port_num);
> +	if (!netdev)
> +		return -ENODEV;
> +
> +	dev_put(netdev);
> +
> +	port_attr->max_mtu = IB_MTU_4096;
> +	port_attr->active_mtu = ib_mtu_int_to_enum(netdev->mtu);
> +
> +	if (!netif_carrier_ok(netdev)) {
> +		port_attr->state = IB_PORT_DOWN;
> +		port_attr->phys_state = IB_PORT_PHYS_STATE_DISABLED;
> +	} else {
> +		inetdev = in_dev_get(netdev);
> +
> +		if (inetdev && inetdev->ifa_list) {
> +			port_attr->state = IB_PORT_ACTIVE;
> +			port_attr->phys_state =
> IB_PORT_PHYS_STATE_LINK_UP;
> +			in_dev_put(inetdev);
> +		} else {
> +			port_attr->state = IB_PORT_INIT;
> +			port_attr->phys_state =
> +
> 	IB_PORT_PHYS_STATE_PORT_CONFIGURATION_TRAINING;
> +		}
> +	}
> +
> +	err = device->ops.query_port(device, port_num, port_attr);
> +	if (err)
> +		return err;
> +
> +	return 0;
> +}
> +
> +static int __ib_query_port(struct ib_device *device,
> +			   u8 port_num,
> +			   struct ib_port_attr *port_attr)
> +{
> +	union ib_gid gid = {};
> +	int err;
> 
>  	memset(port_attr, 0, sizeof(*port_attr));
> +
>  	err = device->ops.query_port(device, port_num, port_attr);
>  	if (err || port_attr->subnet_prefix)
>  		return err;
> 
> -	if (rdma_port_get_link_layer(device, port_num) !=
> IB_LINK_LAYER_INFINIBAND)
> +	if (rdma_port_get_link_layer(device, port_num) !=
> +	    IB_LINK_LAYER_INFINIBAND)
>  		return 0;
> 
>  	err = device->ops.query_gid(device, port_num, 0, &gid); @@ -1974,6
> +2007,28 @@ int ib_query_port(struct ib_device *device,
>  	port_attr->subnet_prefix = be64_to_cpu(gid.global.subnet_prefix);
>  	return 0;
>  }
> +
> +/**
> + * ib_query_port - Query IB port attributes
> + * @device:Device to query
> + * @port_num:Port number to query
> + * @port_attr:Port attributes
> + *
> + * ib_query_port() returns the attributes of a port through the
> + * @port_attr pointer.
> + */
> +int ib_query_port(struct ib_device *device,
> +		  u8 port_num,
> +		  struct ib_port_attr *port_attr)
> +{
> +	if (!rdma_is_port_valid(device, port_num))
> +		return -EINVAL;
> +
> +	if (rdma_node_get_transport(device->node_type) ==
> RDMA_TRANSPORT_IWARP)
Raising a question, in some places we use the macro above and in others
rdma_protocol_iwarp(device, port_num), any reason to prefer one over the other ? 
 thanks,

> +		return iw_query_port(device, port_num, port_attr);
> +	else
> +		return __ib_query_port(device, port_num, port_attr); }
>  EXPORT_SYMBOL(ib_query_port);
> 
>  static void add_ndev_hash(struct ib_port_data *pdata)
> --
> 2.20.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH for-next V2 3/4] RDMA/core: Add common iWARP query port
  2019-08-01 11:10   ` Michal Kalderon
@ 2019-08-01 13:22     ` Kamal Heib
  0 siblings, 0 replies; 10+ messages in thread
From: Kamal Heib @ 2019-08-01 13:22 UTC (permalink / raw)
  To: Michal Kalderon
  Cc: linux-rdma, Jason Gunthorpe, Doug Ledford, Potnuri Bharat Teja,
	Selvin Xavier, Gal Pressman, Leon Romanovsky,
	Christian Benvenuti, Moni Shoua, Bernard Metzler, Shiraz Saleem

On Thu, Aug 01, 2019 at 11:10:11AM +0000, Michal Kalderon wrote:
> > From: linux-rdma-owner@vger.kernel.org <linux-rdma-
> > owner@vger.kernel.org> On Behalf Of Kamal Heib
> > 
> > Add support for a common iWARP query port function, the new function
> > includes a common code that is used by the iWARP devices to update the
> > port attributes like max_mtu, active_mtu, state, and phys_state, the
> > function also includes a call for the driver-specific query_port callback to
> > query the device-specific port attributes.
> > 
> Thanks, the qedr is also a iWARP device but it has most of the code common with
> The RoCE part, so we'll need to split the code earlier between the protocols. 

Yes, That's why I decided not to touch it.

> However, why not make the code for port-state and mtu common for Both iWARP + RoCE? 
> 

I don't think that this is a good idea because they don't share the same
code for determining the port-state and mtu (please see mlx4/mlx5 v.s.
other iWARP drivers), while it is the same code for all iWARP drivers.

> > Signed-off-by: Kamal Heib <kamalheib1@gmail.com>
> > ---
> >  drivers/infiniband/core/device.c | 87 ++++++++++++++++++++++++++------
> >  1 file changed, 71 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/infiniband/core/device.c
> > b/drivers/infiniband/core/device.c
> > index 9773145dee09..860c08ca49e7 100644
> > --- a/drivers/infiniband/core/device.c
> > +++ b/drivers/infiniband/core/device.c
> > @@ -1940,31 +1940,64 @@ void ib_dispatch_event(struct ib_event *event)
> > }  EXPORT_SYMBOL(ib_dispatch_event);
> > 
> > -/**
> > - * ib_query_port - Query IB port attributes
> > - * @device:Device to query
> > - * @port_num:Port number to query
> > - * @port_attr:Port attributes
> > - *
> > - * ib_query_port() returns the attributes of a port through the
> > - * @port_attr pointer.
> > - */
> > -int ib_query_port(struct ib_device *device,
> > -		  u8 port_num,
> > -		  struct ib_port_attr *port_attr)
> > +static int iw_query_port(struct ib_device *device,
> > +			   u8 port_num,
> > +			   struct ib_port_attr *port_attr)
> >  {
> > -	union ib_gid gid;
> > +	struct in_device *inetdev;
> > +	struct net_device *netdev;
> >  	int err;
> > 
> > -	if (!rdma_is_port_valid(device, port_num))
> > -		return -EINVAL;
> > +	memset(port_attr, 0, sizeof(*port_attr));
> > +
> > +	netdev = ib_device_get_netdev(device, port_num);
> > +	if (!netdev)
> > +		return -ENODEV;
> > +
> > +	dev_put(netdev);
> > +
> > +	port_attr->max_mtu = IB_MTU_4096;
> > +	port_attr->active_mtu = ib_mtu_int_to_enum(netdev->mtu);
> > +
> > +	if (!netif_carrier_ok(netdev)) {
> > +		port_attr->state = IB_PORT_DOWN;
> > +		port_attr->phys_state = IB_PORT_PHYS_STATE_DISABLED;
> > +	} else {
> > +		inetdev = in_dev_get(netdev);
> > +
> > +		if (inetdev && inetdev->ifa_list) {
> > +			port_attr->state = IB_PORT_ACTIVE;
> > +			port_attr->phys_state =
> > IB_PORT_PHYS_STATE_LINK_UP;
> > +			in_dev_put(inetdev);
> > +		} else {
> > +			port_attr->state = IB_PORT_INIT;
> > +			port_attr->phys_state =
> > +
> > 	IB_PORT_PHYS_STATE_PORT_CONFIGURATION_TRAINING;
> > +		}
> > +	}
> > +
> > +	err = device->ops.query_port(device, port_num, port_attr);
> > +	if (err)
> > +		return err;
> > +
> > +	return 0;
> > +}
> > +
> > +static int __ib_query_port(struct ib_device *device,
> > +			   u8 port_num,
> > +			   struct ib_port_attr *port_attr)
> > +{
> > +	union ib_gid gid = {};
> > +	int err;
> > 
> >  	memset(port_attr, 0, sizeof(*port_attr));
> > +
> >  	err = device->ops.query_port(device, port_num, port_attr);
> >  	if (err || port_attr->subnet_prefix)
> >  		return err;
> > 
> > -	if (rdma_port_get_link_layer(device, port_num) !=
> > IB_LINK_LAYER_INFINIBAND)
> > +	if (rdma_port_get_link_layer(device, port_num) !=
> > +	    IB_LINK_LAYER_INFINIBAND)
> >  		return 0;
> > 
> >  	err = device->ops.query_gid(device, port_num, 0, &gid); @@ -1974,6
> > +2007,28 @@ int ib_query_port(struct ib_device *device,
> >  	port_attr->subnet_prefix = be64_to_cpu(gid.global.subnet_prefix);
> >  	return 0;
> >  }
> > +
> > +/**
> > + * ib_query_port - Query IB port attributes
> > + * @device:Device to query
> > + * @port_num:Port number to query
> > + * @port_attr:Port attributes
> > + *
> > + * ib_query_port() returns the attributes of a port through the
> > + * @port_attr pointer.
> > + */
> > +int ib_query_port(struct ib_device *device,
> > +		  u8 port_num,
> > +		  struct ib_port_attr *port_attr)
> > +{
> > +	if (!rdma_is_port_valid(device, port_num))
> > +		return -EINVAL;
> > +
> > +	if (rdma_node_get_transport(device->node_type) ==
> > RDMA_TRANSPORT_IWARP)
> Raising a question, in some places we use the macro above and in others
> rdma_protocol_iwarp(device, port_num), any reason to prefer one over the other ?

I thought it's more readable to use rdma_node_get_transport() in this
case than using rdma_protocol_iwarp().

>  thanks,
> 
> > +		return iw_query_port(device, port_num, port_attr);
> > +	else
> > +		return __ib_query_port(device, port_num, port_attr); }
> >  EXPORT_SYMBOL(ib_query_port);
> > 
> >  static void add_ndev_hash(struct ib_port_data *pdata)
> > --
> > 2.20.1
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-08-01 13:23 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-31 20:24 [PATCH for-next V2 0/4] RDMA: Cleanups and improvements Kamal Heib
2019-07-31 20:24 ` [PATCH for-next V2 1/4] RDMA: Introduce ib_port_phys_state enum Kamal Heib
2019-07-31 20:32   ` Andrew Boyer
2019-08-01  8:35     ` Devesh Sharma
2019-07-31 20:24 ` [PATCH for-next V2 2/4] RDMA/cxgb3: Use ib_device_set_netdev() Kamal Heib
2019-07-31 20:24 ` [PATCH for-next V2 3/4] RDMA/core: Add common iWARP query port Kamal Heib
2019-08-01 11:10   ` Michal Kalderon
2019-08-01 13:22     ` Kamal Heib
2019-07-31 20:24 ` [PATCH for-next V2 4/4] RDMA/{cxgb3, cxgb4, i40iw}: Remove common code Kamal Heib
2019-08-01  8:49   ` Potnuri Bharat Teja

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.