diff --git a/pkg/dhcp/server.go b/pkg/dhcp/server.go index 5a7fcdba7..b285b5c9a 100644 --- a/pkg/dhcp/server.go +++ b/pkg/dhcp/server.go @@ -23,6 +23,7 @@ import ( "bytes" "errors" "fmt" + "io" "net" "strings" @@ -304,14 +305,18 @@ func (s *Server) getStaticRoutes(ip net.IPNet) (router, routes []byte, err error continue } } - b.Write(toDestinationDescriptor(route.Dst)) - if gw != nil { - b.Write(gw) - } else { - b.Write([]byte{0, 0, 0, 0}) - } + writeRoute(&b, route.Dst, gw) } + // XXX: classless routes apparently cause a problem: + // https://askubuntu.com/questions/767002/dhcp-connection-does-not-set-default-gateway-automatically + if b.Len() != 0 && router != nil { + writeRoute(&b, net.IPNet{ + IP: net.IPv4zero, + Mask: net.IPv4Mask(0, 0, 0, 0), + }, router) + router = nil + } routes = b.Bytes() return } @@ -327,6 +332,15 @@ func toDestinationDescriptor(network net.IPNet) []byte { ) } +func writeRoute(w io.Writer, dst net.IPNet, gw net.IP) { + w.Write(toDestinationDescriptor(dst)) + if gw != nil { + w.Write(gw) + } else { + w.Write([]byte{0, 0, 0, 0}) + } +} + func widthOfMaskToSignificantOctets(mask int) int { return (mask + 7) / 8 } diff --git a/tests/network/dhcp_test.go b/tests/network/dhcp_test.go index 9eca156ec..f82698a25 100644 --- a/tests/network/dhcp_test.go +++ b/tests/network/dhcp_test.go @@ -31,15 +31,15 @@ import ( "github.com/Mirantis/virtlet/pkg/network" ) -type dhcpTestCase struct { - csn network.ContainerSideNetwork - expectedSubstrings []string -} - func TestDhcpServer(t *testing.T) { clientMac, _ := net.ParseMAC(clientMacAddrs[0]) - testCases := []*dhcpTestCase{ + for _, tc := range []struct { + name string + csn network.ContainerSideNetwork + expectedSubstrings []string + }{ { + name: "with classless routes", csn: network.ContainerSideNetwork{ Result: &cnicurrent.Result{ Interfaces: []*cnicurrent.Interface{ @@ -87,7 +87,62 @@ func TestDhcpServer(t *testing.T) { }, expectedSubstrings: []string{ "new_broadcast_address='10.1.90.255'", - "new_classless_static_routes='10.10.42.0/24 10.1.90.90'", + "new_classless_static_routes='10.10.42.0/24 10.1.90.90 0.0.0.0/0 10.1.90.1'", + "new_dhcp_lease_time='86400'", + "new_dhcp_rebinding_time='64800'", + "new_dhcp_renewal_time='43200'", + "new_dhcp_server_identifier='169.254.254.2'", + "new_domain_name_servers='8.8.8.8'", + "new_ip_address='10.1.90.5'", + "new_interface_mtu='9000'", + "new_network_number='10.1.90.0'", + "new_subnet_cidr='24'", + "new_subnet_mask='255.255.255.0'", + "veth0: offered 10.1.90.5 from 169.254.254.2", + }, + }, + { + name: "without classless routes", + csn: network.ContainerSideNetwork{ + Result: &cnicurrent.Result{ + Interfaces: []*cnicurrent.Interface{ + { + Name: "eth0", + Mac: clientMacAddrs[0], + // Sandbox is clientNS dependent + // so it must be set in runtime + }, + }, + IPs: []*cnicurrent.IPConfig{ + { + Version: "4", + Interface: 0, + Address: net.IPNet{ + IP: net.IP{10, 1, 90, 5}, + Mask: net.IPMask{255, 255, 255, 0}, + }, + Gateway: net.IP{10, 1, 90, 1}, + }, + }, + Routes: []*cnitypes.Route{ + { + Dst: net.IPNet{ + IP: net.IP{0, 0, 0, 0}, + Mask: net.IPMask{0, 0, 0, 0}, + }, + GW: net.IP{10, 1, 90, 1}, + }, + }, + }, + Interfaces: []*network.InterfaceDescription{ + { + HardwareAddr: clientMac, + MTU: 9000, + }, + }, + }, + expectedSubstrings: []string{ + "new_broadcast_address='10.1.90.255'", "new_dhcp_lease_time='86400'", "new_dhcp_rebinding_time='64800'", "new_dhcp_renewal_time='43200'", @@ -103,66 +158,60 @@ func TestDhcpServer(t *testing.T) { }, }, // TODO: add dns test case here + } { + t.Run(tc.name, func(t *testing.T) { + serverNS, err := ns.NewNS() + if err != nil { + t.Fatalf("Failed to create ns for dhcp server: %v", err) + } + defer serverNS.Close() + clientNS, err := ns.NewNS() + if err != nil { + t.Fatalf("Failed to create ns for dhcp client: %v", err) + } + defer clientNS.Close() + + // Sandbox is clientNS dependent so it needs to be set there on all interfaces + for _, iface := range tc.csn.Result.Interfaces { + iface.Sandbox = clientNS.Path() + } + + var clientVeth, serverVeth netlink.Link + if err := serverNS.Do(func(ns.NetNS) error { + serverVeth, clientVeth, err = nettools.CreateEscapeVethPair(clientNS, "veth0", 1500) + if err != nil { + return fmt.Errorf("failed to create escape veth pair: %v", err) + } + addr, err := netlink.ParseAddr("169.254.254.2/24") + if err != nil { + return fmt.Errorf("failed to parse dhcp interface address: %v", err) + } + if err = netlink.AddrAdd(serverVeth, addr); err != nil { + return fmt.Errorf("failed to add addr for serverVeth: %v", err) + } + + return nil + }); err != nil { + t.Fatal(err) + } + + if err := clientNS.Do(func(ns.NetNS) error { + mac, _ := net.ParseMAC(clientMacAddrs[0]) + if err = nettools.SetHardwareAddr(clientVeth, mac); err != nil { + return fmt.Errorf("can't set MAC address on the client interface: %v", err) + } + + return nil + }); err != nil { + t.Fatal(err) + } + + g := NewNetTestGroup(t, 15*time.Second) + defer g.Stop() + g.Add(serverNS, NewDhcpServerTester(&tc.csn)) + + g.Add(clientNS, NewDhcpClient("veth0", tc.expectedSubstrings)) + g.Wait() + }) } - - for _, testCase := range testCases { - // TODO: use subtests https://golang.org/pkg/testing/#hdr-Subtests_and_Sub_benchmarks - // (need newer Go) - runDhcpTestCase(t, testCase) - } -} - -func runDhcpTestCase(t *testing.T, testCase *dhcpTestCase) { - serverNS, err := ns.NewNS() - if err != nil { - t.Fatalf("Failed to create ns for dhcp server: %v", err) - } - defer serverNS.Close() - clientNS, err := ns.NewNS() - if err != nil { - t.Fatalf("Failed to create ns for dhcp client: %v", err) - } - defer clientNS.Close() - - // Sandbox is clientNS dependent so it needs to be set there on all interfaces - for _, iface := range testCase.csn.Result.Interfaces { - iface.Sandbox = clientNS.Path() - } - - var clientVeth, serverVeth netlink.Link - if err := serverNS.Do(func(ns.NetNS) error { - serverVeth, clientVeth, err = nettools.CreateEscapeVethPair(clientNS, "veth0", 1500) - if err != nil { - return fmt.Errorf("failed to create escape veth pair: %v", err) - } - addr, err := netlink.ParseAddr("169.254.254.2/24") - if err != nil { - return fmt.Errorf("failed to parse dhcp interface address: %v", err) - } - if err = netlink.AddrAdd(serverVeth, addr); err != nil { - return fmt.Errorf("failed to add addr for serverVeth: %v", err) - } - - return nil - }); err != nil { - t.Fatal(err) - } - - if err := clientNS.Do(func(ns.NetNS) error { - mac, _ := net.ParseMAC(clientMacAddrs[0]) - if err = nettools.SetHardwareAddr(clientVeth, mac); err != nil { - return fmt.Errorf("can't set MAC address on the client interface: %v", err) - } - - return nil - }); err != nil { - t.Fatal(err) - } - - g := NewNetTestGroup(t, 15*time.Second) - defer g.Stop() - g.Add(serverNS, NewDhcpServerTester(&testCase.csn)) - - g.Add(clientNS, NewDhcpClient("veth0", testCase.expectedSubstrings)) - g.Wait() } diff --git a/tests/network/vm_network_test.go b/tests/network/vm_network_test.go index ab687bae4..75369d673 100644 --- a/tests/network/vm_network_test.go +++ b/tests/network/vm_network_test.go @@ -271,10 +271,9 @@ func TestVmNetwork(t *testing.T) { vnt.addTcpdump(hostVeth, "10.1.90.1.4243 > 10.1.90.5.4242: UDP", "BOOTP/DHCP") vnt.g.Add(contNS, NewDhcpServerTester(csn)) vnt.verifyDhcp("tap0", []string{ - "new_classless_static_routes='10.10.42.0/24 10.1.90.90'", + "new_classless_static_routes='10.10.42.0/24 10.1.90.90 0.0.0.0/0 10.1.90.1'", "new_ip_address='10.1.90.5'", "new_network_number='10.1.90.0'", - "new_routers='10.1.90.1'", "new_subnet_mask='255.255.255.0'", "tap0: offered 10.1.90.5 from 169.254.254.2", }) @@ -424,10 +423,9 @@ func TestTapFDSource(t *testing.T) { tcpdumpStopOn: "10.1.90.1.4243 > 10.1.90.5.4242: UDP", dhcpExpectedSubstrings: [][]string{ { - "new_classless_static_routes='10.10.42.0/24 10.1.90.90'", + "new_classless_static_routes='10.10.42.0/24 10.1.90.90 0.0.0.0/0 10.1.90.1'", "new_ip_address='10.1.90.5'", "new_network_number='10.1.90.0'", - "new_routers='10.1.90.1'", "new_subnet_mask='255.255.255.0'", "tap0: offered 10.1.90.5 from 169.254.254.2", }, @@ -498,10 +496,9 @@ func TestTapFDSource(t *testing.T) { tcpdumpStopOn: "10.1.90.1.4243 > 10.1.90.5.4242: UDP", dhcpExpectedSubstrings: [][]string{ { - "new_classless_static_routes='10.10.42.0/24 10.1.90.90'", + "new_classless_static_routes='10.10.42.0/24 10.1.90.90 0.0.0.0/0 10.1.90.1'", "new_ip_address='10.1.90.5'", "new_network_number='10.1.90.0'", - "new_routers='10.1.90.1'", "new_subnet_mask='255.255.255.0'", "tap0: offered 10.1.90.5 from 169.254.254.2", }, @@ -633,10 +630,9 @@ func TestTapFDSource(t *testing.T) { tcpdumpStopOn: "10.1.90.1.4243 > 10.1.90.5.4242: UDP", dhcpExpectedSubstrings: [][]string{ { - "new_classless_static_routes='10.10.42.0/24 10.1.90.90'", + "new_classless_static_routes='10.10.42.0/24 10.1.90.90 0.0.0.0/0 10.1.90.1'", "new_ip_address='10.1.90.5'", "new_network_number='10.1.90.0'", - "new_routers='10.1.90.1'", "new_subnet_mask='255.255.255.0'", "tap0: offered 10.1.90.5 from 169.254.254.2", }, @@ -671,10 +667,9 @@ func TestTapFDSource(t *testing.T) { tcpdumpStopOn: "10.1.90.1.4243 > 10.1.90.5.4242: UDP", dhcpExpectedSubstrings: [][]string{ { - "new_classless_static_routes='10.10.42.0/24 10.1.90.90'", + "new_classless_static_routes='10.10.42.0/24 10.1.90.90 0.0.0.0/0 10.1.90.1'", "new_ip_address='10.1.90.5'", "new_network_number='10.1.90.0'", - "new_routers='10.1.90.1'", "new_subnet_mask='255.255.255.0'", "tap0: offered 10.1.90.5 from 169.254.254.2", },