* [PATCH] i40iw: Convert page_size to encoded value
From: Henry Orosco @ 2016-11-10 3:25 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Henry Orosco
Passed in page_size was used as encoded value for writing
the WQE and passed in value was usually 4096. This was
working out since bit 0 was 0 and implies 4KB pages,
but would not work for other page sizes.
Signed-off-by: Mustafa Ismail <mustafa.ismail-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Henry Orosco <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/infiniband/hw/i40iw/i40iw_ctrl.c | 12 +++++++++---
drivers/infiniband/hw/i40iw/i40iw_type.h | 5 +++++
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/infiniband/hw/i40iw/i40iw_ctrl.c b/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
index cd71444..bdb4421 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
@@ -2613,7 +2613,9 @@ static enum i40iw_status_code i40iw_sc_alloc_stag(
u64 *wqe;
struct i40iw_sc_cqp *cqp;
u64 header;
+ enum i40iw_page_size page_size;
+ page_size = (info->page_size == 0x200000) ? I40IW_PAGE_SIZE_2M : I40IW_PAGE_SIZE_4K;
cqp = dev->cqp;
wqe = i40iw_sc_cqp_get_next_send_wqe(cqp, scratch);
if (!wqe)
@@ -2633,7 +2635,7 @@ static enum i40iw_status_code i40iw_sc_alloc_stag(
LS_64(1, I40IW_CQPSQ_STAG_MR) |
LS_64(info->access_rights, I40IW_CQPSQ_STAG_ARIGHTS) |
LS_64(info->chunk_size, I40IW_CQPSQ_STAG_LPBLSIZE) |
- LS_64(info->page_size, I40IW_CQPSQ_STAG_HPAGESIZE) |
+ LS_64(page_size, I40IW_CQPSQ_STAG_HPAGESIZE) |
LS_64(info->remote_access, I40IW_CQPSQ_STAG_REMACCENABLED) |
LS_64(info->use_hmc_fcn_index, I40IW_CQPSQ_STAG_USEHMCFNIDX) |
LS_64(info->use_pf_rid, I40IW_CQPSQ_STAG_USEPFRID) |
@@ -2669,7 +2671,9 @@ static enum i40iw_status_code i40iw_sc_mr_reg_non_shared(
u32 pble_obj_cnt;
bool remote_access;
u8 addr_type;
+ enum i40iw_page_size page_size;
+ page_size = (info->page_size == 0x200000) ? I40IW_PAGE_SIZE_2M : I40IW_PAGE_SIZE_4K;
if (info->access_rights & (I40IW_ACCESS_FLAGS_REMOTEREAD_ONLY |
I40IW_ACCESS_FLAGS_REMOTEWRITE_ONLY))
remote_access = true;
@@ -2712,7 +2716,7 @@ static enum i40iw_status_code i40iw_sc_mr_reg_non_shared(
header = LS_64(I40IW_CQP_OP_REG_MR, I40IW_CQPSQ_OPCODE) |
LS_64(1, I40IW_CQPSQ_STAG_MR) |
LS_64(info->chunk_size, I40IW_CQPSQ_STAG_LPBLSIZE) |
- LS_64(info->page_size, I40IW_CQPSQ_STAG_HPAGESIZE) |
+ LS_64(page_size, I40IW_CQPSQ_STAG_HPAGESIZE) |
LS_64(info->access_rights, I40IW_CQPSQ_STAG_ARIGHTS) |
LS_64(remote_access, I40IW_CQPSQ_STAG_REMACCENABLED) |
LS_64(addr_type, I40IW_CQPSQ_STAG_VABASEDTO) |
@@ -2927,7 +2931,9 @@ enum i40iw_status_code i40iw_sc_mr_fast_register(
u64 temp, header;
u64 *wqe;
u32 wqe_idx;
+ enum i40iw_page_size page_size;
+ page_size = (info->page_size == 0x200000) ? I40IW_PAGE_SIZE_2M : I40IW_PAGE_SIZE_4K;
wqe = i40iw_qp_get_next_send_wqe(&qp->qp_uk, &wqe_idx, I40IW_QP_WQE_MIN_SIZE,
0, info->wr_id);
if (!wqe)
@@ -2954,7 +2960,7 @@ enum i40iw_status_code i40iw_sc_mr_fast_register(
LS_64(info->stag_idx, I40IWQPSQ_STAGINDEX) |
LS_64(I40IWQP_OP_FAST_REGISTER, I40IWQPSQ_OPCODE) |
LS_64(info->chunk_size, I40IWQPSQ_LPBLSIZE) |
- LS_64(info->page_size, I40IWQPSQ_HPAGESIZE) |
+ LS_64(page_size, I40IWQPSQ_HPAGESIZE) |
LS_64(info->access_rights, I40IWQPSQ_STAGRIGHTS) |
LS_64(info->addr_type, I40IWQPSQ_VABASEDTO) |
LS_64(info->read_fence, I40IWQPSQ_READFENCE) |
diff --git a/drivers/infiniband/hw/i40iw/i40iw_type.h b/drivers/infiniband/hw/i40iw/i40iw_type.h
index f60f0e2..1c58ba6 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_type.h
+++ b/drivers/infiniband/hw/i40iw/i40iw_type.h
@@ -74,6 +74,11 @@ struct i40iw_cq_shadow_area {
struct i40iw_priv_cq_ops;
struct i40iw_hmc_ops;
+enum i40iw_page_size {
+ I40IW_PAGE_SIZE_4K = 0,
+ I40IW_PAGE_SIZE_2M
+};
+
enum i40iw_resource_indicator_type {
I40IW_RSRC_INDICATOR_TYPE_ADAPTER = 0,
I40IW_RSRC_INDICATOR_TYPE_CQ,
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH] i40iw: Correct values for max_recv_sge, max_send_sge
From: Henry Orosco @ 2016-11-10 3:26 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Henry Orosco
When creating QPs, ensure init_attr->cap.max_recv_sge
is clipped to MAX_FRAG_COUNT.
Expose MAX_FRAG_COUNT for max_recv_sge and max_send_sge in
i40iw_query_qp().
Signed-off-by: Shiraz Saleem <shiraz.saleem-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Henry Orosco <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/infiniband/hw/i40iw/i40iw_verbs.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/i40iw/i40iw_verbs.c b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
index 6329c97..d06fe593 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_verbs.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
@@ -610,6 +610,9 @@ static struct ib_qp *i40iw_create_qp(struct ib_pd *ibpd,
if (init_attr->cap.max_send_sge > I40IW_MAX_WQ_FRAGMENT_COUNT)
init_attr->cap.max_send_sge = I40IW_MAX_WQ_FRAGMENT_COUNT;
+ if (init_attr->cap.max_recv_sge > I40IW_MAX_WQ_FRAGMENT_COUNT)
+ init_attr->cap.max_recv_sge = I40IW_MAX_WQ_FRAGMENT_COUNT;
+
memset(&init_info, 0, sizeof(init_info));
sq_size = init_attr->cap.max_send_wr;
@@ -815,8 +818,9 @@ static int i40iw_query_qp(struct ib_qp *ibqp,
attr->qp_access_flags = 0;
attr->cap.max_send_wr = qp->qp_uk.sq_size;
attr->cap.max_recv_wr = qp->qp_uk.rq_size;
- attr->cap.max_recv_sge = 1;
attr->cap.max_inline_data = I40IW_MAX_INLINE_DATA_SIZE;
+ attr->cap.max_send_sge = I40IW_MAX_WQ_FRAGMENT_COUNT;
+ attr->cap.max_recv_sge = I40IW_MAX_WQ_FRAGMENT_COUNT;
init_attr->event_handler = iwqp->ibqp.event_handler;
init_attr->qp_context = iwqp->ibqp.qp_context;
init_attr->send_cq = iwqp->ibqp.send_cq;
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH] i40iw: Fix for LAN handler removal
From: Henry Orosco @ 2016-11-10 3:27 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Henry Orosco
If i40iw_open() fails for any reason, the LAN handler
is not being removed. Modify i40iw_deinit_device()
to always remove the handler.
Signed-off-by: Mustafa Ismail <mustafa.ismail-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Henry Orosco <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/infiniband/hw/i40iw/i40iw_main.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/infiniband/hw/i40iw/i40iw_main.c b/drivers/infiniband/hw/i40iw/i40iw_main.c
index 078d784..79808cc 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_main.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_main.c
@@ -1404,12 +1404,11 @@ static enum i40iw_status_code i40iw_save_msix_info(struct i40iw_device *iwdev,
* i40iw_deinit_device - clean up the device resources
* @iwdev: iwarp device
* @reset: true if called before reset
- * @del_hdl: true if delete hdl entry
*
* Destroy the ib device interface, remove the mac ip entry and ipv4/ipv6 addresses,
* destroy the device queues and free the pble and the hmc objects
*/
-static void i40iw_deinit_device(struct i40iw_device *iwdev, bool reset, bool del_hdl)
+static void i40iw_deinit_device(struct i40iw_device *iwdev, bool reset)
{
struct i40e_info *ldev = iwdev->ldev;
@@ -1472,8 +1471,7 @@ static void i40iw_deinit_device(struct i40iw_device *iwdev, bool reset, bool del
break;
}
- if (del_hdl)
- i40iw_del_handler(i40iw_find_i40e_handler(ldev));
+ i40iw_del_handler(i40iw_find_i40e_handler(ldev));
kfree(iwdev->hdl);
}
@@ -1635,7 +1633,7 @@ static int i40iw_open(struct i40e_info *ldev, struct i40e_client *client)
} while (0);
i40iw_pr_err("status = %d last completion = %d\n", status, iwdev->init_state);
- i40iw_deinit_device(iwdev, false, false);
+ i40iw_deinit_device(iwdev, false);
return -ERESTART;
}
@@ -1680,7 +1678,7 @@ static void i40iw_close(struct i40e_info *ldev, struct i40e_client *client, bool
iwdev = &hdl->device;
destroy_workqueue(iwdev->virtchnl_wq);
- i40iw_deinit_device(iwdev, reset, true);
+ i40iw_deinit_device(iwdev, reset);
}
/**
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH] i40iw: Optimize inline data copy
From: Henry Orosco @ 2016-11-10 3:28 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Henry Orosco
Use memcpy for inline data copy in sends
and writes instead of byte by byte copy.
Signed-off-by: Mustafa Ismail <mustafa.ismail-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Henry Orosco <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/infiniband/hw/i40iw/i40iw_uk.c | 24 ++++++++++--------------
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/drivers/infiniband/hw/i40iw/i40iw_uk.c b/drivers/infiniband/hw/i40iw/i40iw_uk.c
index 4d28c3c..47cb2e0 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_uk.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_uk.c
@@ -430,7 +430,7 @@ static enum i40iw_status_code i40iw_inline_rdma_write(struct i40iw_qp_uk *qp,
struct i40iw_inline_rdma_write *op_info;
u64 *push;
u64 header = 0;
- u32 i, wqe_idx;
+ u32 wqe_idx;
enum i40iw_status_code ret_code;
bool read_fence = false;
u8 wqe_size;
@@ -465,14 +465,12 @@ static enum i40iw_status_code i40iw_inline_rdma_write(struct i40iw_qp_uk *qp,
src = (u8 *)(op_info->data);
if (op_info->len <= 16) {
- for (i = 0; i < op_info->len; i++, src++, dest++)
- *dest = *src;
+ memcpy(dest, src, op_info->len);
} else {
- for (i = 0; i < 16; i++, src++, dest++)
- *dest = *src;
+ memcpy(dest, src, 16);
+ src += 16;
dest = (u8 *)wqe + 32;
- for (; i < op_info->len; i++, src++, dest++)
- *dest = *src;
+ memcpy(dest, src, op_info->len - 16);
}
wmb(); /* make sure WQE is populated before valid bit is set */
@@ -507,7 +505,7 @@ static enum i40iw_status_code i40iw_inline_send(struct i40iw_qp_uk *qp,
u8 *dest, *src;
struct i40iw_post_inline_send *op_info;
u64 header;
- u32 wqe_idx, i;
+ u32 wqe_idx;
enum i40iw_status_code ret_code;
bool read_fence = false;
u8 wqe_size;
@@ -540,14 +538,12 @@ static enum i40iw_status_code i40iw_inline_send(struct i40iw_qp_uk *qp,
src = (u8 *)(op_info->data);
if (op_info->len <= 16) {
- for (i = 0; i < op_info->len; i++, src++, dest++)
- *dest = *src;
+ memcpy(dest, src, op_info->len);
} else {
- for (i = 0; i < 16; i++, src++, dest++)
- *dest = *src;
+ memcpy(dest, src, 16);
+ src += 16;
dest = (u8 *)wqe + 32;
- for (; i < op_info->len; i++, src++, dest++)
- *dest = *src;
+ memcpy(dest, src, op_info->len - 16);
}
wmb(); /* make sure WQE is populated before valid bit is set */
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH] i40iw: Query device accounts for internal rsrc
From: Henry Orosco @ 2016-11-10 3:30 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Henry Orosco
Some resources are consumed internally and not available to the user.
After hw is initialized, figure out how many resources are consumed
and subtract those numbers from the initial max device capability in
i40iw_query_device().
Signed-off-by: Henry Orosco <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/infiniband/hw/i40iw/i40iw.h | 4 ++++
drivers/infiniband/hw/i40iw/i40iw_hw.c | 2 +-
drivers/infiniband/hw/i40iw/i40iw_main.c | 15 +++++++++++++++
drivers/infiniband/hw/i40iw/i40iw_user.h | 3 ++-
drivers/infiniband/hw/i40iw/i40iw_verbs.c | 8 ++++----
5 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/drivers/infiniband/hw/i40iw/i40iw.h b/drivers/infiniband/hw/i40iw/i40iw.h
index 8ec09e4..22af66a 100644
--- a/drivers/infiniband/hw/i40iw/i40iw.h
+++ b/drivers/infiniband/hw/i40iw/i40iw.h
@@ -297,6 +297,10 @@ struct i40iw_device {
u32 mr_stagmask;
u32 mpa_version;
bool dcb;
+ u32 used_pds;
+ u32 used_cqs;
+ u32 used_mrs;
+ u32 used_qps;
};
struct i40iw_ib_device {
diff --git a/drivers/infiniband/hw/i40iw/i40iw_hw.c b/drivers/infiniband/hw/i40iw/i40iw_hw.c
index 0c92a40..886b8c8 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_hw.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_hw.c
@@ -62,7 +62,7 @@ u32 i40iw_initialize_hw_resources(struct i40iw_device *iwdev)
max_mr = iwdev->sc_dev.hmc_info->hmc_obj[I40IW_HMC_IW_MR].cnt;
arp_table_size = iwdev->sc_dev.hmc_info->hmc_obj[I40IW_HMC_IW_ARP].cnt;
iwdev->max_cqe = 0xFFFFF;
- num_pds = max_qp * 4;
+ num_pds = I40IW_MAX_PDS;
resources_size = sizeof(struct i40iw_arp_entry) * arp_table_size;
resources_size += sizeof(unsigned long) * BITS_TO_LONGS(max_qp);
resources_size += sizeof(unsigned long) * BITS_TO_LONGS(max_mr);
diff --git a/drivers/infiniband/hw/i40iw/i40iw_main.c b/drivers/infiniband/hw/i40iw/i40iw_main.c
index 078d784..ba45b16 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_main.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_main.c
@@ -1540,6 +1540,20 @@ static enum i40iw_status_code i40iw_setup_init_state(struct i40iw_handler *hdl,
}
/**
+ * i40iw_get_used_rsrc - determine resources used internally
+ * @iwdev: iwarp device
+ *
+ * Called after internal allocations
+ */
+static void i40iw_get_used_rsrc(struct i40iw_device *iwdev)
+{
+ iwdev->used_pds = find_next_zero_bit(iwdev->allocated_pds, iwdev->max_pd, 0);
+ iwdev->used_qps = find_next_zero_bit(iwdev->allocated_qps, iwdev->max_qp, 0);
+ iwdev->used_cqs = find_next_zero_bit(iwdev->allocated_cqs, iwdev->max_cq, 0);
+ iwdev->used_mrs = find_next_zero_bit(iwdev->allocated_mrs, iwdev->max_mr, 0);
+}
+
+/**
* i40iw_open - client interface operation open for iwarp/uda device
* @ldev: lan device information
* @client: iwarp client information, provided during registration
@@ -1611,6 +1625,7 @@ static int i40iw_open(struct i40e_info *ldev, struct i40e_client *client)
status = i40iw_initialize_hw_resources(iwdev);
if (status)
break;
+ i40iw_get_used_rsrc(iwdev);
dev->ccq_ops->ccq_arm(dev->ccq);
status = i40iw_hmc_init_pble(&iwdev->sc_dev, iwdev->pble_rsrc);
if (status)
diff --git a/drivers/infiniband/hw/i40iw/i40iw_user.h b/drivers/infiniband/hw/i40iw/i40iw_user.h
index e65c2baa..66263fc 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_user.h
+++ b/drivers/infiniband/hw/i40iw/i40iw_user.h
@@ -76,7 +76,8 @@ enum i40iw_device_capabilities_const {
I40IW_MAX_ORD_SIZE = 127,
I40IW_MAX_WQ_ENTRIES = 2048,
I40IW_Q2_BUFFER_SIZE = (248 + 100),
- I40IW_QP_CTX_SIZE = 248
+ I40IW_QP_CTX_SIZE = 248,
+ I40IW_MAX_PDS = 32768
};
#define i40iw_handle void *
diff --git a/drivers/infiniband/hw/i40iw/i40iw_verbs.c b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
index 6329c97..bfea715 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_verbs.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
@@ -67,13 +67,13 @@ static int i40iw_query_device(struct ib_device *ibdev,
props->vendor_part_id = iwdev->ldev->pcidev->device;
props->hw_ver = (u32)iwdev->sc_dev.hw_rev;
props->max_mr_size = I40IW_MAX_OUTBOUND_MESSAGE_SIZE;
- props->max_qp = iwdev->max_qp;
+ props->max_qp = iwdev->max_qp - iwdev->used_qps;
props->max_qp_wr = (I40IW_MAX_WQ_ENTRIES >> 2) - 1;
props->max_sge = I40IW_MAX_WQ_FRAGMENT_COUNT;
- props->max_cq = iwdev->max_cq;
+ props->max_cq = iwdev->max_cq - iwdev->used_cqs;
props->max_cqe = iwdev->max_cqe;
- props->max_mr = iwdev->max_mr;
- props->max_pd = iwdev->max_pd;
+ props->max_mr = iwdev->max_mr - iwdev->used_mrs;
+ props->max_pd = iwdev->max_pd - iwdev->used_pds;
props->max_sge_rd = I40IW_MAX_SGE_RD;
props->max_qp_rd_atom = I40IW_MAX_IRD_SIZE;
props->max_qp_init_rd_atom = props->max_qp_rd_atom;
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH] i40iw: Remove checks for more than 48 bytes inline data
From: Henry Orosco @ 2016-11-10 3:32 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Henry Orosco
Remove dead code, which isn't executed because we
return error if the data size is greater than 48 bytes.
Inline data size greater than 48 bytes isn't supported
and the maximum WQE size is 64 bytes.
Signed-off-by: Tatyana Nikolova <tatyana.e.nikolova-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Henry Orosco <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/infiniband/hw/i40iw/i40iw_uk.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/infiniband/hw/i40iw/i40iw_uk.c b/drivers/infiniband/hw/i40iw/i40iw_uk.c
index 4d28c3c..338d164 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_uk.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_uk.c
@@ -1190,12 +1190,8 @@ enum i40iw_status_code i40iw_inline_data_size_to_wqesize(u32 data_size,
if (data_size <= 16)
*wqe_size = I40IW_QP_WQE_MIN_SIZE;
- else if (data_size <= 48)
- *wqe_size = 64;
- else if (data_size <= 80)
- *wqe_size = 96;
else
- *wqe_size = 128;
+ *wqe_size = 64;
return 0;
}
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH] i40iw: Remove NULL check for cm_node->iwdev
From: Henry Orosco @ 2016-11-10 3:32 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Henry Orosco
It is not necessary to check cm_node->iwdev in
i40iw_rem_ref_cm_node() as it can never be NULL after
a successful call out of i40iw_make_cm_node().
Signed-off-by: Chien Tin Tung <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Henry Orosco <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/infiniband/hw/i40iw/i40iw_cm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/i40iw/i40iw_cm.c b/drivers/infiniband/hw/i40iw/i40iw_cm.c
index c5e3d7a..71f2452 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_cm.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_cm.c
@@ -2224,7 +2224,7 @@ static void i40iw_rem_ref_cm_node(struct i40iw_cm_node *cm_node)
i40iw_dec_refcnt_listen(cm_core, cm_node->listener, 0, true);
} else {
if (!i40iw_listen_port_in_use(cm_core, cm_node->loc_port) &&
- cm_node->apbvt_set && cm_node->iwdev) {
+ cm_node->apbvt_set) {
i40iw_manage_apbvt(cm_node->iwdev,
cm_node->loc_port,
I40IW_MANAGE_APBVT_DEL);
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH] i40iw: Use actual page size
From: Henry Orosco @ 2016-11-10 3:33 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Henry Orosco
In i40iw_post_send, use the actual page size instead of
encoded page size. This is to be consistent with the
rest of the file.
Signed-off-by: Mustafa Ismail <mustafa.ismail-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Henry Orosco <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/infiniband/hw/i40iw/i40iw_verbs.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/infiniband/hw/i40iw/i40iw_verbs.c b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
index 6329c97..8f138b8 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_verbs.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
@@ -2142,7 +2142,6 @@ static int i40iw_post_send(struct ib_qp *ibqp,
case IB_WR_REG_MR:
{
struct i40iw_mr *iwmr = to_iwmr(reg_wr(ib_wr)->mr);
- int page_shift = ilog2(reg_wr(ib_wr)->mr->page_size);
int flags = reg_wr(ib_wr)->access;
struct i40iw_pble_alloc *palloc = &iwmr->iwpbl.pble_alloc;
struct i40iw_sc_dev *dev = &iwqp->iwdev->sc_dev;
@@ -2153,6 +2152,7 @@ static int i40iw_post_send(struct ib_qp *ibqp,
info.access_rights |= i40iw_get_user_access(flags);
info.stag_key = reg_wr(ib_wr)->key & 0xff;
info.stag_idx = reg_wr(ib_wr)->key >> 8;
+ info.page_size = reg_wr(ib_wr)->mr->page_size;
info.wr_id = ib_wr->wr_id;
info.addr_type = I40IW_ADDR_TYPE_VA_BASED;
@@ -2166,9 +2166,6 @@ static int i40iw_post_send(struct ib_qp *ibqp,
if (iwmr->npages > I40IW_MIN_PAGES_PER_FMR)
info.chunk_size = 1;
- if (page_shift == 21)
- info.page_size = 1; /* 2M page */
-
ret = dev->iw_priv_qp_ops->iw_mr_fast_register(&iwqp->sc_qp, &info, true);
if (ret)
err = -ENOMEM;
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH] i40iw: Use runtime check for IS_ENABLED(CONFIG_IPV6)
From: Henry Orosco @ 2016-11-10 3:34 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Henry Orosco
To be consistent, use the runtime check instead of
conditional compile.
Signed-off-by: Mustafa Ismail <mustafa.ismail-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Henry Orosco <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/infiniband/hw/i40iw/i40iw_cm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/hw/i40iw/i40iw_cm.c b/drivers/infiniband/hw/i40iw/i40iw_cm.c
index c5e3d7a..aaca1d0 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_cm.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_cm.c
@@ -1579,9 +1579,10 @@ static enum i40iw_status_code i40iw_del_multiple_qhash(
static struct net_device *i40iw_netdev_vlan_ipv6(u32 *addr, u16 *vlan_id, u8 *mac)
{
struct net_device *ip_dev = NULL;
-#if IS_ENABLED(CONFIG_IPV6)
struct in6_addr laddr6;
+ if (!IS_ENABLED(CONFIG_IPV6))
+ return NULL;
i40iw_copy_ip_htonl(laddr6.in6_u.u6_addr32, addr);
if (vlan_id)
*vlan_id = I40IW_NO_VLAN;
@@ -1598,7 +1599,6 @@ static struct net_device *i40iw_netdev_vlan_ipv6(u32 *addr, u16 *vlan_id, u8 *ma
}
}
rcu_read_unlock();
-#endif
return ip_dev;
}
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 1/2] i40iw: Remove variable flush_code and check to set qp->sq_flush
From: Henry Orosco @ 2016-11-10 3:42 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Henry Orosco
The flush_code variable in i40iw_bld_terminate_hdr() is obsolete and
the check to set qp->sq_flush is unreachable. Currently flush code is
populated in setup_term_hdr() and both SQ and RQ are flushed always
as part of the tear down flow.
Signed-off-by: Shiraz Saleem <shiraz.saleem-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Henry Orosco <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/infiniband/hw/i40iw/i40iw_ctrl.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/infiniband/hw/i40iw/i40iw_ctrl.c b/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
index cd71444..57a4236 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
@@ -4045,7 +4045,6 @@ static int i40iw_bld_terminate_hdr(struct i40iw_sc_qp *qp,
u16 ddp_seg_len;
int copy_len = 0;
u8 is_tagged = 0;
- enum i40iw_flush_opcode flush_code = FLUSH_INVALID;
u32 opcode;
struct i40iw_terminate_hdr *termhdr;
@@ -4218,9 +4217,6 @@ static int i40iw_bld_terminate_hdr(struct i40iw_sc_qp *qp,
if (copy_len)
memcpy(termhdr + 1, pkt, copy_len);
- if (flush_code && !info->in_rdrsp_wr)
- qp->sq_flush = (info->sq) ? true : false;
-
return sizeof(struct i40iw_terminate_hdr) + copy_len;
}
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH] i40iw: Remove check on return from device_init_pestat()
From: Henry Orosco @ 2016-11-10 3:42 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Henry Orosco
Remove unnecessary check for return code from
device_init_pestat() and change func to void.
Signed-off-by: Shiraz Saleem <shiraz.saleem-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Henry Orosco <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/infiniband/hw/i40iw/i40iw_ctrl.c | 10 ++--------
drivers/infiniband/hw/i40iw/i40iw_p.h | 2 +-
drivers/infiniband/hw/i40iw/i40iw_virtchnl.c | 6 +-----
3 files changed, 4 insertions(+), 14 deletions(-)
diff --git a/drivers/infiniband/hw/i40iw/i40iw_ctrl.c b/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
index 57a4236..9a5f4b8 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
@@ -4709,10 +4709,9 @@ static void i40iw_hw_stat_refresh_all(struct i40iw_dev_pestat *devstat)
* i40iw_device_init_pestat - Initialize the pestat structure
* @dev: pestat struct
*/
-enum i40iw_status_code i40iw_device_init_pestat(struct i40iw_dev_pestat *devstat)
+void i40iw_device_init_pestat(struct i40iw_dev_pestat *devstat)
{
devstat->ops = iw_device_pestat_ops;
- return 0;
}
/**
@@ -4736,12 +4735,7 @@ enum i40iw_status_code i40iw_device_init(struct i40iw_sc_dev *dev,
dev->debug_mask = info->debug_mask;
- ret_code = i40iw_device_init_pestat(&dev->dev_pestat);
- if (ret_code) {
- i40iw_debug(dev, I40IW_DEBUG_DEV,
- "%s: i40iw_device_init_pestat failed\n", __func__);
- return ret_code;
- }
+ i40iw_device_init_pestat(&dev->dev_pestat);
dev->hmc_fn_id = info->hmc_fn_id;
dev->qs_handle = info->qs_handle;
dev->exception_lan_queue = info->exception_lan_queue;
diff --git a/drivers/infiniband/hw/i40iw/i40iw_p.h b/drivers/infiniband/hw/i40iw/i40iw_p.h
index a0b8ca1..8a59c74 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_p.h
+++ b/drivers/infiniband/hw/i40iw/i40iw_p.h
@@ -47,7 +47,7 @@ void i40iw_debug_buf(struct i40iw_sc_dev *dev, enum i40iw_debug_flag mask,
enum i40iw_status_code i40iw_device_init(struct i40iw_sc_dev *dev,
struct i40iw_device_init_info *info);
-enum i40iw_status_code i40iw_device_init_pestat(struct i40iw_dev_pestat *);
+void i40iw_device_init_pestat(struct i40iw_dev_pestat *devstat);
void i40iw_sc_cqp_post_sq(struct i40iw_sc_cqp *cqp);
diff --git a/drivers/infiniband/hw/i40iw/i40iw_virtchnl.c b/drivers/infiniband/hw/i40iw/i40iw_virtchnl.c
index 3041003..dbd39c4 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_virtchnl.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_virtchnl.c
@@ -496,11 +496,7 @@ enum i40iw_status_code i40iw_vchnl_recv_pf(struct i40iw_sc_dev *dev,
i40iw_debug(dev, I40IW_DEBUG_VIRT,
"VF%u error CQP HMC Function operation.\n",
vf_id);
- ret_code = i40iw_device_init_pestat(&vf_dev->dev_pestat);
- if (ret_code)
- i40iw_debug(dev, I40IW_DEBUG_VIRT,
- "VF%u - i40iw_device_init_pestat failed\n",
- vf_id);
+ i40iw_device_init_pestat(&vf_dev->dev_pestat);
vf_dev->dev_pestat.ops.iw_hw_stat_init(&vf_dev->dev_pestat,
(u8)vf_dev->pmf_index,
dev->hw, false);
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* RE: [PATCH 1/2] i40iw: Remove variable flush_code and check to set qp->sq_flush
From: Orosco, Henry @ 2016-11-10 4:17 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
In-Reply-To: <20161110034207.18016-1-henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Please disregard this one. Accidently sent this one as part of a series.
-----Original Message-----
From: Orosco, Henry
Sent: Wednesday, November 09, 2016 9:42 PM
To: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org; Orosco, Henry <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Subject: [PATCH 1/2] i40iw: Remove variable flush_code and check to set qp->sq_flush
The flush_code variable in i40iw_bld_terminate_hdr() is obsolete and the check to set qp->sq_flush is unreachable. Currently flush code is populated in setup_term_hdr() and both SQ and RQ are flushed always as part of the tear down flow.
Signed-off-by: Shiraz Saleem <shiraz.saleem-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Henry Orosco <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/infiniband/hw/i40iw/i40iw_ctrl.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/infiniband/hw/i40iw/i40iw_ctrl.c b/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
index cd71444..57a4236 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
@@ -4045,7 +4045,6 @@ static int i40iw_bld_terminate_hdr(struct i40iw_sc_qp *qp,
u16 ddp_seg_len;
int copy_len = 0;
u8 is_tagged = 0;
- enum i40iw_flush_opcode flush_code = FLUSH_INVALID;
u32 opcode;
struct i40iw_terminate_hdr *termhdr;
@@ -4218,9 +4217,6 @@ static int i40iw_bld_terminate_hdr(struct i40iw_sc_qp *qp,
if (copy_len)
memcpy(termhdr + 1, pkt, copy_len);
- if (flush_code && !info->in_rdrsp_wr)
- qp->sq_flush = (info->sq) ? true : false;
-
return sizeof(struct i40iw_terminate_hdr) + copy_len; }
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH] i40iw: Remove variable flush_code and check to set qp->sq_flush
From: Henry Orosco @ 2016-11-10 4:20 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Henry Orosco
The flush_code variable in i40iw_bld_terminate_hdr() is obsolete and
the check to set qp->sq_flush is unreachable. Currently flush code is
populated in setup_term_hdr() and both SQ and RQ are flushed always
as part of the tear down flow.
Signed-off-by: Shiraz Saleem <shiraz.saleem-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Henry Orosco <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/infiniband/hw/i40iw/i40iw_ctrl.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/infiniband/hw/i40iw/i40iw_ctrl.c b/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
index cd71444..57a4236 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
@@ -4045,7 +4045,6 @@ static int i40iw_bld_terminate_hdr(struct i40iw_sc_qp *qp,
u16 ddp_seg_len;
int copy_len = 0;
u8 is_tagged = 0;
- enum i40iw_flush_opcode flush_code = FLUSH_INVALID;
u32 opcode;
struct i40iw_terminate_hdr *termhdr;
@@ -4218,9 +4217,6 @@ static int i40iw_bld_terminate_hdr(struct i40iw_sc_qp *qp,
if (copy_len)
memcpy(termhdr + 1, pkt, copy_len);
- if (flush_code && !info->in_rdrsp_wr)
- qp->sq_flush = (info->sq) ? true : false;
-
return sizeof(struct i40iw_terminate_hdr) + copy_len;
}
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH rdma-rc 0/9] mlx4 fixes for 4.9
From: Doug Ledford @ 2016-11-10 5:02 UTC (permalink / raw)
To: Leon Romanovsky, Yuval Shaia; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161108085021.GA27883-2ukJVAZIZ/Y@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 326 bytes --]
On 11/8/2016 3:50 AM, Leon Romanovsky wrote:
> On Sat, Nov 05, 2016 at 09:57:13PM +0200, Leon Romanovsky wrote:
>> Hi Doug,
>
> Please drop this patch series, I'll respin it once I'll return to
> office.
Okie dokie.
--
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
GPG Key ID: 0E572FDD
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]
^ permalink raw reply
* Re: [PATCH rdma-next 0/4] Add packet pacing support for IB verbs
From: Leon Romanovsky @ 2016-11-10 7:22 UTC (permalink / raw)
To: Hefty, Sean, Bodong Wang
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1828884A29C6694DAF28B7E6B8A82373AB0A7F0A-P5GAC/sN6hkd3b2yrw5b5LfspsVTdybXVpNB7YpNyf8@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1241 bytes --]
On Wed, Nov 09, 2016 at 05:06:52PM +0000, Hefty, Sean wrote:
> > On Tue, Nov 08, 2016 at 05:49:26PM +0000, Hefty, Sean wrote:
> > > > When sending from a 10G host to a 1G host, it is easy to overrun
> > the
> > > > receiver,
> > > > leading to packet loss and traffic backing off. Similar problems
> > occur
> > > > when
> > > > a 10G host sends data to a sub-10G virtual circuit, or a 40G host
> > > > sending
> > > > to a 10G host. Packet pacing could control packet injection rate
> > and
> > > > reduces
> > > > network congestion to maximize throughput & minimize network
> > latency.
> > >
> > > Why isn't the path record data and existing mechanisms sufficient to
> > handle this?
> > >
> >
> > Packet pacing allows different combinations of traffic shaping: per-
> > CPU,
> > per-flow and their combinations with better and steady QoS requirements
> > without involving subnet management.
>
> The patch adds this as a QP attribute, and we already have a rate for that. I still don't see why the standard mechanisms are insufficient or couldn't be adapted.
I'll let to Bodong to elaborate on it more, but as far as I see, the AH
attribute is relevant to UD QP only, while the packet pacing is intended
for all types of QPs.
Thanks
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCHv12 0/3] rdmacg: IB/core: rdma controller support
From: Parav Pandit @ 2016-11-10 7:41 UTC (permalink / raw)
To: Liran Liss
Cc: Leon Romanovsky, Tejun Heo,
cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-rdma,
Li Zefan, Johannes Weiner, Doug Ledford, Christoph Hellwig,
Hefty, Sean, Jason Gunthorpe, Haggai Eran,
james.l.morris-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org,
Or Gerlitz, Matan Barak
In-Reply-To: <HE1PR0501MB2812298C05431B08B0F408EEB1A60-692Kmc8YnlIVrnpjwTCbp8DSnupUy6xnnBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>
Hi Leon, Tejun, Christoph, Liran, Doug, Matan,
So are you ok with below proposal?
1. Define two resources by rdmacg.
(a) hca_handles (covers doorbell pages)
(b) hca_resources (mr, pd, qp, srq, vendor defined, all consolidated count)
Both cannot be combined as explained in [1].
2. User configures absolute count for above two resources (similar to
today's file descriptors, pid cgroup controller max limit)
Leon,
Let us know if you have any further discussions during LPC on
questions of [2] in using percentage based scheme or otherwise.
Parav
[1] https://www.spinics.net/lists/linux-rdma/msg42771.html
[2] https://www.spinics.net/lists/linux-rdma/msg42768.html
On Tue, Nov 8, 2016 at 1:42 PM, Liran Liss <liranl-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
>> From: Parav Pandit [mailto:pandit.parav-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
>
>> >
>> > Hmm..
>> > I guess that you are right.
>> >
>> > So we can add another count for "HCA handles",
>> I prefer this. This keeps it vendor agnostic and clean if we don't go percentage
>> route.
>
> OK; let's do it.
>
>> Would indirection table also fall in this category?
>>
>
> No. It's just another HCA resource...
>
>> > or alternatively, each provider will restrict the number of handles
>> > per device to a reasonable small number (which won't be treated as one of the
>> "HCA resources").
>> This would require vendor drivers to get the understanding of cgroup object
>> and pid and that breaks the modular approach. I like to avoid this.
>>
>> > Typically, a process shouldn't need to open more than a single handle...
>> Right. well behaved application won't do multiple handles.
^ permalink raw reply
* [PATCH rdma-rc] IB/IPoIB: Remove can't use GFP_NOIO warning
From: Leon Romanovsky @ 2016-11-10 8:16 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA,
yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Kamal Heib
From: Kamal Heib <kamalh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Remove the warning print of "can't use of GFP_NOIO" to avoid prints in
each QP creation when devices aren't supporting IB_QP_CREATE_USE_GFP_NOIO.
This print become more annoying when the IPoIB interface is configured
to work in connected mode.
Fixes: 09b93088d750 ('IB: Add a QP creation flag to use GFP_NOIO allocations')
Signed-off-by: Kamal Heib <kamalh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
drivers/infiniband/ulp/ipoib/ipoib_cm.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_cm.c b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
index 4ad297d..50c9772 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_cm.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
@@ -1053,8 +1053,6 @@ static struct ib_qp *ipoib_cm_create_tx_qp(struct net_device *dev, struct ipoib_
tx_qp = ib_create_qp(priv->pd, &attr);
if (PTR_ERR(tx_qp) == -EINVAL) {
- ipoib_warn(priv, "can't use GFP_NOIO for QPs on device %s, using GFP_KERNEL\n",
- priv->ca->name);
attr.create_flags &= ~IB_QP_CREATE_USE_GFP_NOIO;
tx_qp = ib_create_qp(priv->pd, &attr);
}
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [RFC ABI V5 02/10] RDMA/core: Add support for custom types
From: Matan Barak @ 2016-11-10 8:29 UTC (permalink / raw)
To: Hefty, Sean, Matan Barak
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Doug Ledford,
Jason Gunthorpe, Christoph Lameter, Liran Liss, Haggai Eran,
Majd Dibbiny, Tal Alon, Leon Romanovsky
In-Reply-To: <1828884A29C6694DAF28B7E6B8A82373AB0A8000-P5GAC/sN6hkd3b2yrw5b5LfspsVTdybXVpNB7YpNyf8@public.gmane.org>
On 09/11/2016 20:00, Hefty, Sean wrote:
>> I had thought about that, but the user could initialize its part of
>> the object in the function handler. It can't allocate the object as we
>> need it in order to allocate an IDR entry and co. The assumption here
>> is that the "unlock" stage can't fail.
>
> This is creating a generic OO type of framework, so just add constructor/destructor functions and have all objects inherit from a base ioctl object class.
>
Adding a constructor and destructor to every object would make the
infrastructure slower, it'll open code the locks (which are more related
to the actions themselves and not to the types), it'll duplicate more
code. Anyway, examining our use, I don't really see good value for ctors
and dtors for our usage right now.
>>> In fact, it would be great if we could just cleanup the list in the
>> reverse order that items were created. Maybe this requires supporting
>> a pre-cleanup handler, so that the driver can pluck items out of the
>> list that may need to be destroyed out of order.
>>>
>>
>> So that's essentially one layer of ordering. Why do you consider a
>> driver iterating over all objects simpler than this model?
>
> This problem is a verbs specific issue, and one that only involves MW . We have reference counts that can provide the same functionality. I want to avoid the amount of meta-data that needs to be used to describe objects.
>
It's currently happens in the verbs MW/MRs. However, it could happen
with any types whose bindings happen in the user-space or hardware.
>>>> Adding an object is done in two parts.
>>>> First, an object is allocated and added to IDR/fd table. Then, the
>>>> command's handlers (in downstream patches) could work on this object
>>>> and fill in its required details.
>>>> After a successful command, ib_uverbs_uobject_enable is called and
>>>> this user objects becomes ucontext visible.
>>>
>>> If you have a way to mark that an object is used for exclusive
>> access, you may be able to use that instead of introducing a new
>> variable. (I.e. acquire the object's write lock). I think we want to
>> make an effort to minimize the size of the kernel structure needed to
>> track every user space object (within reason).
>>>
>>
>> I didn't really follow. A command attribute states the nature of the
>> locking (for example, in MODIFY_QP the QP could be exclusively locked,
>> but in QUERY_QP it's only locked for reading). I don't want to really
>> grab a lock, as if I were I could face a dead-lock (user-space could
>> pass parameters in a colliding order), It could be solved by sorting
>> the handles, but that would degrade performance without a good reasob.
>
> I'm suggesting that the locking attribute and command be separate. This allows the framework to acquire the proper type of lock independent of what function it will invoke.
>
The locking is tied to what you want to do on that type. If you query
something, read locks are probably enough. I don't think separating them
will make the code more readable.
> The framework doesn't need to hold locks. It should be able to mark access to an object. If that access is not available, it can abort. This pushes more complex synchronization and thread handling to user space.
>
The try_{read,write} locks achieve exactly that and are simple enough.
>>>> Removing an uboject is done by calling ib_uverbs_uobject_remove.
>>>>
>>>> We should make sure IDR (per-device) and list (per-ucontext) could
>>>> be accessed concurrently without corrupting them.
>>>>
>>>> Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>>> Signed-off-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>>> Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>>> ---
>>>
>>> As a general comment, I do have concerns that the resulting
>> generalized parsing of everything will negatively impact performance
>> for operations that do have to transition into the kernel. Not all
>> devices offload all operations to user space. Plus the resulting code
>> is extremely difficult to read and non-trivial to use. It's equivalent
>> to reading C++ code that has 4 layers of inheritance with overrides to
>> basic operators...
>>
>> There are two parts here. I think the handlers themselves are simpler,
>> easier to read and less error-prone. They contain less code
>> duplications. The macro based define language explicitly declare all
>> attributes, their types, size, etc.
>> The model here is a bit more complex as we want to achieve both code
>> resue and add/override of new types/actions/attributes.
>>
>>
>>>
>>> Pre and post operators per command that can do straightforward
>> validation seem like a better option.
>>>
>>>
>>
>> I think that would duplicate a lot of code and will be more
>> error-prone than one infrastrucutre that automates all that work for
>> you.
>
> I think that's a toss-up. Either you have to write the code correctly or write the rules correctly. Reading code is straightforward, manually converting rules into code is not.
>
> In any case, the two approaches are not exclusive. By forcing the rule language into the framework, everything is forced to deal with it. By leaving it out, each ioctl provider can decide if they need this or not. If you want verbs to process all ioctl's using a single pre-validation function that operates based on these rules you can. Nothing prevents that. But ioctl providers that want better performance can elect for a more straightforward validation model.
>
Please see Jason's response.
>>>> drivers/infiniband/core/Makefile | 3 +-
>>>> drivers/infiniband/core/device.c | 1 +
>>>> drivers/infiniband/core/rdma_core.c | 489
>>>> ++++++++++++++++++++++++++++++++++
>>>> drivers/infiniband/core/rdma_core.h | 75 ++++++
>>>> drivers/infiniband/core/uverbs.h | 1 +
>>>> drivers/infiniband/core/uverbs_main.c | 2 +-
>>>> include/rdma/ib_verbs.h | 28 +-
>>>> include/rdma/uverbs_ioctl.h | 195 ++++++++++++++
>>>> 8 files changed, 789 insertions(+), 5 deletions(-)
>>>> create mode 100644 drivers/infiniband/core/rdma_core.c
>>>> create mode 100644 drivers/infiniband/core/rdma_core.h
>>>> create mode 100644 include/rdma/uverbs_ioctl.h
>>>>
>>>> diff --git a/drivers/infiniband/core/Makefile
>>>> b/drivers/infiniband/core/Makefile
>>>> index edaae9f..1819623 100644
>>>> --- a/drivers/infiniband/core/Makefile
>>>> +++ b/drivers/infiniband/core/Makefile
>>>> @@ -28,4 +28,5 @@ ib_umad-y := user_mad.o
>>>>
>>>> ib_ucm-y := ucm.o
>>>>
>>>> -ib_uverbs-y := uverbs_main.o uverbs_cmd.o
>>>> uverbs_marshall.o
>>>> +ib_uverbs-y := uverbs_main.o uverbs_cmd.o
>>>> uverbs_marshall.o \
>>>> + rdma_core.o
>>>> diff --git a/drivers/infiniband/core/device.c
>>>> b/drivers/infiniband/core/device.c
>>>> index c3b68f5..43994b1 100644
>>>> --- a/drivers/infiniband/core/device.c
>>>> +++ b/drivers/infiniband/core/device.c
>>>> @@ -243,6 +243,7 @@ struct ib_device *ib_alloc_device(size_t size)
>>>> spin_lock_init(&device->client_data_lock);
>>>> INIT_LIST_HEAD(&device->client_data_list);
>>>> INIT_LIST_HEAD(&device->port_list);
>>>> + INIT_LIST_HEAD(&device->type_list);
>>>>
>>>> return device;
>>>> }
>>>> diff --git a/drivers/infiniband/core/rdma_core.c
>>>> b/drivers/infiniband/core/rdma_core.c
>>>> new file mode 100644
>>>> index 0000000..337abc2
>>>> --- /dev/null
>>>> +++ b/drivers/infiniband/core/rdma_core.c
>>>> @@ -0,0 +1,489 @@
>>>> +/*
>>>> + * Copyright (c) 2016, Mellanox Technologies inc. All rights
>>>> reserved.
>>>> + *
>>>> + * This software is available to you under a choice of one of two
>>>> + * licenses. You may choose to be licensed under the terms of the
>> GNU
>>>> + * General Public License (GPL) Version 2, available from the file
>>>> + * COPYING in the main directory of this source tree, or the
>>>> + * OpenIB.org BSD license below:
>>>> + *
>>>> + * Redistribution and use in source and binary forms, with or
>>>> + * without modification, are permitted provided that the
>> following
>>>> + * conditions are met:
>>>> + *
>>>> + * - Redistributions of source code must retain the above
>>>> + * copyright notice, this list of conditions and the
>> following
>>>> + * disclaimer.
>>>> + *
>>>> + * - Redistributions in binary form must reproduce the above
>>>> + * copyright notice, this list of conditions and the
>> following
>>>> + * disclaimer in the documentation and/or other materials
>>>> + * provided with the distribution.
>>>> + *
>>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>>>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
>> OF
>>>> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>>>> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
>> HOLDERS
>>>> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
>> AN
>>>> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR
>> IN
>>>> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
>>>> + * SOFTWARE.
>>>> + */
>>>> +
>>>> +#include <linux/file.h>
>>>> +#include <linux/anon_inodes.h>
>>>> +#include <rdma/ib_verbs.h>
>>>> +#include "uverbs.h"
>>>> +#include "rdma_core.h"
>>>> +#include <rdma/uverbs_ioctl.h>
>>>> +
>>>> +const struct uverbs_type *uverbs_get_type(const struct ib_device
>>>> *ibdev,
>>>> + uint16_t type)
>>>> +{
>>>> + const struct uverbs_types_group *groups = ibdev->types_group;
>>>> + const struct uverbs_types *types;
>>>> + int ret = groups->dist(&type, groups->priv);
>>>> +
>>>> + if (ret >= groups->num_groups)
>>>> + return NULL;
>>>> +
>>>> + types = groups->type_groups[ret];
>>>> +
>>>> + if (type >= types->num_types)
>>>> + return NULL;
>>>> +
>>>> + return types->types[type];
>>>> +}
>>>> +
>>>> +static int uverbs_lock_object(struct ib_uobject *uobj,
>>>> + enum uverbs_idr_access access)
>>>> +{
>>>> + if (access == UVERBS_IDR_ACCESS_READ)
>>>> + return down_read_trylock(&uobj->usecnt) == 1 ? 0 : -
>> EBUSY;
>>>> +
>>>> + /* lock is either WRITE or DESTROY - should be exclusive */
>>>> + return down_write_trylock(&uobj->usecnt) == 1 ? 0 : -EBUSY;
>>>
>>> This function could take the lock type directly (read or write),
>> versus inferring it based on some other access type.
>>>
>>
>> We can, but since we use these enums in the attribute specifications,
>> I thought it could be more convinient.
>>
>>>> +}
>>>> +
>>>> +static struct ib_uobject *get_uobj(int id, struct ib_ucontext
>>>> *context)
>>>> +{
>>>> + struct ib_uobject *uobj;
>>>> +
>>>> + rcu_read_lock();
>>>> + uobj = idr_find(&context->device->idr, id);
>>>> + if (uobj && uobj->live) {
>>>> + if (uobj->context != context)
>>>> + uobj = NULL;
>>>> + }
>>>> + rcu_read_unlock();
>>>> +
>>>> + return uobj;
>>>> +}
>>>> +
>>>> +struct ib_ucontext_lock {
>>>> + struct kref ref;
>>>> + /* locking the uobjects_list */
>>>> + struct mutex lock;
>>>> +};
>>>> +
>>>> +static void init_uobjects_list_lock(struct ib_ucontext_lock *lock)
>>>> +{
>>>> + mutex_init(&lock->lock);
>>>> + kref_init(&lock->ref);
>>>> +}
>>>> +
>>>> +static void release_uobjects_list_lock(struct kref *ref)
>>>> +{
>>>> + struct ib_ucontext_lock *lock = container_of(ref,
>>>> + struct
>> ib_ucontext_lock,
>>>> + ref);
>>>> +
>>>> + kfree(lock);
>>>> +}
>>>> +
>>>> +static void init_uobj(struct ib_uobject *uobj, u64 user_handle,
>>>> + struct ib_ucontext *context)
>>>> +{
>>>> + init_rwsem(&uobj->usecnt);
>>>> + uobj->user_handle = user_handle;
>>>> + uobj->context = context;
>>>> + uobj->live = 0;
>>>> +}
>>>> +
>>>> +static int add_uobj(struct ib_uobject *uobj)
>>>> +{
>>>> + int ret;
>>>> +
>>>> + idr_preload(GFP_KERNEL);
>>>> + spin_lock(&uobj->context->device->idr_lock);
>>>> +
>>>> + ret = idr_alloc(&uobj->context->device->idr, uobj, 0, 0,
>>>> GFP_NOWAIT);
>>>> + if (ret >= 0)
>>>> + uobj->id = ret;
>>>> +
>>>> + spin_unlock(&uobj->context->device->idr_lock);
>>>> + idr_preload_end();
>>>> +
>>>> + return ret < 0 ? ret : 0;
>>>> +}
>>>> +
>>>> +static void remove_uobj(struct ib_uobject *uobj)
>>>> +{
>>>> + spin_lock(&uobj->context->device->idr_lock);
>>>> + idr_remove(&uobj->context->device->idr, uobj->id);
>>>> + spin_unlock(&uobj->context->device->idr_lock);
>>>> +}
>>>> +
>>>> +static void put_uobj(struct ib_uobject *uobj)
>>>> +{
>>>> + kfree_rcu(uobj, rcu);
>>>> +}
>>>> +
>>>> +static struct ib_uobject *get_uobject_from_context(struct
>> ib_ucontext
>>>> *ucontext,
>>>> + const struct
>>>> uverbs_type_alloc_action *type,
>>>> + u32 idr,
>>>> + enum
>> uverbs_idr_access access)
>>>> +{
>>>> + struct ib_uobject *uobj;
>>>> + int ret;
>>>> +
>>>> + rcu_read_lock();
>>>> + uobj = get_uobj(idr, ucontext);
>>>> + if (!uobj)
>>>> + goto free;
>>>> +
>>>> + if (uobj->type != type) {
>>>> + uobj = NULL;
>>>> + goto free;
>>>> + }
>>>> +
>>>> + ret = uverbs_lock_object(uobj, access);
>>>> + if (ret)
>>>> + uobj = ERR_PTR(ret);
>>>> +free:
>>>> + rcu_read_unlock();
>>>> + return uobj;
>>>> +
>>>> + return NULL;
>>>> +}
>>>> +
>>>> +static int ib_uverbs_uobject_add(struct ib_uobject *uobject,
>>>> + const struct uverbs_type_alloc_action
>>>> *uobject_type)
>>>> +{
>>>> + uobject->type = uobject_type;
>>>> + return add_uobj(uobject);
>>>> +}
>>>> +
>>>> +struct ib_uobject *uverbs_get_type_from_idr(const struct
>>>> uverbs_type_alloc_action *type,
>>>> + struct ib_ucontext
>> *ucontext,
>>>> + enum uverbs_idr_access
>> access,
>>>> + uint32_t idr)
>>>> +{
>>>> + struct ib_uobject *uobj;
>>>> + int ret;
>>>> +
>>>> + if (access == UVERBS_IDR_ACCESS_NEW) {
>>>> + uobj = kmalloc(type->obj_size, GFP_KERNEL);
>>>> + if (!uobj)
>>>> + return ERR_PTR(-ENOMEM);
>>>> +
>>>> + init_uobj(uobj, 0, ucontext);
>>>> +
>>>> + /* lock idr */
>>>
>>> Command to lock idr, but no lock is obtained.
>>>
>>
>> ib_uverbs_uobject_add calls add_uobj which locks the IDR.
>>
>>>> + ret = ib_uverbs_uobject_add(uobj, type);
>>>> + if (ret) {
>>>> + kfree(uobj);
>>>> + return ERR_PTR(ret);
>>>> + }
>>>> +
>>>> + } else {
>>>> + uobj = get_uobject_from_context(ucontext, type, idr,
>>>> + access);
>>>> +
>>>> + if (!uobj)
>>>> + return ERR_PTR(-ENOENT);
>>>> + }
>>>> +
>>>> + return uobj;
>>>> +}
>>>> +
>>>> +struct ib_uobject *uverbs_get_type_from_fd(const struct
>>>> uverbs_type_alloc_action *type,
>>>> + struct ib_ucontext
>> *ucontext,
>>>> + enum uverbs_idr_access
>> access,
>>>> + int fd)
>>>> +{
>>>> + if (access == UVERBS_IDR_ACCESS_NEW) {
>>>> + int _fd;
>>>> + struct ib_uobject *uobj = NULL;
>>>> + struct file *filp;
>>>> +
>>>> + _fd = get_unused_fd_flags(O_CLOEXEC);
>>>> + if (_fd < 0 || WARN_ON(type->obj_size < sizeof(struct
>>>> ib_uobject)))
>>>> + return ERR_PTR(_fd);
>>>> +
>>>> + uobj = kmalloc(type->obj_size, GFP_KERNEL);
>>>> + init_uobj(uobj, 0, ucontext);
>>>> +
>>>> + if (!uobj)
>>>> + return ERR_PTR(-ENOMEM);
>>>> +
>>>> + filp = anon_inode_getfile(type->fd.name, type-
>>> fd.fops,
>>>> + uobj + 1, type->fd.flags);
>>>> + if (IS_ERR(filp)) {
>>>> + put_unused_fd(_fd);
>>>> + kfree(uobj);
>>>> + return (void *)filp;
>>>> + }
>>>> +
>>>> + uobj->type = type;
>>>> + uobj->id = _fd;
>>>> + uobj->object = filp;
>>>> +
>>>> + return uobj;
>>>> + } else if (access == UVERBS_IDR_ACCESS_READ) {
>>>> + struct file *f = fget(fd);
>>>> + struct ib_uobject *uobject;
>>>> +
>>>> + if (!f)
>>>> + return ERR_PTR(-EBADF);
>>>> +
>>>> + uobject = f->private_data - sizeof(struct ib_uobject);
>>>> + if (f->f_op != type->fd.fops ||
>>>> + !uobject->live) {
>>>> + fput(f);
>>>> + return ERR_PTR(-EBADF);
>>>> + }
>>>> +
>>>> + /*
>>>> + * No need to protect it with a ref count, as fget
>>>> increases
>>>> + * f_count.
>>>> + */
>>>> + return uobject;
>>>> + } else {
>>>> + return ERR_PTR(-EOPNOTSUPP);
>>>> + }
>>>> +}
>>>> +
>>>> +static void ib_uverbs_uobject_enable(struct ib_uobject *uobject)
>>>> +{
>>>> + mutex_lock(&uobject->context->uobjects_lock->lock);
>>>> + list_add(&uobject->list, &uobject->context->uobjects);
>>>> + mutex_unlock(&uobject->context->uobjects_lock->lock);
>>>
>>> Why not just insert the object into the list on creation?
>>>
>>>> + uobject->live = 1;
>>>
>>> See my comments above on removing the live field.
>>>
>>
>> Seems that the list could suffice, but I'll look into that.
>>
>>>> +}
>>>> +
>>>> +static void ib_uverbs_uobject_remove(struct ib_uobject *uobject,
>> bool
>>>> lock)
>>>> +{
>>>> + /*
>>>> + * Calling remove requires exclusive access, so it's not
>> possible
>>>> + * another thread will use our object.
>>>> + */
>>>> + uobject->live = 0;
>>>> + uobject->type->free_fn(uobject->type, uobject);
>>>> + if (lock)
>>>> + mutex_lock(&uobject->context->uobjects_lock->lock);
>>>> + list_del(&uobject->list);
>>>> + if (lock)
>>>> + mutex_unlock(&uobject->context->uobjects_lock->lock);
>>>> + remove_uobj(uobject);
>>>> + put_uobj(uobject);
>>>> +}
>>>> +
>>>> +static void uverbs_unlock_idr(struct ib_uobject *uobj,
>>>> + enum uverbs_idr_access access,
>>>> + bool success)
>>>> +{
>>>> + switch (access) {
>>>> + case UVERBS_IDR_ACCESS_READ:
>>>> + up_read(&uobj->usecnt);
>>>> + break;
>>>> + case UVERBS_IDR_ACCESS_NEW:
>>>> + if (success) {
>>>> + ib_uverbs_uobject_enable(uobj);
>>>> + } else {
>>>> + remove_uobj(uobj);
>>>> + put_uobj(uobj);
>>>> + }
>>>> + break;
>>>> + case UVERBS_IDR_ACCESS_WRITE:
>>>> + up_write(&uobj->usecnt);
>>>> + break;
>>>> + case UVERBS_IDR_ACCESS_DESTROY:
>>>> + if (success)
>>>> + ib_uverbs_uobject_remove(uobj, true);
>>>> + else
>>>> + up_write(&uobj->usecnt);
>>>> + break;
>>>> + }
>>>> +}
>>>> +
>>>> +static void uverbs_unlock_fd(struct ib_uobject *uobj,
>>>> + enum uverbs_idr_access access,
>>>> + bool success)
>>>> +{
>>>> + struct file *filp = uobj->object;
>>>> +
>>>> + if (access == UVERBS_IDR_ACCESS_NEW) {
>>>> + if (success) {
>>>> + kref_get(&uobj->context->ufile->ref);
>>>> + uobj->uobjects_lock = uobj->context-
>>> uobjects_lock;
>>>> + kref_get(&uobj->uobjects_lock->ref);
>>>> + ib_uverbs_uobject_enable(uobj);
>>>> + fd_install(uobj->id, uobj->object);
>>>
>>> I don't get this. The function is unlocking something, but there are
>> calls to get krefs?
>>>
>>
>> Before invoking the user's callback, we're first locking all objects
>> and afterwards we're unlocking them. When we need to create a new
>> object, the lock becomes object creation and the unlock could become
>> (assuming the user's callback succeeded) enabling this new object.
>> When you add a new object (or fd in this case), we take a reference
>> count to both the uverbs_file and the locking context.
>>
>>>> + } else {
>>>> + fput(uobj->object);
>>>> + put_unused_fd(uobj->id);
>>>> + kfree(uobj);
>>>> + }
>>>> + } else {
>>>> + fput(filp);
>>>> + }
>>>> +}
>>>> +
>>>> +void uverbs_unlock_object(struct ib_uobject *uobj,
>>>> + enum uverbs_idr_access access,
>>>> + bool success)
>>>> +{
>>>> + if (uobj->type->type == UVERBS_ATTR_TYPE_IDR)
>>>> + uverbs_unlock_idr(uobj, access, success);
>>>> + else if (uobj->type->type == UVERBS_ATTR_TYPE_FD)
>>>> + uverbs_unlock_fd(uobj, access, success);
>>>> + else
>>>> + WARN_ON(true);
>>>> +}
>>>> +
>>>> +static void ib_uverbs_remove_fd(struct ib_uobject *uobject)
>>>> +{
>>>> + /*
>>>> + * user should release the uobject in the release
>>>> + * callback.
>>>> + */
>>>> + if (uobject->live) {
>>>> + uobject->live = 0;
>>>> + list_del(&uobject->list);
>>>> + uobject->type->free_fn(uobject->type, uobject);
>>>> + kref_put(&uobject->context->ufile->ref,
>>>> ib_uverbs_release_file);
>>>> + uobject->context = NULL;
>>>> + }
>>>> +}
>>>> +
>>>> +void ib_uverbs_close_fd(struct file *f)
>>>> +{
>>>> + struct ib_uobject *uobject = f->private_data - sizeof(struct
>>>> ib_uobject);
>>>> +
>>>> + mutex_lock(&uobject->uobjects_lock->lock);
>>>> + if (uobject->live) {
>>>> + uobject->live = 0;
>>>> + list_del(&uobject->list);
>>>> + kref_put(&uobject->context->ufile->ref,
>>>> ib_uverbs_release_file);
>>>> + uobject->context = NULL;
>>>> + }
>>>> + mutex_unlock(&uobject->uobjects_lock->lock);
>>>> + kref_put(&uobject->uobjects_lock->ref,
>>>> release_uobjects_list_lock);
>>>> +}
>>>> +
>>>> +void ib_uverbs_cleanup_fd(void *private_data)
>>>> +{
>>>> + struct ib_uboject *uobject = private_data - sizeof(struct
>>>> ib_uobject);
>>>> +
>>>> + kfree(uobject);
>>>> +}
>>>> +
>>>> +void uverbs_unlock_objects(struct uverbs_attr_array *attr_array,
>>>> + size_t num,
>>>> + const struct uverbs_action_spec *spec,
>>>> + bool success)
>>>> +{
>>>> + unsigned int i;
>>>> +
>>>> + for (i = 0; i < num; i++) {
>>>> + struct uverbs_attr_array *attr_spec_array =
>> &attr_array[i];
>>>> + const struct uverbs_attr_group_spec *group_spec =
>>>> + spec->attr_groups[i];
>>>> + unsigned int j;
>>>> +
>>>> + for (j = 0; j < attr_spec_array->num_attrs; j++) {
>>>> + struct uverbs_attr *attr = &attr_spec_array-
>>>>> attrs[j];
>>>> + struct uverbs_attr_spec *spec = &group_spec-
>>>>> attrs[j];
>>>> +
>>>> + if (!attr->valid)
>>>> + continue;
>>>> +
>>>> + if (spec->type == UVERBS_ATTR_TYPE_IDR ||
>>>> + spec->type == UVERBS_ATTR_TYPE_FD)
>>>> + /*
>>>> + * refcounts should be handled at the
>> object
>>>> + * level and not at the uobject level.
>>>> + */
>>>> + uverbs_unlock_object(attr-
>>> obj_attr.uobject,
>>>> + spec->obj.access,
>> success);
>>>> + }
>>>> + }
>>>> +}
>>>> +
>>>> +static unsigned int get_type_orders(const struct uverbs_types_group
>>>> *types_group)
>>>> +{
>>>> + unsigned int i;
>>>> + unsigned int max = 0;
>>>> +
>>>> + for (i = 0; i < types_group->num_groups; i++) {
>>>> + unsigned int j;
>>>> + const struct uverbs_types *types = types_group-
>>>>> type_groups[i];
>>>> +
>>>> + for (j = 0; j < types->num_types; j++) {
>>>> + if (!types->types[j] || !types->types[j]-
>>> alloc)
>>>> + continue;
>>>> + if (types->types[j]->alloc->order > max)
>>>> + max = types->types[j]->alloc->order;
>>>> + }
>>>> + }
>>>> +
>>>> + return max;
>>>> +}
>>>> +
>>>> +void ib_uverbs_uobject_type_cleanup_ucontext(struct ib_ucontext
>>>> *ucontext,
>>>> + const struct
>> uverbs_types_group
>>>> *types_group)
>>>> +{
>>>> + unsigned int num_orders = get_type_orders(types_group);
>>>> + unsigned int i;
>>>> +
>>>> + for (i = 0; i <= num_orders; i++) {
>>>> + struct ib_uobject *obj, *next_obj;
>>>> +
>>>> + /*
>>>> + * No need to take lock here, as cleanup should be
>> called
>>>> + * after all commands finished executing. Newly
>> executed
>>>> + * commands should fail.
>>>> + */
>>>> + mutex_lock(&ucontext->uobjects_lock->lock);
>>>
>>> It's really confusing to see a comment about 'no need to take lock'
>> immediately followed by a call to lock.
>>>
>>
>> Yeah :) That was before adding the fd. I'll delete the comment.
>>
>>>> + list_for_each_entry_safe(obj, next_obj, &ucontext-
>>>>> uobjects,
>>>> + list)
>>>> + if (obj->type->order == i) {
>>>> + if (obj->type->type ==
>> UVERBS_ATTR_TYPE_IDR)
>>>> + ib_uverbs_uobject_remove(obj,
>> false);
>>>> + else
>>>> + ib_uverbs_remove_fd(obj);
>>>> + }
>>>> + mutex_unlock(&ucontext->uobjects_lock->lock);
>>>> + }
>>>> + kref_put(&ucontext->uobjects_lock->ref,
>>>> release_uobjects_list_lock);
>>>> +}
>>>> +
>>>> +int ib_uverbs_uobject_type_initialize_ucontext(struct ib_ucontext
>>>> *ucontext)
>>>
>>> Please work on the function names. This is horrendously long and
>> still doesn't help describe what it does.
>>>
>>
>> This just initialized the types part of the ucontext. Any suggestions?
>>
>>>> +{
>>>> + ucontext->uobjects_lock = kmalloc(sizeof(*ucontext-
>>>>> uobjects_lock),
>>>> + GFP_KERNEL);
>>>> + if (!ucontext->uobjects_lock)
>>>> + return -ENOMEM;
>>>> +
>>>> + init_uobjects_list_lock(ucontext->uobjects_lock);
>>>> + INIT_LIST_HEAD(&ucontext->uobjects);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +void ib_uverbs_uobject_type_release_ucontext(struct ib_ucontext
>>>> *ucontext)
>>>> +{
>>>> + kfree(ucontext->uobjects_lock);
>>>> +}
>>>
>>> No need to wrap a call to 'free'.
>>>
>>
>> In order to abstract away the ucontext type data structure.
>>
>>>> +
>>>> diff --git a/drivers/infiniband/core/rdma_core.h
>>>> b/drivers/infiniband/core/rdma_core.h
>>>> new file mode 100644
>>>> index 0000000..8990115
>>>> --- /dev/null
>>>> +++ b/drivers/infiniband/core/rdma_core.h
>>>> @@ -0,0 +1,75 @@
>>>> +/*
>>>> + * Copyright (c) 2005 Topspin Communications. All rights reserved.
>>>> + * Copyright (c) 2005, 2006 Cisco Systems. All rights reserved.
>>>> + * Copyright (c) 2005-2016 Mellanox Technologies. All rights
>> reserved.
>>>> + * Copyright (c) 2005 Voltaire, Inc. All rights reserved.
>>>> + * Copyright (c) 2005 PathScale, Inc. All rights reserved.
>>>> + *
>>>> + * This software is available to you under a choice of one of two
>>>> + * licenses. You may choose to be licensed under the terms of the
>> GNU
>>>> + * General Public License (GPL) Version 2, available from the file
>>>> + * COPYING in the main directory of this source tree, or the
>>>> + * OpenIB.org BSD license below:
>>>> + *
>>>> + * Redistribution and use in source and binary forms, with or
>>>> + * without modification, are permitted provided that the
>> following
>>>> + * conditions are met:
>>>> + *
>>>> + * - Redistributions of source code must retain the above
>>>> + * copyright notice, this list of conditions and the
>> following
>>>> + * disclaimer.
>>>> + *
>>>> + * - Redistributions in binary form must reproduce the above
>>>> + * copyright notice, this list of conditions and the
>> following
>>>> + * disclaimer in the documentation and/or other materials
>>>> + * provided with the distribution.
>>>> + *
>>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>>>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
>> OF
>>>> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>>>> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
>> HOLDERS
>>>> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
>> AN
>>>> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR
>> IN
>>>> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
>>>> + * SOFTWARE.
>>>> + */
>>>> +
>>>> +#ifndef UOBJECT_H
>>>> +#define UOBJECT_H
>>>> +
>>>> +#include <linux/idr.h>
>>>> +#include <rdma/uverbs_ioctl.h>
>>>> +#include <rdma/ib_verbs.h>
>>>> +#include <linux/mutex.h>
>>>> +
>>>> +const struct uverbs_type *uverbs_get_type(const struct ib_device
>>>> *ibdev,
>>>> + uint16_t type);
>>>> +struct ib_uobject *uverbs_get_type_from_idr(const struct
>>>> uverbs_type_alloc_action *type,
>>>> + struct ib_ucontext
>> *ucontext,
>>>> + enum uverbs_idr_access
>> access,
>>>> + uint32_t idr);
>>>> +struct ib_uobject *uverbs_get_type_from_fd(const struct
>>>> uverbs_type_alloc_action *type,
>>>> + struct ib_ucontext
>> *ucontext,
>>>> + enum uverbs_idr_access
>> access,
>>>> + int fd);
>>>> +void uverbs_unlock_object(struct ib_uobject *uobj,
>>>> + enum uverbs_idr_access access,
>>>> + bool success);
>>>> +void uverbs_unlock_objects(struct uverbs_attr_array *attr_array,
>>>> + size_t num,
>>>> + const struct uverbs_action_spec *spec,
>>>> + bool success);
>>>> +
>>>> +void ib_uverbs_uobject_type_cleanup_ucontext(struct ib_ucontext
>>>> *ucontext,
>>>> + const struct
>> uverbs_types_group
>>>> *types_group);
>>>> +int ib_uverbs_uobject_type_initialize_ucontext(struct ib_ucontext
>>>> *ucontext);
>>>> +void ib_uverbs_uobject_type_release_ucontext(struct ib_ucontext
>>>> *ucontext);
>>>> +void ib_uverbs_close_fd(struct file *f);
>>>> +void ib_uverbs_cleanup_fd(void *private_data);
>>>> +
>>>> +static inline void *uverbs_fd_to_priv(struct ib_uobject *uobj)
>>>> +{
>>>> + return uobj + 1;
>>>> +}
>>>
>>> This seems like a rather useless function.
>>>
>>
>> Why? The user sholdn't know or care how we put our structs together.
>>
>>>> +
>>>> +#endif /* UIDR_H */
>>>> diff --git a/drivers/infiniband/core/uverbs.h
>>>> b/drivers/infiniband/core/uverbs.h
>>>> index 8074705..ae7d4b8 100644
>>>> --- a/drivers/infiniband/core/uverbs.h
>>>> +++ b/drivers/infiniband/core/uverbs.h
>>>> @@ -180,6 +180,7 @@ void idr_remove_uobj(struct ib_uobject *uobj);
>>>> struct file *ib_uverbs_alloc_event_file(struct ib_uverbs_file
>>>> *uverbs_file,
>>>> struct ib_device *ib_dev,
>>>> int is_async);
>>>> +void ib_uverbs_release_file(struct kref *ref);
>>>> void ib_uverbs_free_async_event_file(struct ib_uverbs_file
>>>> *uverbs_file);
>>>> struct ib_uverbs_event_file *ib_uverbs_lookup_comp_file(int fd);
>>>>
>>>> diff --git a/drivers/infiniband/core/uverbs_main.c
>>>> b/drivers/infiniband/core/uverbs_main.c
>>>> index f783723..e63357a 100644
>>>> --- a/drivers/infiniband/core/uverbs_main.c
>>>> +++ b/drivers/infiniband/core/uverbs_main.c
>>>> @@ -341,7 +341,7 @@ static void ib_uverbs_comp_dev(struct
>>>> ib_uverbs_device *dev)
>>>> complete(&dev->comp);
>>>> }
>>>>
>>>> -static void ib_uverbs_release_file(struct kref *ref)
>>>> +void ib_uverbs_release_file(struct kref *ref)
>>>> {
>>>> struct ib_uverbs_file *file =
>>>> container_of(ref, struct ib_uverbs_file, ref);
>>>> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
>>>> index b5d2075..7240615 100644
>>>> --- a/include/rdma/ib_verbs.h
>>>> +++ b/include/rdma/ib_verbs.h
>>>> @@ -1329,8 +1329,11 @@ struct ib_fmr_attr {
>>>>
>>>> struct ib_umem;
>>>>
>>>> +struct ib_ucontext_lock;
>>>> +
>>>> struct ib_ucontext {
>>>> struct ib_device *device;
>>>> + struct ib_uverbs_file *ufile;
>>>> struct list_head pd_list;
>>>> struct list_head mr_list;
>>>> struct list_head mw_list;
>>>> @@ -1344,6 +1347,10 @@ struct ib_ucontext {
>>>> struct list_head rwq_ind_tbl_list;
>>>> int closing;
>>>>
>>>> + /* lock for uobjects list */
>>>> + struct ib_ucontext_lock *uobjects_lock;
>>>> + struct list_head uobjects;
>>>> +
>>>> struct pid *tgid;
>>>> #ifdef CONFIG_INFINIBAND_ON_DEMAND_PAGING
>>>> struct rb_root umem_tree;
>>>> @@ -1363,16 +1370,28 @@ struct ib_ucontext {
>>>> #endif
>>>> };
>>>>
>>>> +struct uverbs_object_list;
>>>> +
>>>> +#define OLD_ABI_COMPAT
>>>> +
>>>> struct ib_uobject {
>>>> u64 user_handle; /* handle given to us
>> by userspace
>>>> */
>>>> struct ib_ucontext *context; /* associated user
>> context
>>>> */
>>>> void *object; /* containing object
>> */
>>>> struct list_head list; /* link to context's
>> list */
>>>> - int id; /* index into kernel
>> idr */
>>>> - struct kref ref;
>>>> - struct rw_semaphore mutex; /* protects .live */
>>>> + int id; /* index into kernel
>> idr/fd */
>>>> +#ifdef OLD_ABI_COMPAT
>>>> + struct kref ref;
>>>> +#endif
>>>> + struct rw_semaphore usecnt; /* protects exclusive
>>>> access */
>>>> +#ifdef OLD_ABI_COMPAT
>>>> + struct rw_semaphore mutex; /* protects .live */
>>>> +#endif
>>>> struct rcu_head rcu; /* kfree_rcu()
>> overhead */
>>>> int live;
>>>> +
>>>> + const struct uverbs_type_alloc_action *type;
>>>> + struct ib_ucontext_lock *uobjects_lock;
>>>> };
>>>>
>>>> struct ib_udata {
>>>> @@ -2101,6 +2120,9 @@ struct ib_device {
>>>> */
>>>> int (*get_port_immutable)(struct ib_device *, u8, struct
>>>> ib_port_immutable *);
>>>> void (*get_dev_fw_str)(struct ib_device *, char *str, size_t
>>>> str_len);
>>>> + struct list_head type_list;
>>>> +
>>>> + const struct uverbs_types_group *types_group;
>>>> };
>>>>
>>>> struct ib_client {
>>>> diff --git a/include/rdma/uverbs_ioctl.h
>> b/include/rdma/uverbs_ioctl.h
>>>> new file mode 100644
>>>> index 0000000..2f50045
>>>> --- /dev/null
>>>> +++ b/include/rdma/uverbs_ioctl.h
>>>> @@ -0,0 +1,195 @@
>>>> +/*
>>>> + * Copyright (c) 2016, Mellanox Technologies inc. All rights
>>>> reserved.
>>>> + *
>>>> + * This software is available to you under a choice of one of two
>>>> + * licenses. You may choose to be licensed under the terms of the
>> GNU
>>>> + * General Public License (GPL) Version 2, available from the file
>>>> + * COPYING in the main directory of this source tree, or the
>>>> + * OpenIB.org BSD license below:
>>>> + *
>>>> + * Redistribution and use in source and binary forms, with or
>>>> + * without modification, are permitted provided that the
>> following
>>>> + * conditions are met:
>>>> + *
>>>> + * - Redistributions of source code must retain the above
>>>> + * copyright notice, this list of conditions and the
>> following
>>>> + * disclaimer.
>>>> + *
>>>> + * - Redistributions in binary form must reproduce the above
>>>> + * copyright notice, this list of conditions and the
>> following
>>>> + * disclaimer in the documentation and/or other materials
>>>> + * provided with the distribution.
>>>> + *
>>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>>>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
>> OF
>>>> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>>>> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
>> HOLDERS
>>>> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
>> AN
>>>> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR
>> IN
>>>> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
>>>> + * SOFTWARE.
>>>> + */
>>>> +
>>>> +#ifndef _UVERBS_IOCTL_
>>>> +#define _UVERBS_IOCTL_
>>>> +
>>>> +#include <linux/kernel.h>
>>>> +
>>>> +struct uverbs_object_type;
>>>> +struct ib_ucontext;
>>>> +struct ib_uobject;
>>>> +struct ib_device;
>>>> +struct uverbs_uobject_type;
>>>> +
>>>> +/*
>>>> + * =======================================
>>>> + * Verbs action specifications
>>>> + * =======================================
>>>> + */
>>>
>>> I intentionally used urdma (though condensed to 3 letters that I
>> don't recall atm), rather than uverbs. This will need to work with
>> non-verbs devices and interfaces -- again, consider how this fits with
>> the rdma cm. Verbs has a very specific meaning, which gets lost if we
>> start referring to everything as 'verbs'. It's bad enough that we're
>> stuck with 'drivers/infiniband' and 'rdma', such that 'infiniband' also
>> means ethernet and rdma means nothing.
>>>
>>
>> IMHO - let's agree on the concept of this infrastructure. One we
>> decide its scope, we could generalize it (i.e - ioctl_provider and
>> ioctl_context) and implement it to rdma-cm as well.
>>
>>>> +
>>>> +enum uverbs_attr_type {
>>>> + UVERBS_ATTR_TYPE_PTR_IN,
>>>> + UVERBS_ATTR_TYPE_PTR_OUT,
>>>> + UVERBS_ATTR_TYPE_IDR,
>>>> + UVERBS_ATTR_TYPE_FD,
>>>> +};
>>>> +
>>>> +enum uverbs_idr_access {
>>>> + UVERBS_IDR_ACCESS_READ,
>>>> + UVERBS_IDR_ACCESS_WRITE,
>>>> + UVERBS_IDR_ACCESS_NEW,
>>>> + UVERBS_IDR_ACCESS_DESTROY
>>>> +};
>>>> +
>>>> +struct uverbs_attr_spec {
>>>> + u16 len;
>>>> + enum uverbs_attr_type type;
>>>> + struct {
>>>> + u16 obj_type;
>>>> + u8 access;
>>>
>>> Is access intended to be an enum uverbs_idr_access value?
>>>
>>
>> Yeah, worth using this enum. Thanks.
>>
>>>> + } obj;
>>>
>>> I would remove (flatten) the substructure and re-order the fields for
>> better alignment.
>>>
>>
>> I noticed there are several places which aren't aliged. It's in my todo
>> list.
>>
>>>> +};
>>>> +
>>>> +struct uverbs_attr_group_spec {
>>>> + struct uverbs_attr_spec *attrs;
>>>> + size_t num_attrs;
>>>> +};
>>>> +
>>>> +struct uverbs_action_spec {
>>>> + const struct uverbs_attr_group_spec **attr_groups;
>>>> + /* if > 0 -> validator, otherwise, error */
>>>
>>> ? not sure what this comment means
>>>
>>>> + int (*dist)(__u16 *attr_id, void *priv);
>>>
>>> What does 'dist' stand for?
>>>
>>
>> dist = distribution function.
>> It maps the attributes you got from the user-space to your groups. You
>> could think of each group as a namespace - where its attributes (or
>> types/actions) starts from zero in the sake of compactness.
>> So, for example, it gets an attribute 0x8010 and maps to to "group 1"
>> (provider) and attribute 0x10.
>>
>>>> + void *priv;
>>>> + size_t num_groups;
>>>> +};
>>>> +
>>>> +struct uverbs_attr_array;
>>>> +struct ib_uverbs_file;
>>>> +
>>>> +struct uverbs_action {
>>>> + struct uverbs_action_spec spec;
>>>> + void *priv;
>>>> + int (*handler)(struct ib_device *ib_dev, struct ib_uverbs_file
>>>> *ufile,
>>>> + struct uverbs_attr_array *ctx, size_t num, void
>>>> *priv);
>>>> +};
>>>> +
>>>> +struct uverbs_type_alloc_action;
>>>> +typedef void (*free_type)(const struct uverbs_type_alloc_action
>>>> *uobject_type,
>>>> + struct ib_uobject *uobject);
>>>> +
>>>> +struct uverbs_type_alloc_action {
>>>> + enum uverbs_attr_type type;
>>>> + int order;
>>>
>>> I think this is being used as destroy order, in which case I would
>> rename it to clarify the intent. Though I'd prefer we come up with a
>> more efficient destruction mechanism than the repeated nested looping.
>>>
>>
>> In one of the earlier revisions I used a sorted list, which was
>> efficient. I recall that Jason didn't like its complexity and
>> re-thinking about that - he's right. Most of your types are "order
>> number" 0 anyway. So you'll probably iterate very few objects in the
>> next round (in verbs, everything but MRs and PDs).
>>
>>>> + size_t obj_size;
>>>
>>> This can be alloc_fn
>>>
>>>> + free_type free_fn;
>>>> + struct {
>>>> + const struct file_operations *fops;
>>>> + const char *name;
>>>> + int flags;
>>>> + } fd;
>>>> +};
>>>> +
>>>> +struct uverbs_type_actions_group {
>>>> + size_t num_actions;
>>>> + const struct uverbs_action **actions;
>>>> +};
>>>> +
>>>> +struct uverbs_type {
>>>> + size_t num_groups;
>>>> + const struct uverbs_type_actions_group **action_groups;
>>>> + const struct uverbs_type_alloc_action *alloc;
>>>> + int (*dist)(__u16 *action_id, void *priv);
>>>> + void *priv;
>>>> +};
>>>> +
>>>> +struct uverbs_types {
>>>> + size_t num_types;
>>>> + const struct uverbs_type **types;
>>>> +};
>>>> +
>>>> +struct uverbs_types_group {
>>>> + const struct uverbs_types **type_groups;
>>>> + size_t num_groups;
>>>> + int (*dist)(__u16 *type_id, void *priv);
>>>> + void *priv;
>>>> +};
>>>> +
>>>> +/* =================================================
>>>> + * Parsing infrastructure
>>>> + * =================================================
>>>> + */
>>>> +
>>>> +struct uverbs_ptr_attr {
>>>> + void * __user ptr;
>>>> + __u16 len;
>>>> +};
>>>> +
>>>> +struct uverbs_fd_attr {
>>>> + int fd;
>>>> +};
>>>> +
>>>> +struct uverbs_uobj_attr {
>>>> + /* idr handle */
>>>> + __u32 idr;
>>>> +};
>>>> +
>>>> +struct uverbs_obj_attr {
>>>> + /* pointer to the kernel descriptor -> type, access, etc */
>>>> + const struct uverbs_attr_spec *val;
>>>> + struct ib_uverbs_attr __user *uattr;
>>>> + const struct uverbs_type_alloc_action *type;
>>>> + struct ib_uobject *uobject;
>>>> + union {
>>>> + struct uverbs_fd_attr fd;
>>>> + struct uverbs_uobj_attr uobj;
>>>> + };
>>>> +};
>>>> +
>>>> +struct uverbs_attr {
>>>> + bool valid; > >> + union {
>>>> + struct uverbs_ptr_attr cmd_attr;
>>>> + struct uverbs_obj_attr obj_attr;
>>>> + };
>>>> +};
>>>
>>> It's odd to have a union that's part of a structure without some
>> field to indicate which union field is accessible.
>>>
>>
>> You index this array but the attribute id from the user's callback
>> funciton. The user should know what's the type of the attribute, as
>> [s]he declared the specification.
>>
>>>> +
>>>> +/* output of one validator */
>>>> +struct uverbs_attr_array {
>>>> + size_t num_attrs;
>>>> + /* arrays of attrubytes, index is the id i.e SEND_CQ */
>>>> + struct uverbs_attr *attrs;
>>>> +};
>>>> +
>>>> +/* =================================================
>>>> + * Types infrastructure
>>>> + * =================================================
>>>> + */
>>>> +
>>>> +int ib_uverbs_uobject_type_add(struct list_head *head,
>>>> + void (*free)(struct uverbs_uobject_type
>> *type,
>>>> + struct ib_uobject
>> *uobject,
>>>> + struct ib_ucontext
>> *ucontext),
>>>> + uint16_t obj_type);
>>>> +void ib_uverbs_uobject_types_remove(struct ib_device *ib_dev);
>>>> +
>>>> +#endif
>>>> --
>>>> 2.7.4
>
> Matan, please re-look at the architecture that I proposed:
>
> https://patchwork.kernel.org/patch/9178991/
>
> including the terminology (and consider using common OOP terms). The *core* of the ioctl framework is to simply invoke a function dispatch table. IMO, that's where we should start. Anything beyond that is extra that we should have a strong reason for including. (Yes, I think we need more.) Starting simple and adding necessary functionality should let us get something upstream quicker and re-use more of the existing code.
Actually, I've looked at your proposal. Some of the ideas are blend
between your proposal and my original proposal.
One of our goals is to get rid of the commonalities between handlers and
push the validation and locking to a common code which could validated
once to be correct. It tends to be a lot easier (and correct) than to
re-examine every function call.
>
> If we're going to re-create netlink as part of the rdma ioctl interface, then why don't we just use netlink directly?
>
We had a netlink based proposal first, but:
(a) We want one call dispatching (as opposed to send-receive).
(b) We don't want to copy the driver's data from the user-space to the
libibverbs buffer.
(c) We don't need the complexity of nesting.
(d) Bare netlink is considered unreliable.
(e) We need semantics of objects.
(f) By using pointers, we could eliminate some copies.
Thanks for looking at this patch.
Matan
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH rdma-rc] IB/IPoIB: Remove can't use GFP_NOIO warning
From: Yuval Shaia @ 2016-11-10 8:33 UTC (permalink / raw)
To: Leon Romanovsky
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, Kamal Heib
In-Reply-To: <1478765808-12517-1-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Reviewed-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
On Thu, Nov 10, 2016 at 10:16:48AM +0200, Leon Romanovsky wrote:
> From: Kamal Heib <kamalh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
> Remove the warning print of "can't use of GFP_NOIO" to avoid prints in
> each QP creation when devices aren't supporting IB_QP_CREATE_USE_GFP_NOIO.
>
> This print become more annoying when the IPoIB interface is configured
> to work in connected mode.
>
> Fixes: 09b93088d750 ('IB: Add a QP creation flag to use GFP_NOIO allocations')
> Signed-off-by: Kamal Heib <kamalh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
> drivers/infiniband/ulp/ipoib/ipoib_cm.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_cm.c b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
> index 4ad297d..50c9772 100644
> --- a/drivers/infiniband/ulp/ipoib/ipoib_cm.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
> @@ -1053,8 +1053,6 @@ static struct ib_qp *ipoib_cm_create_tx_qp(struct net_device *dev, struct ipoib_
>
> tx_qp = ib_create_qp(priv->pd, &attr);
> if (PTR_ERR(tx_qp) == -EINVAL) {
> - ipoib_warn(priv, "can't use GFP_NOIO for QPs on device %s, using GFP_KERNEL\n",
> - priv->ca->name);
> attr.create_flags &= ~IB_QP_CREATE_USE_GFP_NOIO;
> tx_qp = ib_create_qp(priv->pd, &attr);
> }
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH rdma-rc V1 0/9] mlx4 fixes for 4.9
From: Leon Romanovsky @ 2016-11-10 9:30 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Hi Doug,
Please find below second version of fixes for mlx4 driver.
This patchset was generated against v4.9-rc3.
Available in the "topic/mlx4-fixes-4.9" topic branch of this git repo:
git://git.kernel.org/pub/scm/linux/kernel/git/leon/linux-rdma.git
Or for browsing:
https://git.kernel.org/cgit/linux/kernel/git/leon/linux-rdma.git/log/?h=topic/mlx4-fixes-4.9
Changes v0 -> v1:
Patch 1:
* No change
Patch 2:
* Add Yuval's ROB tag
Patch 3:
* Remove function name from title
Patch 4:
* No change
Patch 5:
* Use the port subnet prefix instead of default
Patch 6:
* Add Yuval's ROB tag
Patch 7:
* No change
Patch 8:
* Remove function name from title
* Reword commit message
Patch 9:
* No change
Thanks,
Daniel and Leon.
Daniel Jurgens (1):
IB/mlx4: Check gid_index return value
Eran Ben Elisha (2):
IB/mlx4: Fail to allocate NET_IF QPs when DMFS for IPoIB in
unavailable
IB/mlx4: Check if GRH is available before using it in modify QP
Jack Morgenstein (1):
IB/mlx4: Handle well-known-gid in mad_demux processing
Maor Gottlieb (2):
IB/mlx4: Set traffic class in AH
IB/mlx4: Put non zero value in max_ah device attribute
Matan Barak (1):
IB/mlx4: Fix create CQ error flow
Moni Shoua (1):
IB/mlx4: Handle IPv4 header when demultiplexing MAD
Saeed Mahameed (1):
IB/mlx4: Fix port query for 56Gb Ethernet links
drivers/infiniband/core/verbs.c | 16 +++++++------
drivers/infiniband/hw/mlx4/ah.c | 11 ++++++---
drivers/infiniband/hw/mlx4/cq.c | 5 +++-
drivers/infiniband/hw/mlx4/mad.c | 49 +++++++++++++++++++++++++++++++++------
drivers/infiniband/hw/mlx4/main.c | 30 +++++++++++++++---------
drivers/infiniband/hw/mlx4/qp.c | 4 ++--
include/rdma/ib_verbs.h | 19 +++++++++++++++
7 files changed, 103 insertions(+), 31 deletions(-)
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH rdma-rc V1 1/9] IB/mlx4: Set traffic class in AH
From: Leon Romanovsky @ 2016-11-10 9:30 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Maor Gottlieb, Daniel Jurgens
In-Reply-To: <1478770261-5775-1-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
From: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Set traffic class within sl_tclass_flowlabel when create iboe AH.
Without this the TOS value will be empty when running VLAN tagged
traffic, because the TOS value is taken from the traffic class in the
address handle attributes.
Fixes: 9106c4106974 ('IB/mlx4: Fix SL to 802.1Q priority-bits mapping for IBoE')
Signed-off-by: Maor Gottlieb <maorg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
drivers/infiniband/hw/mlx4/ah.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/hw/mlx4/ah.c b/drivers/infiniband/hw/mlx4/ah.c
index 5fc6233..6be7dc3 100644
--- a/drivers/infiniband/hw/mlx4/ah.c
+++ b/drivers/infiniband/hw/mlx4/ah.c
@@ -111,7 +111,9 @@ static struct ib_ah *create_iboe_ah(struct ib_pd *pd, struct ib_ah_attr *ah_attr
!(1 << ah->av.eth.stat_rate & dev->caps.stat_rate_support))
--ah->av.eth.stat_rate;
}
-
+ ah->av.eth.sl_tclass_flowlabel |=
+ cpu_to_be32((ah_attr->grh.traffic_class << 20) |
+ ah_attr->grh.flow_label);
/*
* HW requires multicast LID so we just choose one.
*/
@@ -119,7 +121,7 @@ static struct ib_ah *create_iboe_ah(struct ib_pd *pd, struct ib_ah_attr *ah_attr
ah->av.ib.dlid = cpu_to_be16(0xc000);
memcpy(ah->av.eth.dgid, ah_attr->grh.dgid.raw, 16);
- ah->av.eth.sl_tclass_flowlabel = cpu_to_be32(ah_attr->sl << 29);
+ ah->av.eth.sl_tclass_flowlabel |= cpu_to_be32(ah_attr->sl << 29);
return &ah->ibah;
}
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH rdma-rc V1 2/9] IB/mlx4: Check gid_index return value
From: Leon Romanovsky @ 2016-11-10 9:30 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Daniel Jurgens
In-Reply-To: <1478770261-5775-1-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
From: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Check the returned GID index value and return an error if it is invalid.
Fixes: 5070cd2239bd ('IB/mlx4: Replace mechanism for RoCE GID management')
Signed-off-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
drivers/infiniband/hw/mlx4/ah.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/mlx4/ah.c b/drivers/infiniband/hw/mlx4/ah.c
index 6be7dc3..8dfc76f 100644
--- a/drivers/infiniband/hw/mlx4/ah.c
+++ b/drivers/infiniband/hw/mlx4/ah.c
@@ -102,7 +102,10 @@ static struct ib_ah *create_iboe_ah(struct ib_pd *pd, struct ib_ah_attr *ah_attr
if (vlan_tag < 0x1000)
vlan_tag |= (ah_attr->sl & 7) << 13;
ah->av.eth.port_pd = cpu_to_be32(to_mpd(pd)->pdn | (ah_attr->port_num << 24));
- ah->av.eth.gid_index = mlx4_ib_gid_index_to_real_index(ibdev, ah_attr->port_num, ah_attr->grh.sgid_index);
+ ret = mlx4_ib_gid_index_to_real_index(ibdev, ah_attr->port_num, ah_attr->grh.sgid_index);
+ if (ret < 0)
+ return ERR_PTR(ret);
+ ah->av.eth.gid_index = ret;
ah->av.eth.vlan = cpu_to_be16(vlan_tag);
ah->av.eth.hop_limit = ah_attr->grh.hop_limit;
if (ah_attr->static_rate) {
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH rdma-rc V1 3/9] IB/mlx4: Fix create CQ error flow
From: Leon Romanovsky @ 2016-11-10 9:30 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Matan Barak, Daniel Jurgens
In-Reply-To: <1478770261-5775-1-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
From: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Currently, if ib_copy_to_udata fails, the CQ
won't be deleted from the radix tree and the HW (HW2SW).
Fixes: 225c7b1feef1 ('IB/mlx4: Add a driver Mellanox ConnectX InfiniBand adapters')
Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
drivers/infiniband/hw/mlx4/cq.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/mlx4/cq.c b/drivers/infiniband/hw/mlx4/cq.c
index 1ea686b..6a0fec3 100644
--- a/drivers/infiniband/hw/mlx4/cq.c
+++ b/drivers/infiniband/hw/mlx4/cq.c
@@ -253,11 +253,14 @@ struct ib_cq *mlx4_ib_create_cq(struct ib_device *ibdev,
if (context)
if (ib_copy_to_udata(udata, &cq->mcq.cqn, sizeof (__u32))) {
err = -EFAULT;
- goto err_dbmap;
+ goto err_cq_free;
}
return &cq->ibcq;
+err_cq_free:
+ mlx4_cq_free(dev->dev, &cq->mcq);
+
err_dbmap:
if (context)
mlx4_ib_db_unmap_user(to_mucontext(context), &cq->db);
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH rdma-rc V1 4/9] IB/mlx4: Handle IPv4 header when demultiplexing MAD
From: Leon Romanovsky @ 2016-11-10 9:30 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Moni Shoua, Daniel Jurgens
In-Reply-To: <1478770261-5775-1-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
From: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
When MAD arrives to the hypervisor, we need to identify which slave it
should be sent by destination GID. When L3 protocol is IPv4 the
GRH is replaced by an IPv4 header. This patch detects when IPv4 header
needs to be parsed instead of GRH.
Fixes: b6ffaeffaea4 ('mlx4: In RoCE allow guests to have multiple GIDS')
Signed-off-by: Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
drivers/infiniband/core/verbs.c | 16 +++++++++-------
drivers/infiniband/hw/mlx4/mad.c | 33 ++++++++++++++++++++++++++++++---
include/rdma/ib_verbs.h | 19 +++++++++++++++++++
3 files changed, 58 insertions(+), 10 deletions(-)
diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index 8368764..b3a915f 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -328,7 +328,7 @@ struct ib_ah *ib_create_ah(struct ib_pd *pd, struct ib_ah_attr *ah_attr)
}
EXPORT_SYMBOL(ib_create_ah);
-static int ib_get_header_version(const union rdma_network_hdr *hdr)
+int ib_get_rdma_header_version(const union rdma_network_hdr *hdr)
{
const struct iphdr *ip4h = (struct iphdr *)&hdr->roce4grh;
struct iphdr ip4h_checked;
@@ -359,6 +359,7 @@ static int ib_get_header_version(const union rdma_network_hdr *hdr)
return 4;
return 6;
}
+EXPORT_SYMBOL(ib_get_rdma_header_version);
static enum rdma_network_type ib_get_net_type_by_grh(struct ib_device *device,
u8 port_num,
@@ -369,7 +370,7 @@ static enum rdma_network_type ib_get_net_type_by_grh(struct ib_device *device,
if (rdma_protocol_ib(device, port_num))
return RDMA_NETWORK_IB;
- grh_version = ib_get_header_version((union rdma_network_hdr *)grh);
+ grh_version = ib_get_rdma_header_version((union rdma_network_hdr *)grh);
if (grh_version == 4)
return RDMA_NETWORK_IPV4;
@@ -415,9 +416,9 @@ static int get_sgid_index_from_eth(struct ib_device *device, u8 port_num,
&context, gid_index);
}
-static int get_gids_from_rdma_hdr(union rdma_network_hdr *hdr,
- enum rdma_network_type net_type,
- union ib_gid *sgid, union ib_gid *dgid)
+int ib_get_gids_from_rdma_hdr(const union rdma_network_hdr *hdr,
+ enum rdma_network_type net_type,
+ union ib_gid *sgid, union ib_gid *dgid)
{
struct sockaddr_in src_in;
struct sockaddr_in dst_in;
@@ -447,6 +448,7 @@ static int get_gids_from_rdma_hdr(union rdma_network_hdr *hdr,
return -EINVAL;
}
}
+EXPORT_SYMBOL(ib_get_gids_from_rdma_hdr);
int ib_init_ah_from_wc(struct ib_device *device, u8 port_num,
const struct ib_wc *wc, const struct ib_grh *grh,
@@ -469,8 +471,8 @@ int ib_init_ah_from_wc(struct ib_device *device, u8 port_num,
net_type = ib_get_net_type_by_grh(device, port_num, grh);
gid_type = ib_network_to_gid_type(net_type);
}
- ret = get_gids_from_rdma_hdr((union rdma_network_hdr *)grh, net_type,
- &sgid, &dgid);
+ ret = ib_get_gids_from_rdma_hdr((union rdma_network_hdr *)grh, net_type,
+ &sgid, &dgid);
if (ret)
return ret;
diff --git a/drivers/infiniband/hw/mlx4/mad.c b/drivers/infiniband/hw/mlx4/mad.c
index 1672907..b8e9013 100644
--- a/drivers/infiniband/hw/mlx4/mad.c
+++ b/drivers/infiniband/hw/mlx4/mad.c
@@ -39,6 +39,8 @@
#include <linux/mlx4/cmd.h>
#include <linux/gfp.h>
#include <rdma/ib_pma.h>
+#include <linux/ip.h>
+#include <net/ipv6.h>
#include <linux/mlx4/driver.h>
#include "mlx4_ib.h"
@@ -480,6 +482,23 @@ static int find_slave_port_pkey_ix(struct mlx4_ib_dev *dev, int slave,
return -EINVAL;
}
+static int get_gids_from_l3_hdr(struct ib_grh *grh, union ib_gid *sgid,
+ union ib_gid *dgid)
+{
+ int version = ib_get_rdma_header_version((const union rdma_network_hdr *)grh);
+ enum rdma_network_type net_type;
+
+ if (version == 4)
+ net_type = RDMA_NETWORK_IPV4;
+ else if (version == 6)
+ net_type = RDMA_NETWORK_IPV6;
+ else
+ return -EINVAL;
+
+ return ib_get_gids_from_rdma_hdr((union rdma_network_hdr *)grh, net_type,
+ sgid, dgid);
+}
+
int mlx4_ib_send_to_slave(struct mlx4_ib_dev *dev, int slave, u8 port,
enum ib_qp_type dest_qpt, struct ib_wc *wc,
struct ib_grh *grh, struct ib_mad *mad)
@@ -538,7 +557,10 @@ int mlx4_ib_send_to_slave(struct mlx4_ib_dev *dev, int slave, u8 port,
memset(&attr, 0, sizeof attr);
attr.port_num = port;
if (is_eth) {
- memcpy(&attr.grh.dgid.raw[0], &grh->dgid.raw[0], 16);
+ union ib_gid sgid;
+
+ if (get_gids_from_l3_hdr(grh, &sgid, &attr.grh.dgid))
+ return -EINVAL;
attr.ah_flags = IB_AH_GRH;
}
ah = ib_create_ah(tun_ctx->pd, &attr);
@@ -651,6 +673,11 @@ static int mlx4_ib_demux_mad(struct ib_device *ibdev, u8 port,
is_eth = 1;
if (is_eth) {
+ union ib_gid dgid;
+ union ib_gid sgid;
+
+ if (get_gids_from_l3_hdr(grh, &sgid, &dgid))
+ return -EINVAL;
if (!(wc->wc_flags & IB_WC_GRH)) {
mlx4_ib_warn(ibdev, "RoCE grh not present.\n");
return -EINVAL;
@@ -659,10 +686,10 @@ static int mlx4_ib_demux_mad(struct ib_device *ibdev, u8 port,
mlx4_ib_warn(ibdev, "RoCE mgmt class is not CM\n");
return -EINVAL;
}
- err = mlx4_get_slave_from_roce_gid(dev->dev, port, grh->dgid.raw, &slave);
+ err = mlx4_get_slave_from_roce_gid(dev->dev, port, dgid.raw, &slave);
if (err && mlx4_is_mf_bonded(dev->dev)) {
other_port = (port == 1) ? 2 : 1;
- err = mlx4_get_slave_from_roce_gid(dev->dev, other_port, grh->dgid.raw, &slave);
+ err = mlx4_get_slave_from_roce_gid(dev->dev, other_port, dgid.raw, &slave);
if (!err) {
port = other_port;
pr_debug("resolved slave %d from gid %pI6 wire port %d other %d\n",
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 5ad43a4..467a4b4 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -2581,6 +2581,24 @@ void ib_dealloc_pd(struct ib_pd *pd);
struct ib_ah *ib_create_ah(struct ib_pd *pd, struct ib_ah_attr *ah_attr);
/**
+ * ib_get_gids_from_rdma_hdr - Get sgid and dgid from GRH or IPv4 header
+ * work completion.
+ * @hdr: the L3 header to parse
+ * @net_type: type of header to parse
+ * @sgid: place to store source gid
+ * @dgid: place to store destination gid
+ */
+int ib_get_gids_from_rdma_hdr(const union rdma_network_hdr *hdr,
+ enum rdma_network_type net_type,
+ union ib_gid *sgid, union ib_gid *dgid);
+
+/**
+ * ib_get_rdma_header_version - Get the header version
+ * @hdr: the L3 header to parse
+ */
+int ib_get_rdma_header_version(const union rdma_network_hdr *hdr);
+
+/**
* ib_init_ah_from_wc - Initializes address handle attributes from a
* work completion.
* @device: Device on which the received message arrived.
@@ -3357,4 +3375,5 @@ int ib_sg_to_pages(struct ib_mr *mr, struct scatterlist *sgl, int sg_nents,
void ib_drain_rq(struct ib_qp *qp);
void ib_drain_sq(struct ib_qp *qp);
void ib_drain_qp(struct ib_qp *qp);
+
#endif /* IB_VERBS_H */
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH rdma-rc V1 5/9] IB/mlx4: Handle well-known-gid in mad_demux processing
From: Leon Romanovsky @ 2016-11-10 9:30 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Jack Morgenstein,
Daniel Jurgens
In-Reply-To: <1478770261-5775-1-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
From: Jack Morgenstein <jackm-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
If OpenSM runs over a ConnectX-3, and there are ConnectX-4 or Connect-IB
VFs active on the network, the OpenSM will receive QP1 packets containing
a GRH where the destination GID is the "Well-Known GID" -- which is not a
GID in the HCA Port's GID Table.
This GID must be tested-for separately -- and packets which contain
this destination GID should be routed to slave 0 (the PF).
Fixes: 37bfc7c1e83f ('IB/mlx4: SR-IOV multiplex and demultiplex MADs')
Signed-off-by: Jack Morgenstein <jackm-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
Signed-off-by: Daniel Jurgens <danielj-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
drivers/infiniband/hw/mlx4/mad.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/infiniband/hw/mlx4/mad.c b/drivers/infiniband/hw/mlx4/mad.c
index b8e9013..7bcae41 100644
--- a/drivers/infiniband/hw/mlx4/mad.c
+++ b/drivers/infiniband/hw/mlx4/mad.c
@@ -729,10 +729,18 @@ static int mlx4_ib_demux_mad(struct ib_device *ibdev, u8 port,
/* If a grh is present, we demux according to it */
if (wc->wc_flags & IB_WC_GRH) {
- slave = mlx4_ib_find_real_gid(ibdev, port, grh->dgid.global.interface_id);
- if (slave < 0) {
- mlx4_ib_warn(ibdev, "failed matching grh\n");
- return -ENOENT;
+ if (grh->dgid.global.interface_id ==
+ cpu_to_be64(IB_SA_WELL_KNOWN_GUID) &&
+ grh->dgid.global.subnet_prefix == cpu_to_be64(
+ atomic64_read(&dev->sriov.demux[port - 1].subnet_prefix))) {
+ slave = 0;
+ } else {
+ slave = mlx4_ib_find_real_gid(ibdev, port,
+ grh->dgid.global.interface_id);
+ if (slave < 0) {
+ mlx4_ib_warn(ibdev, "failed matching grh\n");
+ return -ENOENT;
+ }
}
}
/* Class-specific handling */
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox