[systemd-devel] [RFC] networkd: smooth transition from ipv4ll to dhcp address
Tom Gundersen
teg at jklm.no
Sun Mar 30 12:08:25 PDT 2014
On Sun, Mar 30, 2014 at 8:51 PM, Umut Tezduyar Lindskog
<umut.tezduyar at axis.com> wrote:
> Currently when both ipv4ll and dhcp are enabled, ipv4ll
> address (if one has been claimed) is removed when dhcp
> address is aquired.
>
> This patch provides a smooth transition between ipv4ll
> and dhcp. If ipv4ll address was claimed before dhcp, address
> is marked as deprecated. Deprecated address is still a
> valid address and packets can be received on it but address
> cannot be selected as source address. If dhcp lease cannot
> be extended, then ipv4ll address is marked as valid again.
Sounds good. Code looks good too, just a few nitpicks below.
Cheers,
Tom
> - sd_rtnl_message_new_addr_update is duplicate of
> sd_rtnl_message_new_addr except using NLM_F_REPLACE flag.
> Should they be merged to 1 function with different flags?
I'm also not sure what's the best api here. Comments welcome. I'm
happy to leave it as is though, we can always improve on it later (as
this api will not be exported in the near future).
> - sd_rtnl_message_append_cache_info will be modified to fit
> in Tom's rtnl rework.
>
> - Man page will be updated accordingly.
> ---
> TODO | 1 -
> src/libsystemd-network/sd-ipv4ll.c | 6 +++
> src/libsystemd/sd-rtnl/rtnl-message.c | 68 +++++++++++++++++++++++
> src/network/networkd-address.c | 96 +++++++++++++++++++++++++++++++--
> src/network/networkd-link.c | 86 +++++++++++++++++++++++++++--
> src/network/networkd.h | 4 ++
> src/systemd/sd-ipv4ll.h | 12 +++--
> src/systemd/sd-rtnl.h | 3 ++
> 8 files changed, 261 insertions(+), 15 deletions(-)
>
> diff --git a/TODO b/TODO
> index 50d67f7..2f0e764 100644
> --- a/TODO
> +++ b/TODO
> @@ -661,7 +661,6 @@ Features:
> - add IPv4LL tests (inspire by DHCP)
> - add Scope= parsing option for [Network]
> - change LL address generation and make it predictable like get_mac() (link-config.c)
> - - have smooth transition from LL to routable address, without disconnecting clients.
I suppose the preceding item can also be removed now :)
> * sd-network:
> - make sure ipv4ll and dhcp clients can handle changing mac addresses while running
> diff --git a/src/libsystemd-network/sd-ipv4ll.c b/src/libsystemd-network/sd-ipv4ll.c
> index 155c315..f75b3af 100644
> --- a/src/libsystemd-network/sd-ipv4ll.c
> +++ b/src/libsystemd-network/sd-ipv4ll.c
> @@ -465,6 +465,12 @@ error:
> return r;
> }
>
> +bool sd_ipv4ll_is_running(sd_ipv4ll *ll) {
> + assert_return(ll, -EINVAL);
> +
> + return (ll->state == IPV4LL_STATE_INIT ? false : true);
return ll->state != IPV4LL_STATE_INIT; ?
> +}
> +
> int sd_ipv4ll_start (sd_ipv4ll *ll) {
> int r;
>
> diff --git a/src/libsystemd/sd-rtnl/rtnl-message.c b/src/libsystemd/sd-rtnl/rtnl-message.c
> index e243c7b..f1250c9 100644
> --- a/src/libsystemd/sd-rtnl/rtnl-message.c
> +++ b/src/libsystemd/sd-rtnl/rtnl-message.c
> @@ -234,6 +234,40 @@ int sd_rtnl_message_addr_set_scope(sd_rtnl_message *m, unsigned char scope) {
> return 0;
> }
>
> +int sd_rtnl_message_new_addr_update(sd_rtnl *rtnl, sd_rtnl_message **ret,
> + uint16_t nlmsg_type, int index,
> + unsigned char family) {
> + struct ifaddrmsg *ifa;
> + int r;
> +
> + assert_return(rtnl_message_type_is_addr(nlmsg_type), -EINVAL);
> + assert_return(index > 0, -EINVAL);
> + assert_return(family == AF_INET || family == AF_INET6, -EINVAL);
> + assert_return(ret, -EINVAL);
> +
> + r = message_new(rtnl, ret, NLMSG_SPACE(sizeof(struct ifaddrmsg)));
> + if (r < 0)
> + return r;
> +
> + (*ret)->hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct ifaddrmsg));
> + (*ret)->hdr->nlmsg_type = nlmsg_type;
> + if (nlmsg_type == RTM_NEWADDR)
> + (*ret)->hdr->nlmsg_flags |= NLM_F_REPLACE;
> +
> + ifa = NLMSG_DATA((*ret)->hdr);
> +
> + ifa->ifa_index = index;
> + ifa->ifa_family = family;
> + if (family == AF_INET)
> + ifa->ifa_prefixlen = 32;
> + else if (family == AF_INET6)
> + ifa->ifa_prefixlen = 128;
> +
> + UPDATE_RTA(*ret, IFA_RTA(ifa));
> +
> + return 0;
> +}
Rather than repeat the whole function, I'd just make it a wrapper
around sd_rtnl_message_new_addr(), and then set
> + (*ret)->hdr->nlmsg_flags |= NLM_F_REPLACE;
I guess you should return -EINVAL if the type is anything other than
RTM_NEWADDR? Or simply drop the type argument altogether and simply
pass RTM_NEWADDR to sd_rtnl_message_new_addr()?
> int sd_rtnl_message_new_addr(sd_rtnl *rtnl, sd_rtnl_message **ret,
> uint16_t nlmsg_type, int index,
> unsigned char family) {
> @@ -729,6 +763,40 @@ int sd_rtnl_message_append_ether_addr(sd_rtnl_message *m, unsigned short type, c
> return 0;
> }
>
> +int sd_rtnl_message_append_cache_info(sd_rtnl_message *m, unsigned short type, const struct ifa_cacheinfo *info) {
> + uint16_t rtm_type;
> + int r;
> +
> + assert_return(m, -EINVAL);
> + assert_return(!m->sealed, -EPERM);
> + assert_return(info, -EINVAL);
> +
> + r = sd_rtnl_message_get_type(m, &rtm_type);
> + if (r < 0)
> + return r;
> +
> + switch (rtm_type) {
> + case RTM_NEWADDR:
> + case RTM_GETADDR:
> + case RTM_DELADDR:
> + switch (type) {
> + case IFA_CACHEINFO:
> + break;
> + default:
> + return -ENOTSUP;
> + }
> + break;
> + default:
> + return -ENOTSUP;
> + }
> +
> + r = add_rtattr(m, type, info, sizeof(struct ifa_cacheinfo));
> + if (r < 0)
> + return r;
> +
> + return 0;
> +}
>
> int sd_rtnl_message_open_container(sd_rtnl_message *m, unsigned short type) {
> uint16_t rtm_type;
>
> diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c
> index 414b3bc..8663136 100644
> --- a/src/network/networkd-address.c
> +++ b/src/network/networkd-address.c
> @@ -28,6 +28,15 @@
> #include "conf-parser.h"
> #include "net-util.h"
>
> +static void address_init(Address *address) {
> + assert(address);
> +
> + address->family = AF_UNSPEC;
> + address->scope = RT_SCOPE_UNIVERSE;
> + address->cinfo.ifa_prefered = CACHE_INFO_INFINITY_LIFE_TIME;
> + address->cinfo.ifa_valid = CACHE_INFO_INFINITY_LIFE_TIME;
> +}
> +
> int address_new_static(Network *network, unsigned section, Address **ret) {
> _cleanup_address_free_ Address *address = NULL;
>
> @@ -46,8 +55,7 @@ int address_new_static(Network *network, unsigned section, Address **ret) {
> if (!address)
> return -ENOMEM;
>
> - address->family = AF_UNSPEC;
> - address->scope = RT_SCOPE_UNIVERSE;
> + address_init(address);
>
> address->network = network;
>
> @@ -71,8 +79,7 @@ int address_new_dynamic(Address **ret) {
> if (!address)
> return -ENOMEM;
>
> - address->family = AF_UNSPEC;
> - address->scope = RT_SCOPE_UNIVERSE;
> + address_init(address);
>
> *ret = address;
> address = NULL;
> @@ -140,6 +147,87 @@ int address_drop(Address *address, Link *link,
> return 0;
> }
>
> +int address_update(Address *address, Link *link,
> + sd_rtnl_message_handler_t callback) {
> + _cleanup_rtnl_message_unref_ sd_rtnl_message *req = NULL;
> + int r;
> +
> + assert(address);
> + assert(address->family == AF_INET || address->family == AF_INET6);
> + assert(link->ifindex > 0);
> + assert(link->manager);
> + assert(link->manager->rtnl);
> +
> + r = sd_rtnl_message_new_addr_update(link->manager->rtnl, &req, RTM_NEWADDR,
> + link->ifindex, address->family);
> + if (r < 0) {
> + log_error("Could not allocate RTM_NEWADDR message: %s",
> + strerror(-r));
> + return r;
> + }
> +
> + r = sd_rtnl_message_addr_set_prefixlen(req, address->prefixlen);
> + if (r < 0) {
> + log_error("Could not set prefixlen: %s", strerror(-r));
> + return r;
> + }
> +
> + r = sd_rtnl_message_addr_set_flags(req, IFA_F_PERMANENT);
> + if (r < 0) {
> + log_error("Could not set flags: %s", strerror(-r));
> + return r;
> + }
> +
> + r = sd_rtnl_message_addr_set_scope(req, address->scope);
> + if (r < 0) {
> + log_error("Could not set scope: %s", strerror(-r));
> + return r;
> + }
> +
> + if (address->family == AF_INET)
> + r = sd_rtnl_message_append_in_addr(req, IFA_LOCAL, &address->in_addr.in);
> + else if (address->family == AF_INET6)
> + r = sd_rtnl_message_append_in6_addr(req, IFA_LOCAL, &address->in_addr.in6);
> + if (r < 0) {
> + log_error("Could not append IFA_LOCAL attribute: %s",
> + strerror(-r));
> + return r;
> + }
> +
> + if (address->family == AF_INET) {
> + r = sd_rtnl_message_append_in_addr(req, IFA_BROADCAST, &address->broadcast);
> + if (r < 0) {
> + log_error("Could not append IFA_BROADCAST attribute: %s",
> + strerror(-r));
> + return r;
> + }
> + }
> +
> + if (address->label) {
> + r = sd_rtnl_message_append_string(req, IFA_LABEL, address->label);
> + if (r < 0) {
> + log_error("Could not append IFA_LABEL attribute: %s",
> + strerror(-r));
> + return r;
> + }
> + }
> +
> + r = sd_rtnl_message_append_cache_info(req, IFA_CACHEINFO, &address->cinfo);
> + if (r < 0) {
> + log_error("Could not append IFA_CACHEINFO attribute: %s",
> + strerror(-r));
> + return r;
> + }
> +
> + r = sd_rtnl_call_async(link->manager->rtnl, req, callback, link, 0, NULL);
> + if (r < 0) {
> + log_error("Could not send rtnetlink message: %s", strerror(-r));
> + return r;
> + }
> +
> + return 0;
> +}
> +
> int address_configure(Address *address, Link *link,
> sd_rtnl_message_handler_t callback) {
> _cleanup_rtnl_message_unref_ sd_rtnl_message *req = NULL;
> diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
> index 3131671..0f0d78a 100644
> --- a/src/network/networkd-link.c
> +++ b/src/network/networkd-link.c
> @@ -30,6 +30,9 @@
>
> #include "dhcp-lease-internal.h"
>
> +static int ipv4ll_address_update(Link *link, bool deprecate);
> +static bool ipv4ll_is_bound(sd_ipv4ll *ll);
> +
> int link_new(Manager *manager, struct udev_device *device, Link **ret) {
> _cleanup_link_free_ Link *link = NULL;
> const char *ifname;
> @@ -431,6 +434,28 @@ static int link_enter_set_addresses(Link *link) {
> return 0;
> }
>
> +static int address_update_handler(sd_rtnl *rtnl, sd_rtnl_message *m, void *userdata) {
> + Link *link = userdata;
> + int r;
> +
> + assert(m);
> + assert(link);
> + assert(link->ifname);
> +
> + if (link->state == LINK_STATE_FAILED)
> + return 1;
> +
> + r = sd_rtnl_message_get_errno(m);
> + if (r < 0 && r != -ENOENT)
> + log_struct_link(LOG_WARNING, link,
> + "MESSAGE=%s: could not update address: %s",
> + link->ifname, strerror(-r),
> + "ERRNO=%d", -r,
> + NULL);
> +
> + return 0;
> +}
> +
> static int address_drop_handler(sd_rtnl *rtnl, sd_rtnl_message *m, void *userdata) {
> Link *link = userdata;
> int r;
> @@ -706,7 +731,7 @@ static int dhcp_lease_acquired(sd_dhcp_client *client, Link *link) {
>
> static void dhcp_handler(sd_dhcp_client *client, int event, void *userdata) {
> Link *link = userdata;
> - int r;
> + int r = 0;
>
> assert(link);
> assert(link->network);
> @@ -745,7 +770,10 @@ static void dhcp_handler(sd_dhcp_client *client, int event, void *userdata) {
> }
>
> if (event == DHCP_EVENT_EXPIRED && link->network->ipv4ll) {
> - r = sd_ipv4ll_start (link->ipv4ll);
> + if (!sd_ipv4ll_is_running(link->ipv4ll))
> + r = sd_ipv4ll_start(link->ipv4ll);
> + else if (ipv4ll_is_bound(link->ipv4ll))
> + r = ipv4ll_address_update(link, false);
> if (r < 0) {
> link_enter_failed(link);
> return;
> @@ -760,7 +788,10 @@ static void dhcp_handler(sd_dhcp_client *client, int event, void *userdata) {
> return;
> }
> if (link->ipv4ll) {
> - r = sd_ipv4ll_stop(link->ipv4ll);
> + if (ipv4ll_is_bound(link->ipv4ll))
> + r = ipv4ll_address_update(link, true);
> + else
> + r = sd_ipv4ll_stop(link->ipv4ll);
> if (r < 0) {
> link_enter_failed(link);
> return;
> @@ -778,11 +809,56 @@ static void dhcp_handler(sd_dhcp_client *client, int event, void *userdata) {
> return;
> }
>
> -static int ipv4ll_address_lost(sd_ipv4ll *ll, Link *link) {
> +static bool ipv4ll_is_bound(sd_ipv4ll *ll) {
> int r;
> struct in_addr addr;
>
> assert(ll);
> +
> + r = sd_ipv4ll_get_address(ll, &addr);
> + if (r < 0)
> + return false;
> + return true;
> +}
> +
> +static int ipv4ll_address_update(Link *link, bool deprecate) {
> + int r;
> + struct in_addr addr;
> +
> + assert(link);
> +
> + r = sd_ipv4ll_get_address(link->ipv4ll, &addr);
> + if (r >= 0) {
> + _cleanup_address_free_ Address *address = NULL;
> +
> + log_debug_link(link, "IPv4 link-local %s %u.%u.%u.%u",
> + deprecate ? "deprecate" : "approve",
> + ADDRESS_FMT_VAL(addr));
> +
> + r = address_new_dynamic(&address);
> + if (r < 0) {
> + log_error_link(link, "Could not allocate address: %s", strerror(-r));
> + return r;
> + }
> +
> + address->family = AF_INET;
> + address->in_addr.in = addr;
> + address->prefixlen = 16;
> + address->scope = RT_SCOPE_LINK;
> + address->cinfo.ifa_prefered = deprecate ? 0 : CACHE_INFO_INFINITY_LIFE_TIME;
> + address->broadcast.s_addr = address->in_addr.in.s_addr | htonl(0xfffffffflu >> address->prefixlen);
> +
> + address_update(address, link, &address_update_handler);
> + }
> +
> + return 0;
> +
> +}
> +
> +static int ipv4ll_address_lost(Link *link) {
> + int r;
> + struct in_addr addr;
> +
> assert(link);
>
> r = sd_ipv4ll_get_address(link->ipv4ll, &addr);
> @@ -856,7 +932,7 @@ static void ipv4ll_handler(sd_ipv4ll *ll, int event, void *userdata){
> switch(event) {
> case IPV4LL_EVENT_STOP:
> case IPV4LL_EVENT_CONFLICT:
> - r = ipv4ll_address_lost(ll, link);
> + r = ipv4ll_address_lost(link);
> if (r < 0) {
> link_enter_failed(link);
> return;
> diff --git a/src/network/networkd.h b/src/network/networkd.h
> index 239ef1c..fc097da 100644
> --- a/src/network/networkd.h
> +++ b/src/network/networkd.h
> @@ -36,6 +36,8 @@
> #include "set.h"
> #include "condition-util.h"
>
> +#define CACHE_INFO_INFINITY_LIFE_TIME 0xFFFFFFFFU
> +
> typedef struct NetDev NetDev;
> typedef struct Network Network;
> typedef struct Link Link;
> @@ -150,6 +152,7 @@ struct Address {
> char *label;
>
> struct in_addr broadcast;
> + struct ifa_cacheinfo cinfo;
>
> union {
> struct in_addr in;
> @@ -335,6 +338,7 @@ int address_new_static(Network *network, unsigned section, Address **ret);
> int address_new_dynamic(Address **ret);
> void address_free(Address *address);
> int address_configure(Address *address, Link *link, sd_rtnl_message_handler_t callback);
> +int address_update(Address *address, Link *link, sd_rtnl_message_handler_t callback);
> int address_drop(Address *address, Link *link, sd_rtnl_message_handler_t callback);
>
> DEFINE_TRIVIAL_CLEANUP_FUNC(Address*, address_free);
> diff --git a/src/systemd/sd-ipv4ll.h b/src/systemd/sd-ipv4ll.h
> index 2397d43..99f8ccc 100644
> --- a/src/systemd/sd-ipv4ll.h
> +++ b/src/systemd/sd-ipv4ll.h
> @@ -22,6 +22,7 @@
> along with systemd; If not, see <http://www.gnu.org/licenses/>.
> ***/
>
> +#include <stdbool.h>
> #include <netinet/in.h>
> #include <net/ethernet.h>
>
> @@ -42,10 +43,11 @@ int sd_ipv4ll_get_address(sd_ipv4ll *ll, struct in_addr *address);
> int sd_ipv4ll_set_callback(sd_ipv4ll *ll, sd_ipv4ll_cb_t cb, void *userdata);
> int sd_ipv4ll_set_mac(sd_ipv4ll *ll, const struct ether_addr *addr);
> int sd_ipv4ll_set_index(sd_ipv4ll *ll, int interface_index);
> -int sd_ipv4ll_set_address_seed (sd_ipv4ll *ll, uint64_t entropy);
> -int sd_ipv4ll_start (sd_ipv4ll *ll);
> -int sd_ipv4ll_stop (sd_ipv4ll *ll);
> -void sd_ipv4ll_free (sd_ipv4ll *ll);
> -int sd_ipv4ll_new (sd_ipv4ll **ret);
> +int sd_ipv4ll_set_address_seed(sd_ipv4ll *ll, uint64_t entropy);
> +bool sd_ipv4ll_is_running(sd_ipv4ll *ll);
> +int sd_ipv4ll_start(sd_ipv4ll *ll);
> +int sd_ipv4ll_stop(sd_ipv4ll *ll);
> +void sd_ipv4ll_free(sd_ipv4ll *ll);
> +int sd_ipv4ll_new(sd_ipv4ll **ret);
>
> #endif
> diff --git a/src/systemd/sd-rtnl.h b/src/systemd/sd-rtnl.h
> index 0a24873..0a26e17 100644
> --- a/src/systemd/sd-rtnl.h
> +++ b/src/systemd/sd-rtnl.h
> @@ -70,6 +70,8 @@ int sd_rtnl_detach_event(sd_rtnl *nl);
> int sd_rtnl_message_new_link(sd_rtnl *rtnl, sd_rtnl_message **ret, uint16_t msg_type, int index);
> int sd_rtnl_message_new_addr(sd_rtnl *rtnl, sd_rtnl_message **ret, uint16_t msg_type, int index,
> unsigned char family);
> +int sd_rtnl_message_new_addr_update(sd_rtnl *rtnl, sd_rtnl_message **ret, uint16_t nlmsg_type,
> + int index, unsigned char family);
> int sd_rtnl_message_new_route(sd_rtnl *rtnl, sd_rtnl_message **ret, uint16_t nlmsg_type,
> unsigned char rtm_family);
>
> @@ -99,6 +101,7 @@ int sd_rtnl_message_append_u32(sd_rtnl_message *m, unsigned short type, uint32_t
> int sd_rtnl_message_append_in_addr(sd_rtnl_message *m, unsigned short type, const struct in_addr *data);
> int sd_rtnl_message_append_in6_addr(sd_rtnl_message *m, unsigned short type, const struct in6_addr *data);
> int sd_rtnl_message_append_ether_addr(sd_rtnl_message *m, unsigned short type, const struct ether_addr *data);
> +int sd_rtnl_message_append_cache_info(sd_rtnl_message *m, unsigned short type, const struct ifa_cacheinfo *info);
Please also add a _read function and a test or two to test-rtnl, e.g.,
to check that you get back the same that you appended.
> int sd_rtnl_message_open_container(sd_rtnl_message *m, unsigned short type);
> int sd_rtnl_message_close_container(sd_rtnl_message *m);
More information about the systemd-devel
mailing list