[PATCH rdma-next v2 0/5] RDMA/mana_ib: Enable RNIC adapter and populate it with GIDs

[PATCH rdma-next v2 0/5] RDMA/mana_ib: Enable RNIC adapter and populate it with GIDs

[Konstantin Taranov]

This patch series creates RNIC adapter in mana_ib.
To create the adapter, we must create one EQ.
In the future patches, this EQ will be used for fatal RC QP error events.
In the future patches, we will also add more EQs for CQs.

mana_ib is served by mana ethernet for RAW QPs and by RNIC for RC QPs.
If RNIC is not available in the HW, we do not fail mana_ib and keep only RAW QP
support. RNIC is availale only for port 1.

As a minimal usage, this patch series brings adding and removing RoCEv2 GIDs.
For this, we set master netdev to the ib device and set required port parameters
to get GIDs. RNIC of mana supports IPv6 and IPv4 addresses that are stored in the HW.

v1->v2:
* Fixed rcu_read_unlock() and updated commit message in "Enable RoCE on port 1"

Konstantin Taranov (5):
  RDMA/mana_ib: Add EQ creation for rnic adapter
  RDMA/mana_ib: Create and destroy rnic adapter
  RDMA/mana_ib: Implement port parameters
  RDMA/mana_ib: Enable RoCE on port 1
  RDMA/mana_ib: Adding and deleting GIDs

 drivers/infiniband/hw/mana/device.c  |  28 ++++-
 drivers/infiniband/hw/mana/main.c    | 203 ++++++++++++++++++++++++++++++++++-
 drivers/infiniband/hw/mana/mana_ib.h |  75 +++++++++++++
 3 files changed, 298 insertions(+), 8 deletions(-)

-- 
1.8.3.1

[PATCH rdma-next v2 1/5] RDMA/mana_ib: Add EQ creation for rnic adapter

[Konstantin Taranov]

This patch introduces functions for RNIC creation
and creates one EQ for RNIC creation.

Signed-off-by: Konstantin Taranov <kotaranov@linux.microsoft.com>
---
 drivers/infiniband/hw/mana/device.c  |  9 ++++--
 drivers/infiniband/hw/mana/main.c    | 53 ++++++++++++++++++++++++++++++++++++
 drivers/infiniband/hw/mana/mana_ib.h |  5 ++++
 3 files changed, 64 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/hw/mana/device.c b/drivers/infiniband/hw/mana/device.c
index 6fa902e..d8e8b10 100644
--- a/drivers/infiniband/hw/mana/device.c
+++ b/drivers/infiniband/hw/mana/device.c
@@ -92,15 +92,19 @@ static int mana_ib_probe(struct auxiliary_device *adev,
 		goto deregister_device;
 	}
 
+	mana_ib_gd_create_rnic_adapter(dev);
+
 	ret = ib_register_device(&dev->ib_dev, "mana_%d",
 				 mdev->gdma_context->dev);
 	if (ret)
-		goto deregister_device;
+		goto destroy_rnic_adapter;
 
 	dev_set_drvdata(&adev->dev, dev);
 
 	return 0;
 
+destroy_rnic_adapter:
+	mana_ib_gd_destroy_rnic_adapter(dev);
 deregister_device:
 	mana_gd_deregister_device(dev->gdma_dev);
 free_ib_device:
@@ -113,9 +117,8 @@ static void mana_ib_remove(struct auxiliary_device *adev)
 	struct mana_ib_dev *dev = dev_get_drvdata(&adev->dev);
 
 	ib_unregister_device(&dev->ib_dev);
-
+	mana_ib_gd_destroy_rnic_adapter(dev);
 	mana_gd_deregister_device(dev->gdma_dev);
-
 	ib_dealloc_device(&dev->ib_dev);
 }
 
diff --git a/drivers/infiniband/hw/mana/main.c b/drivers/infiniband/hw/mana/main.c
index 29dd243..c64d569 100644
--- a/drivers/infiniband/hw/mana/main.c
+++ b/drivers/infiniband/hw/mana/main.c
@@ -548,3 +548,56 @@ int mana_ib_gd_query_adapter_caps(struct mana_ib_dev *dev)
 
 	return 0;
 }
