[bt][rfcomm] Open RFCOMM channels

RFCOMM opens both incoming and outgoing channels.

NET-1013 #done
TEST=bt-host-unittests --gtest_filter='RFCOMM_ChannelManagerTest.Open*'

Change-Id: I6ef1babb03e6cf9b9a08b2fef4cf2ad62c85deb7
diff --git a/drivers/bluetooth/lib/rfcomm/channel.cc b/drivers/bluetooth/lib/rfcomm/channel.cc
index 3a4904f..84ef13bb 100644
--- a/drivers/bluetooth/lib/rfcomm/channel.cc
+++ b/drivers/bluetooth/lib/rfcomm/channel.cc
@@ -15,11 +15,8 @@
 
 namespace internal {
 
-void ChannelImpl::CallRxCallback(common::ByteBufferPtr data) {
-  async::PostTask(dispatcher_, [this, data_ = std::move(data)]() mutable {
-    rx_callback_(std::move(data_));
-  });
-}
+ChannelImpl::ChannelImpl(DLCI dlci, Session* session)
+    : Channel(dlci, session) {}
 
 void ChannelImpl::Activate(RxCallback rx_callback,
                            ClosedCallback closed_callback,
@@ -30,19 +27,24 @@
 
   while (!pending_rxed_frames_.empty()) {
     auto& data = pending_rxed_frames_.front();
-    CallRxCallback(std::move(data));
+    async::PostTask(dispatcher_, [this, data_ = std::move(data)]() mutable {
+      rx_callback_(std::move(data_));
+    });
     pending_rxed_frames_.pop();
   }
 }
 
 void ChannelImpl::Send(common::ByteBufferPtr data) {
   FXL_DCHECK(session_);
-  session_->Send(dlci_, std::move(data));
+  FXL_DCHECK(rx_callback_) << "Must call Activate() first";
+  session_->SendUserData(dlci_, std::move(data));
 }
 
 void ChannelImpl::Receive(std::unique_ptr<common::ByteBuffer> data) {
   if (rx_callback_) {
-    CallRxCallback(std::move(data));
+    async::PostTask(dispatcher_, [this, data_ = std::move(data)]() mutable {
+      rx_callback_(std::move(data_));
+    });
   } else {
     pending_rxed_frames_.push(std::move(data));
   }
diff --git a/drivers/bluetooth/lib/rfcomm/channel.h b/drivers/bluetooth/lib/rfcomm/channel.h
index dbfd57e..e5c421f 100644
--- a/drivers/bluetooth/lib/rfcomm/channel.h
+++ b/drivers/bluetooth/lib/rfcomm/channel.h
@@ -30,10 +30,13 @@
 
   // Send a buffer of user data. Takes ownership of |data|. This method is
   // asynchronous, and there is no notification of delivery. We operate under
-  // the assumption that the underlying transport is reliable.
+  // the assumption that the underlying transport is reliable. The channel must
+  // be activated prior to sending.
   virtual void Send(common::ByteBufferPtr data) = 0;
 
  protected:
+  friend class rfcomm::Session;
+
   Channel(DLCI dlci, Session* session);
 
   RxCallback rx_callback_;
@@ -63,17 +66,14 @@
   void Send(common::ByteBufferPtr data) override;
 
  private:
-  friend class Session;
+  friend class rfcomm::Session;
 
   // This should only be called from Session.
-  ChannelImpl();
+  ChannelImpl(DLCI dlci, Session* session);
 
   // This should only be called from Session.
   void Receive(std::unique_ptr<common::ByteBuffer> data) override;
 
-  // Post a call to |rx_callback_| on |dispatcher_|, passing in |data|.
-  void CallRxCallback(std::unique_ptr<common::ByteBuffer> data);
-
   std::queue<std::unique_ptr<common::ByteBuffer>> pending_rxed_frames_;
 };
 
diff --git a/drivers/bluetooth/lib/rfcomm/rfcomm_unittest.cc b/drivers/bluetooth/lib/rfcomm/rfcomm_unittest.cc
index 63070c3..d01ee37 100644
--- a/drivers/bluetooth/lib/rfcomm/rfcomm_unittest.cc
+++ b/drivers/bluetooth/lib/rfcomm/rfcomm_unittest.cc
@@ -77,6 +77,24 @@
     handle_to_incoming_frames_.clear();
   }
 
