diff --git a/pkg/pillar/nireconciler/genericitems/dnsmasq.go b/pkg/pillar/nireconciler/genericitems/dnsmasq.go index 0d820bdb8d..ec9c68c19a 100644 --- a/pkg/pillar/nireconciler/genericitems/dnsmasq.go +++ b/pkg/pillar/nireconciler/genericitems/dnsmasq.go @@ -56,12 +56,6 @@ type Dnsmasq struct { type DHCPServer struct { // Subnet : network address + netmask (IPv4 or IPv6). Subnet *net.IPNet - // AllOnesNetmask : if enabled, DHCP server will advertise netmask with all bits - // set to one (e.g. /32 for IPv4) instead of using the actual netmask from Subnet. - // This together with Classless routes (routing traffic for the actual Subnet) - // can be used to force all traffic to go through the configured GatewayIP - // (where ACLs could be applied). - AllOnesNetmask bool // IPRange : a range of IP addresses to allocate from. // Not applicable for IPv6 (SLAAC is used instead). IPRange IPRange @@ -92,10 +86,10 @@ type DHCPServer struct { // String describes DHCPServer config. func (d DHCPServer) String() string { - return fmt.Sprintf("DHCPServer: {subnet: %s, allOnesNetmask: %t, ipRange: <%s-%s>, "+ + return fmt.Sprintf("DHCPServer: {subnet: %s, ipRange: <%s-%s>, "+ "gatewayIP: %s, withDefaultRoute: %t, domainName: %s, dnsServers: %v, ntpServers: %v, "+ "staticEntries: %v, propagateRoutes: %v, MTU: %d}", - d.Subnet, d.AllOnesNetmask, d.IPRange.FromIP, d.IPRange.ToIP, d.GatewayIP, + d.Subnet, d.IPRange.FromIP, d.IPRange.ToIP, d.GatewayIP, d.WithDefaultRoute, d.DomainName, d.DNSServers, d.NTPServers, d.StaticEntries, d.PropagateRoutes, d.MTU) } @@ -103,7 +97,6 @@ func (d DHCPServer) String() string { // Equal compares two DHCPServer instances func (d DHCPServer) Equal(d2 DHCPServer, withStaticEntries bool) bool { return netutils.EqualIPNets(d.Subnet, d2.Subnet) && - d.AllOnesNetmask == d2.AllOnesNetmask && netutils.EqualIPs(d.IPRange.FromIP, d2.IPRange.FromIP) && netutils.EqualIPs(d.IPRange.ToIP, d2.IPRange.ToIP) && netutils.EqualIPs(d.GatewayIP, d2.GatewayIP) && @@ -637,11 +630,6 @@ func (c *DnsmasqConfigurator) CreateDnsmasqConfig(buffer io.Writer, dnsmasq Dnsm if subnet != nil { ipv4Netmask = net.IP(subnet.Mask).String() altIPv4Netmask := ipv4Netmask - if gatewayIP != nil && dnsmasq.DHCPServer.AllOnesNetmask { - // Network prefix "255.255.255.255" will force packets to go through - // dom0 virtual router that makes the packets pass through ACLs and flow log. - altIPv4Netmask = "255.255.255.255" - } if _, err := io.WriteString(buffer, fmt.Sprintf("dhcp-option=option:netmask,%s\n", altIPv4Netmask)); err != nil { return writeErr(err) @@ -651,29 +639,12 @@ func (c *DnsmasqConfigurator) CreateDnsmasqConfig(buffer io.Writer, dnsmasq Dnsm // Prepare the set of all static routes to propagate to applications. var staticRoutes []IPRoute if !isIPv6 { - if dnsmasq.DHCPServer.AllOnesNetmask { - // Make the gateway reachable using a connected route. - if gatewayIP != nil { - staticRoutes = append(staticRoutes, IPRoute{ - DstNetwork: netutils.HostSubnet(gatewayIP), - Gateway: net.IP{0, 0, 0, 0}, - }) - } - if subnet != nil && gatewayIP != nil { - staticRoutes = append(staticRoutes, IPRoute{ - DstNetwork: subnet, - Gateway: gatewayIP, - }) - } - } else { - // With all-ones netmask disabled we use forwarding between apps - // on the same NI. - if subnet != nil { - staticRoutes = append(staticRoutes, IPRoute{ - DstNetwork: subnet, - Gateway: net.IP{0, 0, 0, 0}, - }) - } + // Use L2-forwarding between apps on the same NI. + if subnet != nil { + staticRoutes = append(staticRoutes, IPRoute{ + DstNetwork: subnet, + Gateway: net.IP{0, 0, 0, 0}, + }) } staticRoutes = append(staticRoutes, dnsmasq.DHCPServer.PropagateRoutes...) } @@ -717,9 +688,9 @@ func (c *DnsmasqConfigurator) CreateDnsmasqConfig(buffer io.Writer, dnsmasq Dnsm } } - // For flow-logging purposes, we prefer routing inside the host over forwarding, - // even between apps on the same subnet. The only exception is when all-ones netmask - // for app interfaces is disabled, then we are unable to enforce routing between apps. + // Apply static routes to all endpoints and separately to individual gateways. + // This is to make sure that gateway-app will not receive route that uses app's + // own local IP as the gateway. var appGateways []net.IP for _, ipRoute := range staticRoutes { if subnet != nil && subnet.Contains(ipRoute.Gateway) && @@ -728,30 +699,10 @@ func (c *DnsmasqConfigurator) CreateDnsmasqConfig(buffer io.Writer, dnsmasq Dnsm } } appGateways = generics.FilterDuplicatesFn(appGateways, netutils.EqualIPs) - enforceRouting := func(route IPRoute) IPRoute { - if gatewayIP == nil || !dnsmasq.DHCPServer.AllOnesNetmask { - // Forwarding from app to app is inevitable. - return route - } - if !generics.ContainsItemFn(appGateways, route.Gateway, netutils.EqualIPs) { - // Not from app to app (i.e. already routed). - return route - } - // Traffic will go: app->bridge(L3)->app - return IPRoute{ - DstNetwork: route.DstNetwork, - Gateway: gatewayIP, - } - } - - // Apply static routes to all endpoints and separately to individual gateways. - // This is to make sure that gateway-app will not receive route that uses app's - // own local IP as the gateway. - epRoutes := generics.MapList(staticRoutes, enforceRouting) - if len(epRoutes) > 0 { + if len(staticRoutes) > 0 { if _, err := io.WriteString(buffer, fmt.Sprintf("dhcp-option=tag:%s,option:classless-static-route,%s\n", - endpointTag, c.formatRoutesForConfig(epRoutes))); err != nil { + endpointTag, c.formatRoutesForConfig(staticRoutes))); err != nil { return writeErr(err) } } @@ -761,7 +712,6 @@ func (c *DnsmasqConfigurator) CreateDnsmasqConfig(buffer io.Writer, dnsmasq Dnsm return !netutils.EqualIPs(route.Gateway, appGatewayIP) } gwRoutes := generics.FilterList(staticRoutes, isRouteValid) - gwRoutes = generics.MapList(gwRoutes, enforceRouting) if len(gwRoutes) > 0 { if _, err := io.WriteString(buffer, fmt.Sprintf("dhcp-option=tag:%s,option:classless-static-route,%s\n", diff --git a/pkg/pillar/nireconciler/genericitems/dnsmasq_test.go b/pkg/pillar/nireconciler/genericitems/dnsmasq_test.go index 96d0666a3d..459c22fc88 100644 --- a/pkg/pillar/nireconciler/genericitems/dnsmasq_test.go +++ b/pkg/pillar/nireconciler/genericitems/dnsmasq_test.go @@ -36,8 +36,7 @@ func exampleDnsmasqParams() genericitems.Dnsmasq { dnsmasq.ListenIf.IfName = "br0" _, subnet, _ := net.ParseCIDR("10.0.0.0/24") dnsmasq.DHCPServer = genericitems.DHCPServer{ - Subnet: subnet, - AllOnesNetmask: true, + Subnet: subnet, IPRange: genericitems.IPRange{ FromIP: net.IP{10, 0, 0, 2}, ToIP: net.IP{10, 0, 0, 123}, @@ -156,10 +155,10 @@ hostsdir=/run/zedrouter/hosts.br0 dhcp-hostsdir=/run/zedrouter/dhcp-hosts.br0 dhcp-option=option:dns-server,10.0.0.1,1.1.1.1 dhcp-option=option:ntp-server,94.130.35.4,94.16.114.254 -dhcp-option=option:netmask,255.255.255.255 +dhcp-option=option:netmask,255.255.255.0 dhcp-option=option:router,10.0.0.1 -dhcp-option=tag:endpoint,option:classless-static-route,10.0.0.1/32,0.0.0.0,10.0.0.0/24,10.0.0.1,192.168.1.0/24,10.0.0.1,172.30.0.0/16,10.0.0.1,0.0.0.0/0,10.0.0.1 -dhcp-option=tag:gateway-10-0-0-100,option:classless-static-route,10.0.0.1/32,0.0.0.0,10.0.0.0/24,10.0.0.1,192.168.1.0/24,10.0.0.1,0.0.0.0/0,10.0.0.1 +dhcp-option=tag:endpoint,option:classless-static-route,10.0.0.0/24,0.0.0.0,192.168.1.0/24,10.0.0.1,172.30.0.0/16,10.0.0.100,0.0.0.0/0,10.0.0.1 +dhcp-option=tag:gateway-10-0-0-100,option:classless-static-route,10.0.0.0/24,0.0.0.0,192.168.1.0/24,10.0.0.1,0.0.0.0/0,10.0.0.1 dhcp-range=10.0.0.2,10.0.0.123,255.255.255.0,60m ` if configExpected != config { @@ -210,23 +209,6 @@ func TestCreateDnsmasqConfigWithoutGateway(t *testing.T) { } } -func TestCreateDnsmasqConfigWithDisabledAllOnesNetmask(t *testing.T) { - t.Parallel() - - dnsmasq := exampleDnsmasqParams() - dnsmasq.DHCPServer.AllOnesNetmask = false - config := createDnsmasqConfig(dnsmasq) - - dhcpRangeRex := "(?m)^dhcp-range=10.0.0.2,10.0.0.123,255.255.255.0,60m$" - ok, err := regexp.MatchString(dhcpRangeRex, config) - if err != nil { - panic(err) - } - if !ok { - t.Fatalf("expected to match '%s', but got '%s'", dhcpRangeRex, config) - } -} - func TestRunDnsmasqInvalidDhcpRange(t *testing.T) { t.Parallel() line, err := configurator.CreateDHCPv4RangeConfig(nil, nil) diff --git a/pkg/pillar/nireconciler/linux_config.go b/pkg/pillar/nireconciler/linux_config.go index 293d15a6b1..515217d8e4 100644 --- a/pkg/pillar/nireconciler/linux_config.go +++ b/pkg/pillar/nireconciler/linux_config.go @@ -746,7 +746,7 @@ func (r *LinuxNIReconciler) getIntendedNIL3Cfg(niID uuid.UUID) dg.Graph { continue } isAppGW := gateway != nil && r.getNISubnet(ni).Contains(gateway) - if isAppGW && r.disableAllOnesNetmask { + if isAppGW { // Route is not needed inside the host, traffic is just forwarded // by the bridge. continue @@ -1055,8 +1055,7 @@ func (r *LinuxNIReconciler) getIntendedDnsmasqCfg(niID uuid.UUID) (items []dg.It } propagateRoutes = generics.FilterDuplicatesFn(propagateRoutes, generic.EqualIPRoutes) dhcpCfg := generic.DHCPServer{ - Subnet: r.getNISubnet(ni), - AllOnesNetmask: !r.disableAllOnesNetmask, + Subnet: r.getNISubnet(ni), IPRange: generic.IPRange{ FromIP: ni.config.DhcpRange.Start, ToIP: ni.config.DhcpRange.End, diff --git a/pkg/pillar/nireconciler/linux_reconciler.go b/pkg/pillar/nireconciler/linux_reconciler.go index f0eaedfdc4..23fc8d0ba1 100644 --- a/pkg/pillar/nireconciler/linux_reconciler.go +++ b/pkg/pillar/nireconciler/linux_reconciler.go @@ -67,9 +67,6 @@ type LinuxNIReconciler struct { exportIntendedState bool withKubernetesNetworking bool - // From GCP - disableAllOnesNetmask bool - reconcileMu sync.Mutex currentState dg.Graph intendedState dg.Graph @@ -801,25 +798,7 @@ func (r *LinuxNIReconciler) ResumeReconcile(ctx context.Context) { // ApplyUpdatedGCP : apply change in the global config properties. func (r *LinuxNIReconciler) ApplyUpdatedGCP(ctx context.Context, newGCP types.ConfigItemValueMap) { - contWatcher := r.pauseWatcher() - defer contWatcher() - disableAllOnesNetmask := newGCP.GlobalValueBool(types.DisableDHCPAllOnesNetMask) - if r.disableAllOnesNetmask == disableAllOnesNetmask { - // No change in GCP relevant for network instances. - return - } - r.disableAllOnesNetmask = disableAllOnesNetmask - for niID, ni := range r.nis { - if ni.config.Type == types.NetworkInstanceTypeSwitch { - // Not running DHCP server for switch NI inside EVE. - continue - } - r.scheduleNICfgRebuild(niID, - fmt.Sprintf("global config property %s changed to %t", - types.DisableDHCPAllOnesNetMask, r.disableAllOnesNetmask)) - } - updates := r.reconcile(ctx) - r.publishReconcilerUpdates(updates...) + // NOOP } // AddNI : create this new network instance inside the network stack. diff --git a/pkg/pillar/nireconciler/linux_test.go b/pkg/pillar/nireconciler/linux_test.go index 4c6ac12332..ad95a1dcf1 100644 --- a/pkg/pillar/nireconciler/linux_test.go +++ b/pkg/pillar/nireconciler/linux_test.go @@ -2067,42 +2067,6 @@ func TestIPv4LocalAndSwitchNIsWithFlowlogging(test *testing.T) { ni5Config.EnableFlowlog = false } -func TestDisableAllOnesMask(test *testing.T) { - t := initTest(test, false) - networkMonitor.AddOrUpdateInterface(eth0) - networkMonitor.UpdateRoutes(eth0Routes) - ctx := reconciler.MockRun(context.Background()) - updatesCh := niReconciler.WatchReconcilerUpdates() - niReconciler.RunInitialReconcile(ctx) - - // Create local network instance. - _, err := niReconciler.AddNI(ctx, ni1Config, ni1Bridge) - t.Expect(err).ToNot(HaveOccurred()) - var recUpdate nirec.ReconcilerUpdate - t.Eventually(updatesCh).Should(Receive(&recUpdate)) - t.Expect(recUpdate.UpdateType).To(Equal(nirec.NIReconcileStatusChanged)) - networkMonitor.AddOrUpdateInterface(ni1BridgeIf) - - // dnsmasq should advertise mask with all bits set to one. - dnsmasqConf := itemDescription(dg.Reference( - genericitems.Dnsmasq{ListenIf: genericitems.NetworkIf{IfName: "bn1"}})) - t.Expect(dnsmasqConf).To(ContainSubstring("allOnesNetmask: true")) - - // Update global config to disable all ones mask. - gcp := types.DefaultConfigItemValueMap() - gcp.SetGlobalValueBool(types.DisableDHCPAllOnesNetMask, true) - niReconciler.ApplyUpdatedGCP(ctx, *gcp) - - // dnsmasq should now use the mask configured for the NI subnet. - dnsmasqConf = itemDescription(dg.Reference( - genericitems.Dnsmasq{ListenIf: genericitems.NetworkIf{IfName: "bn1"}})) - t.Expect(dnsmasqConf).To(ContainSubstring("allOnesNetmask: false")) - - // Delete network instance - _, err = niReconciler.DelNI(ctx, ni1UUID.UUID) - t.Expect(err).ToNot(HaveOccurred()) -} - func TestIPv6LocalAndSwitchNIs(test *testing.T) { t := initTest(test, false) networkMonitor.AddOrUpdateInterface(keth2) @@ -2767,7 +2731,9 @@ func TestStaticAndConnectedRoutes(test *testing.T) { } recStatus, err := niReconciler.UpdateNI(ctx, ni5Config, ni5Bridge) t.Expect(err).ToNot(HaveOccurred()) - t.Expect(recStatus.Routes).To(HaveLen(5)) + // The list does not include routes with application as a gateway. In those cases, + // the traffic is simply forwarded by the bridge, not routed. + t.Expect(recStatus.Routes).To(HaveLen(3)) t.Expect(recStatus.Routes[0].Equal(types.IPRouteInfo{ IPVersion: types.AddressTypeIPV4, DstNetwork: ipAddressWithPrefix("0.0.0.0/0"), @@ -2775,24 +2741,12 @@ func TestStaticAndConnectedRoutes(test *testing.T) { OutputPort: "ethernet1", })).To(BeTrue()) t.Expect(recStatus.Routes[1].Equal(types.IPRouteInfo{ - IPVersion: types.AddressTypeIPV4, - DstNetwork: ipAddressWithPrefix("10.50.5.0/30"), - Gateway: ipAddress("10.10.20.2"), - GatewayApp: app2UUID.UUID, - })).To(BeTrue()) - t.Expect(recStatus.Routes[2].Equal(types.IPRouteInfo{ - IPVersion: types.AddressTypeIPV4, - DstNetwork: ipAddressWithPrefix("10.50.19.0/30"), - Gateway: ipAddress("10.10.20.2"), - GatewayApp: app2UUID.UUID, - })).To(BeTrue()) - t.Expect(recStatus.Routes[3].Equal(types.IPRouteInfo{ IPVersion: types.AddressTypeIPV4, DstNetwork: ipAddressWithPrefix("10.50.14.0/26"), Gateway: ipAddress("172.30.30.15"), OutputPort: "ethernet3", })).To(BeTrue()) - t.Expect(recStatus.Routes[4].Equal(types.IPRouteInfo{ + t.Expect(recStatus.Routes[2].Equal(types.IPRouteInfo{ IPVersion: types.AddressTypeIPV4, DstNetwork: ipAddressWithPrefix("10.50.1.0/24"), Gateway: ipAddress("172.20.1.1"), @@ -2848,14 +2802,14 @@ func TestStaticAndConnectedRoutes(test *testing.T) { return false } return route.Table == 801 - })).To(Equal(2 + 1)) // subnet route + unreachable route + 1 static route + })).To(Equal(2)) // only IPv4 + IPv6 unreachable routes (N1 is air-gapped) t.Expect(itemCount(func(item dg.Item) bool { route, isRoute := item.(linuxitems.Route) if !isRoute { return false } return route.Table == 805 - })).To(Equal(2 + 5)) // + 5 static routes + })).To(Equal(2 + 3)) // unreachable IPv4/IPv6 routes + static routes // Disconnect the application. appStatus, err = niReconciler.DelAppConn(ctx, app2UUID.UUID) diff --git a/pkg/pillar/types/global.go b/pkg/pillar/types/global.go index 533e208dc2..ffed09c432 100644 --- a/pkg/pillar/types/global.go +++ b/pkg/pillar/types/global.go @@ -298,9 +298,6 @@ const ( // FmlCustomResolution global setting key FmlCustomResolution GlobalSettingKey = "app.fml.resolution" - // XXX Temporary flag to disable RFC 3442 classless static route usage - DisableDHCPAllOnesNetMask GlobalSettingKey = "debug.disable.dhcp.all-ones.netmask" - // ProcessCloudInitMultiPart to help VMs which do not handle mime multi-part themselves ProcessCloudInitMultiPart GlobalSettingKey = "process.cloud-init.multipart" @@ -963,7 +960,6 @@ func NewConfigItemSpecMap() ConfigItemSpecMap { configItemSpecMap.AddBoolItem(IgnoreMemoryCheckForApps, false) configItemSpecMap.AddBoolItem(IgnoreDiskCheckForApps, false) configItemSpecMap.AddBoolItem(AllowLogFastupload, false) - configItemSpecMap.AddBoolItem(DisableDHCPAllOnesNetMask, false) configItemSpecMap.AddBoolItem(ProcessCloudInitMultiPart, false) configItemSpecMap.AddBoolItem(ConsoleAccess, true) // Controller likely default to false configItemSpecMap.AddBoolItem(VncShimVMAccess, false) diff --git a/pkg/pillar/types/global_test.go b/pkg/pillar/types/global_test.go index 444b41917c..cf30268701 100644 --- a/pkg/pillar/types/global_test.go +++ b/pkg/pillar/types/global_test.go @@ -207,7 +207,6 @@ func TestNewConfigItemSpecMap(t *testing.T) { SyslogLogLevel, KernelLogLevel, FmlCustomResolution, - DisableDHCPAllOnesNetMask, ProcessCloudInitMultiPart, NetDumpEnable, NetDumpTopicMaxCount,