zpl-c / enet

⚡️ ENet reliable UDP networking library
https://blog.zpl.pw
MIT License
676 stars 61 forks source link

Reliable packets not so reliable #53

Open SheridanR opened 4 months ago

SheridanR commented 4 months ago

Hi,

I have a client/server application where each host has two channels, one for reliable packets and another for unreliable ones. On the unreliable channel i transmit about 3/kbps and the reliable channel is quite infrequent but occasional sees whole packets as large as 250 bytes. Trouble is that these "reliable" packets are frequently dropped, and I have no idea why as I'm testing over a local loopback and the throughput is obviously very small. If I raise the traffic on the unreliable channel a few times to eg 10/kbps virtually nothing gets through on the reliable channel. What gives?

inlife commented 4 months ago

Hey @SheridanR It could be related to MTU size

SheridanR commented 4 months ago

Doesn't enet fragment packets to keep them below the mtu? how can I control this behavior? Thanks for the response btw.

inlife commented 4 months ago

It does indeed, but the enet configured MTU size could be misalligned with your actual setup, we've seen a few occurrences similar to thos.

SheridanR commented 4 months ago

I don't have any particularly unusual setup, I'm using quite new equipment, router is a few years old and I'm working on an apple studio (also have a windows desktop i built from new parts about a month ago).

Should I hack the mtu to be lower or higher? Any other advice?

zpl-zak commented 4 months ago

Could you please provide me with a snippet showing enet_host_service being called? I've realized we recently adjusted the MTU limit to accommodate VPN connections and unusual infra setup, so MTU might not be the issue here.

SheridanR commented 4 months ago
void Host::operator()() {
    while (!shutdown) {
        ENetEvent event{};
        using namespace std::chrono;
        uint32_t period = 1000; // milliseconds
        while (enet_host_service(host, &event, period) > 0) {
            const auto timeStart = high_resolution_clock::now();
            std::lock_guard lock(mutex);

            switch (event.type) {
            case ENET_EVENT_TYPE_CONNECT: {
                peers[event.peer->connectID] = {
                    Guid{glm::uvec4{
                        event.data[0],
                        event.data[1],
                        event.data[2],
                        event.data[3]}},
                    event.peer,
                };
                if (state == State::CONNECTING) {
                    Engine::log("Connection to %s (%u) succeeded",
                        address.c_str(), event.peer->connectID);
                    state = State::CONNECTED;
                } else {
                    Engine::log("A new client (%u) connected from %x:%u.",
                        event.peer->connectID, event.peer->address.host, event.peer->address.port);
                }
                if (connect_cb) {
                    (*connect_cb)(event.peer->connectID);
                }
                break;
            }

            case ENET_EVENT_TYPE_RECEIVE: {
                if (receive_cb) {
                    (*receive_cb)(event.packet->data, event.packet->dataLength, event.peer->connectID);
                } else {
                    peers[event.peer->connectID].packets.emplace(event.packet->data, event.packet->data + event.packet->dataLength);
                }
                //Engine::log("A packet of length %lu was received from %u on channel %u.",
                //    event.packet->dataLength, event.peer->connectID, event.channelID);
                enet_packet_destroy(event.packet);
                break;
            }

            case ENET_EVENT_TYPE_DISCONNECT: {
                if (state != State::OPEN && state != State::CLOSING) {
                    Engine::log("Disconnected from server (%u) at '%s'.",
                        event.peer->connectID, address.c_str());
                    state = State::DISCONNECTED;
                    shutdown = true;
                }
                else {
                    Engine::log("Client %u from %x:%u disconnected.",
                        event.peer->connectID, event.peer->address.host, event.peer->address.port);
                }
                if (disconnect_cb) {
                    disconnect_cb(event.peer->connectID);
                }
                peers.erase(event.peer->connectID);
                if (peers.empty() && state == State::CLOSING) {
                    state = State::DISCONNECTED;
                    shutdown = true;
                }
                break;
            }

            case ENET_EVENT_TYPE_DISCONNECT_TIMEOUT: {
                if (state != State::OPEN && state != State::CLOSING) {
                    Engine::log("Disconnected from server (%u) at '%s' due to timeout.",
                        event.peer->connectID, address.c_str());
                    state = State::TIMEOUT;
                    shutdown = true;
                }
                else {
                    Engine::log("Client %u from %x:%u disconnected due to timeout.",
                        event.peer->connectID, event.peer->address.host, event.peer->address.port);
                }
                if (timeout_cb) {
                    timeout_cb(event.peer->connectID);
                }
                peers.erase(event.peer->connectID);
                if (peers.empty() && state == State::CLOSING) {
                    state = State::DISCONNECTED;
                    shutdown = true;
                }
                break;
            }

            default:
            case ENET_EVENT_TYPE_NONE: {
                break;
            }
            }

            const auto timeEnd = high_resolution_clock::now();
            const auto elapsed = duration<double, milliseconds::period>(timeEnd - timeStart).count();
            if ((uint32_t)elapsed >= period) {
                period = 1;
            } else {
                period -= (uint32_t)elapsed;
            }
        }

        ...

        std::this_thread::yield();
    }

    std::lock_guard lock(mutex);
    for (auto& pair : peers) {
        if (timeout_cb) {
            timeout_cb(pair.first);
        }
        enet_peer_reset(pair.second.peer);
    }
    peers.clear();
    enet_host_destroy(host);
}
SheridanR commented 4 months ago

A few things to clarify:

  1. I call enet_host_service() in its own thread, the wait time is set to 1000ms but it makes no difference at any other value.
  2. I had to modify enet very slightly to make it thread safe, which pretty much just meant added a mutex lock on every function that accesses a host or peer. The mutex is unlocked while enet_socket_select() is running, so hosts & peers can be accessed while the socket waits for data.
  3. enet_socket_select would not wake on write in enet_host_service, so i modified that so the thread would wake when i call enet_peer_send. otherwise i noticed my pending outgoing packets would not send until select() was finished.
  4. I modified the connection protocol to include a 128-bit user data field instead of 32-bits so i can include an authoritative guid that uniquely identifies each application instance.
  5. everything else is a stock enet installation.
zpl-zak commented 4 months ago

I recommend testing whether the unmodified library works well for you just to isolate potential issues. Since your enet is modified, it would be difficult for us to assist you with it. Thanks for sharing more information!