On Tue, Jan 09, 2018 at 01:10:57PM +0200, Leon Romanovsky wrote:
> From: Parav Pandit <parav@xxxxxxxxxxxx>
>
> This patch introduces an API that allows legacy applications to query
> GIDs for a rdma_cm_id which is used during connection establishment.
>
> GIDs are stored and created differently for IB, RoCE and iWARP transports.
> Therefore rdma_read_gids() returns GID for all the transports hiding
> such internal details to caller. It is usable for client side and server
> side connections.
>
> The rdma_read_gids() should not be used by any new ULPs.
This is weird statement. Why can't we change the few callers to do
whatever is correct now?
> +void rdma_read_gids(struct rdma_cm_id *cm_id, union ib_gid *sgid,
> + union ib_gid *dgid)
> +{
> + struct rdma_addr *addr = &cm_id->route.addr;
> +
> + if (!cm_id->device)
> + return;
Nope. I looked at only one caller and saw this return leaks uninited
kernel stack memory to userspace. Probably best to zero the sgid/dgid
in the error case??
> diff --git a/include/rdma/ib_addr.h b/include/rdma/ib_addr.h
> index fa809a7b48e7..edb1cdf2a892 100644
> +++ b/include/rdma/ib_addr.h
> @@ -165,6 +165,9 @@ static inline u16 rdma_vlan_dev_vlan_id(const struct net_device *dev)
>
> static inline int rdma_ip2gid(struct sockaddr *addr, union ib_gid *gid)
> {
> + if (!gid)
> + return 0;
> +
> switch (addr->sa_family) {
> case AF_INET:
> ipv6_addr_set_v4mapped(((struct sockaddr_in *)
> @@ -205,6 +208,9 @@ static inline void rdma_gid2ip(struct sockaddr *out, const union ib_gid *gid)
> static inline void rdma_addr_get_sgid(struct rdma_dev_addr *dev_addr,
> union ib_gid *gid)
> {
> + if (!gid)
> + return;
I think it would be clearer to put this test in the rdma_read_gids.
It makes no sense to have a function with one output where the output
is optional. Particularly when it is inlined..
And the prior patch makes a bit more sense after looking here, but the
series seems in the wrong order -> the patch to break
rdma_addr_get_sgid should go after the only rocee code path in ucma is
switched to use rdma_read_gids which hopefully does something similar
to what it did before?
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
