Skip to content

Commit 1fe4256

Browse files
nerzhulnerzhul
committedMar 31, 2015
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
·
5.11.00.4.13
1 parent ab77bf9 commit 1fe4256

File tree

9 files changed

+104
-98
lines changed

9 files changed

+104
-98
lines changed
 

Diff for: ‎src/client.cpp

+8-16
Original file line numberDiff line numberDiff line change
@@ -834,10 +834,9 @@ void Client::ReceiveAll()
834834
void Client::Receive()
835835
{
836836
DSTACK(__FUNCTION_NAME);
837-
SharedBuffer<u8> data;
838-
u16 sender_peer_id;
839-
u32 datasize = m_con.Receive(sender_peer_id, data);
840-
ProcessData(*data, datasize, sender_peer_id);
837+
NetworkPacket pkt;
838+
m_con.Receive(&pkt);
839+
ProcessData(&pkt);
841840
}
842841

843842
inline void Client::handleCommand(NetworkPacket* pkt)
@@ -849,19 +848,12 @@ inline void Client::handleCommand(NetworkPacket* pkt)
849848
/*
850849
sender_peer_id given to this shall be quaranteed to be a valid peer
851850
*/
852-
void Client::ProcessData(u8 *data, u32 datasize, u16 sender_peer_id)
851+
void Client::ProcessData(NetworkPacket *pkt)
853852
{
854853
DSTACK(__FUNCTION_NAME);
855854

856-
// Ignore packets that don't even fit a command
857-
if(datasize < 2) {
858-
m_packetcounter.add(60000);
859-
return;
860-
}
861-
862-
NetworkPacket pkt(data, datasize, sender_peer_id);
863-
864-
ToClientCommand command = (ToClientCommand) pkt.getCommand();
855+
ToClientCommand command = (ToClientCommand) pkt->getCommand();
856+
u32 sender_peer_id = pkt->getPeerId();
865857

866858
//infostream<<"Client: received command="<<command<<std::endl;
867859
m_packetcounter.add((u16)command);
@@ -889,7 +881,7 @@ void Client::ProcessData(u8 *data, u32 datasize, u16 sender_peer_id)
889881
* as a byte mask
890882
*/
891883
if(toClientCommandTable[command].state == TOCLIENT_STATE_NOT_CONNECTED) {
892-
handleCommand(&pkt);
884+
handleCommand(pkt);
893885
return;
894886
}
895887

@@ -904,7 +896,7 @@ void Client::ProcessData(u8 *data, u32 datasize, u16 sender_peer_id)
904896
Handle runtime commands
905897
*/
906898

907-
handleCommand(&pkt);
899+
handleCommand(pkt);
908900
}
909901

910902
void Client::Send(NetworkPacket* pkt)

Diff for: ‎src/client.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ class Client : public con::PeerHandler, public InventoryManager, public IGameDef
392392
void handleCommand_LocalPlayerAnimations(NetworkPacket* pkt);
393393
void handleCommand_EyeOffset(NetworkPacket* pkt);
394394

395-
void ProcessData(u8 *data, u32 datasize, u16 sender_peer_id);
395+
void ProcessData(NetworkPacket *pkt);
396396

397397
// Returns true if something was received
398398
bool AsyncProcessPacket();

Diff for: ‎src/network/connection.cpp

+15-8
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
2424
#include "serialization.h"
2525
#include "log.h"
2626
#include "porting.h"
27+
#include "network/networkpacket.h"
2728
#include "util/serialize.h"
2829
#include "util/numeric.h"
2930
#include "util/string.h"
@@ -2884,30 +2885,36 @@ void Connection::Disconnect()
28842885
putCommand(c);
28852886
}
28862887

