Revert "[netstack] Use optional field for the gateway in RouteTableEntry"
This reverts commit a39843a1ac9db29314069bcf382ec25802ed7c9f.
Reason for revert: the FIDL change breaks the Chromium prebuilts which blocks the garnet roller.
Original change's description:
> [netstack] Use optional field for the gateway in RouteTableEntry
>
> This changes the FIDL RouteTableEntry field for the gateway to an optional
> field and adds code to handle this properly.
>
> This broke recently when we moved to IP Addresses implemented as a union in
> FIDL. Before that we used a struct that had an enum NetAddressFamily that
> supports "UNSPECIFIED".
>
> This also fixes a couple of nits in ifconfig.
>
> NET-2059 #done
>
> Test: CQ and manual tests creating/deleting/showing routes
> Change-Id: Iceed3a4260a0143e9f4bfdc42ad88ba70fa98294
TBR=stijlist@google.com,tamird@google.com,eyalsoha@google.com,ckuiper@google.com
Change-Id: I32a9f53144a30683ba9925fbb8a78ce121cd097e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
diff --git a/go/src/netstack/ifconfig/ifconfig.go b/go/src/netstack/ifconfig/ifconfig.go
index 47c1d6f..6287aa2 100644
--- a/go/src/netstack/ifconfig/ifconfig.go
+++ b/go/src/netstack/ifconfig/ifconfig.go
@@ -117,8 +117,7 @@
var attr, val string
switch attr, val, remaining = args[0], args[1], args[2:]; attr {
case "gateway":
- gateway := toIpAddress(net.ParseIP(val))
- in.Gateway = &gateway
+ in.Gateway = toIpAddress(net.ParseIP(val))
case "iface":
ifaces, err := a.netstack.GetInterfaces()
if err != nil {
@@ -157,10 +156,6 @@
}
}
- if route.Gateway == nil && route.Nicid == 0 {
- return route, fmt.Errorf("either gateway or iface must be provided when adding a route")
- }
-
if len(remaining) != 0 {
return route, fmt.Errorf("could not parse all route arguments. remaining: %s", remaining)
}
@@ -169,6 +164,9 @@
}
func (a *netstackClientApp) addRoute(r netstack.RouteTableEntry) error {
+ if (r.Gateway == netfidl.IpAddress{}) && r.Nicid == 0 {
+ return fmt.Errorf("either gateway or iface must be provided when adding a route")
+ }
req, transactionInterface, err := netstack.NewRouteTableTransactionInterfaceRequest()
if err != nil {
return fmt.Errorf("could not make a new route table transaction: %s", err)
@@ -217,10 +215,10 @@
if !equalIpAddress(target.Netmask, source.Netmask) {
return false
}
- if target.Gateway != nil {
- if source.Gateway == nil || !equalIpAddress(*source.Gateway, *target.Gateway) {
- return false
- }
+ if target.Gateway.Which() != 0 &&
+ !equalIpAddress(source.Gateway, target.Gateway) {
+ // The gateway is neither wildcard nor a match.
+ return false
}
if target.Nicid != 0 && source.Nicid != target.Nicid {
// The Nicid is neither wildcard nor a match.
@@ -272,10 +270,7 @@
case netfidl.IpAddressIpv6:
netAndMask = net.IPNet{IP: r.Destination.Ipv6.Addr[:], Mask: r.Netmask.Ipv6.Addr[:]}
}
- if r.Gateway != nil {
- return fmt.Sprintf("%s via %s %s", netAndMask.String(), netAddrToString(*r.Gateway), ifaceName)
- }
- return fmt.Sprintf("%s via %s", netAndMask.String(), ifaceName)
+ return fmt.Sprintf("%s via %s %s", netAndMask.String(), netAddrToString(r.Gateway), ifaceName)
}
func (a *netstackClientApp) showRoutes() error {
@@ -453,7 +448,7 @@
fmt.Printf(" %s [<interface>] [up|down]\n", os.Args[0])
fmt.Printf(" %s [<interface>] [add|del] [<address>]/[<mask>]\n", os.Args[0])
fmt.Printf(" %s [<interface>] dhcp [start|stop]\n", os.Args[0])
- fmt.Printf(" %s route [add|del] [<address>/<mask>] [iface <name>] [gateway <address>]\n", os.Args[0])
+ fmt.Printf(" %s route [add|del] [<address>/<mask>] [iface <name>] [gateway <address>/<mask>]\n", os.Args[0])
fmt.Printf(" %s route show\n", os.Args[0])
fmt.Printf(" %s bridge [<interface>]+\n", os.Args[0])
os.Exit(1)
diff --git a/go/src/netstack/ifconfig/ifconfig_test.go b/go/src/netstack/ifconfig/ifconfig_test.go
index 8da1fb7..f1eaeb8 100644
--- a/go/src/netstack/ifconfig/ifconfig_test.go
+++ b/go/src/netstack/ifconfig/ifconfig_test.go
@@ -97,7 +97,7 @@
if err == nil {
t.Errorf("ifconfig route add returned success with neither gateway or iface specified. output: \n%s", out)
}
- expected = "Error parsing route"
+ expected = "Error adding route"
if !strings.Contains(string(out), expected) {
t.Errorf("want `ifconfig route add` to print \"%s\", got \"%s\"", expected, out)
}
diff --git a/go/src/netstack/netstack_service.go b/go/src/netstack/netstack_service.go
index f6b77af..a43044a 100644
--- a/go/src/netstack/netstack_service.go
+++ b/go/src/netstack/netstack_service.go
@@ -134,25 +134,26 @@
l = len(route.Destination)
} else if len(route.Mask) > 0 {
l = len(route.Destination)
+ } else if len(route.Gateway) > 0 {
+ l = len(route.Gateway)
}
dest := route.Destination
mask := route.Mask
+ gateway := route.Gateway
if len(dest) == 0 {
dest = tcpip.Address(strings.Repeat("\x00", l))
}
if len(mask) == 0 {
mask = tcpip.AddressMask(strings.Repeat("\x00", l))
}
-
- var gatewayPtr *net.IpAddress
- if len(route.Gateway) != 0 {
- gateway := fidlconv.ToNetIpAddress(route.Gateway)
- gatewayPtr = &gateway
+ if len(gateway) == 0 {
+ gateway = tcpip.Address(strings.Repeat("\x00", l))
}
+
out = append(out, netstack.RouteTableEntry{
Destination: fidlconv.ToNetIpAddress(dest),
Netmask: fidlconv.ToNetIpAddress(tcpip.Address(mask)),
- Gateway: gatewayPtr,
+ Gateway: fidlconv.ToNetIpAddress(gateway),
Nicid: uint32(route.NIC),
})
}
@@ -162,14 +163,10 @@
func routeTableToNs(rt []netstack.RouteTableEntry) []tcpip.Route {
routes := make([]tcpip.Route, 0, len(rt))
for _, r := range rt {
- var gateway tcpip.Address
- if r.Gateway != nil {
- gateway = fidlconv.ToTCPIPAddress(*r.Gateway)
- }
routes = append(routes, tcpip.Route{
Destination: fidlconv.ToTCPIPAddress(r.Destination),
Mask: tcpip.AddressMask(fidlconv.ToTCPIPAddress(r.Netmask)),
- Gateway: gateway,
+ Gateway: fidlconv.ToTCPIPAddress(r.Gateway),
NIC: tcpip.NICID(r.Nicid),
})
}
diff --git a/go/src/netstack/netstack_service_impl_test.go b/go/src/netstack/netstack_service_impl_test.go
index 6cf42af..140027b 100644
--- a/go/src/netstack/netstack_service_impl_test.go
+++ b/go/src/netstack/netstack_service_impl_test.go
@@ -63,12 +63,10 @@
destinationAddress, destinationSubnet, err := net.ParseCIDR("1.2.3.4/24")
AssertNoError(t, err)
gatewayAddress, _, err := net.ParseCIDR("5.6.7.8/32")
- AssertNoError(t, err)
- gateway := toIpAddress(gatewayAddress)
newRouteTableEntry := netstack.RouteTableEntry{
Destination: toIpAddress(destinationAddress),
Netmask: toIpAddress(net.IP(destinationSubnet.Mask)),
- Gateway: &gateway,
+ Gateway: toIpAddress(gatewayAddress),
}
newRouteTable := append(rs, newRouteTableEntry)
diff --git a/go/src/netstack/netstat/netstat.go b/go/src/netstack/netstat/netstat.go
index e5d4073..0d43c54 100644
--- a/go/src/netstack/netstat/netstat.go
+++ b/go/src/netstack/netstat/netstat.go
@@ -147,20 +147,13 @@
return
}
for _, entry := range entries {
- if entry.Gateway != nil && netAddressZero(*entry.Gateway) {
- panic("Gateway IP cannot be ANY ('0.0.0.0' or '::'")
- }
if netAddressZero(entry.Destination) {
- if entry.Gateway != nil {
- fmt.Printf("default via %s, ", netAddressToString(*entry.Gateway))
- } else {
- fmt.Printf("default through ")
- }
+ fmt.Printf("default via %s, ", netAddressToString(entry.Gateway))
} else {
fmt.Printf("Destination: %s, ", netAddressToString(entry.Destination))
fmt.Printf("Mask: %s, ", netAddressToString(entry.Netmask))
- if entry.Gateway != nil {
- fmt.Printf("Gateway: %s, ", netAddressToString(*entry.Gateway))
+ if !netAddressZero(entry.Gateway) {
+ fmt.Printf("Gateway: %s, ", netAddressToString(entry.Gateway))
}
}
fmt.Printf("NICID: %d\n", entry.Nicid)
diff --git a/public/fidl/fuchsia.netstack/netstack.fidl b/public/fidl/fuchsia.netstack/netstack.fidl
index 525a526..de994a2 100644
--- a/public/fidl/fuchsia.netstack/netstack.fidl
+++ b/public/fidl/fuchsia.netstack/netstack.fidl
@@ -118,7 +118,7 @@
struct RouteTableEntry {
fuchsia.net.IpAddress destination;
fuchsia.net.IpAddress netmask;
- fuchsia.net.IpAddress? gateway;
+ fuchsia.net.IpAddress gateway;
uint32 nicid;
};