+  // Emplace a new PeerState for a new fake peer. Should be called for each fake
+  // peer which a test is emulating. The returned PeerState should then be
+  // updated throughout the test (e.g. the multiplexer state should change when
+  // the multiplexer starts up).
+  PeerState& AddFakePeerState(hci::ConnectionHandle handle, PeerState state) {
+    FXL_DCHECK(handle_to_peer_state_.find(handle) ==
+               handle_to_peer_state_.end());
+    handle_to_peer_state_.emplace(handle, std::move(state));
+    return handle_to_peer_state_[handle];
+  }
+
+  fbl::RefPtr<l2cap::testing::FakeChannel> GetFakeChannel(
+      hci::ConnectionHandle handle) {
+    FXL_DCHECK(handle_to_fake_channel_.find(handle) !=
+               handle_to_fake_channel_.end());
+    return handle_to_fake_channel_[handle];
+  }
+
   void ExpectFrame(hci::ConnectionHandle handle, FrameType type, DLCI dlci) {
     auto queue_it = handle_to_incoming_frames_.find(handle);
     EXPECT_FALSE(handle_to_incoming_frames_.end() == queue_it);
@@ -95,31 +113,31 @@
 
   void ReceiveFrame(hci::ConnectionHandle handle,
                     std::unique_ptr<Frame> frame) {
-    auto channel_it = handle_to_fake_channel_.find(handle);
-    EXPECT_FALSE(channel_it == handle_to_fake_channel_.end());
+    auto channel = GetFakeChannel(handle);
 
     auto buffer = common::NewSlabBuffer(frame->written_size());
     frame->Write(buffer->mutable_view());
-    channel_it->second->Receive(buffer->view());
+    channel->Receive(buffer->view());
   }
 
-  // Emplace a new PeerState for a new fake peer. Should be called for each fake
-  // peer which a test is emulating. The returned PeerState should then be
-  // updated throughout the test (e.g. the multiplexer state should change when
-  // the multiplexer starts up).
-  PeerState& AddFakePeerState(hci::ConnectionHandle handle, PeerState state) {
-    FXL_DCHECK(handle_to_peer_state_.find(handle) ==
-               handle_to_peer_state_.end());
-    handle_to_peer_state_.emplace(handle, std::move(state));
-    return handle_to_peer_state_[handle];
-  }
+  // Makes the asynchronous channel-getting process synchronous, for the
+  // purposes of writing clean tests. This function will imitiate (to
+  // |channel_manager_|) an RFCOMM peer, and will send frames to handle
+  // multiplexer startup, optional parameter negotiation, and finally, channel
+  // opening.
+  //
+  // If this is the first channel which will be opened on this handle,
+  // the state of the peer should first be set in |handle_to_peer_state_|.
+  // These state variables will then be used during the session startup
+  // procedure which will ensue.
+  fbl::RefPtr<Channel> OpenOutgoingChannel(hci::ConnectionHandle handle,
+                                           ServerChannel server_channel);
 
-  fbl::RefPtr<l2cap::testing::FakeChannel> GetFakeChannel(
-      hci::ConnectionHandle handle) {
-    FXL_DCHECK(handle_to_fake_channel_.find(handle) !=
-               handle_to_fake_channel_.end());
-    return handle_to_fake_channel_[handle];
-  }
+  // The fake remote peer represented by |handle| attempts to open
+  // |server_channel|. If no session exists, this function will handle session
+  // startup, multiplexer startup, and parameter negotiation.
+  void OpenIncomingChannel(hci::ConnectionHandle handle,
+                           ServerChannel server_channel);
 
   std::unique_ptr<ChannelManager> channel_manager_;
 
@@ -143,6 +161,181 @@
   std::unordered_map<hci::ConnectionHandle, PeerState> handle_to_peer_state_;
 };
 
