----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/75057/#review226574 -----------------------------------------------------------
Can you open a mesos JIRA ticket with all of the information about this bug, and reference it in the reviewboard issues field as well as the comments in the code? src/linux/routing/link/link.hpp Lines 36-37 (patched) <https://reviews.apache.org/r/75057/#comment314839> we tend to avoid macros per the google style guide, so let's avoid this src/linux/routing/link/link.cpp Lines 277-282 (patched) <https://reviews.apache.org/r/75057/#comment314841> Maybe: MAC mismatch between net::mac=X and ioctl=Y for link Z prior to setting to target mac address A Ditto below src/linux/routing/link/link.cpp Lines 282 (patched) <https://reviews.apache.org/r/75057/#comment314840> let's include the target mac address here: "before setting to XXXX" src/linux/routing/link/link.cpp Lines 285-295 (patched) <https://reviews.apache.org/r/75057/#comment314842> probably this comment also needs to go up on the warning log to explain why we do that as well? In the commit message and here, can you also mention the kernel+OS combinations where we've seen this and the combinations where we don't see it? src/linux/routing/link/link.cpp Lines 288 (patched) <https://reviews.apache.org/r/75057/#comment314844> This workaround appears to fix the issue in our testing src/linux/routing/link/link.cpp Lines 294-295 (patched) <https://reviews.apache.org/r/75057/#comment314843> , so we perform a check up front and log initial mismatches as warnings as well src/linux/routing/link/link.cpp Lines 311-317 (patched) <https://reviews.apache.org/r/75057/#comment314845> could we just have a lambda in this function that gives us the mac address of a link via ioctl? ``` auto getMacViaIoCtl = [](const string& link) -> Try<net::MAC> { int fd = ::socket(AF_INET, SOCK_STREAM, 0); if (fd == -1) { return ErrnoError(); } struct ifreq ifr; memset(&ifr, 0, sizeof(ifr)); strncpy(ifr.ifr_name, link.c_str(), IFNAMSIZ); if (ioctl(fd, SIOCGIFHWADDR, &ifr) == -1) { // Save the error string as os::close may overwrite errno. string message = os::strerror(errno); os::close(fd); return Error(std::move(message)); } os::close(fd); return net::MAC(ifr.ifr_hwaddr.sa_data); } ``` ideally there'd be a library function for this, but a lamdba will do for now and I guess since we need the family type.. we have to return the struct: ``` auto getMacViaIoCtl = [](const string& link) -> Try<net::MAC> { // needs to be: auto getAddressViaIoCtl = [](const string& link) -> Try<struct ifreq> { ``` src/linux/routing/link/link.cpp Lines 342 (patched) <https://reviews.apache.org/r/75057/#comment314846> After multiple attempts to set the target MAC address, net::mac and ioctl did not report the correct address - Benjamin Mahler On June 18, 2024, 8:51 p.m., Jason Zhou wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/75057/ > ----------------------------------------------------------- > > (Updated June 18, 2024, 8:51 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Repository: mesos > > > Description > ------- > > It seems that there are scenarios where mesos containers cannot communicate > with agents as the MAC addresses are set incorrectly, leading to dropped > packets. A workaround for this behavior is to check that the MAC address is > set correctly after the ioctl call, and retry the address setting if > necessary. > Observed scenarios with incorrectly assigned MAC addresses: > 1. ioctl returns the correct MAC address, but not net::mac > 2. both net::mac and ioctl return the same MAC address, but are both wrong > 3. There are no cases where ioctl/net::mac come back with the same MAC > address as before setting. i.e. there is no no-op observed. > 4. There is a possibility that ioctl/net::mac results disagree with each > other even before attempting to set our desired MAC address > > > Diffs > ----- > > src/linux/routing/link/link.hpp 35639afa49e083bb9364fe88a95831e9c711563e > src/linux/routing/link/link.cpp bff172dea63f77a2c7bafd7e3fbee5dca1dfbd51 > > > Diff: https://reviews.apache.org/r/75057/diff/4/ > > > Testing > ------- > > > Thanks, > > Jason Zhou > >
