tasklwIP - A Lightweight TCP/IP stack - Tasks: task #6930, Implement SO_LINGER

 
 

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

task #6930: Implement SO_LINGER

Submitted by:  Simon Goldschmidt <goldsimon>
Submitted on:  Wed 23 May 2007 09:49:46 AM UTC  
 
Category:  TCP Should Start On:  Wed 23 May 2007 12:00:00 AM UTC
Should be Finished on:  Wed 23 May 2007 12:00:00 AM UTC Priority:  1 - Later
Status:  Done Privacy:  Public
Percent Complete:  100% Assigned to:  Simon Goldschmidt <goldsimon>
Open/Closed:  Closed Planned Release:  None
Effort:  0.00

( Jump to the original submission)

Tue 10 Feb 2015 09:17:15 PM UTC, comment #26: 

Done.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Mon 09 Jul 2007 01:03:14 PM UTC, comment #25: 

That's why I ask if this is also something legitimate... :)

Frédéric Bernon <fbernon>
Project Member
Mon 09 Jul 2007 12:59:13 PM UTC, comment #24: 

> do you think it's legitimate to recv data on a socket, and next, to set it in "listen" mode?


That doesn't work right now: tcp_listen() tries to remove the given pcb from the list 'bound_pcbs'. If that pcb is on the 'active_pcbs' list, it will stay there and subsequent input packets will be given to it, although it is freed (memp_free is called in tcp_listen).

So while it looks like it works, it doesn't really and so there should be a check for it.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Mon 09 Jul 2007 12:54:29 PM UTC, comment #23: 

I think that Simon change the CVS HEAD like you wish: the netconn_close call the do_close, which do the "do_close_internal" (using poll_tcp & sent_tcp to terminate the operation - if need). The same thing is done by netconn_delete (so, lwip_close doesn't need to call netconn_close).

But about what you describe (close & re-open the netconn), we have another thing like that (something which is possible, but not a common usage) : do you think it's legitimate to recv data on a socket, and next, to set it in "listen" mode?

Frédéric Bernon <fbernon>
Project Member
Mon 09 Jul 2007 12:41:56 PM UTC, comment #22: 

I can see that some users would want to keep netconn_close and netconn_delete separate, and moving the close functionality into the delete function would not work.  I think it's legitimate for example to call netconn_close, then use the same netconn to open a new connection (without calling netconn_delete).  I'd therefore prefer to keep these functions as is.

Kieran Mansley <kieranm>
Project Member
Sun 01 Jul 2007 04:14:42 PM UTC, comment #21: 

I've attached a patch for the close problem at bug #20021 (the conn->sem thing).

I've created an extra static function that closes the connection, this function is called again from poll/sent until it's closed or there is an error other than ERR_BUF (tcp_output failed).

This function is called from do_close and do_delconn, which can be modified later.

-> I'll apply it like that if noone objects!

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Sat 30 Jun 2007 07:37:12 AM UTC, comment #20: 

Hmm, although the function seems unnecessary and a little buggy (for a listening conn, tcp_close is called in netconn_close, the tcp gets freed, tcp_close may be called in netconn_delete again...), I think it's a little weird to not have a netconn_close function.

But on the other side, close and delete are a bit related. So for me, it would be ok.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Fri 29 Jun 2007 07:14:26 PM UTC, comment #19: 

So, ok to remove netconn_close. Perhaps just a define like this for source code compatibility?

#define netconn_close(c) (ERR_OK)

Frédéric Bernon <fbernon>
Project Member
Fri 29 Jun 2007 04:28:58 PM UTC, comment #18: 

> I think that most of time, applications developers with
> netconn layer write (like Magnus said) :
>
> netconn_close(conn);
> netconn_delete(conn);


It's OK if developers do that, but what if there's code in between? I think either we should completely remove netconn_close or close the connection when calling the function. Having an empty function is, in my opinion, a real hack!

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Fri 29 Jun 2007 04:16:41 PM UTC, comment #17: 

>Why an empty netconn_close? What about netconn api software that relies on that functionality?


I think that most of time, applications developers with netconn layer write (like Magnus said) :

netconn_close(conn);
netconn_delete(conn);

So, what is the real use of netcon_close in this case? And I think that's why the socket layer doesn't call it. So, do all the process in netconn_delete (which we have to call in all cases) will help to have a good code in all - current - sequential api layers...

Frédéric Bernon <fbernon>
Project Member
Fri 29 Jun 2007 03:58:19 PM UTC, comment #16: 

Why an empty netconn_close? What about netconn api software that relies on that functionality? After all, I think it's cleaner to close the connection in netconn_close and delete it in netconn_delete, like the names say.

Closing/deleting is not performance-sensitive in most applications, anyway, so it's not that bad to have two functions...

And in a new socket layer, we can do everything in one function ;-)

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Fri 29 Jun 2007 03:42:53 PM UTC, comment #15: 

