Re: [PATCH for-next V3 01/11] IB/core: Add gid_type to gid attribute

2015-12-23 Thread Sagi Grimberg 



+static const char * const gid_type_str[] = {

^^ ^^
IMHO, The white spaces can be a little bit confusing to understand.



Pay attention to the double const - I think it's more clear that way.


I agree.




+ [IB_GID_TYPE_IB]= "IB/RoCE v1",
+};
+
+const char *ib_cache_gid_type_str(enum ib_gid_type gid_type)
+{
+ if (gid_type < ARRAY_SIZE(gid_type_str) && gid_type_str[gid_type])

Why do you need to check second condition?


If we want to leave a gap for an invalid GID type, we could do that.
Anyway, we could remove this as an incremental future patch if that's
really important.


Leave it in, better safe than sorry...
--
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 V3 01/11] IB/core: Add gid_type to gid attribute

2015-12-23 Thread Matan Barak 
On Wed, Dec 23, 2015 at 3:40 PM, Leon Romanovsky  wrote:
> On Wed, Dec 23, 2015 at 02:56:47PM +0200, Matan Barak wrote:
>> In order to support multiple GID types, we need to store the gid_type
>> with each GID. This is also aligned with the RoCE v2 annex "RoCEv2 PORT
>> GID table entries shall have a "GID type" attribute that denotes the L3
>> Address type". The currently supported GID is IB_GID_TYPE_IB which is
>> also RoCE v1 GID type.
>>
>> This implies that gid_type should be added to roce_gid_table meta-data.
>>
>> Signed-off-by: Matan Barak 
>> ---
>>  drivers/infiniband/core/cache.c   |  144 
>> +++-
>>  drivers/infiniband/core/cm.c  |2 +-
>>  drivers/infiniband/core/cma.c |3 +-
>>  drivers/infiniband/core/core_priv.h   |4 +
>>  drivers/infiniband/core/device.c  |9 ++-
>>  drivers/infiniband/core/multicast.c   |2 +-
>>  drivers/infiniband/core/roce_gid_mgmt.c   |   60 ++--
>>  drivers/infiniband/core/sa_query.c|5 +-
>>  drivers/infiniband/core/uverbs_marshall.c |1 +
>>  drivers/infiniband/core/verbs.c   |1 +
>>  include/rdma/ib_cache.h   |4 +
>>  include/rdma/ib_sa.h  |1 +
>>  include/rdma/ib_verbs.h   |   11 ++-
>>  13 files changed, 185 insertions(+), 62 deletions(-)
>>
>> diff --git a/drivers/infiniband/core/cache.c 
>> b/drivers/infiniband/core/cache.c
>> index 097e9df..566fd8f 100644
>> --- a/drivers/infiniband/core/cache.c
>> +++ b/drivers/infiniband/core/cache.c
>> @@ -64,6 +64,7 @@ enum gid_attr_find_mask {
>>   GID_ATTR_FIND_MASK_GID  = 1UL << 0,
>>   GID_ATTR_FIND_MASK_NETDEV   = 1UL << 1,
>>   GID_ATTR_FIND_MASK_DEFAULT  = 1UL << 2,
>> + GID_ATTR_FIND_MASK_GID_TYPE = 1UL << 3,
>>  };
>>
>>  enum gid_table_entry_props {
>> @@ -125,6 +126,19 @@ static void dispatch_gid_change_event(struct ib_device 
>> *ib_dev, u8 port)
>>   }
>>  }
>>
>> +static const char * const gid_type_str[] = {
>^^ ^^
> IMHO, The white spaces can be a little bit confusing to understand.
>

Pay attention to the double const - I think it's more clear that way.

>> + [IB_GID_TYPE_IB]= "IB/RoCE v1",
>> +};
>> +
>> +const char *ib_cache_gid_type_str(enum ib_gid_type gid_type)
>> +{
>> + if (gid_type < ARRAY_SIZE(gid_type_str) && gid_type_str[gid_type])
> Why do you need to check second condition?

If we want to leave a gap for an invalid GID type, we could do that.
Anyway, we could remove this as an incremental future patch if that's
really important.

>> + return gid_type_str[gid_type];
>> +
>> + return "Invalid GID type";
>> +}
>> +EXPORT_SYMBOL(ib_cache_gid_type_str);
>> +
> --
> 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
--
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 V3 01/11] IB/core: Add gid_type to gid attribute

