Welcome! Log In Create A New Profile

Advanced

Re: [PATCH 3 of 4] QUIC: support for setting QUIC methods with LibreSSL

Sergey Kandaurov
November 21, 2022 06:28AM
> 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
Subject Author Views Posted

[PATCH 0 of 4] quic libressl support

Sergey Kandaurov 535 October 06, 2022 06:54PM

[PATCH 4 of 4] QUIC: removed compatibility with older BoringSSL API

Sergey Kandaurov 62 October 06, 2022 06:54PM

[PATCH 3 of 4] QUIC: support for setting QUIC methods with LibreSSL

Sergey Kandaurov 50 October 06, 2022 06:54PM

[PATCH 1 of 4] QUIC: using native TLSv1.3 cipher suite constants

Sergey Kandaurov 70 October 06, 2022 06:54PM

[PATCH 2 of 4] QUIC: do not use SSL_set_quic_early_data_enabled() with LibreSSL

Sergey Kandaurov 76 October 06, 2022 06:54PM

[PATCH 0 of 4] quic libressl support #2

Sergey Kandaurov 55 October 11, 2022 06:44AM

[PATCH 2 of 4] QUIC: do not use SSL_set_quic_early_data_enabled() with LibreSSL

Sergey Kandaurov 46 October 11, 2022 06:44AM

Re: [PATCH 2 of 4] QUIC: do not use SSL_set_quic_early_data_enabled() with LibreSSL

Roman Arutyunyan 32 October 17, 2022 07:10AM

Re: [PATCH 2 of 4] QUIC: do not use SSL_set_quic_early_data_enabled() with LibreSSL

Sergey Kandaurov 31 October 17, 2022 10:06AM

Re: [PATCH 2 of 4] QUIC: do not use SSL_set_quic_early_data_enabled() with LibreSSL

Roman Arutyunyan 54 October 18, 2022 07:48AM

[PATCH 1 of 4] QUIC: using native TLSv1.3 cipher suite constants

Sergey Kandaurov 47 October 11, 2022 06:44AM

[PATCH 3 of 4] QUIC: support for setting QUIC methods with LibreSSL

Sergey Kandaurov 50 October 11, 2022 06:44AM

Re: [PATCH 3 of 4] QUIC: support for setting QUIC methods with LibreSSL

Roman Arutyunyan 42 October 17, 2022 09:32AM

Re: [PATCH 3 of 4] QUIC: support for setting QUIC methods with LibreSSL

Sergey Kandaurov 50 October 17, 2022 10:28AM

Re: [PATCH 3 of 4] QUIC: support for setting QUIC methods with LibreSSL

Maxim Dounin 41 October 20, 2022 08:12PM

Re: [PATCH 3 of 4] QUIC: support for setting QUIC methods with LibreSSL

Sergey Kandaurov 21 November 15, 2022 07:30AM

Re: [PATCH 3 of 4] QUIC: support for setting QUIC methods with LibreSSL

Maxim Dounin 18 November 17, 2022 11:00PM

Re: [PATCH 3 of 4] QUIC: support for setting QUIC methods with LibreSSL

Sergey Kandaurov 15 November 21, 2022 06:28AM

[PATCH 4 of 4] QUIC: removed compatibility with older BoringSSL API

Sergey Kandaurov 61 October 11, 2022 06:44AM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 96
Record Number of Users: 6 on February 13, 2018
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready