Re: [patch] IB/mlx5: stack info leak in mlx5_ib_alloc_ucontext()

2013-07-28 Thread Eli Cohen 
On Thu, Jul 25, 2013 at 08:04:36PM +0300, Dan Carpenter wrote:
 We don't set resp.reserved.  Since it's at the end of the struct that
 means we don't have to copy it to the user.
 
 Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
 
 diff --git a/drivers/infiniband/hw/mlx5/main.c 
 b/drivers/infiniband/hw/mlx5/main.c
 index 8000fff..43dfb84 100644
 --- a/drivers/infiniband/hw/mlx5/main.c
 +++ b/drivers/infiniband/hw/mlx5/main.c
 @@ -619,7 +619,8 @@ static struct ib_ucontext *mlx5_ib_alloc_ucontext(struct 
 ib_device *ibdev,
  
   resp.tot_uuars = req.total_num_uuars;
   resp.num_ports = dev-mdev.caps.num_ports;
 - err = ib_copy_to_udata(udata, resp, sizeof(resp));
 + err = ib_copy_to_udata(udata, resp,
 +sizeof(resp) - sizeof(resp.reserved));
   if (err)
   goto out_uars;


I don't have strong opinion on this one. The title of the patch is
stack info leak but the only leak of a reserved field. Other opinions?
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-3.11 7/7] IB/iser: Introduce fast memory registration model (FRWR)

2013-07-28 Thread Or Gerlitz 

On 26/07/2013 20:15, Vu Pham wrote:

Hello Or/Sagi,

Just a minor

 /**
+ * iser_create_frwr_pool - Creates pool of fast_reg descriptors
+ * for fast registration work requests.
+ * returns 0 on success, or errno code on failure
+ */
+int iser_create_frwr_pool(struct iser_conn *ib_conn, unsigned cmds_max)
+{
+struct iser_device*device = ib_conn-device;
+struct fast_reg_descriptor*desc;
+int i, ret;
+
+ INIT_LIST_HEAD(ib_conn-fastreg.frwr.pool);
+ib_conn-fastreg.frwr.pool_size = 0;
+for (i = 0; i  cmds_max; i++) {
+desc = kmalloc(sizeof(*desc), GFP_KERNEL);
+if (!desc) {
+iser_err(Failed to allocate a new fast_reg descriptor\n);
+ret = -ENOMEM;
+goto err;
+}
+
+desc-data_frpl = 
ib_alloc_fast_reg_page_list(device-ib_device,

+ ISCSI_ISER_SG_TABLESIZE + 1);
+if (IS_ERR(desc-data_frpl)) {

ret = PTR_ERR(desc-data_frpl);
+iser_err(Failed to allocate ib_fast_reg_page_list 
err=%ld\n,

+ PTR_ERR(desc-data_frpl));

using ret

+goto err;
+}
+
+desc-data_mr = ib_alloc_fast_reg_mr(device-pd,
+ ISCSI_ISER_SG_TABLESIZE + 1);
+if (IS_ERR(desc-data_mr)) {

ret = PTR_ERR(desc-data_mr);

+iser_err(Failed to allocate ib_fast_reg_mr err=%ld\n,
+ PTR_ERR(desc-data_mr));

using ret

+ ib_free_fast_reg_page_list(desc-data_frpl);
+goto err;
+}
+desc-valid = true;
+list_add_tail(desc-list, ib_conn-fastreg.frwr.pool);
+ib_conn-fastreg.frwr.pool_size++;
+}
+
+return 0;
+err:
+iser_free_frwr_pool(ib_conn);
+return ret;
+}





Nice catch!

I see that Roland hasn't yet picked this series so I will re-submit it 
with fixes to the issues you have found here.


Or.

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

[PATCH 0/4] librdmacm compiler warning fixes

2013-07-28 Thread Bart Van Assche 
This is a small series of four patches addressing issues I noticed while 
analyzing the warnings reported by a recent gcc version. These patches 
do not change the behavior of librdmacm.

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

[PATCH 1/4] acm: Remove the unused variable 'pri_path'

2013-07-28 Thread Bart Van Assche 
The variable 'pri_path' is assigned a value but is never used.
This triggers the following compiler warning:

src/acm.c:301:26: warning: variable 'pri_path' set but not used 
[-Wunused-but-set-variable]

Hence remove this variable.

Signed-off-by: Bart Van Assche bvanass...@acm.org
---
 src/acm.c |4 
 1 file changed, 4 deletions(-)

