Closed on purpose?

Okay … now I'm confused: How would you listen and except a connection request with this API? And how would you implement a socket wrapper?

On the Socket Wrapper: Actually this could be a problem.

On accepting a passive connection: The server would call connect with a Port to listen on, given in the local endpoint. The Function call blocks until a connection has been established (until a peer tries to connect to that port).

I'll aim for an processing logic like this:

sock_tcp_init(sock);
while(true){
sock_tcp_connect(sock);
// Do Things
sock_tcp_disconnect(sock);
}
sock_tcp_release(sock);

Instead of:

while(true){
sock_tcp_connect(sock);
// Do Things
sock_tcp_disconnect(sock);
}

To avoid multiple receive buffer allocation and freeing inside the packet buffer. I'd prefer to allocate a buffer a single time and reusing it instead of allocation and freeing with each loop.

But with _listen + _accept we wouldn't need any of this:

sock_tcp_listen(queue);
while(true) {
    sock_tcp_accept(queue, &sock);
    /* Do things */
    sock_tcp_disconnect(sock);
}
sock_tcp_stop_listen(queue);

To avoid multiple receive buffer allocation and freeing inside the packet buffer. I'd prefer to allocate a buffer a single time and reusing it instead of allocation and freeing with each loop.

If it is actually a lot faster, consider adding a receive buffer cache (shudder) to tcp_queue_t.

Also: how to handle multiple incoming connection requests with this?

@brummer-simon are you using the mbox mod for netreg (RIOT-OS#5526) or the vanilla netreg? I think using it would simplify things for you a lot, too.

@brummer-simon Did you see any actual performance degradation due to de/reallocating a receive buffer?

By having multiple threads waiting for an incomming connection. The only problem with that: If you want to accept multiple connections and process a single one at a time. This would not be possible.

In the server case: you could handle avoiding freeing and allocating with listen and stop listen. In a client case that doesn't work. By introducing the init and release call it would be a unified way for server and clients.

@ Kaspar: The performance is not the problem but allocating and freeing with multiple connections leads to framentation. In my bulk testing after few hundred connections the pktbuf is not able to find a fitting place for a receive buffer anymore and my test breaks ...

@ Kaspar: The performance is not the problem but allocating and freeing with multiple connections leads to framentation. In my bulk testing after few hundred connections the pktbuf is not able to find a fitting place for a receive buffer anymore and my test breaks ...

Maybe @miri64 can elaborate what malloc algorithm pktbuf is using and how it tries to defeat fragmentation.

Maybe @miri64 can elaborate what malloc algorithm pktbuf is using and how it tries to defeat fragmentation.

It uses first fit. There is no fragmentation prevention, because up until now we assumed there was no use-case that could produce it. Maybe we need a special TCP-pktbuf?

Maybe we need a special TCP-pktbuf?

shudder

Maybe just make a sufficiently large buffer part of sock_tcp_t?

I'll trief that, with a mss of 1220 Bytes the size struct breakes the
platforms stack size. That was the reason why i moved the Receiver Buffer
into pktbuf in the First place.

Am 23.08.2016 12:46 nachm. schrieb "Kaspar Schleiser" <
[email protected]>:

Maybe we need a special TCP-pktbuf?

shudder

Maybe just make a sufficiently large buffer part of sock_tcp_t?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#29 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ADxGJyocCiE6AJmZt3SFlCDbTdl-MQI9ks5qis-CgaJpZM4Jqs3W
.

I'll still think the easiest Solution might be just reusing the sucessully
allocated struct or suppling a Buffer with the Connect call.

The Problem with the external buffer is that the Buffer must be allocated
in the Static section (to prevent Breaking the threads stacks). User have
to know that.

All in All i would prefer the init and release Function and reusing
successfully allocated buffers.

Am 23.08.2016 12:49 nachm. schrieb "Simon Brummer" <
[email protected]>:

I'll trief that, with a mss of 1220 Bytes the size struct breakes the
platforms stack size. That was the reason why i moved the Receiver Buffer
into pktbuf in the First place.

Am 23.08.2016 12:46 nachm. schrieb "Kaspar Schleiser" <
[email protected]>:

Maybe we need a special TCP-pktbuf?

shudder

Maybe just make a sufficiently large buffer part of sock_tcp_t?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#29 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ADxGJyocCiE6AJmZt3SFlCDbTdl-MQI9ks5qis-CgaJpZM4Jqs3W
.

@ Kaspar: True don't allocate on stack is a possibility but this is
something the Users need to know and from a usability Point of view it
feels plain wrong in my opinion.

Am 23.08.2016 1:06 nachm. schrieb "Simon Brummer" <
[email protected]>:

I'll still think the easiest Solution might be just reusing the sucessully
allocated struct or suppling a Buffer with the Connect call.

The Problem with the external buffer is that the Buffer must be allocated
in the Static section (to prevent Breaking the threads stacks). User have
to know that.

All in All i would prefer the init and release Function and reusing
successfully allocated buffers.

Am 23.08.2016 12:49 nachm. schrieb "Simon Brummer" <
[email protected]>:

I'll trief that, with a mss of 1220 Bytes the size struct breakes the
platforms stack size. That was the reason why i moved the Receiver Buffer
into pktbuf in the First place.

Am 23.08.2016 12:46 nachm. schrieb "Kaspar Schleiser" <
[email protected]>:

Maybe we need a special TCP-pktbuf?

shudder

Maybe just make a sufficiently large buffer part of sock_tcp_t?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#29 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ADxGJyocCiE6AJmZt3SFlCDbTdl-MQI9ks5qis-CgaJpZM4Jqs3W
.

I'll still think the easiest Solution might be just reusing the sucessully
allocated struct or suppling a Buffer with the Connect call.

This might be the easiest solution for your implementation together with
pktbuf, but re-using the structs is bad design, and having to supply the
buffer together with the connect call just moves the responsibility to
the user.

So you Go with move the Buffer into the struct and allocate sock_tcp_t
always in the static section ?

Am 23.08.2016 1:18 nachm. schrieb "Kaspar Schleiser" <
[email protected]>:

I'll still think the easiest Solution might be just reusing the
sucessully
allocated struct or suppling a Buffer with the Connect call.

This might be the easiest solution for your implementation together with
pktbuf, but re-using the structs is bad design, and having to supply the
buffer together with the connect call just moves the responsibility to
the user.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#29 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ADxGJ40fAiIqGHBRhgkeJd0xIqAdFjwxks5qitcLgaJpZM4Jqs3W
.

So you Go with move the Buffer into the struct and allocate sock_tcp_t
always in the static section ?

Well, there are other options:

• fix pktbuf
• zero the sock_tcp_t structs on listen, then reuse a buffer if present
• allocate one buffer for all tcp connections, share that between
  connections
• instead of freeing, put buffers in a list of the queue; when
  allocating, check the list first before allocating a new buffer

Maybe we need a special TCP-pktbuf?

shudder

Why shudder? The pktbuf-API was always conceived to allow for adapted implementations for the use-case. For pure UDP the current implementation is optimal so

• fix pktbuf

It is not broken for that use-case, so what is there is no need to fix it. If it does not fit another use-case, I don't see why we should meddle with something that isn't broken.

TCP is so complex anyway. A little extra bytes of code for fragmentation prevention do not hurt there (even if you use UDP with that buffer), but they might for pure UDP.