On 4/16/2015 9:35 AM, Michael Wang wrote:
> 
> 
> On 04/16/2015 03:19 PM, Hal Rosenstock wrote:
> [snip]
>>>  
>>> +static inline int cma_validate_port(struct ib_device *device, u8 port,
>>> +                                 union ib_gid *gid, int dev_type)
>>> +{
>>> +   u8 found_port;
>>> +   int ret = -ENODEV;
>>> +
>>> +   if ((dev_type == ARPHRD_INFINIBAND) && !rdma_tech_ib(device, port))
>>> +           return ret;
>>> +
>>> +   if ((dev_type != ARPHRD_INFINIBAND) && rdma_tech_ib(device, port))
>>> +           return ret;
>>> +
>>> +   ret = ib_find_cached_gid(device, gid, &found_port, NULL);
>>> +
>>> +   if (!ret && (port == found_port))
>>> +           return 0;
>>> +
>>> +   return ret;
>>
>> Should the case where ret = 0 and port != found_port need to be handled
>> the same as currently ? It looks different to me since in this case the
>> port will be saved into id_priv->id.port_num whereas currently it isn't.
> 
> I get your point :-) what about:
> 
>       ret = ib_find_cached_gid(device, gid, &found_port, NULL);
>       if (port != found_port)
>               return -ENODEV;
> 
>       return ret;

Yes, that looks to me to be consistent with the current implementation.

-- Hal

> Regards,
> Michael Wang
> 
>>
>> -- Hal
>>
>>> +}
>>> +
>>>  static int cma_acquire_dev(struct rdma_id_private *id_priv,
>>>                        struct rdma_id_private *listen_id_priv)
>>>  {
>>>     struct rdma_dev_addr *dev_addr = &id_priv->id.route.addr.dev_addr;
>>>     struct cma_device *cma_dev;
>>> -   union ib_gid gid, iboe_gid;
>>> +   union ib_gid gid, iboe_gid, *gidp;
>>>     int ret = -ENODEV;
>>> -   u8 port, found_port;
>>> -   enum rdma_link_layer dev_ll = dev_addr->dev_type == ARPHRD_INFINIBAND ?
>>> -           IB_LINK_LAYER_INFINIBAND : IB_LINK_LAYER_ETHERNET;
>>> +   u8 port;
>>>  
>>> -   if (dev_ll != IB_LINK_LAYER_INFINIBAND &&
>>> +   if (dev_addr->dev_type != ARPHRD_INFINIBAND &&
>>>         id_priv->id.ps == RDMA_PS_IPOIB)
>>>             return -EINVAL;
>>>  
>>> @@ -391,41 +409,36 @@ static int cma_acquire_dev(struct rdma_id_private 
>>> *id_priv,
>>>  
>>>     memcpy(&gid, dev_addr->src_dev_addr +
>>>            rdma_addr_gid_offset(dev_addr), sizeof gid);
>>> -   if (listen_id_priv &&
>>> -       rdma_port_get_link_layer(listen_id_priv->id.device,
>>> -                                listen_id_priv->id.port_num) == dev_ll) {
>>> +
>>> +   if (listen_id_priv) {
>>>             cma_dev = listen_id_priv->cma_dev;
>>>             port = listen_id_priv->id.port_num;
>>> -           if (rdma_node_get_transport(cma_dev->device->node_type) == 
>>> RDMA_TRANSPORT_IB &&
>>> -               rdma_port_get_link_layer(cma_dev->device, port) == 
>>> IB_LINK_LAYER_ETHERNET)
>>> -                   ret = ib_find_cached_gid(cma_dev->device, &iboe_gid,
>>> -                                            &found_port, NULL);
>>> -           else
>>> -                   ret = ib_find_cached_gid(cma_dev->device, &gid,
>>> -                                            &found_port, NULL);
>>> +           gidp = rdma_tech_iboe(cma_dev->device, port) ?
>>> +                  &iboe_gid : &gid;
>>>  
>>> -           if (!ret && (port  == found_port)) {
>>> -                   id_priv->id.port_num = found_port;
>>> +           ret = cma_validate_port(cma_dev->device, port, gidp,
>>> +                                   dev_addr->dev_type);
>>> +           if (!ret) {
>>> +                   id_priv->id.port_num = port;
>>>                     goto out;
>>>             }
>>>     }
>>> +
>>>     list_for_each_entry(cma_dev, &dev_list, list) {
>>>             for (port = 1; port <= cma_dev->device->phys_port_cnt; ++port) {
>>>                     if (listen_id_priv &&
>>>                         listen_id_priv->cma_dev == cma_dev &&
>>>                         listen_id_priv->id.port_num == port)
>>>                             continue;
>>> -                   if (rdma_port_get_link_layer(cma_dev->device, port) == 
>>> dev_ll) {
>>> -                           if 
>>> (rdma_node_get_transport(cma_dev->device->node_type) == RDMA_TRANSPORT_IB &&
>>> -                               rdma_port_get_link_layer(cma_dev->device, 
>>> port) == IB_LINK_LAYER_ETHERNET)
>>> -                                   ret = 
>>> ib_find_cached_gid(cma_dev->device, &iboe_gid, &found_port, NULL);
>>> -                           else
>>> -                                   ret = 
>>> ib_find_cached_gid(cma_dev->device, &gid, &found_port, NULL);
>>> -
>>> -                           if (!ret && (port == found_port)) {
>>> -                                   id_priv->id.port_num = found_port;
>>> -                                   goto out;
>>> -                           }
>>> +
>>> +                   gidp = rdma_tech_iboe(cma_dev->device, port) ?
>>> +                          &iboe_gid : &gid;
>>> +
>>> +                   ret = cma_validate_port(cma_dev->device, port, gidp,
>>> +                                           dev_addr->dev_type);
>>> +                   if (!ret) {
>>> +                           id_priv->id.port_num = port;
>>> +                           goto out;
>>>                     }
>>>             }
>>>     }
>>
> 

--
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

Reply via email to