2021년 10월 6일 (수) 오후 3:52, Namjae Jeon <linkinjeon@xxxxxxxxxx>님이 작성:
>
> 2021-10-06 15:27 GMT+09:00, Hyunchul Lee <hyc.lee@xxxxxxxxx>:
> > 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.
> Okay.
> >
> >> > + 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.
> Okay.
> >
> >> > + 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.
> If so, can we move conn->total_credits = 1 in init_smbX_X_server() to
> ksmbd_conn_alloc() ?
Okay, I will move this into ksmbd_conn_alloc().
Thank you for your comments!
> >
> >>
> >> Thanks!
> >> >
> >> > err_out:
> >> > if (rc < 0)
> >> > --
> >> > 2.25.1
> >> >
> >> >
> >
> >
> >
> > --
> > Thanks,
> > Hyunchul
> >
--
Thanks,
Hyunchul
