Re: [PATCH 0/2] Adding support for RoCE V2 specification

2014-11-29 Thread Moni Shoua
> Yes, that is correct.
>
> The overarching goal behind this patch is to keep RDMA-CM as the central 
> entity that decides the protocol (ROCEV2 /ROCEV1)
> and hides all the address resolution details from applications.
I agree but with one comment. RDMA-CM, although preferred,  is not
mandatory to establish a connection. The solution needs to take care
also for
apps that don't use RDMA-CM

> This  is just a continuation of the existing RDMA-CM design philosophy for IP 
> Based GIDs that lets applications operate over any form
> of RDMA Service in a completely transparent way without any changes to the 
> applications themselves.
>
> Protocol Resolution(RoCEV2 or V1) must be done even before we
> send out the 1st connection request packet, the corresponding MAD pkt must be 
> V2 for a destination that is behind  a router, correct?
> I am not sure we want to try sending out RoCEv2 connection requests first and 
> if that fails ,then fallback/retry with RoCEv1 request.
I agree but when you send the first packet (request or response) you
know the SGID index to be used. Knowing that and assuming you chose
the SGID index correctly there is no need to guess.
A suggestion for choosing the SGID index is:
1. First, populate GID table with GIDs matching the IP address list of
the corresponding net devices. Each GID will appear twice, one for
RoCEv1 and one for RoCEv2.
2. When connection is being established RDMA-CM will choose one of the
2 matching indices based on a
preferred protocol attribute for the rmda_id (new attr.)
3. Proffered protocol is
 a) global for the node/fabric based on the administrator settings
 b) can be changed by applications that want to override the
default (requires extra API)


> That would needlessly complicate and slow down the connection-establishment 
> process, do you agree ?
> Instead it's much more seamless to take help of the IP stack to select the 
> network header type and subsequently the ROCE pkt format while
> attempting to establish the connection.
>
> The one thing that my patch does miss out though is the ability for local 
> subnet end nodes to be able to communicate using RoCEv2 packet formats.
> (The default policy in this patch is to use ROCEV1 for nodes connected 
> directly /within the same local subnet.)
> I believe that can be achieved by extending RDMA-CM to override the default 
> protocol selection policy proposed in this patch by making use of
> the GID Entry attribute type in another patch.
Right, and this seems to fit the proposal above.


> This scheme should still work with your proposal  in point# 2 , where GID 
> Table management would move to a central/stack module instead of being
> vendor specific.
>
> Thanks
> Som
>
--
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] drivers: infiniband: hw: Add pci_dma_mapping_error() call

2014-11-29 Thread Tina Johnson
Added a pci_dma_mapping_error() call to check for mapping errors before
further using the dma handle. The previously allocated skb is freed in
case of a mapping error. Unchecked dma handles were found using Coccinelle:

@rule1@
expression e1;
identifier x;
@@

*x = pci_map_single(...);
 ... when != pci_dma_mapping_error(e1,x)

Signed-off-by: Tina Johnson 
Acked-by: Julia Lawall 
---
 drivers/infiniband/hw/amso1100/c2.c |4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/infiniband/hw/amso1100/c2.c 
b/drivers/infiniband/hw/amso1100/c2.c
index 766a71c..c623e82 100644
--- a/drivers/infiniband/hw/amso1100/c2.c
+++ b/drivers/infiniband/hw/amso1100/c2.c
@@ -231,6 +231,10 @@ static inline int c2_rx_alloc(struct c2_port *c2_port, 
struct c2_element *elem)
mapaddr =
pci_map_single(c2dev->pcidev, skb->data, maplen,
   PCI_DMA_FROMDEVICE);
+   if (pci_dma_mapping_error(c2dev->pcidev, mapaddr)) {
+   dev_kfree_skb(skb);
+   return -ENOMEM;
+   }
 
/* Set the sk_buff RXP_header to RXP_HRXD_READY */
rxp_hdr = (struct c2_rxp_hdr *) skb->data;
-- 
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