Perhaps it will be good to do an empty netconn_close for source code compatibility, and only do the job in netconn_delete?

Frédéric Bernon <fbernon>
Project Member
Fri 29 Jun 2007 03:32:43 PM UTC, comment #14: 

> The problem isn't it more the call (in do_delconn) to
> tcp_abort "too early" ?


I think you might be right. As Jonathan posted in that bug, http://www.opengroup.org/onlinepubs/009695399/functions/setsockopt.html  says: "the system handles the call in a way that allows the calling thread to continue as quickly as possible". But it has to send the data! On the other hand, netconn_delete is called if you really want to free the netconn.

Maybe it's better to call netconn_close and there we should try again if tcp_close fails. After that, we can safely call netconn_delete without fearing that it aborts the connection, since it's already closed.

Otherwise, we implement the same problem twice. Maybe netconn_delete should first call netconn_close so we can leave away the state tests in netconn_delete that are already implemented in netconn_close.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Fri 29 Jun 2007 01:11:25 PM UTC, comment #13: 

A question about that: in the case of a web server, an external client connect my lwip http server and request a "big" file. The web server return the file (with the size in HTTP header), the client receive it, both close the connection.

In this case, the lwIP web server do a lwip_close "in the same time" that the client send a FIN packet (both know when close because the size of the datas is in the header).

In my first time with lwIP I often got problem where the client doesn't receive all the datas... A workaround I test was to add netconn_close in lwip_close (before netconn_delete), and in netconn_close, to poll that unacked & unsent tcp_seg lists were NULL (and next, do the normal "do_close"). A such solution was working, even it is was in fact an experimental workaround.

I think my problem was a little bit the same than Magnus: lwip_close calls netconn_delete which call tcp_close with the send queue still full, so, tcp_abort was called and my datas lost. This problem was difficult to reproduce, since it was a timing problem (faster was the client and the network, less I saw the case). That what I got in bug #19157.

Now, my question: since do_close shouldn't wait that datas could be sent, more, since do_close is not call (by default) in lwip_close, isn't it in the do_delconn that the problem should be processed (like Simon propose)? The problem isn't it more the call (in do_delconn) to tcp_abort "too early" ?

Frédéric Bernon <fbernon>
Project Member
Thu 28 Jun 2007 02:58:34 PM UTC, comment #12: 

Re comment #10:

> netconn_close/do_close now just do a tcp_close for NETCONN_TCP,
> and return ERR_VAL for the other


Yes, ERR_OK might be better for other types.

