Skip to content

Commit

Permalink
fix a race wrt setting route MTU
Browse files Browse the repository at this point in the history
  • Loading branch information
JackDoanRivian committed Sep 20, 2024
1 parent fd96826 commit 09d334f
Showing 1 changed file with 24 additions and 17 deletions.
41 changes: 24 additions & 17 deletions overlay/tun_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,10 +237,10 @@ func (t *tun) RouteFor(ip netip.Addr) netip.Addr {

func (t *tun) Write(b []byte) (int, error) {
var nn int
max := len(b)
maximum := len(b)

for {
n, err := unix.Write(t.fd, b[nn:max])
n, err := unix.Write(t.fd, b[nn:maximum])
if n > 0 {
nn += n
}
Expand Down Expand Up @@ -278,25 +278,22 @@ func hasNetlinkAddr(al []*netlink.Addr, x netlink.Addr) bool {
func (t *tun) addIPs(link netlink.Link) error {
newAddrs := make([]*netlink.Addr, len(t.vpnNetworks))
for i := range t.vpnNetworks {
//todo I wish I didn't need to stringify and re-parse
nlAddr, err := netlink.ParseAddr(t.vpnNetworks[i].String())
if err != nil {
return err
newAddrs[i] = &netlink.Addr{
IPNet: &net.IPNet{
IP: t.vpnNetworks[i].Addr().AsSlice(),
Mask: net.CIDRMask(t.vpnNetworks[i].Bits(), t.vpnNetworks[i].Addr().BitLen()),
},
Label: t.vpnNetworks[i].Addr().Zone(),
}
newAddrs[i] = nlAddr
}

//add all new addresses
for i := range newAddrs {
//todo do we want to stack errors and try as many ops as possible?
//todo AddrReplace should still add new IPs I think
//AddrReplace still adds new IPs, but if their properties change it will change them as well
if err := netlink.AddrReplace(link, newAddrs[i]); err != nil {
return err
}
//newAddrs is the same order as vpnNetworks so this is fine
if err := t.setDefaultRoute(t.vpnNetworks[i]); err != nil {
return err
}
}

//iterate over remainder, remove whoever shouldn't be there
Expand Down Expand Up @@ -370,16 +367,26 @@ func (t *tun) Activate() error {
return err
}

// Set the routes
if err = t.addRoutes(false); err != nil {
return err
}

// Run the interface
ifrf.Flags = ifrf.Flags | unix.IFF_UP | unix.IFF_RUNNING
if err = ioctl(t.ioctlFd, unix.SIOCSIFFLAGS, uintptr(unsafe.Pointer(&ifrf))); err != nil {
return fmt.Errorf("failed to run tun device: %s", err)
}

//set route MTU
for i := range t.vpnNetworks {
if err := t.setDefaultRoute(t.vpnNetworks[i]); err != nil {
t.l.WithError(err).Error("Failed to add routes")
//todo why does this fail but only sometimes?
//todo return err
}
}

// Set the routes
if err = t.addRoutes(false); err != nil {
return err
}

//todo do we want to keep the link-local address?

return nil
Expand Down

0 comments on commit 09d334f

Please sign in to comment.