* [PATCH V2 for-next 03/19] IB/hns: Fix bug of using uninit refcount and free
From: Salil Mehta @ 2016-09-20 16:06 UTC (permalink / raw)
To: dledford
Cc: salil.mehta, xavier.huwei, oulijun, yisen.zhuang, mehta.salil.lnk,
xuwei5, linux-rdma, linux-kernel, linuxarm, Dongdong Huang
In-Reply-To: <20160920160712.1190216-1-salil.mehta@huawei.com>
From: Lijun Ou <oulijun@huawei.com>
In current version, it uses uninitialized parameters named
refcount and free in hns_roce_cq_event.
This patch initializes these parameter in cq alloc and add
correspond process in cq free.
Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Dongdong Huang(Donald) <hdd.huang@huawei.com>
Reviewed-by: Wei Hu <xavier.huwei@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
drivers/infiniband/hw/hns/hns_roce_cq.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/infiniband/hw/hns/hns_roce_cq.c b/drivers/infiniband/hw/hns/hns_roce_cq.c
index a52306f..3095f06 100644
--- a/drivers/infiniband/hw/hns/hns_roce_cq.c
+++ b/drivers/infiniband/hw/hns/hns_roce_cq.c
@@ -152,6 +152,9 @@ static int hns_roce_cq_alloc(struct hns_roce_dev *hr_dev, int nent,
hr_cq->cons_index = 0;
hr_cq->uar = hr_uar;
+ atomic_set(&hr_cq->refcount, 1);
+ init_completion(&hr_cq->free);
+
return 0;
err_radix:
@@ -191,6 +194,11 @@ static void hns_roce_free_cq(struct hns_roce_dev *hr_dev,
/* Waiting interrupt process procedure carried out */
synchronize_irq(hr_dev->eq_table.eq[hr_cq->vector].irq);
+ /* wait for all interrupt processed */
+ if (atomic_dec_and_test(&hr_cq->refcount))
+ complete(&hr_cq->free);
+ wait_for_completion(&hr_cq->free);
+
spin_lock_irq(&cq_table->lock);
radix_tree_delete(&cq_table->tree, hr_cq->cqn);
spin_unlock_irq(&cq_table->lock);
--
1.9.1
^ permalink raw reply related
* [PATCH V2 for-next 02/19] IB/hns: Remove parameters of resize cq
From: Salil Mehta @ 2016-09-20 16:06 UTC (permalink / raw)
To: dledford
Cc: salil.mehta, xavier.huwei, oulijun, yisen.zhuang, mehta.salil.lnk,
xuwei5, linux-rdma, linux-kernel, linuxarm, Dongdong Huang
In-Reply-To: <20160920160712.1190216-1-salil.mehta@huawei.com>
From: Lijun Ou <oulijun@huawei.com>
In old version of RoCE, it doesn't support to resize cq.
So, we remove parameters related to resize cq.
Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Dongdong Huang(Donald) <hdd.huang@huawei.com>
Reviewed-by: Wei Hu <xavier.huwei@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
drivers/infiniband/hw/hns/hns_roce_cq.c | 3 ---
drivers/infiniband/hw/hns/hns_roce_device.h | 9 ---------
2 files changed, 12 deletions(-)
diff --git a/drivers/infiniband/hw/hns/hns_roce_cq.c b/drivers/infiniband/hw/hns/hns_roce_cq.c
index 10fd209..a52306f 100644
--- a/drivers/infiniband/hw/hns/hns_roce_cq.c
+++ b/drivers/infiniband/hw/hns/hns_roce_cq.c
@@ -299,10 +299,7 @@ struct ib_cq *hns_roce_ib_create_cq(struct ib_device *ib_dev,
cq_entries = roundup_pow_of_two((unsigned int)cq_entries);
hr_cq->ib_cq.cqe = cq_entries - 1;
- mutex_init(&hr_cq->resize_mutex);
spin_lock_init(&hr_cq->lock);
- hr_cq->hr_resize_buf = NULL;
- hr_cq->resize_umem = NULL;
if (context) {
if (ib_copy_from_udata(&ucmd, udata, sizeof(ucmd))) {
diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
index db974e4..15bc229 100644
--- a/drivers/infiniband/hw/hns/hns_roce_device.h
+++ b/drivers/infiniband/hw/hns/hns_roce_device.h
@@ -284,20 +284,11 @@ struct hns_roce_cq_buf {
struct hns_roce_mtt hr_mtt;
};
-struct hns_roce_cq_resize {
- struct hns_roce_cq_buf hr_buf;
- int cqe;
-};
-
struct hns_roce_cq {
struct ib_cq ib_cq;
struct hns_roce_cq_buf hr_buf;
- /* pointer to store information after resize*/
- struct hns_roce_cq_resize *hr_resize_buf;
spinlock_t lock;
- struct mutex resize_mutex;
struct ib_umem *umem;
- struct ib_umem *resize_umem;
void (*comp)(struct hns_roce_cq *);
void (*event)(struct hns_roce_cq *, enum hns_roce_event);
--
1.9.1
^ permalink raw reply related
* [PATCH V2 for-next 01/19] IB/hns: Remove unused parameters in some functions
From: Salil Mehta @ 2016-09-20 16:06 UTC (permalink / raw)
To: dledford
Cc: salil.mehta, xavier.huwei, oulijun, yisen.zhuang, mehta.salil.lnk,
xuwei5, linux-rdma, linux-kernel, linuxarm
In-Reply-To: <20160920160712.1190216-1-salil.mehta@huawei.com>
From: Lijun Ou <oulijun@huawei.com>
The parameter named collapsed unused in hns_roce_cq_alloc.
Also, parameter named doorbell_lock unsed in
hns_roce_v1_cq_set_ci. This patch optimize these parameters.
Signed-off-by: Lijun Ou <oulijun@huawei.com>
Reviewed-by: Wei Hu <xavier.huwei@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
drivers/infiniband/hw/hns/hns_roce_cq.c | 7 +++----
drivers/infiniband/hw/hns/hns_roce_device.h | 1 -
drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 13 ++++---------
drivers/infiniband/hw/hns/hns_roce_main.c | 1 -
4 files changed, 7 insertions(+), 15 deletions(-)
diff --git a/drivers/infiniband/hw/hns/hns_roce_cq.c b/drivers/infiniband/hw/hns/hns_roce_cq.c
index 875597b..10fd209 100644
--- a/drivers/infiniband/hw/hns/hns_roce_cq.c
+++ b/drivers/infiniband/hw/hns/hns_roce_cq.c
@@ -83,8 +83,7 @@ static int hns_roce_sw2hw_cq(struct hns_roce_dev *dev,
static int hns_roce_cq_alloc(struct hns_roce_dev *hr_dev, int nent,
struct hns_roce_mtt *hr_mtt,
struct hns_roce_uar *hr_uar,
- struct hns_roce_cq *hr_cq, int vector,
- int collapsed)
+ struct hns_roce_cq *hr_cq, int vector)
{
struct hns_roce_cmd_mailbox *mailbox = NULL;
struct hns_roce_cq_table *cq_table = NULL;
@@ -338,8 +337,8 @@ struct ib_cq *hns_roce_ib_create_cq(struct ib_device *ib_dev,
}
/* Allocate cq index, fill cq_context */
- ret = hns_roce_cq_alloc(hr_dev, cq_entries, &hr_cq->hr_buf.hr_mtt,
- uar, hr_cq, vector, 0);
+ ret = hns_roce_cq_alloc(hr_dev, cq_entries, &hr_cq->hr_buf.hr_mtt, uar,
+ hr_cq, vector);
if (ret) {
dev_err(dev, "Creat CQ .Failed to cq_alloc.\n");
goto err_mtt;
diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
index 4d02bcf..db974e4 100644
--- a/drivers/infiniband/hw/hns/hns_roce_device.h
+++ b/drivers/infiniband/hw/hns/hns_roce_device.h
@@ -536,7 +536,6 @@ struct hns_roce_dev {
struct hns_roce_uar priv_uar;
const char *irq_names[HNS_ROCE_MAX_IRQ_NUM];
spinlock_t sm_lock;
- spinlock_t cq_db_lock;
spinlock_t bt_cmd_lock;
struct hns_roce_ib_iboe iboe;
diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
index 5ffdf7a..48c0862 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
@@ -1189,9 +1189,7 @@ static struct hns_roce_cqe *next_cqe_sw(struct hns_roce_cq *hr_cq)
return get_sw_cqe(hr_cq, hr_cq->cons_index);
}
-void hns_roce_v1_cq_set_ci(struct hns_roce_cq *hr_cq, u32 cons_index,
- spinlock_t *doorbell_lock)
-
+void hns_roce_v1_cq_set_ci(struct hns_roce_cq *hr_cq, u32 cons_index)
{
u32 doorbell[2];
@@ -1251,8 +1249,7 @@ static void __hns_roce_v1_cq_clean(struct hns_roce_cq *hr_cq, u32 qpn,
*/
wmb();
- hns_roce_v1_cq_set_ci(hr_cq, hr_cq->cons_index,
- &to_hr_dev(hr_cq->ib_cq.device)->cq_db_lock);
+ hns_roce_v1_cq_set_ci(hr_cq, hr_cq->cons_index);
}
}
@@ -1588,10 +1585,8 @@ int hns_roce_v1_poll_cq(struct ib_cq *ibcq, int num_entries, struct ib_wc *wc)
break;
}
- if (npolled) {
- hns_roce_v1_cq_set_ci(hr_cq, hr_cq->cons_index,
- &to_hr_dev(ibcq->device)->cq_db_lock);
- }
+ if (npolled)
+ hns_roce_v1_cq_set_ci(hr_cq, hr_cq->cons_index);
spin_unlock_irqrestore(&hr_cq->lock, flags);
diff --git a/drivers/infiniband/hw/hns/hns_roce_main.c b/drivers/infiniband/hw/hns/hns_roce_main.c
index a9960ba..4b44998 100644
--- a/drivers/infiniband/hw/hns/hns_roce_main.c
+++ b/drivers/infiniband/hw/hns/hns_roce_main.c
@@ -951,7 +951,6 @@ static int hns_roce_setup_hca(struct hns_roce_dev *hr_dev)
struct device *dev = &hr_dev->pdev->dev;
spin_lock_init(&hr_dev->sm_lock);
- spin_lock_init(&hr_dev->cq_db_lock);
spin_lock_init(&hr_dev->bt_cmd_lock);
ret = hns_roce_init_uar_table(hr_dev);
--
1.9.1
^ permalink raw reply related
* [PATCH V2 for-next 00/19] IB/hns: Fixes for various misc. bugs
From: Salil Mehta @ 2016-09-20 16:06 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: salil.mehta-hv44wF8Li93QT0dZR+AlfA,
xavier.huwei-hv44wF8Li93QT0dZR+AlfA,
oulijun-hv44wF8Li93QT0dZR+AlfA,
yisen.zhuang-hv44wF8Li93QT0dZR+AlfA,
mehta.salil.lnk-Re5JQEeQqe8AvxtiuMwx3w,
xuwei5-C8/M+/jPZTeaMJb+Lgu22Q, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linuxarm-hv44wF8Li93QT0dZR+AlfA
This patch-set introduces fixes for various bugs and potential
problems found during internal review and testing phases of
the HNS RoCE driver.
NOTE: These fixes have been done over already floated CM
patch: https://lkml.org/lkml/2016/9/1/609
All fixes have been done & tested by Oulijun and Huwei.
V1->V2: * PATCH 3/20 and 13/20 have been dropped after rework
of comments by Leon Romanovsky on V1 patch.
* New patch "Fix for removal of redundant code" has
been added to remove the redundant code.
Lijun Ou (15):
IB/hns: Remove unused parameters in some functions
IB/hns: Remove parameters of resize cq
IB/hns: Fix bug of using uninit refcount and free
IB/hns: Simplify function of pd alloc and qp alloc
IB/hns: Remove unused parameter named qp_type
IB/hns: Modify the init of iboe lock
IB/hns: Fix bug of memory leakage for registering user mr
IB/hns: Return bad wr while post send failed
IB/hns: The Ack timeout need a lower limit value
IB/hns: Some items of qpc need to take user param
IB/hns: Validate mtu when modified qp
IB/hns: Cq has not been freed
IB/hns: Update the rq head when modify qp state
IB/hns: Fix the bug when platform_get_resource() exec fail
IB/hns: Delete the redundant lines in hns_roce_v1_m_qp()
Salil Mehta (1):
IB/hns: Fix for removal of redundant code
Wei Hu (Xavier) (3):
IB/hns: Fix bug of clear hem
IB/hns: Delete the sqp_start from the structure hns_roce_caps
IB/hns: Optimize code of aeq and ceq interrupt handle and fix the bug
of qpn
drivers/infiniband/hw/hns/hns_roce_cq.c | 23 +--
drivers/infiniband/hw/hns/hns_roce_device.h | 13 +-
drivers/infiniband/hw/hns/hns_roce_eq.c | 146 ++++++++++---------
drivers/infiniband/hw/hns/hns_roce_eq.h | 4 +
drivers/infiniband/hw/hns/hns_roce_hem.c | 76 +---------
drivers/infiniband/hw/hns/hns_roce_hem.h | 4 +
drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 211 +++++++++++++++++++++++-----
drivers/infiniband/hw/hns/hns_roce_hw_v1.h | 9 ++
drivers/infiniband/hw/hns/hns_roce_main.c | 4 +-
drivers/infiniband/hw/hns/hns_roce_mr.c | 3 +
drivers/infiniband/hw/hns/hns_roce_pd.c | 18 +--
drivers/infiniband/hw/hns/hns_roce_qp.c | 57 +++-----
12 files changed, 312 insertions(+), 256 deletions(-)
--
1.9.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
* Re: [PATCH libibverbs v3 1/3] Add new call ibv_cmd_create_ah_ex which supports extra parameters
From: Jason Gunthorpe @ 2016-09-20 15:51 UTC (permalink / raw)
To: Knut Omang
Cc: Leon Romanovsky, Yishai Hadas, Doug Ledford,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, Mukesh Kacker,
. Coulter, Susan K, Woodruff, Robert J
In-Reply-To: <1474380350.20134.162.camel-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
On Tue, Sep 20, 2016 at 04:05:50PM +0200, Knut Omang wrote:
> The problem is that libsif is stuck under a BSD only license imposed by
> the first set of lawyers involved, and to be compatible, we need it
> to be dual license GPLv2 + BSD like the rest of the provider libraries.
Here is some (not a lawyer) information you may find helpful.
Oracle is an OFA member and part of the purpose of groups of OFA is to
manage license issues across companies collaborating on the same code
base. Oracle will have agreed to various things on this topic when
they became a member. The OFA may be able to help you with your legal.
The OFA may choose to not distribute improperly licensed code in
OFED/etc.
GPLv2 compatability is important. Not just for the consolidated tree,
but for everyone. Distors will have complicated questions if asked to
ship a GPLv2 incompatible plugin at the same time as shipping a GPLv2
program that uses the plugin. You may find your module undistributed.
That said, it is widely regarded that the BSD 2 and 3 clause license
varients are GPLv2 compatible on their own without any additional
language.
My view on the OFA dual license scheme is that it is used to
provide absolute certainty that the code is GPLv2 compatible.
Many other projects rely on the compatibility without being explicit.
I would have no objection to a BSD 2/3 clause licensed provider in the
consolidated tree. (while being confused why adding the GPLv2 option
is such a difficult problem, and re-emphasising the view that the code
will be used and distributed under the GPLv2 in some situations.)
A GPLv2 incompatible option like the CDDL, Apache license, etc is not
acceptable. Do not get creative and add new clauses to the stanard BSD
license.
So, in short, I would urge you to work with your legal to use the OFA
dual license. If that is completely impossible then a standard BSD
license will work to some degree. Remember, it is very hard to change
licenses after the fact.
Jason
--
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 libibverbs v3 1/3] Add new call ibv_cmd_create_ah_ex which supports extra parameters
From: Jason Gunthorpe @ 2016-09-20 15:32 UTC (permalink / raw)
To: Yishai Hadas
Cc: Knut Omang, Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
Mukesh Kacker
In-Reply-To: <550041ef-b105-c079-068c-d96278ec34f1-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
On Tue, Sep 20, 2016 at 02:17:04PM +0300, Yishai Hadas wrote:
> Any reason to add 1.4 instead of using 1.3 ? in my understanding the idea is
> that new symbols for next release will use a new ABI version, this is what
> 1.3 already supplied. Added Jason for his input as well.
Doug indicated he was going to roll a 1.3 release RSN, up to him what
number it will be eventually.
> >+ global:
> >+ ibv_cmd_create_ah_ex;
>
> When do you plan to contribute the driver code for that ? for now no
> upstream drivers uses this API, taking some code for future usage with no
> current usage is not the preferred way.
Even a github reference with your pre-submission driver would be useful
Jason
--
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: [RFC v2 06/12] qedr: Add support for QP verbs
From: Jason Gunthorpe @ 2016-09-20 15:28 UTC (permalink / raw)
To: Ram Amrani
Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q, dledford-H+wXaHxf7aLQT0dZR+AlfA,
Ariel.Elior-YGCgFSpz5w/QT0dZR+AlfA,
Michal.Kalderon-YGCgFSpz5w/QT0dZR+AlfA,
Yuval.Mintz-YGCgFSpz5w/QT0dZR+AlfA,
rajesh.borundia-YGCgFSpz5w/QT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1474367764-9555-7-git-send-email-Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
On Tue, Sep 20, 2016 at 01:35:58PM +0300, Ram Amrani wrote:
> +++ b/include/uapi/rdma/providers/qedr-abi.h
> @@ -43,4 +43,39 @@ struct qedr_create_cq_uresp {
> u16 icid;
> };
Ugh, each patch keeps adding to this?
> +struct qedr_create_qp_ureq {
> + u32 qp_handle_hi;
> + u32 qp_handle_lo;
> +
> + /* SQ */
> + /* user space virtual address of SQ buffer */
> + u64 sq_addr;
> +
> + /* length of SQ buffer */
> + size_t sq_len;
Do not use size_t, that is just asking for 32/64 bit interop problems.
> +struct qedr_create_qp_uresp {
> + u32 qp_id;
> + int atomic_supported;
int again, etc. You get the idea.
Jason
--
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: [RFC v2 04/12] qedr: Add support for user context verbs
From: Jason Gunthorpe @ 2016-09-20 15:27 UTC (permalink / raw)
To: Ram Amrani
Cc: davem, dledford, Ariel.Elior, Michal.Kalderon, Yuval.Mintz,
rajesh.borundia, linux-rdma, netdev
In-Reply-To: <1474367764-9555-5-git-send-email-Ram.Amrani@cavium.com>
On Tue, Sep 20, 2016 at 01:35:56PM +0300, Ram Amrani wrote:
> +++ b/include/uapi/rdma/providers/qedr-abi.h
> @@ -0,0 +1,27 @@
> +/* QLogic qed NIC Driver
> + * Copyright (c) 2015 QLogic Corporation
> + *
> + * This software is available 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.
> + */
> +#ifndef __QEDR_USER_H__
> +#define __QEDR_USER_H__
> +
> +#define QEDR_ABI_VERSION (6)
> +
> +/* user kernel communication data structures. */
> +
> +struct qedr_alloc_ucontext_resp {
> + u64 db_pa;
> + u32 db_size;
> +
> + u32 max_send_wr;
> + u32 max_recv_wr;
> + u32 max_srq_wr;
> + u32 sges_per_send_wr;
> + u32 sges_per_recv_wr;
> + u32 sges_per_srq_wr;
uapi headers need the __ varients and the #include <linux/types.h>
Follow what Leon has done.
> + int max_cqes;
Do not use int here.
Is there really only 1 struct? Your driver never uses udata for
anything else?
Jason
^ permalink raw reply
* RE: [RFC 02/11] Add RoCE driver framework
From: Elior, Ariel @ 2016-09-20 15:04 UTC (permalink / raw)
To: Leon Romanovsky, Mintz, Yuval
Cc: dledford@redhat.com, Mark Bloch, Ram Amrani, David Miller,
Ariel Elior, Michal Kalderon, Rajesh Borundia,
linux-rdma@vger.kernel.org, netdev
In-Reply-To: <20160915054235.GK26069@leon.nu>
> From: Leon Romanovsky [mailto:leon@kernel.org]
> On Thu, Sep 15, 2016 at 05:11:03AM +0000, Mintz, Yuval wrote:
> > > As a summary, I didn't see in your responses any real life example where you will
> > > need global debug level for your driver.
> >
> > Not sure what you you're expecting - a list of BZs /private e-mails where
> > user reproductions were needed?
> > You're basically ignoring my claims that such are used, instead wanting
> > "evidence". I'm not going to try and produce any such.
>
> I asked an example and not evidence, where "modprobe your_driver
> debug=1" will be superior to "modprobe your_driver dyndbg==pmf".
>
> https://www.kernel.org/doc/Documentation/dynamic-debug-howto.txt
Hi,
dyndbg vs module param:
Dynamic debug has two very nice features: dynamic activation/configuration and per line/file/module/format activation. The module param has neither, but it does have a few merits which I am not sure dyndbg has:
(1)
It can activate printouts according to *flow*. A lot of thought has been put into associating the right printout in our driver at the right verbosity level with the right "flow tag" (e.g. QEDR_MSG_INIT, QEDR_MSG_QP). The module parameter accepts a bitmask which allows setting any subset of these flows. This means that with the correct values for the parameter I can open only "init" printouts, or only "Memory Region" printouts, even if these cross multiple files / functions and don't share a common format. Presumably, one would claim that we could add the "flow tag" to the format to every printout according to its flow, but that would encumber the printouts, and also doesn't scale well to printouts which belong to multiple flows, where the current approach allows this (QEDR_MSG_SQ | QEDR_MSG_RQ).
(2)
As Yuval pointed out, there are users out there which have no trouble loading a driver with a module parameter, at probe or at kernel boot, but would be at a loss in mounting debugfs and dumping a matchspec script into a node there. As kernel developers, educating users is part of what we do, but it comes with a cost.
(3)
debugfs can be compiled out of the kernel in some codesize sensitive environments, or may not exist in an emergency shell or kdump kernel, whereas the module parameter would always be there.
Simply allowing our module parameter mechanism to be dynamically activated is very straightforward - we planned on adding a debugfs node for that anyway. But I would keep the module parameter for the sake of those less capable users and also for when debugfs is not available as detailed in (3) above.
I certainly see the merits of joining an existing infrastructure instead of implementing our own thing, but I would like to know if there are ways of obtaining the merits I listed for our approach via that infrastructure.
Leon, aside of commenting on the above, can you give an example of a driver which in your opinion does a good job of using dyndbg?
Thanks,
Ariel
^ permalink raw reply
* [PATCH v2 perftest] Support for Chelsio T6 devices
From: Steve Wise @ 2016-09-20 14:16 UTC (permalink / raw)
To: Gil Rockah; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Signed-off-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
---
Changes since v1:
fixed nit suggested by Leon
---
src/perftest_parameters.c | 1 +
src/perftest_parameters.h | 3 ++-
2 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/src/perftest_parameters.c b/src/perftest_parameters.c
index a21b6eb..0d1ea2f 100755
--- a/src/perftest_parameters.c
+++ b/src/perftest_parameters.c
@@ -1326,6 +1326,7 @@ enum ctx_device ib_dev_name(struct ibv_context *context)
case 4 : dev_fname = CHELSIO_T4; break;
case 11 :
case 5 : dev_fname = CHELSIO_T5; break;
+ case 6 : dev_fname = CHELSIO_T6; break;
default : dev_fname = UNKNOWN; break;
}
diff --git a/src/perftest_parameters.h b/src/perftest_parameters.h
index 448e398..dc57efb 100755
--- a/src/perftest_parameters.h
+++ b/src/perftest_parameters.h
@@ -269,7 +269,8 @@ enum ctx_device {
CONNECTX4 = 10,
CONNECTX4LX = 11,
QLOGIC_E4 = 12,
- QLOGIC_AH = 13
+ QLOGIC_AH = 13,
+ CHELSIO_T6 = 14,
};
/* Units for rate limiter */
--
1.7.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] Support for Chelsio T6 devices
From: Steve Wise @ 2016-09-20 14:12 UTC (permalink / raw)
To: 'Leon Romanovsky'
Cc: 'Gil Rockah', linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20160920102108.GG26673-2ukJVAZIZ/Y@public.gmane.org>
> > diff --git a/src/perftest_parameters.h b/src/perftest_parameters.h
> > index 448e398..727a65f 100755
> > --- a/src/perftest_parameters.h
> > +++ b/src/perftest_parameters.h
> > @@ -269,7 +269,8 @@ enum ctx_device {
> > CONNECTX4 = 10,
> > CONNECTX4LX = 11,
> > QLOGIC_E4 = 12,
> > - QLOGIC_AH = 13
> > + QLOGIC_AH = 13,
> > + CHELSIO_T6 = 14
>
> Nit comment,
> if you use "14," and not "14", the next submitter will send patch with
> less churn.
Yup. I usually try to adhere to the coding style of the existing code though.
--
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 libibverbs v3 1/3] Add new call ibv_cmd_create_ah_ex which supports extra parameters
From: Knut Omang @ 2016-09-20 14:05 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Yishai Hadas, Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
Mukesh Kacker, Jason Gunthorpe
In-Reply-To: <20160920132945.GO26673-2ukJVAZIZ/Y@public.gmane.org>
On Tue, 2016-09-20 at 16:29 +0300, Leon Romanovsky wrote:
> On Tue, Sep 20, 2016 at 01:37:39PM +0200, Knut Omang wrote:
> > On Tue, 2016-09-20 at 14:17 +0300, Yishai Hadas wrote:
> > > On 9/20/2016 9:22 AM, Knut Omang wrote:
> > > >
> > > > The original call ibv_cmd_create_ah does not allow vendor specific request or
> > > > response parameters to be defined and passed through to kernel space.
> > > >
> > > > To keep backward compatibility, introduce a new extended call which accepts
> > > > cmd and resp parameters similar to other parts of the API.
> > > > Reimplement the old call to just use the new extended call.
> > > >
> > > > The new call is versioned in the shared library as IBVERBS_1.4.
> > > >
> > > > Signed-off-by: Knut Omang <knut.omang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > > > Reviewed-by: Mukesh Kacker <mukesh.kacker-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > > > ---
> > > > include/infiniband/driver.h | 4 +++-
> > > > src/cmd.c | 50 +++++++++++++++++++++-----------------
> > > > src/libibverbs.map | 6 ++++-
> > > > 3 files changed, 37 insertions(+), 23 deletions(-)
> > > >
> > > > diff --git a/include/infiniband/driver.h b/include/infiniband/driver.h
> > > > index ea3dade..9356dc8 100644
> > > > --- a/include/infiniband/driver.h
> > > > +++ b/include/infiniband/driver.h
> > > > @@ -234,6 +234,10 @@ int ibv_cmd_post_srq_recv(struct ibv_srq *srq, struct ibv_recv_wr *wr,
> > > > struct ibv_recv_wr **bad_wr);
> > > > int ibv_cmd_create_ah(struct ibv_pd *pd, struct ibv_ah *ah,
> > > > struct ibv_ah_attr *attr);
> > > > +int ibv_cmd_create_ah_ex(struct ibv_pd *pd, struct ibv_ah *ah,
> > > > + struct ibv_ah_attr *attr,
> > > > + struct ibv_create_ah *cmd, size_t cmd_size,
> > > > + struct ibv_create_ah_resp *resp, size_t resp_size);
> > > > int ibv_cmd_destroy_ah(struct ibv_ah *ah);
> > > > int ibv_cmd_attach_mcast(struct ibv_qp *qp, const union ibv_gid *gid, uint16_t lid);
> > > > int ibv_cmd_detach_mcast(struct ibv_qp *qp, const union ibv_gid *gid, uint16_t lid);
> > > > diff --git a/src/cmd.c b/src/cmd.c
> > > > index 11f6509..58b9dcd 100644
> > > > --- a/src/cmd.c
> > > > +++ b/src/cmd.c
> > > > @@ -1493,38 +1493,44 @@ int ibv_cmd_post_srq_recv(struct ibv_srq *srq, struct ibv_recv_wr *wr,
> > > > return ret;
> > > > }
> > > >
> > > > -int ibv_cmd_create_ah(struct ibv_pd *pd, struct ibv_ah *ah,
> > > > - struct ibv_ah_attr *attr)
> > > > +int ibv_cmd_create_ah_ex(struct ibv_pd *pd, struct ibv_ah *ah, struct ibv_ah_attr *attr,
> > > > + struct ibv_create_ah *cmd, size_t cmd_size,
> > > > + struct ibv_create_ah_resp *resp, size_t resp_size)
> > > > {
> > > > - struct ibv_create_ah cmd;
> > > > - struct ibv_create_ah_resp resp;
> > > > -
> > > > - IBV_INIT_CMD_RESP(&cmd, sizeof cmd, CREATE_AH, &resp, sizeof resp);
> > > > - cmd.user_handle = (uintptr_t) ah;
> > > > - cmd.pd_handle = pd->handle;
> > > > - cmd.attr.dlid = attr->dlid;
> > > > - cmd.attr.sl = attr->sl;
> > > > - cmd.attr.src_path_bits = attr->src_path_bits;
> > > > - cmd.attr.static_rate = attr->static_rate;
> > > > - cmd.attr.is_global = attr->is_global;
> > > > - cmd.attr.port_num = attr->port_num;
> > > > - cmd.attr.grh.flow_label = attr->grh.flow_label;
> > > > - cmd.attr.grh.sgid_index = attr->grh.sgid_index;
> > > > - cmd.attr.grh.hop_limit = attr->grh.hop_limit;
> > > > - cmd.attr.grh.traffic_class = attr->grh.traffic_class;
> > > > - memcpy(cmd.attr.grh.dgid, attr->grh.dgid.raw, 16);
> > > > + IBV_INIT_CMD_RESP(cmd, cmd_size, CREATE_AH, resp, resp_size);
> > > > + cmd->user_handle = (uintptr_t) ah;
> > > > + cmd->pd_handle = pd->handle;
> > > > + cmd->attr.dlid = attr->dlid;
> > > > + cmd->attr.sl = attr->sl;
> > > > + cmd->attr.src_path_bits = attr->src_path_bits;
> > > > + cmd->attr.static_rate = attr->static_rate;
> > > > + cmd->attr.is_global = attr->is_global;
> > > > + cmd->attr.port_num = attr->port_num;
> > > > + cmd->attr.grh.flow_label = attr->grh.flow_label;
> > > > + cmd->attr.grh.sgid_index = attr->grh.sgid_index;
> > > > + cmd->attr.grh.hop_limit = attr->grh.hop_limit;
> > > > + cmd->attr.grh.traffic_class = attr->grh.traffic_class;
> > > > + memcpy(cmd->attr.grh.dgid, attr->grh.dgid.raw, 16);
> > > >
> > > > - if (write(pd->context->cmd_fd, &cmd, sizeof cmd) != sizeof cmd)
> > > > + if (write(pd->context->cmd_fd, cmd, cmd_size) != cmd_size)
> > > > return errno;
> > > >
> > > > - (void) VALGRIND_MAKE_MEM_DEFINED(&resp, sizeof resp);
> > > > + (void) VALGRIND_MAKE_MEM_DEFINED(resp, sizeof *resp);
> > > >
> > > > - ah->handle = resp.handle;
> > > > + ah->handle = resp->handle;
> > > > ah->context = pd->context;
> > > >
> > > > return 0;
> > > > }
> > > >
> > > > +int ibv_cmd_create_ah(struct ibv_pd *pd, struct ibv_ah *ah,
> > > > + struct ibv_ah_attr *attr)
> > > > +{
> > > > + struct ibv_create_ah cmd;
> > > > + struct ibv_create_ah_resp resp;
> > > > + return ibv_cmd_create_ah_ex(pd, ah, attr, &cmd, sizeof(cmd), &resp, sizeof(resp));
> > > > +}
> > > > +
> > > > int ibv_cmd_destroy_ah(struct ibv_ah *ah)
> > > > {
> > > > struct ibv_destroy_ah cmd;
> > > > diff --git a/src/libibverbs.map b/src/libibverbs.map
> > > > index 46744e5..ddba63a 100644
> > > > --- a/src/libibverbs.map
> > > > +++ b/src/libibverbs.map
> > > > @@ -118,7 +118,6 @@ IBVERBS_1.1 {
> > > > ibv_cmd_create_qp_ex2;
> > > > ibv_cmd_open_qp;
> > > > ibv_cmd_rereg_mr;
> > > > -
> > > > } IBVERBS_1.0;
> > > >
> > > > IBVERBS_1.3 {
> > > > @@ -130,3 +129,8 @@ IBVERBS_1.3 {
> > > > ibv_cmd_destroy_rwq_ind_table;
> > > > ibv_query_gid_type;
> > > > } IBVERBS_1.1;
> > > > +
> > > > +IBVERBS_1.4 {
> > > Any reason to add 1.4 instead of using 1.3 ? in my understanding the
> > > idea is that new symbols for next release will use a new ABI version,
> > > this is what 1.3 already supplied. Added Jason for his input as well.
> >
> > good point - Jason?
> >
> > > >
> > > > + global:
> > > > + ibv_cmd_create_ah_ex;
> > > When do you plan to contribute the driver code for that ? for now no
> > > upstream drivers uses this API, taking some code for future usage with
> > > no current usage is not the preferred way.
> >
> > The plan is ASAP, the kernel side has been approved for release by Oracle's
> > legal dep, but the user library is still stuck, and with an undesirable
> > license combination given Jason's recent consolidation efforts :-/
>
> Jason didn't change licenses. You still can release your driver under
> dual-license in similar way to other drivers.
The problem is that libsif is stuck under a BSD only license imposed by
the first set of lawyers involved, and to be compatible, we need it
to be dual license GPLv2 + BSD like the rest of the provider libraries.
Knut
> >
> > Knut
> >
> > > >
> > > > +} IBVERBS_1.3;
> > > >
> > --
> > 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
* Re: [PATCH] rdma_rxe: Ensure rdma_rxe module init occurs at correct time
From: Yonatan Cohen @ 2016-09-20 13:59 UTC (permalink / raw)
To: Stephen Bates, Doug Ledford
Cc: monis-VPRAkNaXOzVWk0Htik3J/w, linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <a3e5f61b65cdf92c192daef1da319718.squirrel-2RFepEojUI2m/tTznewQxVaTQe2KTcn/@public.gmane.org>
On 9/19/2016 2:59 AM, Stephen Bates wrote:
> On Fri, September 16, 2016 1:06 pm, Doug Ledford wrote:
>> On 9/15/2016 5:45 PM, Stephen Bates wrote:
>>
>>> When the Soft-RoCE module is built into the kernel there is a
>>> initialization issue when CONFIG_IPV6 is selected. Use late_initcall to
>>> ensure the rxe module is loaded later. This fix is based on one proposed
>>> by Logan Gunthorpe on a much older code base [1].
>>
>> Your commit message doesn't make sense. You start by saying that
>> soft-RoCE is compiled in, then say "the rxe module is loaded later". Well,
>> which is it? I'm not sure exactly the problem you are fixing. However, I
>> know that the series Leon posted for the rxe driver included a fix
>> specifically for IPv6 tunnel creation issues that result in an oops. I
>> suspect that patch might solve your problem. So, I'm going to drop this
>> patch. Once you get a chance to test with the other patch applied, if
>> your problem still exists, then please repost this patch with a clearer
>> commit message and we can add this as well.
>
> Doug
>
> Yes on reflection the commit message associated with this patch was not as
> clear as it could have been. Apologies. I will be more explicit on the v1.
> To answer your questions:
>
> 1. This patch should be applied on top of the three patches that came from
> Leon [1]. I will make that clear in v1.
>
> 2. My patch applies to the case where rmda_rxe is built-in and not when it
> is a module. Currently the module_init is called before IPv6 is setup and
> that results in a load failure of rxe.
>
> 3. When rdma_rxe is built as a module the problem is not observed because
> the rdma_rxe module is not probed until the system is up and running.
>
> 4. The change from module_init to late_initcall means that IPv6 has had
> enough time to come up befoe the rdma_rxe init is executed.
>
> I hope that clarifies things. I will get a v1 out shortly with an improved
> commit log.
>
> [1] http://marc.info/?l=linux-rdma&m=147307739626544&w=2
>
> Cheers
>
> Stephen Bates
>
Hi Stephen,
I reproduced the issue you've reported, and tested your fix as well and
it looks good to me.
You can add my reviewed-by tag to the future v1 patch.
Yonatan,
Thanks
>>
>>> [1] https://github.com/sbates130272/qemu-minimal/blob/rdma/patches/\
>>> 0001-Changes-needed-to-use-rxe-compiled-into-the-kernel.patch
>>>
>>>
>>> Signed-off-by: Stephen Bates <sbates-pv7U853sEMVWk0Htik3J/w@public.gmane.org>
Reviewed-By: Yonatan Cohen <yonatanc-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>> ---
>>> drivers/infiniband/sw/rxe/rxe.c | 2 +- 1 file changed, 1 insertion(+), 1
>>> deletion(-)
>>>
>>> diff --git a/drivers/infiniband/sw/rxe/rxe.c
>>> b/drivers/infiniband/sw/rxe/rxe.c index 08e09d9..97bc74a 100644 ---
>>> a/drivers/infiniband/sw/rxe/rxe.c +++ b/drivers/infiniband/sw/rxe/rxe.c
>>> @@ -392,5 +392,5 @@ static void __exit rxe_module_exit(void)
>>> pr_info("rxe: unloaded\n");
>>> }
>>>
>>>
>>> -module_init(rxe_module_init);
>>> +late_initcall(rxe_module_init);
>>> module_exit(rxe_module_exit);
>>>
>>
>>
>> --
>> Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> GPG Key ID: 0E572FDD
>>
>>
>>
>
> --
> 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
* Re: [PATCH libibverbs v2 3/3] Provide remote XRC SRQ number in kernel post_send.
From: Knut Omang @ 2016-09-20 13:38 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Mukesh Kacker
In-Reply-To: <20160920121741.GL26673-2ukJVAZIZ/Y@public.gmane.org>
On Tue, 2016-09-20 at 15:17 +0300, Leon Romanovsky wrote:
> On Tue, Sep 20, 2016 at 01:08:35PM +0200, Knut Omang wrote:
> > On Tue, 2016-09-20 at 14:01 +0300, Leon Romanovsky wrote:
> > > On Tue, Sep 20, 2016 at 12:43:30PM +0200, Knut Omang wrote:
> > > >
> > > > On Tue, 2016-09-20 at 13:18 +0300, Leon Romanovsky wrote:
> > > > >
> > > > > On Mon, Sep 19, 2016 at 11:12:36AM +0200, Knut Omang wrote:
> > > > > >
> > > > > >
> > > > > > On Mon, 2016-09-19 at 08:29 +0300, Leon Romanovsky wrote:
> > > > > > >
> > > > > > >
> > > > > > > On Sat, Sep 17, 2016 at 05:59:13AM +0200, Knut Omang wrote:
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > Also proper end align the ibv_kern_send_wr struct.
> > > > > > > >
> > > > > > > > Requires a corresponding kernel change.
> > > > > > > >
> > > > > > > > Signed-off-by: Knut Omang <knut.omang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > > > > > > > Reviewed-by: Mukesh Kacker <mukesh.kacker-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > > > > > > > ---
> > > > > > > > include/infiniband/kern-abi.h | 1 +
> > > > > > > > src/cmd.c | 2 ++
> > > > > > > > 2 files changed, 3 insertions(+), 0 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/include/infiniband/kern-abi.h b/include/infiniband/kern-abi.h
> > > > > > > > index 8bdeef5..7b1d310 100644
> > > > > > > > --- a/include/infiniband/kern-abi.h
> > > > > > > > +++ b/include/infiniband/kern-abi.h
> > > > > > > > @@ -800,6 +800,7 @@ struct ibv_kern_send_wr {
> > > > > > > > union {
> > > > > > > > struct {
> > > > > > > > __u32 remote_srqn;
> > > > > > > > + __u32 reserved;
> > > > > > > > } xrc;
> > > > > > > > } qp_type;
> > > > > > > > };
> > > > > > > > diff --git a/src/cmd.c b/src/cmd.c
> > > > > > > > index a418ee1..a4e2f75 100644
> > > > > > > > --- a/src/cmd.c
> > > > > > > > +++ b/src/cmd.c
> > > > > > > > @@ -1293,6 +1293,8 @@ int ibv_cmd_post_send(struct ibv_qp *ibqp, struct ibv_send_wr *wr,
> > > > > > > > tmp->wr.ud.remote_qpn = i->wr.ud.remote_qpn;
> > > > > > > > tmp->wr.ud.remote_qkey = i->wr.ud.remote_qkey;
> > > > > > > > } else {
> > > > > > > > + if (ibqp->qp_type == IBV_QPT_XRC_SEND)
> > > > > > > > + tmp->qp_type.xrc.remote_srqn = i->qp_type.xrc.remote_srqn;
> > > > > > > It will be checked for all QPTs and for all consumers. Any chances to
> > > > > > > optimize it?
> > > > > > All QPTs except UD, yes.
> > > > > >
> > > > > > The easiest is perhaps just to remove the test (and set the attribute in all cases), which will work fine
> > > > > > as long as the qp_type union is not used for anything else, but is it worth
> > > > > > the loss of intuitiveness/future potential correctness of the code?
> > > > > Let's concentrate on the present and as far as I see there are no kernel users who
> > > > > need this field xrc.remote_srqn field.
> > > > So I assume then that you are ok with this code, given my explanation?
> > > Partially,
> > >
> > > I'm fine with the claim about readability and the fact that
> > > this code is not in data-path relaxed me a little bit.
> >
> > Good!
> >
> > > I'm not fine with the part that this code doesn't do anything in kernel.
> >
> > I know, I have two big patch sets one kernel and one for the new "rdma consolidated"
> > in the queue that should fix that, just want to save a few versions of those
> > ~32000 lines of code by preparing the baseline :-)
>
> It won't save. We DO believe you that this this new addition to UAPI is really
> needed, but we NEED to see that it is unavoidable.
>
> The proper flow for submission is driver->kernel_core->libraries and not
> kernel_core->libraries->driver.
Ok, got it..
Knut
> Thanks
--
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 v4 01/16] vmxnet3: Move PCI Id to pci_ids.h
From: Bjorn Helgaas @ 2016-09-20 13:37 UTC (permalink / raw)
To: Adit Ranadive
Cc: Yuval Shaia, dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, pv-drivers,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Jorgen S. Hansen, Aditya Sarwade, George Zhang, Bryan Tan
In-Reply-To: <DM2PR0501MB84422A1029EE72FC727F960C5F10-SXUgMXdxDSdudfUQJyYm35LWcSx1hRipwIZJ9u9yWa8oOQlpcoRfSA@public.gmane.org>
On Wed, Sep 14, 2016 at 07:36:34PM +0000, Adit Ranadive wrote:
> On Wed, Sep 14, 2016 at 09:25:18 -0700, Yuval Shaia wrote:
> > On Wed, Sep 14, 2016 at 04:00:25PM +0000, Adit Ranadive wrote:
> > > On Wed, Sep 14, 2016 at 04:09:12 -0700, Yuval Shaia wrote:
> > > > Please update vmxnet3_drv.c accordingly.
> > >
> > > Any reason why? I don't think we need to. Vmxnet3 should just pick up
> > > the moved PCI device id from pci_ids.h file.
> >
> > So now you need to include it from vmxnet3_drv.c.
> > Same with pvrdma_main.c
>
> If you're asking me to include pci_ids.h in our drivers we already do that
> by including pci.h in both the drivers.
> pci.h already includes pci_ids.h -
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/linux/pci.h#n35
>
> If that's going to change maybe someone from the PCI group can comment on.
I don't contemplate a change there. It's fine to get pci_ids.h via pci.h.
Almost all drivers do that today. If we do change it someday, we can
change this driver along with all the rest.
Bjorn
--
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: [RFC v2 00/11] QLogic RDMA Driver (qedr) RFC
From: Elior, Ariel @ 2016-09-20 13:33 UTC (permalink / raw)
To: Leon Romanovsky, Amrani, Ram
Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org,
dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
Kalderon, Michal, Mintz, Yuval, Borundia, Rajesh,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20160920121151.GK26673-2ukJVAZIZ/Y@public.gmane.org>
> On Tue, Sep 20, 2016 at 01:35:52PM +0300, Ram Amrani wrote:
...
> > The series adds on top of RFC v1:
> > * a check for all drivers that IB_ACCESS_MW_BIND isn't set for ib_get_dma_mr
> > * relocation of qedr user API to include/rdma/uapi/providers/
> > * removal of qedr_devlist_local
> > * fixed error handling in qedr_alloc_resources()
> > * configuration of PBL in ib_map_mr_sg() driver implementation,
> > rather than post_send's IB_WR_REG_MR
> > * misc.: placed code in proper patch, fixed a few comments,
> > removed extra parentheses
> >
> > Thanks for everyone which pointed out problems in the driver.
> >
> > Any review/comment is appreciated.
>
> Very nice,
> Any reason why didn't you drop debug module parameter and decided to
> mimic already available kernel core functionality?
>
> You got technical explanations why it is bad idea to use it. If you need additional
> voices
> to support my claims, you will find them in thread about VERBOSE flag and responses
> from
> Doug, and Dennis.
>
> Thanks
Hi Leon,
The RFC cover letter lists what has been addressed. Debug printouts are not addressed in V2 as the discussion on that topic is not concluded (more thoughts from us on debug printouts incoming on the thread). There were many comments to V1 which are not relevant to debug printouts which are addressed by V2. We are requesting further comment, hence RFC V2. Rest assured, if it is the final opinion in the relevant discussion that pr_debug is the way to go, that's what we'll do.
Thanks,
Ariel
--
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 libibverbs v3 1/3] Add new call ibv_cmd_create_ah_ex which supports extra parameters
From: Leon Romanovsky @ 2016-09-20 13:29 UTC (permalink / raw)
To: Knut Omang
Cc: Yishai Hadas, Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
Mukesh Kacker, Jason Gunthorpe
In-Reply-To: <1474371459.8837.32.camel-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 6820 bytes --]
On Tue, Sep 20, 2016 at 01:37:39PM +0200, Knut Omang wrote:
> On Tue, 2016-09-20 at 14:17 +0300, Yishai Hadas wrote:
> > On 9/20/2016 9:22 AM, Knut Omang wrote:
> > >
> > > The original call ibv_cmd_create_ah does not allow vendor specific request or
> > > response parameters to be defined and passed through to kernel space.
> > >
> > > To keep backward compatibility, introduce a new extended call which accepts
> > > cmd and resp parameters similar to other parts of the API.
> > > Reimplement the old call to just use the new extended call.
> > >
> > > The new call is versioned in the shared library as IBVERBS_1.4.
> > >
> > > Signed-off-by: Knut Omang <knut.omang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > > Reviewed-by: Mukesh Kacker <mukesh.kacker-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > > ---
> > > include/infiniband/driver.h | 4 +++-
> > > src/cmd.c | 50 +++++++++++++++++++++-----------------
> > > src/libibverbs.map | 6 ++++-
> > > 3 files changed, 37 insertions(+), 23 deletions(-)
> > >
> > > diff --git a/include/infiniband/driver.h b/include/infiniband/driver.h
> > > index ea3dade..9356dc8 100644
> > > --- a/include/infiniband/driver.h
> > > +++ b/include/infiniband/driver.h
> > > @@ -234,6 +234,10 @@ int ibv_cmd_post_srq_recv(struct ibv_srq *srq, struct ibv_recv_wr *wr,
> > > struct ibv_recv_wr **bad_wr);
> > > int ibv_cmd_create_ah(struct ibv_pd *pd, struct ibv_ah *ah,
> > > struct ibv_ah_attr *attr);
> > > +int ibv_cmd_create_ah_ex(struct ibv_pd *pd, struct ibv_ah *ah,
> > > + struct ibv_ah_attr *attr,
> > > + struct ibv_create_ah *cmd, size_t cmd_size,
> > > + struct ibv_create_ah_resp *resp, size_t resp_size);
> > > int ibv_cmd_destroy_ah(struct ibv_ah *ah);
> > > int ibv_cmd_attach_mcast(struct ibv_qp *qp, const union ibv_gid *gid, uint16_t lid);
> > > int ibv_cmd_detach_mcast(struct ibv_qp *qp, const union ibv_gid *gid, uint16_t lid);
> > > diff --git a/src/cmd.c b/src/cmd.c
> > > index 11f6509..58b9dcd 100644
> > > --- a/src/cmd.c
> > > +++ b/src/cmd.c
> > > @@ -1493,38 +1493,44 @@ int ibv_cmd_post_srq_recv(struct ibv_srq *srq, struct ibv_recv_wr *wr,
> > > return ret;
> > > }
> > >
> > > -int ibv_cmd_create_ah(struct ibv_pd *pd, struct ibv_ah *ah,
> > > - struct ibv_ah_attr *attr)
> > > +int ibv_cmd_create_ah_ex(struct ibv_pd *pd, struct ibv_ah *ah, struct ibv_ah_attr *attr,
> > > + struct ibv_create_ah *cmd, size_t cmd_size,
> > > + struct ibv_create_ah_resp *resp, size_t resp_size)
> > > {
> > > - struct ibv_create_ah cmd;
> > > - struct ibv_create_ah_resp resp;
> > > -
> > > - IBV_INIT_CMD_RESP(&cmd, sizeof cmd, CREATE_AH, &resp, sizeof resp);
> > > - cmd.user_handle = (uintptr_t) ah;
> > > - cmd.pd_handle = pd->handle;
> > > - cmd.attr.dlid = attr->dlid;
> > > - cmd.attr.sl = attr->sl;
> > > - cmd.attr.src_path_bits = attr->src_path_bits;
> > > - cmd.attr.static_rate = attr->static_rate;
> > > - cmd.attr.is_global = attr->is_global;
> > > - cmd.attr.port_num = attr->port_num;
> > > - cmd.attr.grh.flow_label = attr->grh.flow_label;
> > > - cmd.attr.grh.sgid_index = attr->grh.sgid_index;
> > > - cmd.attr.grh.hop_limit = attr->grh.hop_limit;
> > > - cmd.attr.grh.traffic_class = attr->grh.traffic_class;
> > > - memcpy(cmd.attr.grh.dgid, attr->grh.dgid.raw, 16);
> > > + IBV_INIT_CMD_RESP(cmd, cmd_size, CREATE_AH, resp, resp_size);
> > > + cmd->user_handle = (uintptr_t) ah;
> > > + cmd->pd_handle = pd->handle;
> > > + cmd->attr.dlid = attr->dlid;
> > > + cmd->attr.sl = attr->sl;
> > > + cmd->attr.src_path_bits = attr->src_path_bits;
> > > + cmd->attr.static_rate = attr->static_rate;
> > > + cmd->attr.is_global = attr->is_global;
> > > + cmd->attr.port_num = attr->port_num;
> > > + cmd->attr.grh.flow_label = attr->grh.flow_label;
> > > + cmd->attr.grh.sgid_index = attr->grh.sgid_index;
> > > + cmd->attr.grh.hop_limit = attr->grh.hop_limit;
> > > + cmd->attr.grh.traffic_class = attr->grh.traffic_class;
> > > + memcpy(cmd->attr.grh.dgid, attr->grh.dgid.raw, 16);
> > >
> > > - if (write(pd->context->cmd_fd, &cmd, sizeof cmd) != sizeof cmd)
> > > + if (write(pd->context->cmd_fd, cmd, cmd_size) != cmd_size)
> > > return errno;
> > >
> > > - (void) VALGRIND_MAKE_MEM_DEFINED(&resp, sizeof resp);
> > > + (void) VALGRIND_MAKE_MEM_DEFINED(resp, sizeof *resp);
> > >
> > > - ah->handle = resp.handle;
> > > + ah->handle = resp->handle;
> > > ah->context = pd->context;
> > >
> > > return 0;
> > > }
> > >
> > > +int ibv_cmd_create_ah(struct ibv_pd *pd, struct ibv_ah *ah,
> > > + struct ibv_ah_attr *attr)
> > > +{
> > > + struct ibv_create_ah cmd;
> > > + struct ibv_create_ah_resp resp;
> > > + return ibv_cmd_create_ah_ex(pd, ah, attr, &cmd, sizeof(cmd), &resp, sizeof(resp));
> > > +}
> > > +
> > > int ibv_cmd_destroy_ah(struct ibv_ah *ah)
> > > {
> > > struct ibv_destroy_ah cmd;
> > > diff --git a/src/libibverbs.map b/src/libibverbs.map
> > > index 46744e5..ddba63a 100644
> > > --- a/src/libibverbs.map
> > > +++ b/src/libibverbs.map
> > > @@ -118,7 +118,6 @@ IBVERBS_1.1 {
> > > ibv_cmd_create_qp_ex2;
> > > ibv_cmd_open_qp;
> > > ibv_cmd_rereg_mr;
> > > -
> > > } IBVERBS_1.0;
> > >
> > > IBVERBS_1.3 {
> > > @@ -130,3 +129,8 @@ IBVERBS_1.3 {
> > > ibv_cmd_destroy_rwq_ind_table;
> > > ibv_query_gid_type;
> > > } IBVERBS_1.1;
> > > +
> > > +IBVERBS_1.4 {
> > Any reason to add 1.4 instead of using 1.3 ? in my understanding the
> > idea is that new symbols for next release will use a new ABI version,
> > this is what 1.3 already supplied. Added Jason for his input as well.
>
> good point - Jason?
>
> > >
> > > + global:
> > > + ibv_cmd_create_ah_ex;
> > When do you plan to contribute the driver code for that ? for now no
> > upstream drivers uses this API, taking some code for future usage with
> > no current usage is not the preferred way.
>
> The plan is ASAP, the kernel side has been approved for release by Oracle's
> legal dep, but the user library is still stuck, and with an undesirable
> license combination given Jason's recent consolidation efforts :-/
Jason didn't change licenses. You still can release your driver under
dual-license in similar way to other drivers.
>
> Knut
>
> > >
> > > +} IBVERBS_1.3;
> > >
> --
> 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: VERBOSE flags
From: Leon Romanovsky @ 2016-09-20 12:30 UTC (permalink / raw)
To: Dalessandro, Dennis
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
In-Reply-To: <1474373496.12415.28.camel-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1458 bytes --]
On Tue, Sep 20, 2016 at 12:11:40PM +0000, Dalessandro, Dennis wrote:
> On Tue, 2016-09-20 at 13:56 +0300, Leon Romanovsky wrote:
> > > Yes we do have that, which illustrates that trace points can not do
> > > everything. I had hoped to deprecate that at some point.
> > > Regardless,
> > > sometimes you want additional code to be added or to do something
> > > slightly different. You can't do that with a tracepoint.
> >
> > Can you elaborate more on this topic? Which limitations did you see
> > in tracepoints?
>
> Just making up an example. Say you have a hash table. You want to dump
> the items. If you iterate over it and do a trace point, yeah the actual
> trace point has little to no perf impact, but you still walk the
> table.
>
> There are situations where you need to do more for debugging than just
> printing a message.
Thanks
>
> > In regards of other discussion in KS 2016, "tracepoints and ABI
> > stability warranties" [1]. Are you strict with this tracepoints ABI?
>
> Honestly we are not super strict about it, but do take care not to
> break/remove something that could be useful. It will be interesting to
> see what comes out of the discussion. Perhaps we do need to be more
> strict about them going forward.
I followed the discussion and it looks like that majority are against to
declare tracepoints as ABI. Otherwise, you will find yourself unable to
change the kernel code.
>
> -Denny
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH libibverbs v2 3/3] Provide remote XRC SRQ number in kernel post_send.
From: Leon Romanovsky @ 2016-09-20 12:17 UTC (permalink / raw)
To: Knut Omang; +Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Mukesh Kacker
In-Reply-To: <1474369715.8837.19.camel-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 3547 bytes --]
On Tue, Sep 20, 2016 at 01:08:35PM +0200, Knut Omang wrote:
> On Tue, 2016-09-20 at 14:01 +0300, Leon Romanovsky wrote:
> > On Tue, Sep 20, 2016 at 12:43:30PM +0200, Knut Omang wrote:
> > >
> > > On Tue, 2016-09-20 at 13:18 +0300, Leon Romanovsky wrote:
> > > >
> > > > On Mon, Sep 19, 2016 at 11:12:36AM +0200, Knut Omang wrote:
> > > > >
> > > > >
> > > > > On Mon, 2016-09-19 at 08:29 +0300, Leon Romanovsky wrote:
> > > > > >
> > > > > >
> > > > > > On Sat, Sep 17, 2016 at 05:59:13AM +0200, Knut Omang wrote:
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > Also proper end align the ibv_kern_send_wr struct.
> > > > > > >
> > > > > > > Requires a corresponding kernel change.
> > > > > > >
> > > > > > > Signed-off-by: Knut Omang <knut.omang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > > > > > > Reviewed-by: Mukesh Kacker <mukesh.kacker-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > > > > > > ---
> > > > > > > include/infiniband/kern-abi.h | 1 +
> > > > > > > src/cmd.c | 2 ++
> > > > > > > 2 files changed, 3 insertions(+), 0 deletions(-)
> > > > > > >
> > > > > > > diff --git a/include/infiniband/kern-abi.h b/include/infiniband/kern-abi.h
> > > > > > > index 8bdeef5..7b1d310 100644
> > > > > > > --- a/include/infiniband/kern-abi.h
> > > > > > > +++ b/include/infiniband/kern-abi.h
> > > > > > > @@ -800,6 +800,7 @@ struct ibv_kern_send_wr {
> > > > > > > union {
> > > > > > > struct {
> > > > > > > __u32 remote_srqn;
> > > > > > > + __u32 reserved;
> > > > > > > } xrc;
> > > > > > > } qp_type;
> > > > > > > };
> > > > > > > diff --git a/src/cmd.c b/src/cmd.c
> > > > > > > index a418ee1..a4e2f75 100644
> > > > > > > --- a/src/cmd.c
> > > > > > > +++ b/src/cmd.c
> > > > > > > @@ -1293,6 +1293,8 @@ int ibv_cmd_post_send(struct ibv_qp *ibqp, struct ibv_send_wr *wr,
> > > > > > > tmp->wr.ud.remote_qpn = i->wr.ud.remote_qpn;
> > > > > > > tmp->wr.ud.remote_qkey = i->wr.ud.remote_qkey;
> > > > > > > } else {
> > > > > > > + if (ibqp->qp_type == IBV_QPT_XRC_SEND)
> > > > > > > + tmp->qp_type.xrc.remote_srqn = i->qp_type.xrc.remote_srqn;
> > > > > > It will be checked for all QPTs and for all consumers. Any chances to
> > > > > > optimize it?
> > > > > All QPTs except UD, yes.
> > > > >
> > > > > The easiest is perhaps just to remove the test (and set the attribute in all cases), which will work fine
> > > > > as long as the qp_type union is not used for anything else, but is it worth
> > > > > the loss of intuitiveness/future potential correctness of the code?
> > > > Let's concentrate on the present and as far as I see there are no kernel users who
> > > > need this field xrc.remote_srqn field.
> > > So I assume then that you are ok with this code, given my explanation?
> > Partially,
> >
> > I'm fine with the claim about readability and the fact that
> > this code is not in data-path relaxed me a little bit.
>
> Good!
>
> > I'm not fine with the part that this code doesn't do anything in kernel.
>
> I know, I have two big patch sets one kernel and one for the new "rdma consolidated"
> in the queue that should fix that, just want to save a few versions of those
> ~32000 lines of code by preparing the baseline :-)
It won't save. We DO believe you that this this new addition to UAPI is really
needed, but we NEED to see that it is unavoidable.
The proper flow for submission is driver->kernel_core->libraries and not
kernel_core->libraries->driver.
Thanks
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [RFC v2 00/11] QLogic RDMA Driver (qedr) RFC
From: Leon Romanovsky @ 2016-09-20 12:11 UTC (permalink / raw)
To: Ram Amrani
Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q, dledford-H+wXaHxf7aLQT0dZR+AlfA,
Ariel.Elior-YGCgFSpz5w/QT0dZR+AlfA,
Michal.Kalderon-YGCgFSpz5w/QT0dZR+AlfA,
Yuval.Mintz-YGCgFSpz5w/QT0dZR+AlfA,
rajesh.borundia-YGCgFSpz5w/QT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1474367764-9555-1-git-send-email-Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1476 bytes --]
On Tue, Sep 20, 2016 at 01:35:52PM +0300, Ram Amrani wrote:
> From: Ram amrani <Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
>
> This series introduces RoCE RDMA driver for the 579xx RDMA products by Qlogic.
> The RDMA support is added as an additional loadable module (qedr) over the
> Ethernet qede driver.
> The qedr module will support both RoCE and iWarp, although this series only
> adds RoCE support.
> The qed and qede drivers are enhanced with functionality required for RDMA
> support.
>
> Based on net-next 4c73195edbe3a5d7e14ea549bb261cf35c29f0cc.
>
> The series adds on top of RFC v1:
> * a check for all drivers that IB_ACCESS_MW_BIND isn't set for ib_get_dma_mr
> * relocation of qedr user API to include/rdma/uapi/providers/
> * removal of qedr_devlist_local
> * fixed error handling in qedr_alloc_resources()
> * configuration of PBL in ib_map_mr_sg() driver implementation,
> rather than post_send's IB_WR_REG_MR
> * misc.: placed code in proper patch, fixed a few comments,
> removed extra parentheses
>
> Thanks for everyone which pointed out problems in the driver.
>
> Any review/comment is appreciated.
Very nice,
Any reason why didn't you drop debug module parameter and decided to
mimic already available kernel core functionality?
You got technical explanations why it is bad idea to use it. If you need additional voices
to support my claims, you will find them in thread about VERBOSE flag and responses from
Doug, and Dennis.
Thanks
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: VERBOSE flags
From: Dalessandro, Dennis @ 2016-09-20 12:11 UTC (permalink / raw)
To: leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
In-Reply-To: <20160920105642.GI26673-2ukJVAZIZ/Y@public.gmane.org>
On Tue, 2016-09-20 at 13:56 +0300, Leon Romanovsky wrote:
> > Yes we do have that, which illustrates that trace points can not do
> > everything. I had hoped to deprecate that at some point.
> > Regardless,
> > sometimes you want additional code to be added or to do something
> > slightly different. You can't do that with a tracepoint.
>
> Can you elaborate more on this topic? Which limitations did you see
> in tracepoints?
Just making up an example. Say you have a hash table. You want to dump
the items. If you iterate over it and do a trace point, yeah the actual
trace point has little to no perf impact, but you still walk the
table.
There are situations where you need to do more for debugging than just
printing a message.
> In regards of other discussion in KS 2016, "tracepoints and ABI
> stability warranties" [1]. Are you strict with this tracepoints ABI?
Honestly we are not super strict about it, but do take care not to
break/remove something that could be useful. It will be interesting to
see what comes out of the discussion. Perhaps we do need to be more
strict about them going forward.
-Denny
^ permalink raw reply
* Re: [PATCH libibverbs v3 1/3] Add new call ibv_cmd_create_ah_ex which supports extra parameters
From: Knut Omang @ 2016-09-20 11:37 UTC (permalink / raw)
To: Yishai Hadas
Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Mukesh Kacker,
Jason Gunthorpe
In-Reply-To: <550041ef-b105-c079-068c-d96278ec34f1-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
On Tue, 2016-09-20 at 14:17 +0300, Yishai Hadas wrote:
> On 9/20/2016 9:22 AM, Knut Omang wrote:
> >
> > The original call ibv_cmd_create_ah does not allow vendor specific request or
> > response parameters to be defined and passed through to kernel space.
> >
> > To keep backward compatibility, introduce a new extended call which accepts
> > cmd and resp parameters similar to other parts of the API.
> > Reimplement the old call to just use the new extended call.
> >
> > The new call is versioned in the shared library as IBVERBS_1.4.
> >
> > Signed-off-by: Knut Omang <knut.omang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > Reviewed-by: Mukesh Kacker <mukesh.kacker-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > ---
> > include/infiniband/driver.h | 4 +++-
> > src/cmd.c | 50 +++++++++++++++++++++-----------------
> > src/libibverbs.map | 6 ++++-
> > 3 files changed, 37 insertions(+), 23 deletions(-)
> >
> > diff --git a/include/infiniband/driver.h b/include/infiniband/driver.h
> > index ea3dade..9356dc8 100644
> > --- a/include/infiniband/driver.h
> > +++ b/include/infiniband/driver.h
> > @@ -234,6 +234,10 @@ int ibv_cmd_post_srq_recv(struct ibv_srq *srq, struct ibv_recv_wr *wr,
> > struct ibv_recv_wr **bad_wr);
> > int ibv_cmd_create_ah(struct ibv_pd *pd, struct ibv_ah *ah,
> > struct ibv_ah_attr *attr);
> > +int ibv_cmd_create_ah_ex(struct ibv_pd *pd, struct ibv_ah *ah,
> > + struct ibv_ah_attr *attr,
> > + struct ibv_create_ah *cmd, size_t cmd_size,
> > + struct ibv_create_ah_resp *resp, size_t resp_size);
> > int ibv_cmd_destroy_ah(struct ibv_ah *ah);
> > int ibv_cmd_attach_mcast(struct ibv_qp *qp, const union ibv_gid *gid, uint16_t lid);
> > int ibv_cmd_detach_mcast(struct ibv_qp *qp, const union ibv_gid *gid, uint16_t lid);
> > diff --git a/src/cmd.c b/src/cmd.c
> > index 11f6509..58b9dcd 100644
> > --- a/src/cmd.c
> > +++ b/src/cmd.c
> > @@ -1493,38 +1493,44 @@ int ibv_cmd_post_srq_recv(struct ibv_srq *srq, struct ibv_recv_wr *wr,
> > return ret;
> > }
> >
> > -int ibv_cmd_create_ah(struct ibv_pd *pd, struct ibv_ah *ah,
> > - struct ibv_ah_attr *attr)
> > +int ibv_cmd_create_ah_ex(struct ibv_pd *pd, struct ibv_ah *ah, struct ibv_ah_attr *attr,
> > + struct ibv_create_ah *cmd, size_t cmd_size,
> > + struct ibv_create_ah_resp *resp, size_t resp_size)
> > {
> > - struct ibv_create_ah cmd;
> > - struct ibv_create_ah_resp resp;
> > -
> > - IBV_INIT_CMD_RESP(&cmd, sizeof cmd, CREATE_AH, &resp, sizeof resp);
> > - cmd.user_handle = (uintptr_t) ah;
> > - cmd.pd_handle = pd->handle;
> > - cmd.attr.dlid = attr->dlid;
> > - cmd.attr.sl = attr->sl;
> > - cmd.attr.src_path_bits = attr->src_path_bits;
> > - cmd.attr.static_rate = attr->static_rate;
> > - cmd.attr.is_global = attr->is_global;
> > - cmd.attr.port_num = attr->port_num;
> > - cmd.attr.grh.flow_label = attr->grh.flow_label;
> > - cmd.attr.grh.sgid_index = attr->grh.sgid_index;
> > - cmd.attr.grh.hop_limit = attr->grh.hop_limit;
> > - cmd.attr.grh.traffic_class = attr->grh.traffic_class;
> > - memcpy(cmd.attr.grh.dgid, attr->grh.dgid.raw, 16);
> > + IBV_INIT_CMD_RESP(cmd, cmd_size, CREATE_AH, resp, resp_size);
> > + cmd->user_handle = (uintptr_t) ah;
> > + cmd->pd_handle = pd->handle;
> > + cmd->attr.dlid = attr->dlid;
> > + cmd->attr.sl = attr->sl;
> > + cmd->attr.src_path_bits = attr->src_path_bits;
> > + cmd->attr.static_rate = attr->static_rate;
> > + cmd->attr.is_global = attr->is_global;
> > + cmd->attr.port_num = attr->port_num;
> > + cmd->attr.grh.flow_label = attr->grh.flow_label;
> > + cmd->attr.grh.sgid_index = attr->grh.sgid_index;
> > + cmd->attr.grh.hop_limit = attr->grh.hop_limit;
> > + cmd->attr.grh.traffic_class = attr->grh.traffic_class;
> > + memcpy(cmd->attr.grh.dgid, attr->grh.dgid.raw, 16);
> >
> > - if (write(pd->context->cmd_fd, &cmd, sizeof cmd) != sizeof cmd)
> > + if (write(pd->context->cmd_fd, cmd, cmd_size) != cmd_size)
> > return errno;
> >
> > - (void) VALGRIND_MAKE_MEM_DEFINED(&resp, sizeof resp);
> > + (void) VALGRIND_MAKE_MEM_DEFINED(resp, sizeof *resp);
> >
> > - ah->handle = resp.handle;
> > + ah->handle = resp->handle;
> > ah->context = pd->context;
> >
> > return 0;
> > }
> >
> > +int ibv_cmd_create_ah(struct ibv_pd *pd, struct ibv_ah *ah,
> > + struct ibv_ah_attr *attr)
> > +{
> > + struct ibv_create_ah cmd;
> > + struct ibv_create_ah_resp resp;
> > + return ibv_cmd_create_ah_ex(pd, ah, attr, &cmd, sizeof(cmd), &resp, sizeof(resp));
> > +}
> > +
> > int ibv_cmd_destroy_ah(struct ibv_ah *ah)
> > {
> > struct ibv_destroy_ah cmd;
> > diff --git a/src/libibverbs.map b/src/libibverbs.map
> > index 46744e5..ddba63a 100644
> > --- a/src/libibverbs.map
> > +++ b/src/libibverbs.map
> > @@ -118,7 +118,6 @@ IBVERBS_1.1 {
> > ibv_cmd_create_qp_ex2;
> > ibv_cmd_open_qp;
> > ibv_cmd_rereg_mr;
> > -
> > } IBVERBS_1.0;
> >
> > IBVERBS_1.3 {
> > @@ -130,3 +129,8 @@ IBVERBS_1.3 {
> > ibv_cmd_destroy_rwq_ind_table;
> > ibv_query_gid_type;
> > } IBVERBS_1.1;
> > +
> > +IBVERBS_1.4 {
> Any reason to add 1.4 instead of using 1.3 ? in my understanding the
> idea is that new symbols for next release will use a new ABI version,
> this is what 1.3 already supplied. Added Jason for his input as well.
good point - Jason?
> >
> > + global:
> > + ibv_cmd_create_ah_ex;
> When do you plan to contribute the driver code for that ? for now no
> upstream drivers uses this API, taking some code for future usage with
> no current usage is not the preferred way.
The plan is ASAP, the kernel side has been approved for release by Oracle's
legal dep, but the user library is still stuck, and with an undesirable
license combination given Jason's recent consolidation efforts :-/
Knut
> >
> > +} IBVERBS_1.3;
> >
--
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 libibverbs v3 1/3] Add new call ibv_cmd_create_ah_ex which supports extra parameters
From: Yishai Hadas @ 2016-09-20 11:17 UTC (permalink / raw)
To: Knut Omang
Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Mukesh Kacker,
Jason Gunthorpe
In-Reply-To: <d551c3792e884443903c67212f505082cd53b135.1474352400.git-series.knut.omang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
On 9/20/2016 9:22 AM, Knut Omang wrote:
> The original call ibv_cmd_create_ah does not allow vendor specific request or
> response parameters to be defined and passed through to kernel space.
>
> To keep backward compatibility, introduce a new extended call which accepts
> cmd and resp parameters similar to other parts of the API.
> Reimplement the old call to just use the new extended call.
>
> The new call is versioned in the shared library as IBVERBS_1.4.
>
> Signed-off-by: Knut Omang <knut.omang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Reviewed-by: Mukesh Kacker <mukesh.kacker-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> ---
> include/infiniband/driver.h | 4 +++-
> src/cmd.c | 50 +++++++++++++++++++++-----------------
> src/libibverbs.map | 6 ++++-
> 3 files changed, 37 insertions(+), 23 deletions(-)
>
> diff --git a/include/infiniband/driver.h b/include/infiniband/driver.h
> index ea3dade..9356dc8 100644
> --- a/include/infiniband/driver.h
> +++ b/include/infiniband/driver.h
> @@ -234,6 +234,10 @@ int ibv_cmd_post_srq_recv(struct ibv_srq *srq, struct ibv_recv_wr *wr,
> struct ibv_recv_wr **bad_wr);
> int ibv_cmd_create_ah(struct ibv_pd *pd, struct ibv_ah *ah,
> struct ibv_ah_attr *attr);
> +int ibv_cmd_create_ah_ex(struct ibv_pd *pd, struct ibv_ah *ah,
> + struct ibv_ah_attr *attr,
> + struct ibv_create_ah *cmd, size_t cmd_size,
> + struct ibv_create_ah_resp *resp, size_t resp_size);
> int ibv_cmd_destroy_ah(struct ibv_ah *ah);
> int ibv_cmd_attach_mcast(struct ibv_qp *qp, const union ibv_gid *gid, uint16_t lid);
> int ibv_cmd_detach_mcast(struct ibv_qp *qp, const union ibv_gid *gid, uint16_t lid);
> diff --git a/src/cmd.c b/src/cmd.c
> index 11f6509..58b9dcd 100644
> --- a/src/cmd.c
> +++ b/src/cmd.c
> @@ -1493,38 +1493,44 @@ int ibv_cmd_post_srq_recv(struct ibv_srq *srq, struct ibv_recv_wr *wr,
> return ret;
> }
>
> -int ibv_cmd_create_ah(struct ibv_pd *pd, struct ibv_ah *ah,
> - struct ibv_ah_attr *attr)
> +int ibv_cmd_create_ah_ex(struct ibv_pd *pd, struct ibv_ah *ah, struct ibv_ah_attr *attr,
> + struct ibv_create_ah *cmd, size_t cmd_size,
> + struct ibv_create_ah_resp *resp, size_t resp_size)
> {
> - struct ibv_create_ah cmd;
> - struct ibv_create_ah_resp resp;
> -
> - IBV_INIT_CMD_RESP(&cmd, sizeof cmd, CREATE_AH, &resp, sizeof resp);
> - cmd.user_handle = (uintptr_t) ah;
> - cmd.pd_handle = pd->handle;
> - cmd.attr.dlid = attr->dlid;
> - cmd.attr.sl = attr->sl;
> - cmd.attr.src_path_bits = attr->src_path_bits;
> - cmd.attr.static_rate = attr->static_rate;
> - cmd.attr.is_global = attr->is_global;
> - cmd.attr.port_num = attr->port_num;
> - cmd.attr.grh.flow_label = attr->grh.flow_label;
> - cmd.attr.grh.sgid_index = attr->grh.sgid_index;
> - cmd.attr.grh.hop_limit = attr->grh.hop_limit;
> - cmd.attr.grh.traffic_class = attr->grh.traffic_class;
> - memcpy(cmd.attr.grh.dgid, attr->grh.dgid.raw, 16);
> + IBV_INIT_CMD_RESP(cmd, cmd_size, CREATE_AH, resp, resp_size);
> + cmd->user_handle = (uintptr_t) ah;
> + cmd->pd_handle = pd->handle;
> + cmd->attr.dlid = attr->dlid;
> + cmd->attr.sl = attr->sl;
> + cmd->attr.src_path_bits = attr->src_path_bits;
> + cmd->attr.static_rate = attr->static_rate;
> + cmd->attr.is_global = attr->is_global;
> + cmd->attr.port_num = attr->port_num;
> + cmd->attr.grh.flow_label = attr->grh.flow_label;
> + cmd->attr.grh.sgid_index = attr->grh.sgid_index;
> + cmd->attr.grh.hop_limit = attr->grh.hop_limit;
> + cmd->attr.grh.traffic_class = attr->grh.traffic_class;
> + memcpy(cmd->attr.grh.dgid, attr->grh.dgid.raw, 16);
>
> - if (write(pd->context->cmd_fd, &cmd, sizeof cmd) != sizeof cmd)
> + if (write(pd->context->cmd_fd, cmd, cmd_size) != cmd_size)
> return errno;
>
> - (void) VALGRIND_MAKE_MEM_DEFINED(&resp, sizeof resp);
> + (void) VALGRIND_MAKE_MEM_DEFINED(resp, sizeof *resp);
>
> - ah->handle = resp.handle;
> + ah->handle = resp->handle;
> ah->context = pd->context;
>
> return 0;
> }
>
> +int ibv_cmd_create_ah(struct ibv_pd *pd, struct ibv_ah *ah,
> + struct ibv_ah_attr *attr)
> +{
> + struct ibv_create_ah cmd;
> + struct ibv_create_ah_resp resp;
> + return ibv_cmd_create_ah_ex(pd, ah, attr, &cmd, sizeof(cmd), &resp, sizeof(resp));
> +}
> +
> int ibv_cmd_destroy_ah(struct ibv_ah *ah)
> {
> struct ibv_destroy_ah cmd;
> diff --git a/src/libibverbs.map b/src/libibverbs.map
> index 46744e5..ddba63a 100644
> --- a/src/libibverbs.map
> +++ b/src/libibverbs.map
> @@ -118,7 +118,6 @@ IBVERBS_1.1 {
> ibv_cmd_create_qp_ex2;
> ibv_cmd_open_qp;
> ibv_cmd_rereg_mr;
> -
> } IBVERBS_1.0;
>
> IBVERBS_1.3 {
> @@ -130,3 +129,8 @@ IBVERBS_1.3 {
> ibv_cmd_destroy_rwq_ind_table;
> ibv_query_gid_type;
> } IBVERBS_1.1;
> +
> +IBVERBS_1.4 {
Any reason to add 1.4 instead of using 1.3 ? in my understanding the
idea is that new symbols for next release will use a new ABI version,
this is what 1.3 already supplied. Added Jason for his input as well.
> + global:
> + ibv_cmd_create_ah_ex;
When do you plan to contribute the driver code for that ? for now no
upstream drivers uses this API, taking some code for future usage with
no current usage is not the preferred way.
> +} IBVERBS_1.3;
>
--
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 libibverbs v2 3/3] Provide remote XRC SRQ number in kernel post_send.
From: Knut Omang @ 2016-09-20 11:08 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Mukesh Kacker
In-Reply-To: <20160920110115.GJ26673-2ukJVAZIZ/Y@public.gmane.org>
On Tue, 2016-09-20 at 14:01 +0300, Leon Romanovsky wrote:
> On Tue, Sep 20, 2016 at 12:43:30PM +0200, Knut Omang wrote:
> >
> > On Tue, 2016-09-20 at 13:18 +0300, Leon Romanovsky wrote:
> > >
> > > On Mon, Sep 19, 2016 at 11:12:36AM +0200, Knut Omang wrote:
> > > >
> > > >
> > > > On Mon, 2016-09-19 at 08:29 +0300, Leon Romanovsky wrote:
> > > > >
> > > > >
> > > > > On Sat, Sep 17, 2016 at 05:59:13AM +0200, Knut Omang wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > Also proper end align the ibv_kern_send_wr struct.
> > > > > >
> > > > > > Requires a corresponding kernel change.
> > > > > >
> > > > > > Signed-off-by: Knut Omang <knut.omang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > > > > > Reviewed-by: Mukesh Kacker <mukesh.kacker-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > > > > > ---
> > > > > > include/infiniband/kern-abi.h | 1 +
> > > > > > src/cmd.c | 2 ++
> > > > > > 2 files changed, 3 insertions(+), 0 deletions(-)
> > > > > >
> > > > > > diff --git a/include/infiniband/kern-abi.h b/include/infiniband/kern-abi.h
> > > > > > index 8bdeef5..7b1d310 100644
> > > > > > --- a/include/infiniband/kern-abi.h
> > > > > > +++ b/include/infiniband/kern-abi.h
> > > > > > @@ -800,6 +800,7 @@ struct ibv_kern_send_wr {
> > > > > > union {
> > > > > > struct {
> > > > > > __u32 remote_srqn;
> > > > > > + __u32 reserved;
> > > > > > } xrc;
> > > > > > } qp_type;
> > > > > > };
> > > > > > diff --git a/src/cmd.c b/src/cmd.c
> > > > > > index a418ee1..a4e2f75 100644
> > > > > > --- a/src/cmd.c
> > > > > > +++ b/src/cmd.c
> > > > > > @@ -1293,6 +1293,8 @@ int ibv_cmd_post_send(struct ibv_qp *ibqp, struct ibv_send_wr *wr,
> > > > > > tmp->wr.ud.remote_qpn = i->wr.ud.remote_qpn;
> > > > > > tmp->wr.ud.remote_qkey = i->wr.ud.remote_qkey;
> > > > > > } else {
> > > > > > + if (ibqp->qp_type == IBV_QPT_XRC_SEND)
> > > > > > + tmp->qp_type.xrc.remote_srqn = i->qp_type.xrc.remote_srqn;
> > > > > It will be checked for all QPTs and for all consumers. Any chances to
> > > > > optimize it?
> > > > All QPTs except UD, yes.
> > > >
> > > > The easiest is perhaps just to remove the test (and set the attribute in all cases), which will work fine
> > > > as long as the qp_type union is not used for anything else, but is it worth
> > > > the loss of intuitiveness/future potential correctness of the code?
> > > Let's concentrate on the present and as far as I see there are no kernel users who
> > > need this field xrc.remote_srqn field.
> > So I assume then that you are ok with this code, given my explanation?
> Partially,
>
> I'm fine with the claim about readability and the fact that
> this code is not in data-path relaxed me a little bit.
Good!
> I'm not fine with the part that this code doesn't do anything in kernel.
I know, I have two big patch sets one kernel and one for the new "rdma consolidated"
in the queue that should fix that, just want to save a few versions of those
~32000 lines of code by preparing the baseline :-)
Knut
>
> >
> >
> > Thanks,
> > Knut
> >
> > >
> > > >
> > > >
> > > >
> > > > Note that this is for user mode post_send implemented by the kernel, not the "fast path"
> > > > pure user mode.
> > > >
> > > > Knut
> > > >
> > > > >
> > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > switch (i->opcode) {
> > > > > > case IBV_WR_RDMA_WRITE:
> > > > > > case IBV_WR_RDMA_WRITE_WITH_IMM:
> > > > > > --
> > > > > > git-series 0.8.10
> > > > > > --
> > > > > > 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
* Re: [PATCH v2 5/8] ib_uverbs: Add padding to end align ib_uverbs_reg_mr_resp
From: Knut Omang @ 2016-09-20 11:02 UTC (permalink / raw)
To: Yishai Hadas
Cc: Doug Ledford, linux-rdma, linux-kernel, Sean Hefty,
Hal Rosenstock
In-Reply-To: <f049462a-5cd0-10bf-320b-c5bc634e93b9@dev.mellanox.co.il>
On Tue, 2016-09-20 at 13:45 +0300, Yishai Hadas wrote:
> On 9/16/2016 9:31 PM, Knut Omang wrote:
> >
> > The ib_uverbs_reg_mr_resp structure was not 64 bit end aligned
> > as required by the protocol. This causes alignment issues
> > if a device specific driver needs to transfer extra response
> > arguments.
> >
> > Avoid breaking backward compatibility by improving the handling
> > of the length checking in ib_uverbs_reg_mr to consider the case
> > where the kernel has been updated, but user space still has
> > the old length without padding.
> >
> > Signed-off-by: Knut Omang <knut.omang@oracle.com>
> > ---
> > drivers/infiniband/core/uverbs_cmd.c | 10 ++++++----
> > include/uapi/rdma/ib_user_verbs.h | 1 +
> > 2 files changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
> > index dbc5885..fa8a717 100644
> > --- a/drivers/infiniband/core/uverbs_cmd.c
> > +++ b/drivers/infiniband/core/uverbs_cmd.c
> > @@ -967,16 +967,18 @@ ssize_t ib_uverbs_reg_mr(struct ib_uverbs_file *file,
> > struct ib_pd *pd;
> > struct ib_mr *mr;
> > int ret;
> > + size_t resp_size = sizeof resp;
> >
> > - if (out_len < sizeof resp)
> > - return -ENOSPC;
> > + if (out_len < resp_size) {
> > + resp_size = out_len;
> > + }
> You don't preserve the minimum response size error checking in your code
> (i.e -ENOSPC). We can expect an error if outlen is less than the
> offset of rkey in ib_uverbs_reg_mr_resp.
I agree, the minimal size needs to be the minimal supported size,
in particular since the memory for cmd may contain arbitrary stack content.
Good catch!
> Please note that below call to copy_to_user will succeed as it copies
> now based on resp_size and won't return the basic mandatory output.
Get it - will fix,
Thanks,
Knut
>
> >
> > if (copy_from_user(&cmd, buf, sizeof cmd))
> > return -EFAULT;
> >
> > INIT_UDATA(&udata, buf + sizeof cmd,
> > (unsigned long) cmd.response + sizeof resp,
> > - in_len - sizeof cmd, out_len - sizeof resp);
> > + in_len - sizeof cmd, out_len - resp_size);
> >
> > if ((cmd.start & ~PAGE_MASK) != (cmd.hca_va & ~PAGE_MASK))
> > return -EINVAL;
> > @@ -1030,7 +1032,7 @@ ssize_t ib_uverbs_reg_mr(struct ib_uverbs_file *file,
> > resp.mr_handle = uobj->id;
> >
> > if (copy_to_user((void __user *) (unsigned long) cmd.response,
> > - &resp, sizeof resp)) {
> > + &resp, resp_size)) {
> > ret = -EFAULT;
> > goto err_copy;
> > }
> > diff --git a/include/uapi/rdma/ib_user_verbs.h b/include/uapi/rdma/ib_user_verbs.h
> > index 7f035f4..6b8c9c0 100644
> > --- a/include/uapi/rdma/ib_user_verbs.h
> > +++ b/include/uapi/rdma/ib_user_verbs.h
> > @@ -307,6 +307,7 @@ struct ib_uverbs_reg_mr_resp {
> > __u32 mr_handle;
> > __u32 lkey;
> > __u32 rkey;
> > + __u32 reserved;
> > };
> >
> > struct ib_uverbs_rereg_mr {
> >
^ permalink raw reply
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