From: Loic Blot Date: Tue, 31 Mar 2015 08:35:51 +0000 (+0200) Subject: Connection::Receive(): receive Network Packet instead of SharedBuffer. X-Git-Tag: 0.4.13~406 X-Git-Url: https://git.librecmc.org/?a=commitdiff_plain;h=1fe4256462826c218ed9bf171be4c07e0db33e25;p=oweals%2Fminetest.git Connection::Receive(): receive Network Packet instead of SharedBuffer. Because we get a Buffer from ConnectionEvent, don't convert it to SharedBuffer and return it to Server/Client::Receive which will convert it to NetworkPacket Instead, put the Buffer directly to NetworkPacket and return it to packet processing This remove a long existing memory copy Also check the packet size directly into Connection::Receive instead of packet processing --- diff --git a/src/client.cpp b/src/client.cpp index 611a73bb1..dc2b54e9b 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -834,10 +834,9 @@ void Client::ReceiveAll() void Client::Receive() { DSTACK(__FUNCTION_NAME); - SharedBuffer data; - u16 sender_peer_id; - u32 datasize = m_con.Receive(sender_peer_id, data); - ProcessData(*data, datasize, sender_peer_id); + NetworkPacket pkt; + m_con.Receive(&pkt); + ProcessData(&pkt); } inline void Client::handleCommand(NetworkPacket* pkt) @@ -849,19 +848,12 @@ inline void Client::handleCommand(NetworkPacket* pkt) /* sender_peer_id given to this shall be quaranteed to be a valid peer */ -void Client::ProcessData(u8 *data, u32 datasize, u16 sender_peer_id) +void Client::ProcessData(NetworkPacket *pkt) { DSTACK(__FUNCTION_NAME); - // Ignore packets that don't even fit a command - if(datasize < 2) { - m_packetcounter.add(60000); - return; - } - - NetworkPacket pkt(data, datasize, sender_peer_id); - - ToClientCommand command = (ToClientCommand) pkt.getCommand(); + ToClientCommand command = (ToClientCommand) pkt->getCommand(); + u32 sender_peer_id = pkt->getPeerId(); //infostream<<"Client: received command="< &data) +void Connection::Receive(NetworkPacket* pkt) { for(;;) { ConnectionEvent e = waitEvent(m_bc_receive_timeout); if (e.type != CONNEVENT_NONE) - LOG(dout_con<(e.data); - return e.data.getSize(); + // Data size is lesser than command size, ignoring packet + if (e.data.getSize() < 2) { + continue; + } + + pkt->putRawPacket(*e.data, e.data.getSize(), e.peer_id); + return; case CONNEVENT_PEER_ADDED: { UDPPeer tmp(e.peer_id, e.address, this); if (m_bc_peerhandler) m_bc_peerhandler->peerAdded(&tmp); - continue; } + continue; + } case CONNEVENT_PEER_REMOVED: { UDPPeer tmp(e.peer_id, e.address, this); if (m_bc_peerhandler) m_bc_peerhandler->deletingPeer(&tmp, e.timeout); - continue; } + continue; + } case CONNEVENT_BIND_FAILED: throw ConnectionBindFailed("Failed to bind socket " "(port already in use?)"); diff --git a/src/network/connection.h b/src/network/connection.h index 9c920cc01..f60c66257 100644 --- a/src/network/connection.h +++ b/src/network/connection.h @@ -34,6 +34,8 @@ with this program; if not, write to the Free Software Foundation, Inc., #include #include +class NetworkPacket; + namespace con { @@ -1025,7 +1027,7 @@ public: void Connect(Address address); bool Connected(); void Disconnect(); - u32 Receive(u16 &peer_id, SharedBuffer &data); + void Receive(NetworkPacket* pkt); void Send(u16 peer_id, u8 channelnum, NetworkPacket* pkt, bool reliable); u16 GetPeerID() { return m_peer_id; } Address GetPeerAddress(u16 peer_id); diff --git a/src/network/networkpacket.cpp b/src/network/networkpacket.cpp index 85d39d91d..d7487af40 100644 --- a/src/network/networkpacket.cpp +++ b/src/network/networkpacket.cpp @@ -22,17 +22,6 @@ with this program; if not, write to the Free Software Foundation, Inc., #include "exceptions.h" #include "util/serialize.h" -NetworkPacket::NetworkPacket(u8 *data, u32 datasize, u16 peer_id): -m_read_offset(0), m_peer_id(peer_id) -{ - m_read_offset = 0; - m_datasize = datasize - 2; - - // split command and datas - m_command = readU16(&data[0]); - m_data = std::vector(&data[2], &data[2 + m_datasize]); -} - NetworkPacket::NetworkPacket(u16 command, u32 datasize, u16 peer_id): m_datasize(datasize), m_read_offset(0), m_command(command), m_peer_id(peer_id) { @@ -50,6 +39,20 @@ NetworkPacket::~NetworkPacket() m_data.clear(); } +void NetworkPacket::putRawPacket(u8 *data, u32 datasize, u16 peer_id) +{ + // If a m_command is already set, we are rewriting on same packet + // This is not permitted + assert(m_command == 0); + + m_datasize = datasize - 2; + m_peer_id = peer_id; + + // split command and datas + m_command = readU16(&data[0]); + m_data = std::vector(&data[2], &data[2 + m_datasize]); +} + char* NetworkPacket::getString(u32 from_offset) { if (from_offset >= m_datasize) diff --git a/src/network/networkpacket.h b/src/network/networkpacket.h index 4a801b444..0afb1e7e3 100644 --- a/src/network/networkpacket.h +++ b/src/network/networkpacket.h @@ -28,11 +28,14 @@ class NetworkPacket { public: - NetworkPacket(u8 *data, u32 datasize, u16 peer_id); NetworkPacket(u16 command, u32 datasize, u16 peer_id); NetworkPacket(u16 command, u32 datasize); + NetworkPacket(): m_datasize(0), m_read_offset(0), m_command(0), + m_peer_id(0) {} ~NetworkPacket(); + void putRawPacket(u8 *data, u32 datasize, u16 peer_id); + // Getters u32 getSize() { return m_datasize; } u16 getPeerId() { return m_peer_id; } diff --git a/src/server.cpp b/src/server.cpp index 3b4c3c3f0..144d933b1 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -1018,10 +1018,11 @@ void Server::Receive() DSTACK(__FUNCTION_NAME); SharedBuffer data; u16 peer_id; - u32 datasize; try { - datasize = m_con.Receive(peer_id,data); - ProcessData(*data, datasize, peer_id); + NetworkPacket pkt; + m_con.Receive(&pkt); + peer_id = pkt.getPeerId(); + ProcessData(&pkt); } catch(con::InvalidIncomingDataException &e) { infostream<<"Server::Receive(): " @@ -1149,13 +1150,14 @@ inline void Server::handleCommand(NetworkPacket* pkt) (this->*opHandle.handler)(pkt); } -void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id) +void Server::ProcessData(NetworkPacket *pkt) { DSTACK(__FUNCTION_NAME); // Environment is locked first. JMutexAutoLock envlock(m_env_mutex); ScopeProfiler sp(g_profiler, "Server::ProcessData"); + u32 peer_id = pkt->getPeerId(); try { Address address = getPeerAddress(peer_id); @@ -1179,18 +1181,13 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id) * respond for some time, your server was overloaded or * things like that. */ - infostream << "Server::ProcessData(): Cancelling: peer " + infostream << "Server::ProcessData(): Canceling: peer " << peer_id << " not found" << std::endl; return; } try { - if(datasize < 2) - return; - - NetworkPacket pkt(data, datasize, peer_id); - - ToServerCommand command = (ToServerCommand) pkt.getCommand(); + ToServerCommand command = (ToServerCommand) pkt->getCommand(); // Command must be handled into ToServerCommandHandler if (command >= TOSERVER_NUM_MSG_TYPES) { @@ -1199,7 +1196,7 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id) } if (toServerCommandTable[command].state == TOSERVER_STATE_NOT_CONNECTED) { - handleCommand(&pkt); + handleCommand(pkt); return; } @@ -1214,7 +1211,7 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id) /* Handle commands related to client startup */ if (toServerCommandTable[command].state == TOSERVER_STATE_STARTUP) { - handleCommand(&pkt); + handleCommand(pkt); return; } @@ -1227,7 +1224,7 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id) return; } - handleCommand(&pkt); + handleCommand(pkt); } catch(SendFailedException &e) { errorstream << "Server::ProcessData(): SendFailedException: " diff --git a/src/server.h b/src/server.h index 0c0a5f91c..a584cbe5a 100644 --- a/src/server.h +++ b/src/server.h @@ -219,7 +219,7 @@ public: void handleCommand_NodeMetaFields(NetworkPacket* pkt); void handleCommand_InventoryFields(NetworkPacket* pkt); - void ProcessData(u8 *data, u32 datasize, u16 peer_id); + void ProcessData(NetworkPacket *pkt); void Send(NetworkPacket* pkt); diff --git a/src/test.cpp b/src/test.cpp index 32a6b102f..258093278 100644 --- a/src/test.cpp +++ b/src/test.cpp @@ -1939,13 +1939,12 @@ struct TestConnection: public TestBase try { - u16 peer_id; - SharedBuffer data; - infostream<<"** running client.Receive()"< sentdata = pkt.oldForgePacket(); + Buffer sentdata = pkt.oldForgePacket(); infostream<<"** running client.Send()"< recvdata; + NetworkPacket recvpacket; infostream << "** running server.Receive()" << std::endl; - u32 size = server.Receive(peer_id, recvdata); - infostream << "** Server received: peer_id=" << peer_id - << ", size=" << size + server.Receive(&recvpacket); + infostream << "** Server received: peer_id=" << pkt.getPeerId() + << ", size=" << pkt.getSize() << ", data=" << (const char*)pkt.getU8Ptr(0) << std::endl; + Buffer recvdata = pkt.oldForgePacket(); + UASSERT(memcmp(*sentdata, *recvdata, recvdata.getSize()) == 0); } @@ -2061,29 +2059,33 @@ struct TestConnection: public TestBase snprintf(buf, 10, "%.2X", ((int)((const char*)pkt.getU8Ptr(0))[i])&0xff); infostream<20) - infostream<<"..."; - infostream< 20) + infostream << "..."; + infostream << std::endl; - SharedBuffer sentdata = pkt.oldForgePacket(); + Buffer sentdata = pkt.oldForgePacket(); server.Send(peer_id_client, 0, &pkt, true); //sleep_ms(3000); - SharedBuffer recvdata; - infostream<<"** running client.Receive()"< recvdata; + infostream << "** running client.Receive()" << std::endl; u16 peer_id = 132; u16 size = 0; bool received = false; u32 timems0 = porting::getTimeMs(); - for(;;){ + for(;;) { if(porting::getTimeMs() - timems0 > 5000 || received) break; - try{ - size = client.Receive(peer_id, recvdata); + try { + NetworkPacket pkt; + client.Receive(&pkt); + size = pkt.getSize(); + peer_id = pkt.getPeerId(); + recvdata = pkt.oldForgePacket(); received = true; - }catch(con::NoIncomingDataException &e){ + } catch(con::NoIncomingDataException &e) { } sleep_ms(10); }