[systemd-devel] [PATCH] sd-dhcp-client: Sets broadcast flag to 1

[Camilo Aguilar]

I spent some time studying the standard ISC's implementation. And, unless
I'm misreading the code, for Linux the client seems to always set the
broadcast bit ON except when   SOCKET_CAN_RECEIVE_UNICAST_UNCONFIGURED is
defined in compilation time, which I don't see anybody doing. For the rest
of the ports they always sets it OFF.

Fedora has a modified version of ISC <https://github.com/greearb/dhcp-ct> where
in addition to the define, they also expose
<https://github.com/greearb/dhcp-ct/blob/master/dhcp-4.2.0/client/dhclient.c#L2768>
setting
the broadcast flag through the client's configuration file.

The best implementation I found is dhcpcd, used by Gentoo. If I understood
well the code, dhcpcd does not send ever the broadcast flag unless two
things happen:

1. The administrador runs the client with the --broadcast flag:
https://github.com/bldewolf/dhcpcd/commit/5aec6bbd13b0de0c8cc8111d82f827ef7d3eed35
2. The network interface says that it needs broadcast:
https://github.com/bldewolf/dhcpcd/commit/fb4477f477d1faa60759f2b94e29c12999e1db49

Any thoughts?

Best,
Camilo

On Wed, May 28, 2014 at 9:39 PM, Camilo Aguilar <camilo.aguilar at gmail.com>
wrote:

> You are right. Apologies. I was stuck on my own work due to this issue and
> was eager to get it solved too soon.
>
>  I'll spend some more time tonight digging about how other DHCP clients
> deal with detecting if the interface supports link level unicast or not.
> —
> Sent from Mailbox <https://www.dropbox.com/mailbox>
>
>
> On Wed, May 28, 2014 at 7:31 PM, Michael Marineau <
> michael.marineau at coreos.com> wrote:
>
>> On Wed, May 28, 2014 at 4:13 PM, Camilo Aguilar
>> <camilo.aguilar at gmail.com> wrote:
>> > Oh, never mind, there is no rush since we are already custom patching
>> in
>> > CoreOS for now.
>>
>> Hey, don't get ahead of yourself. I haven't merged your patch into
>> CoreOS just yet ;-)
>> I'm fine with the patch being a temporary fix as long as we can sort
>> out the final version soon.
>>
>> >
>> >
>> > On Wed, May 28, 2014 at 7:10 PM, Camilo Aguilar <
>> camilo.aguilar at gmail.com>
>> > wrote:
>> >>
>> >> It makes total sense. I can provide that patch in addition if you
>> like,
>> >> But, can we merge this change for now so we can fix
>> >> https://github.com/coreos/bugs/issues/12 and create a new issue to
>> make it
>> >> more robust?
>> >>
>> >>
>> >> On Wed, May 28, 2014 at 7:03 PM, Tom Gundersen <teg at jklm.no> wrote:
>> >>>
>> >>> On Wed, May 28, 2014 at 7:43 PM, Camilo Aguilar
>> >>> <camilo.aguilar at gmail.com> wrote:
>> >>> > In systems running on hypervisors this flag needs to be set ON, so
>> >>> > offers can reach
>> >>> > the virtual machines.
>> >>> >
>> >>> > For more information please refer to this thread in CoreOS:
>> >>> > https://github.com/coreos/bugs/issues/12
>> >>> >
>> >>> > Signed-off-by: Camilo Aguilar <camilo.aguilar at gmail.com>
>> >>> > ---
>> >>> > src/libsystemd-network/sd-dhcp-client.c | 9 +++++++++
>> >>> > 1 file changed, 9 insertions(+)
>> >>> >
>> >>> > diff --git a/src/libsystemd-network/sd-dhcp-client.c
>> >>> > b/src/libsystemd-network/sd-dhcp-client.c
>> >>> > index 0300a6b..8f54906 100644
>> >>> > --- a/src/libsystemd-network/sd-dhcp-client.c
>> >>> > +++ b/src/libsystemd-network/sd-dhcp-client.c
>> >>> > @@ -286,6 +286,15 @@ static int client_message_init(sd_dhcp_client
>> >>> > *client, DHCPPacket **ret,
>> >>> > refuse to issue an DHCP lease if 'secs' is set to zero */
>> >>> > packet->dhcp.secs = htobe16(client->secs);
>> >>> >
>> >>> > + /* RFC2132 section 4.1
>> >>> > + A client that cannot receive unicast IP datagrams until its
>> >>> > protocol
>> >>> > + software has been configured with an IP address SHOULD set
>> >>> > the
>> >>> > + BROADCAST bit in the 'flags' field to 1 in any DHCPDISCOVER
>> >>> > or
>> >>> > + DHCPREQUEST messages that client sends. The BROADCAST bit
>> >>> > will
>> >>> > + provide a hint to the DHCP server and BOOTP relay agent to
>> >>> > broadcast
>> >>> > + any messages to the client on the client's subnet. */
>> >>> > + packet->dhcp.flags = htobe16(0x8000);
>> >>>
>> >>> Hm, most clients can and should use unicast, so we should definitely
>> >>> not request broadcast unconditionally. If there really is a situation
>> >>> where we need broadcast, we should detect that and only request
>> >>> broadcast in this case.
>>
>> Do you have any ideas on how to detect the issue documented here with
>> VMware virtual machines? I haven't dug into this bug yet myself so I'm
>> not sure what other DHCP implementations are doing off the top of my
>> head.
>> https://github.com/coreos/bugs/issues/12
>>
>
>


-- 
*Camilo Aguilar*
Software Engineer
http://github.com/c4milo
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/systemd-devel/attachments/20140529/873557ee/attachment-0001.html>