Re: [systemd-devel] [PATCH] sd-dhcp-client: allow getting/setting the client ID

2014-11-18 Thread Dan Williams 
On Tue, 2014-11-04 at 11:48 -0600, Dan Williams 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.

Anyone had a chance to review this patch?

 ---
  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
  #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) */
 +}
 +
 +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;
 +

Re: [systemd-devel] [PATCH] sd-dhcp-client: allow getting/setting the client ID

2014-11-18 Thread Tom Gundersen 
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 d...@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)) {
 +

[systemd-devel] [PATCH] sd-dhcp-client: allow getting/setting the client ID

2014-11-04 Thread Dan Williams 
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
 #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) 
*/
+}
+
+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);