+
+static int mana_ib_create_eqs(struct mana_ib_dev *mdev)
+{
+	struct gdma_context *gc = mdev_to_gc(mdev);
+	struct gdma_queue_spec spec = {};
+	int err;
+
+	spec.type = GDMA_EQ;
+	spec.monitor_avl_buf = false;
+	spec.queue_size = EQ_SIZE;
+	spec.eq.callback = NULL;
+	spec.eq.context = mdev;
+	spec.eq.log2_throttle_limit = LOG2_EQ_THROTTLE;
+	spec.eq.msix_index = 0;
+
+	err = mana_gd_create_mana_eq(&gc->mana_ib, &spec, &mdev->fatal_err_eq);
+	if (err)
+		return err;
+
+	return 0;
+}
+
+static void mana_ib_destroy_eqs(struct mana_ib_dev *mdev)
+{
+	if (!mdev->fatal_err_eq)
+		return;
+
+	mana_gd_destroy_queue(mdev_to_gc(mdev), mdev->fatal_err_eq);
+	mdev->fatal_err_eq = NULL;
+}
+
+void mana_ib_gd_create_rnic_adapter(struct mana_ib_dev *mdev)
+{
+	int err;
+
+	err = mana_ib_create_eqs(mdev);
+	if (err) {
+		ibdev_err(&mdev->ib_dev, "Failed to create EQs for RNIC err %d", err);
+		goto cleanup;
+	}
+
+	return;
+
+cleanup:
+	ibdev_warn(&mdev->ib_dev,
+		   "RNIC is not available. Only RAW QPs are supported");
+	mana_ib_destroy_eqs(mdev);
+}
+
+void mana_ib_gd_destroy_rnic_adapter(struct mana_ib_dev *mdev)
+{
+	mana_ib_destroy_eqs(mdev);
+}
diff --git a/drivers/infiniband/hw/mana/mana_ib.h b/drivers/infiniband/hw/mana/mana_ib.h
index 6a03ae6..a4b94ee 100644
--- a/drivers/infiniband/hw/mana/mana_ib.h
+++ b/drivers/infiniband/hw/mana/mana_ib.h
@@ -48,6 +48,7 @@ struct mana_ib_adapter_caps {
 struct mana_ib_dev {
 	struct ib_device ib_dev;
 	struct gdma_dev *gdma_dev;
+	struct gdma_queue *fatal_err_eq;
 	struct mana_ib_adapter_caps adapter_caps;
 };
 
@@ -228,4 +229,8 @@ int mana_ib_query_gid(struct ib_device *ibdev, u32 port, int index,
 void mana_ib_disassociate_ucontext(struct ib_ucontext *ibcontext);
 
 int mana_ib_gd_query_adapter_caps(struct mana_ib_dev *mdev);
+
+void mana_ib_gd_create_rnic_adapter(struct mana_ib_dev *mdev);
+
+void mana_ib_gd_destroy_rnic_adapter(struct mana_ib_dev *mdev);
 #endif
-- 
1.8.3.1

[PATCH rdma-next v2 2/5] RDMA/mana_ib: Create and destroy rnic adapter

[Konstantin Taranov]

This patch adds RNIC creation and destruction.
If creation of RNIC fails, we support only RAW QPs as they are served by
ethernet driver.

Signed-off-by: Konstantin Taranov <kotaranov@linux.microsoft.com>
---
 drivers/infiniband/hw/mana/main.c    | 31 +++++++++++++++++++++++++++++++
 drivers/infiniband/hw/mana/mana_ib.h | 29 +++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+)

diff --git a/drivers/infiniband/hw/mana/main.c b/drivers/infiniband/hw/mana/main.c
index c64d569..33cd69e 100644
--- a/drivers/infiniband/hw/mana/main.c
+++ b/drivers/infiniband/hw/mana/main.c
@@ -581,14 +581,31 @@ static void mana_ib_destroy_eqs(struct mana_ib_dev *mdev)
 
 void mana_ib_gd_create_rnic_adapter(struct mana_ib_dev *mdev)
 {
+	struct mana_rnic_create_adapter_resp resp = {};
+	struct mana_rnic_create_adapter_req req = {};
+	struct gdma_context *gc = mdev_to_gc(mdev);
 	int err;
 
+	mdev->adapter_handle = INVALID_MANA_HANDLE;
+
 	err = mana_ib_create_eqs(mdev);
 	if (err) {
 		ibdev_err(&mdev->ib_dev, "Failed to create EQs for RNIC err %d", err);
 		goto cleanup;
 	}
 
+	mana_gd_init_req_hdr(&req.hdr, MANA_IB_CREATE_ADAPTER, sizeof(req), sizeof(resp));
+	req.hdr.req.msg_version = GDMA_MESSAGE_V2;
+	req.hdr.dev_id = gc->mana_ib.dev_id;
+	req.notify_eq_id = mdev->fatal_err_eq->id;
+
+	err = mana_gd_send_request(gc, sizeof(req), &req, sizeof(resp), &resp);
+	if (err) {
+		ibdev_err(&mdev->ib_dev, "Failed to create RNIC adapter err %d", err);
+		goto cleanup;
+	}
+	mdev->adapter_handle = resp.adapter;
+
 	return;
 
 cleanup:
@@ -599,5 +616,19 @@ void mana_ib_gd_create_rnic_adapter(struct mana_ib_dev *mdev)
 
 void mana_ib_gd_destroy_rnic_adapter(struct mana_ib_dev *mdev)
 {
+	struct mana_rnic_destroy_adapter_resp resp = {};
+	struct mana_rnic_destroy_adapter_req req = {};
+	struct gdma_context *gc;
+
+	if (!rnic_is_enabled(mdev))
+		return;
+
+	gc = mdev_to_gc(mdev);
+	mana_gd_init_req_hdr(&req.hdr, MANA_IB_DESTROY_ADAPTER, sizeof(req), sizeof(resp));
+	req.hdr.dev_id = gc->mana_ib.dev_id;
+	req.adapter = mdev->adapter_handle;
+
+	mana_gd_send_request(gc, sizeof(req), &req, sizeof(resp), &resp);
+	mdev->adapter_handle = INVALID_MANA_HANDLE;
 	mana_ib_destroy_eqs(mdev);
 }
diff --git a/drivers/infiniband/hw/mana/mana_ib.h b/drivers/infiniband/hw/mana/mana_ib.h
index a4b94ee..96454cf 100644
--- a/drivers/infiniband/hw/mana/mana_ib.h
+++ b/drivers/infiniband/hw/mana/mana_ib.h
@@ -48,6 +48,7 @@ struct mana_ib_adapter_caps {
 struct mana_ib_dev {
 	struct ib_device ib_dev;
 	struct gdma_dev *gdma_dev;
+	mana_handle_t adapter_handle;
 	struct gdma_queue *fatal_err_eq;
 	struct mana_ib_adapter_caps adapter_caps;
 };
@@ -115,6 +116,8 @@ struct mana_ib_rwq_ind_table {
 
 enum mana_ib_command_code {
 	MANA_IB_GET_ADAPTER_CAP = 0x30001,
+	MANA_IB_CREATE_ADAPTER  = 0x30002,
+	MANA_IB_DESTROY_ADAPTER = 0x30003,
 };
 
 struct mana_ib_query_adapter_caps_req {
@@ -143,6 +146,32 @@ struct mana_ib_query_adapter_caps_resp {
 	u32 max_inline_data_size;
 }; /* HW Data */
 
+struct mana_rnic_create_adapter_req {
+	struct gdma_req_hdr hdr;
+	u32 notify_eq_id;
+	u32 reserved;
+	u64 feature_flags;
+}; /*HW Data */
+
+struct mana_rnic_create_adapter_resp {
+	struct gdma_resp_hdr hdr;
+	mana_handle_t adapter;
+}; /* HW Data */
+
+struct mana_rnic_destroy_adapter_req {
+	struct gdma_req_hdr hdr;
+	mana_handle_t adapter;
+}; /*HW Data */
+
+struct mana_rnic_destroy_adapter_resp {
+	struct gdma_resp_hdr hdr;
+}; /* HW Data */
+
+static inline bool rnic_is_enabled(struct mana_ib_dev *mdev)
+{
+	return mdev->adapter_handle != INVALID_MANA_HANDLE;
+}
+
 static inline struct gdma_context *mdev_to_gc(struct mana_ib_dev *mdev)
 {
 	return mdev->gdma_dev->gdma_context;
-- 
1.8.3.1

[PATCH rdma-next v2 3/5] RDMA/mana_ib: Implement port parameters

[Konstantin Taranov]

Implement port parameters for RNIC. Only port 1 is RoCEv2 capable.

Signed-off-by: Konstantin Taranov <kotaranov@linux.microsoft.com>
---
 drivers/infiniband/hw/mana/device.c  |  2 ++
 drivers/infiniband/hw/mana/main.c    | 37 +++++++++++++++++++++++++++++++++++-
 drivers/infiniband/hw/mana/mana_ib.h |  4 ++++
 3 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/mana/device.c b/drivers/infiniband/hw/mana/device.c
index d8e8b10..11b0410 100644
--- a/drivers/infiniband/hw/mana/device.c
+++ b/drivers/infiniband/hw/mana/device.c
@@ -29,12 +29,14 @@
 	.destroy_rwq_ind_table = mana_ib_destroy_rwq_ind_table,
 	.destroy_wq = mana_ib_destroy_wq,
 	.disassociate_ucontext = mana_ib_disassociate_ucontext,
+	.get_link_layer = mana_ib_get_link_layer,
 	.get_port_immutable = mana_ib_get_port_immutable,
 	.mmap = mana_ib_mmap,
 	.modify_qp = mana_ib_modify_qp,
 	.modify_wq = mana_ib_modify_wq,
 	.query_device = mana_ib_query_device,
 	.query_gid = mana_ib_query_gid,
+	.query_pkey = mana_ib_query_pkey,
 	.query_port = mana_ib_query_port,
 	.reg_user_mr = mana_ib_reg_user_mr,
 
diff --git a/drivers/infiniband/hw/mana/main.c b/drivers/infiniband/hw/mana/main.c
index 33cd69e..3e05a62 100644
--- a/drivers/infiniband/hw/mana/main.c
+++ b/drivers/infiniband/hw/mana/main.c
@@ -492,7 +492,42 @@ int mana_ib_query_device(struct ib_device *ibdev, struct ib_device_attr *props,
 int mana_ib_query_port(struct ib_device *ibdev, u32 port,
 		       struct ib_port_attr *props)
 {
-	/* This version doesn't return port properties */
+	struct net_device *ndev = mana_ib_get_netdev(ibdev, port);
+
+	if (!ndev)
+		return -EINVAL;
+
+	memset(props, 0, sizeof(*props));
+	props->max_mtu = IB_MTU_4096;
+	props->active_mtu = ib_mtu_int_to_enum(ndev->mtu);
+
+	if (netif_carrier_ok(ndev) && netif_running(ndev)) {
+		props->state = IB_PORT_ACTIVE;
+		props->phys_state = IB_PORT_PHYS_STATE_LINK_UP;
+	} else {
+		props->state = IB_PORT_DOWN;
+		props->phys_state = IB_PORT_PHYS_STATE_DISABLED;
+	}
+
+	props->active_width = IB_WIDTH_4X;
+	props->active_speed = IB_SPEED_EDR;
+	props->pkey_tbl_len = 1;
+	if (port == 1)
+		props->gid_tbl_len = 16;
+
+	return 0;
+}
+
+enum rdma_link_layer mana_ib_get_link_layer(struct ib_device *device, u32 port_num)
+{
+	return IB_LINK_LAYER_ETHERNET;
+}
+
+int mana_ib_query_pkey(struct ib_device *ibdev, u32 port, u16 index, u16 *pkey)
+{
+	if (index != 0)
+		return -EINVAL;
+	*pkey = IB_DEFAULT_PKEY_FULL;
 	return 0;
 }
 
diff --git a/drivers/infiniband/hw/mana/mana_ib.h b/drivers/infiniband/hw/mana/mana_ib.h
index 96454cf..196f3c8 100644
--- a/drivers/infiniband/hw/mana/mana_ib.h
+++ b/drivers/infiniband/hw/mana/mana_ib.h
@@ -262,4 +262,8 @@ int mana_ib_query_gid(struct ib_device *ibdev, u32 port, int index,
 void mana_ib_gd_create_rnic_adapter(struct mana_ib_dev *mdev);
 
 void mana_ib_gd_destroy_rnic_adapter(struct mana_ib_dev *mdev);
+
+int mana_ib_query_pkey(struct ib_device *ibdev, u32 port, u16 index, u16 *pkey);
+
+enum rdma_link_layer mana_ib_get_link_layer(struct ib_device *device, u32 port_num);
 #endif
-- 
1.8.3.1

[PATCH rdma-next v2 4/5] RDMA/mana_ib: Enable RoCE on port 1

[Konstantin Taranov]

Set netdev and RoCEv2 flag to be used in GID population.
We need GIDs of the master netdev and mc->ports stores slave devices.

Signed-off-by: Konstantin Taranov <kotaranov@linux.microsoft.com>
---
 drivers/infiniband/hw/mana/device.c | 15 +++++++++++++++
 drivers/infiniband/hw/mana/main.c   | 16 ++++++++++++----
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/mana/device.c b/drivers/infiniband/hw/mana/device.c
index 11b0410..2b362f5 100644
--- a/drivers/infiniband/hw/mana/device.c
+++ b/drivers/infiniband/hw/mana/device.c
@@ -53,6 +53,7 @@ static int mana_ib_probe(struct auxiliary_device *adev,
 {
 	struct mana_adev *madev = container_of(adev, struct mana_adev, adev);
 	struct gdma_dev *mdev = madev->mdev;
+	struct net_device *upper_ndev;
 	struct mana_context *mc;
 	struct mana_ib_dev *dev;
 	int ret;
@@ -79,6 +80,20 @@ static int mana_ib_probe(struct auxiliary_device *adev,
 	dev->ib_dev.num_comp_vectors = 1;
 	dev->ib_dev.dev.parent = mdev->gdma_context->dev;
 
+	rcu_read_lock(); /* required to get upper dev */
+	upper_ndev = netdev_master_upper_dev_get_rcu(mc->ports[0]);
+	if (!upper_ndev) {
+		rcu_read_unlock();
+		ibdev_err(&dev->ib_dev, "Failed to get master netdev");
+		goto free_ib_device;
+	}
+	ret = ib_device_set_netdev(&dev->ib_dev, upper_ndev, 1);
+	rcu_read_unlock();
+	if (ret) {
+		ibdev_err(&dev->ib_dev, "Failed to set ib netdev, ret %d", ret);
+		goto free_ib_device;
+	}
+
 	ret = mana_gd_register_device(&mdev->gdma_context->mana_ib);
 	if (ret) {
 		ibdev_err(&dev->ib_dev, "Failed to register device, ret %d",
diff --git a/drivers/infiniband/hw/mana/main.c b/drivers/infiniband/hw/mana/main.c
index 3e05a62..645abf3 100644
--- a/drivers/infiniband/hw/mana/main.c
+++ b/drivers/infiniband/hw/mana/main.c
@@ -462,11 +462,19 @@ int mana_ib_mmap(struct ib_ucontext *ibcontext, struct vm_area_struct *vma)
 int mana_ib_get_port_immutable(struct ib_device *ibdev, u32 port_num,
 			       struct ib_port_immutable *immutable)
 {
-	/*
-	 * This version only support RAW_PACKET
-	 * other values need to be filled for other types
-	 */
+	struct mana_ib_dev *mdev = container_of(ibdev, struct mana_ib_dev, ib_dev);
+	struct ib_port_attr attr;
+	int err;
+
+	err = ib_query_port(ibdev, port_num, &attr);
+	if (err)
+		return err;
+
+	immutable->pkey_tbl_len = attr.pkey_tbl_len;
+	immutable->gid_tbl_len = attr.gid_tbl_len;
 	immutable->core_cap_flags = RDMA_CORE_PORT_RAW_PACKET;
+	if (port_num == 1 && rnic_is_enabled(mdev))
+		immutable->core_cap_flags |= RDMA_CORE_PORT_IBA_ROCE_UDP_ENCAP;
 
 	return 0;
 }
-- 
1.8.3.1

[PATCH rdma-next v2 5/5] RDMA/mana_ib: Adding and deleting GIDs

[Konstantin Taranov]

Implement add_gid and del_gid for RNIC.
We support ipv4 and ipv6 addresses.

Signed-off-by: Konstantin Taranov <kotaranov@linux.microsoft.com>
---
 drivers/infiniband/hw/mana/device.c  |  2 ++
 drivers/infiniband/hw/mana/main.c    | 66 ++++++++++++++++++++++++++++++++++++
 drivers/infiniband/hw/mana/mana_ib.h | 37 ++++++++++++++++++++
 3 files changed, 105 insertions(+)

diff --git a/drivers/infiniband/hw/mana/device.c b/drivers/infiniband/hw/mana/device.c
index 2b362f5..9fb515b 100644
--- a/drivers/infiniband/hw/mana/device.c
+++ b/drivers/infiniband/hw/mana/device.c
@@ -15,6 +15,7 @@
 	.driver_id = RDMA_DRIVER_MANA,
 	.uverbs_abi_ver = MANA_IB_UVERBS_ABI_VERSION,
 
+	.add_gid = mana_ib_gd_add_gid,
 	.alloc_pd = mana_ib_alloc_pd,
 	.alloc_ucontext = mana_ib_alloc_ucontext,
 	.create_cq = mana_ib_create_cq,
@@ -23,6 +24,7 @@
 	.create_wq = mana_ib_create_wq,
 	.dealloc_pd = mana_ib_dealloc_pd,
 	.dealloc_ucontext = mana_ib_dealloc_ucontext,
+	.del_gid = mana_ib_gd_del_gid,
 	.dereg_mr = mana_ib_dereg_mr,
 	.destroy_cq = mana_ib_destroy_cq,
 	.destroy_qp = mana_ib_destroy_qp,
diff --git a/drivers/infiniband/hw/mana/main.c b/drivers/infiniband/hw/mana/main.c
index 645abf3..282c024 100644
--- a/drivers/infiniband/hw/mana/main.c
+++ b/drivers/infiniband/hw/mana/main.c
@@ -675,3 +675,69 @@ void mana_ib_gd_destroy_rnic_adapter(struct mana_ib_dev *mdev)
 	mdev->adapter_handle = INVALID_MANA_HANDLE;
 	mana_ib_destroy_eqs(mdev);
 }
+
+int mana_ib_gd_add_gid(const struct ib_gid_attr *attr, void **context)
+{
+	struct mana_ib_dev *mdev = container_of(attr->device, struct mana_ib_dev, ib_dev);
+	enum rdma_network_type ntype = rdma_gid_attr_network_type(attr);
+	struct mana_rnic_config_addr_resp resp = {};
+	struct gdma_context *gc = mdev_to_gc(mdev);
+	struct mana_rnic_config_addr_req req = {};
+	int err;
+
+	if (!rnic_is_enabled(mdev))
+		return -EINVAL;
+
+	if (ntype != RDMA_NETWORK_IPV4 && ntype != RDMA_NETWORK_IPV6) {
+		ibdev_dbg(&mdev->ib_dev, "Unsupported rdma network type %d", ntype);
+		return -EINVAL;
+	}
+
+	mana_gd_init_req_hdr(&req.hdr, MANA_IB_CONFIG_IP_ADDR, sizeof(req), sizeof(resp));
+	req.hdr.dev_id = gc->mana_ib.dev_id;
+	req.adapter = mdev->adapter_handle;
+	req.op = ADDR_OP_ADD;
+	req.sgid_type = (ntype == RDMA_NETWORK_IPV6) ? SGID_TYPE_IPV6 : SGID_TYPE_IPV4;
+	copy_in_reverse(req.ip_addr, attr->gid.raw, sizeof(union ib_gid));
+
+	err = mana_gd_send_request(gc, sizeof(req), &req, sizeof(resp), &resp);
+	if (err) {
+		ibdev_err(&mdev->ib_dev, "Failed to config IP addr err %d\n", err);
+		return err;
+	}
+
+	return 0;
+}
+
+int mana_ib_gd_del_gid(const struct ib_gid_attr *attr, void **context)
+{
+	struct mana_ib_dev *mdev = container_of(attr->device, struct mana_ib_dev, ib_dev);
+	enum rdma_network_type ntype = rdma_gid_attr_network_type(attr);
+	struct mana_rnic_config_addr_resp resp = {};
+	struct gdma_context *gc = mdev_to_gc(mdev);
+	struct mana_rnic_config_addr_req req = {};
+	int err;
+
+	if (!rnic_is_enabled(mdev))
+		return -EINVAL;
+
+	if (ntype != RDMA_NETWORK_IPV4 && ntype != RDMA_NETWORK_IPV6) {
+		ibdev_dbg(&mdev->ib_dev, "Unsupported rdma network type %d", ntype);
+		return -EINVAL;
+	}
+
+	mana_gd_init_req_hdr(&req.hdr, MANA_IB_CONFIG_IP_ADDR, sizeof(req), sizeof(resp));
+	req.hdr.dev_id = gc->mana_ib.dev_id;
+	req.adapter = mdev->adapter_handle;
+	req.op = ADDR_OP_REMOVE;
+	req.sgid_type = (ntype == RDMA_NETWORK_IPV6) ? SGID_TYPE_IPV6 : SGID_TYPE_IPV4;
+	copy_in_reverse(req.ip_addr, attr->gid.raw, sizeof(union ib_gid));
+
+	err = mana_gd_send_request(gc, sizeof(req), &req, sizeof(resp), &resp);
+	if (err) {
+		ibdev_err(&mdev->ib_dev, "Failed to config IP addr err %d\n", err);
+		return err;
+	}
+
+	return 0;
+}
diff --git a/drivers/infiniband/hw/mana/mana_ib.h b/drivers/infiniband/hw/mana/mana_ib.h
index 196f3c8..2a3e3b0 100644
--- a/drivers/infiniband/hw/mana/mana_ib.h
+++ b/drivers/infiniband/hw/mana/mana_ib.h
@@ -118,6 +118,7 @@ enum mana_ib_command_code {
 	MANA_IB_GET_ADAPTER_CAP = 0x30001,
 	MANA_IB_CREATE_ADAPTER  = 0x30002,
 	MANA_IB_DESTROY_ADAPTER = 0x30003,
+	MANA_IB_CONFIG_IP_ADDR	= 0x30004,
 };
 
 struct mana_ib_query_adapter_caps_req {
@@ -167,6 +168,30 @@ struct mana_rnic_destroy_adapter_resp {
 	struct gdma_resp_hdr hdr;
 }; /* HW Data */
 
+enum mana_ib_addr_op {
+	ADDR_OP_ADD = 1,
+	ADDR_OP_REMOVE,
+};
+
+enum sgid_entry_type {
+	SGID_TYPE_INVALID = 0,
+	SGID_TYPE_IPV4 = 1,
+	SGID_TYPE_IPV6 = 2,
+	SGID_TYPE_HYBRID = 3
+};
+
+struct mana_rnic_config_addr_req {
+	struct gdma_req_hdr hdr;
+	mana_handle_t adapter;
+	enum mana_ib_addr_op op;
+	enum sgid_entry_type sgid_type;
+	u8 ip_addr[16];
+}; /* HW Data */
+
+struct mana_rnic_config_addr_resp {
+	struct gdma_resp_hdr hdr;
+}; /* HW Data */
+
 static inline bool rnic_is_enabled(struct mana_ib_dev *mdev)
 {
 	return mdev->adapter_handle != INVALID_MANA_HANDLE;
@@ -188,6 +213,14 @@ static inline struct net_device *mana_ib_get_netdev(struct ib_device *ibdev, u32
 	return mc->ports[port - 1];
 }
 
+static inline void copy_in_reverse(u8 *dst, const u8 *src, u32 size)
+{
+	u32 i;
+
+	for (i = 0; i < size; i++)
+		dst[size - 1 - i] = src[i];
+}
+
 int mana_ib_install_cq_cb(struct mana_ib_dev *mdev, struct mana_ib_cq *cq);
 
 int mana_ib_gd_create_dma_region(struct mana_ib_dev *dev, struct ib_umem *umem,
@@ -266,4 +299,8 @@ int mana_ib_query_gid(struct ib_device *ibdev, u32 port, int index,
 int mana_ib_query_pkey(struct ib_device *ibdev, u32 port, u16 index, u16 *pkey);
 
 enum rdma_link_layer mana_ib_get_link_layer(struct ib_device *device, u32 port_num);
+
+int mana_ib_gd_add_gid(const struct ib_gid_attr *attr, void **context);
+
+int mana_ib_gd_del_gid(const struct ib_gid_attr *attr, void **context);
 #endif
-- 
1.8.3.1

Re: [PATCH rdma-next v2 1/5] RDMA/mana_ib: Add EQ creation for rnic adapter

[Leon Romanovsky]

On Fri, Feb 02, 2024 at 07:06:33AM -0800, Konstantin Taranov wrote:
> This patch introduces functions for RNIC creation
> and creates one EQ for RNIC creation.

Please invest more time in commit messages, it is obvious that this
patch "introduces functions for RNIC creation"" by looking in the code.

> 
> Signed-off-by: Konstantin Taranov <kotaranov@linux.microsoft.com>
> ---
>  drivers/infiniband/hw/mana/device.c  |  9 ++++--
>  drivers/infiniband/hw/mana/main.c    | 53 ++++++++++++++++++++++++++++++++++++
>  drivers/infiniband/hw/mana/mana_ib.h |  5 ++++
>  3 files changed, 64 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/mana/device.c b/drivers/infiniband/hw/mana/device.c
> index 6fa902e..d8e8b10 100644
> --- a/drivers/infiniband/hw/mana/device.c
> +++ b/drivers/infiniband/hw/mana/device.c
> @@ -92,15 +92,19 @@ static int mana_ib_probe(struct auxiliary_device *adev,
>  		goto deregister_device;
>  	}
>  
> +	mana_ib_gd_create_rnic_adapter(dev);
> +
>  	ret = ib_register_device(&dev->ib_dev, "mana_%d",
>  				 mdev->gdma_context->dev);
>  	if (ret)
> -		goto deregister_device;
> +		goto destroy_rnic_adapter;
>  
>  	dev_set_drvdata(&adev->dev, dev);
>  
>  	return 0;
>  
> +destroy_rnic_adapter:
> +	mana_ib_gd_destroy_rnic_adapter(dev);
>  deregister_device:
>  	mana_gd_deregister_device(dev->gdma_dev);
>  free_ib_device:
> @@ -113,9 +117,8 @@ static void mana_ib_remove(struct auxiliary_device *adev)
>  	struct mana_ib_dev *dev = dev_get_drvdata(&adev->dev);
>  
>  	ib_unregister_device(&dev->ib_dev);
> -
> +	mana_ib_gd_destroy_rnic_adapter(dev);
>  	mana_gd_deregister_device(dev->gdma_dev);
> -
>  	ib_dealloc_device(&dev->ib_dev);
>  }
>  
> diff --git a/drivers/infiniband/hw/mana/main.c b/drivers/infiniband/hw/mana/main.c
> index 29dd243..c64d569 100644
> --- a/drivers/infiniband/hw/mana/main.c
> +++ b/drivers/infiniband/hw/mana/main.c
> @@ -548,3 +548,56 @@ int mana_ib_gd_query_adapter_caps(struct mana_ib_dev *dev)
>  
>  	return 0;
>  }
> +
> +static int mana_ib_create_eqs(struct mana_ib_dev *mdev)
> +{
> +	struct gdma_context *gc = mdev_to_gc(mdev);
> +	struct gdma_queue_spec spec = {};
> +	int err;
> +
> +	spec.type = GDMA_EQ;
> +	spec.monitor_avl_buf = false;
> +	spec.queue_size = EQ_SIZE;
> +	spec.eq.callback = NULL;
> +	spec.eq.context = mdev;
> +	spec.eq.log2_throttle_limit = LOG2_EQ_THROTTLE;
> +	spec.eq.msix_index = 0;
> +
> +	err = mana_gd_create_mana_eq(&gc->mana_ib, &spec, &mdev->fatal_err_eq);
> +	if (err)
> +		return err;
> +
> +	return 0;
> +}
> +
> +static void mana_ib_destroy_eqs(struct mana_ib_dev *mdev)
> +{
> +	if (!mdev->fatal_err_eq)
> +		return;
> +
> +	mana_gd_destroy_queue(mdev_to_gc(mdev), mdev->fatal_err_eq);
> +	mdev->fatal_err_eq = NULL;

Please don't set NULL to the pointers if they are not used anymore.

> +}
> +
> +void mana_ib_gd_create_rnic_adapter(struct mana_ib_dev *mdev)
> +{
> +	int err;
> +
> +	err = mana_ib_create_eqs(mdev);
> +	if (err) {
> +		ibdev_err(&mdev->ib_dev, "Failed to create EQs for RNIC err %d", err);
> +		goto cleanup;
> +	}
> +
> +	return;
> +
> +cleanup:
> +	ibdev_warn(&mdev->ib_dev,
> +		   "RNIC is not available. Only RAW QPs are supported");
> +	mana_ib_destroy_eqs(mdev);

If mana_ib_create_eqs() fails, you shouldn't call to mana_ib_destroy_eqs().
mana_ib_create_eqs() needs to clean everything when it fails.

Thanks

> +}
> +
> +void mana_ib_gd_destroy_rnic_adapter(struct mana_ib_dev *mdev)
> +{
> +	mana_ib_destroy_eqs(mdev);
> +}
> diff --git a/drivers/infiniband/hw/mana/mana_ib.h b/drivers/infiniband/hw/mana/mana_ib.h
> index 6a03ae6..a4b94ee 100644
> --- a/drivers/infiniband/hw/mana/mana_ib.h
> +++ b/drivers/infiniband/hw/mana/mana_ib.h
> @@ -48,6 +48,7 @@ struct mana_ib_adapter_caps {
>  struct mana_ib_dev {
>  	struct ib_device ib_dev;
>  	struct gdma_dev *gdma_dev;
> +	struct gdma_queue *fatal_err_eq;
>  	struct mana_ib_adapter_caps adapter_caps;
>  };
>  
> @@ -228,4 +229,8 @@ int mana_ib_query_gid(struct ib_device *ibdev, u32 port, int index,
>  void mana_ib_disassociate_ucontext(struct ib_ucontext *ibcontext);
>  
>  int mana_ib_gd_query_adapter_caps(struct mana_ib_dev *mdev);
> +
> +void mana_ib_gd_create_rnic_adapter(struct mana_ib_dev *mdev);
> +
> +void mana_ib_gd_destroy_rnic_adapter(struct mana_ib_dev *mdev);
>  #endif
> -- 
> 1.8.3.1
> 

Re: [PATCH rdma-next v2 2/5] RDMA/mana_ib: Create and destroy rnic adapter

[Leon Romanovsky]

On Fri, Feb 02, 2024 at 07:06:34AM -0800, Konstantin Taranov wrote:
> This patch adds RNIC creation and destruction.
> If creation of RNIC fails, we support only RAW QPs as they are served by
> ethernet driver.

So please make sure that you are creating RNIC only when you are
supporting it. The idea that some function tries-and-fails with dmesg
errors is not good idea.

Thanks

> 
> Signed-off-by: Konstantin Taranov <kotaranov@linux.microsoft.com>
> ---
>  drivers/infiniband/hw/mana/main.c    | 31 +++++++++++++++++++++++++++++++
>  drivers/infiniband/hw/mana/mana_ib.h | 29 +++++++++++++++++++++++++++++
>  2 files changed, 60 insertions(+)
> 
> diff --git a/drivers/infiniband/hw/mana/main.c b/drivers/infiniband/hw/mana/main.c
> index c64d569..33cd69e 100644
> --- a/drivers/infiniband/hw/mana/main.c
> +++ b/drivers/infiniband/hw/mana/main.c
> @@ -581,14 +581,31 @@ static void mana_ib_destroy_eqs(struct mana_ib_dev *mdev)
>  
>  void mana_ib_gd_create_rnic_adapter(struct mana_ib_dev *mdev)
>  {
> +	struct mana_rnic_create_adapter_resp resp = {};
> +	struct mana_rnic_create_adapter_req req = {};
> +	struct gdma_context *gc = mdev_to_gc(mdev);
>  	int err;
>  
> +	mdev->adapter_handle = INVALID_MANA_HANDLE;
> +
>  	err = mana_ib_create_eqs(mdev);
>  	if (err) {
>  		ibdev_err(&mdev->ib_dev, "Failed to create EQs for RNIC err %d", err);
>  		goto cleanup;
>  	}
>  
> +	mana_gd_init_req_hdr(&req.hdr, MANA_IB_CREATE_ADAPTER, sizeof(req), sizeof(resp));
> +	req.hdr.req.msg_version = GDMA_MESSAGE_V2;
> +	req.hdr.dev_id = gc->mana_ib.dev_id;
> +	req.notify_eq_id = mdev->fatal_err_eq->id;
> +
> +	err = mana_gd_send_request(gc, sizeof(req), &req, sizeof(resp), &resp);
> +	if (err) {
> +		ibdev_err(&mdev->ib_dev, "Failed to create RNIC adapter err %d", err);
> +		goto cleanup;
> +	}
> +	mdev->adapter_handle = resp.adapter;
> +
>  	return;
>  
>  cleanup:
> @@ -599,5 +616,19 @@ void mana_ib_gd_create_rnic_adapter(struct mana_ib_dev *mdev)
>  
>  void mana_ib_gd_destroy_rnic_adapter(struct mana_ib_dev *mdev)
>  {
> +	struct mana_rnic_destroy_adapter_resp resp = {};
> +	struct mana_rnic_destroy_adapter_req req = {};
> +	struct gdma_context *gc;
> +
> +	if (!rnic_is_enabled(mdev))
> +		return;
> +
> +	gc = mdev_to_gc(mdev);
> +	mana_gd_init_req_hdr(&req.hdr, MANA_IB_DESTROY_ADAPTER, sizeof(req), sizeof(resp));
> +	req.hdr.dev_id = gc->mana_ib.dev_id;
> +	req.adapter = mdev->adapter_handle;
> +
> +	mana_gd_send_request(gc, sizeof(req), &req, sizeof(resp), &resp);
> +	mdev->adapter_handle = INVALID_MANA_HANDLE;
>  	mana_ib_destroy_eqs(mdev);
>  }
> diff --git a/drivers/infiniband/hw/mana/mana_ib.h b/drivers/infiniband/hw/mana/mana_ib.h
> index a4b94ee..96454cf 100644
> --- a/drivers/infiniband/hw/mana/mana_ib.h
> +++ b/drivers/infiniband/hw/mana/mana_ib.h
> @@ -48,6 +48,7 @@ struct mana_ib_adapter_caps {
>  struct mana_ib_dev {
>  	struct ib_device ib_dev;
>  	struct gdma_dev *gdma_dev;
> +	mana_handle_t adapter_handle;
>  	struct gdma_queue *fatal_err_eq;
>  	struct mana_ib_adapter_caps adapter_caps;
>  };
> @@ -115,6 +116,8 @@ struct mana_ib_rwq_ind_table {
>  
>  enum mana_ib_command_code {
>  	MANA_IB_GET_ADAPTER_CAP = 0x30001,
> +	MANA_IB_CREATE_ADAPTER  = 0x30002,
> +	MANA_IB_DESTROY_ADAPTER = 0x30003,
>  };
>  
>  struct mana_ib_query_adapter_caps_req {
> @@ -143,6 +146,32 @@ struct mana_ib_query_adapter_caps_resp {
>  	u32 max_inline_data_size;
>  }; /* HW Data */
>  
> +struct mana_rnic_create_adapter_req {
> +	struct gdma_req_hdr hdr;
> +	u32 notify_eq_id;
> +	u32 reserved;
> +	u64 feature_flags;
> +}; /*HW Data */
> +
> +struct mana_rnic_create_adapter_resp {
> +	struct gdma_resp_hdr hdr;
> +	mana_handle_t adapter;
> +}; /* HW Data */
> +
> +struct mana_rnic_destroy_adapter_req {
> +	struct gdma_req_hdr hdr;
> +	mana_handle_t adapter;
> +}; /*HW Data */
> +
> +struct mana_rnic_destroy_adapter_resp {
> +	struct gdma_resp_hdr hdr;
> +}; /* HW Data */
> +
> +static inline bool rnic_is_enabled(struct mana_ib_dev *mdev)
> +{
> +	return mdev->adapter_handle != INVALID_MANA_HANDLE;
> +}
> +
>  static inline struct gdma_context *mdev_to_gc(struct mana_ib_dev *mdev)
>  {
>  	return mdev->gdma_dev->gdma_context;
> -- 
> 1.8.3.1
> 

Re: [PATCH rdma-next v2 5/5] RDMA/mana_ib: Adding and deleting GIDs

[Leon Romanovsky]

On Fri, Feb 02, 2024 at 07:06:37AM -0800, Konstantin Taranov wrote:
> Implement add_gid and del_gid for RNIC.
> We support ipv4 and ipv6 addresses.
> 
> Signed-off-by: Konstantin Taranov <kotaranov@linux.microsoft.com>
> ---
>  drivers/infiniband/hw/mana/device.c  |  2 ++
>  drivers/infiniband/hw/mana/main.c    | 66 ++++++++++++++++++++++++++++++++++++
>  drivers/infiniband/hw/mana/mana_ib.h | 37 ++++++++++++++++++++
>  3 files changed, 105 insertions(+)
> 
> diff --git a/drivers/infiniband/hw/mana/device.c b/drivers/infiniband/hw/mana/device.c
> index 2b362f5..9fb515b 100644
> --- a/drivers/infiniband/hw/mana/device.c
> +++ b/drivers/infiniband/hw/mana/device.c
> @@ -15,6 +15,7 @@
>  	.driver_id = RDMA_DRIVER_MANA,
>  	.uverbs_abi_ver = MANA_IB_UVERBS_ABI_VERSION,
>  
> +	.add_gid = mana_ib_gd_add_gid,
>  	.alloc_pd = mana_ib_alloc_pd,
>  	.alloc_ucontext = mana_ib_alloc_ucontext,
>  	.create_cq = mana_ib_create_cq,
> @@ -23,6 +24,7 @@
>  	.create_wq = mana_ib_create_wq,
>  	.dealloc_pd = mana_ib_dealloc_pd,
>  	.dealloc_ucontext = mana_ib_dealloc_ucontext,
> +	.del_gid = mana_ib_gd_del_gid,
>  	.dereg_mr = mana_ib_dereg_mr,
>  	.destroy_cq = mana_ib_destroy_cq,
>  	.destroy_qp = mana_ib_destroy_qp,
> diff --git a/drivers/infiniband/hw/mana/main.c b/drivers/infiniband/hw/mana/main.c
> index 645abf3..282c024 100644
> --- a/drivers/infiniband/hw/mana/main.c
> +++ b/drivers/infiniband/hw/mana/main.c
> @@ -675,3 +675,69 @@ void mana_ib_gd_destroy_rnic_adapter(struct mana_ib_dev *mdev)
>  	mdev->adapter_handle = INVALID_MANA_HANDLE;
>  	mana_ib_destroy_eqs(mdev);
>  }
> +
> +int mana_ib_gd_add_gid(const struct ib_gid_attr *attr, void **context)
> +{
> +	struct mana_ib_dev *mdev = container_of(attr->device, struct mana_ib_dev, ib_dev);
> +	enum rdma_network_type ntype = rdma_gid_attr_network_type(attr);
> +	struct mana_rnic_config_addr_resp resp = {};
> +	struct gdma_context *gc = mdev_to_gc(mdev);
> +	struct mana_rnic_config_addr_req req = {};
> +	int err;
> +
> +	if (!rnic_is_enabled(mdev))
> +		return -EINVAL;

Set .add_gid./del_gid callbacks only when RNIC is enabled.
ib_set_device_ops() allows partial set of the callbacks.

> +
> +	if (ntype != RDMA_NETWORK_IPV4 && ntype != RDMA_NETWORK_IPV6) {
> +		ibdev_dbg(&mdev->ib_dev, "Unsupported rdma network type %d", ntype);
> +		return -EINVAL;
> +	}
> +
> +	mana_gd_init_req_hdr(&req.hdr, MANA_IB_CONFIG_IP_ADDR, sizeof(req), sizeof(resp));
> +	req.hdr.dev_id = gc->mana_ib.dev_id;
> +	req.adapter = mdev->adapter_handle;
> +	req.op = ADDR_OP_ADD;
> +	req.sgid_type = (ntype == RDMA_NETWORK_IPV6) ? SGID_TYPE_IPV6 : SGID_TYPE_IPV4;
> +	copy_in_reverse(req.ip_addr, attr->gid.raw, sizeof(union ib_gid));
> +
> +	err = mana_gd_send_request(gc, sizeof(req), &req, sizeof(resp), &resp);
> +	if (err) {
> +		ibdev_err(&mdev->ib_dev, "Failed to config IP addr err %d\n", err);
> +		return err;
> +	}
> +
> +	return 0;
> +}
> +
> +int mana_ib_gd_del_gid(const struct ib_gid_attr *attr, void **context)
> +{
> +	struct mana_ib_dev *mdev = container_of(attr->device, struct mana_ib_dev, ib_dev);
> +	enum rdma_network_type ntype = rdma_gid_attr_network_type(attr);
> +	struct mana_rnic_config_addr_resp resp = {};
> +	struct gdma_context *gc = mdev_to_gc(mdev);
> +	struct mana_rnic_config_addr_req req = {};
> +	int err;
> +
> +	if (!rnic_is_enabled(mdev))
> +		return -EINVAL;
> +
> +	if (ntype != RDMA_NETWORK_IPV4 && ntype != RDMA_NETWORK_IPV6) {
> +		ibdev_dbg(&mdev->ib_dev, "Unsupported rdma network type %d", ntype);
> +		return -EINVAL;
> +	}
> +
> +	mana_gd_init_req_hdr(&req.hdr, MANA_IB_CONFIG_IP_ADDR, sizeof(req), sizeof(resp));
> +	req.hdr.dev_id = gc->mana_ib.dev_id;
> +	req.adapter = mdev->adapter_handle;
> +	req.op = ADDR_OP_REMOVE;
> +	req.sgid_type = (ntype == RDMA_NETWORK_IPV6) ? SGID_TYPE_IPV6 : SGID_TYPE_IPV4;
> +	copy_in_reverse(req.ip_addr, attr->gid.raw, sizeof(union ib_gid));
> +
> +	err = mana_gd_send_request(gc, sizeof(req), &req, sizeof(resp), &resp);
> +	if (err) {
> +		ibdev_err(&mdev->ib_dev, "Failed to config IP addr err %d\n", err);
> +		return err;
> +	}
> +
> +	return 0;
> +}
> diff --git a/drivers/infiniband/hw/mana/mana_ib.h b/drivers/infiniband/hw/mana/mana_ib.h
> index 196f3c8..2a3e3b0 100644
> --- a/drivers/infiniband/hw/mana/mana_ib.h
> +++ b/drivers/infiniband/hw/mana/mana_ib.h
> @@ -118,6 +118,7 @@ enum mana_ib_command_code {
>  	MANA_IB_GET_ADAPTER_CAP = 0x30001,
>  	MANA_IB_CREATE_ADAPTER  = 0x30002,
>  	MANA_IB_DESTROY_ADAPTER = 0x30003,
> +	MANA_IB_CONFIG_IP_ADDR	= 0x30004,
>  };
>  
>  struct mana_ib_query_adapter_caps_req {
> @@ -167,6 +168,30 @@ struct mana_rnic_destroy_adapter_resp {
>  	struct gdma_resp_hdr hdr;
>  }; /* HW Data */
>  
> +enum mana_ib_addr_op {
> +	ADDR_OP_ADD = 1,
> +	ADDR_OP_REMOVE,
> +};
> +
> +enum sgid_entry_type {
> +	SGID_TYPE_INVALID = 0,
> +	SGID_TYPE_IPV4 = 1,
> +	SGID_TYPE_IPV6 = 2,
> +	SGID_TYPE_HYBRID = 3

This is not used, please remove it.

Thanks

RE: [EXTERNAL] Re: [PATCH rdma-next v2 2/5] RDMA/mana_ib: Create and destroy rnic adapter

[Konstantin Taranov]

> From: Leon Romanovsky <leon@kernel.org>
> On Fri, Feb 02, 2024 at 07:06:34AM -0800, Konstantin Taranov wrote:
> > This patch adds RNIC creation and destruction.
> > If creation of RNIC fails, we support only RAW QPs as they are served
> > by ethernet driver.
> 
> So please make sure that you are creating RNIC only when you are supporting
> it. The idea that some function tries-and-fails with dmesg errors is not good
> idea.
> 
> Thanks
> 

Hi Leon. Thanks for your comments and suggestion. I will incorporate them in the next version.
Regarding this "try-and-fail", we cannot guarantee now that RNIC is supported, and try-and-fail is the only way
to skip RNIC creation without impeding RAW QPs. Could you, please, suggest how we could correctly incorporate
the "try-and-fail" strategy to get it upstreamed?

> >
> > Signed-off-by: Konstantin Taranov <kotaranov@linux.microsoft.com>
> > ---
> >  drivers/infiniband/hw/mana/main.c    | 31
> +++++++++++++++++++++++++++++++
> >  drivers/infiniband/hw/mana/mana_ib.h | 29
> > +++++++++++++++++++++++++++++
> >  2 files changed, 60 insertions(+)
> >
> > diff --git a/drivers/infiniband/hw/mana/main.c
> > b/drivers/infiniband/hw/mana/main.c
> > index c64d569..33cd69e 100644
> > --- a/drivers/infiniband/hw/mana/main.c
> > +++ b/drivers/infiniband/hw/mana/main.c
> > @@ -581,14 +581,31 @@ static void mana_ib_destroy_eqs(struct
> > mana_ib_dev *mdev)
> >
> >  void mana_ib_gd_create_rnic_adapter(struct mana_ib_dev *mdev)  {
> > +     struct mana_rnic_create_adapter_resp resp = {};
> > +     struct mana_rnic_create_adapter_req req = {};
> > +     struct gdma_context *gc = mdev_to_gc(mdev);
> >       int err;
> >
> > +     mdev->adapter_handle = INVALID_MANA_HANDLE;
> > +
> >       err = mana_ib_create_eqs(mdev);
> >       if (err) {
> >               ibdev_err(&mdev->ib_dev, "Failed to create EQs for RNIC err %d",
> err);
> >               goto cleanup;
> >       }
> >
> > +     mana_gd_init_req_hdr(&req.hdr, MANA_IB_CREATE_ADAPTER,
> sizeof(req), sizeof(resp));
> > +     req.hdr.req.msg_version = GDMA_MESSAGE_V2;
> > +     req.hdr.dev_id = gc->mana_ib.dev_id;
> > +     req.notify_eq_id = mdev->fatal_err_eq->id;
> > +
> > +     err = mana_gd_send_request(gc, sizeof(req), &req, sizeof(resp), &resp);
> > +     if (err) {
> > +             ibdev_err(&mdev->ib_dev, "Failed to create RNIC adapter err %d",
> err);
> > +             goto cleanup;
> > +     }
> > +     mdev->adapter_handle = resp.adapter;
> > +
> >       return;
> >
> >  cleanup:
> > @@ -599,5 +616,19 @@ void mana_ib_gd_create_rnic_adapter(struct
> > mana_ib_dev *mdev)
> >
> >  void mana_ib_gd_destroy_rnic_adapter(struct mana_ib_dev *mdev)  {
> > +     struct mana_rnic_destroy_adapter_resp resp = {};
> > +     struct mana_rnic_destroy_adapter_req req = {};
> > +     struct gdma_context *gc;
> > +
> > +     if (!rnic_is_enabled(mdev))
> > +             return;
> > +
> > +     gc = mdev_to_gc(mdev);
> > +     mana_gd_init_req_hdr(&req.hdr, MANA_IB_DESTROY_ADAPTER,
> sizeof(req), sizeof(resp));
> > +     req.hdr.dev_id = gc->mana_ib.dev_id;
> > +     req.adapter = mdev->adapter_handle;
> > +
> > +     mana_gd_send_request(gc, sizeof(req), &req, sizeof(resp), &resp);
> > +     mdev->adapter_handle = INVALID_MANA_HANDLE;
> >       mana_ib_destroy_eqs(mdev);
> >  }
> > diff --git a/drivers/infiniband/hw/mana/mana_ib.h
> > b/drivers/infiniband/hw/mana/mana_ib.h
> > index a4b94ee..96454cf 100644
> > --- a/drivers/infiniband/hw/mana/mana_ib.h
> > +++ b/drivers/infiniband/hw/mana/mana_ib.h
> > @@ -48,6 +48,7 @@ struct mana_ib_adapter_caps {  struct mana_ib_dev {
> >       struct ib_device ib_dev;
> >       struct gdma_dev *gdma_dev;
> > +     mana_handle_t adapter_handle;
> >       struct gdma_queue *fatal_err_eq;
> >       struct mana_ib_adapter_caps adapter_caps;  }; @@ -115,6 +116,8
> > @@ struct mana_ib_rwq_ind_table {
> >
> >  enum mana_ib_command_code {
> >       MANA_IB_GET_ADAPTER_CAP = 0x30001,
> > +     MANA_IB_CREATE_ADAPTER  = 0x30002,
> > +     MANA_IB_DESTROY_ADAPTER = 0x30003,
> >  };
> >
> >  struct mana_ib_query_adapter_caps_req { @@ -143,6 +146,32 @@ struct
> > mana_ib_query_adapter_caps_resp {
> >       u32 max_inline_data_size;
> >  }; /* HW Data */
> >
> > +struct mana_rnic_create_adapter_req {
> > +     struct gdma_req_hdr hdr;
> > +     u32 notify_eq_id;
> > +     u32 reserved;
> > +     u64 feature_flags;
> > +}; /*HW Data */
> > +
> > +struct mana_rnic_create_adapter_resp {
> > +     struct gdma_resp_hdr hdr;
> > +     mana_handle_t adapter;
> > +}; /* HW Data */
> > +
> > +struct mana_rnic_destroy_adapter_req {
> > +     struct gdma_req_hdr hdr;
> > +     mana_handle_t adapter;
> > +}; /*HW Data */
> > +
> > +struct mana_rnic_destroy_adapter_resp {
> > +     struct gdma_resp_hdr hdr;
> > +}; /* HW Data */
> > +
> > +static inline bool rnic_is_enabled(struct mana_ib_dev *mdev) {
> > +     return mdev->adapter_handle != INVALID_MANA_HANDLE; }
> > +
> >  static inline struct gdma_context *mdev_to_gc(struct mana_ib_dev
> > *mdev)  {
> >       return mdev->gdma_dev->gdma_context;
> > --
> > 1.8.3.1
> >

Re: [EXTERNAL] Re: [PATCH rdma-next v2 2/5] RDMA/mana_ib: Create and destroy rnic adapter

[Leon Romanovsky]

On Sun, Feb 04, 2024 at 03:50:40PM +0000, Konstantin Taranov wrote:
> > From: Leon Romanovsky <leon@kernel.org>
> > On Fri, Feb 02, 2024 at 07:06:34AM -0800, Konstantin Taranov wrote:
> > > This patch adds RNIC creation and destruction.
> > > If creation of RNIC fails, we support only RAW QPs as they are served
> > > by ethernet driver.
> > 
> > So please make sure that you are creating RNIC only when you are supporting
> > it. The idea that some function tries-and-fails with dmesg errors is not good
> > idea.
> > 
> > Thanks
> > 
> 
> Hi Leon. Thanks for your comments and suggestion. I will incorporate them in the next version.
> Regarding this "try-and-fail", we cannot guarantee now that RNIC is supported, and try-and-fail is the only way
> to skip RNIC creation without impeding RAW QPs. Could you, please, suggest how we could correctly incorporate
> the "try-and-fail" strategy to get it upstreamed?

You already query NIC for its capabilities, so you can check if it supports RNIC.

> 
> > >
> > > Signed-off-by: Konstantin Taranov <kotaranov@linux.microsoft.com>
> > > ---
> > >  drivers/infiniband/hw/mana/main.c    | 31
> > +++++++++++++++++++++++++++++++
> > >  drivers/infiniband/hw/mana/mana_ib.h | 29
> > > +++++++++++++++++++++++++++++
> > >  2 files changed, 60 insertions(+)
> > >
> > > diff --git a/drivers/infiniband/hw/mana/main.c
> > > b/drivers/infiniband/hw/mana/main.c
> > > index c64d569..33cd69e 100644
> > > --- a/drivers/infiniband/hw/mana/main.c
> > > +++ b/drivers/infiniband/hw/mana/main.c
> > > @@ -581,14 +581,31 @@ static void mana_ib_destroy_eqs(struct
> > > mana_ib_dev *mdev)
> > >
> > >  void mana_ib_gd_create_rnic_adapter(struct mana_ib_dev *mdev)  {
> > > +     struct mana_rnic_create_adapter_resp resp = {};
> > > +     struct mana_rnic_create_adapter_req req = {};
> > > +     struct gdma_context *gc = mdev_to_gc(mdev);
> > >       int err;
> > >
> > > +     mdev->adapter_handle = INVALID_MANA_HANDLE;
> > > +
> > >       err = mana_ib_create_eqs(mdev);
> > >       if (err) {
> > >               ibdev_err(&mdev->ib_dev, "Failed to create EQs for RNIC err %d",
> > err);
> > >               goto cleanup;
> > >       }
> > >
> > > +     mana_gd_init_req_hdr(&req.hdr, MANA_IB_CREATE_ADAPTER,
> > sizeof(req), sizeof(resp));
> > > +     req.hdr.req.msg_version = GDMA_MESSAGE_V2;
> > > +     req.hdr.dev_id = gc->mana_ib.dev_id;
> > > +     req.notify_eq_id = mdev->fatal_err_eq->id;
> > > +
> > > +     err = mana_gd_send_request(gc, sizeof(req), &req, sizeof(resp), &resp);
> > > +     if (err) {
> > > +             ibdev_err(&mdev->ib_dev, "Failed to create RNIC adapter err %d",
> > err);
> > > +             goto cleanup;
> > > +     }
> > > +     mdev->adapter_handle = resp.adapter;
> > > +
> > >       return;
> > >
> > >  cleanup:
> > > @@ -599,5 +616,19 @@ void mana_ib_gd_create_rnic_adapter(struct
> > > mana_ib_dev *mdev)
> > >
> > >  void mana_ib_gd_destroy_rnic_adapter(struct mana_ib_dev *mdev)  {
> > > +     struct mana_rnic_destroy_adapter_resp resp = {};
> > > +     struct mana_rnic_destroy_adapter_req req = {};
> > > +     struct gdma_context *gc;
> > > +
> > > +     if (!rnic_is_enabled(mdev))
> > > +             return;
> > > +
> > > +     gc = mdev_to_gc(mdev);
> > > +     mana_gd_init_req_hdr(&req.hdr, MANA_IB_DESTROY_ADAPTER,
> > sizeof(req), sizeof(resp));
> > > +     req.hdr.dev_id = gc->mana_ib.dev_id;
> > > +     req.adapter = mdev->adapter_handle;
> > > +
> > > +     mana_gd_send_request(gc, sizeof(req), &req, sizeof(resp), &resp);
> > > +     mdev->adapter_handle = INVALID_MANA_HANDLE;
> > >       mana_ib_destroy_eqs(mdev);
> > >  }
> > > diff --git a/drivers/infiniband/hw/mana/mana_ib.h
> > > b/drivers/infiniband/hw/mana/mana_ib.h
> > > index a4b94ee..96454cf 100644
> > > --- a/drivers/infiniband/hw/mana/mana_ib.h
> > > +++ b/drivers/infiniband/hw/mana/mana_ib.h
> > > @@ -48,6 +48,7 @@ struct mana_ib_adapter_caps {  struct mana_ib_dev {
> > >       struct ib_device ib_dev;
> > >       struct gdma_dev *gdma_dev;
> > > +     mana_handle_t adapter_handle;
> > >       struct gdma_queue *fatal_err_eq;
> > >       struct mana_ib_adapter_caps adapter_caps;  }; @@ -115,6 +116,8
> > > @@ struct mana_ib_rwq_ind_table {
> > >
> > >  enum mana_ib_command_code {
> > >       MANA_IB_GET_ADAPTER_CAP = 0x30001,
> > > +     MANA_IB_CREATE_ADAPTER  = 0x30002,
> > > +     MANA_IB_DESTROY_ADAPTER = 0x30003,
> > >  };
> > >
> > >  struct mana_ib_query_adapter_caps_req { @@ -143,6 +146,32 @@ struct
> > > mana_ib_query_adapter_caps_resp {
> > >       u32 max_inline_data_size;
> > >  }; /* HW Data */
> > >
> > > +struct mana_rnic_create_adapter_req {
> > > +     struct gdma_req_hdr hdr;
> > > +     u32 notify_eq_id;
> > > +     u32 reserved;
> > > +     u64 feature_flags;
> > > +}; /*HW Data */
> > > +
> > > +struct mana_rnic_create_adapter_resp {
> > > +     struct gdma_resp_hdr hdr;
> > > +     mana_handle_t adapter;
> > > +}; /* HW Data */
> > > +
> > > +struct mana_rnic_destroy_adapter_req {
> > > +     struct gdma_req_hdr hdr;
> > > +     mana_handle_t adapter;
> > > +}; /*HW Data */
> > > +
> > > +struct mana_rnic_destroy_adapter_resp {
> > > +     struct gdma_resp_hdr hdr;
> > > +}; /* HW Data */
> > > +
> > > +static inline bool rnic_is_enabled(struct mana_ib_dev *mdev) {
> > > +     return mdev->adapter_handle != INVALID_MANA_HANDLE; }
> > > +
> > >  static inline struct gdma_context *mdev_to_gc(struct mana_ib_dev
> > > *mdev)  {
> > >       return mdev->gdma_dev->gdma_context;
> > > --
> > > 1.8.3.1
> > >

RE: [EXTERNAL] Re: [PATCH rdma-next v2 2/5] RDMA/mana_ib: Create and destroy rnic adapter

[Konstantin Taranov]

> From: Leon Romanovsky <leon@kernel.org>
> On Sun, Feb 04, 2024 at 03:50:40PM +0000, Konstantin Taranov wrote:
> > > From: Leon Romanovsky <leon@kernel.org> On Fri, Feb 02, 2024 at
> > > 07:06:34AM -0800, Konstantin Taranov wrote:
> > > > This patch adds RNIC creation and destruction.
> > > > If creation of RNIC fails, we support only RAW QPs as they are
> > > > served by ethernet driver.
> > >
> > > So please make sure that you are creating RNIC only when you are
> > > supporting it. The idea that some function tries-and-fails with
> > > dmesg errors is not good idea.
> > >
> > > Thanks
> > >
> >
> > Hi Leon. Thanks for your comments and suggestion. I will incorporate them
> in the next version.
> > Regarding this "try-and-fail", we cannot guarantee now that RNIC is
> > supported, and try-and-fail is the only way to skip RNIC creation
> > without impeding RAW QPs. Could you, please, suggest how we could
> correctly incorporate the "try-and-fail" strategy to get it upstreamed?
> 
> You already query NIC for its capabilities, so you can check if it supports RNIC.

At the moment, the capabilities do not indicate whether RNIC creation will be successful.
The reason is additional checks during RNIC creation that are not reflected in capabilities.
The question is whether we can have the proposed "try and disable" or we must opt for failing the whole mana_ib.

> 
> >
> > > >
> > > > Signed-off-by: Konstantin Taranov <kotaranov@linux.microsoft.com>
> > > > ---
> > > >  drivers/infiniband/hw/mana/main.c    | 31
> > > +++++++++++++++++++++++++++++++
> > > >  drivers/infiniband/hw/mana/mana_ib.h | 29
> > > > +++++++++++++++++++++++++++++
> > > >  2 files changed, 60 insertions(+)
> > > >
> > > > diff --git a/drivers/infiniband/hw/mana/main.c
> > > > b/drivers/infiniband/hw/mana/main.c
> > > > index c64d569..33cd69e 100644
> > > > --- a/drivers/infiniband/hw/mana/main.c
> > > > +++ b/drivers/infiniband/hw/mana/main.c
> > > > @@ -581,14 +581,31 @@ static void mana_ib_destroy_eqs(struct
> > > > mana_ib_dev *mdev)
> > > >
> > > >  void mana_ib_gd_create_rnic_adapter(struct mana_ib_dev *mdev)  {
> > > > +     struct mana_rnic_create_adapter_resp resp = {};
> > > > +     struct mana_rnic_create_adapter_req req = {};
> > > > +     struct gdma_context *gc = mdev_to_gc(mdev);
> > > >       int err;
> > > >
> > > > +     mdev->adapter_handle = INVALID_MANA_HANDLE;
> > > > +
> > > >       err = mana_ib_create_eqs(mdev);
> > > >       if (err) {
> > > >               ibdev_err(&mdev->ib_dev, "Failed to create EQs for
> > > > RNIC err %d",
> > > err);
> > > >               goto cleanup;
> > > >       }
> > > >
> > > > +     mana_gd_init_req_hdr(&req.hdr, MANA_IB_CREATE_ADAPTER,
> > > sizeof(req), sizeof(resp));
> > > > +     req.hdr.req.msg_version = GDMA_MESSAGE_V2;
> > > > +     req.hdr.dev_id = gc->mana_ib.dev_id;
> > > > +     req.notify_eq_id = mdev->fatal_err_eq->id;
> > > > +
> > > > +     err = mana_gd_send_request(gc, sizeof(req), &req, sizeof(resp),
> &resp);
> > > > +     if (err) {
> > > > +             ibdev_err(&mdev->ib_dev, "Failed to create RNIC
> > > > + adapter err %d",
> > > err);
> > > > +             goto cleanup;
> > > > +     }
> > > > +     mdev->adapter_handle = resp.adapter;
> > > > +
> > > >       return;
> > > >
> > > >  cleanup:
> > > > @@ -599,5 +616,19 @@ void mana_ib_gd_create_rnic_adapter(struct
> > > > mana_ib_dev *mdev)
> > > >
> > > >  void mana_ib_gd_destroy_rnic_adapter(struct mana_ib_dev *mdev)  {
> > > > +     struct mana_rnic_destroy_adapter_resp resp = {};
> > > > +     struct mana_rnic_destroy_adapter_req req = {};
> > > > +     struct gdma_context *gc;
> > > > +
> > > > +     if (!rnic_is_enabled(mdev))
> > > > +             return;
> > > > +
> > > > +     gc = mdev_to_gc(mdev);
> > > > +     mana_gd_init_req_hdr(&req.hdr, MANA_IB_DESTROY_ADAPTER,
> > > sizeof(req), sizeof(resp));
> > > > +     req.hdr.dev_id = gc->mana_ib.dev_id;
> > > > +     req.adapter = mdev->adapter_handle;
> > > > +
> > > > +     mana_gd_send_request(gc, sizeof(req), &req, sizeof(resp), &resp);
> > > > +     mdev->adapter_handle = INVALID_MANA_HANDLE;
> > > >       mana_ib_destroy_eqs(mdev);
> > > >  }
> > > > diff --git a/drivers/infiniband/hw/mana/mana_ib.h
> > > > b/drivers/infiniband/hw/mana/mana_ib.h
> > > > index a4b94ee..96454cf 100644
> > > > --- a/drivers/infiniband/hw/mana/mana_ib.h
> > > > +++ b/drivers/infiniband/hw/mana/mana_ib.h
> > > > @@ -48,6 +48,7 @@ struct mana_ib_adapter_caps {  struct
> mana_ib_dev {
> > > >       struct ib_device ib_dev;
> > > >       struct gdma_dev *gdma_dev;
> > > > +     mana_handle_t adapter_handle;
> > > >       struct gdma_queue *fatal_err_eq;
> > > >       struct mana_ib_adapter_caps adapter_caps;  }; @@ -115,6
> > > > +116,8 @@ struct mana_ib_rwq_ind_table {
> > > >
> > > >  enum mana_ib_command_code {
> > > >       MANA_IB_GET_ADAPTER_CAP = 0x30001,
> > > > +     MANA_IB_CREATE_ADAPTER  = 0x30002,
> > > > +     MANA_IB_DESTROY_ADAPTER = 0x30003,
> > > >  };
> > > >
> > > >  struct mana_ib_query_adapter_caps_req { @@ -143,6 +146,32 @@
> > > > struct mana_ib_query_adapter_caps_resp {
> > > >       u32 max_inline_data_size;
> > > >  }; /* HW Data */
> > > >
> > > > +struct mana_rnic_create_adapter_req {
> > > > +     struct gdma_req_hdr hdr;
> > > > +     u32 notify_eq_id;
> > > > +     u32 reserved;
> > > > +     u64 feature_flags;
> > > > +}; /*HW Data */
> > > > +
> > > > +struct mana_rnic_create_adapter_resp {
> > > > +     struct gdma_resp_hdr hdr;
> > > > +     mana_handle_t adapter;
> > > > +}; /* HW Data */
> > > > +
> > > > +struct mana_rnic_destroy_adapter_req {
> > > > +     struct gdma_req_hdr hdr;
> > > > +     mana_handle_t adapter;
> > > > +}; /*HW Data */
> > > > +
> > > > +struct mana_rnic_destroy_adapter_resp {
> > > > +     struct gdma_resp_hdr hdr;
> > > > +}; /* HW Data */
> > > > +
> > > > +static inline bool rnic_is_enabled(struct mana_ib_dev *mdev) {
> > > > +     return mdev->adapter_handle != INVALID_MANA_HANDLE; }
> > > > +
> > > >  static inline struct gdma_context *mdev_to_gc(struct mana_ib_dev
> > > > *mdev)  {
> > > >       return mdev->gdma_dev->gdma_context;
> > > > --
> > > > 1.8.3.1
> > > >

Re: [EXTERNAL] Re: [PATCH rdma-next v2 2/5] RDMA/mana_ib: Create and destroy rnic adapter

[Leon Romanovsky]

On Sun, Feb 04, 2024 at 05:17:59PM +0000, Konstantin Taranov wrote:
> > From: Leon Romanovsky <leon@kernel.org>
> > On Sun, Feb 04, 2024 at 03:50:40PM +0000, Konstantin Taranov wrote:
> > > > From: Leon Romanovsky <leon@kernel.org> On Fri, Feb 02, 2024 at
> > > > 07:06:34AM -0800, Konstantin Taranov wrote:
> > > > > This patch adds RNIC creation and destruction.
> > > > > If creation of RNIC fails, we support only RAW QPs as they are
> > > > > served by ethernet driver.
> > > >
> > > > So please make sure that you are creating RNIC only when you are
> > > > supporting it. The idea that some function tries-and-fails with
> > > > dmesg errors is not good idea.
> > > >
> > > > Thanks
> > > >
> > >
> > > Hi Leon. Thanks for your comments and suggestion. I will incorporate them
> > in the next version.
> > > Regarding this "try-and-fail", we cannot guarantee now that RNIC is
> > > supported, and try-and-fail is the only way to skip RNIC creation
> > > without impeding RAW QPs. Could you, please, suggest how we could
> > correctly incorporate the "try-and-fail" strategy to get it upstreamed?
> > 
> > You already query NIC for its capabilities, so you can check if it supports RNIC.
> 
> At the moment, the capabilities do not indicate whether RNIC creation will be successful.
> The reason is additional checks during RNIC creation that are not reflected in capabilities.
> The question is whether we can have the proposed "try and disable" or we must opt for failing the whole mana_ib.

RNIC creation can be seen as an example of any other feature which will
be added later, you will never know if it will be successful or not
without capabilities.

If you continue with this try-and-fail approach, I afraid that you will
end up with whole driver written in this style. Style where you don't
separate between "real" failures (wrong configuration, OOM e.t.c) and
"expected" failures (feature is not supported).

Thanks

> 
> > 
> > >
> > > > >
> > > > > Signed-off-by: Konstantin Taranov <kotaranov@linux.microsoft.com>
> > > > > ---
> > > > >  drivers/infiniband/hw/mana/main.c    | 31
> > > > +++++++++++++++++++++++++++++++
> > > > >  drivers/infiniband/hw/mana/mana_ib.h | 29
> > > > > +++++++++++++++++++++++++++++
> > > > >  2 files changed, 60 insertions(+)
> > > > >
> > > > > diff --git a/drivers/infiniband/hw/mana/main.c
> > > > > b/drivers/infiniband/hw/mana/main.c
> > > > > index c64d569..33cd69e 100644
> > > > > --- a/drivers/infiniband/hw/mana/main.c
> > > > > +++ b/drivers/infiniband/hw/mana/main.c
> > > > > @@ -581,14 +581,31 @@ static void mana_ib_destroy_eqs(struct
> > > > > mana_ib_dev *mdev)
> > > > >
> > > > >  void mana_ib_gd_create_rnic_adapter(struct mana_ib_dev *mdev)  {
> > > > > +     struct mana_rnic_create_adapter_resp resp = {};
> > > > > +     struct mana_rnic_create_adapter_req req = {};
> > > > > +     struct gdma_context *gc = mdev_to_gc(mdev);
> > > > >       int err;
> > > > >
> > > > > +     mdev->adapter_handle = INVALID_MANA_HANDLE;
> > > > > +
> > > > >       err = mana_ib_create_eqs(mdev);
> > > > >       if (err) {
> > > > >               ibdev_err(&mdev->ib_dev, "Failed to create EQs for
> > > > > RNIC err %d",
> > > > err);
> > > > >               goto cleanup;
> > > > >       }
> > > > >
> > > > > +     mana_gd_init_req_hdr(&req.hdr, MANA_IB_CREATE_ADAPTER,
> > > > sizeof(req), sizeof(resp));
> > > > > +     req.hdr.req.msg_version = GDMA_MESSAGE_V2;
> > > > > +     req.hdr.dev_id = gc->mana_ib.dev_id;
> > > > > +     req.notify_eq_id = mdev->fatal_err_eq->id;
> > > > > +
> > > > > +     err = mana_gd_send_request(gc, sizeof(req), &req, sizeof(resp),
> > &resp);
> > > > > +     if (err) {
> > > > > +             ibdev_err(&mdev->ib_dev, "Failed to create RNIC
> > > > > + adapter err %d",
> > > > err);
> > > > > +             goto cleanup;
> > > > > +     }
> > > > > +     mdev->adapter_handle = resp.adapter;
> > > > > +
> > > > >       return;
> > > > >
> > > > >  cleanup:
> > > > > @@ -599,5 +616,19 @@ void mana_ib_gd_create_rnic_adapter(struct
> > > > > mana_ib_dev *mdev)
> > > > >
> > > > >  void mana_ib_gd_destroy_rnic_adapter(struct mana_ib_dev *mdev)  {
> > > > > +     struct mana_rnic_destroy_adapter_resp resp = {};
> > > > > +     struct mana_rnic_destroy_adapter_req req = {};
> > > > > +     struct gdma_context *gc;
> > > > > +
> > > > > +     if (!rnic_is_enabled(mdev))
> > > > > +             return;
> > > > > +
> > > > > +     gc = mdev_to_gc(mdev);
> > > > > +     mana_gd_init_req_hdr(&req.hdr, MANA_IB_DESTROY_ADAPTER,
> > > > sizeof(req), sizeof(resp));
> > > > > +     req.hdr.dev_id = gc->mana_ib.dev_id;
> > > > > +     req.adapter = mdev->adapter_handle;
> > > > > +
> > > > > +     mana_gd_send_request(gc, sizeof(req), &req, sizeof(resp), &resp);
> > > > > +     mdev->adapter_handle = INVALID_MANA_HANDLE;
> > > > >       mana_ib_destroy_eqs(mdev);
> > > > >  }
> > > > > diff --git a/drivers/infiniband/hw/mana/mana_ib.h
> > > > > b/drivers/infiniband/hw/mana/mana_ib.h
> > > > > index a4b94ee..96454cf 100644
> > > > > --- a/drivers/infiniband/hw/mana/mana_ib.h
> > > > > +++ b/drivers/infiniband/hw/mana/mana_ib.h
> > > > > @@ -48,6 +48,7 @@ struct mana_ib_adapter_caps {  struct
> > mana_ib_dev {
> > > > >       struct ib_device ib_dev;
> > > > >       struct gdma_dev *gdma_dev;
> > > > > +     mana_handle_t adapter_handle;
> > > > >       struct gdma_queue *fatal_err_eq;
> > > > >       struct mana_ib_adapter_caps adapter_caps;  }; @@ -115,6
> > > > > +116,8 @@ struct mana_ib_rwq_ind_table {
> > > > >
> > > > >  enum mana_ib_command_code {
> > > > >       MANA_IB_GET_ADAPTER_CAP = 0x30001,
> > > > > +     MANA_IB_CREATE_ADAPTER  = 0x30002,
> > > > > +     MANA_IB_DESTROY_ADAPTER = 0x30003,
> > > > >  };
> > > > >
> > > > >  struct mana_ib_query_adapter_caps_req { @@ -143,6 +146,32 @@
> > > > > struct mana_ib_query_adapter_caps_resp {
> > > > >       u32 max_inline_data_size;
> > > > >  }; /* HW Data */
> > > > >
> > > > > +struct mana_rnic_create_adapter_req {
> > > > > +     struct gdma_req_hdr hdr;
> > > > > +     u32 notify_eq_id;
> > > > > +     u32 reserved;
> > > > > +     u64 feature_flags;
> > > > > +}; /*HW Data */
> > > > > +
> > > > > +struct mana_rnic_create_adapter_resp {
> > > > > +     struct gdma_resp_hdr hdr;
> > > > > +     mana_handle_t adapter;
> > > > > +}; /* HW Data */
> > > > > +
> > > > > +struct mana_rnic_destroy_adapter_req {
> > > > > +     struct gdma_req_hdr hdr;
> > > > > +     mana_handle_t adapter;
> > > > > +}; /*HW Data */
> > > > > +
> > > > > +struct mana_rnic_destroy_adapter_resp {
> > > > > +     struct gdma_resp_hdr hdr;
> > > > > +}; /* HW Data */
> > > > > +
> > > > > +static inline bool rnic_is_enabled(struct mana_ib_dev *mdev) {
> > > > > +     return mdev->adapter_handle != INVALID_MANA_HANDLE; }
> > > > > +
> > > > >  static inline struct gdma_context *mdev_to_gc(struct mana_ib_dev
> > > > > *mdev)  {
> > > > >       return mdev->gdma_dev->gdma_context;
> > > > > --
> > > > > 1.8.3.1
> > > > >

RE: [EXTERNAL] Re: [PATCH rdma-next v2 2/5] RDMA/mana_ib: Create and destroy rnic adapter

[Konstantin Taranov]

> From: Leon Romanovsky <leon@kernel.org>
> On Sun, Feb 04, 2024 at 05:17:59PM +0000, Konstantin Taranov wrote:
> > > From: Leon Romanovsky <leon@kernel.org> On Sun, Feb 04, 2024 at
> > > 03:50:40PM +0000, Konstantin Taranov wrote:
> > > > > From: Leon Romanovsky <leon@kernel.org> On Fri, Feb 02, 2024 at
> > > > > 07:06:34AM -0800, Konstantin Taranov wrote:
> > > > > > This patch adds RNIC creation and destruction.
> > > > > > If creation of RNIC fails, we support only RAW QPs as they are
> > > > > > served by ethernet driver.
> > > > >
> > > > > So please make sure that you are creating RNIC only when you are
> > > > > supporting it. The idea that some function tries-and-fails with
> > > > > dmesg errors is not good idea.
> > > > >
> > > > > Thanks
> > > > >
> > > >
> > > > Hi Leon. Thanks for your comments and suggestion. I will
> > > > incorporate them
> > > in the next version.
> > > > Regarding this "try-and-fail", we cannot guarantee now that RNIC
> > > > is supported, and try-and-fail is the only way to skip RNIC
> > > > creation without impeding RAW QPs. Could you, please, suggest how
> > > > we could
> > > correctly incorporate the "try-and-fail" strategy to get it upstreamed?
> > >
> > > You already query NIC for its capabilities, so you can check if it supports
> RNIC.
> >
> > At the moment, the capabilities do not indicate whether RNIC creation will
> be successful.
> > The reason is additional checks during RNIC creation that are not reflected
> in capabilities.
> > The question is whether we can have the proposed "try and disable" or we
> must opt for failing the whole mana_ib.
> 
> RNIC creation can be seen as an example of any other feature which will be
> added later, you will never know if it will be successful or not without
> capabilities.
> 
> If you continue with this try-and-fail approach, I afraid that you will end up
> with whole driver written in this style. Style where you don't separate
> between "real" failures (wrong configuration, OOM e.t.c) and "expected"
> failures (feature is not supported).
> 

Hi Leon. I understand your concerns and I see how try-and-fail approach can go wrong.
I think you misunderstood the current HW limitation we have. We *do* distinguish between 
failures and this " try-and-fail " will be used once during initialization. As I mentioned above,
our current HW capabilities cannot reflect whether RNIC is supported. Therefore, we must try
to create it to understand whether it is really supported. So, if we succeed then the RNIC feature
is supported and all RNIC-related operations will work. Otherwise, RNIC capability is not present
and in this case, we just wanted to warn the user about it. If it concerns you, I can remove this warn message. 

Given the provided explanation, I would appreciate if you wrote whether this approach of querying RNIC support
could be accepted. 

Thanks!

> Thanks
> 
> >
> > >
> > > >
> > > > > >
> > > > > > Signed-off-by: Konstantin Taranov
> > > > > > <kotaranov@linux.microsoft.com>
> > > > > > ---
> > > > > >  drivers/infiniband/hw/mana/main.c    | 31
> > > > > +++++++++++++++++++++++++++++++
> > > > > >  drivers/infiniband/hw/mana/mana_ib.h | 29
> > > > > > +++++++++++++++++++++++++++++
> > > > > >  2 files changed, 60 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/infiniband/hw/mana/main.c
> > > > > > b/drivers/infiniband/hw/mana/main.c
> > > > > > index c64d569..33cd69e 100644
> > > > > > --- a/drivers/infiniband/hw/mana/main.c
> > > > > > +++ b/drivers/infiniband/hw/mana/main.c
> > > > > > @@ -581,14 +581,31 @@ static void mana_ib_destroy_eqs(struct
> > > > > > mana_ib_dev *mdev)
> > > > > >
> > > > > >  void mana_ib_gd_create_rnic_adapter(struct mana_ib_dev *mdev)
> > > > > > {
> > > > > > +     struct mana_rnic_create_adapter_resp resp = {};
> > > > > > +     struct mana_rnic_create_adapter_req req = {};
> > > > > > +     struct gdma_context *gc = mdev_to_gc(mdev);
> > > > > >       int err;
> > > > > >
> > > > > > +     mdev->adapter_handle = INVALID_MANA_HANDLE;
> > > > > > +
> > > > > >       err = mana_ib_create_eqs(mdev);
> > > > > >       if (err) {
> > > > > >               ibdev_err(&mdev->ib_dev, "Failed to create EQs
> > > > > > for RNIC err %d",
> > > > > err);
> > > > > >               goto cleanup;
> > > > > >       }
> > > > > >
> > > > > > +     mana_gd_init_req_hdr(&req.hdr, MANA_IB_CREATE_ADAPTER,
> > > > > sizeof(req), sizeof(resp));
> > > > > > +     req.hdr.req.msg_version = GDMA_MESSAGE_V2;
> > > > > > +     req.hdr.dev_id = gc->mana_ib.dev_id;
> > > > > > +     req.notify_eq_id = mdev->fatal_err_eq->id;
> > > > > > +
> > > > > > +     err = mana_gd_send_request(gc, sizeof(req), &req,
> > > > > > + sizeof(resp),
> > > &resp);
> > > > > > +     if (err) {
> > > > > > +             ibdev_err(&mdev->ib_dev, "Failed to create RNIC
> > > > > > + adapter err %d",
> > > > > err);
> > > > > > +             goto cleanup;
> > > > > > +     }
> > > > > > +     mdev->adapter_handle = resp.adapter;
> > > > > > +
> > > > > >       return;
> > > > > >
> > > > > >  cleanup:
> > > > > > @@ -599,5 +616,19 @@ void
> > > > > > mana_ib_gd_create_rnic_adapter(struct
> > > > > > mana_ib_dev *mdev)
> > > > > >
> > > > > >  void mana_ib_gd_destroy_rnic_adapter(struct mana_ib_dev
> > > > > > *mdev)  {
> > > > > > +     struct mana_rnic_destroy_adapter_resp resp = {};
> > > > > > +     struct mana_rnic_destroy_adapter_req req = {};
> > > > > > +     struct gdma_context *gc;
> > > > > > +
> > > > > > +     if (!rnic_is_enabled(mdev))
> > > > > > +             return;
> > > > > > +
> > > > > > +     gc = mdev_to_gc(mdev);
> > > > > > +     mana_gd_init_req_hdr(&req.hdr,
> MANA_IB_DESTROY_ADAPTER,
> > > > > sizeof(req), sizeof(resp));
> > > > > > +     req.hdr.dev_id = gc->mana_ib.dev_id;
> > > > > > +     req.adapter = mdev->adapter_handle;
> > > > > > +
> > > > > > +     mana_gd_send_request(gc, sizeof(req), &req, sizeof(resp),
> &resp);
> > > > > > +     mdev->adapter_handle = INVALID_MANA_HANDLE;
> > > > > >       mana_ib_destroy_eqs(mdev);  } diff --git
> > > > > > a/drivers/infiniband/hw/mana/mana_ib.h
> > > > > > b/drivers/infiniband/hw/mana/mana_ib.h
> > > > > > index a4b94ee..96454cf 100644
> > > > > > --- a/drivers/infiniband/hw/mana/mana_ib.h
> > > > > > +++ b/drivers/infiniband/hw/mana/mana_ib.h
> > > > > > @@ -48,6 +48,7 @@ struct mana_ib_adapter_caps {  struct
> > > mana_ib_dev {
> > > > > >       struct ib_device ib_dev;
> > > > > >       struct gdma_dev *gdma_dev;
> > > > > > +     mana_handle_t adapter_handle;
> > > > > >       struct gdma_queue *fatal_err_eq;
> > > > > >       struct mana_ib_adapter_caps adapter_caps;  }; @@ -115,6
> > > > > > +116,8 @@ struct mana_ib_rwq_ind_table {
> > > > > >
> > > > > >  enum mana_ib_command_code {
> > > > > >       MANA_IB_GET_ADAPTER_CAP = 0x30001,
> > > > > > +     MANA_IB_CREATE_ADAPTER  = 0x30002,
> > > > > > +     MANA_IB_DESTROY_ADAPTER = 0x30003,
> > > > > >  };
> > > > > >
> > > > > >  struct mana_ib_query_adapter_caps_req { @@ -143,6 +146,32 @@
> > > > > > struct mana_ib_query_adapter_caps_resp {
> > > > > >       u32 max_inline_data_size;  }; /* HW Data */
> > > > > >
> > > > > > +struct mana_rnic_create_adapter_req {
> > > > > > +     struct gdma_req_hdr hdr;
> > > > > > +     u32 notify_eq_id;
> > > > > > +     u32 reserved;
> > > > > > +     u64 feature_flags;
> > > > > > +}; /*HW Data */
> > > > > > +
> > > > > > +struct mana_rnic_create_adapter_resp {
> > > > > > +     struct gdma_resp_hdr hdr;
> > > > > > +     mana_handle_t adapter;
> > > > > > +}; /* HW Data */
> > > > > > +
> > > > > > +struct mana_rnic_destroy_adapter_req {
> > > > > > +     struct gdma_req_hdr hdr;
> > > > > > +     mana_handle_t adapter;
> > > > > > +}; /*HW Data */
> > > > > > +
> > > > > > +struct mana_rnic_destroy_adapter_resp {
> > > > > > +     struct gdma_resp_hdr hdr; }; /* HW Data */
> > > > > > +
> > > > > > +static inline bool rnic_is_enabled(struct mana_ib_dev *mdev) {
> > > > > > +     return mdev->adapter_handle != INVALID_MANA_HANDLE; }
> > > > > > +
> > > > > >  static inline struct gdma_context *mdev_to_gc(struct
> > > > > > mana_ib_dev
> > > > > > *mdev)  {
> > > > > >       return mdev->gdma_dev->gdma_context;
> > > > > > --
> > > > > > 1.8.3.1
> > > > > >

Re: [EXTERNAL] Re: [PATCH rdma-next v2 2/5] RDMA/mana_ib: Create and destroy rnic adapter

[Leon Romanovsky]

On Mon, Feb 05, 2024 at 09:15:19AM +0000, Konstantin Taranov wrote:
> > From: Leon Romanovsky <leon@kernel.org>
> > On Sun, Feb 04, 2024 at 05:17:59PM +0000, Konstantin Taranov wrote:
> > > > From: Leon Romanovsky <leon@kernel.org> On Sun, Feb 04, 2024 at
> > > > 03:50:40PM +0000, Konstantin Taranov wrote:
> > > > > > From: Leon Romanovsky <leon@kernel.org> On Fri, Feb 02, 2024 at
> > > > > > 07:06:34AM -0800, Konstantin Taranov wrote:
> > > > > > > This patch adds RNIC creation and destruction.
> > > > > > > If creation of RNIC fails, we support only RAW QPs as they are
> > > > > > > served by ethernet driver.
> > > > > >
> > > > > > So please make sure that you are creating RNIC only when you are
> > > > > > supporting it. The idea that some function tries-and-fails with
> > > > > > dmesg errors is not good idea.
> > > > > >
> > > > > > Thanks
> > > > > >
> > > > >
> > > > > Hi Leon. Thanks for your comments and suggestion. I will
> > > > > incorporate them
> > > > in the next version.
> > > > > Regarding this "try-and-fail", we cannot guarantee now that RNIC
> > > > > is supported, and try-and-fail is the only way to skip RNIC
> > > > > creation without impeding RAW QPs. Could you, please, suggest how
> > > > > we could
> > > > correctly incorporate the "try-and-fail" strategy to get it upstreamed?
> > > >
> > > > You already query NIC for its capabilities, so you can check if it supports
> > RNIC.
> > >
> > > At the moment, the capabilities do not indicate whether RNIC creation will
> > be successful.
> > > The reason is additional checks during RNIC creation that are not reflected
> > in capabilities.
> > > The question is whether we can have the proposed "try and disable" or we
> > must opt for failing the whole mana_ib.
> > 
> > RNIC creation can be seen as an example of any other feature which will be
> > added later, you will never know if it will be successful or not without
> > capabilities.
> > 
> > If you continue with this try-and-fail approach, I afraid that you will end up
> > with whole driver written in this style. Style where you don't separate
> > between "real" failures (wrong configuration, OOM e.t.c) and "expected"
> > failures (feature is not supported).
> > 
> 
> Hi Leon. I understand your concerns and I see how try-and-fail approach can go wrong.
> I think you misunderstood the current HW limitation we have. We *do* distinguish between 
> failures 

This is not what the code is doing, you are ignoring real errors.
The distinguish is usually done by checking the return value of the function after looking
after specific error code returned by FW/HW.

> and this " try-and-fail " will be used once during initialization. As I mentioned above,
> our current HW capabilities cannot reflect whether RNIC is supported. Therefore, we must try
> to create it to understand whether it is really supported. So, if we succeed then the RNIC feature
> is supported and all RNIC-related operations will work. Otherwise, RNIC capability is not present
> and in this case, we just wanted to warn the user about it. If it concerns you, I can remove this warn message. 
> 
> Given the provided explanation, I would appreciate if you wrote whether this approach of querying RNIC support
> could be accepted. 

Unless you have a good explanation why you can add new FW command to configure RNIC, but can't add FW command
to query if RNIC is supported. I'm not keen on adopting this approach.

Thanks

> 
> Thanks!
> 
> > Thanks
> > 
> > >
> > > >
> > > > >
> > > > > > >
> > > > > > > Signed-off-by: Konstantin Taranov
> > > > > > > <kotaranov@linux.microsoft.com>
> > > > > > > ---
> > > > > > >  drivers/infiniband/hw/mana/main.c    | 31
> > > > > > +++++++++++++++++++++++++++++++
> > > > > > >  drivers/infiniband/hw/mana/mana_ib.h | 29
> > > > > > > +++++++++++++++++++++++++++++
> > > > > > >  2 files changed, 60 insertions(+)
> > > > > > >
> > > > > > > diff --git a/drivers/infiniband/hw/mana/main.c
> > > > > > > b/drivers/infiniband/hw/mana/main.c
> > > > > > > index c64d569..33cd69e 100644
> > > > > > > --- a/drivers/infiniband/hw/mana/main.c
> > > > > > > +++ b/drivers/infiniband/hw/mana/main.c
> > > > > > > @@ -581,14 +581,31 @@ static void mana_ib_destroy_eqs(struct
> > > > > > > mana_ib_dev *mdev)
> > > > > > >
> > > > > > >  void mana_ib_gd_create_rnic_adapter(struct mana_ib_dev *mdev)
> > > > > > > {
> > > > > > > +     struct mana_rnic_create_adapter_resp resp = {};
> > > > > > > +     struct mana_rnic_create_adapter_req req = {};
> > > > > > > +     struct gdma_context *gc = mdev_to_gc(mdev);
> > > > > > >       int err;
> > > > > > >
> > > > > > > +     mdev->adapter_handle = INVALID_MANA_HANDLE;
> > > > > > > +
> > > > > > >       err = mana_ib_create_eqs(mdev);
> > > > > > >       if (err) {
> > > > > > >               ibdev_err(&mdev->ib_dev, "Failed to create EQs
> > > > > > > for RNIC err %d",
> > > > > > err);
> > > > > > >               goto cleanup;
> > > > > > >       }
> > > > > > >
> > > > > > > +     mana_gd_init_req_hdr(&req.hdr, MANA_IB_CREATE_ADAPTER,
> > > > > > sizeof(req), sizeof(resp));
> > > > > > > +     req.hdr.req.msg_version = GDMA_MESSAGE_V2;
> > > > > > > +     req.hdr.dev_id = gc->mana_ib.dev_id;
> > > > > > > +     req.notify_eq_id = mdev->fatal_err_eq->id;
> > > > > > > +
> > > > > > > +     err = mana_gd_send_request(gc, sizeof(req), &req,
> > > > > > > + sizeof(resp),
> > > > &resp);
> > > > > > > +     if (err) {
> > > > > > > +             ibdev_err(&mdev->ib_dev, "Failed to create RNIC
> > > > > > > + adapter err %d",
> > > > > > err);
> > > > > > > +             goto cleanup;
> > > > > > > +     }
> > > > > > > +     mdev->adapter_handle = resp.adapter;
> > > > > > > +
> > > > > > >       return;
> > > > > > >
> > > > > > >  cleanup:
> > > > > > > @@ -599,5 +616,19 @@ void
> > > > > > > mana_ib_gd_create_rnic_adapter(struct
> > > > > > > mana_ib_dev *mdev)
> > > > > > >
> > > > > > >  void mana_ib_gd_destroy_rnic_adapter(struct mana_ib_dev
> > > > > > > *mdev)  {
> > > > > > > +     struct mana_rnic_destroy_adapter_resp resp = {};
> > > > > > > +     struct mana_rnic_destroy_adapter_req req = {};
> > > > > > > +     struct gdma_context *gc;
> > > > > > > +
> > > > > > > +     if (!rnic_is_enabled(mdev))
> > > > > > > +             return;
> > > > > > > +
> > > > > > > +     gc = mdev_to_gc(mdev);
> > > > > > > +     mana_gd_init_req_hdr(&req.hdr,
> > MANA_IB_DESTROY_ADAPTER,
> > > > > > sizeof(req), sizeof(resp));
> > > > > > > +     req.hdr.dev_id = gc->mana_ib.dev_id;
> > > > > > > +     req.adapter = mdev->adapter_handle;
> > > > > > > +
> > > > > > > +     mana_gd_send_request(gc, sizeof(req), &req, sizeof(resp),
> > &resp);
> > > > > > > +     mdev->adapter_handle = INVALID_MANA_HANDLE;
> > > > > > >       mana_ib_destroy_eqs(mdev);  } diff --git
> > > > > > > a/drivers/infiniband/hw/mana/mana_ib.h
> > > > > > > b/drivers/infiniband/hw/mana/mana_ib.h
> > > > > > > index a4b94ee..96454cf 100644
> > > > > > > --- a/drivers/infiniband/hw/mana/mana_ib.h
> > > > > > > +++ b/drivers/infiniband/hw/mana/mana_ib.h
> > > > > > > @@ -48,6 +48,7 @@ struct mana_ib_adapter_caps {  struct
> > > > mana_ib_dev {
> > > > > > >       struct ib_device ib_dev;
> > > > > > >       struct gdma_dev *gdma_dev;
> > > > > > > +     mana_handle_t adapter_handle;
> > > > > > >       struct gdma_queue *fatal_err_eq;
> > > > > > >       struct mana_ib_adapter_caps adapter_caps;  }; @@ -115,6
> > > > > > > +116,8 @@ struct mana_ib_rwq_ind_table {
> > > > > > >
> > > > > > >  enum mana_ib_command_code {
> > > > > > >       MANA_IB_GET_ADAPTER_CAP = 0x30001,
> > > > > > > +     MANA_IB_CREATE_ADAPTER  = 0x30002,
> > > > > > > +     MANA_IB_DESTROY_ADAPTER = 0x30003,
> > > > > > >  };
> > > > > > >
> > > > > > >  struct mana_ib_query_adapter_caps_req { @@ -143,6 +146,32 @@
> > > > > > > struct mana_ib_query_adapter_caps_resp {
> > > > > > >       u32 max_inline_data_size;  }; /* HW Data */
> > > > > > >
> > > > > > > +struct mana_rnic_create_adapter_req {
> > > > > > > +     struct gdma_req_hdr hdr;
> > > > > > > +     u32 notify_eq_id;
> > > > > > > +     u32 reserved;
> > > > > > > +     u64 feature_flags;
> > > > > > > +}; /*HW Data */
> > > > > > > +
> > > > > > > +struct mana_rnic_create_adapter_resp {
> > > > > > > +     struct gdma_resp_hdr hdr;
> > > > > > > +     mana_handle_t adapter;
> > > > > > > +}; /* HW Data */
> > > > > > > +
> > > > > > > +struct mana_rnic_destroy_adapter_req {
> > > > > > > +     struct gdma_req_hdr hdr;
> > > > > > > +     mana_handle_t adapter;
> > > > > > > +}; /*HW Data */
> > > > > > > +
> > > > > > > +struct mana_rnic_destroy_adapter_resp {
> > > > > > > +     struct gdma_resp_hdr hdr; }; /* HW Data */
> > > > > > > +
> > > > > > > +static inline bool rnic_is_enabled(struct mana_ib_dev *mdev) {
> > > > > > > +     return mdev->adapter_handle != INVALID_MANA_HANDLE; }
> > > > > > > +
> > > > > > >  static inline struct gdma_context *mdev_to_gc(struct
> > > > > > > mana_ib_dev
> > > > > > > *mdev)  {
> > > > > > >       return mdev->gdma_dev->gdma_context;
> > > > > > > --
> > > > > > > 1.8.3.1
> > > > > > >

Re: [PATCH rdma-next v2 2/5] RDMA/mana_ib: Create and destroy rnic adapter

[Konstantin Taranov]

> From: Leon Romanovsky <leon@kernel.org>
> Sent: Monday, 5 February 2024 10:57
> To: Konstantin Taranov <kotaranov@microsoft.com>
> Cc: Konstantin Taranov <kotaranov@linux.microsoft.com>;
> sharmaajay@microsoft.com; Long Li <longli@microsoft.com>; jgg@ziepe.ca;
> linux-rdma@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [EXTERNAL] Re: [PATCH rdma-next v2 2/5] RDMA/mana_ib:
> Create and destroy rnic adapter
> 
> [You don't often get email from leon@kernel.org. Learn why this is important
> at https://aka.ms/LearnAboutSenderIdentification ]
> 
> On Mon, Feb 05, 2024 at 09:15:19AM +0000, Konstantin Taranov wrote:
> > > From: Leon Romanovsky <leon@kernel.org> On Sun, Feb 04, 2024 at
> > > 05:17:59PM +0000, Konstantin Taranov wrote:
> > > > > From: Leon Romanovsky <leon@kernel.org> On Sun, Feb 04, 2024 at
> > > > > 03:50:40PM +0000, Konstantin Taranov wrote:
> > > > > > > From: Leon Romanovsky <leon@kernel.org> On Fri, Feb 02, 2024
> > > > > > > at 07:06:34AM -0800, Konstantin Taranov wrote:
> > > > > > > > This patch adds RNIC creation and destruction.
> > > > > > > > If creation of RNIC fails, we support only RAW QPs as they
> > > > > > > > are served by ethernet driver.
> > > > > > >
> > > > > > > So please make sure that you are creating RNIC only when you
> > > > > > > are supporting it. The idea that some function
> > > > > > > tries-and-fails with dmesg errors is not good idea.
> > > > > > >
> > > > > > > Thanks
> > > > > > >
> > > > > >
> > > > > > Hi Leon. Thanks for your comments and suggestion. I will
> > > > > > incorporate them
> > > > > in the next version.
> > > > > > Regarding this "try-and-fail", we cannot guarantee now that
> > > > > > RNIC is supported, and try-and-fail is the only way to skip
> > > > > > RNIC creation without impeding RAW QPs. Could you, please,
> > > > > > suggest how we could
> > > > > correctly incorporate the "try-and-fail" strategy to get it upstreamed?
> > > > >
> > > > > You already query NIC for its capabilities, so you can check if
> > > > > it supports
> > > RNIC.
> > > >
> > > > At the moment, the capabilities do not indicate whether RNIC
> > > > creation will
> > > be successful.
> > > > The reason is additional checks during RNIC creation that are not
> > > > reflected
> > > in capabilities.
> > > > The question is whether we can have the proposed "try and disable"
> > > > or we
> > > must opt for failing the whole mana_ib.
> > >
> > > RNIC creation can be seen as an example of any other feature which
> > > will be added later, you will never know if it will be successful or
> > > not without capabilities.
> > >
> > > If you continue with this try-and-fail approach, I afraid that you
> > > will end up with whole driver written in this style. Style where you
> > > don't separate between "real" failures (wrong configuration, OOM e.t.c)
> and "expected"
> > > failures (feature is not supported).
> > >
> >
> > Hi Leon. I understand your concerns and I see how try-and-fail approach can
> go wrong.
> > I think you misunderstood the current HW limitation we have. We *do*
> > distinguish between failures
> 
> This is not what the code is doing, you are ignoring real errors.
> The distinguish is usually done by checking the return value of the function
> after looking after specific error code returned by FW/HW.
> 
> > and this " try-and-fail " will be used once during initialization. As
> > I mentioned above, our current HW capabilities cannot reflect whether
> > RNIC is supported. Therefore, we must try to create it to understand
> > whether it is really supported. So, if we succeed then the RNIC
> > feature is supported and all RNIC-related operations will work. Otherwise,
> RNIC capability is not present and in this case, we just wanted to warn the
> user about it. If it concerns you, I can remove this warn message.
> >
> > Given the provided explanation, I would appreciate if you wrote
> > whether this approach of querying RNIC support could be accepted.
> 
> Unless you have a good explanation why you can add new FW command to
> configure RNIC, but can't add FW command to query if RNIC is supported. I'm
> not keen on adopting this approach.
> 

The main reason was backward compatibility with old firmware that had the
aforementioned limitation. Anyway, we will try to internally retire the old firmware
and will send the v3 patches without the "try and fail" approach (in 2-3 weeks).

Thanks.

> Thanks
> 
> >
> > Thanks!
> >
> > > Thanks
> > >
> > > >
> > > > >
> > > > > >
> > > > > > > >
> > > > > > > > Signed-off-by: Konstantin Taranov
> > > > > > > > <kotaranov@linux.microsoft.com>
> > > > > > > > ---
> > > > > > > >  drivers/infiniband/hw/mana/main.c    | 31
> > > > > > > +++++++++++++++++++++++++++++++
> > > > > > > >  drivers/infiniband/hw/mana/mana_ib.h | 29
> > > > > > > > +++++++++++++++++++++++++++++
> > > > > > > >  2 files changed, 60 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/drivers/infiniband/hw/mana/main.c
> > > > > > > > b/drivers/infiniband/hw/mana/main.c
> > > > > > > > index c64d569..33cd69e 100644
> > > > > > > > --- a/drivers/infiniband/hw/mana/main.c
> > > > > > > > +++ b/drivers/infiniband/hw/mana/main.c
> > > > > > > > @@ -581,14 +581,31 @@ static void
> > > > > > > > mana_ib_destroy_eqs(struct mana_ib_dev *mdev)
> > > > > > > >
> > > > > > > >  void mana_ib_gd_create_rnic_adapter(struct mana_ib_dev
> > > > > > > > *mdev) {
> > > > > > > > +     struct mana_rnic_create_adapter_resp resp = {};
> > > > > > > > +     struct mana_rnic_create_adapter_req req = {};
> > > > > > > > +     struct gdma_context *gc = mdev_to_gc(mdev);
> > > > > > > >       int err;
> > > > > > > >
> > > > > > > > +     mdev->adapter_handle = INVALID_MANA_HANDLE;
> > > > > > > > +
> > > > > > > >       err = mana_ib_create_eqs(mdev);
> > > > > > > >       if (err) {
> > > > > > > >               ibdev_err(&mdev->ib_dev, "Failed to create
> > > > > > > > EQs for RNIC err %d",
> > > > > > > err);
> > > > > > > >               goto cleanup;
> > > > > > > >       }
> > > > > > > >
> > > > > > > > +     mana_gd_init_req_hdr(&req.hdr,
> > > > > > > > + MANA_IB_CREATE_ADAPTER,
> > > > > > > sizeof(req), sizeof(resp));
> > > > > > > > +     req.hdr.req.msg_version = GDMA_MESSAGE_V2;
> > > > > > > > +     req.hdr.dev_id = gc->mana_ib.dev_id;
> > > > > > > > +     req.notify_eq_id = mdev->fatal_err_eq->id;
> > > > > > > > +
> > > > > > > > +     err = mana_gd_send_request(gc, sizeof(req), &req,
> > > > > > > > + sizeof(resp),
> > > > > &resp);
> > > > > > > > +     if (err) {
> > > > > > > > +             ibdev_err(&mdev->ib_dev, "Failed to create
> > > > > > > > + RNIC adapter err %d",
> > > > > > > err);
> > > > > > > > +             goto cleanup;
> > > > > > > > +     }
> > > > > > > > +     mdev->adapter_handle = resp.adapter;
> > > > > > > > +
> > > > > > > >       return;
> > > > > > > >
> > > > > > > >  cleanup:
> > > > > > > > @@ -599,5 +616,19 @@ void
> > > > > > > > mana_ib_gd_create_rnic_adapter(struct
> > > > > > > > mana_ib_dev *mdev)
> > > > > > > >
> > > > > > > >  void mana_ib_gd_destroy_rnic_adapter(struct mana_ib_dev
> > > > > > > > *mdev)  {
> > > > > > > > +     struct mana_rnic_destroy_adapter_resp resp = {};
> > > > > > > > +     struct mana_rnic_destroy_adapter_req req = {};
> > > > > > > > +     struct gdma_context *gc;
> > > > > > > > +
> > > > > > > > +     if (!rnic_is_enabled(mdev))
> > > > > > > > +             return;
> > > > > > > > +
> > > > > > > > +     gc = mdev_to_gc(mdev);
> > > > > > > > +     mana_gd_init_req_hdr(&req.hdr,
> > > MANA_IB_DESTROY_ADAPTER,
> > > > > > > sizeof(req), sizeof(resp));
> > > > > > > > +     req.hdr.dev_id = gc->mana_ib.dev_id;
> > > > > > > > +     req.adapter = mdev->adapter_handle;
> > > > > > > > +
> > > > > > > > +     mana_gd_send_request(gc, sizeof(req), &req,
> > > > > > > > + sizeof(resp),
> > > &resp);
> > > > > > > > +     mdev->adapter_handle = INVALID_MANA_HANDLE;
> > > > > > > >       mana_ib_destroy_eqs(mdev);  } diff --git
> > > > > > > > a/drivers/infiniband/hw/mana/mana_ib.h
> > > > > > > > b/drivers/infiniband/hw/mana/mana_ib.h
> > > > > > > > index a4b94ee..96454cf 100644
> > > > > > > > --- a/drivers/infiniband/hw/mana/mana_ib.h
> > > > > > > > +++ b/drivers/infiniband/hw/mana/mana_ib.h
> > > > > > > > @@ -48,6 +48,7 @@ struct mana_ib_adapter_caps {  struct
> > > > > mana_ib_dev {
> > > > > > > >       struct ib_device ib_dev;
> > > > > > > >       struct gdma_dev *gdma_dev;
> > > > > > > > +     mana_handle_t adapter_handle;
> > > > > > > >       struct gdma_queue *fatal_err_eq;
> > > > > > > >       struct mana_ib_adapter_caps adapter_caps;  }; @@
> > > > > > > > -115,6
> > > > > > > > +116,8 @@ struct mana_ib_rwq_ind_table {
> > > > > > > >
> > > > > > > >  enum mana_ib_command_code {
> > > > > > > >       MANA_IB_GET_ADAPTER_CAP = 0x30001,
> > > > > > > > +     MANA_IB_CREATE_ADAPTER  = 0x30002,
> > > > > > > > +     MANA_IB_DESTROY_ADAPTER = 0x30003,
> > > > > > > >  };
> > > > > > > >
> > > > > > > >  struct mana_ib_query_adapter_caps_req { @@ -143,6 +146,32
> > > > > > > > @@ struct mana_ib_query_adapter_caps_resp {
> > > > > > > >       u32 max_inline_data_size;  }; /* HW Data */
> > > > > > > >
> > > > > > > > +struct mana_rnic_create_adapter_req {
> > > > > > > > +     struct gdma_req_hdr hdr;
> > > > > > > > +     u32 notify_eq_id;
> > > > > > > > +     u32 reserved;
> > > > > > > > +     u64 feature_flags;
> > > > > > > > +}; /*HW Data */
> > > > > > > > +
> > > > > > > > +struct mana_rnic_create_adapter_resp {
> > > > > > > > +     struct gdma_resp_hdr hdr;
> > > > > > > > +     mana_handle_t adapter; }; /* HW Data */
> > > > > > > > +
> > > > > > > > +struct mana_rnic_destroy_adapter_req {
> > > > > > > > +     struct gdma_req_hdr hdr;
> > > > > > > > +     mana_handle_t adapter; }; /*HW Data */
> > > > > > > > +
> > > > > > > > +struct mana_rnic_destroy_adapter_resp {
> > > > > > > > +     struct gdma_resp_hdr hdr; }; /* HW Data */
> > > > > > > > +
> > > > > > > > +static inline bool rnic_is_enabled(struct mana_ib_dev *mdev) {
> > > > > > > > +     return mdev->adapter_handle != INVALID_MANA_HANDLE;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > >  static inline struct gdma_context *mdev_to_gc(struct
> > > > > > > > mana_ib_dev
> > > > > > > > *mdev)  {
> > > > > > > >       return mdev->gdma_dev->gdma_context;
> > > > > > > > --
> > > > > > > > 1.8.3.1
> > > > > > > >

Re: [PATCH rdma-next v2 2/5] RDMA/mana_ib: Create and destroy rnic adapter

[Jason Gunthorpe]

On Tue, Feb 06, 2024 at 02:20:35PM +0000, Konstantin Taranov wrote:

> > Unless you have a good explanation why you can add new FW command to
> > configure RNIC, but can't add FW command to query if RNIC is supported. I'm
> > not keen on adopting this approach.
> 
> The main reason was backward compatibility with old firmware that had the
> aforementioned limitation. Anyway, we will try to internally retire the old firmware
> and will send the v3 patches without the "try and fail" approach (in 2-3 weeks).

I think this is the right thing to do for these cloud devices that can
reliably retire old software. It is how Amazon has been running
EFA. Get your deployment in good shape and then get patches comitted
upstream. No reason to suffer with backwards compatability forever in
the software to save a few weeks.

Jason