* [PATCH] IB/core: Add inline function to validate port
@ 2017-01-25 16:41 Yuval Shaia
[not found] ` <1485362497-18723-1-git-send-email-yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Yuval Shaia @ 2017-01-25 16:41 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w,
leon-DgEjT+Ai2ygdnm+yROfE0A, yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA,
sagig-VPRAkNaXOzVWk0Htik3J/w, talatb-VPRAkNaXOzVWk0Htik3J/w,
avivh-VPRAkNaXOzVWk0Htik3J/w, pandit.parav-Re5JQEeQqe8AvxtiuMwx3w,
swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW,
bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ,
valex-VPRAkNaXOzVWk0Htik3J/w, haggaie-VPRAkNaXOzVWk0Htik3J/w,
arnd-r2nGTMty4D4, weiyj.lk-Re5JQEeQqe8AvxtiuMwx3w,
markb-VPRAkNaXOzVWk0Htik3J/w, ira.weiny-ral2JQCrhuEAvxtiuMwx3w,
eli-VPRAkNaXOzVWk0Htik3J/w, dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA,
yishaih-VPRAkNaXOzVWk0Htik3J/w, monis-VPRAkNaXOzVWk0Htik3J/w,
linux-rdma-u79uwXL29TY76Z2rM5mHXA
Signed-off-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
drivers/infiniband/core/cache.c | 18 ++++++++----------
drivers/infiniband/core/cma.c | 6 ++----
drivers/infiniband/core/device.c | 4 ++--
drivers/infiniband/core/verbs.c | 3 +--
include/rdma/ib_verbs.h | 7 +++++++
5 files changed, 20 insertions(+), 18 deletions(-)
diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
index ae04826..0718b21 100644
--- a/drivers/infiniband/core/cache.c
+++ b/drivers/infiniband/core/cache.c
@@ -510,8 +510,7 @@ int ib_find_cached_gid_by_port(struct ib_device *ib_dev,
struct ib_gid_attr val = {.ndev = ndev, .gid_type = gid_type};
unsigned long flags;
- if (port < rdma_start_port(ib_dev) ||
- port > rdma_end_port(ib_dev))
+ if (!rdma_is_port_valid(ib_dev, port))
return -ENOENT;
table = ports_table[port - rdma_start_port(ib_dev)];
@@ -571,8 +570,7 @@ static int ib_cache_gid_find_by_filter(struct ib_device *ib_dev,
if (!ports_table)
return -EOPNOTSUPP;
- if (port < rdma_start_port(ib_dev) ||
- port > rdma_end_port(ib_dev) ||
+ if (!rdma_is_port_valid(ib_dev, port) ||
!rdma_protocol_roce(ib_dev, port))
return -EPROTONOSUPPORT;
@@ -863,7 +861,7 @@ int ib_get_cached_gid(struct ib_device *device,
struct ib_gid_table **ports_table = device->cache.gid_cache;
struct ib_gid_table *table = ports_table[port_num - rdma_start_port(device)];
- if (port_num < rdma_start_port(device) || port_num > rdma_end_port(device))
+ if (!rdma_is_port_valid(device, port_num))
return -EINVAL;
read_lock_irqsave(&table->rwlock, flags);
@@ -912,7 +910,7 @@ int ib_get_cached_pkey(struct ib_device *device,
unsigned long flags;
int ret = 0;
- if (port_num < rdma_start_port(device) || port_num > rdma_end_port(device))
+ if (!rdma_is_port_valid(device, port_num))
return -EINVAL;
read_lock_irqsave(&device->cache.lock, flags);
@@ -941,7 +939,7 @@ int ib_find_cached_pkey(struct ib_device *device,
int ret = -ENOENT;
int partial_ix = -1;
- if (port_num < rdma_start_port(device) || port_num > rdma_end_port(device))
+ if (!rdma_is_port_valid(device, port_num))
return -EINVAL;
read_lock_irqsave(&device->cache.lock, flags);
@@ -981,7 +979,7 @@ int ib_find_exact_cached_pkey(struct ib_device *device,
int i;
int ret = -ENOENT;
- if (port_num < rdma_start_port(device) || port_num > rdma_end_port(device))
+ if (!rdma_is_port_valid(device, port_num))
return -EINVAL;
read_lock_irqsave(&device->cache.lock, flags);
@@ -1010,7 +1008,7 @@ int ib_get_cached_lmc(struct ib_device *device,
unsigned long flags;
int ret = 0;
- if (port_num < rdma_start_port(device) || port_num > rdma_end_port(device))
+ if (!rdma_is_port_valid(device, port_num))
return -EINVAL;
read_lock_irqsave(&device->cache.lock, flags);
@@ -1037,7 +1035,7 @@ static void ib_cache_update(struct ib_device *device,
bool use_roce_gid_table =
rdma_cap_roce_gid_table(device, port);
- if (port < rdma_start_port(device) || port > rdma_end_port(device))
+ if (!rdma_is_port_valid(device, port))
return;
table = ports_table[port - rdma_start_port(device)];
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index e7dcfac..69f37b1 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -269,8 +269,7 @@ struct cma_device *cma_enum_devices_by_ibdev(cma_device_filter filter,
int cma_get_default_gid_type(struct cma_device *cma_dev,
unsigned int port)
{
- if (port < rdma_start_port(cma_dev->device) ||
- port > rdma_end_port(cma_dev->device))
+ if (!rdma_is_port_valid(cma_dev->device, port))
return -EINVAL;
return cma_dev->default_gid_type[port - rdma_start_port(cma_dev->device)];
@@ -282,8 +281,7 @@ int cma_set_default_gid_type(struct cma_device *cma_dev,
{
unsigned long supported_gids;
- if (port < rdma_start_port(cma_dev->device) ||
- port > rdma_end_port(cma_dev->device))
+ if (!rdma_is_port_valid(cma_dev->device, port))
return -EINVAL;
supported_gids = roce_gid_type_mask_support(cma_dev->device, port);
diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 571974c..f2e4865 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -659,7 +659,7 @@ int ib_query_port(struct ib_device *device,
union ib_gid gid;
int err;
- if (port_num < rdma_start_port(device) || port_num > rdma_end_port(device))
+ if (!rdma_is_port_valid(device, port_num))
return -EINVAL;
memset(port_attr, 0, sizeof(*port_attr));
@@ -825,7 +825,7 @@ int ib_modify_port(struct ib_device *device,
if (!device->modify_port)
return -ENOSYS;
- if (port_num < rdma_start_port(device) || port_num > rdma_end_port(device))
+ if (!rdma_is_port_valid(device, port_num))
return -EINVAL;
return device->modify_port(device, port_num, port_modify_mask,
diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index 71580cc..9b77fbc 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -1205,8 +1205,7 @@ int ib_resolve_eth_dmac(struct ib_device *device,
{
int ret = 0;
- if (ah_attr->port_num < rdma_start_port(device) ||
- ah_attr->port_num > rdma_end_port(device))
+ if (!rdma_is_port_valid(device, ah_attr->port_num))
return -EINVAL;
if (!rdma_cap_eth_ah(device, ah_attr->port_num))
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 958a24d..81c68e9 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -2275,6 +2275,13 @@ static inline u8 rdma_end_port(const struct ib_device *device)
return rdma_cap_ib_switch(device) ? 0 : device->phys_port_cnt;
}
+static inline int rdma_is_port_valid(const struct ib_device *device,
+ unsigned int port)
+{
+ return (port >= rdma_start_port(device) &&
+ port <= rdma_end_port(device));
+}
+
static inline bool rdma_protocol_ib(const struct ib_device *device, u8 port_num)
{
return device->port_immutable[port_num].core_cap_flags & RDMA_CORE_CAP_PROT_IB;
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] IB/core: Add inline function to validate port
[not found] ` <1485362497-18723-1-git-send-email-yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
@ 2017-01-25 16:50 ` Leon Romanovsky
[not found] ` <20170125165022.GQ6005-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
2017-01-27 19:34 ` Doug Ledford
1 sibling, 1 reply; 6+ messages in thread
From: Leon Romanovsky @ 2017-01-25 16:50 UTC (permalink / raw)
To: Yuval Shaia
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w,
sagig-VPRAkNaXOzVWk0Htik3J/w, talatb-VPRAkNaXOzVWk0Htik3J/w,
avivh-VPRAkNaXOzVWk0Htik3J/w, pandit.parav-Re5JQEeQqe8AvxtiuMwx3w,
swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW,
bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ,
valex-VPRAkNaXOzVWk0Htik3J/w, haggaie-VPRAkNaXOzVWk0Htik3J/w,
arnd-r2nGTMty4D4, weiyj.lk-Re5JQEeQqe8AvxtiuMwx3w,
markb-VPRAkNaXOzVWk0Htik3J/w, ira.weiny-ral2JQCrhuEAvxtiuMwx3w,
eli-VPRAkNaXOzVWk0Htik3J/w, dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA,
yishaih-VPRAkNaXOzVWk0Htik3J/w, monis-VPRAkNaXOzVWk0Htik3J/w,
linux-rdma-u79uwXL29TY76Z2rM5mHXA
[-- Attachment #1: Type: text/plain, Size: 6607 bytes --]
On Wed, Jan 25, 2017 at 06:41:37PM +0200, Yuval Shaia wrote:
> Signed-off-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> ---
> drivers/infiniband/core/cache.c | 18 ++++++++----------
> drivers/infiniband/core/cma.c | 6 ++----
> drivers/infiniband/core/device.c | 4 ++--
> drivers/infiniband/core/verbs.c | 3 +--
> include/rdma/ib_verbs.h | 7 +++++++
> 5 files changed, 20 insertions(+), 18 deletions(-)
Nice change, however all places are pretended with "!".
Can we change the name of function to something like
"rdma_is_port_invalid"?
>
> diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
> index ae04826..0718b21 100644
> --- a/drivers/infiniband/core/cache.c
> +++ b/drivers/infiniband/core/cache.c
> @@ -510,8 +510,7 @@ int ib_find_cached_gid_by_port(struct ib_device *ib_dev,
> struct ib_gid_attr val = {.ndev = ndev, .gid_type = gid_type};
> unsigned long flags;
>
> - if (port < rdma_start_port(ib_dev) ||
> - port > rdma_end_port(ib_dev))
> + if (!rdma_is_port_valid(ib_dev, port))
> return -ENOENT;
>
> table = ports_table[port - rdma_start_port(ib_dev)];
> @@ -571,8 +570,7 @@ static int ib_cache_gid_find_by_filter(struct ib_device *ib_dev,
> if (!ports_table)
> return -EOPNOTSUPP;
>
> - if (port < rdma_start_port(ib_dev) ||
> - port > rdma_end_port(ib_dev) ||
> + if (!rdma_is_port_valid(ib_dev, port) ||
> !rdma_protocol_roce(ib_dev, port))
> return -EPROTONOSUPPORT;
>
> @@ -863,7 +861,7 @@ int ib_get_cached_gid(struct ib_device *device,
> struct ib_gid_table **ports_table = device->cache.gid_cache;
> struct ib_gid_table *table = ports_table[port_num - rdma_start_port(device)];
>
> - if (port_num < rdma_start_port(device) || port_num > rdma_end_port(device))
> + if (!rdma_is_port_valid(device, port_num))
> return -EINVAL;
>
> read_lock_irqsave(&table->rwlock, flags);
> @@ -912,7 +910,7 @@ int ib_get_cached_pkey(struct ib_device *device,
> unsigned long flags;
> int ret = 0;
>
> - if (port_num < rdma_start_port(device) || port_num > rdma_end_port(device))
> + if (!rdma_is_port_valid(device, port_num))
> return -EINVAL;
>
> read_lock_irqsave(&device->cache.lock, flags);
> @@ -941,7 +939,7 @@ int ib_find_cached_pkey(struct ib_device *device,
> int ret = -ENOENT;
> int partial_ix = -1;
>
> - if (port_num < rdma_start_port(device) || port_num > rdma_end_port(device))
> + if (!rdma_is_port_valid(device, port_num))
> return -EINVAL;
>
> read_lock_irqsave(&device->cache.lock, flags);
> @@ -981,7 +979,7 @@ int ib_find_exact_cached_pkey(struct ib_device *device,
> int i;
> int ret = -ENOENT;
>
> - if (port_num < rdma_start_port(device) || port_num > rdma_end_port(device))
> + if (!rdma_is_port_valid(device, port_num))
> return -EINVAL;
>
> read_lock_irqsave(&device->cache.lock, flags);
> @@ -1010,7 +1008,7 @@ int ib_get_cached_lmc(struct ib_device *device,
> unsigned long flags;
> int ret = 0;
>
> - if (port_num < rdma_start_port(device) || port_num > rdma_end_port(device))
> + if (!rdma_is_port_valid(device, port_num))
> return -EINVAL;
>
> read_lock_irqsave(&device->cache.lock, flags);
> @@ -1037,7 +1035,7 @@ static void ib_cache_update(struct ib_device *device,
> bool use_roce_gid_table =
> rdma_cap_roce_gid_table(device, port);
>
> - if (port < rdma_start_port(device) || port > rdma_end_port(device))
> + if (!rdma_is_port_valid(device, port))
> return;
>
> table = ports_table[port - rdma_start_port(device)];
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index e7dcfac..69f37b1 100644
> --- a/drivers/infiniband/core/cma.c
> +++ b/drivers/infiniband/core/cma.c
> @@ -269,8 +269,7 @@ struct cma_device *cma_enum_devices_by_ibdev(cma_device_filter filter,
> int cma_get_default_gid_type(struct cma_device *cma_dev,
> unsigned int port)
> {
> - if (port < rdma_start_port(cma_dev->device) ||
> - port > rdma_end_port(cma_dev->device))
> + if (!rdma_is_port_valid(cma_dev->device, port))
> return -EINVAL;
>
> return cma_dev->default_gid_type[port - rdma_start_port(cma_dev->device)];
> @@ -282,8 +281,7 @@ int cma_set_default_gid_type(struct cma_device *cma_dev,
> {
> unsigned long supported_gids;
>
> - if (port < rdma_start_port(cma_dev->device) ||
> - port > rdma_end_port(cma_dev->device))
> + if (!rdma_is_port_valid(cma_dev->device, port))
> return -EINVAL;
>
> supported_gids = roce_gid_type_mask_support(cma_dev->device, port);
> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> index 571974c..f2e4865 100644
> --- a/drivers/infiniband/core/device.c
> +++ b/drivers/infiniband/core/device.c
> @@ -659,7 +659,7 @@ int ib_query_port(struct ib_device *device,
> union ib_gid gid;
> int err;
>
> - if (port_num < rdma_start_port(device) || port_num > rdma_end_port(device))
> + if (!rdma_is_port_valid(device, port_num))
> return -EINVAL;
>
> memset(port_attr, 0, sizeof(*port_attr));
> @@ -825,7 +825,7 @@ int ib_modify_port(struct ib_device *device,
> if (!device->modify_port)
> return -ENOSYS;
>
> - if (port_num < rdma_start_port(device) || port_num > rdma_end_port(device))
> + if (!rdma_is_port_valid(device, port_num))
> return -EINVAL;
>
> return device->modify_port(device, port_num, port_modify_mask,
> diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
> index 71580cc..9b77fbc 100644
> --- a/drivers/infiniband/core/verbs.c
> +++ b/drivers/infiniband/core/verbs.c
> @@ -1205,8 +1205,7 @@ int ib_resolve_eth_dmac(struct ib_device *device,
> {
> int ret = 0;
>
> - if (ah_attr->port_num < rdma_start_port(device) ||
> - ah_attr->port_num > rdma_end_port(device))
> + if (!rdma_is_port_valid(device, ah_attr->port_num))
> return -EINVAL;
>
> if (!rdma_cap_eth_ah(device, ah_attr->port_num))
> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index 958a24d..81c68e9 100644
> --- a/include/rdma/ib_verbs.h
> +++ b/include/rdma/ib_verbs.h
> @@ -2275,6 +2275,13 @@ static inline u8 rdma_end_port(const struct ib_device *device)
> return rdma_cap_ib_switch(device) ? 0 : device->phys_port_cnt;
> }
>
> +static inline int rdma_is_port_valid(const struct ib_device *device,
> + unsigned int port)
> +{
> + return (port >= rdma_start_port(device) &&
> + port <= rdma_end_port(device));
> +}
> +
> static inline bool rdma_protocol_ib(const struct ib_device *device, u8 port_num)
> {
> return device->port_immutable[port_num].core_cap_flags & RDMA_CORE_CAP_PROT_IB;
> --
> 1.8.3.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] IB/core: Add inline function to validate port
[not found] ` <20170125165022.GQ6005-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
@ 2017-01-25 17:00 ` Bart Van Assche
2017-01-26 8:15 ` Yuval Shaia
1 sibling, 0 replies; 6+ messages in thread
From: Bart Van Assche @ 2017-01-25 17:00 UTC (permalink / raw)
To: yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org,
leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Cc: talatb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org,
dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org,
ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
pandit.parav-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
avivh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
valex-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
On Wed, 2017-01-25 at 18:50 +0200, Leon Romanovsky wrote:
> On Wed, Jan 25, 2017 at 06:41:37PM +0200, Yuval Shaia wrote:
> > Signed-off-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > ---
> > drivers/infiniband/core/cache.c | 18 ++++++++----------
> > drivers/infiniband/core/cma.c | 6 ++----
> > drivers/infiniband/core/device.c | 4 ++--
> > drivers/infiniband/core/verbs.c | 3 +--
> > include/rdma/ib_verbs.h | 7 +++++++
> > 5 files changed, 20 insertions(+), 18 deletions(-)
>
> Nice change, however all places are pretended with "!".
> Can we change the name of function to something like
> "rdma_is_port_invalid"?
I prefer the current approach, namely with a positive function name.
Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] IB/core: Add inline function to validate port
[not found] ` <20170125165022.GQ6005-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
2017-01-25 17:00 ` Bart Van Assche
@ 2017-01-26 8:15 ` Yuval Shaia
[not found] ` <20170126081534.GC3939-Hxa29pjIrETlQW142y8m19+IiqhCXseY@public.gmane.org>
1 sibling, 1 reply; 6+ messages in thread
From: Yuval Shaia @ 2017-01-26 8:15 UTC (permalink / raw)
To: Leon Romanovsky
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w,
sagig-VPRAkNaXOzVWk0Htik3J/w, talatb-VPRAkNaXOzVWk0Htik3J/w,
avivh-VPRAkNaXOzVWk0Htik3J/w, pandit.parav-Re5JQEeQqe8AvxtiuMwx3w,
swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW,
bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ,
valex-VPRAkNaXOzVWk0Htik3J/w, haggaie-VPRAkNaXOzVWk0Htik3J/w,
arnd-r2nGTMty4D4, weiyj.lk-Re5JQEeQqe8AvxtiuMwx3w,
markb-VPRAkNaXOzVWk0Htik3J/w, ira.weiny-ral2JQCrhuEAvxtiuMwx3w,
eli-VPRAkNaXOzVWk0Htik3J/w, dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA,
yishaih-VPRAkNaXOzVWk0Htik3J/w, monis-VPRAkNaXOzVWk0Htik3J/w,
linux-rdma-u79uwXL29TY76Z2rM5mHXA
On Wed, Jan 25, 2017 at 06:50:22PM +0200, Leon Romanovsky wrote:
> On Wed, Jan 25, 2017 at 06:41:37PM +0200, Yuval Shaia wrote:
> > Signed-off-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > ---
> > drivers/infiniband/core/cache.c | 18 ++++++++----------
> > drivers/infiniband/core/cma.c | 6 ++----
> > drivers/infiniband/core/device.c | 4 ++--
> > drivers/infiniband/core/verbs.c | 3 +--
> > include/rdma/ib_verbs.h | 7 +++++++
> > 5 files changed, 20 insertions(+), 18 deletions(-)
>
> Nice change, however all places are pretended with "!".
> Can we change the name of function to something like
> "rdma_is_port_invalid"?
As a matter of fact this thought cross my mind as usually callers cares more
about 'invalid port' and not 'valid port' because it is used as exit criteria
from a function. But as Bart mentioned in other reply, positive looks
better that negative.
>
> >
> > diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
> > index ae04826..0718b21 100644
> > --- a/drivers/infiniband/core/cache.c
> > +++ b/drivers/infiniband/core/cache.c
> > @@ -510,8 +510,7 @@ int ib_find_cached_gid_by_port(struct ib_device *ib_dev,
> > struct ib_gid_attr val = {.ndev = ndev, .gid_type = gid_type};
> > unsigned long flags;
> >
> > - if (port < rdma_start_port(ib_dev) ||
> > - port > rdma_end_port(ib_dev))
> > + if (!rdma_is_port_valid(ib_dev, port))
> > return -ENOENT;
> >
> > table = ports_table[port - rdma_start_port(ib_dev)];
> > @@ -571,8 +570,7 @@ static int ib_cache_gid_find_by_filter(struct ib_device *ib_dev,
> > if (!ports_table)
> > return -EOPNOTSUPP;
> >
> > - if (port < rdma_start_port(ib_dev) ||
> > - port > rdma_end_port(ib_dev) ||
> > + if (!rdma_is_port_valid(ib_dev, port) ||
> > !rdma_protocol_roce(ib_dev, port))
> > return -EPROTONOSUPPORT;
> >
> > @@ -863,7 +861,7 @@ int ib_get_cached_gid(struct ib_device *device,
> > struct ib_gid_table **ports_table = device->cache.gid_cache;
> > struct ib_gid_table *table = ports_table[port_num - rdma_start_port(device)];
> >
> > - if (port_num < rdma_start_port(device) || port_num > rdma_end_port(device))
> > + if (!rdma_is_port_valid(device, port_num))
> > return -EINVAL;
> >
> > read_lock_irqsave(&table->rwlock, flags);
> > @@ -912,7 +910,7 @@ int ib_get_cached_pkey(struct ib_device *device,
> > unsigned long flags;
> > int ret = 0;
> >
> > - if (port_num < rdma_start_port(device) || port_num > rdma_end_port(device))
> > + if (!rdma_is_port_valid(device, port_num))
> > return -EINVAL;
> >
> > read_lock_irqsave(&device->cache.lock, flags);
> > @@ -941,7 +939,7 @@ int ib_find_cached_pkey(struct ib_device *device,
> > int ret = -ENOENT;
> > int partial_ix = -1;
> >
> > - if (port_num < rdma_start_port(device) || port_num > rdma_end_port(device))
> > + if (!rdma_is_port_valid(device, port_num))
> > return -EINVAL;
> >
> > read_lock_irqsave(&device->cache.lock, flags);
> > @@ -981,7 +979,7 @@ int ib_find_exact_cached_pkey(struct ib_device *device,
> > int i;
> > int ret = -ENOENT;
> >
> > - if (port_num < rdma_start_port(device) || port_num > rdma_end_port(device))
> > + if (!rdma_is_port_valid(device, port_num))
> > return -EINVAL;
> >
> > read_lock_irqsave(&device->cache.lock, flags);
> > @@ -1010,7 +1008,7 @@ int ib_get_cached_lmc(struct ib_device *device,
> > unsigned long flags;
> > int ret = 0;
> >
> > - if (port_num < rdma_start_port(device) || port_num > rdma_end_port(device))
> > + if (!rdma_is_port_valid(device, port_num))
> > return -EINVAL;
> >
> > read_lock_irqsave(&device->cache.lock, flags);
> > @@ -1037,7 +1035,7 @@ static void ib_cache_update(struct ib_device *device,
> > bool use_roce_gid_table =
> > rdma_cap_roce_gid_table(device, port);
> >
> > - if (port < rdma_start_port(device) || port > rdma_end_port(device))
> > + if (!rdma_is_port_valid(device, port))
> > return;
> >
> > table = ports_table[port - rdma_start_port(device)];
> > diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> > index e7dcfac..69f37b1 100644
> > --- a/drivers/infiniband/core/cma.c
> > +++ b/drivers/infiniband/core/cma.c
> > @@ -269,8 +269,7 @@ struct cma_device *cma_enum_devices_by_ibdev(cma_device_filter filter,
> > int cma_get_default_gid_type(struct cma_device *cma_dev,
> > unsigned int port)
> > {
> > - if (port < rdma_start_port(cma_dev->device) ||
> > - port > rdma_end_port(cma_dev->device))
> > + if (!rdma_is_port_valid(cma_dev->device, port))
> > return -EINVAL;
> >
> > return cma_dev->default_gid_type[port - rdma_start_port(cma_dev->device)];
> > @@ -282,8 +281,7 @@ int cma_set_default_gid_type(struct cma_device *cma_dev,
> > {
> > unsigned long supported_gids;
> >
> > - if (port < rdma_start_port(cma_dev->device) ||
> > - port > rdma_end_port(cma_dev->device))
> > + if (!rdma_is_port_valid(cma_dev->device, port))
> > return -EINVAL;
> >
> > supported_gids = roce_gid_type_mask_support(cma_dev->device, port);
> > diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> > index 571974c..f2e4865 100644
> > --- a/drivers/infiniband/core/device.c
> > +++ b/drivers/infiniband/core/device.c
> > @@ -659,7 +659,7 @@ int ib_query_port(struct ib_device *device,
> > union ib_gid gid;
> > int err;
> >
> > - if (port_num < rdma_start_port(device) || port_num > rdma_end_port(device))
> > + if (!rdma_is_port_valid(device, port_num))
> > return -EINVAL;
> >
> > memset(port_attr, 0, sizeof(*port_attr));
> > @@ -825,7 +825,7 @@ int ib_modify_port(struct ib_device *device,
> > if (!device->modify_port)
> > return -ENOSYS;
> >
> > - if (port_num < rdma_start_port(device) || port_num > rdma_end_port(device))
> > + if (!rdma_is_port_valid(device, port_num))
> > return -EINVAL;
> >
> > return device->modify_port(device, port_num, port_modify_mask,
> > diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
> > index 71580cc..9b77fbc 100644
> > --- a/drivers/infiniband/core/verbs.c
> > +++ b/drivers/infiniband/core/verbs.c
> > @@ -1205,8 +1205,7 @@ int ib_resolve_eth_dmac(struct ib_device *device,
> > {
> > int ret = 0;
> >
> > - if (ah_attr->port_num < rdma_start_port(device) ||
> > - ah_attr->port_num > rdma_end_port(device))
> > + if (!rdma_is_port_valid(device, ah_attr->port_num))
> > return -EINVAL;
> >
> > if (!rdma_cap_eth_ah(device, ah_attr->port_num))
> > diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> > index 958a24d..81c68e9 100644
> > --- a/include/rdma/ib_verbs.h
> > +++ b/include/rdma/ib_verbs.h
> > @@ -2275,6 +2275,13 @@ static inline u8 rdma_end_port(const struct ib_device *device)
> > return rdma_cap_ib_switch(device) ? 0 : device->phys_port_cnt;
> > }
> >
> > +static inline int rdma_is_port_valid(const struct ib_device *device,
> > + unsigned int port)
> > +{
> > + return (port >= rdma_start_port(device) &&
> > + port <= rdma_end_port(device));
> > +}
> > +
> > static inline bool rdma_protocol_ib(const struct ib_device *device, u8 port_num)
> > {
> > return device->port_immutable[port_num].core_cap_flags & RDMA_CORE_CAP_PROT_IB;
> > --
> > 1.8.3.1
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] IB/core: Add inline function to validate port
[not found] ` <20170126081534.GC3939-Hxa29pjIrETlQW142y8m19+IiqhCXseY@public.gmane.org>
@ 2017-01-26 12:59 ` Leon Romanovsky
0 siblings, 0 replies; 6+ messages in thread
From: Leon Romanovsky @ 2017-01-26 12:59 UTC (permalink / raw)
To: Yuval Shaia
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w,
sagig-VPRAkNaXOzVWk0Htik3J/w, talatb-VPRAkNaXOzVWk0Htik3J/w,
avivh-VPRAkNaXOzVWk0Htik3J/w, pandit.parav-Re5JQEeQqe8AvxtiuMwx3w,
swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW,
bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ,
valex-VPRAkNaXOzVWk0Htik3J/w, haggaie-VPRAkNaXOzVWk0Htik3J/w,
arnd-r2nGTMty4D4, weiyj.lk-Re5JQEeQqe8AvxtiuMwx3w,
markb-VPRAkNaXOzVWk0Htik3J/w, ira.weiny-ral2JQCrhuEAvxtiuMwx3w,
eli-VPRAkNaXOzVWk0Htik3J/w, dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA,
yishaih-VPRAkNaXOzVWk0Htik3J/w, monis-VPRAkNaXOzVWk0Htik3J/w,
linux-rdma-u79uwXL29TY76Z2rM5mHXA
[-- Attachment #1: Type: text/plain, Size: 1138 bytes --]
On Thu, Jan 26, 2017 at 10:15:35AM +0200, Yuval Shaia wrote:
> On Wed, Jan 25, 2017 at 06:50:22PM +0200, Leon Romanovsky wrote:
> > On Wed, Jan 25, 2017 at 06:41:37PM +0200, Yuval Shaia wrote:
> > > Signed-off-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > > ---
> > > drivers/infiniband/core/cache.c | 18 ++++++++----------
> > > drivers/infiniband/core/cma.c | 6 ++----
> > > drivers/infiniband/core/device.c | 4 ++--
> > > drivers/infiniband/core/verbs.c | 3 +--
> > > include/rdma/ib_verbs.h | 7 +++++++
> > > 5 files changed, 20 insertions(+), 18 deletions(-)
> >
> > Nice change, however all places are pretended with "!".
> > Can we change the name of function to something like
> > "rdma_is_port_invalid"?
>
> As a matter of fact this thought cross my mind as usually callers cares more
> about 'invalid port' and not 'valid port' because it is used as exit criteria
> from a function. But as Bart mentioned in other reply, positive looks
> better that negative.
I'm fine with both options.
Thanks,
Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] IB/core: Add inline function to validate port
[not found] ` <1485362497-18723-1-git-send-email-yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
2017-01-25 16:50 ` Leon Romanovsky
@ 2017-01-27 19:34 ` Doug Ledford
1 sibling, 0 replies; 6+ messages in thread
From: Doug Ledford @ 2017-01-27 19:34 UTC (permalink / raw)
To: Yuval Shaia, sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w,
leon-DgEjT+Ai2ygdnm+yROfE0A, sagig-VPRAkNaXOzVWk0Htik3J/w,
talatb-VPRAkNaXOzVWk0Htik3J/w, avivh-VPRAkNaXOzVWk0Htik3J/w,
pandit.parav-Re5JQEeQqe8AvxtiuMwx3w,
swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW,
bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ,
valex-VPRAkNaXOzVWk0Htik3J/w, haggaie-VPRAkNaXOzVWk0Htik3J/w,
arnd-r2nGTMty4D4, weiyj.lk-Re5JQEeQqe8AvxtiuMwx3w,
markb-VPRAkNaXOzVWk0Htik3J/w, ira.weiny-ral2JQCrhuEAvxtiuMwx3w,
eli-VPRAkNaXOzVWk0Htik3J/w, dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA,
yishaih-VPRAkNaXOzVWk0Htik3J/w, monis-VPRAkNaXOzVWk0Htik3J/w,
linux-rdma-u79uwXL29TY76Z2rM5mHXA
[-- Attachment #1: Type: text/plain, Size: 683 bytes --]
On Wed, 2017-01-25 at 18:41 +0200, Yuval Shaia wrote:
> Signed-off-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> ---
> drivers/infiniband/core/cache.c | 18 ++++++++----------
> drivers/infiniband/core/cma.c | 6 ++----
> drivers/infiniband/core/device.c | 4 ++--
> drivers/infiniband/core/verbs.c | 3 +--
> include/rdma/ib_verbs.h | 7 +++++++
> 5 files changed, 20 insertions(+), 18 deletions(-)
Thanks, applied.
--
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-01-27 19:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-25 16:41 [PATCH] IB/core: Add inline function to validate port Yuval Shaia
[not found] ` <1485362497-18723-1-git-send-email-yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
2017-01-25 16:50 ` Leon Romanovsky
[not found] ` <20170125165022.GQ6005-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
2017-01-25 17:00 ` Bart Van Assche
2017-01-26 8:15 ` Yuval Shaia
[not found] ` <20170126081534.GC3939-Hxa29pjIrETlQW142y8m19+IiqhCXseY@public.gmane.org>
2017-01-26 12:59 ` Leon Romanovsky
2017-01-27 19:34 ` Doug Ledford
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox