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

[Camilo Aguilar]

gah, the link for 2 is
https://github.com/bldewolf/dhcpcd/commit/3bc21a1a09852407df89a363c67d7575efa081c8
instead


On Thu, May 29, 2014 at 4:41 AM, Camilo Aguilar <camilo.aguilar at gmail.com>
wrote:

> 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
>
>
>


-- 
*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/3dfab29d/attachment.html>