[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