From 786cd854d7503aef0ea1718253b50f629c8b17fc Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Wed, 7 Jul 2021 09:56:50 +0200 Subject: [PATCH 1/2] acd: log the MAC when announcing an IP (cherry picked from commit 314024ea968e44199e44969b8e948f97ed15965a) --- src/core/devices/nm-acd-manager.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/core/devices/nm-acd-manager.c b/src/core/devices/nm-acd-manager.c index 995abacded..145947bbbe 100644 --- a/src/core/devices/nm-acd-manager.c +++ b/src/core/devices/nm-acd-manager.c @@ -188,6 +188,7 @@ acd_event(int fd, GIOCondition condition, gpointer data) char to_string_buffer[ACD_EVENT_TO_STRING_BUF_SIZE]; gs_free char *hwaddr_str = NULL; gboolean check_probing_done = FALSE; + char buf[ETH_ALEN * 3]; switch (event->event) { case N_ACD_EVENT_READY: @@ -202,8 +203,9 @@ acd_event(int fd, GIOCondition condition, gpointer data) nm_platform_link_get_name(NM_PLATFORM_GET, self->ifindex), acd_error_to_string(r)); } else { - _LOGD("announcing address %s", - _nm_utils_inet4_ntop(info->address, address_str)); + _LOGD("announcing address %s (hw-addr %s)", + _nm_utils_inet4_ntop(info->address, address_str), + _nm_utils_hwaddr_ntoa(self->hwaddr, ETH_ALEN, TRUE, buf, sizeof(buf))); } } check_probing_done = TRUE; @@ -394,6 +396,7 @@ nm_acd_manager_announce_addresses(NMAcdManager *self) int r; int fd; gboolean success = TRUE; + char buf[ETH_ALEN * 3]; r = acd_init(self); if (r) { @@ -428,7 +431,9 @@ nm_acd_manager_announce_addresses(NMAcdManager *self) acd_error_to_string(r)); success = FALSE; } else - _LOGD("announcing address %s", _nm_utils_inet4_ntop(info->address, sbuf)); + _LOGD("announcing address %s (hw-addr %s)", + _nm_utils_inet4_ntop(info->address, sbuf), + _nm_utils_hwaddr_ntoa(self->hwaddr, ETH_ALEN, TRUE, buf, sizeof(buf))); } } From 70aeccf605dc54715c9f4a3862a81b170c97afd5 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Wed, 7 Jul 2021 10:09:45 +0200 Subject: [PATCH 2/2] device: send ARP announcements when there is carrier Previously we sent announcements immediately for non-controllers, or after the first port was attached for controllers. This has two problems: - announcements can be sent when there is no carrier and they would be lost; - if a controller has a port, the port could be itself a controller; in that case we start sending ARPs with the fake address of the port. Later, when a leaf port is added to the second-level controller, the correct port MAC will be propagated by kernel up to both controllers. To solve both problems, send ARP announcements only when the interface has carrier. This also solves the second issue because controllers created by NM have carrier only when there is a port with carrier. Fixes: de1022285a6c ('device: do ARP announcements only after masters have a slave') https://bugzilla.redhat.com/show_bug.cgi?id=1956793 (cherry picked from commit 1377f160ed700f6e8ceff89d9c9934f02aee47c0) --- src/core/devices/nm-device.c | 41 ++++++++++-------------------------- 1 file changed, 11 insertions(+), 30 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 2ab7360b0c..d63b3a7a88 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -4774,10 +4774,6 @@ nm_device_master_enslave_slave(NMDevice *self, NMDevice *slave, NMConnection *co */ nm_device_update_hw_address(self); - /* Send ARP announcements if did not yet and have addresses. */ - if (priv->ip_state_4 == NM_DEVICE_IP_STATE_DONE && !priv->acd.announcing) - nm_device_arp_announce(self); - /* Restart IP configuration if we're waiting for slaves. Do this * after updating the hardware address as IP config may need the * new address. @@ -5106,6 +5102,10 @@ nm_device_set_carrier(NMDevice *self, gboolean carrier) nm_device_remove_pending_action(self, NM_PENDING_ACTION_CARRIER_WAIT, FALSE); _carrier_wait_check_queued_act_request(self); } + + /* Send ARP announcements if did not yet and have carrier. */ + if (priv->ip_state_4 == NM_DEVICE_IP_STATE_DONE && !priv->acd.announcing) + nm_device_arp_announce(self); } else { if (priv->carrier_wait_id) nm_device_add_pending_action(self, NM_PENDING_ACTION_CARRIER_WAIT, FALSE); @@ -11916,7 +11916,6 @@ activate_stage5_ip_config_result_x(NMDevice *self, int addr_family) const char * method; int ip_ifindex; int errsv; - gboolean do_announce = FALSE; req = nm_device_get_act_request(self); g_assert(req); @@ -12044,31 +12043,13 @@ activate_stage5_ip_config_result_x(NMDevice *self, int addr_family) } } - if (IS_IPv4) { - /* Send ARP announcements */ - - if (nm_device_is_master(self)) { - CList * iter; - SlaveInfo *info; - - /* Skip announcement if there are no device enslaved, for two reasons: - * 1) the master has a temporary MAC address until the first slave comes - * 2) announcements are going to be dropped anyway without slaves - */ - do_announce = FALSE; - - c_list_for_each (iter, &priv->slaves) { - info = c_list_entry(iter, SlaveInfo, lst_slave); - if (info->slave_is_enslaved) { - do_announce = TRUE; - break; - } - } - } else - do_announce = TRUE; - - if (do_announce) - nm_device_arp_announce(self); + if (IS_IPv4 && priv->carrier) { + /* We send ARP announcements only when the link gets carrier, + * otherwise the announcements would be lost. Furthermore, for + * controllers having carrier implies that there is at least one + * port and therefore the MAC address is the correct one. + */ + nm_device_arp_announce(self); } if (IS_IPv4) {