[systemd-devel] [PATCH] sd-dhcp-client: allow getting/setting the client ID
Tom Gundersen
teg at jklm.no
Tue Nov 18 09:51:05 PST 2014
Hi Dan,
Sorry for the delay. This patch looks good, minor nits inline.
Cheers,
Tom
On Tue, Nov 4, 2014 at 6:48 PM, Dan Williams <dcbw at redhat.com> wrote:
> The client identifier can be in many different formats, not just
> the one that systemd creates from the Ethernet MAC address. Non-
> ethernet interfaces have different client IDs formats too. Users
> may also have custom client IDs that they wish to use to preserve
> lease options delivered by servers configured with the existing
> client ID.
> ---
> src/libsystemd-network/sd-dhcp-client.c | 116 ++++++++++++++++++++++++++++----
> src/systemd/sd-dhcp-client.h | 4 ++
> 2 files changed, 108 insertions(+), 12 deletions(-)
>
> diff --git a/src/libsystemd-network/sd-dhcp-client.c b/src/libsystemd-network/sd-dhcp-client.c
> index 689163c..36f05ca 100644
> --- a/src/libsystemd-network/sd-dhcp-client.c
> +++ b/src/libsystemd-network/sd-dhcp-client.c
> @@ -38,6 +38,7 @@
> #include "dhcp-lease-internal.h"
> #include "sd-dhcp-client.h"
>
> +#define MAX_CLIENT_ID_LEN 32
Where did this value come from, could you add a comment (I didn't see
it in the RFC).
> #define MAX_MAC_ADDR_LEN INFINIBAND_ALEN
>
> struct sd_dhcp_client {
> @@ -56,13 +57,33 @@ struct sd_dhcp_client {
> size_t req_opts_allocated;
> size_t req_opts_size;
> be32_t last_addr;
> - struct {
> - uint8_t type;
> - struct ether_addr mac_addr;
> - } _packed_ client_id;
> uint8_t mac_addr[MAX_MAC_ADDR_LEN];
> size_t mac_addr_len;
> uint16_t arp_type;
> + union {
> + struct {
> + uint8_t type; /* 0: Generic (non-LL) (RFC 2132) */
> + uint8_t data[MAX_CLIENT_ID_LEN];
> + } _packed_ gen;
> + struct {
> + uint8_t type; /* 1: Ethernet Link-Layer (RFC 2132) */
> + uint8_t haddr[ETH_ALEN];
> + } _packed_ eth;
> + struct {
> + uint8_t type; /* 2 - 254: ARP/Link-Layer (RFC 2132) */
> + uint8_t haddr[0];
> + } _packed_ ll;
> + struct {
> + uint8_t type; /* 255: Node-specific (RFC 4361) */
> + uint8_t iaid[4];
> + uint8_t duid[MAX_CLIENT_ID_LEN - 4];
> + } _packed_ ns;
> + struct {
> + uint8_t type;
> + uint8_t data[MAX_CLIENT_ID_LEN];
> + } _packed_ raw;
> + } client_id;
> + size_t client_id_len;
> char *hostname;
> char *vendor_class_identifier;
> uint32_t mtu;
> @@ -201,8 +222,69 @@ int sd_dhcp_client_set_mac(sd_dhcp_client *client, const uint8_t *addr,
> client->mac_addr_len = addr_len;
> client->arp_type = arp_type;
>
> - memcpy(&client->client_id.mac_addr, addr, ETH_ALEN);
> - client->client_id.type = 0x01;
> + if (need_restart && client->state != DHCP_STATE_STOPPED)
> + sd_dhcp_client_start(client);
> +
> + return 0;
> +}
> +
> +int sd_dhcp_client_get_client_id(sd_dhcp_client *client, uint8_t *type,
> + const uint8_t **data, size_t *data_len) {
> +
> + assert_return(client, -EINVAL);
> + assert_return(type, -EINVAL);
> + assert_return(data, -EINVAL);
> + assert_return(data_len, -EINVAL);
> +
> + *type = 0;
> + *data = NULL;
> + *data_len = 0;
> + if (client->client_id_len) {
> + *type = client->client_id.raw.type;
> + *data = client->client_id.raw.data;
> + *data_len = client->client_id_len - 1; /* -1 for sizeof(type) */
Maybe make this (and other instances like it) self-documenting by doing
client->client_id_len - sizeof(client->client_id.raw.type)?
> + }
> +
> + return 0;
> +}
> +
> +int sd_dhcp_client_set_client_id(sd_dhcp_client *client, uint8_t type,
> + const uint8_t *data, size_t data_len) {
> + DHCP_CLIENT_DONT_DESTROY(client);
> + bool need_restart = false;
> +
> + assert_return(client, -EINVAL);
> + assert_return(data, -EINVAL);
> + assert_return(data_len > 0 && data_len <= MAX_CLIENT_ID_LEN, -EINVAL);
> +
> + switch (type) {
> + case ARPHRD_ETHER:
> + if (data_len != ETH_ALEN)
> + return -EINVAL;
> + break;
> + case ARPHRD_INFINIBAND:
> + if (data_len != INFINIBAND_ALEN)
> + return -EINVAL;
> + break;
> + default:
> + break;
> + }
> +
> + if (client->client_id_len == data_len + 1 &&
> + client->client_id.raw.type == type &&
> + memcmp(&client->client_id.raw.data, data, data_len) == 0)
> + return 0;
> +
> + if (!IN_SET(client->state, DHCP_STATE_INIT, DHCP_STATE_STOPPED)) {
> + log_dhcp_client(client, "Changing client ID on running DHCP "
> + "client, restarting");
> + need_restart = true;
> + client_stop(client, DHCP_EVENT_STOP);
> + }
> +
> + client->client_id.raw.type = type;
> + memcpy(&client->client_id.raw.data, data, data_len);
> + client->client_id_len = data_len + 1; /* +1 for sizeof(type) */
>
> if (need_restart && client->state != DHCP_STATE_STOPPED)
> sd_dhcp_client_start(client);
> @@ -369,14 +451,24 @@ static int client_message_init(sd_dhcp_client *client, DHCPPacket **ret,
> if (client->arp_type == ARPHRD_ETHER)
> memcpy(&packet->dhcp.chaddr, &client->mac_addr, ETH_ALEN);
>
> + /* If no client identifier exists, construct one from an ethernet
> + address if present */
> + if (client->client_id_len == 0 && client->arp_type == ARPHRD_ETHER) {
> + client->client_id.eth.type = ARPHRD_ETHER;
> + memcpy(&client->client_id.eth.haddr, &client->mac_addr, ETH_ALEN);
> + client->client_id_len = sizeof (client->client_id.eth);
> + }
> +
> /* Some DHCP servers will refuse to issue an DHCP lease if the Client
> Identifier option is not set */
> - r = dhcp_option_append(&packet->dhcp, optlen, &optoffset, 0,
> - DHCP_OPTION_CLIENT_IDENTIFIER,
> - sizeof(client->client_id), &client->client_id);
> - if (r < 0)
> - return r;
> -
> + if (client->client_id_len) {
> + r = dhcp_option_append(&packet->dhcp, optlen, &optoffset, 0,
> + DHCP_OPTION_CLIENT_IDENTIFIER,
> + client->client_id_len,
> + &client->client_id.raw);
> + if (r < 0)
> + return r;
> + }
>
> /* RFC2131 section 3.5:
> in its initial DHCPDISCOVER or DHCPREQUEST message, a
> diff --git a/src/systemd/sd-dhcp-client.h b/src/systemd/sd-dhcp-client.h
> index 7416f82..951662e 100644
> --- a/src/systemd/sd-dhcp-client.h
> +++ b/src/systemd/sd-dhcp-client.h
> @@ -51,6 +51,10 @@ int sd_dhcp_client_set_request_broadcast(sd_dhcp_client *client, int broadcast);
> int sd_dhcp_client_set_index(sd_dhcp_client *client, int interface_index);
> int sd_dhcp_client_set_mac(sd_dhcp_client *client, const uint8_t *addr,
> size_t addr_len, uint16_t arp_type);
> +int sd_dhcp_client_set_client_id(sd_dhcp_client *client, uint8_t type,
> + const uint8_t *data, size_t data_len);
> +int sd_dhcp_client_get_client_id(sd_dhcp_client *client, uint8_t *type,
> + const uint8_t **data, size_t *data_len);
> int sd_dhcp_client_set_mtu(sd_dhcp_client *client, uint32_t mtu);
> int sd_dhcp_client_set_hostname(sd_dhcp_client *client, const char *hostname);
> int sd_dhcp_client_set_vendor_class_identifier(sd_dhcp_client *client, const char *vci);
> --
> 1.9.3
>
>
> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
More information about the systemd-devel
mailing list