Improve the way to get default gateway on macOS #1379

zhengfeihe · 2024-05-02T00:03:29Z

Implement thoughts from #1355

This PR follows the source code of route on a Mac to use a raw socket for retrieving default gateway information from the routing table, thus eliminating the need for system calls.

Although it might be desirable to reuse code logic behind route -n on both Mac and FreeBSD to get the gateway info of the device, the implementation is not the same on two different OS systems. Therefore, we might want to separate macOS and FreeBSD into two PRs, where the FreeBSD would be the next one.

Pcap++/header/PcapLiveDevice.h

tigercosmos

Thanks @zhengfeihe, it's a good start.
Could you address all comments on the PR, add a few remarks for the code, and add some testing for the PR?

Pcap++/header/PcapLiveDevice.h

Pcap++/src/PcapLiveDevice.cpp

…d_in_macos

tigercosmos · 2024-05-05T09:59:18Z

@zhengfeihe next time don't resolve the issue yourself, so other people can easily track the issues. Also, could you let the tests pass?

…d_in_macos

zhengfeihe · 2024-05-05T10:06:16Z

@zhengfeihe next time don't resolve the issue yourself, so other people can easily track the issues. Also, could you let the tests pass?

Thanks for the reminder. I will watch out next time.

tigercosmos · 2024-05-07T04:33:46Z

Pcap++/src/PcapLiveDevice.cpp

