Welcome, Guest. Please login or register. Did you miss your activation email?

Author Topic: Packet transmission is unreliable and badly designed  (Read 3753 times)

0 Members and 2 Guests are viewing this topic.

torokati44

  • Newbie
  • *
  • Posts: 14
    • View Profile
Packet transmission is unreliable and badly designed
« on: June 10, 2011, 08:37:17 pm »
Dear Laurent!

I am developing a UDP-based client-server networking framework (netframework) using SFML 1.6. You can browse the whole source code here: https://bitbucket.org/torokati44/netframework. (It is heavily under development, but some parts of it are more or less functional all the time. And yes, I know it is a single executable application, but this is only for testing purposes. I plan to make it a library when it will be more mature.)

Recently I have had serious problems with the sending and receiving of sf::Packets. As you can see, my application sends keepalive packets to keep the connection(s) open. But not on a local network, even on localhost the connection broke after a varying amount of time. I have double-checked all my code, monitored traffic with Wireshark, but haven't got a clue what is causing this.

Only after this looked I into the implementation of the handling of sf::Packets in sf::SocketUDP. I was well surprised. I don't think it is a good idea to keep the ability to split packets into more UDP datagrams, because the whole, assembled packet is not checked for valid reassembly upon receive. And as we know (Because we do know, am I right?) UDP might swap, drop, duplicate, torture, or read Swedish folk tales to the datagrams during delivery. (But fortunately they are checksummed of course.) So you can't be sure that the same data arrives at the other end that you sent on this end. This is why it's wrong to make users believe that sf::Packet will arrive as just the same as it was sent, because it is not sure at all.

Moreover, what could happen if you sent a small packet that fits into a single datagram?
SFML sends the length of the packet (N bytes) first in a separate datagram as a 32 bit integer. (At least in SFML1.6. Anyway, the newer method in SFML2 isn't better either.) Then it waits for N bytes to arrive in whatever number of UDP datagrams. In this case there would be only one, but what if it got lost, and the other end won't send any more data? sf::SocketUDP::Receive(sf::Packet, ...) would wait forever, and dropped any other packet from any other address. I think this is very bad. (In SFML2 the tailing zero-length packet also can be lost, so you merged 2 sf::Packets together. Still dropping ones from any other source in the meantime.)

I suggest to make sf::Packet just wrap a single UDP datagram, and don't ever send it's data in multiple parts. (I'm speaking only about UDP, not TCP.) UDP packets' size is stored in a 2-byte unsigned integer, so theoretically they can be 65535 bytes long. IP will automatically fragment and reassemble the datagrams longer than the MTU, and drop them if some parts of them are lost. But would never let a corrupted UDP datagram through because of CRC checking. (Yes, it is highly recommended not to send Packets longer than the path MTU, I think this is why SFML splits packets. There shouldn't be need to send longer packets though.) And I think dropping data is still better than allowing faulty data through, without notifying, or even knowing about it.

If you don't want to change SFML like this, you should at least make a note of this in the documentation, because it was very frustrating to find.

So I solved the problem by replacing the sf::SocketUDP::Send(sf::Packet, ...) calls with sf::SocketUDP::Send(sf::Packet::GetData(), sf::Packet::GetDataSize(), ...), and making sure that no packet can exceed a constant size, like 500-512 bytes. This way the extra (currently 100%) packet overhead is avoided, and the connections are rock solid even over the internet.

Please don't take this post offensive, I still love and will use SFML, and appreciate your work. This is just a suggestion.
Sorry for my bad English, I'm not a native speaker, and I hope you will understand what I mean. Also sorry for the lengthy post.

Best regards:
TÖRÖK Attila

Laurent

  • Administrator
  • Hero Member
  • *****
  • Posts: 32504
    • View Profile
    • SFML's website
    • Email
Packet transmission is unreliable and badly designed
« Reply #1 on: June 10, 2011, 09:24:01 pm »
Hi

Thank you very much for your post, I really appreciate when users take some time to investigate in SFML, find problems or badly designed stuff, and report them in such a detailed way. That's the beauty of open-source.

I think you're 100% right, and I agree with everything (yes, it happens sometimes :D). UDP & packets were just bad in SFML 1, they are better in SFML 2 but it's still not the right solution. I know UDP and its limitations, but unfortunately my everyday work doesn't involve intensive use of UDP (and not with SFML nor low-level sockets, anyway). So it's hard for me to be aware of this kind of real-life problems. So again, thanks for your help.

Anyway... it seems reasonable to:
- add more details to the documentation
- limit UdpSocket::Send(Packet) to one datagram

Given the limitations of UDP, it makes more sense than allowing corrupted data, like you said.

I'll change the implementation and update the corresponding documentation as soon as possible.
Laurent Gomila - SFML developer

torokati44

  • Newbie
  • *
  • Posts: 14
    • View Profile
Packet transmission is unreliable and badly designed
« Reply #2 on: June 10, 2011, 10:05:10 pm »
Oh, what a fast response! Thank you very much! It's "the beauty of open-source", isn't it? :)
I'm glad you see the problem too, and that you agree with me.
Thanks for your attention, and I can't wait to see the fixes!

BTW what do you think about my project? Well, I don't expect you to read and analyze all the code, your time is surely expensive. But if you find it interesting, or worth inspecting, it would be great if you wrote a few words about it. Well, maybe it'd be better to describe it only if it's finished, and maybe I'll post it then into the "projects" section, so it will be somewhat stable and polished.

My pleasure that I could help make SFML better. Keep up the good work, and thank you for it!

Laurent

  • Administrator
  • Hero Member
  • *****
  • Posts: 32504
    • View Profile
    • SFML's website
    • Email
Packet transmission is unreliable and badly designed
« Reply #3 on: June 10, 2011, 10:34:31 pm »
Quote
BTW what do you think about my project?

It's hard to make an opinion about a new project by directly looking at its source code. I think I'll have a more detailed look at it when you describe it officially in the Projects forum (which you should definitely do, by the way).
Laurent Gomila - SFML developer

torokati44

  • Newbie
  • *
  • Posts: 14
    • View Profile
Packet transmission is unreliable and badly designed
« Reply #4 on: June 11, 2011, 09:47:42 am »
Quote from: "Laurent"
I think I'll have a more detailed look at it when you describe it officially in the Projects forum (which you should definitely do, by the way).


Okay then, I will do so when it's ready.

Laurent

  • Administrator
  • Hero Member
  • *****
  • Posts: 32504
    • View Profile
    • SFML's website
    • Email
Packet transmission is unreliable and badly designed
« Reply #5 on: June 11, 2011, 11:29:53 am »
Quote
I'll change the implementation and update the corresponding documentation as soon as possible.

Done.
Laurent Gomila - SFML developer

torokati44

  • Newbie
  • *
  • Posts: 14
    • View Profile
Packet transmission is unreliable and badly designed
« Reply #6 on: June 11, 2011, 12:08:07 pm »
Quote from: "Laurent"
Done.


Great! Thanks!

Fouf

  • Newbie
  • *
  • Posts: 23
    • View Profile
Packet transmission is unreliable and badly designed
« Reply #7 on: June 11, 2011, 09:31:03 pm »
Quote from: "Laurent"
Quote
I'll change the implementation and update the corresponding documentation as soon as possible.

Done.


This is great Laurent, it is nice to see someone respond so quickly to a matter ^^. Especially since I'll be using UDP networking with sfml soon! :p
- Fouf