On Wed, Apr 22, 2020 at 11:39:47AM +0300, Maor Gottlieb wrote:
> Add a call to rdma_lag_get_ah_roce_slave when
> Address handle is created.
> Low driver can use it to select the QP's affinity port.
Lower
> Signed-off-by: Maor Gottlieb <maorg@xxxxxxxxxxxx>
> Reviewed-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> drivers/infiniband/core/verbs.c | 44 ++++++++++++++++++++++-----------
> 1 file changed, 30 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
> index 56a71337112c..a0d60376ba6b 100644
> +++ b/drivers/infiniband/core/verbs.c
> @@ -50,6 +50,7 @@
> #include <rdma/ib_cache.h>
> #include <rdma/ib_addr.h>
> #include <rdma/rw.h>
> +#include <rdma/lag.h>
>
> #include "core_priv.h"
> #include <trace/events/rdma_core.h>
> @@ -554,8 +555,14 @@ struct ib_ah *rdma_create_ah(struct ib_pd *pd, struct rdma_ah_attr *ah_attr,
> if (ret)
> return ERR_PTR(ret);
>
> - ah = _rdma_create_ah(pd, ah_attr, flags, NULL);
> + ret = rdma_lag_get_ah_roce_slave(pd->device, ah_attr);
> + if (ret) {
> + rdma_unfill_sgid_attr(ah_attr, old_sgid_attr);
> + return ERR_PTR(ret);
> + }
>
> + ah = _rdma_create_ah(pd, ah_attr, flags, NULL);
> + rdma_lag_put_ah_roce_slave(ah_attr);
> rdma_unfill_sgid_attr(ah_attr, old_sgid_attr);
> return ah;
> }
> @@ -1638,6 +1645,25 @@ static int _ib_modify_qp(struct ib_qp *qp, struct ib_qp_attr *attr,
> &old_sgid_attr_av);
> if (ret)
> return ret;
> +
> + if (attr->ah_attr.type == RDMA_AH_ATTR_TYPE_ROCE &&
> + is_qp_type_connected(qp)) {
> + /*
> + * If the user provided the qp_attr then we have to
> + * resolve it. Kerne users have to provide already
Kernel
> + * resolved rdma_ah_attr's.
> + */
> + if (udata) {
> + ret = ib_resolve_eth_dmac(qp->device,
> + &attr->ah_attr);
> + if (ret)
> + goto out_av;
> + }
> + ret = rdma_lag_get_ah_roce_slave(qp->device,
> + &attr->ah_attr);
> + if (ret)
> + goto out_av;
> + }
> }
> if (attr_mask & IB_QP_ALT_PATH) {
> /*
> @@ -1664,18 +1690,6 @@ static int _ib_modify_qp(struct ib_qp *qp, struct ib_qp_attr *attr,
> }
> }
>
> - /*
> - * If the user provided the qp_attr then we have to resolve it. Kernel
> - * users have to provide already resolved rdma_ah_attr's
> - */
> - if (udata && (attr_mask & IB_QP_AV) &&
> - attr->ah_attr.type == RDMA_AH_ATTR_TYPE_ROCE &&
> - is_qp_type_connected(qp)) {
> - ret = ib_resolve_eth_dmac(qp->device, &attr->ah_attr);
> - if (ret)
> - goto out;
> - }
Why did this need to move up?
> -
> if (rdma_ib_or_roce(qp->device, port)) {
> if (attr_mask & IB_QP_RQ_PSN && attr->rq_psn & ~0xffffff) {
> dev_warn(&qp->device->dev,
> @@ -1717,8 +1731,10 @@ static int _ib_modify_qp(struct ib_qp *qp, struct ib_qp_attr *attr,
> if (attr_mask & IB_QP_ALT_PATH)
> rdma_unfill_sgid_attr(&attr->alt_ah_attr, old_sgid_attr_alt_av);
> out_av:
> - if (attr_mask & IB_QP_AV)
> + if (attr_mask & IB_QP_AV) {
> + rdma_lag_put_ah_roce_slave(&attr->ah_attr);
This seems wwrong why doesn't rdma_unfill_sgid_attr do this?
Jason
