Re: Question about IB_QP_CUR_STATE

[Leon Romanovsky <leon@kernel.org>]

On Sun, Jul 12, 2020 at 01:53:13PM +0300, Gal Pressman wrote:
> On 12/07/2020 13:44, Leon Romanovsky wrote:
> > On Thu, Jul 09, 2020 at 09:13:45AM +0300, Gal Pressman wrote:
> >> On 08/07/2020 18:44, Leon Romanovsky wrote:
> >>> On Wed, Jul 08, 2020 at 03:42:34PM +0300, Gal Pressman wrote:
> >>>> On 08/07/2020 12:41, liweihang wrote:
> >>>>> Hi all,
> >>>>>
> >>>>> I'm a little confused about the role of IB_QP_CUR_STATE in the enumeration
> >>>>> ib_qp_attr_mask.
> >>>>>
> >>>>> In manual page of ibv_modify_qp(), comments of cur_qp_state is "Assume this
> >>>>> is the current QP state". Why we need to get current qp state from users
> >>>>> instead of drivers?
> >>>>>
> >>>>> For example, why the users are allowed to modify qp from RTR to RTS again
> >>>>> even if the qp's state in driver and hardware has already been RTS.
> >>>>>
> >>>>> I would be appretiate it if someone can help with this.
> >>>>>
> >>>>> Weihang
> >>>>>
> >>>>
> >>>> Talking about IB_QP_CUR_STATE, I see many drivers filling it in their query QP
> >>>> callback although it should only be used in modify operations.. Is there a
> >>>> reason not to remove it?
> >>>
> >>> IBTA section "11.2.5.3 QUERY QUEUE PAIR" has line about IB_QP_CUR_STATE.
> >>> It is one of output modifiers.
> >>>
> >>> Thanks
> >>>
> >>
> >> It says the current QP state should be returned, that's what qp_state field is used for.
> >> According to the man pages:
> >>
> >> libibverbs/man/ibv_query_qp.3:
> >>                enum ibv_qp_state       qp_state;            /* Current QP state */
> >>                enum ibv_qp_state       cur_qp_state;        /* Current QP state - irrelevant for ibv_query_qp */
> >
> > I don't think that users read it, because ibv_cmd_query_qp() filled
> > qp_state to be equal to cur_qp_state from day one.
>
> Where do you see that?
>
> I see:
> ibv_cmd_query_qp():
> 	attr->qp_state                      = resp.qp_state;
> 	attr->cur_qp_state                  = resp.cur_qp_state;

It comes from the the kernel as is and in the kernel:
mlx5_ib_query_qp():
  4699         qp_attr->qp_state            = qp->state;
  4700         qp_attr->cur_qp_state        = qp_attr->qp_state;

mlx4_ib_query_qp():
  4049         qp->state                    = to_ib_qp_state(mlx4_state);
  4050         qp_attr->qp_state            = qp->state;

Thanks