lwIP - A Lightweight TCP/IP stack - Tasks: task #6969, Review usage of conn->err in...
You are not allowed to post comments on this tracker with your current authentication level.
task #6969: Review usage of conn->err in netconn layer
Submitter: | Simon Goldschmidt <goldsimon> | ||
Submitted: | Fri 01 Jun 2007 05:22:37 PM UTC | ||
Category: | None | Should Start On: | Fri 01 Jun 2007 12:00:00 AM UTC |
Should be Finished on: | Fri 01 Jun 2007 12:00:00 AM UTC | Priority: | 5 - Normal |
Status: | Done | Privacy: | Public |
Assigned to: | goldsimon | Percent Complete: | 70% |
Open/Closed: | Closed | Planned Release: | 1.3.0 |
Effort: | 0.00 |
Jump to the original submission
Tue 11 Dec 2007 07:42:25 PM UTC, comment #34: |
Simon Goldschmidt <goldsimon> |
Tue 04 Dec 2007 10:09:51 PM UTC, comment #33: It's ok for me. "Minor" errors don't dead a pcb.
|
Frédéric Bernon <fbernon> |
Wed 28 Nov 2007 09:36:57 PM UTC, comment #32: If it's OK I think we can finally close this as done! |
Simon Goldschmidt <goldsimon> |
Tue 27 Nov 2007 10:23:19 PM UTC, comment #31: Seems good for me. But I will try to simulate some cases to be sure . |
Frédéric Bernon <fbernon> |
Tue 27 Nov 2007 09:28:53 PM UTC, comment #30: It's checked in: ERR_MEM, ERR_BUF and ERR_RTE are non-fatal. Also updated the err_to_errno table in sockets.c |
Simon Goldschmidt <goldsimon> |
Tue 27 Nov 2007 07:40:30 AM UTC, comment #29: definitively should go in! |
Simon Goldschmidt <goldsimon> |
Mon 26 Nov 2007 09:19:03 PM UTC, comment #28: If there are concerns about the suggestion, then just drop it. It's not important enough to be worth spending time debating. Go with the way you've been planning as far as I'm concerned!
|
Jonathan Larmour <jifl> |
Mon 26 Nov 2007 08:36:12 PM UTC, comment #27: Just to be sure, can we change the "planned release" to 1.3.0 ?
|
Frédéric Bernon <fbernon> |
Mon 26 Nov 2007 08:35:35 PM UTC, comment #26:
|
Frédéric Bernon <fbernon> |
Mon 26 Nov 2007 06:51:48 PM UTC, comment #25: On the one hand, you're right: marking the whole connection as dead because of one packet isn't good. But on the other hand, a TCP packet that is enqueued and can't be sent because it is too long will never be split up, so the whole connection is dead, anyway.
|
Simon Goldschmidt <goldsimon> |
Mon 26 Nov 2007 06:00:03 PM UTC, comment #24:
|
Jonathan Larmour <jifl> |
Mon 26 Nov 2007 05:23:50 PM UTC, comment #23: Our consensus was to only let ERR_MEM, ERR_BUF and ERR_RTE be minor errors. In addition to that, we wanted to add ERR_RETRY (although I'm not sure we really need this, in ip_output_if (my new 'patch' for bug #21643), we can return ERR_RTE also). All other errors would be fatal. This solution might not solve all kinds of errors in the really correct way, but it's way better than now.
|
Simon Goldschmidt <goldsimon> |
Mon 26 Nov 2007 04:35:44 PM UTC, comment #22: Define what could be "fatal" and what is not is not very easy. As you have see in the IRC chat, we have spend time to try to define which error codes should be. But, of course, it's mainly based on the lwIP core, and since each developers can use any code like he wants in his driver. This is mainly before the is no a "strict" definition of each code and when to return it. So, I think that once the fatal code would be defined, there will be some port breakages. But in fact, since any code different than ERR_OK already cause to block send/recv operations, I'm not sure this is a real problem.
|
Frédéric Bernon <fbernon> |
Mon 26 Nov 2007 04:20:04 PM UTC, comment #21: I saw some discussion on IRC about this task. Two things occur to me:
|
Jonathan Larmour <jifl> |
Mon 12 Nov 2007 11:02:03 PM UTC, comment #20: I don't see the problem with netconn->callback, can you give me for details about that? About comment #18, thoughts?
|
Frédéric Bernon <fbernon> |
Mon 12 Nov 2007 08:12:23 PM UTC, comment #19: Can this really wait until after 1.3.0? I think this (and maybe netconn->callback not being locked - what was the status of that?) is one of the last regions where lwIP isn't thread-safe (at least once bug #20900 is fixed)! |
Simon Goldschmidt <goldsimon> |
Sun 21 Oct 2007 07:34:27 PM UTC, comment #18: I think we should not forgot the initial problem: some errors are fatals, some others not.
|
Frédéric Bernon <fbernon> |
Sun 21 Oct 2007 04:05:31 PM UTC, comment #17: To make progress on this, I've made a list of all the places where conn->err is checked:
|
Simon Goldschmidt <goldsimon> |
Wed 29 Aug 2007 09:30:23 AM UTC, comment #16: It's always "in progress" to my point of view.
|
Frédéric Bernon <fbernon> |
Wed 29 Aug 2007 09:04:58 AM UTC, comment #15: Is this done? Anyone working on it? |
Kieran Mansley <kieranm> |
Tue 26 Jun 2007 06:05:33 PM UTC, comment #14: Yes, you're right, there is a memp_free(MEMP_TCP_PCB, pcb); with each TCP_EVENT_ERR(pcb->errf, pcb->callback_arg, ERR_RST);
|
Frédéric Bernon <fbernon> |
Tue 26 Jun 2007 06:01:40 PM UTC, comment #13: About "conn->pcb.tcp = NULL;", in err_tcp, I feel that like an error: if the field is invalided, how could do_delconn do the tcp_close (and indirectly call tcp_pcb_remove)? Or tcp_pcb_remove is called by the function which call err_tcp?
|
Frédéric Bernon <fbernon> |
Tue 26 Jun 2007 05:49:20 PM UTC, comment #12:
|
Simon Goldschmidt <goldsimon> |
Tue 26 Jun 2007 05:38:57 PM UTC, comment #11: About comment #8, I should add that api_lib should never set conn->err in any case (and should just read it). But about ERR_MEM, I'm not agree: if a lwip_sendto operation doesn't get a pbuf or a buffer in the device to send a datagram, it is an error (lwip_sendto should return -1), but not a "fatal" one (the application can retry).
|
Frédéric Bernon <fbernon> |
Tue 26 Jun 2007 04:27:30 PM UTC, comment #10: Sorry, forgot:
|
Simon Goldschmidt <goldsimon> |
Tue 26 Jun 2007 04:16:12 PM UTC, comment #9: What if we simply ignore conn->err on entering the functions?
|
Simon Goldschmidt <goldsimon> |
Thu 21 Jun 2007 07:42:00 PM UTC, comment #8: I think for this to solve, we should never set conn->err in api_lib.c (netconn_recv currently does that) and never set it to ERR_MEM in api_msg.c.
|
Simon Goldschmidt <goldsimon> |
Wed 13 Jun 2007 07:01:00 PM UTC, comment #7: Checked in the change in pcb_new() as I said in comment #3 (returning ERR_VAL like in comment #4). |
Simon Goldschmidt <goldsimon> |
Wed 13 Jun 2007 06:04:57 PM UTC, comment #6: I check in change like said un comment #5...
|
Frédéric Bernon <fbernon> |
Tue 12 Jun 2007 01:51:35 PM UTC, comment #5: No objects to change return instruction in netconn_peer and netconn_addr like this ?
|
Frédéric Bernon <fbernon> |
Mon 11 Jun 2007 08:04:08 PM UTC, comment #4: Right! ERR_VAL ? |
Frédéric Bernon <fbernon> |
Mon 11 Jun 2007 07:55:57 PM UTC, comment #3: Should we set msg->conn->err to != ERR_OK in pcb_new() in the dafault case (e.g. if a protocol pcb should be created that is disabled)??? |
Simon Goldschmidt <goldsimon> |
Sat 02 Jun 2007 11:11:12 AM UTC, comment #2:
|
Simon Goldschmidt <goldsimon> |
Sat 02 Jun 2007 10:51:28 AM UTC, comment #1: For netconn_peer and netconn_addr, I propose to simply replace :
|
Frédéric Bernon <fbernon> |
Fri 01 Jun 2007 05:22:37 PM UTC, original submission:
Currently, if conn->err != ERR_OK, many functions will do nothing and simply return.
|
Simon Goldschmidt <goldsimon> |
No files currently attached
Depends on the following items: None found
Items that depend on this one: None found
There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.
Follow 12 latest changes.
Date | Changed by | Updated Field | Previous Value | => | Replaced by |
---|---|---|---|---|---|
2007-12-11 | goldsimon | Status | Ready For Test | Done | |
Open/Closed | Open | Closed | |||
2007-11-27 | goldsimon | Status | None | Ready For Test | |
Percent Complete | 10% | 70% | |||
Assigned to | None | goldsimon | |||
2007-11-27 | goldsimon | Planned Release | None | 1.3.0 | |
2007-11-09 | kieranm | Priority | 7 - High | 5 - Normal | |
2007-11-09 | kieranm | Planned Release | 1.3.0 | None | |
2007-08-29 | kieranm | Priority | 5 - Normal | 7 - High | |
2007-07-13 | kieranm | Priority | 1 - Later | 5 - Normal | |
2007-07-13 | kieranm | Planned Release | None | 1.3.0 | |
2007-06-13 | goldsimon | Percent Complete | 0% | 10% |
Powered by Savane 3.13-02a9.
Corresponding source code
Fixed