Skip to content

Commit 3494475

Browse files
sfan5sfan5
authoredApr 8, 2020
Miscellaneous networking improvements (#9611)
fixes #2862
·
5.11.05.3.0
1 parent 143a37e commit 3494475

File tree

5 files changed

+87
-43
lines changed

5 files changed

+87
-43
lines changed
 

‎src/network/clientopcodes.cpp

+14-4
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,16 @@ const ToClientCommandHandler toClientCommandTable[TOCLIENT_NUM_MSG_TYPES] =
126126

127127
const static ServerCommandFactory null_command_factory = { "TOSERVER_NULL", 0, false };
128128

129+
/*
130+
Channels used for Client -> Server communication
131+
2: Notifications back to the server (e.g. GOTBLOCKS)
132+
1: Init and Authentication
133+
0: everything else
134+
135+
Packet order is only guaranteed inside a channel, so packets that operate on
136+
the same objects are *required* to be in the same channel.
137+
*/
138+
129139
const ServerCommandFactory serverCommandFactoryTable[TOSERVER_NUM_MSG_TYPES] =
130140
{
131141
null_command_factory, // 0x00
@@ -143,7 +153,7 @@ const ServerCommandFactory serverCommandFactoryTable[TOSERVER_NUM_MSG_TYPES] =
143153
null_command_factory, // 0x0c
144154
null_command_factory, // 0x0d
145155
null_command_factory, // 0x0e
146-
null_command_factory, // 0x0F
156+
null_command_factory, // 0x0f
147157
null_command_factory, // 0x10
148158
{ "TOSERVER_INIT2", 1, true }, // 0x11
149159
null_command_factory, // 0x12
@@ -186,16 +196,16 @@ const ServerCommandFactory serverCommandFactoryTable[TOSERVER_NUM_MSG_TYPES] =
186196
{ "TOSERVER_PLAYERITEM", 0, true }, // 0x37
187197
{ "TOSERVER_RESPAWN", 0, true }, // 0x38
188198
{ "TOSERVER_INTERACT", 0, true }, // 0x39
189-
{ "TOSERVER_REMOVED_SOUNDS", 1, true }, // 0x3a
199+
{ "TOSERVER_REMOVED_SOUNDS", 2, true }, // 0x3a
190200
{ "TOSERVER_NODEMETA_FIELDS", 0, true }, // 0x3b
191201
{ "TOSERVER_INVENTORY_FIELDS", 0, true }, // 0x3c
192202
null_command_factory, // 0x3d
193203
null_command_factory, // 0x3e
194204
null_command_factory, // 0x3f
195205
{ "TOSERVER_REQUEST_MEDIA", 1, true }, // 0x40
196206
null_command_factory, // 0x41
197-
{ "TOSERVER_BREATH", 0, true }, // 0x42 old TOSERVER_BREATH. Ignored by servers
198-
{ "TOSERVER_CLIENT_READY", 0, true }, // 0x43
207+
null_command_factory, // 0x42
208+
{ "TOSERVER_CLIENT_READY", 1, true }, // 0x43
199209
null_command_factory, // 0x44
200210
null_command_factory, // 0x45
201211
null_command_factory, // 0x46

‎src/network/connection.cpp

+22-13
Original file line numberDiff line numberDiff line change
@@ -924,7 +924,7 @@ UDPPeer::UDPPeer(u16 a_id, Address a_address, Connection* connection) :
924924
Peer(a_address,a_id,connection)
925925
{
926926
for (Channel &channel : channels)
927-
channel.setWindowSize(g_settings->getU16("max_packets_per_iteration"));
927+
channel.setWindowSize(START_RELIABLE_WINDOW_SIZE);
928928
}
929929

930930
bool UDPPeer::getAddress(MTProtocols type,Address& toset)
@@ -975,22 +975,29 @@ void UDPPeer::PutReliableSendCommand(ConnectionCommand &c,
975975
if (m_pending_disconnect)
976976
return;
977977

978-
if ( channels[c.channelnum].queued_commands.empty() &&
978+
Channel &chan = channels[c.channelnum];
979+
980+
if (chan.queued_commands.empty() &&
979981
/* don't queue more packets then window size */
980-
(channels[c.channelnum].queued_reliables.size()
981-
< (channels[c.channelnum].getWindowSize()/2))) {
982+
(chan.queued_reliables.size() < chan.getWindowSize() / 2)) {
982983
LOG(dout_con<<m_connection->getDesc()
983984
<<" processing reliable command for peer id: " << c.peer_id
984985
<<" data size: " << c.data.getSize() << std::endl);
985986
if (!processReliableSendCommand(c,max_packet_size)) {
986-
channels[c.channelnum].queued_commands.push_back(c);
987+
chan.queued_commands.push_back(c);
987988
}
988989
}
989990
else {
990991
LOG(dout_con<<m_connection->getDesc()
991992
<<" Queueing reliable command for peer id: " << c.peer_id
992993
<<" data size: " << c.data.getSize() <<std::endl);
993-
channels[c.channelnum].queued_commands.push_back(c);
994+
chan.queued_commands.push_back(c);
995+
if (chan.queued_commands.size() >= chan.getWindowSize() / 2) {
996+
LOG(derr_con << m_connection->getDesc()
997+
<< "Possible packet stall to peer id: " << c.peer_id
998+
<< " queued_commands=" << chan.queued_commands.size()
999+
<< std::endl);
1000+
}
9941001
}
9951002
}
9961003

@@ -1001,20 +1008,22 @@ bool UDPPeer::processReliableSendCommand(
10011008
if (m_pending_disconnect)
10021009
return true;
10031010

1011+
Channel &chan = channels[c.channelnum];
1012+
10041013
u32 chunksize_max = max_packet_size
10051014
- BASE_HEADER_SIZE
10061015
- RELIABLE_HEADER_SIZE;
10071016

10081017
sanity_check(c.data.getSize() < MAX_RELIABLE_WINDOW_SIZE*512);
10091018

10101019
std::list<SharedBuffer<u8>> originals;
1011-
u16 split_sequence_number = channels[c.channelnum].readNextSplitSeqNum();
1020+
u16 split_sequence_number = chan.readNextSplitSeqNum();
10121021

10131022
if (c.raw) {
10141023
originals.emplace_back(c.data);
10151024
} else {
10161025
makeAutoSplitPacket(c.data, chunksize_max,split_sequence_number, &originals);
1017-
channels[c.channelnum].setNextSplitSeqNum(split_sequence_number);
1026+
chan.setNextSplitSeqNum(split_sequence_number);
10181027
}
10191028

10201029
bool have_sequence_number = true;
@@ -1023,7 +1032,7 @@ bool UDPPeer::processReliableSendCommand(
10231032
volatile u16 initial_sequence_number = 0;
10241033

10251034
for (SharedBuffer<u8> &original : originals) {
1026-
u16 seqnum = channels[c.channelnum].getOutgoingSequenceNumber(have_sequence_number);
1035+
u16 seqnum = chan.getOutgoingSequenceNumber(have_sequence_number);
10271036

10281037
/* oops, we don't have enough sequence numbers to send this packet */
10291038
if (!have_sequence_number)
@@ -1055,10 +1064,10 @@ bool UDPPeer::processReliableSendCommand(
10551064
// << " channel: " << (c.channelnum&0xFF)
10561065
// << " seqnum: " << readU16(&p.data[BASE_HEADER_SIZE+1])
10571066
// << std::endl)
1058-
channels[c.channelnum].queued_reliables.push(p);
1067+
chan.queued_reliables.push(p);
10591068
pcount++;
10601069
}
1061-
sanity_check(channels[c.channelnum].queued_reliables.size() < 0xFFFF);
1070+
sanity_check(chan.queued_reliables.size() < 0xFFFF);
10621071
return true;
10631072
}
10641073

@@ -1073,15 +1082,15 @@ bool UDPPeer::processReliableSendCommand(
10731082
toadd.pop();
10741083

10751084
bool successfully_put_back_sequence_number
1076-
= channels[c.channelnum].putBackSequenceNumber(
1085+
= chan.putBackSequenceNumber(
10771086
(initial_sequence_number+toadd.size() % (SEQNUM_MAX+1)));
10781087

10791088
FATAL_ERROR_IF(!successfully_put_back_sequence_number, "error");
10801089
}
10811090

10821091
// DO NOT REMOVE n_queued! It avoids a deadlock of async locked
10831092
// 'log_message_mutex' and 'm_list_mutex'.
1084-
u32 n_queued = channels[c.channelnum].outgoing_reliables_sent.size();
1093+
u32 n_queued = chan.outgoing_reliables_sent.size();
10851094

10861095
LOG(dout_con<<m_connection->getDesc()
10871096
<< " Windowsize exceeded on reliable sending "

‎src/network/connection.h

+8-8
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,6 @@ struct IncomingSplitPacket
141141
=== NOTES ===
142142
143143
A packet is sent through a channel to a peer with a basic header:
144-
TODO: Should we have a receiver_peer_id also?
145144
Header (7 bytes):
146145
[0] u32 protocol_id
147146
[4] session_t sender_peer_id
@@ -152,8 +151,7 @@ TODO: Should we have a receiver_peer_id also?
152151
value 1 (PEER_ID_SERVER) is reserved for server
153152
these constants are defined in constants.h
154153
channel:
155-
The lower the number, the higher the priority is.
156-
Only channels 0, 1 and 2 exist.
154+
Channel numbers have no intrinsic meaning. Currently only 0, 1, 2 exist.
157155
*/
158156
#define BASE_HEADER_SIZE 7
159157
#define CHANNEL_COUNT 3
@@ -386,12 +384,14 @@ struct ConnectionCommand
386384
}
387385
};
388386

389-
/* maximum window size to use, 0xFFFF is theoretical maximum don't think about
387+
/* maximum window size to use, 0xFFFF is theoretical maximum. don't think about
390388
* touching it, the less you're away from it the more likely data corruption
391389
* will occur
392390
*/
393391
#define MAX_RELIABLE_WINDOW_SIZE 0x8000
394-
/* starting value for window size */
392+
/* starting value for window size */
393+
#define START_RELIABLE_WINDOW_SIZE 0x400
394+
/* minimum value for window size */
395395
#define MIN_RELIABLE_WINDOW_SIZE 0x40
396396

397397
class Channel
@@ -555,15 +555,15 @@ class Peer {
555555

556556
bool isTimedOut(float timeout);
557557

558-
unsigned int m_increment_packets_remaining = 9;
559-
unsigned int m_increment_bytes_remaining = 0;
558+
unsigned int m_increment_packets_remaining = 0;
560559

561560
virtual u16 getNextSplitSequenceNumber(u8 channel) { return 0; };
562561
virtual void setNextSplitSequenceNumber(u8 channel, u16 seqnum) {};
563562
virtual SharedBuffer<u8> addSplitPacket(u8 channel, const BufferedPacket &toadd,
564563
bool reliable)
565564
{
566-
fprintf(stderr,"Peer: addSplitPacket called, this is supposed to be never called!\n");
565+
errorstream << "Peer::addSplitPacket called,"
566+
<< " this is supposed to be never called!" << std::endl;
567567
return SharedBuffer<u8>(0);
568568
};
569569

‎src/network/connectionthreads.cpp

+28-13
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ ConnectionSendThread::ConnectionSendThread(unsigned int max_packet_size,
7373
m_timeout(timeout),
7474
m_max_data_packets_per_iteration(g_settings->getU16("max_packets_per_iteration"))
7575
{
76+
SANITY_CHECK(m_max_data_packets_per_iteration > 1);
7677
}
7778

7879
void *ConnectionSendThread::run()
@@ -107,8 +108,13 @@ void *ConnectionSendThread::run()
107108
curtime = porting::getTimeMs();
108109
float dtime = CALC_DTIME(lasttime, curtime);
109110

110-
/* first do all the reliable stuff */
111+
/* first resend timed-out packets */
111112
runTimeouts(dtime);
113+
if (m_iteration_packets_avaialble == 0) {
114+
LOG(warningstream << m_connection->getDesc()
115+
<< " Packet quota used up after re-sending packets, "
116+
<< "max=" << m_max_data_packets_per_iteration << std::endl);
117+
}
112118

113119
/* translate commands to packets */
114120
ConnectionCommand c = m_connection->m_command_queue.pop_frontNoEx(0);
@@ -121,7 +127,7 @@ void *ConnectionSendThread::run()
121127
c = m_connection->m_command_queue.pop_frontNoEx(0);
122128
}
123129

124-
/* send non reliable packets */
130+
/* send queued packets */
125131
sendPackets(dtime);
126132

127133
END_DEBUG_EXCEPTION_HANDLER
@@ -644,6 +650,9 @@ void ConnectionSendThread::sendPackets(float dtime)
644650
std::list<session_t> pendingDisconnect;
645651
std::map<session_t, bool> pending_unreliable;
646652

653+
const unsigned int peer_packet_quota = m_iteration_packets_avaialble
654+
/ MYMAX(peerIds.size(), 1);
655+
647656
for (session_t peerId : peerIds) {
648657
PeerHelper peer = m_connection->getPeerNoEx(peerId);
649658
//peer may have been removed
@@ -653,8 +662,7 @@ void ConnectionSendThread::sendPackets(float dtime)
653662
<< std::endl);
654663
continue;
655664
}
656-
peer->m_increment_packets_remaining =
657-
m_iteration_packets_avaialble / m_connection->m_peers.size();
665+
peer->m_increment_packets_remaining = peer_packet_quota;
658666

659667
UDPPeer *udpPeer = dynamic_cast<UDPPeer *>(&peer);
660668

@@ -751,23 +759,30 @@ void ConnectionSendThread::sendPackets(float dtime)
751759
}
752760

753761
/* send acks immediately */
754-
if (packet.ack) {
762+
if (packet.ack || peer->m_increment_packets_remaining > 0 || stopRequested()) {
755763
rawSendAsPacket(packet.peer_id, packet.channelnum,
756764
packet.data, packet.reliable);
757-
peer->m_increment_packets_remaining =
758-
MYMIN(0, peer->m_increment_packets_remaining--);
759-
} else if (
760-
(peer->m_increment_packets_remaining > 0) ||
761-
(stopRequested())) {
762-
rawSendAsPacket(packet.peer_id, packet.channelnum,
763-
packet.data, packet.reliable);
764-
peer->m_increment_packets_remaining--;
765+
if (peer->m_increment_packets_remaining > 0)
766+
peer->m_increment_packets_remaining--;
765767
} else {
766768
m_outgoing_queue.push(packet);
767769
pending_unreliable[packet.peer_id] = true;
768770
}
769771
}
770772

773+
if (peer_packet_quota > 0) {
774+
for (session_t peerId : peerIds) {
775+
PeerHelper peer = m_connection->getPeerNoEx(peerId);
776+
if (!peer)
777+
continue;
778+
if (peer->m_increment_packets_remaining == 0) {
779+
LOG(warningstream << m_connection->getDesc()
780+
<< " Packet quota used up for peer_id=" << peerId
781+
<< ", was " << peer_packet_quota << " pkts" << std::endl);
782+
}
783+
}
784+
}
785+
771786
for (session_t peerId : pendingDisconnect) {
772787
if (!pending_unreliable[peerId]) {
773788
m_connection->deletePeer(peerId, false);

‎src/network/serveropcodes.cpp

+15-5
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,16 @@ const ToServerCommandHandler toServerCommandTable[TOSERVER_NUM_MSG_TYPES] =
111111

112112
const static ClientCommandFactory null_command_factory = { "TOCLIENT_NULL", 0, false };
113113

114+
/*
115+
Channels used for Server -> Client communication
116+
2: Bulk data (mapblocks, media, ...)
117+
1: HUD packets
118+
0: everything else
119+
120+
Packet order is only guaranteed inside a channel, so packets that operate on
121+
the same objects are *required* to be in the same channel.
122+
*/
123+
114124
const ClientCommandFactory clientCommandFactoryTable[TOCLIENT_NUM_MSG_TYPES] =
115125
{
116126
null_command_factory, // 0x00
@@ -163,7 +173,7 @@ const ClientCommandFactory clientCommandFactoryTable[TOCLIENT_NUM_MSG_TYPES] =
163173
{ "TOCLIENT_CHAT_MESSAGE", 0, true }, // 0x2F
164174
null_command_factory, // 0x30
165175
{ "TOCLIENT_ACTIVE_OBJECT_REMOVE_ADD", 0, true }, // 0x31
166-
{ "TOCLIENT_ACTIVE_OBJECT_MESSAGES", 0, true }, // 0x32 Special packet, sent by 0 (rel) and 1 (unrel) channel
176+
{ "TOCLIENT_ACTIVE_OBJECT_MESSAGES", 0, true }, // 0x32 (may be sent as unrel over channel 1 too)
167177
{ "TOCLIENT_HP", 0, true }, // 0x33
168178
{ "TOCLIENT_MOVE_PLAYER", 0, true }, // 0x34
169179
{ "TOCLIENT_ACCESS_DENIED_LEGACY", 0, true }, // 0x35
@@ -176,7 +186,7 @@ const ClientCommandFactory clientCommandFactoryTable[TOCLIENT_NUM_MSG_TYPES] =
176186
{ "TOCLIENT_ANNOUNCE_MEDIA", 0, true }, // 0x3C
177187
{ "TOCLIENT_ITEMDEF", 0, true }, // 0x3D
178188
null_command_factory, // 0x3E
179-
{ "TOCLIENT_PLAY_SOUND", 0, true }, // 0x3f
189+
{ "TOCLIENT_PLAY_SOUND", 0, true }, // 0x3f (may be sent as unrel too)
180190
{ "TOCLIENT_STOP_SOUND", 0, true }, // 0x40
181191
{ "TOCLIENT_PRIVILEGES", 0, true }, // 0x41
182192
{ "TOCLIENT_INVENTORY_FORMSPEC", 0, true }, // 0x42
@@ -188,9 +198,9 @@ const ClientCommandFactory clientCommandFactoryTable[TOCLIENT_NUM_MSG_TYPES] =
188198
null_command_factory, // 0x48
189199
{ "TOCLIENT_HUDADD", 1, true }, // 0x49
190200
{ "TOCLIENT_HUDRM", 1, true }, // 0x4a
191-
{ "TOCLIENT_HUDCHANGE", 0, true }, // 0x4b
192-
{ "TOCLIENT_HUD_SET_FLAGS", 0, true }, // 0x4c
193-
{ "TOCLIENT_HUD_SET_PARAM", 0, true }, // 0x4d
201+
{ "TOCLIENT_HUDCHANGE", 1, true }, // 0x4b
202+
{ "TOCLIENT_HUD_SET_FLAGS", 1, true }, // 0x4c
203+
{ "TOCLIENT_HUD_SET_PARAM", 1, true }, // 0x4d
194204
{ "TOCLIENT_BREATH", 0, true }, // 0x4e
195205
{ "TOCLIENT_SET_SKY", 0, true }, // 0x4f
196206
{ "TOCLIENT_OVERRIDE_DAY_NIGHT_RATIO", 0, true }, // 0x50

0 commit comments

Comments
 (0)
Please sign in to comment.