Connection::Receive(): receive Network Packet instead of SharedBuffer…

…<u8>. Because we get a Buffer<u8> from ConnectionEvent, don't convert it to SharedBuffer<u8> and return it to Server/Client::Receive which will convert it to NetworkPacket Instead, put the Buffer<u8> 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
minetest · Mar 31, 2015 · 1fe4256 · 1fe4256
1 parent ab77bf9
commit 1fe4256
Show file tree

Hide file tree

Showing 9 changed files with 104 additions and 98 deletions.
diff --git a/src/client.cpp b/src/client.cpp
@@ -834,10 +834,9 @@ void Client::ReceiveAll()
 void Client::Receive()
 {
 	DSTACK(__FUNCTION_NAME);
-	SharedBuffer<u8> 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="<<command<<std::endl;
 	m_packetcounter.add((u16)command);
@@ -889,7 +881,7 @@ void Client::ProcessData(u8 *data, u32 datasize, u16 sender_peer_id)
 	 * as a byte mask
 	 */
 	if(toClientCommandTable[command].state == TOCLIENT_STATE_NOT_CONNECTED) {
-		handleCommand(&pkt);
+		handleCommand(pkt);
 		return;
 	}
 
@@ -904,7 +896,7 @@ void Client::ProcessData(u8 *data, u32 datasize, u16 sender_peer_id)
 	  Handle runtime commands
 	*/
 
-	handleCommand(&pkt);
+	handleCommand(pkt);
 }
 
 void Client::Send(NetworkPacket* pkt)

diff --git a/src/client.h b/src/client.h
@@ -392,7 +392,7 @@ class Client : public con::PeerHandler, public InventoryManager, public IGameDef
 	void handleCommand_LocalPlayerAnimations(NetworkPacket* pkt);
 	void handleCommand_EyeOffset(NetworkPacket* pkt);
 
-	void ProcessData(u8 *data, u32 datasize, u16 sender_peer_id);
+	void ProcessData(NetworkPacket *pkt);
 
 	// Returns true if something was received
 	bool AsyncProcessPacket();

diff --git a/src/network/connection.cpp b/src/network/connection.cpp
@@ -24,6 +24,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "serialization.h"
 #include "log.h"
 #include "porting.h"
+#include "network/networkpacket.h"
 #include "util/serialize.h"
 #include "util/numeric.h"
 #include "util/string.h"
@@ -2884,30 +2885,36 @@ void Connection::Disconnect()
 	putCommand(c);
 }
 
-u32 Connection::Receive(u16 &peer_id, SharedBuffer<u8> &data)
+void Connection::Receive(NetworkPacket* pkt)
 {
 	for(;;) {
 		ConnectionEvent e = waitEvent(m_bc_receive_timeout);
 		if (e.type != CONNEVENT_NONE)
-			LOG(dout_con<<getDesc()<<": Receive: got event: "
-					<<e.describe()<<std::endl);
+			LOG(dout_con << getDesc() << ": Receive: got event: "
+					<< e.describe() << std::endl);
 		switch(e.type) {
 		case CONNEVENT_NONE:
 			throw NoIncomingDataException("No incoming data");
 		case CONNEVENT_DATA_RECEIVED:
-			peer_id = e.peer_id;
-			data = SharedBuffer<u8>(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
@@ -34,6 +34,8 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include <list>
 #include <map>
 
+class NetworkPacket;
+
 namespace con
 {
 
@@ -1025,7 +1027,7 @@ class Connection
 	void Connect(Address address);
 	bool Connected();
 	void Disconnect();
-	u32 Receive(u16 &peer_id, SharedBuffer<u8> &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
@@ -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<u8>(&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<u8>(&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
@@ -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
@@ -1018,10 +1018,11 @@ void Server::Receive()
 	DSTACK(__FUNCTION_NAME);
 	SharedBuffer<u8> 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
@@ -219,7 +219,7 @@ class Server : public con::PeerHandler, public MapEventReceiver,
 	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);