On Tue, Nov 14, 2017 at 02:51:53PM +0200, Leon Romanovsky wrote:
> From: Erez Shitrit <erezsh@xxxxxxxxxxxx>
>
> If from some reason the SM responses to path query request with response
> that doesn't contain the exact SGID, ipoib should warn and change that
> part of the response before push it to the path record DB.
> Otherwise, new record will be added to the path record DB with no access
> to them from the ipoib.
>
> Demonstration of the bug is as the follow:
> ipoib wants to send to GID fe80:0000:0000:0000:0002:c903:00ef:5ee2, it
> creates new record in the DB with that gid as a key, and issues a new
> request to the sm.
> Now, the SM from some reason returns path-record with other SGID (for
> example, fe80:0000:0000:0001:0002:c903:00ef:5ee2 that contains the local
> subnet prefix) now ipoib will overwrite the current entry with the new
> one, and if new request to the original GID arrives ipoib will not find
> it in the DB (was overwritten) and will create new record that in its
> turn will also be overwritten by the response from the SM, and so on
> till the driver eats all the device memory.
>
> Signed-off-by: Erez Shitrit <erezsh@xxxxxxxxxxxx>
> Signed-off-by: Leon Romanovsky <leon@xxxxxxxxxx>
> drivers/infiniband/ulp/ipoib/ipoib_main.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> index 12b7f911f0e5..b173d618c59c 100644
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> @@ -775,6 +775,16 @@ static void path_rec_completion(int status,
> spin_lock_irqsave(&priv->lock, flags);
>
> if (!IS_ERR_OR_NULL(ah)) {
> + /* check there is no mismatch from the request */
> + if (memcmp(pathrec->dgid.raw, path->pathrec.dgid.raw,
> + sizeof(union ib_gid))) {
> + pr_warn("%s got PathRec for gid %pI6 while asked for %pI6\n",
> + dev->name, pathrec->dgid.raw, path->pathrec.dgid.raw);
> + /* overwrite the response from the sm before copy to the db */
> + memcpy(pathrec->dgid.raw, path->pathrec.dgid.raw,
> + sizeof(union ib_gid));
This doesn't seem like it should be a warning, and replacing the
good DGID from the SA with local garbage seems really wrong.
The SA should be free to return a correct DGID, for instance by
replacing a old or link local subnet prefix with the current value of
the subnet prefix, or by returning a different gid index for the
destination depending on mulitpath requirements.
So the right thing to do here is to somehow make ipoib use the data
from the SA.
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
