Opened 9 months ago

Closed 9 months ago

#2505 closed enhancement (fixed)

A question about the way of rttvar calculation in QUIC

Reported by: yangfurong@… Owned by:
Priority: minor Milestone:
Component: other Version: 1.23.x
Keywords: rttvar, quic Cc:
uname -a: Linux aserver-dev011122072160.na131 4.19.91-012.ali4000.alios7.x86_64 #1 SMP Wed Sep 15 17:27:09 CST 2021 x86_64 x86_64 x86_64 GNU/Linux
nginx -V: Tengine version: Tengine/2.0.1 (nginx/1.23.0)
built by gcc 4.8.5 20150623 (Red Hat 4.8.5-44) (GCC)
built with OpenSSL 1.1.1h 22 Sep 2020
TLS SNI support enabled
nginx: loaded modules:
nginx: ngx_core_module (static)
nginx: ngx_errlog_module (static)
nginx: ngx_conf_module (static)
nginx: ngx_openssl_module (static)
nginx: ngx_regex_module (static)
nginx: ngx_syslog_module (static)
nginx: ngx_procs_module (static)
nginx: ngx_proc_core_module (static)
nginx: ngx_proc_sl_resend_module (static)
nginx: ngx_proc_node_status_module (static)
nginx: ngx_tmd_config_procs_module (static)
nginx: ngx_events_module (static)
nginx: ngx_event_core_module (static)
nginx: ngx_epoll_module (static)
nginx: ngx_tcp_module (static)
nginx: ngx_tcp_core_module (static)
nginx: ngx_tcp_upstream_module (static)
nginx: ngx_tcp_proxy_module (static)
nginx: ngx_tcp_websocket_module (static)
nginx: ngx_tcp_upstream_ip_hash_module (static)
nginx: ngx_tcp_upstream_busyness_module (static)
nginx: ngx_tcp_debug_module (static)
nginx: ngx_tcp_ssl_module (static)
nginx: ngx_tcp_keyless_module (static)
nginx: ngx_tcp_lurk_module (static)
nginx: ngx_limit_tcp_module (static)
nginx: ngx_quic_module (static)
nginx: ngx_quic_bpf_module (static)
nginx: ngx_http_module (static)
nginx: ngx_http_core_module (static)
nginx: ngx_http_log_module (static)
nginx: ngx_http_upstream_module (static)
nginx: ngx_http_v2_module (static)
nginx: ngx_http_v2_stat_module (static)
nginx: ngx_http_v2_proxy_module (static)
nginx: ngx_http_spdy_module (static)
nginx: ngx_http_v3_module (static)
nginx: ngx_http_static_module (static)
nginx: ngx_http_gzip_static_module (static)
nginx: ngx_http_dav_module (static)
nginx: ngx_http_autoindex_module (static)
nginx: ngx_http_index_module (static)
nginx: ngx_http_random_index_module (static)
nginx: ngx_http_mirror_module (static)
nginx: ngx_http_try_files_module (static)
nginx: ngx_http_concat_module (static)
nginx: ngx_http_auth_basic_module (static)
nginx: ngx_http_access_module (static)
nginx: ngx_http_limit_conn_module (static)
nginx: ngx_http_limit_req_module (static)
nginx: ngx_http_realip_module (static)
nginx: ngx_http_geo_module (static)
nginx: ngx_http_map_module (static)
nginx: ngx_http_split_clients_module (static)
nginx: ngx_http_referer_module (static)
nginx: ngx_http_rewrite_module (static)
nginx: ngx_http_ssl_lurk_module (static)
nginx: ngx_http_ssl_module (static)
nginx: ngx_http_proxy_module (static)
nginx: ngx_http_fastcgi_module (static)
nginx: ngx_http_scgi_module (static)
nginx: ngx_http_grpc_module (static)
nginx: ngx_http_browser_module (static)
nginx: ngx_http_user_agent_module (static)
nginx: ngx_http_secure_link_module (static)
nginx: ngx_http_sysguard_module (static)
nginx: ngx_http_flv_module (static)
nginx: ngx_http_upstream_hash_module (static)
nginx: ngx_http_upstream_ip_hash_module (static)
nginx: ngx_http_upstream_consistent_hash_module (static)
nginx: ngx_http_upstream_check_module (static)
nginx: ngx_http_upstream_least_conn_module (static)
nginx: ngx_http_upstream_random_module (static)
nginx: ngx_http_upstream_keepalive_module (static)
nginx: ngx_http_upstream_zone_module (static)
nginx: ngx_http_stub_status_module (static)
nginx: ngx_http_addrs_keepalive_module (static)
nginx: ngx_http_quic_status_module (static)
nginx: ngx_http_quic_log_module (static)
nginx: ngx_http_malloc_stat_module (static)
nginx: ndk_http_module (static)
nginx: ngx_http_diamond_module (static)
nginx: ngx_http_tbpass_module (static)
nginx: ngx_http_hourglass_module (static)
nginx: ngx_http_tair_module (static)
nginx: ngx_http_etair_module (static)
nginx: ngx_reqstat_module (static)
nginx: ngx_http_cookie_pass_module (static)
nginx: ngx_http_referer_acl_module (static)
nginx: ngx_http_scroll_log_module (static)
nginx: ngx_http_sl_upstream_module (static)
nginx: ngx_http_slu_master_slave_module (static)
nginx: ngx_http_slu_round_robin_module (static)
nginx: ngx_http_bucket_id_module (static)
nginx: ngx_http_copy_module (static)
nginx: ngx_http_eagleeye_module (static)
nginx: ngx_http_reqstat_module (static)
nginx: ngx_http_wireless_variable_module (static)
nginx: ngx_http_p2pqos_module (static)
nginx: ngx_http_slow_log_module (static)
nginx: ngx_http_admin_module (static)
nginx: ngx_http_loop_detect_module (static)
nginx: ngx_http_req_auth_module (static)
nginx: ngx_http_cookie_auth_module (static)
nginx: ngx_http_dynamic_route_module (static)
nginx: ngx_http_unified_code_module (static)
nginx: ngx_http_backend_proxy_module (static)
nginx: ngx_http_backend_module (static)
nginx: ngx_http_backend_check_module (static)
nginx: ngx_http_backend_keepalive_module (static)
nginx: ngx_http_backend_ip_hash_module (static)
nginx: ngx_http_backend_least_conn_module (static)
nginx: ngx_http_backend_addrs_keepalive_module (static)
nginx: ngx_http_backend_consistent_hash_module (static)
nginx: ngx_http_debug_pool_module (static)
nginx: ngx_http_debug_conn_module (static)
nginx: ngx_http_cdn_debug_module (static)
nginx: ngx_http_xredirect_module (static)
nginx: ngx_http_slight_ssl_module (static)
nginx: ngx_http_server_names_module (static)
nginx: ngx_http_request_monitor_module (static)
nginx: ngx_http_slab_stat_module (static)
nginx: ngx_http_limit_rt_module (static)
nginx: ngx_http_med_module (static)
nginx: ngx_http_global_traffic_limit_module (static)
nginx: ngx_http_ssl_proxy_status_module (static)
nginx: ngx_tcp_upstream_check_status_module (static)
nginx: ngx_http_limit_speed_module (static)
nginx: ngx_http_tmd_module (static)
nginx: ngx_http_proxy_variables_module (static)
nginx: ngx_http_tbip_module (static)
nginx: ngx_http_write_filter_module (static)
nginx: ngx_http_header_filter_module (static)
nginx: ngx_http_chunked_filter_module (static)
nginx: ngx_http_v2_filter_module (static)
nginx: ngx_http_spdy_filter_module (static)
nginx: ngx_http_v3_filter_module (static)
nginx: ngx_http_range_header_filter_module (static)
nginx: ngx_http_gzip_filter_module (static)
nginx: ngx_http_postpone_filter_module (static)
nginx: ngx_http_ssi_filter_module (static)
nginx: ngx_http_charset_filter_module (static)
nginx: ngx_http_sub_filter_module (static)
nginx: ngx_http_addition_filter_module (static)
nginx: ngx_http_footer_filter_module (static)
nginx: ngx_http_ssl_status_module (static)
nginx: ngx_http_headers_filter_module (static)
nginx: ngx_http_upstream_session_sticky_module (static)
nginx: ngx_http_trim_filter_module (static)
nginx: ngx_http_beacon_filter_module (static)
nginx: ngx_http_echo_module (static)
nginx: ngx_http_trans_filter_module (static)
nginx: ngx_http_cookie_beacon_filter_module (static)
nginx: ngx_http_hat_filter_module (static)
nginx: ngx_http_hot_module (static)
nginx: ngx_http_video_slice_module (static)
nginx: ngx_http_video_module (static)
nginx: ngx_http_video_ts_module (static)
nginx: ngx_http_cdn_variables_module (static)
nginx: ngx_http_alibeacon_filter_module (static)
nginx: ngx_http_gunzip_filter_module (static)
nginx: ngx_http_slice_file_module (static)
nginx: ngx_http_content_verify_filter_module (static)
nginx: ngx_http_headers_more_filter_module (static)
nginx: ngx_http_tcprt_option_module (static)
nginx: ngx_http_brotli_filter_module (static)
nginx: ngx_http_inject_tail_module (static)
nginx: ngx_http_img_adapt_module (static)
nginx: ngx_http_tesla_copy_filter_module (static)
nginx: ngx_http_split_video_module (static)
nginx: ngx_http_parse_metadata_filter_module (static)
nginx: ngx_http_limit_rate_gop_filter_module (static)
nginx: ngx_http_drop_traffic_filter_module (static)
nginx: ngx_http_asyn_auth_module (static)
nginx: ngx_http_lua_module (static)
nginx: ngx_http_copy_filter_module (static)
nginx: ngx_http_range_body_filter_module (static)
nginx: ngx_http_not_modified_filter_module (static)
nginx: ngx_http_slice_filter_module (static)
nginx: ngx_mail_module (static)
nginx: ngx_mail_core_module (static)
nginx: ngx_mail_ssl_module (static)
nginx: ngx_mail_pop3_module (static)
nginx: ngx_mail_imap_module (static)
nginx: ngx_mail_smtp_module (static)
nginx: ngx_mail_auth_http_module (static)
nginx: ngx_mail_proxy_module (static)
nginx: ngx_mail_realip_module (static)
nginx: ngx_stream_module (static)
nginx: ngx_stream_core_module (static)
nginx: ngx_stream_log_module (static)
nginx: ngx_stream_proxy_module (static)
nginx: ngx_stream_upstream_module (static)
nginx: ngx_stream_write_filter_module (static)
nginx: ngx_stream_limit_conn_module (static)
nginx: ngx_stream_access_module (static)
nginx: ngx_stream_geo_module (static)
nginx: ngx_stream_map_module (static)
nginx: ngx_stream_split_clients_module (static)
nginx: ngx_stream_return_module (static)
nginx: ngx_stream_set_module (static)
nginx: ngx_stream_upstream_hash_module (static)
nginx: ngx_stream_upstream_least_conn_module (static)
nginx: ngx_stream_upstream_random_module (static)
nginx: ngx_ip_list_module (static)
nginx: ngx_http_upstream_proxy_protocol_module (static)