+fbl::RefPtr<Channel> RFCOMM_ChannelManagerTest::OpenOutgoingChannel(
+    hci::ConnectionHandle handle, ServerChannel server_channel) {
+  FXL_DCHECK(handle_to_peer_state_.find(handle) != handle_to_peer_state_.end())
+      << "No peer state for handle " << handle;
+  PeerState& state = handle_to_peer_state_[handle];
+
+  fbl::RefPtr<Channel> return_channel = nullptr;
+  channel_manager_->OpenRemoteChannel(
+      handle, server_channel,
+      [&return_channel](auto channel, auto server_channel) {
+        return_channel = channel;
+      },
+      dispatcher());
+  RunLoopUntilIdle();
+
+  // If the fake L2CAP channel doesn't exist yet, we need to trigger its
+  // creation with TriggerOutboundChannel.
+  if (handle_to_fake_channel_.find(handle) == handle_to_fake_channel_.end()) {
+    l2cap_->TriggerOutboundChannel(handle, l2cap::kRFCOMM, kL2CAPChannelId1,
+                                   kL2CAPChannelId2);
+    RunLoopUntilIdle();
+  }
+
+  EXPECT_FALSE(handle_to_incoming_frames_.find(handle) ==
+               handle_to_incoming_frames_.end());
+  auto& queue = handle_to_incoming_frames_[handle];
+
+  EXPECT_FALSE(queue.empty());
+  auto frame =
+      Frame::Parse(state.credit_based_flow, state.role, queue.front()->view());
+
+  queue.pop();
+  FXL_DCHECK(frame);
+
+  // If we received a mux startup request, respond to it.
+  if (static_cast<FrameType>(frame->control()) ==
+          FrameType::kSetAsynchronousBalancedMode &&
+      frame->dlci() == kMuxControlDLCI) {
+    ReceiveFrame(handle, std::make_unique<UnnumberedAcknowledgementResponse>(
+                             state.role, kMuxControlDLCI));
+
+    state.role = Role::kResponder;
+
+    RunLoopUntilIdle();
+
+    // Expect that another frame has arrived.
+    EXPECT_FALSE(queue.empty());
+    frame = Frame::Parse(state.credit_based_flow, state.role,
+                         queue.front()->view());
+    queue.pop();
+  }
+
+  // If we received a parameter negotiation request, respond to it.
+  if (static_cast<FrameType>(frame->control()) ==
+          FrameType::kUnnumberedInfoHeaderCheck &&
+      frame->dlci() == kMuxControlDLCI) {
+    auto pn_command_mux_command =
+        static_cast<MuxCommandFrame*>(frame.get())->TakeMuxCommand();
+    EXPECT_EQ(MuxCommandType::kDLCParameterNegotiation,
+              pn_command_mux_command->command_type());
+    EXPECT_EQ(CommandResponse::kCommand,
+              pn_command_mux_command->command_response());
+
+    auto pn_command = std::unique_ptr<DLCParameterNegotiationCommand>(
+        static_cast<DLCParameterNegotiationCommand*>(
+            pn_command_mux_command.release()));
+
+    // For now, just send back the same parameters (making sure to send the
+    // correct credit-based flow response based on our credit-based flow
+    // setting).
+    ParameterNegotiationParams params = pn_command->params();
+    params.credit_based_flow_handshake =
+        state.credit_based_flow ? CreditBasedFlowHandshake::kSupportedResponse
+                                : CreditBasedFlowHandshake::kUnsupported;
+    ReceiveFrame(handle, std::make_unique<MuxCommandFrame>(
+                             state.role, state.credit_based_flow,
+                             std::make_unique<DLCParameterNegotiationCommand>(
+                                 CommandResponse::kResponse, params)));
+
+    RunLoopUntilIdle();
+
+    // Expect that another frame has arrived.
+    EXPECT_FALSE(queue.empty());
+    frame = Frame::Parse(state.credit_based_flow, state.role,
+                         queue.front()->view());
+    queue.pop();
+  }
+
+  EXPECT_EQ(FrameType::kSetAsynchronousBalancedMode,
+            FrameType(frame->control()));
+  DLCI dlci = ServerChannelToDLCI(server_channel, state.role);
+  EXPECT_EQ(dlci, frame->dlci());
+
+  ReceiveFrame(handle, std::make_unique<UnnumberedAcknowledgementResponse>(
+                           state.role, dlci));
+
+  RunLoopUntilIdle();
+
+  EXPECT_TRUE(return_channel);
+
+  return return_channel;
+}
+
+void RFCOMM_ChannelManagerTest::OpenIncomingChannel(
+    hci::ConnectionHandle handle, ServerChannel server_channel) {
+  FXL_DCHECK(handle_to_peer_state_.find(handle) != handle_to_peer_state_.end())
+      << "Please set peer state for handle " << handle
+      << " before attempting to open a channel on this handle";
+
+  PeerState& state = handle_to_peer_state_[handle];
+
+  if (handle_to_fake_channel_.find(handle) == handle_to_fake_channel_.end()) {
+    l2cap_->TriggerInboundChannel(handle, l2cap::kRFCOMM, kL2CAPChannelId1,
+                                  kL2CAPChannelId2);
+    RunLoopUntilIdle();
+
+    // If channel didn't exist, then we need to do mux startup and parameter
+    // negotiation.
+    auto l2cap_channel = GetFakeChannel(handle);
+
+    ReceiveFrame(handle, std::make_unique<SetAsynchronousBalancedModeCommand>(
+                             Role::kUnassigned, kMuxControlDLCI));
+    RunLoopUntilIdle();
+    ExpectFrame(handle, FrameType::kUnnumberedAcknowledgement, kMuxControlDLCI);
+    state.role = Role::kInitiator;
+
+    DLCI dlci = ServerChannelToDLCI(server_channel, OppositeRole(state.role));
+
+    // Send parameter negotiation
+    ParameterNegotiationParams params;
+    params.dlci = dlci;
+    params.credit_based_flow_handshake =
+        CreditBasedFlowHandshake::kSupportedRequest;
+    params.priority = 61;
+    params.maximum_frame_size =
+        l2cap_channel->rx_mtu() < l2cap_channel->tx_mtu()
+            ? l2cap_channel->rx_mtu()
+            : l2cap_channel->tx_mtu();
+    params.initial_credits = kMaxInitialCredits;
+    ReceiveFrame(handle, std::make_unique<MuxCommandFrame>(
+                             state.role, state.credit_based_flow,
+                             std::make_unique<DLCParameterNegotiationCommand>(
+                                 CommandResponse::kCommand, params)));
+    RunLoopUntilIdle();
+
+    // Expect parameter negotiation response
+    EXPECT_TRUE(handle_to_incoming_frames_[handle].size());
+    auto frame =
+        Frame::Parse(state.credit_based_flow, state.role,
+                     handle_to_incoming_frames_[handle].front()->view());
+    handle_to_incoming_frames_[handle].pop();
+    EXPECT_EQ(FrameType::kUnnumberedInfoHeaderCheck,
+              static_cast<FrameType>(frame->control()));
+    EXPECT_EQ(kMuxControlDLCI, frame->dlci());
+    auto mux_command =
+        static_cast<MuxCommandFrame*>(frame.get())->TakeMuxCommand();
+    EXPECT_EQ(MuxCommandType::kDLCParameterNegotiation,
+              mux_command->command_type());
+    EXPECT_EQ(CommandResponse::kResponse, mux_command->command_response());
+  }
+
+  // Otherwise, a session must already exist with this remote peer. We can
+  // furthermore assume that a channel must be open, and thus that the
+  // multiplexer has also been started, and parameter negotiation is complete.
+
+  DLCI dlci = ServerChannelToDLCI(server_channel, OppositeRole(state.role));
+
+  // Send SABM.
+  ReceiveFrame(handle, std::make_unique<SetAsynchronousBalancedModeCommand>(
+                           state.role, dlci));
+  RunLoopUntilIdle();
+
+  // Expect UA response.
+  ExpectFrame(handle, FrameType::kUnnumberedAcknowledgement, dlci);
+}
 // Expect that registration of an L2CAP channel with the Channel Manager results
 // in the L2CAP channel's eventual activation.
 TEST_F(RFCOMM_ChannelManagerTest, RegisterL2CAPChannel) {
@@ -218,6 +411,7 @@
   RunLoopUntilIdle();
 
   state.role = Role::kResponder;
+  DLCI dlci = ServerChannelToDLCI(kMinServerChannel, state.role);
 
   {
     // Expect a PN command from the session
@@ -229,8 +423,6 @@
     queue_it->second.pop();
     EXPECT_EQ(FrameType::kUnnumberedInfoHeaderCheck,
               static_cast<FrameType>(frame->control()));
-    DLCI dlci =
-        ServerChannelToDLCI(kMinServerChannel, OppositeRole(state.role));
     auto mux_command =
         static_cast<MuxCommandFrame*>(frame.get())->TakeMuxCommand();
     EXPECT_EQ(MuxCommandType::kDLCParameterNegotiation,
@@ -251,8 +443,13 @@
     RunLoopUntilIdle();
   }
 
+  ExpectFrame(kHandle1, FrameType::kSetAsynchronousBalancedMode, dlci);
+  ReceiveFrame(kHandle1, std::make_unique<UnnumberedAcknowledgementResponse>(
+                             state.role, dlci));
+  RunLoopUntilIdle();
+
   EXPECT_TRUE(channel_received);
-  EXPECT_FALSE(channel);
+  EXPECT_TRUE(channel);
 }
 
 // Test multiplexer startup conflict procedure (resulting role: initiator).
@@ -295,6 +492,7 @@
   RunLoopUntilIdle();
 
   state.role = Role::kResponder;
+  DLCI dlci = ServerChannelToDLCI(kMinServerChannel, state.role);
 
   {
     // Expect a PN command from the session
@@ -306,8 +504,6 @@
     queue_it->second.pop();
     EXPECT_EQ(FrameType::kUnnumberedInfoHeaderCheck,
               static_cast<FrameType>(frame->control()));
-    DLCI dlci =
-        ServerChannelToDLCI(kMinServerChannel, OppositeRole(state.role));
     auto mux_command =
         static_cast<MuxCommandFrame*>(frame.get())->TakeMuxCommand();
     EXPECT_EQ(MuxCommandType::kDLCParameterNegotiation,
@@ -328,8 +524,13 @@
     RunLoopUntilIdle();
   }
 
+  ExpectFrame(kHandle1, FrameType::kSetAsynchronousBalancedMode, dlci);
+  ReceiveFrame(kHandle1, std::make_unique<UnnumberedAcknowledgementResponse>(
+                             state.role, dlci));
+  RunLoopUntilIdle();
+
   EXPECT_TRUE(channel_received);
-  EXPECT_FALSE(channel);
+  EXPECT_TRUE(channel);
 }
 
 // Test multiplexer startup conflict procedure (resulting role: responder).
@@ -382,8 +583,7 @@
     queue.pop();
     EXPECT_EQ(FrameType::kUnnumberedInfoHeaderCheck,
               static_cast<FrameType>(frame->control()));
-    DLCI dlci =
-        ServerChannelToDLCI(kMinServerChannel, OppositeRole(state.role));
+    DLCI dlci = ServerChannelToDLCI(kMinServerChannel, state.role);
     auto mux_command =
         static_cast<MuxCommandFrame*>(frame.get())->TakeMuxCommand();
     EXPECT_EQ(MuxCommandType::kDLCParameterNegotiation,
@@ -434,7 +634,7 @@
   RunLoopUntilIdle();
 
   state.role = Role::kResponder;
-  DLCI dlci = ServerChannelToDLCI(kMinServerChannel, OppositeRole(state.role));
+  DLCI dlci = ServerChannelToDLCI(kMinServerChannel, state.role);
 
   {
     // Expect a PN command from the session
@@ -447,8 +647,7 @@
     queue.pop();
     EXPECT_EQ(FrameType::kUnnumberedInfoHeaderCheck,
               static_cast<FrameType>(frame->control()));
-    DLCI dlci =
-        ServerChannelToDLCI(kMinServerChannel, OppositeRole(state.role));
+    DLCI dlci = ServerChannelToDLCI(kMinServerChannel, state.role);
     auto mux_command =
         static_cast<MuxCommandFrame*>(frame.get())->TakeMuxCommand();
     EXPECT_EQ(MuxCommandType::kDLCParameterNegotiation,
@@ -500,6 +699,90 @@
   RunLoopUntilIdle();
 }
 
+TEST_F(RFCOMM_ChannelManagerTest, OpenOutgoingChannel) {
+  handle_to_peer_state_.emplace(kHandle1, PeerState{true, Role::kUnassigned});
+  PeerState& state = handle_to_peer_state_[kHandle1];
+
+  auto channel = OpenOutgoingChannel(kHandle1, kMinServerChannel);
+  EXPECT_TRUE(channel);
+
+  DLCI dlci = ServerChannelToDLCI(kMinServerChannel, state.role);
+
+  common::ByteBufferPtr received_data;
+  channel->Activate(
+      [&received_data](auto data) { received_data = std::move(data); }, []() {},
+      dispatcher());
+
+  auto pattern = common::CreateStaticByteBuffer(1, 2, 3, 4);
+  auto buffer = std::make_unique<common::DynamicByteBuffer>(pattern);
+  channel->Send(std::move(buffer));
+  RunLoopUntilIdle();
+
+  auto frame =
+      Frame::Parse(state.credit_based_flow, OppositeRole(state.role),
+                   handle_to_incoming_frames_[kHandle1].front()->view());
+  EXPECT_TRUE(frame);
+  EXPECT_EQ(FrameType::kUnnumberedInfoHeaderCheck,
+            static_cast<FrameType>(frame->control()));
+  EXPECT_EQ(dlci, frame->dlci());
+  EXPECT_EQ(pattern,
+            *static_cast<UserDataFrame*>(frame.get())->TakeInformation());
+
+  buffer = std::make_unique<common::DynamicByteBuffer>(pattern);
+  ReceiveFrame(kHandle1, std::make_unique<UserDataFrame>(
+                             state.role, state.credit_based_flow, dlci,
+                             std::move(buffer)));
+  RunLoopUntilIdle();
+
+  EXPECT_TRUE(received_data);
+  EXPECT_EQ(pattern, *received_data);
+}
+
+TEST_F(RFCOMM_ChannelManagerTest, OpenIncomingChannel) {
+  auto& state = AddFakePeerState(
+      kHandle1, PeerState{true /* credit-based flow */, Role::kUnassigned});
+
+  fbl::RefPtr<Channel> channel;
+  auto server_channel = channel_manager_->AllocateLocalChannel(
+      [&channel](auto received_channel, auto) { channel = received_channel; },
+      dispatcher());
+
+  OpenIncomingChannel(kHandle1, server_channel);
+  RunLoopUntilIdle();
+  EXPECT_TRUE(channel);
+
+  DLCI dlci = ServerChannelToDLCI(server_channel, OppositeRole(state.role));
+
+  common::ByteBufferPtr received_data;
+  channel->Activate(
+      [&received_data](auto data) { received_data = std::move(data); }, []() {},
+      dispatcher());
+
+  auto pattern = common::CreateStaticByteBuffer(1, 2, 3, 4);
+  auto buffer = std::make_unique<common::DynamicByteBuffer>(pattern);
+  channel->Send(std::move(buffer));
+  RunLoopUntilIdle();
+
+  auto frame =
+      Frame::Parse(state.credit_based_flow, OppositeRole(state.role),
+                   handle_to_incoming_frames_[kHandle1].front()->view());
+  EXPECT_TRUE(frame);
+  EXPECT_EQ(FrameType::kUnnumberedInfoHeaderCheck,
+            static_cast<FrameType>(frame->control()));
+  EXPECT_EQ(dlci, frame->dlci());
+  EXPECT_EQ(pattern,
+            *static_cast<UserDataFrame*>(frame.get())->TakeInformation());
+
+  buffer = std::make_unique<common::DynamicByteBuffer>(pattern);
+  ReceiveFrame(kHandle1, std::make_unique<UserDataFrame>(
+                             state.role, state.credit_based_flow, dlci,
+                             std::move(buffer)));
+  RunLoopUntilIdle();
+
+  EXPECT_TRUE(received_data);
+  EXPECT_EQ(pattern, *received_data);
+}
+
 }  // namespace
 }  // namespace rfcomm
 }  // namespace btlib