+	//route message struct for communication in BSD / APPLE device
+	struct BSDRoutingMessage{
+		struct	rt_msghdr header;
+		char	messageSpace[512];


wondering if 512 is a proper number?

The number used is consistent with the one in the macOS source code. I believe it would be safer to retain it as is.

Pcap++/src/PcapLiveDevice.cpp

…eway_method_in_macos

…d_in_macos

tigercosmos · 2024-05-25T06:59:20Z

@zhengfeihe As we previously discussed, it appears that FreeBSD cannot easily share its implementation with macOS. Therefore, let's separate the FreeBSD implementation from macOS, allowing the macOS implementation to be merged first.

…d_in_macos

zhengfeihe · 2024-06-05T11:02:29Z

@zhengfeihe As we previously discussed, it appears that FreeBSD cannot easily share its implementation with macOS. Therefore, let's separate the FreeBSD implementation from macOS, allowing the macOS implementation to be merged first.

Sorry for the delay. I have changed based on the request.

Pcap++/src/PcapLiveDevice.cpp

tigercosmos · 2024-06-05T11:40:12Z

@zhengfeihe could you check if the current test already covers your change? if not please add the corresponding tests. Also, please modify the description of the PR to explain why you separated the implementation for macOS/FreeBSD.

Pcap++/src/PcapLiveDevice.cpp

zhengfeihe · 2024-06-08T04:23:46Z

@zhengfeihe could you check if the current test already covers your change? if not please add the corresponding tests. Also, please modify the description of the PR to explain why you separated the implementation for macOS/FreeBSD.

I think the test TestPcapLiveDeviceList already check the getDefaultGateway across different platforms, we don't need to add new tests.

…d_in_macos

Pcap++/src/PcapLiveDevice.cpp

tigercosmos · 2024-06-08T06:40:04Z

@seladb I think the PR is ready. Could you take a look?

…d_in_macos

Dimi1010

Please fix the file indentation to use tabs. Other then that, LGTM.

Pcap++/src/PcapLiveDevice.cpp

seladb · 2024-06-11T08:44:50Z

This PR is quite complex, I need to go over it carefully...
@zhengfeihe can you share a link to the source code of route and some references on how its code works?

zhengfeihe · 2024-06-11T10:42:30Z

This PR is quite complex, I need to go over it carefully... @zhengfeihe can you share a link to the source code of route and some references on how its code works?

Here's the source code link : https://github.com/apple-oss-distributions/network_cmds/blob/main/route.tproj/route.c.

I cannot find too much valuable info when I search online, so I implement the pr mainly based on running the source code on my own Mac. This is the test code that can work on my machine, maybe it will help

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/socket.h>
#include <net/route.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <net/if_dl.h>



#define ROUNDUP(a) \
	((a) > 0 ? (1 + (((a) - 1) | (sizeof(uint32_t) - 1))) : sizeof(uint32_t))
#define ADVANCE(x, n) (x += ROUNDUP((n)->sa_len))
#define C(x) ((unsigned char)((x) & 0xff))

int main() {
    int sockfd;

    struct bsd_route_message{
        struct	rt_msghdr m_rtm;
        char	m_space[512];
    } m_rtmsg;

    // Create the socket
    sockfd = socket(PF_ROUTE, SOCK_RAW, 0);
    if (sockfd < 0) {
        perror("socket");
        return EXIT_FAILURE;
    }


    bzero((char *)&m_rtmsg, sizeof(m_rtmsg));

    char* spacePtr = m_rtmsg.m_space;

    bzero((char *)&m_rtmsg, sizeof(m_rtmsg));
    m_rtmsg.m_rtm.rtm_msglen = sizeof(struct rt_msghdr);
    m_rtmsg.m_rtm.rtm_version = RTM_VERSION;
    m_rtmsg.m_rtm.rtm_type = RTM_GET;
    m_rtmsg.m_rtm.rtm_addrs = RTA_DST | RTA_NETMASK | RTA_IFP;
    m_rtmsg.m_rtm.rtm_flags = RTF_UP | RTF_GATEWAY | RTF_STATIC;

    struct	sockaddr_in so_dst = {0} , so_mask = {0};
    struct	sockaddr_dl so_ifp = {0};

    size_t len = sizeof(sockaddr_in);
    bcopy(reinterpret_cast<char*>(&so_dst), spacePtr, len);
    spacePtr += len;
    bcopy(reinterpret_cast<char*>(&so_mask), spacePtr, len);
    spacePtr += len;
    m_rtmsg.m_rtm.rtm_msglen += 2*len ;
    len = sizeof(sockaddr_dl);
    spacePtr += len;
    bcopy(reinterpret_cast<char*>(&so_ifp), spacePtr, len);
    m_rtmsg.m_rtm.rtm_msglen += len ;

    if (write(sockfd, reinterpret_cast<char*>(&m_rtmsg), m_rtmsg.m_rtm.rtm_msglen) < 0) {
    //    PCPP_LOG_ERROR("Error retrieving default gateway address: couldn't write into the routing socket");
        return 1;
    }

    // Read the response from the route socket
    if (read(sockfd,reinterpret_cast<char*>(&m_rtmsg), sizeof(m_rtmsg)) < 0) {
     //   PCPP_LOG_ERROR("Error retrieving default gateway address: couldn't read from the routing socket");
        return 1;
    }

    struct sockaddr_in  *gate = nullptr;
    struct sockaddr_dl *ifp = nullptr;
    struct sockaddr *sa = nullptr;
    spacePtr = (reinterpret_cast<char*>(&m_rtmsg.m_rtm+1));
    for (int i = 1; i > 0; i <<= 1)
    {
        if (i & m_rtmsg.m_rtm.rtm_addrs)
        {
            sa =  reinterpret_cast<sockaddr *>(spacePtr);
            switch (i)
            {
                case RTA_GATEWAY:
                    gate = (sockaddr_in* )sa;
                    break;
                case RTA_IFP:
                    if (sa->sa_family == AF_LINK &&
                        ((sockaddr_dl *)sa)->sdl_nlen)
                        ifp = (sockaddr_dl *)sa;
                    break;
            }
            // Make sure the increment is the nearest multiple of the size of uint32_t
            spacePtr += sa->sa_len > 0 ? (1 + (((sa->sa_len) - 1) | (sizeof(uint32_t) - 1))) : sizeof(uint32_t);
        }
    }

    printf("%d", gate->sin_addr.s_addr);
    gate->sin_addr.s_addr = ntohl(gate->sin_addr.s_addr);
    printf("%d", gate->sin_addr.s_addr);
    printf("\n%u.%u.%u.%u",C(gate->sin_addr.s_addr>> 24), C(gate->sin_addr.s_addr>> 16), C(gate->sin_addr.s_addr>> 8), C(gate->sin_addr.s_addr));
    // Read the response from the route socket

    close(sockfd);
    
    return EXIT_SUCCESS;
}

seladb · 2024-06-12T07:03:20Z

I cannot find too much valuable info when I search online, so I implement the pr mainly based on running the source code on my own Mac. This is the test code that can work on my machine, maybe it will help

This is great, thank you @zhengfeihe ! Let me try to run this code on my Mac to better understand how it works

seladb · 2024-06-12T08:05:56Z

Pcap++/src/PcapLiveDevice.cpp

+	size_t len = sizeof(sockaddr_in);
+	bcopy(reinterpret_cast<char*>(&so_dst), spacePtr, len);
+	spacePtr += len;
+	bcopy(reinterpret_cast<char*>(&so_mask), spacePtr, len);
+	spacePtr += len;
+	routingMessage.header.rtm_msglen += 2*len ;
+	len = sizeof(sockaddr_dl);
+	bcopy(reinterpret_cast<char*>(&so_ifp), spacePtr, len);


Both so_dst, so_ifp and so_mask are all zeros, why do we need to copy them to spacePtr?

We should probably use memcpy instead of bcopy

I think we can make this code much simpler by just calculating the length:

routingMessage.header.rtm_msglen += (sizeof(sockaddr_in) + sizeof(sockaddr_dl));

I mainly follow the code in the route.c source code.I think they want to ensure that in the routing message data space, these structures are laid out in order, so that they can be read when we receive the returning routing message.

I will try the new code see if it works well.

seladb · 2024-06-12T08:09:08Z

Pcap++/src/PcapLiveDevice.cpp

+	struct sockaddr_dl *ifp = nullptr;
+	struct sockaddr *sa = nullptr;
+	spacePtr = (reinterpret_cast<char*>(&routingMessage.header+1));
+	for (int i = 1; i != 0; i <<= 1)


Instead of going over all integers, maybe we can check which bits are set in routingMessage.header.rtm_addrs, something like this:

auto rtmAddrs = routingMessage.header.rtm_addrs; int index = 0; while (rtmAddrs) { if (rtmAddrs & 1) { // check if index is RTA_GATEWAY or RTA_IFP } index++; rtmAddrs >>= 1; }

seladb · 2024-06-12T08:10:19Z

Pcap++/src/PcapLiveDevice.cpp

+			case RTA_IFP:
+				if (sa->sa_family == AF_LINK &&
+						reinterpret_cast<sockaddr_dl*>(sa)->sdl_nlen)
+					ifp = reinterpret_cast<sockaddr_dl *>(sa);
+				break;


Why do we need RTA_IFP in addition to RTA_GATEWAY?

I'm not even sure what is IFP 🙂

RTA_IFP is used in route messages to specify the interface through which packets should be sent. By print ifp, we can get the interface info, eg add this line

(void)printf(" interface: %.*s\n", ifp->sdl_nlen, ifp->sdl_data);

we can get

interface: en0

We can still get the default gateway info if we remove RTA_IFP, but I think maybe it's ok to remain the check of the interface?

seladb · 2024-06-12T08:43:25Z

Pcap++/src/PcapLiveDevice.cpp

+			spacePtr += sa->sa_len > 0 ? (1 + (((sa->sa_len) - 1) | (sizeof(uint32_t) - 1))) : sizeof(uint32_t);
+		}


maybe we can extract this to a separate function in this .cpp file to make the code more readable?

int closestMultipleOf(int multiple, int num) { return ((num + multiple - 1) / multiple) * multiple; } ... spacePtr += sa->sa_len > 0 ? closestMultipleOf(sizeof(uint32_t), sa->sa_len) : sizeof(uint32_t);

seladb · 2024-06-12T08:44:05Z

Pcap++/src/PcapLiveDevice.cpp

+			spacePtr += sa->sa_len > 0 ? (1 + (((sa->sa_len) - 1) | (sizeof(uint32_t) - 1))) : sizeof(uint32_t);
+		}
+	}
+	if(gateAddr == nullptr || ifp == nullptr)


