On Fri, 2014-10-03 at 16:10 +0300, Patrik Flykt wrote: > On Fri, 2014-10-03 at 15:48 +0300, Patrik Flykt wrote: > > On Fri, 2014-09-26 at 15:15 -0500, Dan Williams wrote: > > > /* RFC2132 section 4.1.1: > > > The client MUST include its hardware address in the ’chaddr’ > > > field, if > > > - necessary for delivery of DHCP reply messages. > > > + necessary for delivery of DHCP reply messages. Non-Ethernet > > > + interfaces will leave 'chaddr' empty and use the client > > > identifier > > > + instead (eg, RFC 4390 section 2.1). > > > */ > > > - memcpy(&packet->dhcp.chaddr, &client->client_id.mac_addr, > > > ETH_ALEN); > > > + if (client->mac_addr_len == ETH_ALEN) > > > + memcpy(&packet->dhcp.chaddr, &client->mac_addr, > > > ETH_ALEN); > > > > Sorry about the late review, but shouldn't this be more generic if > > written: > > if (client->mac_addr_len) > > memcpy(&packet->dhcp.chaddr, &client->mac_addr, > > client->mac_addr_len); > > > > With that, all cases are covered. Which, AFAIK, are none in addition to > > ethernet. An assert() should check the given MAC address length at some > > point in the code so that it cannot be > 16. > > And then I notice that infiniband has a MAC address length of 20. For > all practical purposes this works as expected. Except if we want to go > nitpicking when setting the MAC address and also require the address > type to be supplied? Probably not...
sd_dhcp_client_set_mac() does have an 'arp_type' parameter that's cached in the client struct, so that could be changed to: if (client->arp_type == ARPHRD_ETHER) if you'd like. Dan _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
