3

The below code is from Git. It joins a multicast group and receives packets.

Here we loop and receive the data in a buffer called msgbuf:

while (1) 
{
    char msgbuf[MSGBUFSIZE];
    const int addrlen = sizeof(addr);
    const int nbytes = recvfrom(fd, msgbuf, MSGBUFSIZE, 0, (struct sockaddr *) &addr, &addrlen);

How do I choose the size for the buffer msgBuf? Does it just have to be the max packet size? Or do I need to store multiple packets whilst I process the first?

Full code:

int main(int argc, char *argv[])
{
    if (argc != 3) {
       printf("Command line args should be multicast group and port\n");
       printf("(e.g. for SSDP, `listener 239.255.255.250 1900`)\n");
       return 1;
    }

    char* group = argv[1]; // e.g. 239.255.255.250 for SSDP
    int port = atoi(argv[2]); // 0 if error, which is an invalid port

    if(port <= 0)
    {
        perror("Invalid port");
        return 1;
    }


    // create what looks like an ordinary UDP socket
    //
    int fd = socket(AF_INET, SOCK_DGRAM, 0);
    if (fd < 0) 
    {
        perror("socket");
        return 1;
    }

    // allow multiple sockets to use the same PORT number
    //
    u_int yes = 1;
    if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (char*) &yes, sizeof(yes)) < 0)
    {
       perror("Reusing ADDR failed");
       return 1;
    }

        // set up destination address
    //
    struct sockaddr_in addr;
    memset(&addr, 0, sizeof(addr));
    addr.sin_family = AF_INET;
    addr.sin_addr.s_addr = htonl(INADDR_ANY); // differs from sender
    addr.sin_port = htons(port);

    // bind to receive address
    //
    if (bind(fd, (struct sockaddr*) &addr, sizeof(addr)) < 0) 
    {
        perror("bind");
        return 1;
    }

    // use setsockopt() to request that the kernel join a multicast group
    //
    struct ip_mreq mreq;
    mreq.imr_multiaddr.s_addr = inet_addr(group);
    mreq.imr_interface.s_addr = htonl(INADDR_ANY);

    if (setsockopt(fd, IPPROTO_IP, IP_ADD_MEMBERSHIP, (char*) &mreq, sizeof(mreq)) < 0)
    {
        perror("setsockopt");
        return 1;
    }

    // now just enter a read-print loop
    //
    while (1) 
    {
        char msgbuf[MSGBUFSIZE];
        const int addrlen = sizeof(addr);
        const int nbytes = recvfrom(fd, msgbuf, MSGBUFSIZE, 0, (struct sockaddr *) &addr, &addrlen);

        if (nbytes < 0) 
        {
            perror("recvfrom");
            return 1;
        }
        msgbuf[nbytes] = '\0';
        puts(msgbuf);
     }

    return 0;
}
user997112
  • 29,025
  • 43
  • 182
  • 361
  • The code causes UB if `recvfrom()` reads `MSGBUFSIZE` bytes. Then `msgbuf[nbytes] = '\0':` causes buffer overflow. If that's production code it seems that in practice packages are smaller – Ingo Leonhardt Oct 25 '19 at 12:46
  • @IngoLeonhardt I appreciate your point but the question is about how to size the buffer, so saying there's an overflow is a little meaningless. I need to know, generally, how does one determine the buffer size. Is it based on the largest packet size? Or, do I have to store multiple packets and therefore just make the buffer huge? – user997112 Oct 25 '19 at 12:56
  • 1
    I know it was a little bit OT. An answer to your question of course depends on the specific communication, that I don't know. But at least the buffer overflow could indicate that there *should* be no need to store multiple packets in that communication because I think in this case the overflow would occur – Ingo Leonhardt Oct 25 '19 at 13:00
  • @IngoLeonhardt No worries, I didn't mean to sound rude, I just wanted to redirect to the question does it need to store one packet, or multiple. I think it's just one packet (and the OS buffer probably stores any accumulating?) – user997112 Oct 25 '19 at 13:13
  • 1
    I've never had the feeling that you were rude :-) If my last comment gave you the impression it's just because I'm not a native speaker. Hopefully your question reaches the interest of someone who knows more – Ingo Leonhardt Oct 25 '19 at 13:22

2 Answers2

2

Unlike TCP which combines packets into a stream, UDP respects packet boundaries so recvfrom only gets one packet at a time.

So MSGBUFSIZE only needs to be as big as a single packet. If you're not using jumbo packets that would be 1500, otherwise it would be 9000.

dbush
  • 205,898
  • 23
  • 218
  • 273
1

as noted by @Ingo, in this code you should be using:

    char msgbuf[MSGBUFSIZE + 1];

the + 1 is because recvfrom can write upto MSGBUFSIZE bytes into the array, and you then write another NUL byte at the end.

as far as choosing a value for MSGBUFSIZE, that would depend on the protocol specification. given that most physical networks would struggle to send more than 1500 bytes without fragmentation something like 2048 might be a reasonable value. you could also check for nbytes == MSGBUFSIZE (maybe also using MSG_TRUNC) and report a "packet truncated" warning, but this basically wouldn't happen for packets routed over the public internet

in response to:

do I need to store multiple packets whilst I process the first?

you'd normally let the network stack take care of that. recv maintains packet/datagram boundaries and hence will always start writing the next packet at the supplied buffer address. again it depends on the protocol how you detect and handle errors, e.g. missing or out-of-order packets, and timeouts

Sam Mason
  • 15,216
  • 1
  • 41
  • 60