core/internal/network: use DHCP router/gateway
This makes us actually set up a default route now. We also stop using github.com/insomniacslk/dhcp types, and instead use our type for the DHCP status. Finally, we also comment the DHCP client a bit better.
This fixes T651.
Test Plan: lacking a regression test, working on one now.
Bug: T651
X-Origin-Diff: phab/D403
GitOrigin-RevId: caead83016cfe2f1783fad33e8d71723a3a32057
diff --git a/core/internal/network/dhcp.go b/core/internal/network/dhcp.go
index f3f25aa..9a2ba8c 100644
--- a/core/internal/network/dhcp.go
+++ b/core/internal/network/dhcp.go
@@ -18,6 +18,8 @@
import (
"context"
+ "fmt"
+ "net"
"github.com/insomniacslk/dhcp/dhcpv4"
"github.com/insomniacslk/dhcp/dhcpv4/nclient4"
@@ -38,19 +40,34 @@
}
type dhcpStatusReq struct {
- resC chan *dhcpv4.DHCPv4
+ resC chan *dhcpStatus
wait bool
}
-func (r *dhcpStatusReq) fulfill(p *dhcpv4.DHCPv4) {
+func (r *dhcpStatusReq) fulfill(s *dhcpStatus) {
go func() {
- r.resC <- p
+ r.resC <- s
}()
}
+// dhcpStatus is the IPv4 configuration provisioned via DHCP for a given interface. It does not necessarily represent
+// a configuration that is active or even valid.
+type dhcpStatus struct {
+ // address is 'our' (the node's) IPv4 address on the network.
+ address net.IPNet
+ // gateway is the default gateway/router of this network, or 0.0.0.0 if none was given.
+ gateway net.IP
+ // dns is a list of IPv4 DNS servers to use.
+ dns []net.IP
+}
+
+func (s *dhcpStatus) String() string {
+ return fmt.Sprintf("Address: %s, Gateway: %s, DNS: %v", s.address.String(), s.gateway.String(), s.dns)
+}
+
func (c *dhcpClient) run(ctx context.Context, iface netlink.Link) {
// Channel updated with address once one gets assigned/updated
- newC := make(chan *dhcpv4.DHCPv4)
+ newC := make(chan *dhcpStatus)
// Status requests waiting for configuration
waiters := []*dhcpStatusReq{}
@@ -67,7 +84,7 @@
c.logger.Error("DHCP lease request failed", zap.Error(err))
// TODO(q3k): implement retry logic with full state machine
}
- newC <- ack
+ newC <- parseAck(ack)
}()
// State machine
@@ -75,7 +92,7 @@
// We start at WAITING, once we get a current config we move to ASSIGNED
// Once this becomes more complex (ie. has to handle link state changes)
// this should grow into a real state machine.
- var current *dhcpv4.DHCPv4
+ var current *dhcpStatus
c.logger.Info("DHCP client WAITING")
for {
select {
@@ -101,8 +118,28 @@
}
}
-func (c *dhcpClient) status(ctx context.Context, wait bool) (*dhcpv4.DHCPv4, error) {
- resC := make(chan *dhcpv4.DHCPv4)
+// parseAck turns an internal status (from the dhcpv4 library) into a dhcpStatus
+func parseAck(ack *dhcpv4.DHCPv4) *dhcpStatus {
+ address := net.IPNet{IP: ack.YourIPAddr, Mask: ack.SubnetMask()}
+
+ // DHCP routers are optional - if none are provided, assume no router and set gateway to 0.0.0.0
+ // (this makes gateway.IsUnspecified() == true)
+ gateway, _, _ := net.ParseCIDR("0.0.0.0/0")
+ if routers := ack.Router(); len(routers) > 0 {
+ gateway = routers[0]
+ }
+ return &dhcpStatus{
+ address: address,
+ gateway: gateway,
+ dns: ack.DNS(),
+ }
+}
+
+// status returns the DHCP configuration requested from us by the local DHCP server.
+// If wait is true, this function will block until a DHCP configuration is available. Otherwise, a nil status may be
+// returned to indicate that no configuration has been received yet.
+func (c *dhcpClient) status(ctx context.Context, wait bool) (*dhcpStatus, error) {
+ resC := make(chan *dhcpStatus)
c.reqC <- &dhcpStatusReq{
resC: resC,
wait: wait,
diff --git a/core/internal/network/main.go b/core/internal/network/main.go
index 7c22249..01760c7 100644
--- a/core/internal/network/main.go
+++ b/core/internal/network/main.go
@@ -76,16 +76,23 @@
return unix.Rename(resolvConfSwapPath, resolvConfPath)
}
-func addNetworkRoutes(link netlink.Link, addr net.IPNet, gw net.IP) error {
+func (s *Service) addNetworkRoutes(link netlink.Link, addr net.IPNet, gw net.IP) error {
if err := netlink.AddrReplace(link, &netlink.Addr{IPNet: &addr}); err != nil {
return err
}
- if err := netlink.RouteAdd(&netlink.Route{
+
+ if gw.IsUnspecified() {
+ s.Logger.Info("No default route set, only local network will be reachable", zap.String("local", addr.String()))
+ return nil
+ }
+
+ route := &netlink.Route{
Dst: &net.IPNet{IP: net.IPv4(0, 0, 0, 0), Mask: net.IPv4Mask(0, 0, 0, 0)},
Gw: gw,
Scope: netlink.SCOPE_UNIVERSE,
- }); err != nil {
- return fmt.Errorf("failed to add default route: %w", err)
+ }
+ if err := netlink.RouteAdd(route); err != nil {
+ return fmt.Errorf("could not add default route: netlink.RouteAdd(%+v): %v", route, err)
}
return nil
}
@@ -98,11 +105,11 @@
return fmt.Errorf("could not get DHCP status: %v", err)
}
- if err := setResolvconf(status.DNS(), []string{}); err != nil {
+ if err := setResolvconf(status.dns, []string{}); err != nil {
s.Logger.Warn("failed to set resolvconf", zap.Error(err))
}
- if err := addNetworkRoutes(iface, net.IPNet{IP: status.YourIPAddr, Mask: status.SubnetMask()}, status.GatewayIPAddr); err != nil {
+ if err := s.addNetworkRoutes(iface, net.IPNet{IP: status.address.IP, Mask: status.address.Mask}, status.gateway); err != nil {
s.Logger.Warn("failed to add routes", zap.Error(err))
}
return nil
@@ -114,7 +121,7 @@
if err != nil {
return nil, err
}
- return &status.YourIPAddr, nil
+ return &status.address.IP, nil
}
func (s *Service) OnStart() error {