Re: [systemd-devel] [PATCH] sd-dhcp: implement IPv4 link-local support
Hi, -Original Message- From: Lennart Poettering [mailto:lenn...@poettering.net] Sent: den 28 februari 2014 00:56 To: Umut Tezduyar Lindskog Cc: systemd-devel@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.tezdu...@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
Re: [systemd-devel] [PATCH] sd-dhcp: implement IPv4 link-local support
Hi,
-Original Message-
From: Zbigniew Jędrzejewski-Szmek [mailto:zbys...@in.waw.pl]
Sent: den 28 februari 2014 04:08
To: Umut Tezduyar Lindskog
Cc: systemd-devel@lists.freedesktop.org; Umut Tezduyar Lindskog
Subject: Re: [systemd-devel] [PATCH] sd-dhcp: implement IPv4 link-local
support
On Thu, Feb 27, 2014 at 09:54:17PM +0100, Umut Tezduyar Lindskog 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.
Looks good.
Minor thoughts below:
+#define log_ipv4ll(ll, fmt, ...) log_meta(LOG_DEBUG, __FILE__,
+__LINE__, __func__, IPv4LL: fmt, ##__VA_ARGS__)
I imagine that a user might want to control the log level of various
subsystems independently. It's nice that this is already a macro:
at some point we might want to turn the level into something configurable
on its own.
+ipv4ll_set_state (ll, IPV4LL_STATE_INIT, 1);
Indentation.
+if (ll-address) {
+do {
+uint32_t r = random_u32() 0x;
+addr = htonl(IPV4LL_NETWORK | r);
+} while (addr == ll-address || !(
+((ntohl(addr) IPV4LL_NETMASK) == IPV4LL_NETWORK)
+((ntohl(addr) 0xFF00) != 0x)
+((ntohl(addr) 0xFF00) != 0xFF00)));
Maybe one of the negations can be inverted:
} while (addr == ll-address ||
(ntohl(addr) IPV4LL_NETMASK) != IPV4LL_NETWORK ||
(ntohl(addr) 0xFF00) == 0x ||
(ntohl(addr) 0xFF00) == 0xFF00);
This seems more readable without all those parantheses.
I agree.
+if (ll-state == IPV4LL_STATE_ANNOUNCING ||
+ll-state == IPV4LL_STATE_RUNNING) {
IN_SET(ll-state, IPV4LL_STATE_ANNOUNCING, IPV4LL_STATE_RUNNING)
+
+if (ipv4ll_arp_conflict(ll, in_packet)) {
+
+r = sd_event_get_now_monotonic(ll-event,
time_now);
+if (r 0)
+goto out;
+
+/* Defend address */
+if (time_now ll-defend_window) {
+ll-defend_window = time_now +
DEFEND_INTERVAL
* USEC_PER_SEC;
+
arp_packet_announcement(out_packet, ll-address,
ll-mac_addr);
+out_packet_ready = 1;
+} else
+conflicted = 1;
+}
+
+} else if (ll-state == IPV4LL_STATE_WAITING_PROBE ||
+ ll-state == IPV4LL_STATE_PROBING ||
+ ll-state ==
+ IPV4LL_STATE_WAITING_ANNOUNCE) {
IN_SET...
Sorry, didn't understand it. What is this?
+
+conflicted = ipv4ll_arp_probe_conflict(ll,
in_packet);
+}
+
+if (conflicted) {
+log_ipv4ll(ll, CONFLICT);
Could we log more information here... With this we'd just have IPv4LL:
CONFLICT
in the logs. It would be nice to at least see the details of the conflicting
address of something.
It is actually just a conflict. Meaning you will drop your previously ANNOUNCED
address. And after CONFLICT, you will see PROBE ANNOUNCE with new picked
address. But maybe it is best to write down the conflicting address in case
logs were rotated many many times. Either way for me.
+r = ipv4ll_client_notify(ll,
IPV4LL_EVENT_CONFLICT);
+ll-claimed_address = 0;
+
+/* Pick a new address */
+ll-address = ipv4ll_pick_address(ll);
+ll-conflict++;
+ll-defend_window = 0;
+ipv4ll_set_state(ll,
+ IPV4LL_STATE_WAITING_PROBE, 1);
+
+if (ll-conflict = MAX_CONFLICTS) {
+log_ipv4ll (ll, MAX_CONFLICTS);
Whitespace.
Thanks
+int sd_ipv4ll_start (sd_ipv4ll *ll) {
+int r;
+
+assert_return(ll, -EINVAL);
+assert_return(ll-event, -EINVAL);
+assert_return(ll-index 0, -EINVAL);
+assert_return(ll-state == IPV4LL_STATE_INIT, -EBUSY);
+
+r = arp_network_bind_raw_socket(ll-index, ll-link);
+
+if (r 0)
+goto error
Re: [systemd-devel] [PATCH] sd-dhcp: implement IPv4 link-local support
On Fri, Feb 28, 2014 at 9:05 AM, Umut Tezduyar Lindskog umut.tezdu...@axis.com wrote: 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. IPv6 addresses are 'deprecated' by setting prefferred_lft = 0. I imagine the same should work for IPv4, but I never actually tried. Cheers, Tom ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] sd-dhcp: implement IPv4 link-local support
On Fri, 28.02.14 09:17, Umut Tezduyar Lindskog (umut.tezdu...@axis.com) wrote:
+} else if (ll-state == IPV4LL_STATE_WAITING_PROBE ||
+ ll-state == IPV4LL_STATE_PROBING ||
+ ll-state ==
+ IPV4LL_STATE_WAITING_ANNOUNCE) {
IN_SET...
Sorry, didn't understand it. What is this?
IN_SET is a macro we have that makes comparison of a one variable
against a number of constants a bit nicer.
This:
if (ll-state == IPV4LL_STATE_WAITING_PROBE ||
ll-state == IPV4LL_STATE_PROBING ||
ll-state == IPV4LL_STATE_WAITING_ANNOUNCE) ...
Becomes this:
if (IN_SET(ll-state,
IPV4LL_STATE_WAITING_PROBE,
IPV4LL_STATE_PROBING,
IPV4LL_STATE_WAITING_ANNOUNCE) ...
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] sd-dhcp: implement IPv4 link-local support
On Fri, 28.02.14 09:05, Umut Tezduyar Lindskog (umut.tezdu...@axis.com) wrote: 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. OK. 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. Well, in embedded environments (unlike on mobile/desktop cases) you probably want to leave the network interface continiously configured as soon as you got an IP address, and not take it down as soon as the link beat vanishes, to provide stability for local apps. However, even in that case, each time the link sensing reports that a cable got connected when it wasn't before we should refresh the DHCP release and check with ARP whether the IPv4LL address is still without conflicts, simply as a safety measure. 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. Hmm, what do you mean by predicatable? I mean, if there's a conflict, and both sides notice that and decide to pick a new address but do that with the same PRNG sequence they will try the same address next and so on and so on. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] sd-dhcp: implement IPv4 link-local support
On Fri, Feb 28, 2014 at 2:24 PM, Lennart Poettering lenn...@poettering.net wrote: 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. Well, in embedded environments (unlike on mobile/desktop cases) you probably want to leave the network interface continiously configured as soon as you got an IP address, and not take it down as soon as the link beat vanishes, to provide stability for local apps. However, even in that case, each time the link sensing reports that a cable got connected when it wasn't before we should refresh the DHCP release and check with ARP whether the IPv4LL address is still without conflicts, simply as a safety measure. I guess we could use CriticalConnection for this? Though currently the policy there for DHCP is that we never ever drop the address, so not sure how that would work with an IPv4LL conflict. 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. Hmm, what do you mean by predicatable? I mean, if there's a conflict, and both sides notice that and decide to pick a new address but do that with the same PRNG sequence they will try the same address next and so on and so on. My suggestion was to seed the PRNG like we do for the persistent MAC addresses. The sequence of attempted IP's will be the same for a given NIC on a given machine, but different machines/NICs will have different sequences. -t ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] sd-dhcp: implement IPv4 link-local support
On Fri, 28.02.14 14:34, Tom Gundersen (t...@jklm.no) wrote: On Fri, Feb 28, 2014 at 2:24 PM, Lennart Poettering lenn...@poettering.net wrote: 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. Well, in embedded environments (unlike on mobile/desktop cases) you probably want to leave the network interface continiously configured as soon as you got an IP address, and not take it down as soon as the link beat vanishes, to provide stability for local apps. However, even in that case, each time the link sensing reports that a cable got connected when it wasn't before we should refresh the DHCP release and check with ARP whether the IPv4LL address is still without conflicts, simply as a safety measure. I guess we could use CriticalConnection for this? Though currently the policy there for DHCP is that we never ever drop the address, so not sure how that would work with an IPv4LL conflict. Maybe a new WatchLinkBeat= or so? If yes this would mean that the iface is unconfigured as soon as link beat is lost, if no this would mean that the iface is always left configured as soon as it got an address, but still refreshed if link beat changes comes back. 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. Hmm, what do you mean by predicatable? I mean, if there's a conflict, and both sides notice that and decide to pick a new address but do that with the same PRNG sequence they will try the same address next and so on and so on. My suggestion was to seed the PRNG like we do for the persistent MAC addresses. The sequence of attempted IP's will be the same for a given NIC on a given machine, but different machines/NICs will have different sequences. This only works if the PRNG state can be stored per interface. Can we do this with the glibc one? Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] sd-dhcp: implement IPv4 link-local support
On Fri, Feb 28, 2014 at 2:39 PM, Lennart Poettering lenn...@poettering.net wrote: On Fri, 28.02.14 14:34, Tom Gundersen (t...@jklm.no) wrote: On Fri, Feb 28, 2014 at 2:24 PM, Lennart Poettering lenn...@poettering.net wrote: 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. Well, in embedded environments (unlike on mobile/desktop cases) you probably want to leave the network interface continiously configured as soon as you got an IP address, and not take it down as soon as the link beat vanishes, to provide stability for local apps. However, even in that case, each time the link sensing reports that a cable got connected when it wasn't before we should refresh the DHCP release and check with ARP whether the IPv4LL address is still without conflicts, simply as a safety measure. I guess we could use CriticalConnection for this? Though currently the policy there for DHCP is that we never ever drop the address, so not sure how that would work with an IPv4LL conflict. Maybe a new WatchLinkBeat= or so? If yes this would mean that the iface is unconfigured as soon as link beat is lost, if no this would mean that the iface is always left configured as soon as it got an address, but still refreshed if link beat changes comes back. Yeah, I guess we might need this weaker version of CriticalConnection. 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. Hmm, what do you mean by predicatable? I mean, if there's a conflict, and both sides notice that and decide to pick a new address but do that with the same PRNG sequence they will try the same address next and so on and so on. My suggestion was to seed the PRNG like we do for the persistent MAC addresses. The sequence of attempted IP's will be the same for a given NIC on a given machine, but different machines/NICs will have different sequences. This only works if the PRNG state can be stored per interface. Can we do this with the glibc one? I guess rand_r() should allow this? -t ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] sd-dhcp: implement IPv4 link-local support
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. --- Makefile.am |7 +- TODO |5 + src/libsystemd-dhcp/ipv4ll-internal.h| 38 +++ src/libsystemd-dhcp/ipv4ll-network.c | 57 src/libsystemd-dhcp/ipv4ll-packet.c | 71 src/libsystemd-dhcp/sd-ipv4ll.c | 521 ++ src/libsystemd/sd-rtnl/rtnl-message.c| 14 + src/network/networkd-address.c |4 +- src/network/networkd-link.c | 312 -- src/network/networkd-network-gperf.gperf |1 + src/network/networkd-route.c | 85 + src/network/networkd.h |8 + src/systemd/sd-ipv4ll.h | 50 +++ src/systemd/sd-rtnl.h|1 + 14 files changed, 1143 insertions(+), 31 deletions(-) create mode 100644 src/libsystemd-dhcp/ipv4ll-internal.h create mode 100644 src/libsystemd-dhcp/ipv4ll-network.c create mode 100644 src/libsystemd-dhcp/ipv4ll-packet.c create mode 100644 src/libsystemd-dhcp/sd-ipv4ll.c create mode 100644 src/systemd/sd-ipv4ll.h diff --git a/Makefile.am b/Makefile.am index 25b48e5..ba69a17 100644 --- a/Makefile.am +++ b/Makefile.am @@ -2409,7 +2409,12 @@ libsystemd_dhcp_la_SOURCES = \ src/libsystemd-dhcp/dhcp-option.c \ src/libsystemd-dhcp/dhcp-packet.c \ src/libsystemd-dhcp/dhcp-internal.h \ - src/libsystemd-dhcp/dhcp-protocol.h + src/libsystemd-dhcp/dhcp-protocol.h \ + src/systemd/sd-ipv4ll.h \ + src/libsystemd-dhcp/sd-ipv4ll.c \ + src/libsystemd-dhcp/ipv4ll-network.c \ + src/libsystemd-dhcp/ipv4ll-packet.c \ + src/libsystemd-dhcp/ipv4ll-internal.h libsystemd_dhcp_la_LIBADD = \ libsystemd-internal.la \ diff --git a/TODO b/TODO index 6cac3e2..711d2ad 100644 --- a/TODO +++ b/TODO @@ -679,6 +679,11 @@ Features: - add support for more DHCPv4 options (and, longer term, other kinds of dynamic config) - add proper initrd support (in particular generate .network/.link files based on /proc/cmdline) - add reduced [Link] support to .network files + - add IPv4LL tests (inspire by DHCP) + - add IPv4LL to man pages. Explain coexistance between DHCP + - add Scope= parsing option for [Network] + - change LL address generation and make it predictable like get_mac() (link-config.c) + - have smooth transition from LL to routable address, without disconnecting clients. External: diff --git a/src/libsystemd-dhcp/ipv4ll-internal.h b/src/libsystemd-dhcp/ipv4ll-internal.h new file mode 100644 index 000..fe5ef8e --- /dev/null +++ b/src/libsystemd-dhcp/ipv4ll-internal.h @@ -0,0 +1,38 @@ +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/ + +#pragma once + +/*** + This file is part of systemd. + + Copyright (C) 2014 Axis Communications AB. All rights reserved. + + systemd is free software; you can redistribute it and/or modify it + under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation; either version 2.1 of the License, or + (at your option) any later version. + + systemd is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with systemd; If not, see http://www.gnu.org/licenses/. +***/ + +#include netinet/if_ether.h + +#include sparse-endian.h +#include socket-util.h + +int arp_network_bind_raw_socket(int index, union sockaddr_union *link); +int arp_network_send_raw_socket(int fd, const union sockaddr_union *link, +const struct ether_arp *arp); + +void arp_packet_init(struct ether_arp *arp); +void arp_packet_probe(struct ether_arp *arp, be32_t pa, const struct ether_addr *ha); +void arp_packet_announcement(struct ether_arp *arp, be32_t pa, const struct ether_addr *ha); +int arp_packet_verify_headers(struct ether_arp *arp); + +#define log_ipv4ll(ll, fmt, ...) log_meta(LOG_DEBUG, __FILE__, __LINE__, __func__, IPv4LL: fmt, ##__VA_ARGS__) diff --git a/src/libsystemd-dhcp/ipv4ll-network.c b/src/libsystemd-dhcp/ipv4ll-network.c new file mode 100644 index 000..b325445 --- /dev/null +++ b/src/libsystemd-dhcp/ipv4ll-network.c @@ -0,0 +1,57 @@ +/*** + This file is part of systemd. + + Copyright (C) 2014 Axis Communications AB. All rights reserved. + + systemd is free software; you can redistribute it and/or
Re: [systemd-devel] [PATCH] sd-dhcp: implement IPv4 link-local support
This is a lot of code, and this approach is largely obsoleted by link-local IPv6 addressing, which also has the benefits of being simpler, deterministic (at least with RFC 4862), and collision-proof. Both Apple [1] and Microsoft [2] prefer IPv6 link-local as the best practice. Is it really that important to support self-assigned IPv4? [1] https://developer.apple.com/library/ios/documentation/Cocoa/Conceptual/NetServices/Articles/about.html [2] https://blogs.technet.com/b/jlosey/archive/2011/02/02/why-you-should-leave-ipv6-alone.aspx ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] sd-dhcp: implement IPv4 link-local support
On Thu, 27.02.14 21:54, Umut Tezduyar Lindskog (umut.tezdu...@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 assume the IPv4ll state machine is started again if the DHCP leas runs
out and no DHCP wants to provide a new one?
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 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.
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.
Some very superficial comments on the code (Tom and Patrik are probably
better folks to review this).
+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...
+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...
+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!
Thanks for doing this work!
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] sd-dhcp: implement IPv4 link-local support
On Thu, 27.02.14 14:28, David Timothy Strauss (da...@davidstrauss.net) wrote: This is a lot of code, and this approach is largely obsoleted by link-local IPv6 addressing, which also has the benefits of being simpler, deterministic (at least with RFC 4862), and collision-proof. Both Apple [1] and Microsoft [2] prefer IPv6 link-local as the best practice. Is it really that important to support self-assigned IPv4? [1] https://developer.apple.com/library/ios/documentation/Cocoa/Conceptual/NetServices/Articles/about.html [2] https://blogs.technet.com/b/jlosey/archive/2011/02/02/why-you-should-leave-ipv6-alone.aspx Well, APIPA has been supported in OSes since a lot longer than IPv6 link-local addresses, for example in Windows 98... And I fear IPv4 is not going to die so soon... It might even be a useful thing for local containers since it might be faster than DHCPv4 is some cases, if people actually want IPv4 there. Maybe I am biased, because I wrote that IPv4ll implementation in Avahi a long time ago, but I am pretty sure this is something to support if we ever want to be a generic solution that speaks enough of these protocols to be comprehensively useful. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] sd-dhcp: implement IPv4 link-local support
On Thu, Feb 27, 2014 at 11:28 PM, David Timothy Strauss da...@davidstrauss.net wrote: This is a lot of code, and this approach is largely obsoleted by link-local IPv6 addressing, which also has the benefits of being simpler, deterministic (at least with RFC 4862), and collision-proof. Both Apple [1] and Microsoft [2] prefer IPv6 link-local as the best practice. Is it really that important to support self-assigned IPv4? While IPv6 link-local is nice, if the other devices on your local network only support ipv4 it won't do you much good. So I think this is something we definitely need to support out of the box. Moreover, the fact that people (Axis in this case) are still shipping products using this, probably means it will stay with us for quite some time. Cheers, Tom [1] https://developer.apple.com/library/ios/documentation/Cocoa/Conceptual/NetServices/Articles/about.html [2] https://blogs.technet.com/b/jlosey/archive/2011/02/02/why-you-should-leave-ipv6-alone.aspx ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] sd-dhcp: implement IPv4 link-local support
On Thu, Feb 27, 2014 at 09:54:17PM +0100, Umut Tezduyar Lindskog 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.
Looks good.
Minor thoughts below:
+#define log_ipv4ll(ll, fmt, ...) log_meta(LOG_DEBUG, __FILE__, __LINE__,
__func__, IPv4LL: fmt, ##__VA_ARGS__)
I imagine that a user might want to control the log level of various
subsystems independently. It's nice that this is already a macro:
at some point we might want to turn the level into something configurable
on its own.
+ipv4ll_set_state (ll, IPV4LL_STATE_INIT, 1);
Indentation.
+if (ll-address) {
+do {
+uint32_t r = random_u32() 0x;
+addr = htonl(IPV4LL_NETWORK | r);
+} while (addr == ll-address || !(
+((ntohl(addr) IPV4LL_NETMASK) == IPV4LL_NETWORK)
+((ntohl(addr) 0xFF00) != 0x)
+((ntohl(addr) 0xFF00) != 0xFF00)));
Maybe one of the negations can be inverted:
} while (addr == ll-address ||
(ntohl(addr) IPV4LL_NETMASK) != IPV4LL_NETWORK ||
(ntohl(addr) 0xFF00) == 0x ||
(ntohl(addr) 0xFF00) == 0xFF00);
This seems more readable without all those parantheses.
+if (ll-state == IPV4LL_STATE_ANNOUNCING ||
+ll-state == IPV4LL_STATE_RUNNING) {
IN_SET(ll-state, IPV4LL_STATE_ANNOUNCING, IPV4LL_STATE_RUNNING)
+
+if (ipv4ll_arp_conflict(ll, in_packet)) {
+
+r = sd_event_get_now_monotonic(ll-event,
time_now);
+if (r 0)
+goto out;
+
+/* Defend address */
+if (time_now ll-defend_window) {
+ll-defend_window = time_now +
DEFEND_INTERVAL * USEC_PER_SEC;
+arp_packet_announcement(out_packet,
ll-address, ll-mac_addr);
+out_packet_ready = 1;
+} else
+conflicted = 1;
+}
+
+} else if (ll-state == IPV4LL_STATE_WAITING_PROBE ||
+ ll-state == IPV4LL_STATE_PROBING ||
+ ll-state == IPV4LL_STATE_WAITING_ANNOUNCE) {
IN_SET...
+
+conflicted = ipv4ll_arp_probe_conflict(ll,
in_packet);
+}
+
+if (conflicted) {
+log_ipv4ll(ll, CONFLICT);
Could we log more information here... With this we'd just have IPv4LL:
CONFLICT
in the logs. It would be nice to at least see the details of the conflicting
address
of something.
+r = ipv4ll_client_notify(ll, IPV4LL_EVENT_CONFLICT);
+ll-claimed_address = 0;
+
+/* Pick a new address */
+ll-address = ipv4ll_pick_address(ll);
+ll-conflict++;
+ll-defend_window = 0;
+ipv4ll_set_state(ll, IPV4LL_STATE_WAITING_PROBE, 1);
+
+if (ll-conflict = MAX_CONFLICTS) {
+log_ipv4ll (ll, MAX_CONFLICTS);
Whitespace.
+int sd_ipv4ll_start (sd_ipv4ll *ll) {
+int r;
+
+assert_return(ll, -EINVAL);
+assert_return(ll-event, -EINVAL);
+assert_return(ll-index 0, -EINVAL);
+assert_return(ll-state == IPV4LL_STATE_INIT, -EBUSY);
+
+r = arp_network_bind_raw_socket(ll-index, ll-link);
+
+if (r 0)
+goto error;
+
+ll-fd = r;
+ll-conflict = 0;
+ll-defend_window = 0;
+ll-claimed_address = 0;
+
+if (ll-address == 0)
+ll-address = ipv4ll_pick_address(ll);
+
+ipv4ll_set_state (ll, IPV4LL_STATE_INIT, 1);
+
+r = sd_event_add_io(ll-event, ll-receive_message, ll-fd,
+EPOLLIN, ipv4ll_receive_message, ll);
+if (r 0)
+goto error;
+
+r = sd_event_source_set_priority(ll-receive_message,
ll-event_priority);
+if (r 0)
+goto error;
+
+r = sd_event_add_monotonic(ll-event, ll-timer,
now(CLOCK_MONOTONIC), 0,
+ ipv4ll_timer, ll);
+
+if (r 0)
+goto error;