why ifp == nullptr means that the gateway couldn't be extracted?

when ifp == nullptr, it means there's no valid interface info to be extracted through which we send/read packets, so this routing of getting default gateway maybe invalid as well.

tigercosmos · 2024-06-17T13:22:47Z

@zhengfeihe could you modify the remaining parts?

improve macOS get default gateway

82b02d1

zhengfeihe requested a review from seladb as a code owner May 2, 2024 00:03

zhengfeihe marked this pull request as draft May 2, 2024 00:04

zhengfeihe commented May 2, 2024

View reviewed changes

Pcap++/header/PcapLiveDevice.h Outdated Show resolved Hide resolved

tigercosmos requested changes May 2, 2024

View reviewed changes

tigercosmos and others added 2 commits May 3, 2024 11:02

Merge branch 'dev' into feature/improve_getting_default_gateway_metho…

dc56b9a

…d_in_macos

fix based on comments

5f6750a

Merge branch 'dev' into feature/improve_getting_default_gateway_metho…

d9b34b2

…d_in_macos

zhengfeihe closed this May 5, 2024

zhengfeihe reopened this May 5, 2024

fix header file issue in windows system

2605bed

tigercosmos reviewed May 7, 2024

View reviewed changes

Pcap++/src/PcapLiveDevice.cpp Outdated Show resolved Hide resolved

tigercosmos reviewed May 7, 2024

View reviewed changes

Pcap++/src/PcapLiveDevice.cpp Outdated Show resolved Hide resolved

zhengfeihe and others added 3 commits May 17, 2024 16:09

Merge branch 'seladb:master' into feature/improve_getting_default_gat…

8d520c6

…eway_method_in_macos

Merge branch 'dev' into feature/improve_getting_default_gateway_metho…

66296ae

…d_in_macos

Update Pcap++/src/PcapLiveDevice.cpp

e4a1289

egecetin added this to the Augest 2024 Release milestone Jun 5, 2024

zhengfeihe and others added 2 commits June 5, 2024 18:31

Merge branch 'dev' into feature/improve_getting_default_gateway_metho…