>  do you want or not to call netconn_close (and also
> netconn_delete) in lwip_close (it's not in your patch)?


netconn_delete is definitely needed in lwip_close. since it already calls tcp_close, netconn_close is not needed, I think.

But my patch isn't correct after reading Magnus Berglund's post. Waiting for tcp_close returning ERR_OK is definitely needed. I would handle it by not acknowledging in do_close and call tcp_close again from sent_tcp or poll_tcp. That way, we both avoid the loop in netconn_close and the need for conn->sem (faster, less context-switches, no race conditions).

I'll prepare a patch.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Thu 28 Jun 2007 02:54:23 PM UTC, comment #11: 

Magnus Berglund wrote on lwip-devel:

I've been seeing some strange behaviour in 1.2.0 related to the fact that do_close does not call tcp_close if the state is ESTABLISHED.

What happens is the following:

1. The app calls:
   netconn_write() (a couple of time really fast),
   netconn_close()
   netconn_delete()

2. netconn_write() fills up the send queue to TCP_SND_QUEUELEN
   The loop condition in netconn_write loops until there is room on the sendqueue.

3. netconn_close() does nothing in do_close() since the state is ESTABLISHED.

4. netconn_delete()/do_delconn() does the following:
   - Calls tcp_close(). At this time the send queue is still full since no ack has been
     received for the sent packets. tcp_close() will therefore fail with EMEM.
   - tcp_abort is called, and the connection is aborted.

I have done a similar patch to your patch to do_close, and this seems to fix my problem.
This is due to the fact that netconn_close will loop and wait for space to be available on the sendqueue before returning.

I think your patch to do_close is correct since tcp_close handles all the pcb states.
But I think maybe a loop similiar to the one in netconn_close is still needed. Otherwise, if the sendqueue is full the close will fail.

What do you think?

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Thu 28 Jun 2007 08:07:45 AM UTC, comment #10: 

If I resume your idea:

- netconn_close/do_close now just do a tcp_close for NETCONN_TCP, and return ERR_VAL for the other (in this case, I think to return ERR_OK will be better to simplify using, after all, at socket layer, we also "close" the socket - even if in fact, we only delete it & its conn).

- Not sure: do you want or not to call netconn_close (and also netconn_delete) in lwip_close (it's not in your patch)?

Frédéric Bernon <fbernon>
Project Member
Tue 26 Jun 2007 06:34:46 PM UTC, comment #9: 

Not that I would really need netconn_close() since lwip_close() calls netconn_delete() but doesn't netconn_close/do_close simply have to call tcp_close() instead of tcp_output()? That way it would be the same behaviour for both (tcp timers and wait-states handle unsent/unacked data).

I've attached a patch for this. We then can also remove conn->sem since it was only used for write/close errors.

(file #13178)

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Thu 24 May 2007 10:32:35 AM UTC, comment #8: 

Ooops, that shouldn't get sent since I discovered that the summary can be changed... Can we delete postings? :-)

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Thu 24 May 2007 10:31:02 AM UTC, comment #7: 

Ha, only now I realize it: is it possible to chenge

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Wed 23 May 2007 01:54:21 PM UTC, comment #6: 

Agreed: lets do SO_LINGER, and then if there's a compelling case for SO_DONTLINGER (which personally I doubt, but prepared to wait and see) we can make this decision then.

Kieran Mansley <kieranm>
Project Member
Wed 23 May 2007 01:40:38 PM UTC, comment #5: 

It's something that we could talk once someone will implement the SO_LINGER (if someone do it...)

Frédéric Bernon <fbernon>
Project Member
Wed 23 May 2007 01:37:45 PM UTC, comment #4: 

For the avoidance of doubt, my view is unchanged.

Jonathan Larmour <jifl>
Project Member
Wed 23 May 2007 12:23:28 PM UTC, comment #3: 

I'm agree that SO_DONTLINGER is only a "helper" option for "Windows" application developers (you can do the same thing with SO_LINGER), but I think there is no real reason to add ~2-3 lines for that...

Frédéric Bernon <fbernon>
Project Member
Wed 23 May 2007 10:44:38 AM UTC, comment #2: 

Re comment #1
As Jonathan said in bug #19157:
"SO_DONTLINGER is a non-standard option. I would be against its provision in lwIP on that basis"

That's my point of view, too. Anyway, can't linger be turned off by calling setsockopt(SO_LINGER) with l_onoff=0? Doesn't that achieve the same?

http://docs.hp.com/en/B2355-90136/ch03s01.html seems a little different from http://www.opengroup.org/onlinepubs/000095399/basedefs/sys/socket.h.html because the opengroup specification allows l_linger to provide a time, whereas hp only sets it on or off. Still, the HP link provides a good explanation:

>>>


  • If l_onoff is zero, close returns immediately, but any unsent data is transmitted (after close returns).
  • If l_onoff is nonzero and l_linger is zero, close returns immediately, and any unsent data is discarded.
  • If l_onoff is nonzero and l_linger is nonzero, close does not return until all unsent data is transmitted (or the connection is closed by the remote system).

In the default case (SO_LINGER is off), close is not blocked. The socket itself, however, goes through graceful disconnect, and no data is lost.
<<<

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Wed 23 May 2007 10:14:36 AM UTC, comment #1: 

Because SO_DONTLINGER would not add code to netconn api, but only to sockets api, I think we should add it. As I said in https://savannah.nongnu.org/bugs/?19157, comment #25:

"If one day, we integrate SO_LINGER, so I think that adding SO_DONTLINGER could be done in the same time. First, because it is already defined in sockets.h (for source code compatibility I think), second, because there will no lot of job to do. I could be useful to someone who activate SO_LINGER on a sockets, and, due to his application or protocol, need to come back to SO_DONTLINGER."

Even if it's come from Microsoft, I don't think we shouldn't refuse to propose a API compatible for these applications developers (because, the footprint increase would be really insensible...). We are "open" or not? ;)

Frédéric Bernon <fbernon>
Project Member
Wed 23 May 2007 09:49:46 AM UTC, original submission:  

Remove SO_DONTLINGER since it's Microsoft'ish.

A good explanation of SO_LINGER can be found at http://docs.hp.com/en/B2355-90136/ch03s01.html, although I don't know if that's standard (to be fiound out).

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.

 

Attached Files
file #13178:  netconn_close.patch added by goldsimon (2KiB - 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 kieranm (Posted a comment)
  • -email is unavailable- added by jifl (Posted a comment)
  • -email is unavailable- added by fbernon (Posted a comment)
  • -email is unavailable- added by goldsimon (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 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2015-02-10 goldsimon StatusNone => Done
        Percent Complete0% => 100%
        Assigned toNone => goldsimon
        Open/ClosedOpen => Closed
    2013-01-15 goldsimon CategoryNone => TCP
    2007-06-26 goldsimon Attached File- => Added netconn_close.patch, #13178
    2007-05-24 goldsimon SummaryImpelemt SO_LINGER => Implement SO_LINGER

    Back to the top


    Powered by Savane 3.5