2887-
u32 Connection::Receive(u16 &peer_id, SharedBuffer<u8> &data)
2888+
void Connection::Receive(NetworkPacket* pkt)
28882889
{
28892890
for(;;) {
28902891
ConnectionEvent e = waitEvent(m_bc_receive_timeout);
28912892
if (e.type != CONNEVENT_NONE)
2892-
LOG(dout_con<<getDesc()<<": Receive: got event: "
2893-
<<e.describe()<<std::endl);
2893+
LOG(dout_con << getDesc() << ": Receive: got event: "
2894+
<< e.describe() << std::endl);
28942895
switch(e.type) {
28952896
case CONNEVENT_NONE:
28962897
throw NoIncomingDataException("No incoming data");
28972898
case CONNEVENT_DATA_RECEIVED:
2898-
peer_id = e.peer_id;
2899-
data = SharedBuffer<u8>(e.data);
2900-
return e.data.getSize();
2899+
// Data size is lesser than command size, ignoring packet
2900+
if (e.data.getSize() < 2) {
2901+
continue;
2902+
}
2903+
2904+
pkt->putRawPacket(*e.data, e.data.getSize(), e.peer_id);
2905+
return;
29012906
case CONNEVENT_PEER_ADDED: {
29022907
UDPPeer tmp(e.peer_id, e.address, this);
29032908
if (m_bc_peerhandler)
29042909
m_bc_peerhandler->peerAdded(&tmp);
2905-
continue; }
2910+
continue;
2911+
}
29062912
case CONNEVENT_PEER_REMOVED: {
29072913
UDPPeer tmp(e.peer_id, e.address, this);
29082914
if (m_bc_peerhandler)
29092915
m_bc_peerhandler->deletingPeer(&tmp, e.timeout);
2910-
continue; }
2916+
continue;
2917+
}
29112918
case CONNEVENT_BIND_FAILED:
29122919
throw ConnectionBindFailed("Failed to bind socket "
29132920
"(port already in use?)");

Diff for: ‎src/network/connection.h

+3-1
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ with this program; if not, write to the Free Software Foundation, Inc.,
3434
#include <list>
3535
#include <map>
3636

37+
class NetworkPacket;
38+
3739
namespace con
3840
{
3941

@@ -1025,7 +1027,7 @@ class Connection
10251027
void Connect(Address address);
10261028
bool Connected();
10271029
void Disconnect();
1028-
u32 Receive(u16 &peer_id, SharedBuffer<u8> &data);
1030+
void Receive(NetworkPacket* pkt);
10291031
void Send(u16 peer_id, u8 channelnum, NetworkPacket* pkt, bool reliable);
10301032
u16 GetPeerID() { return m_peer_id; }
10311033
Address GetPeerAddress(u16 peer_id);

Diff for: ‎src/network/networkpacket.cpp

+14-11
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,6 @@ with this program; if not, write to the Free Software Foundation, Inc.,
2222
#include "exceptions.h"
2323
#include "util/serialize.h"
2424

25-
NetworkPacket::NetworkPacket(u8 *data, u32 datasize, u16 peer_id):
26-
m_read_offset(0), m_peer_id(peer_id)
27-
{
28-
m_read_offset = 0;
29-
m_datasize = datasize - 2;
30-
31-
// split command and datas
32-
m_command = readU16(&data[0]);
33-
m_data = std::vector<u8>(&data[2], &data[2 + m_datasize]);
34-
}
35-
3625
NetworkPacket::NetworkPacket(u16 command, u32 datasize, u16 peer_id):
3726
m_datasize(datasize), m_read_offset(0), m_command(command), m_peer_id(peer_id)
3827
{
@@ -50,6 +39,20 @@ NetworkPacket::~NetworkPacket()
5039
m_data.clear();
5140
}
5241

42+
void NetworkPacket::putRawPacket(u8 *data, u32 datasize, u16 peer_id)
43+
{
44+
// If a m_command is already set, we are rewriting on same packet
45+
// This is not permitted
46+
assert(m_command == 0);
47+
48+
m_datasize = datasize - 2;
49+
m_peer_id = peer_id;
50+
51+
// split command and datas
52+
m_command = readU16(&data[0]);
53+
m_data = std::vector<u8>(&data[2], &data[2 + m_datasize]);
54+
}
55+
5356
char* NetworkPacket::getString(u32 from_offset)
5457
{
5558
if (from_offset >= m_datasize)

Diff for: ‎src/network/networkpacket.h

+4-1
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,14 @@ class NetworkPacket
2828
{
2929

3030
public:
31-
NetworkPacket(u8 *data, u32 datasize, u16 peer_id);
3231
NetworkPacket(u16 command, u32 datasize, u16 peer_id);
3332
NetworkPacket(u16 command, u32 datasize);
33+
NetworkPacket(): m_datasize(0), m_read_offset(0), m_command(0),
34+
m_peer_id(0) {}
3435
~NetworkPacket();
3536

37+
void putRawPacket(u8 *data, u32 datasize, u16 peer_id);
38+
3639
// Getters
3740
u32 getSize() { return m_datasize; }
3841
u16 getPeerId() { return m_peer_id; }

Diff for: ‎src/server.cpp

+11-14
Original file line numberDiff line numberDiff line change
@@ -1018,10 +1018,11 @@ void Server::Receive()
10181018
DSTACK(__FUNCTION_NAME);
10191019
SharedBuffer<u8> data;
10201020
u16 peer_id;
1021-
u32 datasize;
10221021
try {
1023-
datasize = m_con.Receive(peer_id,data);
1024-
ProcessData(*data, datasize, peer_id);
1022+
NetworkPacket pkt;
1023+
m_con.Receive(&pkt);
1024+
peer_id = pkt.getPeerId();
1025+
ProcessData(&pkt);
10251026
}
10261027
catch(con::InvalidIncomingDataException &e) {
10271028
infostream<<"Server::Receive(): "
@@ -1149,13 +1150,14 @@ inline void Server::handleCommand(NetworkPacket* pkt)
11491150
(this->*opHandle.handler)(pkt);
11501151
}
11511152

1152-
void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
1153+
void Server::ProcessData(NetworkPacket *pkt)
11531154
{
11541155
DSTACK(__FUNCTION_NAME);
11551156
// Environment is locked first.
11561157
JMutexAutoLock envlock(m_env_mutex);
11571158

11581159
ScopeProfiler sp(g_profiler, "Server::ProcessData");
1160+
u32 peer_id = pkt->getPeerId();
11591161

11601162
try {
11611163
Address address = getPeerAddress(peer_id);
@@ -1179,18 +1181,13 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
11791181
* respond for some time, your server was overloaded or
11801182
* things like that.
11811183
*/
1182-
infostream << "Server::ProcessData(): Cancelling: peer "
1184+
infostream << "Server::ProcessData(): Canceling: peer "
11831185
<< peer_id << " not found" << std::endl;
11841186
return;
11851187
}
11861188

11871189
try {
1188-
if(datasize < 2)
1189-
return;
1190-
1191-
NetworkPacket pkt(data, datasize, peer_id);
1192-
1193-
ToServerCommand command = (ToServerCommand) pkt.getCommand();
1190+
ToServerCommand command = (ToServerCommand) pkt->getCommand();
11941191

11951192
// Command must be handled into ToServerCommandHandler
11961193
if (command >= TOSERVER_NUM_MSG_TYPES) {
@@ -1199,7 +1196,7 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
11991196
}
12001197

12011198
if (toServerCommandTable[command].state == TOSERVER_STATE_NOT_CONNECTED) {
1202-
handleCommand(&pkt);
1199+
handleCommand(pkt);
12031200
return;
12041201
}
12051202

@@ -1214,7 +1211,7 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
12141211

12151212
/* Handle commands related to client startup */
12161213
if (toServerCommandTable[command].state == TOSERVER_STATE_STARTUP) {
1217-
handleCommand(&pkt);
1214+
handleCommand(pkt);
12181215
return;
12191216
}
12201217

@@ -1227,7 +1224,7 @@ void Server::ProcessData(u8 *data, u32 datasize, u16 peer_id)
12271224
return;
12281225
}
12291226

1230-
handleCommand(&pkt);
1227+
handleCommand(pkt);
12311228
}
12321229
catch(SendFailedException &e) {
12331230
errorstream << "Server::ProcessData(): SendFailedException: "

Diff for: ‎src/server.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ class Server : public con::PeerHandler, public MapEventReceiver,
219219
void handleCommand_NodeMetaFields(NetworkPacket* pkt);
220220
void handleCommand_InventoryFields(NetworkPacket* pkt);
221221

222-
void ProcessData(u8 *data, u32 datasize, u16 peer_id);
222+
void ProcessData(NetworkPacket *pkt);
223223

224224
void Send(NetworkPacket* pkt);
225225

0 commit comments

Comments
 (0)
Please sign in to comment.