From 03401969d48dcc51ede605c79ec0f9e6bcc8e4c1 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 15 Apr 2024 11:57:07 +0900 Subject: [PATCH 1/2] sd-radv: allow to modify RA header elements without stopping sd-radv This also makes too large retransmit time handled gracefully, as even if a too large value is specified, usec_to_be32_msec() in radv_send_router() will handle it gracefully anyway. Note, networkd still refuses such invalid values. Hence, this does not change any effective behavior. --- src/libsystemd-network/sd-radv.c | 29 ++++---------------------- src/libsystemd-network/test-ndisc-ra.c | 2 +- src/systemd/sd-radv.h | 4 ++-- 3 files changed, 7 insertions(+), 28 deletions(-) diff --git a/src/libsystemd-network/sd-radv.c b/src/libsystemd-network/sd-radv.c index 053dd0653b..0709a55c13 100644 --- a/src/libsystemd-network/sd-radv.c +++ b/src/libsystemd-network/sd-radv.c @@ -537,23 +537,13 @@ int sd_radv_set_mtu(sd_radv *ra, uint32_t mtu) { int sd_radv_set_hop_limit(sd_radv *ra, uint8_t hop_limit) { assert_return(ra, -EINVAL); - if (ra->state != RADV_STATE_IDLE) - return -EBUSY; - ra->hop_limit = hop_limit; - return 0; } int sd_radv_set_retransmit(sd_radv *ra, uint64_t usec) { assert_return(ra, -EINVAL); - if (ra->state != RADV_STATE_IDLE) - return -EBUSY; - - if (usec > RADV_MAX_RETRANSMIT_USEC) - return -EINVAL; - ra->retransmit_usec = usec; return 0; } @@ -561,9 +551,6 @@ int sd_radv_set_retransmit(sd_radv *ra, uint64_t usec) { int sd_radv_set_router_lifetime(sd_radv *ra, uint64_t usec) { assert_return(ra, -EINVAL); - if (ra->state != RADV_STATE_IDLE) - return -EBUSY; - if (!router_lifetime_is_valid(usec)) return -EINVAL; @@ -577,25 +564,17 @@ int sd_radv_set_router_lifetime(sd_radv *ra, uint64_t usec) { return 0; } -int sd_radv_set_managed_information(sd_radv *ra, int managed) { +int sd_radv_set_managed_information(sd_radv *ra, int b) { assert_return(ra, -EINVAL); - if (ra->state != RADV_STATE_IDLE) - return -EBUSY; - - SET_FLAG(ra->flags, ND_RA_FLAG_MANAGED, managed); - + SET_FLAG(ra->flags, ND_RA_FLAG_MANAGED, b); return 0; } -int sd_radv_set_other_information(sd_radv *ra, int other) { +int sd_radv_set_other_information(sd_radv *ra, int b) { assert_return(ra, -EINVAL); - if (ra->state != RADV_STATE_IDLE) - return -EBUSY; - - SET_FLAG(ra->flags, ND_RA_FLAG_OTHER, other); - + SET_FLAG(ra->flags, ND_RA_FLAG_OTHER, b); return 0; } diff --git a/src/libsystemd-network/test-ndisc-ra.c b/src/libsystemd-network/test-ndisc-ra.c index 76f4d7ba3f..c63d9dfdab 100644 --- a/src/libsystemd-network/test-ndisc-ra.c +++ b/src/libsystemd-network/test-ndisc-ra.c @@ -208,7 +208,7 @@ TEST(radv) { ASSERT_RETURN_EXPECTED_SE(sd_radv_set_retransmit(NULL, 10 * USEC_PER_MSEC) < 0); assert_se(sd_radv_set_retransmit(ra, 10 * USEC_PER_MSEC) >= 0); assert_se(sd_radv_set_retransmit(ra, 0) >= 0); - assert_se(sd_radv_set_retransmit(ra, usec_add(UINT32_MAX * USEC_PER_MSEC, USEC_PER_MSEC)) < 0); + assert_se(sd_radv_set_retransmit(ra, USEC_INFINITY) >= 0); ASSERT_RETURN_EXPECTED_SE(sd_radv_set_rdnss(NULL, 0, NULL, 0) < 0); assert_se(sd_radv_set_rdnss(ra, 0, NULL, 0) >= 0); diff --git a/src/systemd/sd-radv.h b/src/systemd/sd-radv.h index 6d17dcc7f6..2538f2ab6f 100644 --- a/src/systemd/sd-radv.h +++ b/src/systemd/sd-radv.h @@ -58,8 +58,8 @@ int sd_radv_set_mtu(sd_radv *ra, uint32_t mtu); int sd_radv_set_hop_limit(sd_radv *ra, uint8_t hop_limit); int sd_radv_set_retransmit(sd_radv *ra, uint64_t usec); int sd_radv_set_router_lifetime(sd_radv *ra, uint64_t usec); -int sd_radv_set_managed_information(sd_radv *ra, int managed); -int sd_radv_set_other_information(sd_radv *ra, int other); +int sd_radv_set_managed_information(sd_radv *ra, int b); +int sd_radv_set_other_information(sd_radv *ra, int b); int sd_radv_set_preference(sd_radv *ra, unsigned preference); int sd_radv_add_prefix(sd_radv *ra, sd_radv_prefix *p); int sd_radv_add_route_prefix(sd_radv *ra, sd_radv_route_prefix *p); From ba5dc3afad64028a885859c7f972a5928b9ea30c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 15 Apr 2024 12:04:23 +0900 Subject: [PATCH 2/2] sd-radv: set router preference gracefully Rather than refusing to set non-zero preference, when lifetime is zero, let's handle that gracefully on send. --- src/libsystemd-network/radv-internal.h | 1 + src/libsystemd-network/sd-radv.c | 20 +++++--------------- src/libsystemd-network/test-ndisc-ra.c | 6 ------ src/systemd/sd-radv.h | 2 +- 4 files changed, 7 insertions(+), 22 deletions(-) diff --git a/src/libsystemd-network/radv-internal.h b/src/libsystemd-network/radv-internal.h index d2ec912d49..cf3fe9e226 100644 --- a/src/libsystemd-network/radv-internal.h +++ b/src/libsystemd-network/radv-internal.h @@ -106,6 +106,7 @@ struct sd_radv { struct ether_addr mac_addr; uint8_t hop_limit; uint8_t flags; + uint8_t preference; uint32_t mtu; usec_t retransmit_usec; usec_t lifetime_usec; /* timespan */ diff --git a/src/libsystemd-network/sd-radv.c b/src/libsystemd-network/sd-radv.c index 0709a55c13..e6c375f311 100644 --- a/src/libsystemd-network/sd-radv.c +++ b/src/libsystemd-network/sd-radv.c @@ -201,7 +201,9 @@ static int radv_send_router(sd_radv *ra, const struct in6_addr *dst) { /* The nd_ra_curhoplimit and nd_ra_flags_reserved fields cannot specified with nd_ra_router_lifetime * simultaneously in the structured initializer in the above. */ adv.nd_ra_curhoplimit = ra->hop_limit; - adv.nd_ra_flags_reserved = ra->flags; + /* RFC 4191, Section 2.2, + * "...If the Router Lifetime is zero, the preference value MUST be set to (00) by the sender..." */ + adv.nd_ra_flags_reserved = ra->flags | (ra->lifetime_usec > 0 ? (ra->preference << 3) : 0); iov[msg.msg_iovlen++] = IOVEC_MAKE(&adv, sizeof(adv)); /* MAC address is optional, either because the link does not use L2 addresses or load sharing is @@ -554,12 +556,6 @@ int sd_radv_set_router_lifetime(sd_radv *ra, uint64_t usec) { if (!router_lifetime_is_valid(usec)) return -EINVAL; - /* RFC 4191, Section 2.2, "...If the Router Lifetime is zero, the preference value MUST be set - * to (00) by the sender..." */ - if (usec == 0 && - (ra->flags & (0x3 << 3)) != (SD_NDISC_PREFERENCE_MEDIUM << 3)) - return -EINVAL; - ra->lifetime_usec = usec; return 0; } @@ -578,20 +574,14 @@ int sd_radv_set_other_information(sd_radv *ra, int b) { return 0; } -int sd_radv_set_preference(sd_radv *ra, unsigned preference) { +int sd_radv_set_preference(sd_radv *ra, uint8_t preference) { assert_return(ra, -EINVAL); assert_return(IN_SET(preference, SD_NDISC_PREFERENCE_LOW, SD_NDISC_PREFERENCE_MEDIUM, SD_NDISC_PREFERENCE_HIGH), -EINVAL); - /* RFC 4191, Section 2.2, "...If the Router Lifetime is zero, the preference value MUST be set - * to (00) by the sender..." */ - if (ra->lifetime_usec == 0 && preference != SD_NDISC_PREFERENCE_MEDIUM) - return -EINVAL; - - ra->flags = (ra->flags & ~(0x3 << 3)) | (preference << 3); - + ra->preference = preference; return 0; } diff --git a/src/libsystemd-network/test-ndisc-ra.c b/src/libsystemd-network/test-ndisc-ra.c index c63d9dfdab..78635ee657 100644 --- a/src/libsystemd-network/test-ndisc-ra.c +++ b/src/libsystemd-network/test-ndisc-ra.c @@ -191,12 +191,6 @@ TEST(radv) { assert_se(sd_radv_set_preference(ra, SD_NDISC_PREFERENCE_HIGH) >= 0); ASSERT_RETURN_EXPECTED_SE(sd_radv_set_preference(ra, ~0) < 0); - assert_se(sd_radv_set_preference(ra, SD_NDISC_PREFERENCE_HIGH) >= 0); - assert_se(sd_radv_set_router_lifetime(ra, 300 * USEC_PER_SEC) >= 0); - assert_se(sd_radv_set_router_lifetime(ra, 0) < 0); - assert_se(sd_radv_set_preference(ra, SD_NDISC_PREFERENCE_MEDIUM) >= 0); - assert_se(sd_radv_set_router_lifetime(ra, 0) >= 0); - ASSERT_RETURN_EXPECTED_SE(sd_radv_set_managed_information(NULL, true) < 0); assert_se(sd_radv_set_managed_information(ra, true) >= 0); assert_se(sd_radv_set_managed_information(ra, false) >= 0); diff --git a/src/systemd/sd-radv.h b/src/systemd/sd-radv.h index 2538f2ab6f..55b123973e 100644 --- a/src/systemd/sd-radv.h +++ b/src/systemd/sd-radv.h @@ -60,7 +60,7 @@ int sd_radv_set_retransmit(sd_radv *ra, uint64_t usec); int sd_radv_set_router_lifetime(sd_radv *ra, uint64_t usec); int sd_radv_set_managed_information(sd_radv *ra, int b); int sd_radv_set_other_information(sd_radv *ra, int b); -int sd_radv_set_preference(sd_radv *ra, unsigned preference); +int sd_radv_set_preference(sd_radv *ra, uint8_t preference); int sd_radv_add_prefix(sd_radv *ra, sd_radv_prefix *p); int sd_radv_add_route_prefix(sd_radv *ra, sd_radv_route_prefix *p); int sd_radv_add_pref64_prefix(sd_radv *ra, sd_radv_pref64_prefix *p);