diff --git a/drivers/bluetooth/lib/rfcomm/session.cc b/drivers/bluetooth/lib/rfcomm/session.cc
index c907fcf..23e3dd0 100644
--- a/drivers/bluetooth/lib/rfcomm/session.cc
+++ b/drivers/bluetooth/lib/rfcomm/session.cc
@@ -4,9 +4,9 @@
 
 #include <lib/async/default.h>
 
-#include "garnet/drivers/bluetooth/lib/rfcomm/session.h"
 #include "garnet/drivers/bluetooth/lib/common/slab_allocator.h"
 #include "garnet/drivers/bluetooth/lib/rfcomm/rfcomm.h"
+#include "garnet/drivers/bluetooth/lib/rfcomm/session.h"
 
 namespace btlib {
 namespace rfcomm {
@@ -66,8 +66,25 @@
   }
 }
 
+// Returns true if this user DLCI "belongs to" the side of the session with the
+// given |role|. See RFCOMM 5.2: "...this partitions the DLCI value space such
+// that server applications on the non-initiating device are reachable on DLCIs
+// 2,4,6,...,60, and server applications on the initiating device are reachable
+// on 3,5,7,...,61."
+constexpr bool IsValidLocalChannel(Role role, DLCI dlci) {
+  FXL_DCHECK(IsMultiplexerStarted(role));
+  FXL_DCHECK(IsUserDLCI(dlci));
+  return (role == Role::kInitiator ? 1 : 0) == dlci % 2;
+}
+
 }  // namespace
 
