On Thu, Jul 27, 2017 at 10:57:26AM -0400, Dennis Dalessandro wrote:
> On 7/27/2017 10:52 AM, Dennis Dalessandro wrote:
> > On 7/26/2017 2:12 PM, Leon Romanovsky wrote:
> > > From: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > >
> > > The commit ebc9ca43e1d5 ("IB/core: Allow QP state transition from
> > > reset to error")
> > > allowed transition from Reset to Error state for the QPs. This behavior
> > > doesn't follow the IBTA specification 1.3, which in 10.3.1 QUEUE PAIR AND
> > > EE CONTEXT STATES section.
> > >
> > > The quote from the spec:
> > > "An error can be forced from any state, except Reset, with
> > > the Modify QP/EE Verb."
> > >
> > > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > > Signed-off-by: Leon Romanovsky <leon@xxxxxxxxxx>
> > > ---
> > > drivers/infiniband/core/verbs.c | 1 -
> > > 1 file changed, 1 deletion(-)
> > >
> > > diff --git a/drivers/infiniband/core/verbs.c
> > > b/drivers/infiniband/core/verbs.c
> > > index fb98ed67d5bc..7f8fe443df46 100644
> > > --- a/drivers/infiniband/core/verbs.c
> > > +++ b/drivers/infiniband/core/verbs.c
> > > @@ -895,7 +895,6 @@ static const struct {
> > > } qp_state_table[IB_QPS_ERR + 1][IB_QPS_ERR + 1] = {
> > > [IB_QPS_RESET] = {
> > > [IB_QPS_RESET] = { .valid = 1 },
> > > - [IB_QPS_ERR] = { .valid = 1 },
> > > [IB_QPS_INIT] = {
> > > .valid = 1,
> > > .req_param = {
> > >
> >
> > The diagram from that section clearly shows you can't go from reset to
> > error. However table 91 as in our original commit message says it could
> > go to error from any state. Taking a step back and thinking about it,
> > I'm not sure it really makes sense to be able to go from reset to error
> > anyway. There is nothing that can go wrong until the qp is transitioned
> > out of reset really.
> >
> > Any idea why this patch was never adopted:
> > http://www.spinics.net/lists/linux-rdma/msg07627.html
> >
> > This was during my storage hiatus so I wasn't following things RDMA back
> > then. However this seems like the correct approach to me. Granted that
> > patch would look different as the code has moved around but the same
> > sort of check could go in ipoib_ib_dev_stop_default().
> >
> > -Denny
>
> Ah guess I should have waited until reading the next patch 11/11!
>
> Reviewed-by: Dennis Dalessandro <dennis.dalessandro@xxxxxxxxx>
Thanks Dennis,
I agree with you, the spec is really misleading.
Attachment:
signature.asc
Description: PGP signature
