platform: accept 0 as valid rto_min value

iproute2 and the kernel accept 0 as valid rto_min value:

  # ip route add 172.16.0.1 dev enp1s0 rto_min 0ms
  # ip route show
  172.16.0.1 dev enp1s0 scope link rto_min lock 0ms

Even if a value of 0ms would not be useful in practice, it is better
to exactly track what kernel reports, instead of assuming that when
the value is zero it is "not set".
This commit is contained in:
Beniamino Galvani 2025-04-03 15:39:37 +02:00
parent 14106431fb
commit 2b922a93a5
5 changed files with 52 additions and 30 deletions

View file

@ -1509,7 +1509,6 @@ nm_utils_ip_route_attribute_to_platform(int addr_family,
GET_ATTR(NM_IP_ROUTE_ATTRIBUTE_INITCWND, r->initcwnd, UINT32, uint32, 0);
GET_ATTR(NM_IP_ROUTE_ATTRIBUTE_INITRWND, r->initrwnd, UINT32, uint32, 0);
GET_ATTR(NM_IP_ROUTE_ATTRIBUTE_MTU, r->mtu, UINT32, uint32, 0);
GET_ATTR(NM_IP_ROUTE_ATTRIBUTE_RTO_MIN, r->rto_min, UINT32, uint32, 0);
GET_ATTR(NM_IP_ROUTE_ATTRIBUTE_QUICKACK, r->quickack, BOOLEAN, boolean, FALSE);
GET_ATTR(NM_IP_ROUTE_ATTRIBUTE_LOCK_WINDOW, r->lock_window, BOOLEAN, boolean, FALSE);
GET_ATTR(NM_IP_ROUTE_ATTRIBUTE_LOCK_CWND, r->lock_cwnd, BOOLEAN, boolean, FALSE);
@ -1518,6 +1517,18 @@ nm_utils_ip_route_attribute_to_platform(int addr_family,
GET_ATTR(NM_IP_ROUTE_ATTRIBUTE_LOCK_MTU, r->lock_mtu, BOOLEAN, boolean, FALSE);
GET_ATTR(NM_IP_ROUTE_ATTRIBUTE_LOCK_ADVMSS, r->lock_mss, BOOLEAN, boolean, FALSE);
{
GVariant *_variant = nm_ip_route_get_attribute(s_route, NM_IP_ROUTE_ATTRIBUTE_RTO_MIN);
if (_variant && g_variant_is_of_type(_variant, G_VARIANT_TYPE_UINT32)) {
r->rto_min = g_variant_get_uint32(_variant);
r->rto_min_set = TRUE;
} else {
r->rto_min = 0;
r->rto_min_set = FALSE;
}
}
if ((variant = nm_ip_route_get_attribute(s_route, NM_IP_ROUTE_ATTRIBUTE_SRC))
&& g_variant_is_of_type(variant, G_VARIANT_TYPE_STRING)) {
if (inet_pton(addr_family, g_variant_get_string(variant, NULL), &addr) == 1) {

View file

@ -640,22 +640,23 @@ test_ip4_route_options(gconstpointer test_data)
switch (TEST_IDX) {
case 1:
rts_add[rts_n++] = ((NMPlatformIP4Route) {
.ifindex = IFINDEX,
.rt_source = NM_IP_CONFIG_SOURCE_USER,
.network = nmtst_inet4_from_string("172.16.1.0"),
.plen = 24,
.metric = 20,
.tos = 0x28,
.window = 10000,
.cwnd = 16,
.initcwnd = 30,
.initrwnd = 50,
.mtu = 1350,
.lock_cwnd = TRUE,
.mss = 1300,
.quickack = TRUE,
.rto_min = 1000,
.n_nexthops = 1,
.ifindex = IFINDEX,
.rt_source = NM_IP_CONFIG_SOURCE_USER,
.network = nmtst_inet4_from_string("172.16.1.0"),
.plen = 24,
.metric = 20,
.tos = 0x28,
.window = 10000,
.cwnd = 16,
.initcwnd = 30,
.initrwnd = 50,
.mtu = 1350,
.lock_cwnd = TRUE,
.mss = 1300,
.quickack = TRUE,
.rto_min = 1000,
.rto_min_set = TRUE,
.n_nexthops = 1,
});
break;
case 2:

View file

@ -4037,14 +4037,15 @@ _new_from_nl_route(const struct nlmsghdr *nlh, gboolean id_only, ParseNlmsgIter
NMPlatformIP4RtNextHop *v4_nh_extra_nexthops = v4_nh_extra_nexthops_stack;
guint v4_nh_extra_alloc = G_N_ELEMENTS(v4_nh_extra_nexthops_stack);
guint32 mss;
guint32 window = 0;
guint32 cwnd = 0;
guint32 initcwnd = 0;
guint32 initrwnd = 0;
guint32 mtu = 0;
guint32 rto_min = 0;
guint32 lock = 0;
gboolean quickack = FALSE;
guint32 window = 0;
guint32 cwnd = 0;
guint32 initcwnd = 0;
guint32 initrwnd = 0;
guint32 mtu = 0;
guint32 rto_min = 0;
guint32 lock = 0;
gboolean quickack = FALSE;
gboolean rto_min_set = FALSE;
nm_assert((parse_nlmsg_iter->iter_more && parse_nlmsg_iter->ip6_route.next_multihop > 0)
|| (!parse_nlmsg_iter->iter_more && parse_nlmsg_iter->ip6_route.next_multihop == 0));
@ -4290,8 +4291,10 @@ rta_multipath_done:
initrwnd = nla_get_u32(mtb[RTAX_INITRWND]);
if (mtb[RTAX_MTU])
mtu = nla_get_u32(mtb[RTAX_MTU]);
if (mtb[RTAX_RTO_MIN])
rto_min = nla_get_u32(mtb[RTAX_RTO_MIN]);
if (mtb[RTAX_RTO_MIN]) {
rto_min = nla_get_u32(mtb[RTAX_RTO_MIN]);
rto_min_set = TRUE;
}
if (mtb[RTAX_QUICKACK])
quickack = !!nla_get_u32(mtb[RTAX_QUICKACK]);
}
@ -4362,6 +4365,7 @@ rta_multipath_done:
obj->ip_route.initcwnd = initcwnd;
obj->ip_route.initrwnd = initrwnd;
obj->ip_route.rto_min = rto_min;
obj->ip_route.rto_min_set = rto_min_set;
obj->ip_route.quickack = quickack;
obj->ip_route.mtu = mtu;
obj->ip_route.lock_window = NM_FLAGS_HAS(lock, 1 << RTAX_WINDOW);
@ -5803,7 +5807,7 @@ _nl_msg_new_route(uint16_t nlmsg_type, uint16_t nlmsg_flags, const NMPObject *ob
NLA_PUT_U32(msg, RTAX_INITRWND, obj->ip_route.initrwnd);
if (obj->ip_route.mtu)
NLA_PUT_U32(msg, RTAX_MTU, obj->ip_route.mtu);
if (obj->ip_route.rto_min)
if (obj->ip_route.rto_min_set)
NLA_PUT_U32(msg, RTAX_RTO_MIN, obj->ip_route.rto_min);
if (obj->ip_route.quickack)
NLA_PUT_U32(msg, RTAX_QUICKACK, obj->ip_route.quickack);

View file

@ -7315,8 +7315,9 @@ nm_platform_ip4_route_to_string_full(const NMPlatformIP4Route *route,
route->lock_initrwnd ? "lock " : "",
route->initrwnd)
: "",
route->rto_min ? nm_sprintf_buf(str_rto_min, " rto_min %" G_GUINT32_FORMAT, route->rto_min)
: "",
route->rto_min_set
? nm_sprintf_buf(str_rto_min, " rto_min %" G_GUINT32_FORMAT, route->rto_min)
: "",
route->quickack ? " quickack 1" : "",
route->mtu || route->lock_mtu ? nm_sprintf_buf(str_mtu,
" mtu %s%" G_GUINT32_FORMAT,

View file

@ -314,6 +314,7 @@ guint _nm_platform_signal_id_get(NMPlatformSignalIdType signal_type);
guint32 initrwnd; \
\
/* RTA_METRICS.RTAX_RTO_MIN (iproute2: rto_min) */ \
/* Valid only when 'rto_min_set' is true. */ \
guint32 rto_min; \
\
/* RTA_METRICS.RTAX_MTU (iproute2: mtu) */ \
@ -367,6 +368,10 @@ guint _nm_platform_signal_id_get(NMPlatformSignalIdType signal_type);
/* RTA_METRICS.RTAX_QUICKACK (iproute2: quickack) */ \
bool quickack : 1; \
\
/* RTA_METRICS.RTAX_RTO_MIN (iproute2: rto_min) */ \
/* If true, the 'rto_min' value is valid. */ \
bool rto_min_set : 1; \
\
/* if TRUE, the "metric" field is interpreted as an offset that is added to a default
* metric. For example, form a DHCP lease we don't know the actually used metric, because
* that is determined by upper layers (the configuration). However, we have a default