[systemd-devel] [PATCH] sd-dhcp: implement IPv4 link-local support

[Umut Tezduyar Lindskog]

Hi,

> -----Original Message-----
> From: Lennart Poettering [mailto:lennart at poettering.net]
> Sent: den 28 februari 2014 00:56
> To: Umut Tezduyar Lindskog
> Cc: systemd-devel at lists.freedesktop.org; Umut Tezduyar Lindskog
> Subject: Re: [systemd-devel] [PATCH] sd-dhcp: implement IPv4 link-local
> support
> 
> On Thu, 27.02.14 21:54, Umut Tezduyar Lindskog (umut.tezduyar at axis.com)
> wrote:
> 
> > Implements IPv4LL with respect to RFC 3927
> > (http://tools.ietf.org/rfc/rfc3927.txt) and integrates it with
> > networkd. Majority of the IPv4LL state machine is taken from avahi
> > (http://avahi.org/) project's autoip.
> >
> > IPv4LL can be enabled by IPv4LL=yes under [Network] section of
> > .network file.
> >
> > IPv4LL works independent of DHCP but if DHCP lease is aquired, then LL
> > address will be dropped.
> 
> Hmm, how is this hooked up in detail? i.e. when is the IPv4LL state machine
> started? I think I'd like to see this started after a short while when no DHCP
> response is seen, and immediately stopped as soon as one is found later on.
> Or what are your ideas here? Tom?

I have discussed this with Tom and Patrick and we have initially agreed on starting ipv4ll as soon as possible and let the state machine probe/announce/defend an address but let the networkd make the LL assignment not before DHCP times out. I, on the other hand, think being able to access to a product as early as possible is very important. Example would be retrieving a burglary image before waiting for dhcp times out. LL state machine is ready for both approaches and changing networkd would be relatively easy. 

Assuming both IPv4LL and DHCP are enabled, both of them are started as soon as link is up. Then if LL address is received before a DHCP lease, then network stack will have the LL address. If later on we receive a DHCP lease, then LL state machine will be turned off and LL address will be removed from network stack. If DHCP lease is acquired before LL address probe/announce, then LL state machine is turned off. 

> 
> I assume the IPv4ll state machine is started again if the DHCP leas runs out
> and no DHCP wants to provide a new one?
That's correct.
> 
> How is this hooked up with the link beat? I mean, if we watch the link beat
> and it changes, and we already have an ipv4ll address we probably want to
> check for conflicts immediately?
If you stop LL state machine and start again, then you will go through the probe/announce/defend process anyways. Tell you the truth I didn't quite understand your question.
> 
> If an IPv4LL address has been acquired, and then a DHCP server becomes
> available, do we really want to drop the address entirely? At least for
> IPv6 there's this concept of "deprecated" addresses for this purpose. I am
> pretty sure that's actually available for IPv4 as well, so maybe we should keep
> the ipv4ll adderss around and just mark it "deprecated"?
> This would allow ongoing TCP conenctions to survive, but the kernel wouldn't
> use the address as source address anymore.

This is a great topic. Patrik has been repeating the importance of it and if we can implement this, it would be a great advantage. As you are saying, we really don't want to drop the LL address immediately in case clients are already connected and using it. We should keep the current connections and never let any more connections on LL. I have played around with routing table and couldn't find a nice solution. It is possible to add route rules for the IP address of current connections but I think that way is way messy. But I will look in to this "deprecated" concept you are referring. Maybe you can give more information.
Patch adds a TODO item "have smooth transition from LL to routable address, without disconnecting clients." just to overcome this problem in the future.

> 
> I think IPv4LL would be a really valuable addition to networkd. But in contrast
> to what we did with avahi-autoipd and how it was used by NetworkManager
> I'd really make it so good that we can enable it by default, so that it just
> works.

I am all for "it just works!" 

> 
> Some very superficial comments on the code (Tom and Patrik are probably
> better folks to review this).

They were nice enough to take a peek before I send it to the ML.

> 
> > +        ll->receive_message = sd_event_source_unref(ll->receive_message);
> > +        if (ll->fd)
> > +                close_nointr_nofail(ll->fd);
> 
> This looks wrong. You want to check >= 0 here, since 0 is actually a valid fd,
> and -1 is not...
OK.
> 
> > +                uint32_t a = 1;
> > +                int i;
> > +
> > +                for (i = 0; i < ETH_ALEN; i++)
> > +                        a += ll->mac_addr.ether_addr_octet[i]*i;
> > +                a = (a % 0xFE00) + 0x0100;
> > +                addr = htonl(IPV4LL_NETWORK | (uint32_t) a);
> 
> 
> Hmm, this hash function is probably not that great... We probably should just
> use siphash here, we kind try to standardize on that...

We are going to change the way we generate LL addresses and there is a TODO item for this too. We are going to have a predictable sequence. This way, if network crashes after assigning a LL address, our start off address will be the last one we have assigned. 

> 
> > +        if (random_sec) {
> > +                next_timeout += random_u32() % (random_sec *
> USEC_PER_SEC);
> > +        }
> 
> Strictly speaking, this is evil, since it doesn't result in even distribution. But
> then again, it doesn't really matter....
> 
> Btw, please avoid extranous {} for single-line blocks like in the case above.
> This isn't PHP after all!

I must have missed it. 

> 
> Thanks for doing this work!

My pleasure. Smile at Axis cameras at Munich airport next time you are there. Maybe your picture will be carried in an IPv4LL packet.

> 
> Lennart
> 
> --
> Lennart Poettering, Red Hat

Thanks for doing a review.
Umut