Hey everyone,
I'm looking at some old networking code that I wrote some time ago and I'm looking for a way to improve it and change the design. I enjoy doing this, so that's my motive.
This is all written in C++ and on top of BSD sockets and the goal was to create a connection layer on top of UDP. I'll first show how I achieved this goal previously and point out what I don't like about it and then continue with the ideas that have been running through my mind but seem to fall short.
So currently I have a socket class that's very similar to the one used in Glenn Fielder's Networking for Game Programmers articles, which is basically a class wrapped around the BSD functions.
Managing which packets should go where is done by a class called Tranceiver(<insert oh god why meme>). This class owns a Socket object used to send and receive packets. Connections can be registered here to listen for a specific or any address, once a connection has been setup the Transceiver class passes the packets on to the corresponding connection.
The connection class is responsible for setting up connections, keeping them alive with keep alive packets, detecting disconnects and passing on the packet data(packet minus the header) to some handler and forms the gateway between the two ends to send data across. This class holds a reference to the Tranceiver it's associated with and next to sending packets, it's used to register/de-register on connects/disconnects.
It's that last part that I dislike the most. I don't like de dependency of the two classes on each other and it should be possible to set up a connection with just a socket, since a client that's only connected to a server shouldn't need a manager.
The solution seems simple, have the connection class hold a reference to a socket instead, but now not only do you need to make sure the connection you're registering to a manager uses the same socket but you'll have to manually register/de-register connections.
Another option is to have the connection manager create connections with the appropriate socket and not allow external connections to be registered. Still have to manually register/de-register connections and re-registering becomes confusing, do I destroy and recreate a connection? Calling a method to re-register a connection, accepting a connection as an argument seems weird since only connections created by the manager should be accepted.
What design can I use to make it more consistent, have less dependency and avoid manual registration? Any help or feedback is welcome.
Connection layer design
Unless I misunderstood something, I don't understand why it has to be so complicated.
What I'd do is store unknown conection waiting to be accepted in a queue, store the established connections in a connection map, and the connections that I have closed inside another list. I close connections by sending a few 'close' packets over a few frames, and don't bother with hand-shaking usually.
if a new connection is accepted (game not full, player not banned), the connection is moved from the accept queue onto the established connection map. If the connection is refused, the connection is moved from the accept queue into the closed map.
There is a case when a party host wants to reserve slots for his party members, but that should be trivial to deal with.
You just keep pumping listen_connection_request() until it returns false, and either accept or refuse them depending on the client data content.
That's it, really.
What I'd do is store unknown conection waiting to be accepted in a queue, store the established connections in a connection map, and the connections that I have closed inside another list. I close connections by sending a few 'close' packets over a few frames, and don't bother with hand-shaking usually.
if a new connection is accepted (game not full, player not banned), the connection is moved from the accept queue onto the established connection map. If the connection is refused, the connection is moved from the accept queue into the closed map.
There is a case when a party host wants to reserve slots for his party members, but that should be trivial to deal with.
struct Socket
{
Socket(unsigned short listening_port=0);
~Socket();
// deal with incoming connections.
size_t listen_connection_request(ConnectionAddr& addr, void* connection_request_data, size_t connection_request_data_limit);
void refuse_connection_request(const ConnectionAddr& addr, const void* connection_refuse_data, size_t connection_refuse_data_size);
void accept_connection_request(const ConnectionAddr& addr);
// close an established connection
void close_connection(const ConnectionAddr& addr, const void* connection_close_data, size_t connection_close_data_size);
// messaging.
void send_message(const ConnectionAddr& addr, const void* connection_message_data, size_t connection_message_data_size, unsigned int flags);
size_t recv_message(ConnectionAddr& addr, void* connection_message_data, size_t connection_message_data_limit, unsigned int& flags);
};
You just keep pumping listen_connection_request() until it returns false, and either accept or refuse them depending on the client data content.
That's it, really.
Hi, thank you for taking interest and your help.
So the goal is to come up with a robust design for adding a connection layer on top of UDP. I think we can agree on having a basic socket class to send retrieve data on and from a specific port is trivial, but where do we go from there? I don’t think adding connection management should be handled in the socket class(SRP). What are our options then? Offloading this responsibility to another class is an option and I think it’s the best candidate for a good design, but this is not where the design choices end. Coming up with a complete and robust design is what’s been challenging for me.
So the goal is to come up with a robust design for adding a connection layer on top of UDP. I think we can agree on having a basic socket class to send retrieve data on and from a specific port is trivial, but where do we go from there? I don’t think adding connection management should be handled in the socket class(SRP). What are our options then? Offloading this responsibility to another class is an option and I think it’s the best candidate for a good design, but this is not where the design choices end. Coming up with a complete and robust design is what’s been challenging for me.
Why is a manager required? You can have a socket object wrapping UDP sockets, if you want, and a Connection object that uses the interface of that socket to keep a connection alive. The main thing you need to get right is the mapping from UDP address in recvfrom() to specific connection. This can, conceivably, be done with a hash table in the socket itself.
But it also makes sense to have a manager in between, to do that dispatching, if you like the "single responsibility principle." After all, what should you do if a datagram comes in to the client that's not from the actual server IP? What if you want to be connected to more than one server at the same time? (For UDP, you still only need the single socket)
You can also make sure that talking to a socket uses an interface, so you could swap that out if needed. The manager also uses an interface (and, if the socket does callbacks for data, that might be an interface.) And the connection can use an interface, too. That way, you could implement a simpler manager for the case of a client, than for the case of the server -- perhaps a manager that discards all source addresses other than the determined server.
Interfaces in C++ are typically implemented using pure abstract base classes, but could also be template "protocols" if you really want static dependencies.
But it also makes sense to have a manager in between, to do that dispatching, if you like the "single responsibility principle." After all, what should you do if a datagram comes in to the client that's not from the actual server IP? What if you want to be connected to more than one server at the same time? (For UDP, you still only need the single socket)
You can also make sure that talking to a socket uses an interface, so you could swap that out if needed. The manager also uses an interface (and, if the socket does callbacks for data, that might be an interface.) And the connection can use an interface, too. That way, you could implement a simpler manager for the case of a client, than for the case of the server -- perhaps a manager that discards all source addresses other than the determined server.
Interfaces in C++ are typically implemented using pure abstract base classes, but could also be template "protocols" if you really want static dependencies.
Thank you for your help, you've given me some new insights. Here's some psuedo code showing a design in mind:
[source lang="cpp"]struct ISocket
{
virtual bool open(unsigned short port) = 0;
virtual void close() = 0;
virtual bool send(const Address& destination, const void* data, int size) = 0;
virtual int receive(Address& sender, char* data, int size) = 0;
};
struct UDPSocket : public ISocket
{
IDispatcher* dispatcher;
UDPSocket(IDispatcher* dispatcher);
bool open(unsigned short port);
void close();
bool send(const Address& destination, const void* data, int size);
int receive(Address& sender, char* data, int size);
};
struct IDispatcher
{
virtual void dispatch(const Address& sender, const char* data, int size) = 0;
};
struct SingleDispatcher : public IDispatcher
{
Connection* connection;
SingleDispatcher(Connection* connection);
void dispatch(const Address& sender, const char* data, int size);
};
struct MultiDispatcher : public IDispatcher
{
std::vector<Connection*> connected, listening, closed;
void addConnection(Connection* connection);
void dispatch(const Address& sender, const char* data, int size);
};[/source]
This seems like a robust design. Adding in the connection object is a bit tricky though, if the connection object uses the socket interface to keep connections alive, it should be the same socket the dispatcher is attached to. How can we design this to make sure that's always the case? How should the dispatcher be notified of connections being closed? I was thinking of checking for return values during dispatch.
I'd like to come back at your second point after I get a clear picture of the rest.
Again, thank you for your help. Any comments or feedback are more than welcome.
[source lang="cpp"]struct ISocket
{
virtual bool open(unsigned short port) = 0;
virtual void close() = 0;
virtual bool send(const Address& destination, const void* data, int size) = 0;
virtual int receive(Address& sender, char* data, int size) = 0;
};
struct UDPSocket : public ISocket
{
IDispatcher* dispatcher;
UDPSocket(IDispatcher* dispatcher);
bool open(unsigned short port);
void close();
bool send(const Address& destination, const void* data, int size);
int receive(Address& sender, char* data, int size);
};
struct IDispatcher
{
virtual void dispatch(const Address& sender, const char* data, int size) = 0;
};
struct SingleDispatcher : public IDispatcher
{
Connection* connection;
SingleDispatcher(Connection* connection);
void dispatch(const Address& sender, const char* data, int size);
};
struct MultiDispatcher : public IDispatcher
{
std::vector<Connection*> connected, listening, closed;
void addConnection(Connection* connection);
void dispatch(const Address& sender, const char* data, int size);
};[/source]
This seems like a robust design. Adding in the connection object is a bit tricky though, if the connection object uses the socket interface to keep connections alive, it should be the same socket the dispatcher is attached to. How can we design this to make sure that's always the case? How should the dispatcher be notified of connections being closed? I was thinking of checking for return values during dispatch.
Interfaces in C++ are typically implemented using pure abstract base classes, but could also be template "protocols" if you really want static dependencies.
I'd like to come back at your second point after I get a clear picture of the rest.
Again, thank you for your help. Any comments or feedback are more than welcome.
Then you need another component, I suppose.
struct ISocket
{
virtual bool open(unsigned short port) = 0;
virtual void close() = 0;
virtual bool send(const Address& destination, const void* data, int size) = 0;
virtual int receive(Address& sender, char* data, int size) = 0;
};
struct ITransmitter
{
virtual bool send(const Address& destination, const void* data, int size) = 0;
}
struct IReceiver
{
virtual int receive(Address& sender, char* data, int size) = 0;
}
struct IConnectionFactory
{
virtual IConnection* createConnection(Address& remote_address)=0;
virtual IConnection* destroyConnection(Address& remote_address)=0;
};
struct UDPSocket : public ISocket
{
IDispatcher* dispatcher;
ITransmitter* transmitter;
IReceiver* receiver;
IConnectionFactory* connectionFactory;
UDPSocket(IDispatcher* , IConnectionFactory* , ITransmitter* , IReceiver* );
bool open(unsigned short port);
void close();
bool send(const Address& destination, const void* data, int size);
int receive(Address& sender, char* data, int size);
};
struct IDispatcher
{
virtual void dispatch(const Address& sender, const char* data, int size) = 0;
};
struct SingleDispatcher : public IDispatcher
{
ConnectionFactory* connectionFactory;
ITransmitter* transmitter;
IReceiver* receiver;
Connection* connection;
SingleDispatcher(IConnectionFactory* , ITransmitter* , IReceiver* );
void dispatch(const Address& sender, const char* data, int size);
};
struct MultiDispatcher : public IDispatcher
{
ConnectionFactory* connectionFactory;
ITransmitter* transmitter;
IReceiver* receiver;
std::vector connected;
std::vector established;
std::vector closed;
MultiDispatcher(ConnectionFactory* , ITransmitter* , IReceiver* );
void addConnection(Connection* connection);
void dispatch(const Address& sender, const char* data, int size);
};
Thank you for taking the time to help out again. Could you please elaborate a bit on that design? What's the reasoning behind the transmitter and receiver interfaces? I'm also not sure how this design solves the problem pointed out in my last post.
if the connection object uses the socket interface to keep connections alive, it should be the same socket the dispatcher is attached to.[/quote]
That's a problem in the context of OO. You want the component to use its parent to transmit packets, that forms a circular hierarchical dependency that you don't really want.
Moving the transport layer (Transmitter + Receiver) into a component, flattens the hierarchy a bit. instead of having A(B(A)), you have A(B, C(B)).
If you have virtual interfaces on components like a connection, then a factory looks to be in order. You may have different types of connections (point to point connection, a connection to a broadcasting station like SourceTV, a service, ect...). As you may have different sorts of transport layers (WinSock, replay buffer, ect...).
How should the dispatcher be notified of connections being closed? I was thinking of checking for return values during dispatch.[/quote]
Who's responsible for managing the lifetime of connections? Or in what container they should be stored and move to and from (pending, established, closed)? Would it be yet another class that manages connection states and validity (keep alives, connection handshake, disconnection handshake)? Would connections be autonomous, with say, an internal state machine that the dispatcher can monitor?
TBH, I'm not a fan of such over complication. Having a whole army of components, each micro-managing things. I'm not a fan of monolithic design either, but this looks to be going a bit too far for my liking.
Hey, thank you for clearing that up.
I have to agree. I'm going to do some thinking and decide where to settle. In the meanwhile I'm still open for suggestions.
TBH, I'm not a fan of such over complication. Having a whole army of components, each micro-managing things. I'm not a fan of monolithic design either, but this looks to be going a bit too far for my liking.
I have to agree. I'm going to do some thinking and decide where to settle. In the meanwhile I'm still open for suggestions.
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