+void Session::SendUserData(DLCI dlci, common::ByteBufferPtr data) {
+  bool sent = SendFrame(std::make_unique<UserDataFrame>(
+      role_, credit_based_flow_, dlci, std::move(data)));
+  FXL_DCHECK(sent);
+}
+
 std::unique_ptr<Session> Session::Create(
     fbl::RefPtr<l2cap::Channel> l2cap_channel,
     ChannelOpenedCallback channel_opened_cb) {
@@ -115,7 +132,10 @@
     return;
   }
 
-  DLCI dlci = ServerChannelToDLCI(server_channel, role_);
+  // RFCOMM 5.4: An RFCOMM entity making a new DLC on an existing session forms
+  // the DLCI by combining the Server Channel for the application on the other
+  // device, and the inverse of its own direction bit for the session."
+  DLCI dlci = ServerChannelToDLCI(server_channel, OppositeRole(role_));
 
   if (initial_param_negotiation_state_ !=
       ParameterNegotiationState::kNegotiated) {
@@ -132,15 +152,40 @@
     return;
   }
 
-  // TODO(NET-1013): implement channel open
+  SendCommand(
+      FrameType::kSetAsynchronousBalancedMode, dlci,
+      [this, dlci, server_channel, cb = std::move(channel_opened_cb)](
+          std::unique_ptr<Frame> response) mutable {
+        FXL_DCHECK(response);
+        FrameType type = FrameType(response->control());
+        fbl::RefPtr<Channel> new_channel;
+        switch (type) {
+          case FrameType::kUnnumberedAcknowledgement: {
+            FXL_LOG(INFO) << "rfcomm: Channel " << static_cast<unsigned>(dlci)
+                          << " started successfully";
+            new_channel = fbl::AdoptRef(new internal::ChannelImpl(dlci, this));
+            FXL_DCHECK(channels_.find(dlci) == channels_.end());
+            channels_.emplace(dlci, new_channel);
+            break;
+          }
+          case FrameType::kDisconnectedMode:
+            FXL_LOG(WARNING)
+                << "rfcomm: Channel " << static_cast<unsigned>(dlci)
+                << " failed to start";
+            break;
+          default:
+            FXL_LOG(WARNING) << "rfcomm: Unexpected response to SABM: "
+                             << static_cast<unsigned>(type);
+            break;
+        }
 
-  // Return a nullptr channel if we fail to open
-  FXL_LOG(WARNING) << "Not implemented";
-  async::PostTask(dispatcher_, [cb = std::move(channel_opened_cb)] {
-    cb(nullptr, kInvalidServerChannel);
-  });
+        // Send the result.
+        async::PostTask(dispatcher_,
+                        [server_channel, new_channel, cb_ = std::move(cb)] {
+                          cb_(new_channel, server_channel);
+                        });
+      });
 }
-
 void Session::RxCallback(const l2cap::SDU& sdu) {
   l2cap::PDU::Reader reader(&sdu);
   reader.ReadNext(sdu.length(), [&](const common::ByteBuffer& buffer) {
@@ -182,8 +227,19 @@
           HandleMuxCommand(
               static_cast<MuxCommandFrame*>(frame.get())->TakeMuxCommand());
           return;
+        } else if (IsUserDLCI(dlci)) {
+          auto channel_it = channels_.find(dlci);
+          if (channel_it == channels_.end()) {
+            FXL_LOG(WARNING) << "rfcomm: User data received for unopened DLCI "
+                             << static_cast<unsigned>(dlci);
+            return;
+          }
+          channel_it->second->Receive(
+              static_cast<UserDataFrame*>(frame.get())->TakeInformation());
+          return;
         } else {
-          FXL_NOTIMPLEMENTED();
+          FXL_LOG(WARNING) << "rfcomm: UIH frame on invalid DLCI "
+                           << static_cast<unsigned>(dlci);
         }
       }
 
@@ -421,17 +477,53 @@
       case Role::kInitiator:
       case Role::kResponder:
         // TODO(gusss): should we send a DM in this case?
-        FXL_LOG(WARNING) << "rfcomm: Request to start alreadys started"
+        FXL_LOG(WARNING) << "rfcomm: Request to start already started"
                          << " multiplexer";
         return;
       default:
         FXL_NOTREACHED();
         return;
     }
-  } else {
-    // TODO(NET-1014): open channel when requested by remote peer
-    FXL_NOTIMPLEMENTED();
   }
+
+  // If it isn't a multiplexer startup request, it must be a request for a user
+  // channel.
+
+  // TODO(NET-1301): unit test this case.
+  if (!IsUserDLCI(dlci) || !IsValidLocalChannel(role_, dlci)) {
+    FXL_LOG(WARNING) << "rfcomm: Remote requested invalid DLCI "
+                     << static_cast<unsigned>(dlci);
+    SendResponse(FrameType::kDisconnectedMode, dlci);
+    return;
+  }
+
+  // TODO(NET-1301): unit test this case.
+  if (channels_.find(dlci) != channels_.end()) {
+    // If the channel is already open, the remote is confused about the state of
+    // the Session. Send a DM and a DISC for that channel.
+    // TODO(NET-1274): do we want to just shut down the whole session here?
+    // Things would be in a nasty state at this point.
+    FXL_LOG(WARNING) << "rfcomm: Remote requested already open channel";
+    SendResponse(FrameType::kDisconnectedMode, dlci);
+    SendCommand(FrameType::kDisconnect, dlci, [](auto response) {
+      // TODO(NET-1273): implement clean channel close + state reset
+    });
+
+    return;
+  }
+
+  // Start the channel by first responding positively.
+  SendResponse(FrameType::kUnnumberedAcknowledgement, dlci);
+
+  // Now form the channel and pass it off.
+  auto channel = fbl::AdoptRef(new internal::ChannelImpl(dlci, this));
+  channels_.emplace(dlci, channel);
+  async::PostTask(dispatcher_, [this, dlci, channel] {
+    channel_opened_cb_(channel, DLCIToServerChannel(dlci));
+  });
+
+  FXL_LOG(INFO) << "rfcomm: Remote peer opened channel with DLCI "
+                << static_cast<unsigned>(dlci);
 }
 
 void Session::HandleMuxCommand(std::unique_ptr<MuxCommand> mux_command) {
diff --git a/drivers/bluetooth/lib/rfcomm/session.h b/drivers/bluetooth/lib/rfcomm/session.h
index 8220877..e520a16 100644
--- a/drivers/bluetooth/lib/rfcomm/session.h
+++ b/drivers/bluetooth/lib/rfcomm/session.h
@@ -25,7 +25,7 @@
 
 class Session {
  public:
-  void Send(DLCI dlci, common::ByteBufferPtr data);
+  void SendUserData(DLCI dlci, common::ByteBufferPtr data);
 
  private:
   friend class ChannelManager;
@@ -138,7 +138,7 @@
   bool credit_based_flow_;
 
   // Keeps track of opened channels.
-  std::unordered_map<DLCI, fxl::WeakPtr<l2cap::Channel>> channels_;
+  std::unordered_map<DLCI, fbl::RefPtr<Channel>> channels_;
 
   // Called when the remote peer opens a new incoming channel. The session
   // object constructs a new channel and then passes ownership of the channel