Description

Hi Nginx folks,

In the ngx_quic_rtt_sample function (ngx_event_quic_ack.c), nginx strictly follows the way defined in RFC9002 to compute smoothed_rtt and rtt_var. However, this may lead to an underestimation of rtt_var, as it uses the updated smoothed_rtt to calculate rtt_var. In some other QUIC implementations (e.g. MSQUIC), they update rtt_var before updating smoothed_rtt, which is also how the Linux Kernel TCP implementation updates rtt_var.

I would like to know if it is better to change the order of updating smoothed_rtt and rtt_var. The modification is as follows.


rttvar_sample = ngx_abs((ngx_msec_int_t) (qc->avg_rtt - adjusted_rtt));
qc->avg_rtt += (adjusted_rtt >> 3) - (qc->avg_rtt >> 3);
qc->rttvar += (rttvar_sample >> 2) - (qc->rttvar >> 2);


Best,
Furong

Change History (3)

comment:1 by Sergey Kandaurov, 9 months ago

Thanks for spotting this.

Indeed, looks like in RFC 9002 a wrong order there.
From 5.3. Estimating smoothed_rtt and rttvar (looks reversed):

On subsequent RTT samples, smoothed_rtt and rttvar evolve as follows:

<..>
smoothed_rtt = 7/8 * smoothed_rtt + 1/8 * adjusted_rtt
rttvar_sample = abs(smoothed_rtt - adjusted_rtt)
rttvar = 3/4 * rttvar + 1/4 * rttvar_sample

