> On 18 Nov 2022, at 07:58, Maxim Dounin <mdounin@mdounin.ru> wrote:
>
> Hello!
>
> On Tue, Nov 15, 2022 at 04:28:30PM +0400, Sergey Kandaurov wrote:
>
>>
>>> On 21 Oct 2022, at 04:10, Maxim Dounin <mdounin@mdounin.ru> wrote:
>>>
>>> Hello!
>>>
>>> On Tue, Oct 11, 2022 at 02:35:52PM +0400, Sergey Kandaurov wrote:
>>>
>>>> # HG changeset patch
>>>> # User Sergey Kandaurov <pluknet@nginx.com>
>>>> # Date 1665484414 -14400
>>>> # Tue Oct 11 14:33:34 2022 +0400
>>>> # Branch quic
>>>> # Node ID c0165ddcb1c6981f8e5230081f03a277f62d20c3
>>>> # Parent caced81ce0a9cb218ae8cdd6176c12e0614acee9
>>>> QUIC: support for setting QUIC methods with LibreSSL.
>>>>
>>>> Setting QUIC methods is converted to use C99 designated initializers
>>>> for simplicity, as LibreSSL 3.6.0 has different SSL_QUIC_METHOD layout.
>>>
>>> I'm somewhat sceptical about C99 designated initializers. These
>>> aren't supported by MSVC till 2012: in particular, this includes
>>> all MSVC versions available via wineticks, as well as MSVC
>>> versions currently used to build nginx win32 binaries.
>>>
>>> A more portable solution might be to use run-time initialization
>>> instead.
>>
>> While I agree in principle, fixing build with MSVC would require
>> a larger work to implement support for UDP connections on win32.
>> Specifically, nginx-quic uses c->udp->buffer and sockets API that
>> needs porting to win32, see ngx_quic_send() / ngx_quic_recvmsg().
>
> This shouldn't be a major problem, given the relevant APIs are
> available on Windows.
>
> But the point is: there are no reasons to introduce additional
> issues in advance, we'll have to fix this sooner or later.
>
>> Other than that, currently the build fails in other places
>> as seen with MSVC 2010. Below the patches to address this.

[..]

>
> Looks good.
>
>> # HG changeset patch
>> # User Sergey Kandaurov <pluknet@nginx.com>
>> # Date 1668513998 -14400
>> # Tue Nov 15 16:06:38 2022 +0400
>> # Branch quic
>> # Node ID 1ba7992c7bb0f801e069adb15b3378b5211a85a8
>> # Parent d760c2e49d4c4647621765fef3404c34d1aef81b
>> QUIC: silenced C4334 MSVC warning about 32 to 64 bits convertion.
>
> converSion?

Sure, tnx.

>
>>
>> diff --git a/src/event/quic/ngx_event_quic_ack.c b/src/event/quic/ngx_event_quic_ack.c
>> --- a/src/event/quic/ngx_event_quic_ack.c
>> +++ b/src/event/quic/ngx_event_quic_ack.c
>> @@ -195,7 +195,7 @@ ngx_quic_rtt_sample(ngx_connection_t *c,
>> } else {
>> qc->min_rtt = ngx_min(qc->min_rtt, latest_rtt);
>>
>> - ack_delay = ack->delay * (1 << qc->ctp.ack_delay_exponent) / 1000;
>> + ack_delay = ack->delay * (1ULL << qc->ctp.ack_delay_exponent) / 1000;
>>
>> if (c->ssl->handshaked) {
>> ack_delay = ngx_min(ack_delay, qc->ctp.max_ack_delay);
>
> Why "1 << ..." at all?
> Shouldn't it be
>
> diff -r d9ef59e283e3 src/event/quic/ngx_event_quic_ack.c
> --- a/src/event/quic/ngx_event_quic_ack.c Tue Nov 15 16:02:08 2022 +0400
> +++ b/src/event/quic/ngx_event_quic_ack.c Fri Nov 18 06:55:05 2022 +0300
> @@ -195,7 +195,7 @@ ngx_quic_rtt_sample(ngx_connection_t *c,
> } else {
> qc->min_rtt = ngx_min(qc->min_rtt, latest_rtt);
>
> - ack_delay = ack->delay * (1 << qc->ctp.ack_delay_exponent) / 1000;
> + ack_delay = (ack->delay << qc->ctp.ack_delay_exponent) / 1000;
>
> if (c->ssl->handshaked) {
> ack_delay = ngx_min(ack_delay, qc->ctp.max_ack_delay);
>
>
> instead?

Yes, indeed.

BTW, this prevented to emit efficient code of just shift left.
Previously, it used to store and sign extend an intermediate result
and apply the multiply instruction even on high optimization level.

--
Sergey Kandaurov

_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org