diff --git a/src/acm.c b/src/acm.c
index 04cddee..49bda48 100644
--- a/src/acm.c
+++ b/src/acm.c
@@ -297,7 +297,6 @@ static void ucma_ib_save_resp(struct rdma_addrinfo *rai, 
struct acm_msg *msg)
 {
struct acm_ep_addr_data *ep_data;
struct ibv_path_data *path_data = NULL;
-   struct ibv_path_record *pri_path = NULL;
struct sockaddr_in *sin;
struct sockaddr_in6 *sin6;
int i, cnt, path_cnt = 0;
@@ -311,9 +310,6 @@ static void ucma_ib_save_resp(struct rdma_addrinfo *rai, 
struct acm_msg *msg)
if (!path_data)
path_data = (struct ibv_path_data *) ep_data;
path_cnt++;
-   if (ep_data-flags |
-   (IBV_PATH_FLAG_PRIMARY | IBV_PATH_FLAG_OUTBOUND))
-   pri_path = path_data[i].path;
break;
case ACM_EP_INFO_ADDRESS_IP:
if (!(ep_data-flags  ACM_EP_FLAG_SOURCE) || 
rai-ai_src_len)
-- 
1.7.10.4

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

[PATCH 2/4] cma: Remove the unused variable 'id_priv'

2013-07-28 Thread Bart Van Assche 
The variable 'id_priv' is assigned a value but is never used.
This triggers the following compiler warning:

src/cma.c:1178:25: warning: variable 'id_priv' set but not used 
[-Wunused-but-set-variable]

Hence remove this variable.

Signed-off-by: Bart Van Assche bvanass...@acm.org
---
 src/cma.c |2 --
 1 file changed, 2 deletions(-)

diff --git a/src/cma.c b/src/cma.c
index baebecd..374844c 100644
--- a/src/cma.c
+++ b/src/cma.c
@@ -1175,11 +1175,9 @@ err:
 int rdma_create_srq(struct rdma_cm_id *id, struct ibv_pd *pd,
struct ibv_srq_init_attr *attr)
 {
-   struct cma_id_private *id_priv;
struct ibv_srq *srq;
int ret;
 
-   id_priv = container_of(id, struct cma_id_private, id);
if (!pd)
pd = id-pd;
 
-- 
1.7.10.4

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

[PATCH 3/4] rsocket: Remove the unused variable 'ret'

2013-07-28 Thread Bart Van Assche 
The variable 'ret' is assigned a value but that value is never used.
This triggers the following compiler warning:

src/rsocket.c:3720:9: warning: variable 'ret' set but not used 
[-Wunused-but-set-variable]

Hence remove this variable.

Signed-off-by: Bart Van Assche bvanass...@acm.org
---
 src/rsocket.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/rsocket.c b/src/rsocket.c
index abdd392..ef24dfb 100644
--- a/src/rsocket.c
+++ b/src/rsocket.c
@@ -3717,11 +3717,11 @@ static void rs_svc_process_sock(void)
 static uint8_t rs_svc_sgid_index(struct ds_dest *dest, union ibv_gid *sgid)
 {
union ibv_gid gid;
-   int i, ret;
+   int i;
 
for (i = 0; i  16; i++) {
-   ret = ibv_query_gid(dest-qp-cm_id-verbs, 
dest-qp-cm_id-port_num,
-   i, gid);
+   ibv_query_gid(dest-qp-cm_id-verbs, dest-qp-cm_id-port_num,
+ i, gid);
if (!memcmp(sgid, gid, sizeof gid))
return i;
}
-- 
1.7.10.4

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

[PATCH 4/4] Declare 'server_port' as an unsigned variable

2013-07-28 Thread Bart Van Assche 
Change the data type of the 'server_port' variable from signed to
unsigned such that the cast in the fscanf() call can be removed.

Signed-off-by: Bart Van Assche bvanass...@acm.org
---
 src/acm.c |5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/acm.c b/src/acm.c
index 49bda48..33379cf 100644
--- a/src/acm.c
+++ b/src/acm.c
@@ -35,6 +35,7 @@
 #endif /* HAVE_CONFIG_H */
 
 #include stdio.h
+#include inttypes.h
 #include sys/types.h
 #include sys/socket.h
 #include netdb.h
@@ -113,7 +114,7 @@ struct acm_msg {
 
 static pthread_mutex_t acm_lock = PTHREAD_MUTEX_INITIALIZER;
 static int sock = -1;
-static short server_port;
+static uint16_t server_port;
 
 struct ib_connect_hdr {
uint8_t  cma_version;
@@ -132,7 +133,7 @@ static int ucma_set_server_port(void)
FILE *f;
 
if ((f = fopen(/var/run/ibacm.port, r STREAM_CLOEXEC))) {
-   fscanf(f, %hu, (unsigned short *) server_port);
+   fscanf(f, % SCNu16, server_port);
fclose(f);
}
return server_port;
-- 
1.7.10.4

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

Re: [PATCH for-3.11 7/7] IB/iser: Introduce fast memory registration model (FRWR)

2013-07-28 Thread Sagi Grimberg 

On 7/28/2013 11:15 AM, Or Gerlitz wrote:

On 26/07/2013 20:15, Vu Pham wrote:

Hello Or/Sagi,

Just a minor

 /**
+ * iser_create_frwr_pool - Creates pool of fast_reg descriptors
+ * for fast registration work requests.
+ * returns 0 on success, or errno code on failure
+ */
+int iser_create_frwr_pool(struct iser_conn *ib_conn, unsigned 
cmds_max)

+{
+struct iser_device*device = ib_conn-device;
+struct fast_reg_descriptor*desc;
+int i, ret;
+
+ INIT_LIST_HEAD(ib_conn-fastreg.frwr.pool);
+ib_conn-fastreg.frwr.pool_size = 0;
+for (i = 0; i  cmds_max; i++) {
+desc = kmalloc(sizeof(*desc), GFP_KERNEL);
+if (!desc) {
+iser_err(Failed to allocate a new fast_reg 
descriptor\n);

+ret = -ENOMEM;
+goto err;
+}
+
+desc-data_frpl = 
ib_alloc_fast_reg_page_list(device-ib_device,

+ ISCSI_ISER_SG_TABLESIZE + 1);
+if (IS_ERR(desc-data_frpl)) {

ret = PTR_ERR(desc-data_frpl);
+iser_err(Failed to allocate ib_fast_reg_page_list 
err=%ld\n,

+ PTR_ERR(desc-data_frpl));

using ret

+goto err;
+}
+
+desc-data_mr = ib_alloc_fast_reg_mr(device-pd,
+ ISCSI_ISER_SG_TABLESIZE + 1);
+if (IS_ERR(desc-data_mr)) {

ret = PTR_ERR(desc-data_mr);

+iser_err(Failed to allocate ib_fast_reg_mr err=%ld\n,
+ PTR_ERR(desc-data_mr));

using ret

+ ib_free_fast_reg_page_list(desc-data_frpl);
+goto err;
+}
+desc-valid = true;
+list_add_tail(desc-list, ib_conn-fastreg.frwr.pool);
+ib_conn-fastreg.frwr.pool_size++;
+}
+
+return 0;
+err:
+iser_free_frwr_pool(ib_conn);
+return ret;
+}





Nice catch!

I see that Roland hasn't yet picked this series so I will re-submit it 
with fixes to the issues you have found here.


Or.



Nice catch indeed, thanks Vu.

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

[PATCH V1 for-3.11 1/7] IB/iser: Use proper debug level value for info prints

2013-07-28 Thread Or Gerlitz 
Commit 4f363882612 IB/iser: Move informational messages from error to info 
level
was setting info prints to require lower value for the debug level vs warning 
prints
which isn't the common convention, fix that. Also move the prints on unaligned 
SG
from warning to debug level.

Signed-off-by: Or Gerlitz ogerl...@mellanox.com
---
 drivers/infiniband/ulp/iser/iscsi_iser.h  |4 ++--
 drivers/infiniband/ulp/iser/iser_memory.c |   13 ++---
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h 
b/drivers/infiniband/ulp/iser/iscsi_iser.h
index 4f069c0..d694bcd 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -78,14 +78,14 @@
 
 #define iser_warn(fmt, arg...) \
do {\
-   if (iser_debug_level  1)   \
+   if (iser_debug_level  0)   \
pr_warn(PFX %s: fmt,  \
__func__ , ## arg); \
} while (0)
 
 #define iser_info(fmt, arg...) \
do {\
-   if (iser_debug_level  0)   \
+   if (iser_debug_level  1)   \
pr_info(PFX %s: fmt,  \
__func__ , ## arg); \
} while (0)
diff --git a/drivers/infiniband/ulp/iser/iser_memory.c 
b/drivers/infiniband/ulp/iser/iser_memory.c
index 7827baf..797e49f 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -267,11 +267,8 @@ static void iser_data_buf_dump(struct iser_data_buf *data,
struct scatterlist *sg;
int i;
 
-   if (iser_debug_level == 0)
-   return;
-
for_each_sg(sgl, sg, data-dma_nents, i)
-   iser_warn(sg[%d] dma_addr:0x%lX page:0x%p 
+   iser_dbg(sg[%d] dma_addr:0x%lX page:0x%p 
 off:0x%x sz:0x%x dma_len:0x%x\n,
 i, (unsigned long)ib_sg_dma_address(ibdev, sg),
 sg_page(sg), sg-offset,
@@ -373,9 +370,11 @@ int iser_reg_rdma_mem(struct iscsi_iser_task *iser_task,
if (aligned_len != mem-dma_nents ||
(!ib_conn-fmr_pool  mem-dma_nents  1)) {
iscsi_conn-fmr_unalign_cnt++;
-   iser_warn(rdma alignment violation (%d/%d aligned) or FMR not 
supported\n,
- aligned_len, mem-size);
-   iser_data_buf_dump(mem, ibdev);
+   iser_dbg(rdma alignment violation (%d/%d aligned) or FMR not 
supported\n,
+aligned_len, mem-size);
+
+   if (iser_debug_level  0)
+   iser_data_buf_dump(mem, ibdev);
 
/* unmap the command data before accessing it */
iser_dma_unmap_task_data(iser_task);
-- 
1.7.1

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

[PATCH V1 for-3.11 2/7] IB/iser: Restructure allocation/deallocation of connection resources

2013-07-28 Thread Or Gerlitz 
From: Shlomo Pongratz shlo...@mellanox.com

This is a preparation step to a patch that accepts the number of max SCSI
commands to be supported for the session from the user space iSCSI tools.

Move the allocation of the login buffer, FMR pool and its associated
page vector from iser_create_ib_conn_res() which is called prior to
a point in time where we actually know how many commands should be
supported to iser_alloc_rx_descriptors() which is called during the
iscsi connection bind step where this quantity is known.

Also do small refactoring around the deallocation to make that path
similar to the allocation one.

Signed-off-by: Shlomo Pongratz shlo...@mellanox.com
Signed-off-by: Or Gerlitz ogerl...@mellanox.com
---
 drivers/infiniband/ulp/iser/iscsi_iser.h |2 +
 drivers/infiniband/ulp/iser/iser_initiator.c |   92 ++-
 drivers/infiniband/ulp/iser/iser_verbs.c |  128 --
 3 files changed, 151 insertions(+), 71 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h 
b/drivers/infiniband/ulp/iser/iscsi_iser.h
index d694bcd..fee8829 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -395,4 +395,6 @@ void iser_dma_unmap_task_data(struct iscsi_iser_task 
*iser_task);
 int  iser_initialize_task_headers(struct iscsi_task *task,
struct iser_tx_desc *tx_desc);
 int iser_alloc_rx_descriptors(struct iser_conn *ib_conn);
+int iser_create_fmr_pool(struct iser_conn *ib_conn);
+void iser_free_fmr_pool(struct iser_conn *ib_conn);
 #endif
diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c 
b/drivers/infiniband/ulp/iser/iser_initiator.c
index b6d81a8..626d950 100644
--- a/drivers/infiniband/ulp/iser/iser_initiator.c
+++ b/drivers/infiniband/ulp/iser/iser_initiator.c
@@ -170,6 +170,76 @@ static void iser_create_send_desc(struct iser_conn 
*ib_conn,
}
 }
 
+static void iser_free_login_buf(struct iser_conn *ib_conn)
+{
+   if (!ib_conn-login_buf)
+   return;
+
+   if (ib_conn-login_req_dma)
+   ib_dma_unmap_single(ib_conn-device-ib_device,
+   ib_conn-login_req_dma,
+   ISCSI_DEF_MAX_RECV_SEG_LEN, DMA_TO_DEVICE);
+
+   if (ib_conn-login_resp_dma)
+   ib_dma_unmap_single(ib_conn-device-ib_device,
+   ib_conn-login_resp_dma,
+   ISER_RX_LOGIN_SIZE, DMA_FROM_DEVICE);
+
+   kfree(ib_conn-login_buf);
+
+   /* make sure we never redo any unmapping */
+   ib_conn-login_req_dma = 0;
+   ib_conn-login_resp_dma = 0;
+   ib_conn-login_buf = NULL;
+}
+
+static int iser_alloc_login_buf(struct iser_conn *ib_conn)
+{
+   struct iser_device  *device;
+   int req_err, resp_err;
+
+   BUG_ON(ib_conn-device == NULL);
+
+   device = ib_conn-device;
+
+   ib_conn-login_buf = kmalloc(ISCSI_DEF_MAX_RECV_SEG_LEN +
+ISER_RX_LOGIN_SIZE, GFP_KERNEL);
+   if (!ib_conn-login_buf)
+   goto out_err;
+
+   ib_conn-login_req_buf  = ib_conn-login_buf;
+   ib_conn-login_resp_buf = ib_conn-login_buf +
+   ISCSI_DEF_MAX_RECV_SEG_LEN;
+
+   ib_conn-login_req_dma = ib_dma_map_single(ib_conn-device-ib_device,
+   (void *)ib_conn-login_req_buf,
+   ISCSI_DEF_MAX_RECV_SEG_LEN, DMA_TO_DEVICE);
+
+   ib_conn-login_resp_dma = ib_dma_map_single(ib_conn-device-ib_device,
+   (void *)ib_conn-login_resp_buf,
+   ISER_RX_LOGIN_SIZE, DMA_FROM_DEVICE);
+
+   req_err  = ib_dma_mapping_error(device-ib_device,
+   ib_conn-login_req_dma);
+   resp_err = ib_dma_mapping_error(device-ib_device,
+   ib_conn-login_resp_dma);
+
+   if (req_err || resp_err) {
+   if (req_err)
+   ib_conn-login_req_dma = 0;
+   if (resp_err)
+   ib_conn-login_resp_dma = 0;
+   goto free_login_buf;
+   }
+   return 0;
+
+free_login_buf:
+   iser_free_login_buf(ib_conn);
+
+out_err:
+   iser_err(unable to alloc or map login buf\n);
+   return -ENOMEM;
+}
 
 int iser_alloc_rx_descriptors(struct iser_conn *ib_conn)
 {
@@ -179,6 +249,12 @@ int iser_alloc_rx_descriptors(struct iser_conn *ib_conn)
struct ib_sge   *rx_sg;
struct iser_device  *device = ib_conn-device;
 
+   if (iser_create_fmr_pool(ib_conn))
+   goto create_fmr_pool_failed;
+
+   if (iser_alloc_login_buf(ib_conn))
+   goto alloc_login_buf_fail;
+
ib_conn-rx_descs = kmalloc(ISER_QP_MAX_RECV_DTOS *
sizeof(struct iser_rx_desc), GFP_KERNEL);
if (!ib_conn-rx_descs)
@@

[PATCH V1 for-3.11 0/7] Add Fast-Reg support to the iser initiator driver

2013-07-28 Thread Or Gerlitz 
changes from V0:

  - fixed error flow in iser_create_frwr_pool() to return correct value, as
pointed out by Vu Pham through code-review.

  - changed iser_free_rx_descriptors() so it handles properly failures which
take place early in the connection establishment process. E.g need to check
if the device exists and it has iser_free_rdma_reg_res function pointer set,
before calling it over (found using error injection during regression).

Hi Roland,

Here's the FRWR (Fast Reg WR) patch series for iser from Sagi Grimberg.

This is kind of critical so we can avoid using bounce buffers when running
over Connect-IB (the newly introduced mlx5 driver) and ConnectX VF which both
don't support FMRs.

Patches 1-3 are the other iser updates for 3.11, which reposted here for ease
of merging, I have submitted them earlier for 3.11 but you missed picking them.

Patches 4-6 are simple pre-steps for allowing more registration models such as
FastReg WRs except for FMR, and the last patch adds the actual support for 
fastreg.


Or.


Or Gerlitz (1):
  IB/iser: Use proper debug level value for info prints

Sagi Grimberg (4):
  IB/iser: Generalize rdma memory registration
  IB/iser: Handle unaligned SG in separate function
  IB/iser: Place the fmr pool into a union in iser's IB conn struct
  IB/iser: Introduce fast memory registration model (FRWR)

Shlomo Pongratz (2):
  IB/iser: Restructure allocation/deallocation of connection resources
  IB/iser: Accept session-cmds_max from user space

 drivers/infiniband/ulp/iser/iscsi_iser.c |   19 +-
 drivers/infiniband/ulp/iser/iscsi_iser.h |   73 ++--
 drivers/infiniband/ulp/iser/iser_initiator.c |  139 ++---
 drivers/infiniband/ulp/iser/iser_memory.c|  231 +
 drivers/infiniband/ulp/iser/iser_verbs.c |  288 ++---
 5 files changed, 585 insertions(+), 165 deletions(-)

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

[PATCH V1 for-3.11 3/7] IB/iser: Accept session-cmds_max from user space

2013-07-28 Thread Or Gerlitz 
From: Shlomo Pongratz shlo...@mellanox.com

Use cmds_max passed from user space to be the number of PDUs to be
supported for the session instead of hard-coded ISCSI_DEF_XMIT_CMDS_MAX.
Specifically, this allows to control the max number of SCSI commands
for the seesion. Also don't ignore the qdepth passed from user space.

Derive from session-cmds_max the actual number of RX buffers
and FMR pool size to allocate during the connection bind phase.

Since the iser transport connection is established before the iscsi
session/connection are created and bounded, we still use one hard coded
quantity ISER_DEF_XMIT_CMDS_MAX to compute the maximum number of
work-requests to be supported by the RC QP used for the connection.

The above quantity is made to be a power of two between ISCSI_TOTAL_CMDS_MIN
(16) and ISER_DEF_XMIT_CMDS_MAX (512) inclusive.

Signed-off-by: Shlomo Pongratz shlo...@mellanox.com
Signed-off-by: Or Gerlitz ogerl...@mellanox.com
---
 drivers/infiniband/ulp/iser/iscsi_iser.c |   19 ---
 drivers/infiniband/ulp/iser/iscsi_iser.h |   21 +++--
 drivers/infiniband/ulp/iser/iser_initiator.c |   25 +++--
 drivers/infiniband/ulp/iser/iser_verbs.c |8 
 4 files changed, 46 insertions(+), 27 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c 
b/drivers/infiniband/ulp/iser/iscsi_iser.c
index 2e84ef8..705de7b 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -347,6 +347,7 @@ iscsi_iser_conn_bind(struct iscsi_cls_session *cls_session,
 {
struct iscsi_conn *conn = cls_conn-dd_data;
struct iscsi_iser_conn *iser_conn;
+   struct iscsi_session *session;
struct iser_conn *ib_conn;
struct iscsi_endpoint *ep;
int error;
@@ -365,7 +366,8 @@ iscsi_iser_conn_bind(struct iscsi_cls_session *cls_session,
}
ib_conn = ep-dd_data;
 
-   if (iser_alloc_rx_descriptors(ib_conn))
+   session = conn-session;
+   if (iser_alloc_rx_descriptors(ib_conn, session))
return -ENOMEM;
 
/* binds the iSER connection retrieved from the previously
@@ -419,12 +421,13 @@ iscsi_iser_session_create(struct iscsi_endpoint *ep,
struct iscsi_cls_session *cls_session;
struct iscsi_session *session;
struct Scsi_Host *shost;
-   struct iser_conn *ib_conn;
+   struct iser_conn *ib_conn = NULL;
 
shost = iscsi_host_alloc(iscsi_iser_sht, 0, 0);
if (!shost)
return NULL;
shost-transportt = iscsi_iser_scsi_transport;
+   shost-cmd_per_lun = qdepth;
shost-max_lun = iscsi_max_lun;
shost-max_id = 0;
shost-max_channel = 0;
@@ -441,12 +444,14 @@ iscsi_iser_session_create(struct iscsi_endpoint *ep,
   ep ? ib_conn-device-ib_device-dma_device : NULL))
goto free_host;
 
-   /*
-* we do not support setting can_queue cmd_per_lun from userspace yet
-* because we preallocate so many resources
-*/
+   if (cmds_max  ISER_DEF_XMIT_CMDS_MAX) {
+   iser_info(cmds_max changed from %u to %u\n,
+ cmds_max, ISER_DEF_XMIT_CMDS_MAX);
+   cmds_max = ISER_DEF_XMIT_CMDS_MAX;
+   }
+
cls_session = iscsi_session_setup(iscsi_iser_transport, shost,
- ISCSI_DEF_XMIT_CMDS_MAX, 0,
+ cmds_max, 0,
  sizeof(struct iscsi_iser_task),
  initial_cmdsn, 0);
if (!cls_session)
diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h 
b/drivers/infiniband/ulp/iser/iscsi_iser.h
index fee8829..d2fc55a 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -102,7 +102,13 @@
 
/* support up to 512KB in one RDMA */
 #define ISCSI_ISER_SG_TABLESIZE (0x8  SHIFT_4K)
-#define ISER_DEF_CMD_PER_LUN   ISCSI_DEF_XMIT_CMDS_MAX
+#define ISER_DEF_XMIT_CMDS_DEFAULT 512
+#if ISCSI_DEF_XMIT_CMDS_MAX  ISER_DEF_XMIT_CMDS_DEFAULT
+   #define ISER_DEF_XMIT_CMDS_MAX  ISCSI_DEF_XMIT_CMDS_MAX
+#else
+   #define ISER_DEF_XMIT_CMDS_MAX  ISER_DEF_XMIT_CMDS_DEFAULT
+#endif
+#define ISER_DEF_CMD_PER_LUN   ISER_DEF_XMIT_CMDS_MAX
 
 /* QP settings */
 /* Maximal bounds on received asynchronous PDUs */
@@ -111,9 +117,9 @@
 #define ISER_MAX_TX_MISC_PDUS  6 /* NOOP_OUT(2), TEXT(1), *
   * SCSI_TMFUNC(2), LOGOUT(1) */
 
-#define ISER_QP_MAX_RECV_DTOS  (ISCSI_DEF_XMIT_CMDS_MAX)
+#define ISER_QP_MAX_RECV_DTOS  (ISER_DEF_XMIT_CMDS_MAX)
 
-#define ISER_MIN_POSTED_RX (ISCSI_DEF_XMIT_CMDS_MAX  2)
+#define ISER_MIN_POSTED_RX (ISER_DEF_XMIT_CMDS_MAX  2)
 
 /* the max TX (send)

[PATCH V1 for-3.11 6/7] IB/iser: Place the fmr pool into a union in iser's IB conn struct

2013-07-28 Thread Or Gerlitz 
From: Sagi Grimberg sa...@mellanox.com

This is preparation step for other memory registration methods to be
added. In addition, change reg/unreg routines signature to indicate
they use FMRs.

Signed-off-by: Sagi Grimberg sa...@mellanox.com
Signed-off-by: Or Gerlitz ogerl...@mellanox.com
---
 drivers/infiniband/ulp/iser/iscsi_iser.h  |   18 +++
 drivers/infiniband/ulp/iser/iser_memory.c |   24 ---
 drivers/infiniband/ulp/iser/iser_verbs.c  |   47 +++--
 3 files changed, 48 insertions(+), 41 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h 
b/drivers/infiniband/ulp/iser/iscsi_iser.h
index 78117be..75c5352 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -286,7 +286,6 @@ struct iser_conn {
struct iser_device   *device;   /* device context  
*/
struct rdma_cm_id*cma_id;   /* CMA ID  
*/
struct ib_qp *qp;   /* QP  
*/
-   struct ib_fmr_pool   *fmr_pool; /* pool of IB FMRs 
*/
wait_queue_head_twait;  /* waitq for conn/disconn  
*/
unsigned qp_max_recv_dtos; /* num of rx buffers */
unsigned qp_max_recv_dtos_mask; /* above minus 1 */
@@ -294,8 +293,6 @@ struct iser_conn {
int  post_recv_buf_count; /* posted rx count  */
atomic_t post_send_buf_count; /* posted tx count   
*/
char name[ISER_OBJECT_NAME_SIZE];
-   struct iser_page_vec *page_vec; /* represents SG to fmr 
maps*
-* maps serialized as tx 
is*/
struct list_head conn_list;   /* entry in ig conn list 
*/
 
char *login_buf;
@@ -304,6 +301,13 @@ struct iser_conn {
unsigned int rx_desc_head;
struct iser_rx_desc  *rx_descs;
struct ib_recv_wrrx_wr[ISER_MIN_POSTED_RX];
+   union {
+   struct {
+   struct ib_fmr_pool  *pool; /* pool of IB FMRs   
  */
+   struct iser_page_vec*page_vec; /* represents SG to 
fmr maps*
+   * maps serialized 
as tx is*/
+   } fmr;
+   } fastreg;
 };
 
 struct iscsi_iser_conn {
@@ -387,8 +391,8 @@ void iser_free_rx_descriptors(struct iser_conn *ib_conn);
 void iser_finalize_rdma_unaligned_sg(struct iscsi_iser_task *task,
 enum iser_data_dir cmd_dir);
 
-int  iser_reg_rdma_mem(struct iscsi_iser_task *task,
-  enum   iser_data_dircmd_dir);
+int  iser_reg_rdma_mem_fmr(struct iscsi_iser_task *task,
+  enum iser_data_dir cmd_dir);
 
 int  iser_connect(struct iser_conn   *ib_conn,
  struct sockaddr_in *src_addr,
@@ -399,8 +403,8 @@ int  iser_reg_page_vec(struct iser_conn *ib_conn,
   struct iser_page_vec *page_vec,
   struct iser_mem_reg  *mem_reg);
 
-void iser_unreg_mem(struct iscsi_iser_task *iser_task,
-   enum iser_data_dir cmd_dir);
+void iser_unreg_mem_fmr(struct iscsi_iser_task *iser_task,
+   enum iser_data_dir cmd_dir);
 
 int  iser_post_recvl(struct iser_conn *ib_conn);
 int  iser_post_recvm(struct iser_conn *ib_conn, int count);
diff --git a/drivers/infiniband/ulp/iser/iser_memory.c 
b/drivers/infiniband/ulp/iser/iser_memory.c
index 4dea1ba..1985e90 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -374,13 +374,13 @@ static int fall_to_bounce_buf(struct iscsi_iser_task 
*iser_task,
 }
 
 /**
- * iser_reg_rdma_mem - Registers memory intended for RDMA,
- * obtaining rkey and va
+ * iser_reg_rdma_mem_fmr - Registers memory intended for RDMA,
+ * using FMR (if possible) obtaining rkey and va
  *
  * returns 0 on success, errno code on failure
  */
-int iser_reg_rdma_mem(struct iscsi_iser_task *iser_task,
- enum   iser_data_dircmd_dir)
+int iser_reg_rdma_mem_fmr(struct iscsi_iser_task *iser_task,
+ enum iser_data_dir cmd_dir)
 {
struct iser_conn *ib_conn = iser_task-iser_conn-ib_conn;
struct iser_device   *device = ib_conn-device;
@@ -396,7 +396,7 @@ int iser_reg_rdma_mem(struct iscsi_iser_task *iser_task,
 
aligned_len = iser_data_buf_aligned_len(mem, ibdev);
if (aligned_len != mem-dma_nents ||
-   (!ib_conn-fmr_pool  mem-dma_nents  1)) {
+   (!ib_conn-fastreg.fmr.pool  mem-dma_nents  1)) {
err = fall_to_bounce_buf(iser_task, ibdev,
 cmd_dir, aligned_len);
if (err)

[PATCH V1 for-3.11 5/7] IB/iser: Handle unaligned SG in separate function

2013-07-28 Thread Or Gerlitz 
From: Sagi Grimberg sa...@mellanox.com

This routine will be shared with other rdma management schemes. The bounce
buffer solution for unaligned SG-lists and the sg_to_page_vec routine are
likely to be used for other registration schemes and not just FMR.

Move them out of the FMR specific code, and call them from there, later
they will be called also from other reg methods code.

Signed-off-by: Sagi Grimberg sa...@mellanox.com
Signed-off-by: Or Gerlitz ogerl...@mellanox.com
---
 drivers/infiniband/ulp/iser/iser_memory.c |   66 +++--
 1 files changed, 43 insertions(+), 23 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iser_memory.c 
b/drivers/infiniband/ulp/iser/iser_memory.c
index 797e49f..4dea1ba 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -170,8 +170,8 @@ void iser_finalize_rdma_unaligned_sg(struct iscsi_iser_task 
*iser_task,
  */
 
 static int iser_sg_to_page_vec(struct iser_data_buf *data,
-  struct iser_page_vec *page_vec,
-  struct ib_device *ibdev)
+  struct ib_device *ibdev, u64 *pages,
+  int *offset, int *data_size)
 {
struct scatterlist *sg, *sgl = (struct scatterlist *)data-buf;
u64 start_addr, end_addr, page, chunk_start = 0;
@@ -180,7 +180,7 @@ static int iser_sg_to_page_vec(struct iser_data_buf *data,
int i, new_chunk, cur_page, last_ent = data-dma_nents - 1;
 
/* compute the offset of first element */
-   page_vec-offset = (u64) sgl[0].offset  ~MASK_4K;
+   *offset = (u64) sgl[0].offset  ~MASK_4K;
 
new_chunk = 1;
cur_page  = 0;
@@ -204,13 +204,14 @@ static int iser_sg_to_page_vec(struct iser_data_buf *data,
   which might be unaligned */
page = chunk_start  MASK_4K;
do {
-   page_vec-pages[cur_page++] = page;
+   pages[cur_page++] = page;
page += SIZE_4K;
} while (page  end_addr);
}
 
-   page_vec-data_size = total_sz;
-   iser_dbg(page_vec-data_size:%d cur_page %d\n, 
page_vec-data_size,cur_page);
+   *data_size = total_sz;
+   iser_dbg(page_vec-data_size:%d cur_page %d\n,
+*data_size, cur_page);
return cur_page;
 }
 
@@ -295,8 +296,10 @@ static void iser_page_vec_build(struct iser_data_buf *data,
page_vec-offset = 0;
 
iser_dbg(Translating sg sz: %d\n, data-dma_nents);
-   page_vec_len = iser_sg_to_page_vec(data, page_vec, ibdev);
-   iser_dbg(sg len %d page_vec_len %d\n, data-dma_nents,page_vec_len);
+   page_vec_len = iser_sg_to_page_vec(data, ibdev, page_vec-pages,
+  page_vec-offset,
+  page_vec-data_size);
+   iser_dbg(sg len %d page_vec_len %d\n, data-dma_nents, page_vec_len);
 
page_vec-length = page_vec_len;
 
@@ -344,6 +347,32 @@ void iser_dma_unmap_task_data(struct iscsi_iser_task 
*iser_task)
}
 }
 
+static int fall_to_bounce_buf(struct iscsi_iser_task *iser_task,
+ struct ib_device *ibdev,
+ enum iser_data_dir cmd_dir,
+ int aligned_len)
+{
+   struct iscsi_conn*iscsi_conn = iser_task-iser_conn-iscsi_conn;
+   struct iser_data_buf *mem = iser_task-data[cmd_dir];
+
+   iscsi_conn-fmr_unalign_cnt++;
+   iser_warn(rdma alignment violation (%d/%d aligned) or FMR not 
supported\n,
+ aligned_len, mem-size);
+
+   if (iser_debug_level  0)
+   iser_data_buf_dump(mem, ibdev);
+
+   /* unmap the command data before accessing it */
+   iser_dma_unmap_task_data(iser_task);
+
+   /* allocate copy buf, if we are writing, copy the */
+   /* unaligned scatterlist, dma map the copy*/
+   if (iser_start_rdma_unaligned_sg(iser_task, cmd_dir) != 0)
+   return -ENOMEM;
+
+   return 0;
+}
+
 /**
  * iser_reg_rdma_mem - Registers memory intended for RDMA,
  * obtaining rkey and va
@@ -353,7 +382,6 @@ void iser_dma_unmap_task_data(struct iscsi_iser_task 
*iser_task)
 int iser_reg_rdma_mem(struct iscsi_iser_task *iser_task,
  enum   iser_data_dircmd_dir)
 {
-   struct iscsi_conn*iscsi_conn = iser_task-iser_conn-iscsi_conn;
struct iser_conn *ib_conn = iser_task-iser_conn-ib_conn;
struct iser_device   *device = ib_conn-device;
struct ib_device *ibdev = device-ib_device;
@@ -369,20 +397,12 @@ int iser_reg_rdma_mem(struct iscsi_iser_task *iser_task,
aligned_len = iser_data_buf_aligned_len(mem, ibdev);
if (aligned_len != mem-dma_nents ||
(!ib_conn-fmr_pool  mem-dma_nents  1)) {
-   iscsi_conn-fmr_unalign_cnt++;
-   iser_dbg(rdma alignment

[PATCH V1 for-3.11 7/7] IB/iser: Introduce fast memory registration model (FRWR)

2013-07-28 Thread Or Gerlitz 
From: Sagi Grimberg sa...@mellanox.com

Newer HCAs and Virtual functions may not support FMRs but rather a fast
registration model, which we call FRWR - Fast Registration Work Requests.

This model was introduced in 00f7ec36c RDMA/core: Add memory management
extensions support and works when the IB device supports the
IB_DEVICE_MEM_MGT_EXTENSIONS capability.

Upon creating the iser device iser will test whether the HCA supports FMRs.
If no support for FMRs, check if IB_DEVICE_MEM_MGT_EXTENSIONS is supported
and assign function pointers that handle fast registration and allocation
of appropriate resources (fast_reg descriptors).

Registration is done using posting IB_WR_FAST_REG_MR to the QP and
invalidations using posting IB_WR_LOCAL_INV.

Signed-off-by: Sagi Grimberg sa...@mellanox.com
Signed-off-by: Or Gerlitz ogerl...@mellanox.com
---
 drivers/infiniband/ulp/iser/iscsi_iser.h  |   21 -
 drivers/infiniband/ulp/iser/iser_memory.c |  140 -
 drivers/infiniband/ulp/iser/iser_verbs.c  |  138 ++--
 3 files changed, 287 insertions(+), 12 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h 
b/drivers/infiniband/ulp/iser/iscsi_iser.h
index 75c5352..6791402 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -211,7 +211,7 @@ struct iser_mem_reg {
u64  va;
u64  len;
void *mem_h;
-   int  is_fmr;
+   int  is_mr;
 };
 
 struct iser_regd_buf {
@@ -277,6 +277,15 @@ struct iser_device {
enum iser_data_dir 
cmd_dir);
 };
 
+struct fast_reg_descriptor {
+   struct list_head  list;
+   /* For fast registration - FRWR */
+   struct ib_mr *data_mr;
+   struct ib_fast_reg_page_list *data_frpl;
+   /* Valid for fast registration flag */
+   bool  valid;
+};
+
 struct iser_conn {
struct iscsi_iser_conn   *iser_conn; /* iser conn for upcalls  */
struct iscsi_endpoint*ep;
@@ -307,6 +316,10 @@ struct iser_conn {
struct iser_page_vec*page_vec; /* represents SG to 
fmr maps*
* maps serialized 
as tx is*/
} fmr;
+   struct {
+   struct list_headpool;
+   int pool_size;
+   } frwr;
} fastreg;
 };
 
@@ -393,6 +406,8 @@ void iser_finalize_rdma_unaligned_sg(struct iscsi_iser_task 
*task,
 
 int  iser_reg_rdma_mem_fmr(struct iscsi_iser_task *task,
   enum iser_data_dir cmd_dir);
+int  iser_reg_rdma_mem_frwr(struct iscsi_iser_task *task,
+   enum iser_data_dir cmd_dir);
 
 int  iser_connect(struct iser_conn   *ib_conn,
  struct sockaddr_in *src_addr,
@@ -405,6 +420,8 @@ int  iser_reg_page_vec(struct iser_conn *ib_conn,
 
 void iser_unreg_mem_fmr(struct iscsi_iser_task *iser_task,
enum iser_data_dir cmd_dir);
+void iser_unreg_mem_frwr(struct iscsi_iser_task *iser_task,
+enum iser_data_dir cmd_dir);
 
 int  iser_post_recvl(struct iser_conn *ib_conn);
 int  iser_post_recvm(struct iser_conn *ib_conn, int count);
@@ -421,4 +438,6 @@ int  iser_initialize_task_headers(struct iscsi_task *task,
 int iser_alloc_rx_descriptors(struct iser_conn *ib_conn, struct iscsi_session 
*session);
 int iser_create_fmr_pool(struct iser_conn *ib_conn, unsigned cmds_max);
 void iser_free_fmr_pool(struct iser_conn *ib_conn);
+int iser_create_frwr_pool(struct iser_conn *ib_conn, unsigned cmds_max);
+void iser_free_frwr_pool(struct iser_conn *ib_conn);
 #endif
diff --git a/drivers/infiniband/ulp/iser/iser_memory.c 
b/drivers/infiniband/ulp/iser/iser_memory.c
index 1985e90..1ce0c97 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -395,8 +395,7 @@ int iser_reg_rdma_mem_fmr(struct iscsi_iser_task *iser_task,
regd_buf = iser_task-rdma_regd[cmd_dir];
 
aligned_len = iser_data_buf_aligned_len(mem, ibdev);
-   if (aligned_len != mem-dma_nents ||
-   (!ib_conn-fastreg.fmr.pool  mem-dma_nents  1)) {
+   if (aligned_len != mem-dma_nents) {
err = fall_to_bounce_buf(iser_task, ibdev,
 cmd_dir, aligned_len);
if (err) {
@@ -414,7 +413,7 @@ int iser_reg_rdma_mem_fmr(struct iscsi_iser_task *iser_task,
regd_buf-reg.rkey = device-mr-rkey;
regd_buf-reg.len  = ib_sg_dma_len(ibdev, sg[0]);
regd_buf-reg.va   = ib_sg_dma_address(ibdev, sg[0]);
-   regd_buf-reg.is_fmr = 0;
+   regd_buf-reg.is_mr = 0;
 
iser_dbg(PHYSICAL Mem.register: lkey: 0x%08X rkey: 0x%08X  
 va: 0x%08lX sz:

Re: [patch] IB/mlx5: stack info leak in mlx5_ib_alloc_ucontext()

2013-07-28 Thread Dan Carpenter 
On Sun, Jul 28, 2013 at 10:23:36AM +0300, Eli Cohen wrote:
 On Thu, Jul 25, 2013 at 08:04:36PM +0300, Dan Carpenter wrote:
  We don't set resp.reserved.  Since it's at the end of the struct that
  means we don't have to copy it to the user.
  
  Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
  
  diff --git a/drivers/infiniband/hw/mlx5/main.c 
  b/drivers/infiniband/hw/mlx5/main.c
  index 8000fff..43dfb84 100644
  --- a/drivers/infiniband/hw/mlx5/main.c
  +++ b/drivers/infiniband/hw/mlx5/main.c
  @@ -619,7 +619,8 @@ static struct ib_ucontext 
  *mlx5_ib_alloc_ucontext(struct ib_device *ibdev,
   
  resp.tot_uuars = req.total_num_uuars;
  resp.num_ports = dev-mdev.caps.num_ports;
  -   err = ib_copy_to_udata(udata, resp, sizeof(resp));
  +   err = ib_copy_to_udata(udata, resp,
  +  sizeof(resp) - sizeof(resp.reserved));
  if (err)
  goto out_uars;
 
 
 I don't have strong opinion on this one. The title of the patch is
 stack info leak but the only leak of a reserved field. Other opinions?

First let me say that I don't know how this code is called, it may
be root only, but even in that case I think it's still worth
applying my patch.

These info leak problems are a well known security problem so I
didn't put a long explanation.  What you do is you fill the stack
with function pointers, then you call the function that leaks.  Then
you have a potentially useful pointer which was supposed to be
secret.  Something like that anyway.

There are probably lots of other easier ways to defeat address space
randomization.  There may be other ways you can use info leaks as
well...

Anyway, regardless, static checkers and code auditors look for these
leaks so applying the patch makes sense just to silence a warning.

regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html