d699597

…d_in_macos

separate freeBSD implementation from macOS

3a50896

tigercosmos reviewed Jun 5, 2024

View reviewed changes

Pcap++/src/PcapLiveDevice.cpp Outdated Show resolved Hide resolved

tigercosmos reviewed Jun 5, 2024

View reviewed changes

Pcap++/src/PcapLiveDevice.cpp Outdated Show resolved Hide resolved

Dimi1010 reviewed Jun 5, 2024

View reviewed changes

Pcap++/src/PcapLiveDevice.cpp Outdated Show resolved Hide resolved

Pcap++/src/PcapLiveDevice.cpp Outdated Show resolved Hide resolved

Pcap++/src/PcapLiveDevice.cpp Outdated Show resolved Hide resolved

Dimi1010 reviewed Jun 5, 2024

View reviewed changes

Pcap++/src/PcapLiveDevice.cpp Outdated Show resolved Hide resolved

zhengfeihe force-pushed the feature/improve_getting_default_gateway_method_in_macos branch from 9800dfa to 6d90884 Compare June 8, 2024 02:41

change c style code to cpp style

0c9db45

zhengfeihe force-pushed the feature/improve_getting_default_gateway_method_in_macos branch from 6d90884 to 0c9db45 Compare June 8, 2024 04:32

tigercosmos marked this pull request as ready for review June 8, 2024 06:28

Merge branch 'dev' into feature/improve_getting_default_gateway_metho…

6236fb8

…d_in_macos

tigercosmos reviewed Jun 8, 2024

View reviewed changes

Pcap++/src/PcapLiveDevice.cpp Show resolved Hide resolved

zhengfeihe and others added 3 commits June 9, 2024 10:18

separate MacOS and FreeBSD header files

968f4ca

Merge branch 'dev' into feature/improve_getting_default_gateway_metho…

d8b6cf1

…d_in_macos

add sys header for macOS

7543cbd

tigercosmos approved these changes Jun 9, 2024

View reviewed changes

Dimi1010 approved these changes Jun 9, 2024

View reviewed changes

Pcap++/src/PcapLiveDevice.cpp Outdated Show resolved Hide resolved

Pcap++/src/PcapLiveDevice.cpp Outdated Show resolved Hide resolved

Pcap++/src/PcapLiveDevice.cpp Outdated Show resolved Hide resolved

use tabs for indent

4072592

seladb reviewed Jun 12, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve the way to get default gateway on macOS #1379

Improve the way to get default gateway on macOS #1379

zhengfeihe commented May 2, 2024 •

edited by tigercosmos

tigercosmos left a comment

tigercosmos commented May 5, 2024

zhengfeihe commented May 5, 2024

tigercosmos May 7, 2024

zhengfeihe Jun 5, 2024

tigercosmos commented May 25, 2024

zhengfeihe commented Jun 5, 2024 •

edited

tigercosmos commented Jun 5, 2024

zhengfeihe commented Jun 8, 2024

tigercosmos commented Jun 8, 2024 •

edited

Dimi1010 left a comment

seladb commented Jun 11, 2024

zhengfeihe commented Jun 11, 2024 •

edited

seladb commented Jun 12, 2024

seladb Jun 12, 2024

zhengfeihe Jun 16, 2024

seladb Jun 12, 2024

seladb Jun 12, 2024

zhengfeihe Jun 16, 2024

seladb Jun 12, 2024

seladb Jun 12, 2024

zhengfeihe Jun 16, 2024

tigercosmos commented Jun 17, 2024

		spacePtr += sa->sa_len > 0 ? (1 + (((sa->sa_len) - 1) \| (sizeof(uint32_t) - 1))) : sizeof(uint32_t);
		}

Improve the way to get default gateway on macOS #1379

Are you sure you want to change the base?

Improve the way to get default gateway on macOS #1379

Conversation

zhengfeihe commented May 2, 2024 • edited by tigercosmos

tigercosmos left a comment

Choose a reason for hiding this comment

tigercosmos commented May 5, 2024

zhengfeihe commented May 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tigercosmos commented May 25, 2024

zhengfeihe commented Jun 5, 2024 • edited

tigercosmos commented Jun 5, 2024

zhengfeihe commented Jun 8, 2024

tigercosmos commented Jun 8, 2024 • edited

Dimi1010 left a comment

Choose a reason for hiding this comment

seladb commented Jun 11, 2024

zhengfeihe commented Jun 11, 2024 • edited

seladb commented Jun 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tigercosmos commented Jun 17, 2024

zhengfeihe commented May 2, 2024 •

edited by tigercosmos

zhengfeihe commented Jun 5, 2024 •

edited

tigercosmos commented Jun 8, 2024 •

edited

zhengfeihe commented Jun 11, 2024 •

edited