From 314024ea968e44199e44969b8e948f97ed15965a 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 --- 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 eb2da53a12..c041163ef8 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; @@ -392,6 +394,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) { @@ -426,7 +429,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 1377f160ed700f6e8ceff89d9c9934f02aee47c0 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 --- 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 68920bf888..d8867c5fbb 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); @@ -11923,7 +11923,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); @@ -12051,31 +12050,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) {