Re: [PATCH] infiniband:core:Add needed error path in cm_init_av_by_path

2015-12-06 Thread Haggai Eran
On Friday, December 4, 2015 8:02 PM, Nicholas Krause  
wrote:
> To: dledf...@redhat.com
> Cc: sean.he...@intel.com; hal.rosenst...@gmail.com; Haggai Eran; 
> jguntho...@obsidianresearch.com; Matan Barak; yun.w...@profitbricks.com; 
> ted.h@oracle.com; Doron Tsur; Erez Shitrit; david.ah...@oracle.com; 
> linux-rdma@vger.kernel.org; linux-ker...@vger.kernel.org
> Subject: [PATCH] infiniband:core:Add needed error path in cm_init_av_by_path
> 
> This adds a needed error path in the function, cm_init_av_by_path
> after the call to ib_init_ah_from_path in order to avoid incorrectly
> accessing the path pointer of structure type ib_sa_path_rec if this
> function call fails to complete its intended work successfully by
> returning a error code.
> 
> Signed-off-by: Nicholas Krause 

The subject doesn't seem to match the convention but apart from that,

Reviewed-by: Haggai Eran 

I wonder if this should go to stable. If I understand correctly, this will fail 
only when the SGID isn't found in the GID table, but such connections would 
fail later on when creating a QP, right?

Haggai--
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-next V2 05/11] IB/core: Add rdma_network_type to wc

2015-12-06 Thread Moni Shoua
On Mon, Dec 7, 2015 at 8:34 AM, Jason Gunthorpe
 wrote:
> On Mon, Dec 07, 2015 at 08:15:40AM +0200, Moni Shoua wrote:
>
>> What you have though is the sgid taken from the GRH that is scattered
>> to the first 40/20 bytes of the receive WQE.  This is not enough to
>> determine the network type.
>
> It is enough to discover the sgid index which will tell you the type.
but how? all you have in hand is the sgid which can appear several
times in the GID table in different indices.
--
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-next V2 05/11] IB/core: Add rdma_network_type to wc

2015-12-06 Thread Jason Gunthorpe
On Mon, Dec 07, 2015 at 08:15:40AM +0200, Moni Shoua wrote:

> What you have though is the sgid taken from the GRH that is scattered
> to the first 40/20 bytes of the receive WQE.  This is not enough to
> determine the network type.

It is enough to discover the sgid index which will tell you the type.

Jason
--
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-next V2 05/11] IB/core: Add rdma_network_type to wc

2015-12-06 Thread Parav Pandit
On Mon, Dec 7, 2015 at 11:32 AM, Jason Gunthorpe
 wrote:
> On Thu, Dec 03, 2015 at 04:20:50PM +, Liran Liss wrote:
>> > From: linux-rdma-ow...@vger.kernel.org [mailto:linux-rdma-
>>
>> > Subject: Re: [PATCH for-next V2 05/11] IB/core: Add rdma_network_type to
>> > wc
>> >
>> > Bloating the WC with a field that's not really useful for the ULPs seems 
>> > pretty
>> > sad..
>>
>> You need per packet (read per-WC) network type both for handling incoming 
>> connections over QP1 and in UD QPs.
>> It looks like this patch doesn't extend the structure size due to alignment, 
>> so no real harm in any case...
>
> Why? The sgid index must tell you the network type.
>

User space might not have access to internal properties of gid entry
like kernel. Not sure if more plumbing needs to be added in user space
to get such property and async updates of it.
So Liran might be thinking to have unified way to report required fields via wc?
--
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-next V2 05/11] IB/core: Add rdma_network_type to wc

2015-12-06 Thread Moni Shoua
On Mon, Dec 7, 2015 at 8:02 AM, Jason Gunthorpe
 wrote:
> Why? The sgid index must tell you the network type.

struct ib_wc doesn't have sgid or sgid_index field.
What you have though is the sgid taken from the GRH that is scattered
to the first 40/20 bytes of the receive WQE.  This is not enough to
determine the network type.
--
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-next V2 05/11] IB/core: Add rdma_network_type to wc

2015-12-06 Thread Jason Gunthorpe
On Thu, Dec 03, 2015 at 04:20:50PM +, Liran Liss wrote:
> > From: linux-rdma-ow...@vger.kernel.org [mailto:linux-rdma-
> 
> > Subject: Re: [PATCH for-next V2 05/11] IB/core: Add rdma_network_type to
> > wc
> > 
> > Bloating the WC with a field that's not really useful for the ULPs seems 
> > pretty
> > sad..
> 
> You need per packet (read per-WC) network type both for handling incoming 
> connections over QP1 and in UD QPs.
> It looks like this patch doesn't extend the structure size due to alignment, 
> so no real harm in any case...

Why? The sgid index must tell you the network type.

Jason
--
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-next V2 05/11] IB/core: Add rdma_network_type to wc

2015-12-06 Thread Moni Shoua
On Sun, Dec 6, 2015 at 4:03 PM, Christoph Hellwig  wrote:
> The ULPs couldn't really care less about the network type.  The problem

In WC, ULPs care about network type as much as they care for dgid or port_num
So, network_type is indeed an information that is relevant only to
RoCE but it only follows the tradition
--
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-next V2 05/11] IB/core: Add rdma_network_type to wc

2015-12-06 Thread Christoph Hellwig
On Thu, Dec 03, 2015 at 04:20:50PM +, Liran Liss wrote:
> > From: linux-rdma-ow...@vger.kernel.org [mailto:linux-rdma-
> 
> > Subject: Re: [PATCH for-next V2 05/11] IB/core: Add rdma_network_type to
> > wc
> > 
> > Bloating the WC with a field that's not really useful for the ULPs seems 
> > pretty
> > sad..
> 
> You need per packet (read per-WC) network type both for handling incoming 
> connections over QP1 and in UD QPs.
> It looks like this patch doesn't extend the structure size due to alignment, 
> so no real harm in any case...

The ULPs couldn't really care less about the network type.  The problem
is that one monolithic structure is used both for communicatation with
the protocol drivers and the RoCE implementation.
--
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 5/6] IB core: Fix ib_sg_to_pages()

2015-12-06 Thread Christoph Hellwig
Looks good,

Reviewed-by: Christoph Hellwig 
--
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 5/6] IB core: Fix ib_sg_to_pages()

2015-12-06 Thread Sagi Grimberg

Looks fine,

Reviewed-by: Sagi Grimberg 
--
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