patchpanel: move ConnectNamespace to Datapath
This patch moves the inner implementation of patchpanel's
ConnectNamespace DBus API to Datapath, and adds unit test coverage for
it. This allows to hide most implementation details from the upper
layers and will facilitate more refactoring in Datapath to make
ConnectNamespace iptables setup compatible with the fwmark based traffic
tagging system.
BUG=b:147712924
BUG=b:161508179
TEST=unit tests
Change-Id: I4415f66f6527032f5f20e6351ee47cbf4ddec3b8
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2397098
Tested-by: Hugo Benichi <hugobenichi@google.com>
Commit-Queue: Hugo Benichi <hugobenichi@google.com>
Reviewed-by: Garrick Evans <garrick@chromium.org>
diff --git a/patchpanel/datapath.cc b/patchpanel/datapath.cc
index eb2e4fa..ee10018 100644
--- a/patchpanel/datapath.cc
+++ b/patchpanel/datapath.cc
@@ -417,6 +417,122 @@
src_ip, "-j", "DROP", "-w"}) == 0;
}
+bool Datapath::StartRoutingNamespace(pid_t pid,
+ const std::string& netns_name,
+ const std::string& host_ifname,
+ const std::string& peer_ifname,
+ uint32_t subnet_ipv4_addr,
+ uint32_t subnet_prefixlen,
+ uint32_t host_ipv4_addr,
+ uint32_t peer_ipv4_addr,
+ const MacAddress& peer_mac_addr) {
+ // Veth interface configuration and client routing configuration:
+ // - attach a name to the client namespace.
+ // - create veth pair across the current namespace and the client namespace.
+ // - configure IPv4 address on remote veth inside client namespace.
+ // - configure IPv4 address on local veth inside host namespace.
+ // - add a default IPv4 /0 route sending traffic to that remote veth.
+ if (!NetnsAttachName(netns_name, pid)) {
+ LOG(ERROR) << "Failed to attach name " << netns_name << " to namespace pid "
+ << pid;
+ return false;
+ }
+
+ if (!ConnectVethPair(pid, netns_name, host_ifname, peer_ifname, peer_mac_addr,
+ peer_ipv4_addr, subnet_prefixlen,
+ false /* enable_multicast */)) {
+ LOG(ERROR) << "Failed to create veth pair for"
+ " namespace pid "
+ << pid;
+ NetnsDeleteName(netns_name);
+ return false;
+ }
+
+ if (!ConfigureInterface(host_ifname, peer_mac_addr, host_ipv4_addr,
+ subnet_prefixlen, true /* link up */,
+ false /* enable_multicast */)) {
+ LOG(ERROR) << "Cannot configure host interface " << host_ifname;
+ RemoveInterface(host_ifname);
+ NetnsDeleteName(netns_name);
+ return false;
+ }
+
+ {
+ ScopedNS ns(pid);
+ if (!ns.IsValid() && pid != kTestPID) {
+ LOG(ERROR) << "Invalid namespace pid " << pid;
+ RemoveInterface(host_ifname);
+ NetnsDeleteName(netns_name);
+ return false;
+ }
+
+ if (!AddIPv4Route(host_ipv4_addr, INADDR_ANY, INADDR_ANY)) {
+ LOG(ERROR) << "Failed to add default /0 route to " << host_ifname
+ << " inside namespace pid " << pid;
+ RemoveInterface(host_ifname);
+ NetnsDeleteName(netns_name);
+ return false;
+ }
+ }
+
+ // Host namespace routing configuration
+ // - ingress: add route to client subnet via |host_ifname|.
+ // - egress: - allow forwarding for traffic outgoing |host_ifname|.
+ // - add SNAT mark 0x1/0x1 for traffic outgoing |host_ifname|.
+ // Note that by default unsolicited ingress traffic is not forwarded to the
+ // client namespace unless the client specifically set port forwarding
+ // through permission_broker DBus APIs.
+ // TODO(hugobenichi) If allow_user_traffic is false, then prevent forwarding
+ // both ways between client namespace and other guest containers and VMs.
+ // TODO(b/161507671) If outbound_physical_device is defined, then set strong
+ // routing to that interface routing table.
+ uint32_t netmask = Ipv4Netmask(subnet_prefixlen);
+ if (!AddIPv4Route(host_ipv4_addr, subnet_ipv4_addr, netmask)) {
+ LOG(ERROR) << "Failed to set route to client namespace";
+ RemoveInterface(host_ifname);
+ NetnsDeleteName(netns_name);
+ return false;
+ }
+
+ if (!AddOutboundIPv4(host_ifname)) {
+ LOG(ERROR) << "Failed to allow FORWARD for"
+ " traffic outgoing from "
+ << host_ifname;
+ RemoveInterface(host_ifname);
+ DeleteIPv4Route(host_ipv4_addr, subnet_ipv4_addr, netmask);
+ NetnsDeleteName(netns_name);
+ return false;
+ }
+
+ // TODO(b/161508179) Add fwmark source tagging based on client usage.
+ // TODO(b/161508179) Do not rely on legacy fwmark 1 for SNAT.
+ if (!AddOutboundIPv4SNATMark(host_ifname)) {
+ LOG(ERROR) << "Failed to set SNAT for traffic"
+ " outgoing from "
+ << host_ifname;
+ RemoveInterface(host_ifname);
+ DeleteIPv4Route(host_ipv4_addr, subnet_ipv4_addr, netmask);
+ RemoveOutboundIPv4(host_ifname);
+ NetnsDeleteName(netns_name);
+ return false;
+ }
+
+ return true;
+}
+
+void Datapath::StopRoutingNamespace(const std::string& netns_name,
+ const std::string& host_ifname,
+ uint32_t subnet_ipv4_addr,
+ uint32_t subnet_prefixlen,
+ uint32_t host_ipv4_addr) {
+ RemoveInterface(host_ifname);
+ RemoveOutboundIPv4(host_ifname);
+ RemoveOutboundIPv4SNATMark(host_ifname);
+ DeleteIPv4Route(host_ipv4_addr, subnet_ipv4_addr,
+ Ipv4Netmask(subnet_prefixlen));
+ NetnsDeleteName(netns_name);
+}
+
void Datapath::StartRoutingDevice(const std::string& ext_ifname,
const std::string& int_ifname,
uint32_t int_ipv4_addr,
@@ -713,7 +829,7 @@
args.push_back(iif);
}
args.insert(args.end(), {"-j", "CONNMARK", "--restore-mark", "--mask",
- kFwmarkRoutingMask.ToString(), "-w"});
+ kFwmarkRoutingMask.ToString(), "-w"});
bool success = true;
if (family & IPv4)
diff --git a/patchpanel/datapath.h b/patchpanel/datapath.h
index 5ec53a5..91467f6 100644
--- a/patchpanel/datapath.h
+++ b/patchpanel/datapath.h
@@ -134,6 +134,28 @@
virtual bool RemoveSourceIPv4DropRule(const std::string& oif,
const std::string& src_ip);
+ // Creates a virtual ethernet interface pair shared with the client namespace
+ // of |pid| and sets up routing outside and inside the client namespace for
+ // connecting the client namespace to the network.
+ bool StartRoutingNamespace(pid_t pid,
+ const std::string& netns_name,
+ const std::string& host_ifname,
+ const std::string& peer_ifname,
+ uint32_t subnet_ipv4_addr,
+ uint32_t subnet_prefixlen,
+ uint32_t host_ipv4_addr,
+ uint32_t peer_ipv4_addr,
+ const MacAddress& peer_mac_addr);
+ // Destroys the virtual ethernet interface, routing, and network namespace
+ // name set for |netns_name| by StartRoutingNamespace. The default route set
+ // inside the |netns_name| by patchpanel is not destroyed and it is assumed
+ // the client will teardown the namespace.
+ void StopRoutingNamespace(const std::string& netns_name,
+ const std::string& host_ifname,
+ uint32_t subnet_ipv4_addr,
+ uint32_t subnet_prefixlen,
+ uint32_t host_ipv4_addr);
+
// Sets up IPv4 SNAT, IP forwarding, and traffic marking for the given
// virtual device |int_ifname| associated to |source|. if |ext_ifname| is
// empty, the device is implicitly routed through the highest priority
diff --git a/patchpanel/datapath_fuzzer.cc b/patchpanel/datapath_fuzzer.cc
index bc2773c..a54bd91 100644
--- a/patchpanel/datapath_fuzzer.cc
+++ b/patchpanel/datapath_fuzzer.cc
@@ -44,6 +44,8 @@
namespace {
+constexpr pid_t kTestPID = -2;
+
class Environment {
public:
Environment() {
@@ -66,6 +68,8 @@
std::string ifname2 = provider.ConsumeRandomLengthString(IFNAMSIZ - 1);
std::string bridge = provider.ConsumeRandomLengthString(IFNAMSIZ - 1);
uint32_t addr = provider.ConsumeIntegral<uint32_t>();
+ uint32_t addr2 = provider.ConsumeIntegral<uint32_t>();
+ uint32_t addr3 = provider.ConsumeIntegral<uint32_t>();
std::string addr_str = IPv4AddressToString(addr);
uint32_t prefix_len = provider.ConsumeIntegralInRange<uint32_t>(0, 31);
Subnet subnet(provider.ConsumeIntegral<int32_t>(), prefix_len,
@@ -81,6 +85,9 @@
datapath.RemoveBridge(ifname);
datapath.StartRoutingDevice(ifname, ifname2, addr, TrafficSource::UNKNOWN);
datapath.StopRoutingDevice(ifname, ifname2, addr, TrafficSource::UNKNOWN);
+ datapath.StartRoutingNamespace(kTestPID, netns_name, ifname, ifname2, addr,
+ prefix_len, addr2, addr3, mac);
+ datapath.StopRoutingNamespace(netns_name, ifname, addr, prefix_len, addr2);
datapath.AddVirtualInterfacePair(netns_name, ifname, bridge);
datapath.ToggleInterface(ifname, provider.ConsumeBool());
datapath.ConfigureInterface(ifname, mac, addr, prefix_len,
diff --git a/patchpanel/datapath_test.cc b/patchpanel/datapath_test.cc
index 2c17804..93c062f 100644
--- a/patchpanel/datapath_test.cc
+++ b/patchpanel/datapath_test.cc
@@ -276,6 +276,7 @@
SIOCSIFHWADDR, SIOCGIFFLAGS, SIOCSIFFLAGS};
EXPECT_EQ(ioctl_reqs, expected);
ioctl_reqs.clear();
+ ioctl_rtentry_args.clear();
}
TEST(DatapathTest, AddTAPWithOwner) {
@@ -292,6 +293,7 @@
SIOCSIFNETMASK, SIOCSIFHWADDR, SIOCGIFFLAGS, SIOCSIFFLAGS};
EXPECT_EQ(ioctl_reqs, expected);
ioctl_reqs.clear();
+ ioctl_rtentry_args.clear();
}
TEST(DatapathTest, AddTAPNoAddrs) {
@@ -304,6 +306,7 @@
SIOCSIFFLAGS};
EXPECT_EQ(ioctl_reqs, expected);
ioctl_reqs.clear();
+ ioctl_rtentry_args.clear();
}
TEST(DatapathTest, RemoveTAP) {
@@ -460,6 +463,80 @@
datapath.RemoveSourceIPv4DropRule("eth+", "100.115.92.0/24");
}
+TEST(DatapathTest, StartRoutingNamespace) {
+ MockProcessRunner runner;
+ MockFirewall firewall;
+ MacAddress mac = {1, 2, 3, 4, 5, 6};
+
+ EXPECT_CALL(runner, ip_netns_delete(StrEq("netns_foo"), false));
+ EXPECT_CALL(runner, ip_netns_attach(StrEq("netns_foo"), kTestPID, true));
+ EXPECT_CALL(runner, ip(StrEq("link"), StrEq("add"),
+ ElementsAre("arc_ns0", "type", "veth", "peer", "name",
+ "veth0", "netns", "netns_foo"),
+ true));
+ EXPECT_CALL(runner, ip(StrEq("addr"), StrEq("add"),
+ ElementsAre("100.115.92.130/30", "brd",
+ "100.115.92.131", "dev", "veth0"),
+ true))
+ .WillOnce(Return(0));
+ EXPECT_CALL(runner, ip(StrEq("link"), StrEq("set"),
+ ElementsAre("dev", "veth0", "up", "addr",
+ "01:02:03:04:05:06", "multicast", "off"),
+ true))
+ .WillOnce(Return(0));
+ EXPECT_CALL(runner, ip(StrEq("link"), StrEq("set"),
+ ElementsAre("arc_ns0", "up"), true));
+ EXPECT_CALL(runner, ip(StrEq("addr"), StrEq("add"),
+ ElementsAre("100.115.92.129/30", "brd",
+ "100.115.92.131", "dev", "arc_ns0"),
+ true))
+ .WillOnce(Return(0));
+ EXPECT_CALL(runner, ip(StrEq("link"), StrEq("set"),
+ ElementsAre("dev", "arc_ns0", "up", "addr",
+ "01:02:03:04:05:06", "multicast", "off"),
+ true))
+ .WillOnce(Return(0));
+ EXPECT_CALL(runner, iptables(StrEq("filter"),
+ ElementsAre("-A", "FORWARD", "-o", "arc_ns0",
+ "-j", "ACCEPT", "-w"),
+ true, nullptr));
+ EXPECT_CALL(runner,
+ iptables(StrEq("mangle"),
+ ElementsAre("-A", "PREROUTING", "-i", "arc_ns0", "-j",
+ "MARK", "--set-mark", "1/1", "-w"),
+ true, nullptr));
+
+ Datapath datapath(&runner, &firewall, (ioctl_t)ioctl_rtentry_cap);
+ datapath.StartRoutingNamespace(
+ kTestPID, "netns_foo", "arc_ns0", "veth0", Ipv4Addr(100, 115, 92, 128),
+ 30, Ipv4Addr(100, 115, 92, 129), Ipv4Addr(100, 115, 92, 130), mac);
+ ioctl_reqs.clear();
+ ioctl_rtentry_args.clear();
+}
+
+TEST(DatapathTest, StopRoutingNamespace) {
+ MockProcessRunner runner;
+ MockFirewall firewall;
+
+ EXPECT_CALL(runner, iptables(StrEq("filter"),
+ ElementsAre("-D", "FORWARD", "-o", "arc_ns0",
+ "-j", "ACCEPT", "-w"),
+ true, nullptr));
+ EXPECT_CALL(runner,
+ iptables(StrEq("mangle"),
+ ElementsAre("-D", "PREROUTING", "-i", "arc_ns0", "-j",
+ "MARK", "--set-mark", "1/1", "-w"),
+ true, nullptr));
+ EXPECT_CALL(runner, ip_netns_delete(StrEq("netns_foo"), true));
+ EXPECT_CALL(runner, ip(StrEq("link"), StrEq("delete"), ElementsAre("arc_ns0"),
+ false));
+
+ Datapath datapath(&runner, &firewall);
+ datapath.StopRoutingNamespace("netns_foo", "arc_ns0",
+ Ipv4Addr(100, 115, 92, 128), 30,
+ Ipv4Addr(100, 115, 92, 129));
+}
+
TEST(DatapathTest, StartRoutingDevice_Arc) {
MockProcessRunner runner;
MockFirewall firewall;
@@ -838,11 +915,13 @@
MockProcessRunner runner;
MockFirewall firewall;
Datapath datapath(&runner, &firewall, ioctl_req_cap);
+
bool result = datapath.MaskInterfaceFlags("foo0", IFF_DEBUG);
EXPECT_TRUE(result);
std::vector<ioctl_req_t> expected = {SIOCGIFFLAGS, SIOCSIFFLAGS};
EXPECT_EQ(ioctl_reqs, expected);
ioctl_reqs.clear();
+ ioctl_rtentry_args.clear();
}
TEST(DatapathTest, AddIPv6Forwarding) {
@@ -928,7 +1007,6 @@
std::vector<ioctl_req_t> expected_reqs = {SIOCADDRT, SIOCDELRT, SIOCADDRT,
SIOCDELRT};
EXPECT_EQ(expected_reqs, ioctl_reqs);
- ioctl_reqs.clear();
std::string route1 =
"{rt_dst: {family: AF_INET, port: 0, addr: 100.115.93.0}, rt_genmask: "
@@ -945,11 +1023,12 @@
stream << route.second;
captured_routes.emplace_back(stream.str());
}
- ioctl_rtentry_args.clear();
EXPECT_EQ(route1, captured_routes[0]);
EXPECT_EQ(route1, captured_routes[1]);
EXPECT_EQ(route2, captured_routes[2]);
EXPECT_EQ(route2, captured_routes[3]);
+ ioctl_reqs.clear();
+ ioctl_rtentry_args.clear();
}
TEST(DatapathTest, AddSNATMarkRules) {
diff --git a/patchpanel/manager.cc b/patchpanel/manager.cc
index 2f14964..8562316 100644
--- a/patchpanel/manager.cc
+++ b/patchpanel/manager.cc
@@ -691,21 +691,21 @@
dbus::MessageWriter writer(dbus_response.get());
patchpanel::ConnectNamespaceRequest request;
- patchpanel::ConnectNamespaceResponse response;
- bool success = true;
if (!reader.PopArrayOfBytesAsProto(&request)) {
LOG(ERROR) << "Unable to parse ConnectNamespaceRequest";
// Do not return yet to make sure we close the received fd and
// validate other arguments.
- success = false;
+ writer.AppendProtoAsArrayOfBytes(patchpanel::ConnectNamespaceResponse());
+ return dbus_response;
}
base::ScopedFD client_fd;
reader.PopFileDescriptor(&client_fd);
if (!client_fd.is_valid()) {
LOG(ERROR) << "ConnectNamespaceRequest: invalid file descriptor";
- success = false;
+ writer.AppendProtoAsArrayOfBytes(patchpanel::ConnectNamespaceResponse());
+ return dbus_response;
}
pid_t pid = request.pid();
@@ -713,7 +713,8 @@
ScopedNS ns(pid);
if (!ns.IsValid()) {
LOG(ERROR) << "ConnectNamespaceRequest: invalid namespace pid " << pid;
- success = false;
+ writer.AppendProtoAsArrayOfBytes(patchpanel::ConnectNamespaceResponse());
+ return dbus_response;
}
}
@@ -721,13 +722,12 @@
if (!outbound_ifname.empty() && !shill_client_->has_device(outbound_ifname)) {
LOG(ERROR) << "ConnectNamespaceRequest: invalid outbound ifname "
<< outbound_ifname;
- success = false;
+ writer.AppendProtoAsArrayOfBytes(patchpanel::ConnectNamespaceResponse());
+ return dbus_response;
}
- if (success)
- ConnectNamespace(std::move(client_fd), request, response);
-
- writer.AppendProtoAsArrayOfBytes(response);
+ auto response = ConnectNamespace(std::move(client_fd), request);
+ writer.AppendProtoAsArrayOfBytes(*response);
return dbus_response;
}
@@ -828,127 +828,27 @@
dbus_svc_path_->SendSignal(&signal);
}
-void Manager::ConnectNamespace(
+std::unique_ptr<patchpanel::ConnectNamespaceResponse> Manager::ConnectNamespace(
base::ScopedFD client_fd,
- const patchpanel::ConnectNamespaceRequest& request,
- patchpanel::ConnectNamespaceResponse& response) {
+ const patchpanel::ConnectNamespaceRequest& request) {
+ auto response = std::make_unique<patchpanel::ConnectNamespaceResponse>();
+
std::unique_ptr<Subnet> subnet =
addr_mgr_.AllocateIPv4Subnet(AddressManager::Guest::MINIJAIL_NETNS);
if (!subnet) {
- LOG(ERROR) << "ConnectNamespaceRequest: exhausted IPv4 subnet space";
- return;
- }
-
- const std::string ifname_id = std::to_string(connected_namespaces_next_id_);
- const std::string netns_name = "connected_netns_" + ifname_id;
- const std::string host_ifname = "arc_ns" + ifname_id;
- const std::string client_ifname = "veth" + ifname_id;
- const uint32_t host_ipv4_addr = subnet->AddressAtOffset(0);
- const uint32_t client_ipv4_addr = subnet->AddressAtOffset(1);
-
- // Veth interface configuration and client routing configuration:
- // - attach a name to the client namespace.
- // - create veth pair across the current namespace and the client namespace.
- // - configure IPv4 address on remote veth inside client namespace.
- // - configure IPv4 address on local veth inside host namespace.
- // - add a default IPv4 /0 route sending traffic to that remote veth.
- pid_t pid = request.pid();
- if (!datapath_->NetnsAttachName(netns_name, pid)) {
- LOG(ERROR) << "ConnectNamespaceRequest: failed to attach name "
- << netns_name << " to namespace pid " << pid;
- return;
- }
- if (!datapath_->ConnectVethPair(pid, netns_name, host_ifname, client_ifname,
- addr_mgr_.GenerateMacAddress(),
- client_ipv4_addr, subnet->PrefixLength(),
- false /* enable_multicast */)) {
- LOG(ERROR) << "ConnectNamespaceRequest: failed to create veth pair for "
- "namespace pid "
- << pid;
- datapath_->NetnsDeleteName(netns_name);
- return;
- }
- if (!datapath_->ConfigureInterface(
- host_ifname, addr_mgr_.GenerateMacAddress(), host_ipv4_addr,
- subnet->PrefixLength(), true /* link up */,
- false /* enable_multicast */)) {
- LOG(ERROR) << "ConnectNamespaceRequest: cannot configure host interface "
- << host_ifname;
- datapath_->RemoveInterface(host_ifname);
- datapath_->NetnsDeleteName(netns_name);
- return;
- }
- bool peer_route_setup_success;
- {
- ScopedNS ns(pid);
- peer_route_setup_success =
- ns.IsValid() &&
- datapath_->AddIPv4Route(host_ipv4_addr, INADDR_ANY, INADDR_ANY);
- }
- if (!peer_route_setup_success) {
- LOG(ERROR) << "ConnectNamespaceRequest: failed to add default /0 route to "
- << host_ifname << " inside namespace pid " << pid;
- datapath_->RemoveInterface(host_ifname);
- datapath_->NetnsDeleteName(netns_name);
- return;
- }
-
- // Host namespace routing configuration
- // - ingress: add route to client subnet via |host_ifname|.
- // - egress: - allow forwarding for traffic outgoing |host_ifname|.
- // - add SNAT mark 0x1/0x1 for traffic outgoing |host_ifname|.
- // Note that by default unsolicited ingress traffic is not forwarded to the
- // client namespace unless the client specifically set port forwarding
- // through permission_broker DBus APIs.
- // TODO(hugobenichi) If allow_user_traffic is false, then prevent forwarding
- // both ways between client namespace and other guest containers and VMs.
- // TODO(hugobenichi) If outbound_physical_device is defined, then set strong
- // routing to that interface routing table.
- if (!datapath_->AddIPv4Route(host_ipv4_addr, subnet->BaseAddress(),
- subnet->Netmask())) {
- LOG(ERROR)
- << "ConnectNamespaceRequest: failed to set route to client namespace";
- datapath_->RemoveInterface(host_ifname);
- datapath_->NetnsDeleteName(netns_name);
- return;
- }
- if (!datapath_->AddOutboundIPv4(host_ifname)) {
- LOG(ERROR) << "ConnectNamespaceRequest: failed to allow FORWARD for "
- "traffic outgoing from "
- << host_ifname;
- datapath_->RemoveInterface(host_ifname);
- datapath_->DeleteIPv4Route(host_ipv4_addr, subnet->BaseAddress(),
- subnet->Netmask());
- datapath_->NetnsDeleteName(netns_name);
- return;
- }
- if (!datapath_->AddOutboundIPv4SNATMark(host_ifname)) {
- LOG(ERROR) << "ConnectNamespaceRequest: failed to set SNAT for traffic "
- "outgoing from "
- << host_ifname;
- datapath_->RemoveInterface(host_ifname);
- datapath_->DeleteIPv4Route(host_ipv4_addr, subnet->BaseAddress(),
- subnet->Netmask());
- datapath_->RemoveOutboundIPv4(host_ifname);
- datapath_->NetnsDeleteName(netns_name);
- return;
+ LOG(ERROR) << "ConnectNamespace: exhausted IPv4 subnet space";
+ return response;
}
// Dup the client fd into our own: this guarantees that the fd number will
// be stable and tied to the actual kernel resources used by the client.
base::ScopedFD local_client_fd(dup(client_fd.get()));
if (!local_client_fd.is_valid()) {
- PLOG(ERROR) << "ConnectNamespaceRequest: failed to dup() client fd";
- datapath_->RemoveInterface(host_ifname);
- datapath_->DeleteIPv4Route(host_ipv4_addr, subnet->BaseAddress(),
- subnet->Netmask());
- datapath_->RemoveOutboundIPv4(host_ifname);
- datapath_->RemoveOutboundIPv4SNATMark(host_ifname);
- datapath_->NetnsDeleteName(netns_name);
- return;
+ PLOG(ERROR) << "ConnectNamespace: failed to dup() client fd";
+ return response;
}
- // Add the dupe fd to the epoll watcher.
+ // Add the duped fd to the epoll watcher.
// TODO(hugobenichi) Find a way to reuse base::FileDescriptorWatcher for
// listening to EPOLLHUP.
struct epoll_event epevent;
@@ -956,22 +856,33 @@
epevent.data.fd = local_client_fd.get();
if (epoll_ctl(connected_namespaces_epollfd_, EPOLL_CTL_ADD,
local_client_fd.get(), &epevent) != 0) {
- PLOG(ERROR) << "ConnectNamespaceResponse: epoll_ctl(EPOLL_CTL_ADD) failed";
- datapath_->RemoveInterface(host_ifname);
- datapath_->DeleteIPv4Route(host_ipv4_addr, subnet->BaseAddress(),
- subnet->Netmask());
- datapath_->RemoveOutboundIPv4(host_ifname);
- datapath_->RemoveOutboundIPv4SNATMark(host_ifname);
- datapath_->NetnsDeleteName(netns_name);
- return;
+ PLOG(ERROR) << "ConnectNamespace: epoll_ctl(EPOLL_CTL_ADD) failed";
+ return response;
}
+ const std::string ifname_id = std::to_string(connected_namespaces_next_id_);
+ const std::string netns_name = "connected_netns_" + ifname_id;
+ const std::string host_ifname = "arc_ns" + ifname_id;
+ const std::string client_ifname = "veth" + ifname_id;
+ if (!datapath_->StartRoutingNamespace(
+ request.pid(), netns_name, host_ifname, client_ifname,
+ subnet->BaseAddress(), subnet->PrefixLength(),
+ subnet->AddressAtOffset(0), subnet->AddressAtOffset(1),
+ addr_mgr_.GenerateMacAddress())) {
+ LOG(ERROR) << "ConnectNamespace: failed to setup datapath";
+ if (epoll_ctl(connected_namespaces_epollfd_, EPOLL_CTL_DEL,
+ local_client_fd.get(), nullptr) != 0)
+ PLOG(ERROR) << "ConnectNamespace: epoll_ctl(EPOLL_CTL_DEL) failed";
+ return response;
+ }
+
+ // TODO(hugobenichi) The peer and host addresses are swapped in the response
// Prepare the response before storing ConnectNamespaceInfo.
- response.set_peer_ifname(client_ifname);
- response.set_peer_ipv4_address(host_ipv4_addr);
- response.set_host_ifname(host_ifname);
- response.set_host_ipv4_address(client_ipv4_addr);
- auto* response_subnet = response.mutable_ipv4_subnet();
+ response->set_peer_ifname(client_ifname);
+ response->set_peer_ipv4_address(subnet->AddressAtOffset(0));
+ response->set_host_ifname(host_ifname);
+ response->set_host_ipv4_address(subnet->AddressAtOffset(1));
+ auto* response_subnet = response->mutable_ipv4_subnet();
response_subnet->set_base_addr(subnet->BaseAddress());
response_subnet->set_prefix_len(subnet->PrefixLength());
@@ -993,6 +904,8 @@
LOG(INFO) << "Starting ConnectNamespace client fds monitoring";
CheckConnectedNamespaces();
}
+
+ return response;
}
void Manager::DisconnectNamespace(int client_fd) {
@@ -1009,21 +922,10 @@
if (close(client_fd) < 0)
PLOG(ERROR) << "DisconnectNamespace: close(client_fd) failed";
- // Destroy the interface configuration and routing configuration:
- // - destroy veth pair.
- // - remove forwarding rules on host namespace.
- // - remove SNAT marking rule on host namespace.
- // Delete the network namespace attached to the client namespace.
- // Note that the default route set inside the client namespace by patchpanel
- // is not destroyed: it is assumed the client will also teardown its
- // namespace if it triggered DisconnectNamespace.
- datapath_->RemoveInterface(it->second.host_ifname);
- datapath_->RemoveOutboundIPv4(it->second.host_ifname);
- datapath_->RemoveOutboundIPv4SNATMark(it->second.host_ifname);
- datapath_->DeleteIPv4Route(it->second.client_subnet->AddressAtOffset(0),
- it->second.client_subnet->BaseAddress(),
- it->second.client_subnet->Netmask());
- datapath_->NetnsDeleteName(it->second.netns_name);
+ datapath_->StopRoutingNamespace(it->second.netns_name, it->second.host_ifname,
+ it->second.client_subnet->BaseAddress(),
+ it->second.client_subnet->PrefixLength(),
+ it->second.client_subnet->AddressAtOffset(0));
LOG(INFO) << "Disconnected network namespace " << it->second;
diff --git a/patchpanel/manager.h b/patchpanel/manager.h
index 2901327..c28238c 100644
--- a/patchpanel/manager.h
+++ b/patchpanel/manager.h
@@ -162,9 +162,9 @@
NeighborLinkMonitor::NeighborRole role,
bool connected);
- void ConnectNamespace(base::ScopedFD client_fd,
- const patchpanel::ConnectNamespaceRequest& request,
- patchpanel::ConnectNamespaceResponse& response);
+ std::unique_ptr<patchpanel::ConnectNamespaceResponse> ConnectNamespace(
+ base::ScopedFD client_fd,
+ const patchpanel::ConnectNamespaceRequest& request);
void DisconnectNamespace(int client_fd);
// Detects if any file descriptor committed in ConnectNamespace DBus API has
// been invalidated by the caller. Calls DisconnectNamespace for any invalid
