> On 6 Jul 2023, at 17:57, Roman Arutyunyan <arut@nginx.com> wrote:
>
> # HG changeset patch
> # User Roman Arutyunyan <arut@nginx.com>
> # Date 1688481216 -14400
> # Tue Jul 04 18:33:36 2023 +0400
> # Node ID 1a49fd5d2013b6c8d50263499e6ced440927e949
> # Parent a1ea543d009311765144351b2557c00c8e6445bf
> QUIC: path MTU discovery.
>
> MTU selection starts by stepping up until the first failure. Then binary
> search is used to find the path MTU.
>
> diff --git a/src/core/ngx_connection.c b/src/core/ngx_connection.c
> --- a/src/core/ngx_connection.c
> +++ b/src/core/ngx_connection.c
> @@ -1583,6 +1583,10 @@ ngx_connection_error(ngx_connection_t *c
> }
> #endif
>
> + if (err == NGX_EMSGSIZE && c->log_error == NGX_ERROR_IGNORE_EMSGSIZE) {
> + return 0;
> + }
> +
> if (err == 0
> || err == NGX_ECONNRESET
> #if (NGX_WIN32)
> @@ -1600,6 +1604,7 @@ ngx_connection_error(ngx_connection_t *c
> {
> switch (c->log_error) {
>
> + case NGX_ERROR_IGNORE_EMSGSIZE:
> case NGX_ERROR_IGNORE_EINVAL:
> case NGX_ERROR_IGNORE_ECONNRESET:
> case NGX_ERROR_INFO:
> diff --git a/src/core/ngx_connection.h b/src/core/ngx_connection.h
> --- a/src/core/ngx_connection.h
> +++ b/src/core/ngx_connection.h
> @@ -97,7 +97,8 @@ typedef enum {
> NGX_ERROR_ERR,
> NGX_ERROR_INFO,
> NGX_ERROR_IGNORE_ECONNRESET,
> - NGX_ERROR_IGNORE_EINVAL
> + NGX_ERROR_IGNORE_EINVAL,
> + NGX_ERROR_IGNORE_EMSGSIZE
> } ngx_connection_log_error_e;
>
>
> diff --git a/src/event/quic/ngx_event_quic.c b/src/event/quic/ngx_event_quic.c
> --- a/src/event/quic/ngx_event_quic.c
> +++ b/src/event/quic/ngx_event_quic.c
> @@ -149,11 +149,6 @@ ngx_quic_apply_transport_params(ngx_conn
> ngx_log_error(NGX_LOG_INFO, c->log, 0,
> "quic maximum packet size is invalid");
> return NGX_ERROR;
> -
> - } else if (ctp->max_udp_payload_size > ngx_quic_max_udp_payload(c)) {
> - ctp->max_udp_payload_size = ngx_quic_max_udp_payload(c);
> - ngx_log_debug0(NGX_LOG_DEBUG_EVENT, c->log, 0,
> - "quic client maximum packet size truncated");
> }
>
> if (ctp->active_connection_id_limit < 2) {
> @@ -286,7 +281,7 @@ ngx_quic_new_connection(ngx_connection_t
>
> qc->path_validation.log = c->log;
> qc->path_validation.data = c;
> - qc->path_validation.handler = ngx_quic_path_validation_handler;
> + qc->path_validation.handler = ngx_quic_path_handler;
>
> qc->conf = conf;
>
> @@ -297,7 +292,7 @@ ngx_quic_new_connection(ngx_connection_t
> ctp = &qc->ctp;
>
> /* defaults to be used before actual client parameters are received */
> - ctp->max_udp_payload_size = ngx_quic_max_udp_payload(c);
> + ctp->max_udp_payload_size = NGX_QUIC_MAX_UDP_PAYLOAD_SIZE;
> ctp->ack_delay_exponent = NGX_QUIC_DEFAULT_ACK_DELAY_EXPONENT;
> ctp->max_ack_delay = NGX_QUIC_DEFAULT_MAX_ACK_DELAY;
> ctp->active_connection_id_limit = 2;
> 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
> @@ -229,6 +229,12 @@ ngx_quic_handle_ack_frame_range(ngx_conn
>
> qc = ngx_quic_get_connection(c);
>
> + if (ctx->level == ssl_encryption_application) {
> + if (ngx_quic_handle_path_mtu_ack(c, qc->path, min, max) != NGX_OK) {
> + return NGX_ERROR;
> + }
> + }
> +
> st->max_pn = NGX_TIMER_INFINITE;
> found = 0;
>
> diff --git a/src/event/quic/ngx_event_quic_connection.h b/src/event/quic/ngx_event_quic_connection.h
> --- a/src/event/quic/ngx_event_quic_connection.h
> +++ b/src/event/quic/ngx_event_quic_connection.h
> @@ -66,6 +66,14 @@ typedef struct ngx_quic_keys_s ng
> #define ngx_quic_get_socket(c) ((ngx_quic_socket_t *)((c)->udp))
>
>
> +typedef enum {
> + NGX_QUIC_PATH_IDLE = 0,
> + NGX_QUIC_PATH_VALIDATING,
> + NGX_QUIC_PATH_WAITING,
> + NGX_QUIC_PATH_DISCOVERING_MTU

Nitpicking on too long name.
What about NGX_QUIC_PATH_MTUD or something.

> +} ngx_quic_path_state_e;
> +
> +
> struct ngx_quic_client_id_s {
> ngx_queue_t queue;
> uint64_t seqnum;
> @@ -89,18 +97,22 @@ struct ngx_quic_path_s {
> ngx_sockaddr_t sa;
> socklen_t socklen;
> ngx_quic_client_id_t *cid;
> + ngx_quic_path_state_e state;
> ngx_msec_t expires;
> ngx_uint_t tries;
> ngx_uint_t tag;
> + size_t mtu;
> + size_t mtud;
> + size_t max_mtu;
> off_t sent;
> off_t received;
> u_char challenge1[8];
> u_char challenge2[8];
> uint64_t seqnum;
> + uint64_t mtu_pnum[NGX_QUIC_PATH_RETRIES];
> ngx_str_t addr_text;
> u_char text[NGX_SOCKADDR_STRLEN];
> - unsigned validated:1;
> - unsigned validating:1;
> + ngx_uint_t validated; /* unsigned validated:1; */
> };
>
>
> 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
> @@ -10,6 +10,11 @@
> #include <ngx_event_quic_connection.h>
>
>
> +#define NGX_QUIC_PATH_MTU_DELAY 1000

The value looks too much,
it would delay each subsequent probe (which is not retry).

> +#define NGX_QUIC_PATH_MTU_STEP 256
> +#define NGX_QUIC_PATH_MTU_PRECISION 16
> +
> +
> static void ngx_quic_set_connection_path(ngx_connection_t *c,
> ngx_quic_path_t *path);
> static ngx_int_t ngx_quic_validate_path(ngx_connection_t *c,
> @@ -17,7 +22,15 @@ static ngx_int_t ngx_quic_validate_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_int_t ngx_quic_expire_path_validation(ngx_connection_t *c,
> + ngx_quic_path_t *path);
> +static ngx_int_t ngx_quic_expire_path_mtu_delay(ngx_connection_t *c,
> + ngx_quic_path_t *path);
> +static ngx_int_t ngx_quic_expire_path_mtu_discovery(ngx_connection_t *c,
> + ngx_quic_path_t *path);
> static ngx_quic_path_t *ngx_quic_get_path(ngx_connection_t *c, ngx_uint_t tag);
> +static ngx_int_t ngx_quic_send_path_mtu_probe(ngx_connection_t *c,
> + ngx_quic_path_t *path);
>
>
> ngx_int_t
> @@ -97,7 +110,7 @@ ngx_quic_handle_path_response_frame(ngx_
> {
> path = ngx_queue_data(q, ngx_quic_path_t, queue);
>
> - if (!path->validating) {
> + if (path->state != NGX_QUIC_PATH_VALIDATING) {
> continue;
> }
>
> @@ -136,6 +149,9 @@ valid:
> {
> /* address did not change */
> rst = 0;
> +
> + path->mtu = prev->mtu;
> + path->max_mtu = prev->max_mtu;
> }
> }
>
> @@ -167,9 +183,8 @@ valid:
> ngx_quic_path_dbg(c, "is validated", path);
>
> path->validated = 1;
> - path->validating = 0;
>
> - ngx_quic_set_path_timer(c);
> + ngx_quic_discover_path_mtu(c, path);
>
> return NGX_OK;
> }
> @@ -217,6 +232,8 @@ ngx_quic_new_path(ngx_connection_t *c,
> path->addr_text.len = ngx_sock_ntop(sockaddr, socklen, path->text,
> NGX_SOCKADDR_STRLEN, 1);
>
> + path->mtu = NGX_QUIC_MIN_INITIAL_SIZE;
> +
> ngx_log_debug2(NGX_LOG_DEBUG_EVENT, c->log, 0,
> "quic path seq:%uL created addr:%V",
> path->seqnum, &path->addr_text);
> @@ -464,7 +481,7 @@ ngx_quic_handle_migration(ngx_connection
>
> ngx_quic_set_connection_path(c, next);
>
> - if (!next->validated && !next->validating) {
> + if (!next->validated && next->state != NGX_QUIC_PATH_VALIDATING) {
> if (ngx_quic_validate_path(c, next) != NGX_OK) {
> return NGX_ERROR;
> }
> @@ -492,7 +509,6 @@ ngx_quic_validate_path(ngx_connection_t
> ngx_log_debug1(NGX_LOG_DEBUG_EVENT, c->log, 0,
> "quic initiated validation of path seq:%uL", path->seqnum);
>
> - path->validating = 1;
> path->tries = 0;
>
> if (RAND_bytes(path->challenge1, 8) != 1) {
> @@ -511,6 +527,7 @@ ngx_quic_validate_path(ngx_connection_t
> pto = ngx_max(ngx_quic_pto(c, ctx), 1000);
>
> path->expires = ngx_current_msec + pto;
> + path->state = NGX_QUIC_PATH_VALIDATING;
>
> ngx_quic_set_path_timer(c);
>
> @@ -558,6 +575,42 @@ ngx_quic_send_path_challenge(ngx_connect
> }
>
>
> +void
> +ngx_quic_discover_path_mtu(ngx_connection_t *c, ngx_quic_path_t *path)
> +{
> + ngx_quic_connection_t *qc;
> +
> + qc = ngx_quic_get_connection(c);
> +
> + if (path->max_mtu) {
> + if (path->max_mtu - path->mtu <= NGX_QUIC_PATH_MTU_PRECISION) {
> + path->state = NGX_QUIC_PATH_IDLE;
> + ngx_quic_set_path_timer(c);
> + return;
> + }
> +
> + path->mtud = (path->mtu + path->max_mtu) / 2;
> +
> + } else {
> + path->mtud = path->mtu + NGX_QUIC_PATH_MTU_STEP;

I like this approach, compared to multiplying probe steps.
This better fits to the pattern to keep around a well-known
initial value, which is typically 1500 bytes.
Another one could be to let user specify its own initial
value for cases when he knows well its network environment.

> +
> + if (path->mtud >= qc->ctp.max_udp_payload_size) {
> + path->mtud = qc->ctp.max_udp_payload_size;
> + path->max_mtu = qc->ctp.max_udp_payload_size;
> + }
> + }
> +
> + path->state = NGX_QUIC_PATH_WAITING;
> + path->expires = ngx_current_msec + NGX_QUIC_PATH_MTU_DELAY;
> +
> + ngx_log_debug2(NGX_LOG_DEBUG_EVENT, c->log, 0,
> + "quic path seq:%uL schedule mtu:%uz",
> + path->seqnum, path->mtud);
> +
> + ngx_quic_set_path_timer(c);
> +}
> +
> +
> static void
> ngx_quic_set_path_timer(ngx_connection_t *c)
> {
> @@ -578,7 +631,7 @@ ngx_quic_set_path_timer(ngx_connection_t
> {
> path = ngx_queue_data(q, ngx_quic_path_t, queue);
>
> - if (!path->validating) {
> + if (path->state == NGX_QUIC_PATH_IDLE) {
> continue;
> }
>
> @@ -600,22 +653,18 @@ ngx_quic_set_path_timer(ngx_connection_t
>
>
> void
> -ngx_quic_path_validation_handler(ngx_event_t *ev)
> +ngx_quic_path_handler(ngx_event_t *ev)
> {
> ngx_msec_t now;
> ngx_queue_t *q;
> - ngx_msec_int_t left, next, pto;
> - ngx_quic_path_t *path, *bkp;
> + ngx_msec_int_t left;
> + ngx_quic_path_t *path;
> ngx_connection_t *c;
> - ngx_quic_send_ctx_t *ctx;
> ngx_quic_connection_t *qc;
>
> c = ev->data;
> qc = ngx_quic_get_connection(c);
>
> - ctx = ngx_quic_get_send_ctx(qc, ssl_encryption_application);
> -
> - next = -1;
> now = ngx_current_msec;
>
> q = ngx_queue_head(&qc->paths);
> @@ -625,83 +674,274 @@ ngx_quic_path_validation_handler(ngx_eve
> path = ngx_queue_data(q, ngx_quic_path_t, queue);
> q = ngx_queue_next(q);
>
> - if (!path->validating) {
> + if (path->state == NGX_QUIC_PATH_IDLE) {
> continue;
> }
>
> left = path->expires - now;
>
> if (left > 0) {
> -
> - if (next == -1 || left < next) {
> - next = left;
> - }
> -
> - continue;
> - }
> -
> - if (++path->tries < NGX_QUIC_PATH_RETRIES) {
> - pto = ngx_max(ngx_quic_pto(c, ctx), 1000) << path->tries;
> -
> - path->expires = ngx_current_msec + pto;
> -
> - if (next == -1 || pto < next) {
> - next = pto;
> - }
> -
> - /* retransmit */
> - (void) ngx_quic_send_path_challenge(c, path);
> -
> continue;
> }
>
> - ngx_log_debug1(NGX_LOG_DEBUG_EVENT, ev->log, 0,
> - "quic path seq:%uL validation failed", path->seqnum);
> + switch (path->state) {
> + case NGX_QUIC_PATH_VALIDATING:
> + if (ngx_quic_expire_path_validation(c, path) != NGX_OK) {
> + goto failed;
> + }
> +
> + break;
> +
> + case NGX_QUIC_PATH_WAITING:
> + if (ngx_quic_expire_path_mtu_delay(c, path) != NGX_OK) {
> + goto failed;
> + }
> +
> + break;
> +
> + case NGX_QUIC_PATH_DISCOVERING_MTU:
> + if (ngx_quic_expire_path_mtu_discovery(c, path) != NGX_OK) {
> + goto failed;
> + }
> +
> + break;
> +

indentation in several places here

> + default:
> + break;
> + }
> + }
> +
> + ngx_quic_set_path_timer(c);
> +
> + return;
>
> - /* found expired path */
> +failed:
> +
> + ngx_quic_close_connection(c, NGX_ERROR);
> +}
> +
> +
> +static ngx_int_t
> +ngx_quic_expire_path_validation(ngx_connection_t *c, ngx_quic_path_t *path)
> +{
> + ngx_msec_int_t pto;
> + ngx_quic_path_t *bkp;
> + ngx_quic_send_ctx_t *ctx;
> + ngx_quic_connection_t *qc;
>
> - path->validated = 0;
> - path->validating = 0;
> + qc = ngx_quic_get_connection(c);
> + ctx = ngx_quic_get_send_ctx(qc, ssl_encryption_application);
> +
> + if (++path->tries < NGX_QUIC_PATH_RETRIES) {
> + pto = ngx_max(ngx_quic_pto(c, ctx), 1000) << path->tries;
> + path->expires = ngx_current_msec + pto;
> +
> + (void) ngx_quic_send_path_challenge(c, path);
> +
> + return NGX_OK;
> + }
> +
> + ngx_log_debug1(NGX_LOG_DEBUG_EVENT, c->log, 0,
> + "quic path seq:%uL validation failed", path->seqnum);
> +
> + /* found expired path */
> +
> + path->validated = 0;
>
>
> - /* RFC 9000, 9.3.2. On-Path Address Spoofing
> - *
> - * To protect the connection from failing due to such a spurious
> - * migration, an endpoint MUST revert to using the last validated
> - * peer address when validation of a new peer address fails.
> - */
> -
> - if (qc->path == path) {
> - /* active path validation failed */
> -
> - bkp = ngx_quic_get_path(c, NGX_QUIC_PATH_BACKUP);
> + /* RFC 9000, 9.3.2. On-Path Address Spoofing
> + *
> + * To protect the connection from failing due to such a spurious
> + * migration, an endpoint MUST revert to using the last validated
> + * peer address when validation of a new peer address fails.
> + */
>
> - if (bkp == NULL) {
> - qc->error = NGX_QUIC_ERR_NO_VIABLE_PATH;
> - qc->error_reason = "no viable path";
> - ngx_quic_close_connection(c, NGX_ERROR);
> - return;
> - }
> + if (qc->path == path) {
> + /* active path validation failed */
> +
> + bkp = ngx_quic_get_path(c, NGX_QUIC_PATH_BACKUP);
>
> - qc->path = bkp;
> - qc->path->tag = NGX_QUIC_PATH_ACTIVE;
> -
> - ngx_quic_set_connection_path(c, qc->path);
> -
> - ngx_log_error(NGX_LOG_INFO, c->log, 0,
> - "quic path seq:%uL addr:%V is restored from backup",
> - qc->path->seqnum, &qc->path->addr_text);
> -
> - ngx_quic_path_dbg(c, "is active", qc->path);
> + if (bkp == NULL) {
> + qc->error = NGX_QUIC_ERR_NO_VIABLE_PATH;
> + qc->error_reason = "no viable path";
> + return NGX_ERROR;
> }
>
> - if (ngx_quic_free_path(c, path) != NGX_OK) {
> - ngx_quic_close_connection(c, NGX_ERROR);
> - return;
> + qc->path = bkp;
> + qc->path->tag = NGX_QUIC_PATH_ACTIVE;
> +
> + ngx_quic_set_connection_path(c, qc->path);
> +
> + ngx_log_error(NGX_LOG_INFO, c->log, 0,
> + "quic path seq:%uL addr:%V is restored from backup",
> + qc->path->seqnum, &qc->path->addr_text);
> +
> + ngx_quic_path_dbg(c, "is active", qc->path);
> + }
> +
> + return ngx_quic_free_path(c, path);
> +}
> +
> +
> +static ngx_int_t
> +ngx_quic_expire_path_mtu_delay(ngx_connection_t *c, ngx_quic_path_t *path)
> +{
> + ngx_int_t rc;
> + ngx_uint_t i;
> + ngx_msec_t pto;
> + ngx_quic_send_ctx_t *ctx;
> + ngx_quic_connection_t *qc;
> +
> + qc = ngx_quic_get_connection(c);
> + ctx = ngx_quic_get_send_ctx(qc, ssl_encryption_application);
> +
> + path->tries = 0;
> +
> + for ( ;; ) {
> +
> + for (i = 0; i < NGX_QUIC_PATH_RETRIES; i++) {
> + path->mtu_pnum[i] = NGX_QUIC_UNSET_PN;
> + }
> +
> + rc = ngx_quic_send_path_mtu_probe(c, path);
> +
> + if (rc == NGX_ERROR) {
> + return NGX_ERROR;
> + }
> +
> + if (rc == NGX_OK) {
> + pto = ngx_quic_pto(c, ctx);
> + path->expires = ngx_current_msec + pto;
> + path->state = NGX_QUIC_PATH_DISCOVERING_MTU;
> + return NGX_OK;
> + }
> +
> + /* rc == NGX_DECLINED */
> +
> + path->max_mtu = path->mtud;
> +
> + if (path->max_mtu - path->mtu <= NGX_QUIC_PATH_MTU_PRECISION) {
> + path->state = NGX_QUIC_PATH_IDLE;
> + return NGX_OK;
> + }
> +
> + path->mtud = (path->mtu + path->max_mtu) / 2;
> + }
> +}
> +
> +
> +static ngx_int_t
> +ngx_quic_expire_path_mtu_discovery(ngx_connection_t *c, ngx_quic_path_t *path)
> +{
> + ngx_int_t rc;
> + ngx_msec_int_t pto;
> + ngx_quic_send_ctx_t *ctx;
> + ngx_quic_connection_t *qc;
> +
> + qc = ngx_quic_get_connection(c);
> + ctx = ngx_quic_get_send_ctx(qc, ssl_encryption_application);
> +
> + if (++path->tries < NGX_QUIC_PATH_RETRIES) {
> + pto = ngx_quic_pto(c, ctx) << path->tries;
> + path->expires = ngx_current_msec + pto;
> +

Setting path->expires there seems useless.
It don't seem to be used in ngx_quic_send_path_mtu_probe(),
and ngx_quic_discover_path_mtu() used to reset path->expires.

> + rc = ngx_quic_send_path_mtu_probe(c, path);
> + if (rc != NGX_DECLINED) {
> + return rc;
> }
> }
>
> - if (next != -1) {
> - ngx_add_timer(&qc->path_validation, next);
> + ngx_log_debug2(NGX_LOG_DEBUG_EVENT, c->log, 0,
> + "quic path seq:%uL expired mtu:%uz",
> + path->seqnum, path->mtud);
> +
> + path->max_mtu = path->mtud;
> +
> + ngx_quic_discover_path_mtu(c, path);
> +
> + return NGX_OK;
> +}
> +
> +
> +static ngx_int_t
> +ngx_quic_send_path_mtu_probe(ngx_connection_t *c, ngx_quic_path_t *path)
> +{
> + ngx_int_t rc;
> + ngx_uint_t log_error;
> + ngx_quic_frame_t frame;
> + ngx_quic_send_ctx_t *ctx;
> + ngx_quic_connection_t *qc;
> +
> + ngx_memzero(&frame, sizeof(ngx_quic_frame_t));
> +
> + frame.level = ssl_encryption_application;
> + frame.type = NGX_QUIC_FT_PING;
> +
> + qc = ngx_quic_get_connection(c);
> + ctx = ngx_quic_get_send_ctx(qc, ssl_encryption_application);
> + path->mtu_pnum[path->tries] = ctx->pnum;
> +
> + ngx_log_debug4(NGX_LOG_DEBUG_EVENT, c->log, 0,
> + "quic path seq:%uL send probe "
> + "mtu:%uz pnum:%uL tries:%ui",
> + path->seqnum, path->mtud, ctx->pnum, path->tries);
> +
> + log_error = c->log_error;
> + c->log_error = NGX_ERROR_IGNORE_EMSGSIZE;
> +
> + rc = ngx_quic_frame_sendto(c, &frame, path->mtud, path);
> + c->log_error = log_error;
> +
> + if (rc == NGX_ERROR) {
> + if (c->write->error) {
> + c->write->error = 0;
> +
> + ngx_log_debug2(NGX_LOG_DEBUG_EVENT, c->log, 0,
> + "quic path seq:%uL rejected mtu:%uz",
> + path->seqnum, path->mtud);
> +
> + return NGX_DECLINED;

The error handling looks awkward to decline on write error.
ngx_sendmsg() sets write error on fatal sendmsg errors,
and keeps it unset NGX_EAGAIN and NGX_EINTR, which
are typically non-fatal transient errors.
I understand this is done mostly to ignore EMSGSIZE,
maybe it makes sense to ignore the rest sendmsg() errors.

> + }
> +
> + return NGX_ERROR;
> }
> +
> + return NGX_OK;
> }
> +
> +
> +ngx_int_t
> +ngx_quic_handle_path_mtu_ack(ngx_connection_t *c, ngx_quic_path_t *path,
> + uint64_t min, uint64_t max)
> +{
> + uint64_t pnum;
> + ngx_uint_t i;
> +
> + if (path->state != NGX_QUIC_PATH_DISCOVERING_MTU) {
> + return NGX_OK;
> + }
> +
> + for (i = 0; i < NGX_QUIC_PATH_RETRIES; i++) {
> + pnum = path->mtu_pnum[i];
> +
> + if (pnum == NGX_QUIC_UNSET_PN) {
> + break;
> + }
> +
> + if (pnum < min || pnum > max) {
> + continue;
> + }
> +
> + path->mtu = path->mtud;
> +
> + ngx_log_debug2(NGX_LOG_DEBUG_EVENT, c->log, 0,
> + "quic path seq:%uL ack mtu:%uz",
> + path->seqnum, path->mtu);
> +
> + ngx_quic_discover_path_mtu(c, path);
> +
> + break;

What if two probes appeared ACKed in one range.
Though that seems to be safe to break there, as two
probes barely have different probe sizes, rather retries.

> + }
> +
> + return NGX_OK;
> +}
> diff --git a/src/event/quic/ngx_event_quic_migration.h b/src/event/quic/ngx_event_quic_migration.h
> --- a/src/event/quic/ngx_event_quic_migration.h
> +++ b/src/event/quic/ngx_event_quic_migration.h
> @@ -18,10 +18,10 @@
> #define NGX_QUIC_PATH_BACKUP 2
>
> #define ngx_quic_path_dbg(c, msg, path) \
> - ngx_log_debug6(NGX_LOG_DEBUG_EVENT, c->log, 0, \
> - "quic path seq:%uL %s sent:%O recvd:%O state:%s%s", \
> + ngx_log_debug7(NGX_LOG_DEBUG_EVENT, c->log, 0, \
> + "quic path seq:%uL %s tx:%O rx:%O valid:%d st:%d mtu:%uz", \
> path->seqnum, msg, path->sent, path->received, \
> - path->validated ? "V": "N", path->validating ? "R": "");
> + path->validated, path->state, path->mtu);
>
> ngx_int_t ngx_quic_handle_path_challenge_frame(ngx_connection_t *c,
> ngx_quic_header_t *pkt, ngx_quic_path_challenge_frame_t *f);
> @@ -36,6 +36,10 @@ ngx_int_t ngx_quic_set_path(ngx_connecti
> ngx_int_t ngx_quic_handle_migration(ngx_connection_t *c,
> ngx_quic_header_t *pkt);
>
> -void ngx_quic_path_validation_handler(ngx_event_t *ev);
> +void ngx_quic_path_handler(ngx_event_t *ev);
> +
> +void ngx_quic_discover_path_mtu(ngx_connection_t *c, ngx_quic_path_t *path);
> +ngx_int_t ngx_quic_handle_path_mtu_ack(ngx_connection_t *c,
> + ngx_quic_path_t *path, uint64_t min, uint64_t max);

Nitpicking:
maybe just ngx_quic_handle_path_mtu() ?
This would match ngx_quic_discover_path_mtu() naming.

>
> #endif /* _NGX_EVENT_QUIC_MIGRATION_H_INCLUDED_ */
> diff --git a/src/event/quic/ngx_event_quic_output.c b/src/event/quic/ngx_event_quic_output.c
> --- a/src/event/quic/ngx_event_quic_output.c
> +++ b/src/event/quic/ngx_event_quic_output.c
> @@ -10,9 +10,6 @@
> #include <ngx_event_quic_connection.h>
>
>
> -#define NGX_QUIC_MAX_UDP_PAYLOAD_OUT 1252
> -#define NGX_QUIC_MAX_UDP_PAYLOAD_OUT6 1232
> -
> #define NGX_QUIC_MAX_UDP_SEGMENT_BUF 65487 /* 65K - IPv6 header */
> #define NGX_QUIC_MAX_SEGMENTS 64 /* UDP_MAX_SEGMENTS */
>
> @@ -61,21 +58,6 @@ static size_t ngx_quic_path_limit(ngx_co
> size_t size);
>
>
> -size_t
> -ngx_quic_max_udp_payload(ngx_connection_t *c)
> -{
> - /* TODO: path MTU discovery */
> -
> -#if (NGX_HAVE_INET6)
> - if (c->sockaddr->sa_family == AF_INET6) {
> - return NGX_QUIC_MAX_UDP_PAYLOAD_OUT6;
> - }
> -#endif
> -
> - return NGX_QUIC_MAX_UDP_PAYLOAD_OUT;
> -}
> -
> -
> ngx_int_t
> ngx_quic_output(ngx_connection_t *c)
> {
> @@ -142,10 +124,7 @@ ngx_quic_create_datagrams(ngx_connection
>
> p = dst;
>
> - len = ngx_min(qc->ctp.max_udp_payload_size,
> - NGX_QUIC_MAX_UDP_PAYLOAD_SIZE);
> -
> - len = ngx_quic_path_limit(c, path, len);
> + len = ngx_quic_path_limit(c, path, path->mtu);
>
> pad = ngx_quic_get_padding_level(c);
>
> @@ -299,9 +278,7 @@ ngx_quic_allow_segmentation(ngx_connecti
> ctx = ngx_quic_get_send_ctx(qc, ssl_encryption_application);
>
> bytes = 0;
> -
> - len = ngx_min(qc->ctp.max_udp_payload_size,
> - NGX_QUIC_MAX_UDP_SEGMENT_BUF);
> + len = ngx_min(qc->path->mtu, NGX_QUIC_MAX_UDP_SEGMENT_BUF);
>
> for (q = ngx_queue_head(&ctx->frames);
> q != ngx_queue_sentinel(&ctx->frames);
> @@ -345,8 +322,7 @@ ngx_quic_create_segments(ngx_connection_
> return NGX_ERROR;
> }
>
> - segsize = ngx_min(qc->ctp.max_udp_payload_size,
> - NGX_QUIC_MAX_UDP_SEGMENT_BUF);
> + segsize = ngx_min(path->mtu, NGX_QUIC_MAX_UDP_SEGMENT_BUF);
> p = dst;
> end = dst + sizeof(dst);
>
> diff --git a/src/event/quic/ngx_event_quic_output.h b/src/event/quic/ngx_event_quic_output.h
> --- a/src/event/quic/ngx_event_quic_output.h
> +++ b/src/event/quic/ngx_event_quic_output.h
> @@ -12,8 +12,6 @@
> #include <ngx_core.h>
>
>
> -size_t ngx_quic_max_udp_payload(ngx_connection_t *c);
> -
> ngx_int_t ngx_quic_output(ngx_connection_t *c);
>
> ngx_int_t ngx_quic_negotiate_version(ngx_connection_t *c,
> diff --git a/src/event/quic/ngx_event_quic_ssl.c b/src/event/quic/ngx_event_quic_ssl.c
> --- a/src/event/quic/ngx_event_quic_ssl.c
> +++ b/src/event/quic/ngx_event_quic_ssl.c
> @@ -494,6 +494,8 @@ ngx_quic_crypto_input(ngx_connection_t *
> */
> ngx_quic_discard_ctx(c, ssl_encryption_handshake);
>
> + ngx_quic_discover_path_mtu(c, qc->path);
> +
> /* start accepting clients on negotiated number of server ids */
> if (ngx_quic_create_sockets(c) != NGX_OK) {
> return NGX_ERROR;
> diff --git a/src/os/unix/ngx_errno.h b/src/os/unix/ngx_errno.h
> --- a/src/os/unix/ngx_errno.h
> +++ b/src/os/unix/ngx_errno.h
> @@ -54,6 +54,7 @@ typedef int ngx_err_t;
> #define NGX_ENOMOREFILES 0
> #define NGX_ELOOP ELOOP
> #define NGX_EBADF EBADF
> +#define NGX_EMSGSIZE EMSGSIZE
>
> #if (NGX_HAVE_OPENAT)
> #define NGX_EMLINK EMLINK
> _______________________________________________
> nginx-devel mailing list
> nginx-devel@nginx.org
> https://mailman.nginx.org/mailman/listinfo/nginx-devel

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