[systemd-devel] [PATCH] sd-dhcp-client: support non-Ethernet hardware addresses

[Dan Williams]

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