Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tun_darwin #163

Merged
merged 20 commits into from
Nov 10, 2021
Merged

tun_darwin #163

merged 20 commits into from
Nov 10, 2021

Conversation

harpchad
Copy link
Contributor

Two changes:

  • Remove water and replace with sys call for tun setup
  • Support named interfaces

This needs testing

I submitted the PR to get some more help reviewing and testing. I've tested on Catalina.

If anyone has pointers on sending route messages to an AF_ROUTE socket so we can replace the execs I'd love to see an example.

tun_darwin.go Outdated
h[0] = 0x00
h[1] = 0x00
h[2] = 0x00
h[3] = unix.AF_INET
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you provide a link for the description of this behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's doing the same thing IFF_NO_PI does on Linux (doesn't appear that flag exists in Darwin/BSD). We have to strip the 4 byte protocol information header header on read, and add it on write.

tun_darwin.go Outdated
h[1] = 0x00
h[2] = 0x00
h[3] = unix.AF_INET
b = append(h[:], b[:]...)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will trash the GC. Ideally we'd find a way to pass a slice with h prepended and avoid the copy. That said, darwin is not a likely maximum performance target.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}

/*
// Set the transmit queue length
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be managed by some ring buffer sizes and slot size. I need to read the code some more, but if we want to target performance we'd want to expose these options in config

These control options in specific
UTUN_OPT_SLOT_SIZE
UTUN_OPT_NETIF_RING_SIZE
UTUN_OPT_TX_FSW_RING_SIZE
UTUN_OPT_RX_FSW_RING_SIZE
UTUN_OPT_MAX_PENDING_PACKETS

@nbrownus
Copy link
Collaborator

Whew there's a lot to learn here. I believe this will come in handy for establishing routes without exec.

newroute function https://opensource.apple.com/source/network_cmds/network_cmds-307.0.1/route.tproj/route.c.auto.html

@wadey wadey added this to the v1.3.0 milestone Sep 15, 2020
@wadey wadey modified the milestones: v1.3.0, v1.4.0 Sep 22, 2020
@CLAassistant
Copy link

CLAassistant commented Feb 17, 2021

CLA assistant check
All committers have signed the CLA.

@wadey wadey modified the milestones: v1.4.0, v1.5.0 Apr 26, 2021
wadey added 7 commits November 8, 2021 09:16
Also don't make it fatal if an invalid name is set, because someone may
have historically set it and not know it will break Nebula when
upgrading.
Surely there must be a better way to get the ifIndex, but it doesn't
seem like SIOCGIFINDEX exists.
@wadey
Copy link
Member

wadey commented Nov 9, 2021

i've updated this branch up to master, and also removed the os/exec usage (replaced with proper AF_ROUTE socket calls).

This is ready for more testing!

nbrownus
nbrownus previously approved these changes Nov 9, 2021
@wadey wadey merged commit 1915fab into slackhq:master Nov 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants