crimson/net: cleanup SocketFRef from Socket interfaces

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>

diff --git a/src/crimson/net/Protocol.h b/src/crimson/net/Protocol.h
index ca1b08896e1c78439c2ae50008fa05b13cab8881..4df2549c37c9a4c51dde917d111fc4daf58699dc 100644 (file)
--- a/src/crimson/net/Protocol.h
+++ b/src/crimson/net/Protocol.h
@@ -32,7 +32,7 @@ class Protocol {
   virtual void start_connect(const entity_addr_t& peer_addr,
                              const entity_type_t& peer_type) = 0;
 
-  virtual void start_accept(SocketFRef&& socket,
+  virtual void start_accept(SocketRef&& socket,
                             const entity_addr_t& peer_addr) = 0;
 
  protected:
@@ -58,7 +58,7 @@ class Protocol {
   Dispatcher &dispatcher;
   SocketConnection &conn;
 
-  SocketFRef socket;
+  SocketRef socket;
   seastar::gate pending_dispatch;
   AuthConnectionMetaRef auth_meta;
 

diff --git a/src/crimson/net/ProtocolV1.cc b/src/crimson/net/ProtocolV1.cc
index 9bd41a8fe2e3a7b9c8c08d65284f813ca3c55aa9..e079a290916353c1863c946b31b7fe825e729c3b 100644 (file)
--- a/src/crimson/net/ProtocolV1.cc
+++ b/src/crimson/net/ProtocolV1.cc
@@ -320,7 +320,7 @@ void ProtocolV1::start_connect(const entity_addr_t& _peer_addr,
     seastar::static_pointer_cast<SocketConnection>(conn.shared_from_this()));
   (void) seastar::with_gate(pending_dispatch, [this] {
       return Socket::connect(conn.peer_addr)
-        .then([this](SocketFRef sock) {
+        .then([this](SocketRef sock) {
           socket = std::move(sock);
           if (state == state_t::closing) {
             return socket->close().then([] {
@@ -600,7 +600,7 @@ seastar::future<stop_t> ProtocolV1::repeat_handle_connect()
     });
 }
 
-void ProtocolV1::start_accept(SocketFRef&& sock,
+void ProtocolV1::start_accept(SocketRef&& sock,
                               const entity_addr_t& _peer_addr)
 {
   ceph_assert(state == state_t::none);

diff --git a/src/crimson/net/ProtocolV1.h b/src/crimson/net/ProtocolV1.h
index b1b1dbf1918b29600235f42558021ef594e4169d..2ebfcc9ca929046d806077da9af88ea8d6704740 100644 (file)
--- a/src/crimson/net/ProtocolV1.h
+++ b/src/crimson/net/ProtocolV1.h
@@ -21,7 +21,7 @@ class ProtocolV1 final : public Protocol {
   void start_connect(const entity_addr_t& peer_addr,
                      const entity_type_t& peer_type) override;
 
-  void start_accept(SocketFRef&& socket,
+  void start_accept(SocketRef&& socket,
                     const entity_addr_t& peer_addr) override;
 
   void trigger_close() override;

diff --git a/src/crimson/net/ProtocolV2.cc b/src/crimson/net/ProtocolV2.cc
index 36867b07d42ff25269cb42be366e16c9fa190be4..2535584bb6efbb4a5c0d255c2c5614617099fa61 100644 (file)
--- a/src/crimson/net/ProtocolV2.cc
+++ b/src/crimson/net/ProtocolV2.cc
@@ -129,7 +129,7 @@ namespace crimson::net {
 
 #ifdef UNIT_TESTS_BUILT
 void intercept(Breakpoint bp, bp_type_t type,
-               SocketConnection& conn, SocketFRef& socket) {
+               SocketConnection& conn, SocketRef& socket) {
   if (conn.interceptor) {
     auto action = conn.interceptor->intercept(conn, Breakpoint(bp));
     socket->set_trap(type, action, &conn.interceptor->blocker);
@@ -203,7 +203,7 @@ void ProtocolV2::start_connect(const entity_addr_t& _peer_addr,
   execute_connecting();
 }
 
-void ProtocolV2::start_accept(SocketFRef&& sock,
+void ProtocolV2::start_accept(SocketRef&& sock,
                               const entity_addr_t& _peer_addr)
 {
   ceph_assert(state == state_t::NONE);
@@ -923,7 +923,7 @@ void ProtocolV2::execute_connecting()
           }
           INTERCEPT_N_RW(custom_bp_t::SOCKET_CONNECTING);
           return Socket::connect(conn.peer_addr);
-        }).then([this](SocketFRef sock) {
+        }).then([this](SocketRef sock) {
           logger().debug("{} socket connected", conn);
           if (unlikely(state != state_t::CONNECTING)) {
             logger().debug("{} triggered {} during Socket::connect()",
@@ -1739,7 +1739,7 @@ ProtocolV2::send_server_ident()
 
 void ProtocolV2::trigger_replacing(bool reconnect,
                                    bool do_reset,
-                                   SocketFRef&& new_socket,
+                                   SocketRef&& new_socket,
                                    AuthConnectionMetaRef&& new_auth_meta,
                                    ceph::crypto::onwire::rxtx_t new_rxtx,
                                    uint64_t new_peer_global_seq,

diff --git a/src/crimson/net/ProtocolV2.h b/src/crimson/net/ProtocolV2.h
index d3800bc821a1d1a4cc25a3d7c408381be08491e0..0e8f2ff90e823f2d8f73ba96bec5034ac7499297 100644 (file)
--- a/src/crimson/net/ProtocolV2.h
+++ b/src/crimson/net/ProtocolV2.h
@@ -22,7 +22,7 @@ class ProtocolV2 final : public Protocol {
   void start_connect(const entity_addr_t& peer_addr,
                      const entity_type_t& peer_type) override;
 
-  void start_accept(SocketFRef&& socket,
+  void start_accept(SocketRef&& socket,
                     const entity_addr_t& peer_addr) override;
 
   void trigger_close() override;
@@ -182,7 +182,7 @@ class ProtocolV2 final : public Protocol {
   // REPLACING (server)
   void trigger_replacing(bool reconnect,
                          bool do_reset,
-                         SocketFRef&& new_socket,
+                         SocketRef&& new_socket,
                          AuthConnectionMetaRef&& new_auth_meta,
                          ceph::crypto::onwire::rxtx_t new_rxtx,
                          uint64_t new_peer_global_seq,

diff --git a/src/crimson/net/Socket.h b/src/crimson/net/Socket.h
index 95201dfcabfb28950181acd121ed01790c781ecd..2123136bdda28d2c6b6a703624adb961084a67c9 100644 (file)
--- a/src/crimson/net/Socket.h
+++ b/src/crimson/net/Socket.h
@@ -22,7 +22,6 @@ namespace crimson::net {
 
 class Socket;
 using SocketRef = std::unique_ptr<Socket>;
-using SocketFRef = seastar::foreign_ptr<SocketRef>;
 
 class Socket
 {
@@ -60,27 +59,12 @@ class Socket
 
   Socket(Socket&& o) = delete;
 
-  static seastar::future<SocketFRef>
+  static seastar::future<SocketRef>
   connect(const entity_addr_t& peer_addr) {
-    return seastar::connect(peer_addr.in4_addr())
-      .then([] (seastar::connected_socket socket) {
-        return seastar::make_foreign(std::make_unique<Socket>(std::move(socket),
-                                                             construct_tag{}));
-      });
-  }
-
-  static seastar::future<SocketFRef, entity_addr_t>
-  accept(seastar::server_socket& listener) {
-    return listener.accept().then([] (seastar::accept_result accept_result) {
-        auto [socket, paddr] = std::move(accept_result);
-        entity_addr_t peer_addr;
-        peer_addr.set_sockaddr(&paddr.as_posix_sockaddr());
-        peer_addr.set_type(entity_addr_t::TYPE_ANY);
-        return seastar::make_ready_future<SocketFRef, entity_addr_t>(
-          seastar::make_foreign(std::make_unique<Socket>(std::move(socket),
-                                                        construct_tag{})),
-         peer_addr);
-      });
+    return seastar::connect(peer_addr.in4_addr()
+    ).then([] (seastar::connected_socket socket) {
+      return std::make_unique<Socket>(std::move(socket), construct_tag{});
+    });
   }
 
   /// read the requested number of bytes into a bufferlist
@@ -141,7 +125,9 @@ class Socket
 
  public:
   void set_trap(bp_type_t type, bp_action_t action, socket_blocker* blocker_);
+
 #endif
+  friend class FixedCPUServerSocket;
 };
 
 class FixedCPUServerSocket
@@ -218,12 +204,17 @@ public:
       std::ignore = seastar::with_gate(ss.shutdown_gate,
           [&ss, fn_accept = std::move(fn_accept)] () mutable {
         return seastar::keep_doing([&ss, fn_accept = std::move(fn_accept)] () mutable {
-          return Socket::accept(*ss.listener
-          ).then([&ss, fn_accept = std::move(fn_accept)]
-                 (auto socket, entity_addr_t peer_addr) mutable {
+          return ss.listener->accept().then(
+              [&ss, fn_accept = std::move(fn_accept)]
+              (seastar::accept_result accept_result) mutable {
             // assert seastar::listen_options::set_fixed_cpu() works
             assert(seastar::engine().cpu_id() == ss.cpu);
-            SocketRef _socket = socket.release();
+            auto [socket, paddr] = std::move(accept_result);
+            entity_addr_t peer_addr;
+            peer_addr.set_sockaddr(&paddr.as_posix_sockaddr());
+            peer_addr.set_type(entity_addr_t::TYPE_ANY);
+            SocketRef _socket = std::make_unique<Socket>(
+                std::move(socket), Socket::construct_tag{});
             std::ignore = seastar::with_gate(ss.shutdown_gate,
                 [socket = std::move(_socket), peer_addr,
                  &ss, fn_accept = std::move(fn_accept)] () mutable {

diff --git a/src/crimson/net/SocketConnection.cc b/src/crimson/net/SocketConnection.cc
index f6dfc3eddc4bb54347fabc9d543b935dbc9740c9..89d89557ab0b472e3c6c254854e6ca2c2a57c046 100644 (file)
--- a/src/crimson/net/SocketConnection.cc
+++ b/src/crimson/net/SocketConnection.cc
@@ -119,7 +119,7 @@ SocketConnection::start_connect(const entity_addr_t& _peer_addr,
 }
 
 void
-SocketConnection::start_accept(SocketFRef&& sock,
+SocketConnection::start_accept(SocketRef&& sock,
                                const entity_addr_t& _peer_addr)
 {
   protocol->start_accept(std::move(sock), _peer_addr);

diff --git a/src/crimson/net/SocketConnection.h b/src/crimson/net/SocketConnection.h
index e436754c98d18ef67b62177a24e770a32c6feb16..c6f58c7630e05ef9c32458c332f1104d2b1c01cc 100644 (file)
--- a/src/crimson/net/SocketConnection.h
+++ b/src/crimson/net/SocketConnection.h
@@ -98,7 +98,7 @@ class SocketConnection : public Connection {
                      const entity_type_t& peer_type);
   /// start a handshake from the server's perspective,
   /// only call when SocketConnection first construct
-  void start_accept(SocketFRef&& socket,
+  void start_accept(SocketRef&& socket,
                     const entity_addr_t& peer_addr);
 
   bool is_server_side() const {

diff --git a/src/crimson/net/SocketMessenger.cc b/src/crimson/net/SocketMessenger.cc
index b5c58f43999fad06989e3939b4efba04b5757bd0..d5c0f5a38efacc77ffab22be06ce37b2d0508ac2 100644 (file)
--- a/src/crimson/net/SocketMessenger.cc
+++ b/src/crimson/net/SocketMessenger.cc
@@ -125,8 +125,7 @@ seastar::future<> SocketMessenger::start(Dispatcher *disp) {
       assert(seastar::engine().cpu_id() == master_sid);
       SocketConnectionRef conn = seastar::make_shared<SocketConnection>(
           *this, *dispatcher, get_myaddr().is_msgr2());
-      // TODO: use SocketRef
-      conn->start_accept(seastar::make_foreign(std::move(socket)), peer_addr);
+      conn->start_accept(std::move(socket), peer_addr);
       return seastar::now();
     });
   }

diff --git a/src/test/crimson/test_socket.cc b/src/test/crimson/test_socket.cc
index fc2e7ac20b6d51d25483a82302c3800b315e2446..3810f4fd072915f80fc7666746d5bc84dee465cc 100644 (file)
--- a/src/test/crimson/test_socket.cc
+++ b/src/test/crimson/test_socket.cc
@@ -18,10 +18,11 @@ using seastar::future;
 using crimson::net::error;
 using crimson::net::FixedCPUServerSocket;
 using crimson::net::Socket;
-using crimson::net::SocketFRef;
 using crimson::net::SocketRef;
 using crimson::net::stop_t;
 
+using SocketFRef = seastar::foreign_ptr<SocketRef>;
+
 static seastar::logger logger{"crimsontest"};
 static entity_addr_t server_addr = [] {
   entity_addr_t saddr;
@@ -33,7 +34,7 @@ future<SocketRef> socket_connect() {
   logger.debug("socket_connect()...");
   return Socket::connect(server_addr).then([] (auto socket) {
     logger.debug("socket_connect() connected");
-    return socket.release();
+    return socket;
   });
 }