Welcome! Log In Create A New Profile

Advanced

Re: [PATCH 1 of 3] QUIC: changed path validation timeout

Sergey Kandaurov
May 09, 2023 11:08AM
> On 9 May 2023, at 16:58, Sergey Kandaurov <pluknet@nginx.com> wrote:
>
>>
>> On 9 May 2023, at 14:11, Sergey Kandaurov <pluknet@nginx.com> wrote:
>>
>>>
>>> On 3 May 2023, at 17:00, Roman Arutyunyan <arut@nginx.com> wrote:
>>>
>>> Hi,
>>>
>>> On Mon, Apr 24, 2023 at 04:15:21PM +0400, Sergey Kandaurov wrote:
>>>>
>>>>
>>>
>>>> [..]
>>>> # HG changeset patch
>>>> # User Sergey Kandaurov <pluknet@nginx.com>
>>>> # Date 1682338151 -14400
>>>> # Mon Apr 24 16:09:11 2023 +0400
>>>> # Branch quic
>>>> # Node ID 808fe808e276496a9b026690c141201720744ab3
>>>> # Parent f49aba6e3fb54843d3e3bd5df26dbb45f5d3d687
>>>> QUIC: separated path validation retransmit backoff.
>>>>
>>>> Path validation packets containing PATH_CHALLENGE frames are sent separately
>>>> from regular frame queue, because of the need to use a decicated path and pad
>>>> the packets. The packets are sent periodically, separately from the regular
>>>> probe/lost detection mechanism. A path validation packet is resent up to 3
>>>> times, each time after PTO expiration, with increasing per-path PTO backoff.
>>>>
>>>> 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
>>>> @@ -736,7 +736,8 @@ ngx_quic_set_lost_timer(ngx_connection_t
>>>>
>>>> q = ngx_queue_last(&ctx->sent);
>>>> f = ngx_queue_data(q, ngx_quic_frame_t, queue);
>>>> - w = (ngx_msec_int_t) (f->last + ngx_quic_pto(c, ctx) - now);
>>>> + w = (ngx_msec_int_t) (f->last + (ngx_quic_pto(c, ctx) << qc->pto_count)
>>>> + - now);
>>>>
>>>> if (w < 0) {
>>>> w = 0;
>>>> @@ -785,10 +786,9 @@ ngx_quic_pto(ngx_connection_t *c, ngx_qu
>>>>
>>>> duration = qc->avg_rtt;
>>>> duration += ngx_max(4 * qc->rttvar, NGX_QUIC_TIME_GRANULARITY);
>>>> - duration <<= qc->pto_count;
>>>>
>>>> if (ctx->level == ssl_encryption_application && c->ssl->handshaked) {
>>>> - duration += qc->ctp.max_ack_delay << qc->pto_count;
>>>> + duration += qc->ctp.max_ack_delay;
>>>> }
>>>>
>>>> return duration;
>>>> @@ -846,7 +846,9 @@ ngx_quic_pto_handler(ngx_event_t *ev)
>>>> continue;
>>>> }
>>>>
>>>> - if ((ngx_msec_int_t) (f->last + ngx_quic_pto(c, ctx) - now) > 0) {
>>>> + if ((ngx_msec_int_t) (f->last + (ngx_quic_pto(c, ctx) << qc->pto_count)
>>>> + - now) > 0)
>>>> + {
>>>> continue;
>>>> }
>>>>
>>>> diff --git a/src/event/quic/ngx_event_quic_migration.c b/src/event/quic/ngx_event_quic_migration.c
>>>> --- a/src/event/quic/ngx_event_quic_migration.c
>>>> +++ b/src/event/quic/ngx_event_quic_migration.c
>>>> @@ -496,6 +496,7 @@ ngx_quic_validate_path(ngx_connection_t
>>>> "quic initiated validation of path seq:%uL", path->seqnum);
>>>>
>>>> path->validating = 1;
>>>> + path->tries = 0;
>>>>
>>>> if (RAND_bytes(path->challenge1, 8) != 1) {
>>>> return NGX_ERROR;
>>>> @@ -513,7 +514,6 @@ ngx_quic_validate_path(ngx_connection_t
>>>> pto = ngx_quic_pto(c, ctx);
>>>>
>>>> path->expires = ngx_current_msec + pto;
>>>> - path->tries = NGX_QUIC_PATH_RETRIES;
>>>>
>>>> if (!qc->path_validation.timer_set) {
>>>> ngx_add_timer(&qc->path_validation, pto);
>>>> @@ -578,7 +578,6 @@ ngx_quic_path_validation_handler(ngx_eve
>>>> qc = ngx_quic_get_connection(c);
>>>>
>>>> ctx = ngx_quic_get_send_ctx(qc, ssl_encryption_application);
>>>> - pto = ngx_quic_pto(c, ctx);
>>>>
>>>> next = -1;
>>>> now = ngx_current_msec;
>>>> @@ -605,7 +604,9 @@ ngx_quic_path_validation_handler(ngx_eve
>>>> continue;
>>>> }
>>>>
>>>> - if (--path->tries) {
>>>> + if (++path->tries < NGX_QUIC_PATH_RETRIES) {
>>>> + pto = ngx_quic_pto(c, ctx) << path->tries;
>>>
>>> Here we schedule a timer for 2 * PTO or 4 * PTO. Technically, our path
>>> validation code allows several paths to be validated at the same time. If that
>>> happens, 2 * PTO and 4 * PTO will be too much for the first attempt for the new
>>> path, where the timeout is just PTO. As a result, the last path may wait 2x
>>> or 4x longer than needed.
>>
>> Agree it needs to be addressed.
>>
>> Below is the first glance to set the next available pv timer:
>> 1) in ngx_quic_validate_path(), a new path may own a shorter timer
>> than already established, especially after several timeouts
>> Previously, new path validation could be scheduled late.
>> 2) in ngx_quic_handle_path_response_frame(), a validated path
>> might left a spurious timer, while nothing more to validate
>> or remaining paths have more distant timer
>> This one is less severe, just extra work added.
>>
>> Unlike the two above, pv handler has a more complex
>> logic I decided not to touch it.
>>
>
> Updated patch, with timers logic moved inside
> similar to ngx_quic_set_lost_timer().
>
> # HG changeset patch
> # User Sergey Kandaurov <pluknet@nginx.com>
> # Date 1683636833 -14400
> # Tue May 09 16:53:53 2023 +0400
> # Branch quic
> # Node ID a40ce69032547110b1a10e953ca55dfc684f131d
> # Parent c9a6960c548501b5234ca7f7ff5ae23fad75b023
> QUIC: reschedule path validation on path insertion/removal.
>
> Two issues fixed:
> - new path validation could be scheduled late
> - a validated path could leave a spurious timer
>
> diff --git a/src/event/quic/ngx_event_quic_migration.c b/src/event/quic/ngx_event_quic_migration.c
> --- a/src/event/quic/ngx_event_quic_migration.c
> +++ b/src/event/quic/ngx_event_quic_migration.c
> @@ -16,6 +16,7 @@ static ngx_int_t ngx_quic_validate_path(
> ngx_quic_path_t *path);
> static ngx_int_t ngx_quic_send_path_challenge(ngx_connection_t *c,
> ngx_quic_path_t *path);
> +static void ngx_quic_set_path_timer(ngx_connection_t *c);
> static ngx_quic_path_t *ngx_quic_get_path(ngx_connection_t *c, ngx_uint_t tag);
>
>
> @@ -169,6 +170,8 @@ valid:
> path->validating = 0;
> path->limited = 0;
>
> + ngx_quic_set_path_timer(c);
> +
> return NGX_OK;
> }
>
> @@ -515,9 +518,7 @@ ngx_quic_validate_path(ngx_connection_t
>
> path->expires = ngx_current_msec + pto;
>
> - if (!qc->path_validation.timer_set) {
> - ngx_add_timer(&qc->path_validation, pto);
> - }
> + ngx_quic_set_path_timer(c);
>
> return NGX_OK;
> }
> @@ -563,6 +564,46 @@ ngx_quic_send_path_challenge(ngx_connect
> }
>
>
> +static void
> +ngx_quic_set_path_timer(ngx_connection_t *c)
> +{
> + ngx_msec_t now;
> + ngx_queue_t *q;
> + ngx_msec_int_t left, next;
> + ngx_quic_path_t *path;
> + ngx_quic_connection_t *qc;
> +
> + qc = ngx_quic_get_connection(c);
> +
> + now = ngx_current_msec;
> + next = -1;
> +
> + for (q = ngx_queue_head(&qc->paths);
> + q != ngx_queue_sentinel(&qc->paths);
> + q = ngx_queue_next(q))
> + {
> + path = ngx_queue_data(q, ngx_quic_path_t, queue);
> +
> + if (!path->validating) {
> + continue;
> + }
> +
> + left = path->expires - now;
> +
> + if (next == -1 || left < next) {
> + next = left;
> + }
> + }
> +
> + if (next == -1) {
> + ngx_del_timer(&qc->path_validation);
> +
> + } else {
> + ngx_add_timer(&qc->path_validation, next);
> + }
> +}
> +
> +
> void
> ngx_quic_path_validation_handler(ngx_event_t *ev)
> {
>

Addendum to handle negation expiration time.

Previously, when updating path validation timer, another path being
in the process of validation could already expire, notably when both
were scheduled on apparent migration.
This resulted in negative expiration time and could left a path
without a timer.

diff --git a/src/event/quic/ngx_event_quic_migration.c b/src/event/quic/ngx_event_quic_migration.c
--- a/src/event/quic/ngx_event_quic_migration.c
+++ b/src/event/quic/ngx_event_quic_migration.c
@@ -586,17 +586,18 @@ ngx_quic_set_path_timer(ngx_connection_t
}

left = path->expires - now;
+ left = ngx_max(left, 1);

if (next == -1 || left < next) {
next = left;
}
}

- if (next == -1) {
- ngx_del_timer(&qc->path_validation);
+ if (next != -1) {
+ ngx_add_timer(&qc->path_validation, next);

- } else {
- ngx_add_timer(&qc->path_validation, next);
+ } else if (qc->path_validation.timer_set) {
+ ngx_del_timer(&qc->path_validation);
}
}


--
Sergey Kandaurov
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

[PATCH 0 of 3] QUIC path MTU discovery

Roman Arutyunyan 939 March 28, 2023 10:52AM

[PATCH 1 of 3] QUIC: changed path validation timeout

Roman Arutyunyan 150 March 28, 2023 10:52AM

Re: [PATCH 1 of 3] QUIC: changed path validation timeout

Sergey Kandaurov 162 April 24, 2023 08:16AM

Re: [PATCH 1 of 3] QUIC: changed path validation timeout

Roman Arutyunyan 129 May 03, 2023 09:02AM

Re: [PATCH 1 of 3] QUIC: changed path validation timeout

Sergey Kandaurov 102 May 09, 2023 06:12AM

Re: [PATCH 1 of 3] QUIC: changed path validation timeout

Sergey Kandaurov 114 May 09, 2023 09:00AM

Re: [PATCH 1 of 3] QUIC: changed path validation timeout

Sergey Kandaurov 112 May 09, 2023 11:08AM

Re: [PATCH 1 of 3] QUIC: changed path validation timeout

Roman Arutyunyan 129 May 09, 2023 11:30AM

[PATCH 2 of 3] QUIC: allowed ngx_quic_frame_sendto() to return NGX_AGAIN

Roman Arutyunyan 154 March 28, 2023 10:52AM

[PATCH 3 of 3] QUIC: path MTU discovery

Roman Arutyunyan 207 March 28, 2023 11:00AM

Re: [PATCH 3 of 3] QUIC: path MTU discovery

Sergey Kandaurov 141 May 01, 2023 01:00PM

Re: [PATCH 3 of 3] QUIC: path MTU discovery

Maxim Dounin 127 May 01, 2023 04:42PM

Re: [PATCH 3 of 3] QUIC: path MTU discovery

Roman Arutyunyan 132 May 08, 2023 08:16AM

Re: [PATCH 3 of 3] QUIC: path MTU discovery

Sergey Kandaurov 113 May 08, 2023 11:12AM

[PATCH 0 of 4] QUIC path MTU discovery

Roman Arutyunyan 87 July 06, 2023 09:58AM

[PATCH 1 of 4] QUIC: removed path->limited flag

Roman Arutyunyan 92 July 06, 2023 09:58AM

Re: [PATCH 1 of 4] QUIC: removed path->limited flag

Sergey Kandaurov 91 July 28, 2023 11:52AM

[PATCH 2 of 4] QUIC: removed explicit packet padding for certain frames

Roman Arutyunyan 91 July 06, 2023 09:58AM

Re: [PATCH 2 of 4] QUIC: removed explicit packet padding for certain frames

Sergey Kandaurov 83 July 28, 2023 11:52AM

[PATCH 3 of 4] QUIC: allowed ngx_quic_frame_sendto() to return NGX_AGAIN

Roman Arutyunyan 91 July 06, 2023 09:58AM

Re: [PATCH 3 of 4] QUIC: allowed ngx_quic_frame_sendto() to return NGX_AGAIN

Sergey Kandaurov 86 July 28, 2023 11:52AM

[PATCH 4 of 4] QUIC: path MTU discovery

Roman Arutyunyan 96 July 06, 2023 09:58AM

Re: [PATCH 4 of 4] QUIC: path MTU discovery

Roman Arutyunyan 97 July 07, 2023 04:00AM

Re: [PATCH 4 of 4] QUIC: path MTU discovery

Sergey Kandaurov 100 July 28, 2023 11:54AM

Re: [PATCH 4 of 4] QUIC: path MTU discovery

Roman Arutyunyan 93 July 31, 2023 01:36PM

Re: [PATCH 4 of 4] QUIC: path MTU discovery

Sergey Kandaurov 122 August 08, 2023 05:50AM

Re: [PATCH 4 of 4] QUIC: path MTU discovery

Roman Arutyunyan 93 August 10, 2023 06:22AM

Re: [PATCH 4 of 4] QUIC: path MTU discovery

Sergey Kandaurov 88 August 10, 2023 10:20AM

Re: [PATCH 4 of 4] QUIC: path MTU discovery

Roman Arutyunyan 105 August 14, 2023 09:54AM

Re: [PATCH 0 of 4] QUIC path MTU discovery

Sergey Kandaurov 87 July 28, 2023 11:52AM



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

Online Users

Guests: 327
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready