tasklwIP - A Lightweight TCP/IP stack - Tasks: task #6994, Redesign Socket Layer with...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

task #6994: Redesign Socket Layer with LWIP_TCPIP_CORE_LOCKING

Submitted by:  Frédéric Bernon <fbernon>
Submitted on:  Sat 09 Jun 2007 11:40:29 AM UTC  
 
Category:  socket/netconn Should Start On:  Fri 08 Jun 2007 10:00:00 PM UTC
Should be Finished on:  Fri 08 Jun 2007 10:00:00 PM UTC Priority:  1 - Later
Status:  Cancelled Privacy:  Public
Percent Complete:  0% Assigned to:  Simon Goldschmidt <goldsimon>
Open/Closed:  Closed Planned Release:  None
Effort:  0.00

( Jump to the original submission)

Tue 17 Feb 2015 08:29:22 PM UTC, comment #22: 

Don't think this is necessary any more: when LWIP_SOCKET==1 and LWIP_NETCONN==0, all functions from api_msg.c, api_lib.c and netbuf.c are static, so by using compiler optimizations, the current socket code should effectively be nearly the same as without netconns in between.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Thu 24 Jun 2010 11:55:54 AM UTC, comment #21: 

Not really :(

Frédéric Bernon <fbernon>
Project Member
Sun 10 Jan 2010 11:45:14 AM UTC, comment #20: 

Is this really still in progress? :-)

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Mon 16 Mar 2009 01:59:51 PM UTC, comment #19: 

i had a quick lock to socket2.c code...

just a suggest:

i saw that more function block forever with LOCK_TCPIP_CORE() if another task is locking the core. So, if i have more tha one task, and each task try to send a packet to different socket, each task can block until another free the core. I want to suggest the possibility to call socket functions with a timeout (it could be a define or related to some socket option, for example if non-blocking socket is used), so, function should return with error COREBUSY, and application can manage this error avoiding blocking (if the developer wants this feature). It could be useful (but i'm not sure) if different tasks will try to send to the SAME socket in the SAME moment: tasks should block waiting their timeslot in lwip core, or return immediately and allow to application to manage this situation.

Any comment?
Bye
Piero

piero <piero74>
Mon 16 Mar 2009 11:34:58 AM UTC, comment #18: 

Hi all

just my few cents for this interesting discussion (sorry for my bad english):

1. i'd like the ida to have new socket layer, which can work with OS, but without netconn. I think that it could be useful working toghether the idea of "zero-copy driver with DMA", discussed in mailing, to have very performance full stack (full = from sockets to driver)... implementation should be thinked for supporting this.
2. i'm an happy user of select(). For me it's very important to have it, but poll() function could be used as replacement (but i'm not sure)
3. i'd like the idea that i can user new lwip socket layer with other IO (file access on flash, UART, SPI...), just having the possibility build custom socket "class" (i.e. UartSocket) customizing low-level function (read, write, ... somethinh defined in socket_low_level_prototypes reference); in this way i can use the same application code for managing network stream or (i.e.) uart-streaming, or synchronize them with select() or poll()
3. i downloaded files here (socket2.c and .patch)... i want to study them and try to port over my application (lwip 130 based)... i suppose i'll need some help to use and understanding them... i want to give my help to this idea, trying to developing something or, at least, as a beta-tester

Bye
Piero

piero <piero74>
Thu 23 Aug 2007 07:09:21 AM UTC, comment #17: 

As we have already talk about that in some private emails, of course, it's good for me...

Frédéric Bernon <fbernon>
Project Member
Thu 23 Aug 2007 06:18:43 AM UTC, comment #16: 

Idea from bug #1902: create a timer thread for timeouts instead of waiting on semaphores?

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Thu 09 Aug 2007 02:29:42 PM UTC, comment #15: 

> Perhaps select should also be in the LWIP_POSIX_SOCKETS_IO_NAMES
> part of sockets.h ? Why not?


That might be a good solution if someone really wants to use select on files also.

> Is what you say is that a handle can't be a pointer? Why? Is
> there lot of environements where "int" and "pointer" don't have
> same size ? Perhaps some old ones?


What about windows on 64-bit platforms? Isn't an int 32-bit there while a pointer is 64-bit?

> As I said, select is not (and perhaps will never be) supported in
> sockets2.c.


I would disagree to the 'never will be'! And I also have the (far future-) goal of replacing sockets.c with sockets2.c (without changing the name, of course and after being thoroughly tested!)

Going back to the original discussion, we have two points here:
- replacing 'int' by a pointer casted to int: by now, I favour not to do this, since it doesn't work for all cases and we might to have to change it later if we want to implement select.
- OS integration: I think it would help to extend LWIP_POSIX_SOCKETS_IO_NAMES to not define all functions that are used both for sockets and files, so the user needs to implement a little wrapper function for them.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Fri 27 Jul 2007 10:43:06 PM UTC, comment #14: 

Interesting point of view, even if I don't share it on all the points.

I will try to do as short as possible (and sorry for my bad english). Most of IP stacks providers give a socket api without any OS integration. So, when I say "Socket API", don't think to Microsoft one (even if it's one of possibility), but more to the generic API provided by any stack providers (like Blunk, Fusion etc...).

I think that any "higher" integration with POSIX-IO functions like read, write, close, select, etc... is outside the lwIP scope. That's why such functions names can be disable to avoid conflict with unistd ones. Perhaps select should also be in the LWIP_POSIX_SOCKETS_IO_NAMES part of sockets.h ? Why not?

>...many embedded systems don't have file systems


many, but not all. Of course, mine got a flash file system, and can open file from URL path, UNC path or local flash paths.

> Even then, most people use select for network operations, not file operations


select can also be used to "route" data from a serial port to a network socket for "full" duplex protocols implemented with one task. But I'm agree, most of people mainly think to "select" for socket.

>With eCos you can use the full POSIX-style filesystem API, but you don't have to - it's an optional component.


Yes, I know, I use eCOS on some projects, and there is lot of things to say about eCOS... (good? things, of course ;) )

>But the point is that for those who are wanting to use select(), they expect it to behave like select(). If you want to do something different, don't mislead people by calling it select()


As I said, select is not (and perhaps will never be) supported in sockets2.c.

>I don't think we should have two "socket APIs", both with similar but different semantics and API definitions. This would be a source of confusion for users, maintenance overhead and a distraction for developers. Can't we just as well improve the implementation behind the current APIs?


I think we have already improve the current design, but Simon, me (and some others ?) would like to see a such new design for sockets API. In all cases, since sockets2.c is based on rawapi, maintenance overhead is very low. So, feel free to continue to improve socket/netconn layer if you want (and to use it, of course). You will not to have to use sockets2.c (if you don't want).

>I think it would be best to understand that what's being worked on here is the right approach in principle, before you spend too much time on it. If there's a slight modification to the approach that will make the outcome better, then that saves a lot, yes?


Measures I already did show that this new design give really better results that current socket ones. That's why I think it "better" (more efficient and simple).

One again, perhaps it will stay "outside" lwIP release. Currently, I "swap" between sockets.c and sockets2.c (only with my makefile), without any problems (on the UDP part). I just hope it could help some others developers on their projects when it will be ready...

Thank you for your informations...

Frédéric Bernon <fbernon>
Project Member
Fri 27 Jul 2007 06:46:06 PM UTC, comment #13: 

Frédéric Bernon wrote:

>>This is not the sockets API
>
> Is what you say is that a handle can't be a pointer? Why? Is there lot of
> environements where "int" and "pointer" don't have same size ? Perhaps some
> old ones?


In a standard BSD (or POSIX) sockets API, the "handles" are low numbered integers. There are constraints on their maximum values - FD_SETSIZE and OPEN_MAX at least. They can't be arbitrary values. In practice, they are expected to be used in the implementation as the index to an array - usually of generic file descriptors, but for lwIP that would be the struct lwip_socket array.

>>then I don't think you should provide select() at all.
>
>
> Yes, I'm agree (to not provide select :) ). More, "select" is a part of POSIX
> API, not of which is called Socket API.


I don't understand. The API is precisely based on the BSD style (now standardised in POSIX) API, and lwIP includes the LWIP_COMPAT_SOCKETS option precisely for the reason of providing something which is the BSD/POSIX sockets API to ease porting of existing code. What "Socket API" are you referring to? Microsoft only?

> Current "select" can't even be used
> with a real POSIX OS, since you can used in a same call a file (or a serial
> port) and a socket.


Firstly, that's mainly because many embedded systems don't have file systems (or they can be configured out). Even then, most people use select for network operations, not file operations. Anyway, integrating lwIP sockets into the OS select() call is beyond the scope of what we could ever expect lwIP to do itself. As it happens, my employer has it on my plate some time in the future to integrate lwIP into our OS's sockets API. That would undoubtedly require changes in lwIP, but the details are entirely OS-specific and outside of lwIP.

Additionally, you only get the select() definition if you include the lwIP sockets API header. You don't have to do that in the same files as the ones doing file operations. There's already a more common potential conflict with the read() and write() functions in that respect.

To use my own example, there are quite a few people using lwIP under eCos. With eCos you can use the full POSIX-style filesystem API, but you don't have to - it's an optional component. Either way, you can use the network functions in lwip/sockets.h directly (but if you do have the POSIX-style filesystem, not in the same module).

But the point is that for those who are wanting to use select(), they expect it to behave like select(). If you want to do something different, don't mislead people by calling it select(). Or maybe implement poll() which fits better with what you are wanting to do.

But it all seems like a lot of pain for users to save an array indirection.

>>I disagree strongly. People use it a lot.
>
> I'm not so sure than you, but, except ask to "all" users, I suppose it could
> be difficult to change my (or your) point of view on that.


We have eCos users in the community, and the eCos operating system itself. We have code that works with little modification on both lwIP and a "true" BSD stack. This moves further and further away from that.

> Just to refresh some tips of that (since you perhaps don't have the time to
> read all previous emails), there is two features:
>
> - LWIP_TCPIP_CORE_LOCKING (which can be used with current
> socket/api_lib/api_msg) to avoid task switching by using a semaphore.
>
> - sockets2.c which need LWIP_TCPIP_CORE_LOCKING=1 and where the idea is to
> get something independant of netconn layer, directly based on raw-api. This
> "task" is for that.


So what is this intended to achieve? Another API unique to lwIP that is not BSD/POSIX and is in addition to the lwIP-specific netconn one?

What you're proposing at this point seems to be half way between the netconn API and BSD sockets, but using the names of the BSD API functions.

> About the aim of this new sockets2.c file, the idea isn't to do the same
> thing than the current sockets.c one, but to propose an alternative (and not
> to replace the current one).


I don't think we should have two "socket APIs", both with similar but different semantics and API definitions. This would be a source of confusion for users, maintenance overhead and a distraction for developers. Can't we just as well improve the implementation behind the current APIs?

I think it would be best to understand that what's being worked on here is the right approach in principle, before you spend too much time on it. If there's a slight modification to the approach that will make the outcome better, then that saves a lot, yes?

Jifl

Jonathan Larmour <jifl>
Project Member
Fri 27 Jul 2007 04:00:33 PM UTC, comment #12: 

>This is not the sockets API


Is what you say is that a handle can't be a pointer? Why? Is there lot of environements where "int" and "pointer" don't have same size ? Perhaps some old ones?

>then I don't think you should provide select() at all.


Yes, I'm agree (to not provide select :) ). More, "select" is a part of POSIX API, not of which is called Socket API. Current "select" can't even be used with a real POSIX OS, since you can used in a same call a file (or a serial port) and a socket. OS which want to support that have to got something like a "IO layer" to abstract all "handles" type, and select have to be implement at these layer. So, to my point of view, suchs POSIX-like calls have to be at a "upper" level than the IP stack. I think there was a thread in the mailing list about that (lwIP doesn't have to directly support POSIX calls like read, write, etc...). lwIP integration in such OS, should be done with a system-layer like (in pseudo-code):

read(handle)
  IO subsystem determine the IO driver to use
    IP stack IO driver
      lwIP's read

>I disagree strongly. People use it a lot.


I'm not so sure than you, but, except ask to "all" users, I suppose it could be difficult to change my (or your) point of view on that.

Just to refresh some tips of that (since you perhaps don't have the time to read all previous emails), there is two features:

- LWIP_TCPIP_CORE_LOCKING (which can be used with current socket/api_lib/api_msg) to avoid task switching by using a semaphore.

- sockets2.c which need LWIP_TCPIP_CORE_LOCKING=1 and where the idea is to get something independant of netconn layer, directly based on raw-api. This "task" is for that.

About the aim of this new sockets2.c file, the idea isn't to do the same thing than the current sockets.c one, but to propose an alternative (and not to replace the current one). I even think it could be see like a big raw api sample!!! Except netbuf part, the code is fully separate from current sockets/api_lib/api_msg. It's currently a "draft" with some "dirty" hacks (that's why I set it in "Private", and why it's not in CVS), and lot of things have to be implement. I think to work on it this summer (on the beach!!!). But except Simon and me, there is not lot of developers active on that (snif). Perhaps, it will be something which finally will "live" outside the official lwIP release, perhaps in a contrib module? I don't know...

Thank for your remarks.

Frédéric Bernon <fbernon>
Project Member
Fri 27 Jul 2007 02:30:22 PM UTC, comment #11: 

I don't think it's right to cast a pointer as the returned socket handle (int). This is not the sockets API and will break code that uses it. If this API is to be used for easy porting of code, then you can't just change bits you don't like.

If this is the type of approach you want to take, then I don't think you should provide select() at all. Instead provide poll(). What's the point of providing an API which is sort of POSIX-like and sort of Windows-like? That does no-one any good. It's not like you can dispense with the struct lwip_socket array so the savings are minimal.

As for "I think by now, I favour the windows way, because select is 'rarely' used", I disagree strongly. People use it a lot.

Jonathan Larmour <jifl>
Project Member
Fri 06 Jul 2007 12:45:04 PM UTC, comment #10: 

The sockets2.c file attached here is not "operationnal", so, to avoid questions from users who could see it, I have set it private until it reach an stable level. I have attach it to let members like Simon (and others) do remarks about it...

When it will be "good", we could rechange it to "public" (I have suppose that "Privacy" is done to mask the task to non-members).

Frédéric Bernon <fbernon>
Project Member
Fri 06 Jul 2007 12:31:54 PM UTC, comment #9: 

Why the change to make this "private"?

Kieran Mansley <kieranm>
Project Member
Tue 03 Jul 2007 05:34:34 PM UTC, comment #8: 

(Simon Goldschmidt wrote about comment #7):

>I'm agree, the windows way is good for me. About sizeof problem, since these two types are based on processor word size, I think there is no problem.


It's not? I don't think int is guaranteed to be 64 bit on 64-bit systems (that have 64-bit pointers)! In contrast, microsoft AND gcc seem to declare ints as 32-bit both for 32-bit and 64-bit systems! That could keep us from doing it the microsoft way since we want socket() to return an int, and not a typedef like SOCKET (like microsoft does it).

>...If we want to do that, a semaphore in lwip_socket have to be add.


I wouldn't want to support this, but if we say sockets can be used from multiple threads, we have to protect them from multiple writes! It could be enough to set a flag that someone is writing and in the other thread, return something like EAGAIN.

>... about lwip_shutdown


I'd like it to behave like other socket implementations do, only I don't know how they behave. I'll check that with linux...

Frédéric Bernon <fbernon>
Project Member
Tue 03 Jul 2007 05:31:56 PM UTC, comment #7: 

>This way, select is slower, but normal operations are faster. I think by now, I favour the windows way, because select is 'rarely'

used and the rest of the operations are faster. We have to have a look on sizeof(int) versus sizeof(void*), though!

I'm agree, the windows way is good for me. About sizeof problem, since these two types are based on processor word size, I think there is no problem.

>Then, of course, your idea to replace socket_malloc by memp_malloc is a good one! BTW: sys_sem_wait(socksem) can be replaced by SYS_ARCH_PROTECT as long as you don't call any OS-functions in between that could lead to scheduling.


I could replace it by call to SYS_ARCH_DECL_PROTECT / SYS_ARCH_PROTECT / SYS_ARCH_UNPROTECT since if socksem only used to protect the "sockets" table. But since I think to use memp, I think keep socket_malloc & socket_free for internal initializes/allocations/desallocations, but remove the "sockets" table. To do that, the "struct lwip_sockets" should be in a header file. I thought to a sockets2.h with only this struct inside. But to add it in memp, I have to define an option to avoid conflit with current sockets.c file. I thought to LWIP_SOCKETS2. We could also use it for the next subject...

>I think that's a loss that some can't afford.


The waste could even be 8 bytes for each pbuf is we suppose a 32bit alignement. The problem is the packet got the information inside, but "parse" it to get it has a cost. But there is also a waste if you take you example, since each pbuf header has a cost of 16 bytes (with 32bits pointers). So, if you need 12 PBUF_POOL to receive, you "wasted" 16*11 = 176 bytes. That why, since I transfer big video streams, use PBUF_POOL_BUFSIZE set to LWIP_MEM_ALIGN_SIZE(1518) (+4 bytes for 802.1q VLAN tags). Of course, if you receive small packets, it's not a so good idea... But I still prefer the idea to add these fields if LWIP_SOCKETS2=1.

>I don't think it deadlocks. tcp_recved() calls tcp_output()


I don't think it could cause problem, but it's something to check. The implementation of SO_RCVBUF option could help to process this problem...

>That's a hack, isn't it? (The sleep(0), I mean.)


Yes, it's just to progress, and not a thing to keep.

>we get an error from tcp_write, we should block ...

In a general way, I think we should use something like your changes for remove conn->sem (but I don't find time to study your change).

>We would have to lock other writes until the first has finished to prevent from having both writes mixed their data.


In multithread cases to process, multiples "send" (and not "sendto") on the same socket is not something we have to process. If we want to do that, a semaphore in lwip_socket have to be add.

>about lwip_shutdown...


I don't know, but I'm not sure that - with both sockets implementation - it's very good to do something in lwip_shutdown (since lwip_close could be call twice, and in the second one, the "socket handle" is invalid). I think we should replace it by a define which do nothing, or an empty function.

>set/getsockopt...


Since the first switch is only "parameter checking", and having it help to reduce the "locked time" of the core. More, if also help to synchronize sockets2.c with sockets.c. About code space, I'm not sure it change a lot, so...

>nonblocking I/O...


I don't know, but timeout on accept exists - even on lwIP now ;) Since it could help to reduce a little bit the code, don't you think that SO_RCVTIMEO should be mandatory for sockets2.c? One "int" more in lwip_socket, but it's not a big cost. In current file, I have let "int domain", but I will remove it...

>Nonblocking close should be handled by SO_LINGER.


Agree with you.

>...I would implement fastsock functions...


Agree, extend socket API is a good solution (perhaps there is some studies on the same problem on the Net), but I'm more in flavour to not show lwIP internal structs like pbuf at the application level. About callbacks, It could be a solution, but since they are in "locked core" mode, it's more dangerous to let an application developer play with that (of course, this feature could be "reserved" to advanced users, but in this case, RAW API is already available). But I think we should postpone that.

>The goal for that would be to remove netconn api in the future to have only one simple & fast API above the raw API.


Agree with you, but if both API could exist in the same repository, it's not a problem for me (of course, it give more job to maintain it, but I think that now, the main job is done for current sequential layer).  I think that Jonathan will be not very happy if we decide to remove netconn API. So, to my point of view, the important thing is to have an independant socket API, to be able to add features without having to "negociate" how to do it with netconn users...

>OK, this list is somewhat disordered and might not be complete. But I think the file is a fine start! Thanks for working on that!


And thanks to work with me on that.

Frédéric Bernon <fbernon>
Project Member
Tue 03 Jul 2007 05:23:47 PM UTC, comment #6: 

(Simon Goldschmidt wrote about the sockets2.c file attached):
Hi,

some remarks about the file:

 >>>>
/** @todo we should replace that by a memp_malloc */
#define MEMP_NUM_SOCKETS MEMP_NUM_NETCONN
<<<<

You return a void* casted to an int as the socket handle. That's a cool
& fast way to implement (I also had that idea some time ago, LOL! :) but
it's getting harder later: You have to provide a simple way for the
FD_SETs select() uses to set/get/delete a file descriptor from an
FD_SET. There are two ways to do it:
a) the linux way: return an int as socket file descriptor, require the
programmer to give the highest fd number, create an array from 0 to
highest_fd_number with one bit for each fd in that range. Set/get/delete
is very fast then (e.g. 'fd_set |= (1<<fd)')
b) the windows way: return a void* as socket handle, implement the
FD_SET as an array of structs containing the void* and a bit for set/not
set. This way, select is slower, but normal operations are faster.

I think by now, I favour the windows way, because select is 'rarely'
used and the rest of the operations are faster. We have to have a look
on sizeof(int) versus sizeof(void*), though! Then, of course, your idea
to replace socket_malloc by memp_malloc is a good one! BTW:
sys_sem_wait(socksem) can be replaced by SYS_ARCH_PROTECT as long as you
don't call any OS-functions in between that could lead to scheduling.

 >>>>
<<<<

About the recv mboxes: by now, I favour your idea of somehow including
the ipaddr/port into pbufs for speedup. But that definitively has to be
an option, as it 'wastes' 6 bytes on each pbuf! Or how could we solve
that? I mean, if you have PBUF_POOL size set to 128 bytes, you waste 72
bytes when receiving one packet of 1514 bytes (a full ethernet packet).
I think that's a loss that some can't afford.

 >>>>
  /** @todo is it possible to directly call tcp_recved? deadlocking case? */
  tcp_recved(sock->pcb.tcp, len);
<<<<

I don't think it deadlocks. tcp_recved() calls tcp_output() (through
tcp_ack/tcp_ack_now), which only sends data and doesn't receive, so
pcb->recv shouldn't get called again. We have to watch out for slow
targets, though (where tcp receives faster than the application reads,
especially with the RX-thread having a higher prio than the application
thread).

 >>>>
lwip_send
<<<<

That's a hack, isn't it? (The sleep(0), I mean.) while (tosend > 0) and
we get an error from tcp_write, we should block and let the RX task (or
lwip core task) deal with transmission/ACKing which is (aside from
tcp_poll) the only point where ERR_MEM could have gone.

more general:
- using sockets from multiple threads: does that work if we only lock? I
think it should, but there is a problem with lwip_send/tcp_write when
running out of RAM / send-buffer! We would have to lock other writes
until the first has finished to prevent from having both writes mixed
their data.
- about lwip_shutdown: how is it implemented in other stacks? Is a FIN
sent when closing the TX side? For the RX side, can we simply throw away
the data inside sock_tcp_recv?
- set/getsockopt: I think we could combine the test with the 'actual
option processing' to save some code space (the switch is done twice....)
- nonblocking I/O: we should check RX byte counters for nonblocking
recv, TX bufsize for nonblocking send. Nonblocking connect can return
EAGAIN (so we have to remember we are currently in a connect sequence).
What about nonblocking accept, does that exists? Nonblocking close
should be handled by SO_LINGER.

Last but not least (but that can be postponed), I would implement
fastsock functions that return the pbufs directly (to prevent
copy-on-receive) and take pbuf (to prevent copy-on-write, but that's
harder for the programmer) and maybe also RX-callback functions (e.g.
inside sock_tcp_recv/sock_udp_recv, instead of posting to an mbox, call
a callback function) to be even faster and still kind of compatible to
the sockets API. The goal for that would be to remove netconn api in the
future to have only one simple & fast API above the raw API.

OK, this list is somewhat disordered and might not be complete. But I
think the file is a fine start! Thanks for working on that!

(file #13236)

Frédéric Bernon <fbernon>
Project Member
Sat 30 Jun 2007 06:04:07 PM UTC, comment #5: 

TCP connections should not be limited to returning the contents of one netbuf only when receiving.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Sun 10 Jun 2007 07:03:46 PM UTC, comment #4: 

>remove LOCK code from sys.c to be cleaner, instead LOCK/UNLOCK all timer/timeout functions
>LOCK in tcpip_thread only when a message is processed, UNLOCK after processing (before waiting for next message)


For these two points, in my first test, that what I did, but the current code doesn't really affect performance, and is less "intrusive" in the current source code. So, I prefer the current solution, even if it good be a little bit less efficient - but it's really a little bit. It also avoid to patch each timers function, like for the future integration of AutoIP timers. I will measure each solution, except if others have a real preference for such or such solution.

>LOCK in tcpip_ethinput and call ethernet_input directly (this one should be an option since it does not work if someone calls netif->input() from interrupt context).


Yes, even if it's not one of the first steps we have to do, it's seems to be the good solution. Perhaps add a different tcpip_ethinput function, like tcpip_ethinput_lock (by 
symmetry with tcpip_apimsg_lock).

>The tcpip_thread is effectively used only for timeouts then.


Yes, and in this case, a new design for timers handling can be study (avoiding to alloc/desalloc, but just reinsert the "timeout handler" in order in the list, or just a counter and a check modulos of this counter, like in the previous code of DHCP timers).

Note that all these previous points are more "Sequential API with LWIP_TCPIP_CORE_LOCKING" than pure "Socket Layer".

But, before all that, is there any objections about adding a "sockets2.c" file, to avoid patch like in lwip_sendto? I would like to remove it (the lwip_sendto's patch) as soon as possible...

Frédéric Bernon <fbernon>
Project Member
Sun 10 Jun 2007 11:37:24 AM UTC, comment #3: 

I've worked on that a little and I attach a patch that does the following:
- remove LOCK code from sys.c to be cleaner, instead LOCK/UNLOCK all timer/timeout functions
- LOCK in tcpip_thread only when a message is processed, UNLOCK after processing (before waiting for next message)
- LOCK in tcpip_ethinput and call ethernet_input directly (this one should be an option since it does not work if someone calls netif->input() from interrupt context). The tcpip_thread is effectively used only for timeouts then.

(file #13002)

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Sat 09 Jun 2007 06:02:54 PM UTC, comment #2: 

>Why not include all functions in 'socket2.c'? We don't have to seperate the code into 'application thread code' and 'tcpip_thread code' like with api_lib/api_msg. Just lock the core and go on with the code...


But there is still a part really near the raw api, like all the callbacks. Because most could be similar to current api_msg.c, keeping the same name to avoid to get a very large file is not a so bad idea. This choose is very linked to RX part design (see next point)

>We wouldn't even need the tcpip_thread any more. Simply lock and call into the core from your RX-thread (if it's not done in interrupt context).


I don't think about a such design for the RX part. But when we will be at this step, we could talk about that. Note that most OS have restricted call in IT context. But, ok, we will study that if you want...

>Even timeouts can be done easier: Either in an extra thread or in the RX-thread. Both version would remove the need for a per-thread timeout we have now. That way, sys_sem_wait and sys_mbox_fetch get smaller, too.


If we except PPP, I even thing that the full sys_timeout can be replace by something more optimized (without any memp calls). And if all timers are handled vy tcpip_thread, so, yes, no need to get a per-thread timeouts list.

>In a further step I'd like to introduce finer graned locking so that e.g. only tcp and its pcb is locked while processing a received tcp segment. This could help prioritizing some connections over others.


Once we got something working and reliable with the global semaphore, yes, this step will be a good idea!!!

Frédéric Bernon <fbernon>
Project Member
Sat 09 Jun 2007 04:29:22 PM UTC, comment #1: 

> For this, perhaps alternative api_msg2.c file will be better to avoid problems.


Why not include all functions in 'socket2.c'? We don't have to seperate the code into 'application thread code' and 'tcpip_thread code' like with api_lib/api_msg. Just lock the core and go on with the code...

We wouldn't even need the tcpip_thread any more. Simply lock and call into the core from your RX-thread (if it's not done in interrupt context).

Even timeouts can be done easier: Either in an extra thread or in the RX-thread. Both version would remove the need for a per-thread timeout we have now. That way, sys_sem_wait and sys_mbox_fetch get smaller, too.

In a further step I'd like to introduce finer graned locking so that e.g. only tcp and its pcb is locked while processing a received tcp segment. This could help prioritizing some connections over others.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Sat 09 Jun 2007 11:40:29 AM UTC, original submission:  

Very first thing, it's a low task priority for developers which don't work on other tasks....

task #6935 "Problems to be solved with thecurrent socket/netconn API" have introduced the possiblity to communicate with the lwIP'core with a different way (a global semaphore insight the current mailbox feature).

This task is to define and implement what could be a new socket layer for that.

One of the problems is to avoid to do lot of patchs with #if/#else/#endif in the code (one is already done in lwip_sendto, but just to show the use of LWIP_TCPIP_CORE_LOCKING, and should be remove in some time).

So, a first idea would be to add in the repository a sockets2.c file in the same folder than the current one.

A second one is to process in multiple steps:

1/ Integrate the global mutex in tcpip.h/.c, api_msg.c, and api_lib.c (already done).

2/ Use the global mutex to simplify most of sockets functions (like in the lwip_sendto sample), but keeping the netconn struct. We first have to decide if we are ok to add a sockets2.c file.

3/ Separate socket layer from netconn layer (the problem is of course in api_msg, where lot of fields directly reference netconn). For this, perhaps alternative api_msg2.c file will be better to avoid problems. In this step, I also propose to remove netbuf struct by adding "addr" & "port" fields directly in pbuf struct.

Resume some others goals:

- Should be reliable: something as reliable than raw api can be...

- Should be safe-thread for most of cases (and enable to support full-duplex protocols, see http://savannah.nongnu.org/patch/?5960).

- Easily disable code that is not used (like for select and non-blocking code, see http://savannah.nongnu.org/patch/?5919)

- Increase socket layer performance (speed) to got a performance near of raw api (limit use of netbuf, memp and other "dynamic" allocations if they can be replaced by local variables). Try to reach hardware performance as few of processor time as possible...

- Reduce footprint (for socket layer, of course, if we don't use all api_lib or api_msg),

- Get an independant socket layer from netcon layer, to enable to add some feature without changes in netconn layer.

- Have a better error forwarding: do difference between minor errors (ERR_MEM, ERR_BUF...), and really fatal errors...

- And anything else useful...

Comments?

Frédéric Bernon <fbernon>
Project Member

 

Attached Files
file #13236:  sockets2.c added by fbernon (50KiB - text/x-csrc)
file #13002:  lock_core.patch added by goldsimon (4KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by piero74 (Posted a comment)
  • -email is unavailable- added by jifl (Posted a comment)
  • -email is unavailable- added by kieranm (Posted a comment)
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by fbernon (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    Follow 11 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2015-02-17 goldsimon StatusPostponed => Cancelled
        Assigned toNone => goldsimon
        Open/ClosedOpen => Closed
    2013-01-15 goldsimon CategoryNone => socket/netconn
    2010-06-24 fbernon StatusIn Progress => Postponed
        Assigned tofbernon => None
    2009-03-15 fbernon PrivacyPrivate => Public
    2007-11-02 fbernon Priority3 - Low => 1 - Later
    2007-07-03 fbernon PrivacyPublic => Private
    2007-07-03 fbernon Attached File- => Added sockets2.c, #13236
    2007-06-10 goldsimon Attached File- => Added lock_core.patch, #13002

    Back to the top


    Powered by Savane 3.5