From A.7. On Receiving an Acknowledgment (looks good):

UpdateRtt(ack_delay):
<..>

rttvar = 3/4 * rttvar + 1/4 * abs(smoothed_rtt - adjusted_rtt)
smoothed_rtt = 7/8 * smoothed_rtt + 1/8 * adjusted_rtt

RFC 9002 provides no further explanation on the order.

They refer to RFC 6298, so this is the source of truth, which has the following order specified in the section 2:

   (2.3) When a subsequent RTT measurement R' is made, a host MUST set

            RTTVAR <- (1 - beta) * RTTVAR + beta * |SRTT - R'|
            SRTT <- (1 - alpha) * SRTT + alpha * R'

         The value of SRTT used in the update to RTTVAR is its value
         before updating SRTT itself using the second assignment.  That
         is, updating RTTVAR and SRTT MUST be computed in the above
         order.

The following patch to address this:

# HG changeset patch
# User Sergey Kandaurov <pluknet@nginx.com>
# Date 1686142271 -14400
#      Wed Jun 07 16:51:11 2023 +0400
# Node ID 5cdb69ba657c7ebe34e2ba3421ea806dc4701449
# Parent  be9b4d218cc7f6671482a65d781aded278bbc801
QUIC: fixed rttvar on subsequent RTT samples (ticket #2505).

RFC 9002, section 5.3 appears to have a reversed order in computing rttvar,
compared to Appendix A.7, such that it used an updated smoothed_rtt value.

RFC 9002 refers to RFC 6298 that has the following computation order:
: The value of SRTT used in the update to RTTVAR is its value
: before updating SRTT itself

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
@@ -207,9 +207,9 @@ ngx_quic_rtt_sample(ngx_connection_t *c,
             adjusted_rtt -= ack_delay;
         }
 
-        qc->avg_rtt += (adjusted_rtt >> 3) - (qc->avg_rtt >> 3);
         rttvar_sample = ngx_abs((ngx_msec_int_t) (qc->avg_rtt - adjusted_rtt));
         qc->rttvar += (rttvar_sample >> 2) - (qc->rttvar >> 2);
+        qc->avg_rtt += (adjusted_rtt >> 3) - (qc->avg_rtt >> 3);
     }
 
     ngx_log_debug4(NGX_LOG_DEBUG_EVENT, c->log, 0,

comment:2 by Sergey Kandaurov <pluknet@…>, 9 months ago

In 9122:a32905d6fc10/nginx:

QUIC: fixed rttvar on subsequent RTT samples (ticket #2505).

Previously, computing rttvar used an updated smoothed_rtt value as per
RFC 9002, section 5.3, which appears to be specified in a wrong order.
A technical errata ID 7539 is reported.

comment:3 by Sergey Kandaurov, 9 months ago

Resolution: fixed
Status: newclosed

Fixed, thanks.

Note: See TracTickets for help on using tickets.