Re: [PATCH for-next 10/10] IB/iser: Support the remote invalidation exception

2015-11-19 Thread Sagi Grimberg
Sagi, do we still do IN, OUT, both or none? if not, where this stopped to be supported? how large would be the fix? Or, it's hard for me to say where exactly it stopped being supported because I've never tested it and I'm not convinced it was ever supported. I'm exhausted with this

Re: [PATCH for-next 10/10] IB/iser: Support the remote invalidation exception

2015-11-19 Thread Christoph Hellwig
On Thu, Nov 19, 2015 at 09:12:20AM +0200, Or Gerlitz wrote: > This is wrong assertion. > > Look on the code throughout the iser path done from iser_send_command, we > allowed the command associated with the > iscsi task to be IN, OUT, both or none, when we do all the dma-mapping, > memory

Re: [PATCH for-next 10/10] IB/iser: Support the remote invalidation exception

2015-11-19 Thread Or Gerlitz
On Thu, Nov 19, 2015 at 12:01 PM, Christoph Hellwig wrote: > Internal code structure is one thing, ever supporting bidi is another > one. Without QUEUE_FLAG_BIDI a driver has never supported bidirectional > requests. And iSER never had that flag set in mainline. So even if

Re: [PATCH for-next 10/10] IB/iser: Support the remote invalidation exception

2015-11-18 Thread Sagi Grimberg
AFAIR, in the past there weren't explicit means to do that. What's makes iscsi tcp eligible to support bidi's that we don't have @ iser? In theory, nothing. In practice, iser is missing customer demands, iser targets that support bidi and testing... If someone cared enough about it then I

Re: [PATCH for-next 10/10] IB/iser: Support the remote invalidation exception

2015-11-18 Thread Christoph Hellwig
On Wed, Nov 18, 2015 at 03:33:01PM +0200, Or Gerlitz wrote: > Sagi, it works in TGT and AFAIR with the initiator too. > > Looking on this paper of Pete Wyckoff [1] I see that he says that > few changes to the initiator were needed, not sure which. Or, can you please stop it? Fortunately

Re: [PATCH for-next 10/10] IB/iser: Support the remote invalidation exception

2015-11-18 Thread Christoph Hellwig
On Wed, Nov 18, 2015 at 03:58:48PM +0200, Or Gerlitz wrote: > > Fortunately neither the iSER target or initiator ever support the > > nightmare called bidi commands, > > I honestly don't know why you call it nightmare nor what make you > make that strong assertion. Beause I actually had to deal

Re: [PATCH for-next 10/10] IB/iser: Support the remote invalidation exception

2015-11-18 Thread Sagi Grimberg
Sagi, it works in TGT and AFAIR with the initiator too. Looking on this paper of Pete Wyckoff [1] I see that he says that few changes to the initiator were needed, not sure which. I see. I wasn't aware that TGT supports bidi. However, AFAICT the initiator support was never fully introduced

Re: [PATCH for-next 10/10] IB/iser: Support the remote invalidation exception

2015-11-18 Thread Or Gerlitz
On Wed, Nov 18, 2015 at 1:38 PM, Sagi Grimberg wrote: >> AFAIR, in the past there weren't explicit means to do that. >> What's makes iscsi tcp eligible to support bidi's that we don't have @ iser? > In theory, nothing. In practice, iser is missing customer demands, iser

Re: [PATCH for-next 10/10] IB/iser: Support the remote invalidation exception

2015-11-17 Thread Or Gerlitz
On Tue, Nov 17, 2015 at 12:26 PM, Sagi Grimberg wrote: > >> Why? we don't support and when did we broke it after the initial 2.6.18 >> upstreaming of the driver >> >> Also, do we refuse to queuecommand a bidi? where? > > > Or, bidirectional support is not assumed and

Re: [PATCH for-next 10/10] IB/iser: Support the remote invalidation exception

2015-11-17 Thread Or Gerlitz
On 11/16/2015 6:37 PM, Sagi Grimberg wrote: --- a/drivers/infiniband/ulp/iser/iser_verbs.c +++ b/drivers/infiniband/ulp/iser/iser_verbs.c @@ -847,7 +847,7 @@ static void iser_route_handler(struct rdma_cm_id *cma_id) conn_param.rnr_retry_count = 6; memset(_hdr, 0,

Re: [PATCH for-next 10/10] IB/iser: Support the remote invalidation exception

2015-11-17 Thread Christoph Hellwig
On Tue, Nov 17, 2015 at 12:04:03PM +0200, Or Gerlitz wrote: > Also, do we refuse to queuecommand a bidi? where? Or, can you please do the basic research first? Thanks! (Hint: check how few drivers support bidi commands, and how it's enabled) -- To unsubscribe from this list: send the line

Re: [PATCH for-next 10/10] IB/iser: Support the remote invalidation exception

2015-11-17 Thread Sagi Grimberg
On 17/11/2015 10:05, Or Gerlitz wrote: On 11/16/2015 6:37 PM, Sagi Grimberg wrote: +if (iser_task->dir[ISER_DIR_IN]) +reg = _task->rdma_reg[ISER_DIR_IN]; +else +reg = _task->rdma_reg[ISER_DIR_OUT]; and what happens with bidirectional

Re: [PATCH for-next 10/10] IB/iser: Support the remote invalidation exception

2015-11-17 Thread Sagi Grimberg
Why? we don't support and when did we broke it after the initial 2.6.18 upstreaming of the driver Also, do we refuse to queuecommand a bidi? where? Or, bidirectional support is not assumed and needs to be actively set as a request queue flag (see iscsi_sw_tcp_slave_alloc()). AFAICT iser

Re: [PATCH for-next 10/10] IB/iser: Support the remote invalidation exception

2015-11-17 Thread Sagi Grimberg
On 17/11/2015 10:03, Or Gerlitz wrote: On 11/16/2015 6:37 PM, Sagi Grimberg wrote: void iser_rcv_completion(struct iser_rx_desc *rx_desc, - unsigned long rx_xfer_len, + struct ib_wc *wc, struct ib_conn *ib_conn) { struct iser_conn *iser_conn =

Re: [PATCH for-next 10/10] IB/iser: Support the remote invalidation exception

2015-11-17 Thread Sagi Grimberg
On 17/11/2015 10:08, Or Gerlitz wrote: On 11/16/2015 6:37 PM, Sagi Grimberg wrote: --- a/drivers/infiniband/ulp/iser/iser_verbs.c +++ b/drivers/infiniband/ulp/iser/iser_verbs.c @@ -847,7 +847,7 @@ static void iser_route_handler(struct rdma_cm_id *cma_id) conn_param.rnr_retry_count =

Re: [PATCH for-next 10/10] IB/iser: Support the remote invalidation exception

2015-11-17 Thread Or Gerlitz
On 11/17/2015 11:53 AM, Sagi Grimberg wrote: [...] iSER lacks support for bidirectional commands for as long as I'm involved with it Why? we don't support and when did we broke it after the initial 2.6.18 upstreaming of the driver Also, do we refuse to queuecommand a bidi? where? -- To

Re: [PATCH for-next 10/10] IB/iser: Support the remote invalidation exception

2015-11-17 Thread Or Gerlitz
On 11/16/2015 6:37 PM, Sagi Grimberg wrote: + if (iser_task->dir[ISER_DIR_IN]) + reg = _task->rdma_reg[ISER_DIR_IN]; + else + reg = _task->rdma_reg[ISER_DIR_OUT]; and what happens with

Re: [PATCH for-next 10/10] IB/iser: Support the remote invalidation exception

2015-11-17 Thread Or Gerlitz
On 11/16/2015 6:37 PM, Sagi Grimberg wrote: void iser_rcv_completion(struct iser_rx_desc *rx_desc, -unsigned long rx_xfer_len, +struct ib_wc *wc, struct ib_conn *ib_conn) { struct iser_conn *iser_conn =

[PATCH for-next 10/10] IB/iser: Support the remote invalidation exception

2015-11-16 Thread Sagi Grimberg
From: Jenny Derzhavetz Declare that we support remote invalidation and be able to detect the invalidated rkey so we won't invalidate it locally. The spec mandates that we must not rely on the taget remote invalidate our rkey so we must check it upon a receive (scsi response)