2015-12-23 Thread Leon Romanovsky 
On Wed, Dec 23, 2015 at 02:56:47PM +0200, Matan Barak wrote:
> In order to support multiple GID types, we need to store the gid_type
> with each GID. This is also aligned with the RoCE v2 annex "RoCEv2 PORT
> GID table entries shall have a "GID type" attribute that denotes the L3
> Address type". The currently supported GID is IB_GID_TYPE_IB which is
> also RoCE v1 GID type.
> 
> This implies that gid_type should be added to roce_gid_table meta-data.
> 
> Signed-off-by: Matan Barak 
> ---
>  drivers/infiniband/core/cache.c   |  144 +++-
>  drivers/infiniband/core/cm.c  |2 +-
>  drivers/infiniband/core/cma.c |3 +-
>  drivers/infiniband/core/core_priv.h   |4 +
>  drivers/infiniband/core/device.c  |9 ++-
>  drivers/infiniband/core/multicast.c   |2 +-
>  drivers/infiniband/core/roce_gid_mgmt.c   |   60 ++--
>  drivers/infiniband/core/sa_query.c|5 +-
>  drivers/infiniband/core/uverbs_marshall.c |1 +
>  drivers/infiniband/core/verbs.c   |1 +
>  include/rdma/ib_cache.h   |4 +
>  include/rdma/ib_sa.h  |1 +
>  include/rdma/ib_verbs.h   |   11 ++-
>  13 files changed, 185 insertions(+), 62 deletions(-)
> 
> diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
> index 097e9df..566fd8f 100644
> --- a/drivers/infiniband/core/cache.c
> +++ b/drivers/infiniband/core/cache.c
> @@ -64,6 +64,7 @@ enum gid_attr_find_mask {
>   GID_ATTR_FIND_MASK_GID  = 1UL << 0,
>   GID_ATTR_FIND_MASK_NETDEV   = 1UL << 1,
>   GID_ATTR_FIND_MASK_DEFAULT  = 1UL << 2,
> + GID_ATTR_FIND_MASK_GID_TYPE = 1UL << 3,
>  };
>  
>  enum gid_table_entry_props {
> @@ -125,6 +126,19 @@ static void dispatch_gid_change_event(struct ib_device 
> *ib_dev, u8 port)
>   }
>  }
>  
> +static const char * const gid_type_str[] = {
   ^^ ^^
IMHO, The white spaces can be a little bit confusing to understand.

> + [IB_GID_TYPE_IB]= "IB/RoCE v1",
> +};
> +
> +const char *ib_cache_gid_type_str(enum ib_gid_type gid_type)
> +{
> + if (gid_type < ARRAY_SIZE(gid_type_str) && gid_type_str[gid_type])
Why do you need to check second condition?
> + return gid_type_str[gid_type];
> +
> + return "Invalid GID type";
> +}
> +EXPORT_SYMBOL(ib_cache_gid_type_str);
> +
--
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 for-next V3 01/11] IB/core: Add gid_type to gid attribute

2015-12-23 Thread Matan Barak 
In order to support multiple GID types, we need to store the gid_type
with each GID. This is also aligned with the RoCE v2 annex "RoCEv2 PORT
GID table entries shall have a "GID type" attribute that denotes the L3
Address type". The currently supported GID is IB_GID_TYPE_IB which is
also RoCE v1 GID type.

This implies that gid_type should be added to roce_gid_table meta-data.

Signed-off-by: Matan Barak 
---
 drivers/infiniband/core/cache.c   |  144 +++-
 drivers/infiniband/core/cm.c  |2 +-
 drivers/infiniband/core/cma.c |3 +-
 drivers/infiniband/core/core_priv.h   |4 +
 drivers/infiniband/core/device.c  |9 ++-
 drivers/infiniband/core/multicast.c   |2 +-
 drivers/infiniband/core/roce_gid_mgmt.c   |   60 ++--
 drivers/infiniband/core/sa_query.c|5 +-
 drivers/infiniband/core/uverbs_marshall.c |1 +
 drivers/infiniband/core/verbs.c   |1 +
 include/rdma/ib_cache.h   |4 +
 include/rdma/ib_sa.h  |1 +
 include/rdma/ib_verbs.h   |   11 ++-
 13 files changed, 185 insertions(+), 62 deletions(-)

diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
index 097e9df..566fd8f 100644
--- a/drivers/infiniband/core/cache.c
+++ b/drivers/infiniband/core/cache.c
@@ -64,6 +64,7 @@ enum gid_attr_find_mask {
GID_ATTR_FIND_MASK_GID  = 1UL << 0,
GID_ATTR_FIND_MASK_NETDEV   = 1UL << 1,
GID_ATTR_FIND_MASK_DEFAULT  = 1UL << 2,
+   GID_ATTR_FIND_MASK_GID_TYPE = 1UL << 3,
 };
 
 enum gid_table_entry_props {
@@ -125,6 +126,19 @@ static void dispatch_gid_change_event(struct ib_device 
*ib_dev, u8 port)
}
 }
 
+static const char * const gid_type_str[] = {
+   [IB_GID_TYPE_IB]= "IB/RoCE v1",
+};
+
+const char *ib_cache_gid_type_str(enum ib_gid_type gid_type)
+{
+   if (gid_type < ARRAY_SIZE(gid_type_str) && gid_type_str[gid_type])
+   return gid_type_str[gid_type];
+
+   return "Invalid GID type";
+}
+EXPORT_SYMBOL(ib_cache_gid_type_str);
+
 /* This function expects that rwlock will be write locked in all
  * scenarios and that lock will be locked in sleep-able (RoCE)
  * scenarios.
@@ -233,6 +247,10 @@ static int find_gid(struct ib_gid_table *table, const 
union ib_gid *gid,
if (found >=0)
continue;
 
+   if (mask & GID_ATTR_FIND_MASK_GID_TYPE &&
+   attr->gid_type != val->gid_type)
+   continue;
+
if (mask & GID_ATTR_FIND_MASK_GID &&
memcmp(gid, &data->gid, sizeof(*gid)))
continue;
@@ -296,6 +314,7 @@ int ib_cache_gid_add(struct ib_device *ib_dev, u8 port,
write_lock_irq(&table->rwlock);
 
ix = find_gid(table, gid, attr, false, GID_ATTR_FIND_MASK_GID |
+ GID_ATTR_FIND_MASK_GID_TYPE |
  GID_ATTR_FIND_MASK_NETDEV, &empty);
if (ix >= 0)
goto out_unlock;
@@ -329,6 +348,7 @@ int ib_cache_gid_del(struct ib_device *ib_dev, u8 port,
 
ix = find_gid(table, gid, attr, false,
  GID_ATTR_FIND_MASK_GID  |
+ GID_ATTR_FIND_MASK_GID_TYPE |
  GID_ATTR_FIND_MASK_NETDEV   |
  GID_ATTR_FIND_MASK_DEFAULT,
  NULL);
@@ -427,11 +447,13 @@ static int _ib_cache_gid_table_find(struct ib_device 
*ib_dev,
 
 static int ib_cache_gid_find(struct ib_device *ib_dev,
 const union ib_gid *gid,
+enum ib_gid_type gid_type,
 struct net_device *ndev, u8 *port,
 u16 *index)
 {
-   unsigned long mask = GID_ATTR_FIND_MASK_GID;
-   struct ib_gid_attr gid_attr_val = {.ndev = ndev};
+   unsigned long mask = GID_ATTR_FIND_MASK_GID |
+GID_ATTR_FIND_MASK_GID_TYPE;
+   struct ib_gid_attr gid_attr_val = {.ndev = ndev, .gid_type = gid_type};
 
if (ndev)
mask |= GID_ATTR_FIND_MASK_NETDEV;
@@ -442,14 +464,16 @@ static int ib_cache_gid_find(struct ib_device *ib_dev,
 
 int ib_find_cached_gid_by_port(struct ib_device *ib_dev,
   const union ib_gid *gid,
+  enum ib_gid_type gid_type,
   u8 port, struct net_device *ndev,
   u16 *index)
 {
int local_index;
struct ib_gid_table **ports_table = ib_dev->cache.gid_cache;
struct ib_gid_table *table;
-   unsigned long mask = GID_ATTR_FIND_MASK_GID;
-   struct ib_gid_attr val = {.ndev = ndev};
+   unsigned long mask = GID_ATTR_FIND_MASK_GID |
+GID_ATTR_FIND_MASK_GID_TYPE;
+   struct ib_gid_attr val = {.ndev = ndev, .gid_type = gid_type};
unsigned long flags;