patchpanel: Account for long ifnames
This patch modifies the veth ifname naming function for ARC
container to account for longer device names that, when prefixed, exceed
IFNAMSIZ.
The 'veth' prefix itself must still be used since that's what shill is
paying attention to.
BUG=b:155453410
TEST=units, manual inspection and connectivity check
Change-Id: I3e0abcced0d8a6df1782093e682212a7ce2ae078
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2192631
Tested-by: Garrick Evans <garrick@chromium.org>
Commit-Queue: Garrick Evans <garrick@chromium.org>
Reviewed-by: Taoyu Li <taoyl@chromium.org>
Reviewed-by: Hugo Benichi <hugobenichi@google.com>
diff --git a/patchpanel/arc_service_test.cc b/patchpanel/arc_service_test.cc
index 6897956..52236e3 100644
--- a/patchpanel/arc_service_test.cc
+++ b/patchpanel/arc_service_test.cc
@@ -246,14 +246,14 @@
EXPECT_CALL(*datapath_, AddBridge(StrEq("arcbr0"), kArcHostIP, 30))
.WillOnce(Return(true));
EXPECT_CALL(*datapath_,
- AddVirtualInterfacePair(StrEq("veth_arc0"), StrEq("arc0")))
+ AddVirtualInterfacePair(StrEq("vetharc0"), StrEq("arc0")))
.WillOnce(Return(true));
EXPECT_CALL(*datapath_, ConfigureInterface(StrEq("arc0"), _, kArcGuestIP,
30, true, _))
.WillOnce(Return(true));
- EXPECT_CALL(*datapath_, ToggleInterface(StrEq("veth_arc0"), true))
+ EXPECT_CALL(*datapath_, ToggleInterface(StrEq("vetharc0"), true))
.WillOnce(Return(true));
- EXPECT_CALL(*datapath_, AddToBridge(StrEq("arcbr0"), StrEq("veth_arc0")))
+ EXPECT_CALL(*datapath_, AddToBridge(StrEq("arcbr0"), StrEq("vetharc0")))
.WillOnce(Return(true));
impl->Start(kTestPID);
@@ -284,14 +284,14 @@
EXPECT_CALL(*datapath_, AddBridge(StrEq("arcbr0"), kArcHostIP, 30))
.WillOnce(Return(true));
EXPECT_CALL(*datapath_,
- AddVirtualInterfacePair(StrEq("veth_arc0"), StrEq("arc0")))
+ AddVirtualInterfacePair(StrEq("vetharc0"), StrEq("arc0")))
.WillOnce(Return(true));
EXPECT_CALL(*datapath_,
ConfigureInterface(StrEq("arc0"), _, kArcGuestIP, 30, true, _))
.WillOnce(Return(true));
- EXPECT_CALL(*datapath_, ToggleInterface(StrEq("veth_arc0"), true))
+ EXPECT_CALL(*datapath_, ToggleInterface(StrEq("vetharc0"), true))
.WillOnce(Return(true));
- EXPECT_CALL(*datapath_, AddToBridge(StrEq("arcbr0"), StrEq("veth_arc0")))
+ EXPECT_CALL(*datapath_, AddToBridge(StrEq("arcbr0"), StrEq("vetharc0")))
.WillOnce(Return(true));
EXPECT_CALL(forwarder_, StartForwarding(_, _, _, _)).Times(0);
Impl(false)->Start(kTestPID);
@@ -301,7 +301,7 @@
EXPECT_CALL(*datapath_, AddBridge(StrEq("arcbr0"), kArcHostIP, 30))
.WillOnce(Return(true));
EXPECT_CALL(*datapath_,
- AddVirtualInterfacePair(StrEq("veth_arc0"), StrEq("arc0")))
+ AddVirtualInterfacePair(StrEq("vetharc0"), StrEq("arc0")))
.WillOnce(Return(false));
EXPECT_CALL(*datapath_, ConfigureInterface(_, _, _, _, _, _)).Times(0);
EXPECT_CALL(*datapath_, RemoveBridge(_)).Times(0);
@@ -312,12 +312,12 @@
EXPECT_CALL(*datapath_, AddBridge(StrEq("arcbr0"), kArcHostIP, 30))
.WillOnce(Return(true));
EXPECT_CALL(*datapath_,
- AddVirtualInterfacePair(StrEq("veth_arc0"), StrEq("arc0")))
+ AddVirtualInterfacePair(StrEq("vetharc0"), StrEq("arc0")))
.WillOnce(Return(true));
EXPECT_CALL(*datapath_,
ConfigureInterface(StrEq("arc0"), _, kArcGuestIP, 30, true, _))
.WillOnce(Return(false));
- EXPECT_CALL(*datapath_, ToggleInterface(StrEq("veth_arc0"), true)).Times(0);
+ EXPECT_CALL(*datapath_, ToggleInterface(StrEq("vetharc0"), true)).Times(0);
EXPECT_CALL(*datapath_, RemoveInterface(StrEq("arc0")));
EXPECT_CALL(*datapath_, RemoveBridge(_)).Times(0);
Impl(false)->Start(kTestPID);
@@ -325,15 +325,15 @@
TEST_F(ContainerImplTest, OnStartDevice) {
EXPECT_CALL(*datapath_,
- AddVirtualInterfacePair(StrEq("veth_eth0"), StrEq("eth0")))
+ AddVirtualInterfacePair(StrEq("vetheth0"), StrEq("eth0")))
.WillOnce(Return(true));
EXPECT_CALL(*datapath_,
ConfigureInterface(StrEq("eth0"), _, Ipv4Addr(100, 115, 92, 10),
30, true, _))
.WillOnce(Return(true));
- EXPECT_CALL(*datapath_, ToggleInterface(StrEq("veth_eth0"), true))
+ EXPECT_CALL(*datapath_, ToggleInterface(StrEq("vetheth0"), true))
.WillOnce(Return(true));
- EXPECT_CALL(*datapath_, AddToBridge(StrEq("arc_eth0"), StrEq("veth_eth0")))
+ EXPECT_CALL(*datapath_, AddToBridge(StrEq("arc_eth0"), StrEq("vetheth0")))
.WillOnce(Return(true));
EXPECT_CALL(forwarder_,
StartForwarding(StrEq("eth0"), StrEq("arc_eth0"), _, _));
@@ -345,14 +345,14 @@
TEST_F(ContainerImplTest, Stop) {
EXPECT_CALL(*datapath_,
MaskInterfaceFlags(StrEq("arcbr0"), IFF_DEBUG, IFF_UP));
- EXPECT_CALL(*datapath_, RemoveInterface(StrEq("veth_arc0")));
+ EXPECT_CALL(*datapath_, RemoveInterface(StrEq("vetharc0")));
EXPECT_CALL(forwarder_, StopForwarding(_, _, _, _)).Times(0);
Impl()->Stop(kTestPID);
}
TEST_F(ContainerImplTest, OnStopDevice) {
- EXPECT_CALL(*datapath_, RemoveInterface(StrEq("veth_eth0")));
+ EXPECT_CALL(*datapath_, RemoveInterface(StrEq("vetheth0")));
EXPECT_CALL(forwarder_,
StopForwarding(StrEq("eth0"), StrEq("arc_eth0"), _, _));
diff --git a/patchpanel/datapath.cc b/patchpanel/datapath.cc
index 7670730..3f2c5f4 100644
--- a/patchpanel/datapath.cc
+++ b/patchpanel/datapath.cc
@@ -34,12 +34,17 @@
} // namespace
-std::string ArcVethHostName(std::string ifname) {
- return "veth_" + ifname;
-}
+std::string ArcVethHostName(const std::string& ifname) {
+ std::string n = "veth" + ifname;
+ if (n.length() < IFNAMSIZ)
+ return n;
-std::string ArcVethPeerName(std::string ifname) {
- return "peer_" + ifname;
+ // Best effort attempt to preserve the interface number, assuming it's the
+ // last char in the name.
+ auto c = ifname[ifname.length() - 1];
+ n.resize(IFNAMSIZ - 1);
+ n[n.length() - 1] = c;
+ return n;
}
Datapath::Datapath(MinijailedProcessRunner* process_runner)
diff --git a/patchpanel/datapath.h b/patchpanel/datapath.h
index ed6cae3..526061e 100644
--- a/patchpanel/datapath.h
+++ b/patchpanel/datapath.h
@@ -24,10 +24,7 @@
typedef int (*ioctl_t)(int, ioctl_req_t, ...);
// Returns for given interface name the host name of a ARC veth pair.
-std::string ArcVethHostName(std::string ifname);
-
-// Returns for given interface name the peer name of a ARC veth pair.
-std::string ArcVethPeerName(std::string ifname);
+std::string ArcVethHostName(const std::string& ifname);
// ARC networking data path configuration utility.
// IPV4 addresses are always specified in singular dotted-form (a.b.c.d)
diff --git a/patchpanel/datapath_test.cc b/patchpanel/datapath_test.cc
index 48f3e9d..de2a230 100644
--- a/patchpanel/datapath_test.cc
+++ b/patchpanel/datapath_test.cc
@@ -496,4 +496,14 @@
EXPECT_EQ(route2, captured_routes[3]);
}
+TEST(DatapathTest, ArcVethHostName) {
+ EXPECT_EQ("vetheth0", ArcVethHostName("eth0"));
+ EXPECT_EQ("vethrmnet0", ArcVethHostName("rmnet0"));
+ EXPECT_EQ("vethrmnet_data0", ArcVethHostName("rmnet_data0"));
+ EXPECT_EQ("vethifnamsiz_i0", ArcVethHostName("ifnamsiz_ifnam0"));
+ auto ifname = ArcVethHostName("exceeds_ifnamesiz_checkanyway");
+ EXPECT_EQ("vethexceeds_ify", ifname);
+ EXPECT_LT(ifname.length(), IFNAMSIZ);
+}
+
} // namespace patchpanel