[PATCH] nvmet-auth: complete a request only after freeing the dhchap pointers

[Maurizio Lombardi]

po 16. 10. 2023 v 7:57 odesílatel Christoph Hellwig <hch at infradead.org> napsal:
>
> On Fri, Oct 13, 2023 at 11:42:39AM +0200, Maurizio Lombardi wrote:
> > -     nvmet_req_complete(req, status);
> >       if (req->sq->dhchap_step != NVME_AUTH_DHCHAP_MESSAGE_SUCCESS2 &&
> >           req->sq->dhchap_step != NVME_AUTH_DHCHAP_MESSAGE_FAILURE2) {
> >               unsigned long auth_expire_secs = ctrl->kato ? ctrl->kato : 120;
> >
> >               mod_delayed_work(system_wq, &req->sq->auth_expired_work,
> >                                auth_expire_secs * HZ);
> > +
> > +             nvmet_req_complete(req, status);
> >               return;
> >       }
> >       /* Final states, clear up variables */
> >       nvmet_auth_sq_free(req->sq);
> >       if (req->sq->dhchap_step == NVME_AUTH_DHCHAP_MESSAGE_FAILURE2)
> >               nvmet_ctrl_fatal_error(ctrl);
> > +     nvmet_req_complete(req, status);
>
> I'd be tempted to use a goto here to have a single completion path.

Ok I will modify it.

>
> >  }
> >
> >  static int nvmet_auth_challenge(struct nvmet_req *req, void *d, int al)
> > @@ -514,11 +516,14 @@ void nvmet_execute_auth_receive(struct nvmet_req *req)
> >       kfree(d);
> >  done:
> >       req->cqe->result.u64 = 0;
> > -     nvmet_req_complete(req, status);
> > +
> >       if (req->sq->dhchap_step == NVME_AUTH_DHCHAP_MESSAGE_SUCCESS2)
> >               nvmet_auth_sq_free(req->sq);
> >       else if (req->sq->dhchap_step == NVME_AUTH_DHCHAP_MESSAGE_FAILURE1) {
> >               nvmet_auth_sq_free(req->sq);
> >               nvmet_ctrl_fatal_error(ctrl);
> > +             nvmet_req_complete(req, status);
> > +             return;
> >       }
> > +     nvmet_req_complete(req, status);
>
> And this looks really odd to me.  Why do a manual complete and return
> in the branch just to skip the complete right after it?
>

You are right, it's a leftover of previous experiments and I didn't
realize I could
simply remove this redundant code.
will submit a V2.

thanks,
Maurizio