shill: connection: Fix FixGatewayReachability to handle P-t-P
...and add more thorough unit tests for it.
BUG=chromium-os:30052
TEST=New unit tests
Change-Id: Ic06ab2f84893e63f3b7d3f04c17f71ee4fa2d4b4
Reviewed-on: https://gerrit.chromium.org/gerrit/21202
Reviewed-by: Darin Petkov <petkov@chromium.org>
Commit-Ready: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
diff --git a/connection_unittest.cc b/connection_unittest.cc
index 5c74f28..872b343 100644
--- a/connection_unittest.cc
+++ b/connection_unittest.cc
@@ -395,4 +395,52 @@
FlushAddresses(kTestDeviceInterfaceIndex0));
}
+TEST_F(ConnectionTest, FixGatewayReachability) {
+ static const char kLocal[] = "10.242.2.13";
+ IPAddress local(IPAddress::kFamilyIPv4);
+ ASSERT_TRUE(local.SetAddressFromString(kLocal));
+ const int kPrefix = 24;
+ local.set_prefix(kPrefix);
+ IPAddress gateway(IPAddress::kFamilyIPv4);
+ IPAddress peer(IPAddress::kFamilyIPv4);
+
+ // Should fail because no gateway is set.
+ EXPECT_FALSE(Connection::FixGatewayReachability(&local, gateway, peer));
+ EXPECT_EQ(kPrefix, local.prefix());
+
+ // Should succeed because with the given prefix, this gateway is reachable.
+ static const char kReachableGateway[] = "10.242.2.14";
+ ASSERT_TRUE(gateway.SetAddressFromString(kReachableGateway));
+ EXPECT_TRUE(Connection::FixGatewayReachability(&local, gateway, peer));
+ // Prefix should remain unchanged.
+ EXPECT_EQ(kPrefix, local.prefix());
+
+ // Should succeed because we modified the prefix to match the gateway.
+ static const char kExpandableGateway[] = "10.242.3.14";
+ ASSERT_TRUE(gateway.SetAddressFromString(kExpandableGateway));
+ EXPECT_TRUE(Connection::FixGatewayReachability(&local, gateway, peer));
+ // Prefix should have opened up by 1 bit.
+ EXPECT_EQ(kPrefix - 1, local.prefix());
+
+ // Should fail because we cannot plausibly expand the prefix past 8.
+ local.set_prefix(kPrefix);
+ static const char kUnreachableGateway[] = "11.242.2.14";
+ ASSERT_TRUE(gateway.SetAddressFromString(kUnreachableGateway));
+ EXPECT_FALSE(Connection::FixGatewayReachability(&local, gateway, peer));
+ // Prefix should not have changed.
+ EXPECT_EQ(kPrefix, local.prefix());
+
+ // However, if this is a peer-to-peer interface and the peer matches
+ // the gateway, we should succeed.
+ ASSERT_TRUE(peer.SetAddressFromString(kUnreachableGateway));
+ EXPECT_TRUE(Connection::FixGatewayReachability(&local, gateway, peer));
+ EXPECT_EQ(kPrefix, local.prefix());
+
+ // If there is a peer specified and it does not match the gateway (even
+ // if it was reachable via netmask), we should fail.
+ ASSERT_TRUE(gateway.SetAddressFromString(kReachableGateway));
+ EXPECT_FALSE(Connection::FixGatewayReachability(&local, gateway, peer));
+ EXPECT_EQ(kPrefix, local.prefix());
+}
+
} // namespace shill