This is an automated email from the ASF dual-hosted git repository.
bmahler pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
The following commit(s) were added to refs/heads/master by this push:
new 8b202bbeb [port mapping isolator] Work around apparent MAC address
kernel bug.
8b202bbeb is described below
commit 8b202bbebdc89429ad82c6983aa1c514eb1b8d95
Author: Jason Zhou <[email protected]>
AuthorDate: Thu Jun 20 00:25:18 2024 -0400
[port mapping isolator] Work around apparent MAC address kernel bug.
It seems that there are scenarios where, when using the port mapping
isolator,
mesos containers sometimes cannot communicate with the mesos agent as the
MAC
address of the veth interface is set incorrectly, leading to dropped packets
by the kernel. This was discovered with the use of tcpdump (which reveals
that
the kernel marks the packets as destined for another host), and the latter
of
which reveals that the kernel is indeed dropping the packets due to this. We
then found that when we set the mac address on the veth interface, it
sometimes
does not "stick" despite ioctl returning successfully.
Observed scenarios with incorrectly assigned MAC addresses:
1. After setting the mac address: ioctl returns the correct MAC address, but
net::mac returns an incorrect MAC address (different from the original!)
2. After setting the mac address: both ioctl and net::mac return the same
MAC
address, but are both wrong (and different from the original one!)
3. After setting the mac address: there are no cases where ioctl or net::mac
come back with the same MAC address as before we set the address.
4. Before we set the mac address: there is a possibility that ioctl and
net::mac results disagree with each other!
5. There is a possibility that the MAC address we set ends up overwritten by
a garbage value after setMAC has already completed and checked that the
mac address was set correctly. Since this error happens after this
function has finished, we cannot log nor detect it in setMAC because we
have not yet studied at what point this occurs.
Notes:
1. We have observed this behavior only on CentOS 9 systems at the moment,
CentOS 7 systems under various kernels do not seem to have the issue
(which is quite strange if this was purely a kernel bug).
2. We have tried kernels 5.15.147, 5.15.160, 5.15.161, all of these have
this issue on CentOS 9.
This patch adds a workaround for this bug, which is to check that the MAC
address is set correctly after the ioctl call, and retry the address setting
if necessary. In our testing, this workaround appears to workaround
scenarios
(1), (2), (3), and (4) above, but it does not address scenario (5).
See MESOS-10243 for additional details, follow-ups.
Review: https://reviews.apache.org/r/75057/
---
src/linux/routing/link/link.cpp | 126 ++++++++++++++++++++++++++++++++--------
1 file changed, 103 insertions(+), 23 deletions(-)
diff --git a/src/linux/routing/link/link.cpp b/src/linux/routing/link/link.cpp
index 87ca26c54..bb97e3281 100644
--- a/src/linux/routing/link/link.cpp
+++ b/src/linux/routing/link/link.cpp
@@ -246,45 +246,125 @@ Try<bool> setUp(const string& link)
}
+
+// There seems to be a bug that is occassionally preventing ioctl and libnl
+// from setting correct MAC addresses, despite them not returning errors.
+//
+// Observed scenarios with incorrectly assigned MAC addresses:
+//
+// 1. After setting the mac address: ioctl returns the correct MAC address, but
+// net::mac returns an incorrect MAC address (different from the original!)
+// 2. After setting the mac address: both ioctl and net::mac return the same
MAC
+// address, but are both wrong (and different from the original one!)
+// 3. After setting the mac address: there are no cases where ioctl or net::mac
+// come back with the same MAC address as before we set the address.
+// 4. Before we set the mac address: there is a possibility that ioctl and
+// net::mac results disagree with each other!
+// 5. There is a possibility that the MAC address we set ends up overwritten by
+// a garbage value after setMAC has already completed and checked that the
+// mac address was set correctly. Since this error happens after this
+// function has finished, we cannot log nor detect it in setMAC because we
+// have not yet studied at what point this occurs.
+//
+// Notes:
+//
+// 1. We have observed this behavior only on CentOS 9 systems at the moment,
+// CentOS 7 systems under various kernels do not seem to have the issue
+// (which is quite strange if this was purely a kernel bug).
+// 2. We have tried kernels 5.15.147, 5.15.160, 5.15.161, all of these have
+// this issue on CentOS 9.
+//
+// To workaround this bug, we check that the MAC address is set correctly
+// after the ioctl call, and retry the address setting if necessary. In our
+// testing, this workaround appears to workaround scenarios (1), (2), (3),
+// and (4) above, but it does not address scenario (5).
+//
+// See MESOS-10243 for additional details, follow-ups.
Try<Nothing> setMAC(const string& link, const net::MAC& mac)
{
+ auto getMacViaIoctl = [](int fd, const string& link) -> Try<ifreq> {
+ ifreq ifr;
+ memset(&ifr, 0, sizeof(ifr));
+ strncpy(ifr.ifr_name, link.c_str(), IFNAMSIZ);
+ if (ioctl(fd, SIOCGIFHWADDR, &ifr) == -1) {
+ return ErrnoError();
+ }
+ return ifr;
+ };
+
+ // Since loopback interface has sa_family ARPHRD_LOOPBACK, we need
+ // to get the MAC address of the link first to decide what value the
+ // sa_family should be.
+ //
// TODO(jieyu): We use ioctl to set the MAC address because the
// interfaces in libnl have some issues with virtual devices.
- struct ifreq ifr;
- memset(&ifr, 0, sizeof(ifr));
-
- strncpy(ifr.ifr_name, link.c_str(), IFNAMSIZ);
-
int fd = ::socket(AF_INET, SOCK_STREAM, 0);
if (fd == -1) {
return ErrnoError();
}
- // Since loopback interface has sa_family ARPHRD_LOOPBACK, we need
- // to get the MAC address of the link first to decide what value the
- // sa_family should be.
- if (ioctl(fd, SIOCGIFHWADDR, &ifr) == -1) {
- // Save the error string as os::close may overwrite errno.
- const string message = os::strerror(errno);
+ Try<ifreq> if_request = getMacViaIoctl(fd, link);
+ if (if_request.isError()) {
os::close(fd);
- return Error(message);
+ return Error(if_request.error());
}
- ifr.ifr_hwaddr.sa_data[0] = mac[0];
- ifr.ifr_hwaddr.sa_data[1] = mac[1];
- ifr.ifr_hwaddr.sa_data[2] = mac[2];
- ifr.ifr_hwaddr.sa_data[3] = mac[3];
- ifr.ifr_hwaddr.sa_data[4] = mac[4];
- ifr.ifr_hwaddr.sa_data[5] = mac[5];
+ // See comment above this function on why we check this and use a loop below.
+ Result<net::MAC> net_mac = net::mac(link);
+ net::MAC ioctl_mac = net::MAC(if_request->ifr_hwaddr.sa_data);
+
+ if (!(net_mac.isSome() && *net_mac == ioctl_mac)) {
+ LOG(WARNING)
+ << "MAC mismatch between net::mac = " << (net_mac.isSome()
+ ? stringify(*net_mac) : (net_mac.isError() ? net_mac.error() :
"None"))
+ << " and ioctl = " << stringify(ioctl_mac)
+ << " for link " << stringify(link)
+ << " before setting to target mac address " << stringify(mac);
+ }
- if (ioctl(fd, SIOCSIFHWADDR, &ifr) == -1) {
- // Save the error string as os::close may overwrite errno.
- const string message = os::strerror(errno);
- os::close(fd);
- return Error(message);
+ for (int i = 0; i < 10; ++i) {
+ if_request->ifr_hwaddr.sa_data[0] = mac[0];
+ if_request->ifr_hwaddr.sa_data[1] = mac[1];
+ if_request->ifr_hwaddr.sa_data[2] = mac[2];
+ if_request->ifr_hwaddr.sa_data[3] = mac[3];
+ if_request->ifr_hwaddr.sa_data[4] = mac[4];
+ if_request->ifr_hwaddr.sa_data[5] = mac[5];
+
+ if (ioctl(fd, SIOCSIFHWADDR, &(*if_request)) == -1) {
+ const string message = os::strerror(errno);
+ os::close(fd);
+ return Error(message);
+ }
+
+ // Prepare another read to check if the MAC address was set correctly.
+ if_request = getMacViaIoctl(fd, link);
+ if (if_request.isError()) {
+ os::close(fd);
+ return Error(if_request.error());
+ }
+
+ net_mac = net::mac(link);
+ ioctl_mac = net::MAC(if_request->ifr_hwaddr.sa_data);
+
+ if (net_mac.isSome() && *net_mac == mac && ioctl_mac == mac) {
+ break;
+ }
+
+ LOG(WARNING)
+ << "MAC mismatch between target mac = " << stringify(mac)
+ << " and net::mac = " << (net_mac.isSome()
+ ? stringify(*net_mac) : (net_mac.isError() ? net_mac.error() :
"None"))
+ << " and ioctl: " << stringify(ioctl_mac)
+ << " for link " << stringify(link) << "; retrying set operation...";
}
os::close(fd);
+
+ if (!(net_mac.isSome() && *net_mac == mac && ioctl_mac == mac)) {
+ return Error(
+ "net::mac and ioctl did not report the correct address despite multiple"
+ " attempts to set target MAC address");
+ }
return Nothing();
}
