Re: [PATCH] ksmbd: improve credits management

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2021년 10월 6일 (수) 오전 11:06, Namjae Jeon <linkinjeon@xxxxxxxxxx>님이 작성:
>
> [snip]
> >
> > -     if (credits_requested > 0) {
> > +     /* We must grant 0 credit for the final response of an asynchronous
> > +      * operation.
> > +      */
> > +     if ((hdr->Flags & SMB2_FLAGS_ASYNC_COMMAND) && !work->multiRsp) {
> Can you elaborate what is this check ? especially this !work->multiRsp..

According to 3.3.4.2 Sending an Interim Response for an Asynchronous Operation
in MS-SMB2,
For the request that will be handled asynchronously,  a server should send
an interim response, and grant credits to the interim response.
on the other hand, a server should grant 0 credit to the final response for the
request.

"hdr->Flags & SMB2_FLAGS_ASYNC" means that a response is an interim
or final, and "work->multiRsp == 0" means that a response is final. So in this
case, a server should grant 0 credit.

> > +             credits_granted = 0;
> > +     } else {
> > +             /* according to smb2.credits smbtorture, Windows server
> > +              * 2016 or later grant up to 8192 credits at one.
> > +              */
> >               aux_credits = credits_requested - 1;
> > -             aux_max = 32;
> >               if (hdr->Command == SMB2_NEGOTIATE)
> >                       aux_max = 0;
> > -             aux_credits = (aux_credits < aux_max) ? aux_credits : aux_max;
> > -             credits_granted = aux_credits + credit_charge;
> > +             else
> > +                     aux_max = conn->max_credits - credit_charge;
> > +             aux_credits = min_t(unsigned short, aux_credits, aux_max);
> > +             credits_granted = credit_charge + aux_credits;
> > +
> > +             if (conn->max_credits - conn->total_credits < credits_granted)
> > +                     credits_granted = conn->max_credits -
> > +                             conn->total_credits;
> >
> > -             /* if credits granted per client is getting bigger than default
> > -              * minimum credits then we should wrap it up within the limits.
> > -              */
> > -             if ((conn->total_credits + credits_granted) > min_credits)
> > -                     credits_granted = min_credits - conn->total_credits;
> >               /*
> >                * TODO: Need to adjuct CreditRequest value according to
> >                * current cpu load
> >                */
> > -     } else if (conn->total_credits == 0) {
> > -             credits_granted = 1;
> >       }
> >
> >       conn->total_credits += credits_granted;
> > @@ -371,7 +358,6 @@ int smb2_set_rsp_credits(struct ksmbd_work *work)
> >               /* Update CreditRequest in last request */
> >               hdr->CreditRequest = cpu_to_le16(work->credits_granted);
> >       }
> > -out:
> >       ksmbd_debug(SMB,
> >                   "credits: requested[%d] granted[%d] total_granted[%d]\n",
> >                   credits_requested, credits_granted,
> > @@ -692,13 +678,18 @@ int setup_async_work(struct ksmbd_work *work, void
> > (*fn)(void **), void **arg)
> >
> >  void smb2_send_interim_resp(struct ksmbd_work *work, __le32 status)
> >  {
> > -     struct smb2_hdr *rsp_hdr;
> > +     struct smb2_hdr *rsp_hdr = work->response_buf;
> > +
> > +     work->multiRsp = 1;
> > +     if (status != STATUS_CANCELLED) {
> > +             spin_lock(&work->conn->credits_lock);
> > +             smb2_set_rsp_credits(work);
> Can you explain why this code is needed in smb2_send_interim_resp() ?
>

As I wrote above, a server should grant credits to an interim
response.

> > +             spin_unlock(&work->conn->credits_lock);
> > +     }
> >
> > -     rsp_hdr = work->response_buf;
> >       smb2_set_err_rsp(work);
> >       rsp_hdr->Status = status;
> >
> > -     work->multiRsp = 1;
> >       ksmbd_conn_write(work);
> >       rsp_hdr->Status = 0;
> >       work->multiRsp = 0;
> > @@ -1233,6 +1224,7 @@ int smb2_handle_negotiate(struct ksmbd_work *work)
> >
> >       conn->srv_sec_mode = le16_to_cpu(rsp->SecurityMode);
> >       ksmbd_conn_set_need_negotiate(work);
> > +     conn->total_credits = 0;
> This line is needed ?

Yes, conn->total_credits becomes 0 when receiving a SMB2_NEGOTIATION
request. But init_smbX_X_server functions are called many times and
conn->total_credits is set to 1 in these functions while negotiation.

>
> Thanks!
> >
> >  err_out:
> >       if (rc < 0)
> > --
> > 2.25.1
> >
> >



-- 
Thanks,
Hyunchul




[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux