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() ?
>
>>
>> Thanks!
>> >
>> > err_out:
>> > if (rc < 0)
>> > --
>> > 2.25.1
>> >
>> >
>
>
>
> --
> Thanks,
> Hyunchul
>
