lwIP - A Lightweight TCP/IP stack - Tasks: task #14314, Implement interface name/index APIs
You are not allowed to post comments on this tracker with your current authentication level.
task #14314: Implement interface name/index APIs
Submitter: | Joel Cunningham <jcunningham> | ||
Submitted: | Fri 20 Jan 2017 04:36:47 AM UTC | ||
Category: | socket/netconn | Should Start On: | Fri 20 Jan 2017 12:00:00 AM UTC |
Should be Finished on: | Fri 20 Jan 2017 12:00:00 AM UTC | Priority: | 5 - Normal |
Status: | Done | Privacy: | Public |
Assigned to: | jcunningham | Percent Complete: | 90% |
Open/Closed: | Closed | Planned Release: | None |
Effort: | 0.00 |
Jump to the original submission
Thu 23 Feb 2017 11:50:40 PM UTC, comment #70: |
Joel Cunningham <jcunningham> |
Thu 23 Feb 2017 07:44:03 PM UTC, comment #69: Fixed in a slightly different way. I copied your comment though (but forgot to mention that in the commit, sorry for that).
|
Simon Goldschmidt <goldsimon> |
Thu 23 Feb 2017 07:17:44 PM UTC, comment #68: No, I didn't. Too many comments here ;-) |
Simon Goldschmidt <goldsimon> |
Thu 23 Feb 2017 04:06:34 PM UTC, comment #67: Just in case you didn't see: I already added a patch to this thread for that.. :) |
David van Moolenbroek <dcvmoole> |
Thu 23 Feb 2017 04:04:54 PM UTC, comment #66: Right. I think we should add a uniqueness check to netif_add. The overhead should be ok for most use cases. I'll see if I can do this in the next days |
Simon Goldschmidt <goldsimon> |
Thu 23 Feb 2017 03:33:41 PM UTC, comment #65: Simon,
|
Joel Cunningham <jcunningham> |
Thu 23 Feb 2017 02:28:36 PM UTC, comment #64: Joel, if only if_nameindex (+free) are missing, shouldn't we just omit them until someone needs them?
|
Simon Goldschmidt <goldsimon> |
Mon 20 Feb 2017 01:44:31 PM UTC, comment #63: FreeRTOS/MPU should support the MPU thing (and was the original motivation for it). I don't know how many people use this, but it should certainly work. I can't test it though as my STM32 don't seem to have an MPU :-( |
Simon Goldschmidt <goldsimon> |
Thu 16 Feb 2017 07:18:19 AM UTC, comment #62: Is there anyone out there using that feature (MPU support)? Does it still work? I never had an opportunity to test it. |
Dirk Ziegelmeier <dziegel> |
Thu 16 Feb 2017 07:17:23 AM UTC, comment #61: Is there anyone out there using that feature? Does it still work? I never had an opportunity to test it. |
Dirk Ziegelmeier <dziegel> |
Wed 15 Feb 2017 10:50:50 PM UTC, comment #60: My two cents are that MPU support is significant extra work when doing API feature work. Almost to the point where making things work in MPU land was as much work as developing the feature.
|
Joel Cunningham <jcunningham> |
Wed 15 Feb 2017 07:50:02 PM UTC, comment #59: RE comment 56:
|
Simon Goldschmidt <goldsimon> |
Fri 10 Feb 2017 06:16:59 AM UTC, comment #58: Travis CI is showing a Unix port build failure with using errno in if_api.c. I took at stab a fixing it, but am currently limited to only having my MSVC port running |
Joel Cunningham <jcunningham> |
Fri 10 Feb 2017 05:03:03 AM UTC, comment #57: Couple of commits tonight:
|
Joel Cunningham <jcunningham> |
Thu 09 Feb 2017 09:24:41 PM UTC, comment #56: Simon,
|
Joel Cunningham <jcunningham> |
Thu 09 Feb 2017 08:36:40 PM UTC, comment #55: Re comment #45++:
|
Simon Goldschmidt <goldsimon> |
Thu 09 Feb 2017 08:00:11 PM UTC, comment #54:
|
Joel Cunningham <jcunningham> |
Thu 09 Feb 2017 07:53:02 PM UTC, comment #53: I'd vote for the _priv.h thing. |
Simon Goldschmidt <goldsimon> |
Thu 09 Feb 2017 07:51:38 PM UTC, comment #52: Would anyone be opposed to me moving the set_errno macro from sockets.c into sockets.h so I don't have to duplicate that logic for use in if_api.c?
|
Joel Cunningham <jcunningham> |
Thu 09 Feb 2017 07:47:12 PM UTC, comment #51: For this kind of new code (and given the few people discussing here), I think you can directly push if you think it's good. If we think differently, we can still change it later ;) Given the few developers sharing opinions here, lwip has evolved to be developed like that in the last years... I know it's much different than e.g. linux development...
|
Simon Goldschmidt <goldsimon> |
Thu 09 Feb 2017 07:32:23 PM UTC, comment #50: Dirk,
|
Joel Cunningham <jcunningham> |
Thu 09 Feb 2017 06:31:56 PM UTC, comment #49: I just noticed that setting errno is missing in if_indextoname
|
Dirk Ziegelmeier <dziegel> |
Thu 09 Feb 2017 04:00:00 PM UTC, comment #48: Ah fair enough, I don't know enough about the multithreading situation to comment on that. |
David van Moolenbroek <dcvmoole> |
Thu 09 Feb 2017 03:42:24 PM UTC, comment #47: David,
|
Joel Cunningham <jcunningham> |
Thu 09 Feb 2017 03:24:42 PM UTC, comment #46:
|
David van Moolenbroek <dcvmoole> |
Thu 09 Feb 2017 03:13:51 PM UTC, comment #45: There are two additional APIs from RFC 3493: if_nameindex() and if_freenameindex(). These two APIs are for getting a list of all the name/index pairs and then freeing that list.
|
Joel Cunningham <jcunningham> |
Thu 09 Feb 2017 11:21:37 AM UTC, comment #44: Joel, could you give me an update about the status here? I'm a bit lost figuring out what the remaining 50% are ;-) |
Simon Goldschmidt <goldsimon> |
Sun 05 Feb 2017 11:55:39 AM UTC, comment #43: Removed usage of netif->num in slipif.c |
Dirk Ziegelmeier <dziegel> |
Fri 03 Feb 2017 11:19:10 PM UTC, comment #42: include clean ups committed in: 4fb7d741657bbdd9e06759af55ca50710e6b4cba
|
Joel Cunningham <jcunningham> |
Fri 03 Feb 2017 09:43:24 PM UTC, comment #41:
|
Joel Cunningham <jcunningham> |
Fri 03 Feb 2017 09:36:54 PM UTC, comment #40: Well, lwIP has the (unwritten?) rule that C files and their header files have the same base name. There are some exceptions to that, I guess... (after all, it's just always been like this for lwIP :-)
|
Simon Goldschmidt <goldsimon> |
Fri 03 Feb 2017 09:35:15 PM UTC, comment #39: Also, I consider the header name more important because that's how an application developer interfaces with the API (and not the C file) |
Joel Cunningham <jcunningham> |
Fri 03 Feb 2017 09:28:30 PM UTC, comment #38: The <net/if.h> name is mandated by RFC 3493 under section 7 (Summary of New Definitions)
|
Joel Cunningham <jcunningham> |
Fri 03 Feb 2017 08:56:45 PM UTC, comment #37:
|
Simon Goldschmidt <goldsimon> |
Thu 02 Feb 2017 03:07:17 PM UTC, comment #36: if.c renamed to if_api.c in b6c995fed8a7783d06ab01fae314afe6e7d85b54 |
Joel Cunningham <jcunningham> |
Wed 01 Feb 2017 03:04:07 PM UTC, comment #35:
|
Joel Cunningham <jcunningham> |
Wed 01 Feb 2017 02:10:44 PM UTC, comment #34:
|
David van Moolenbroek <dcvmoole> |
Wed 01 Feb 2017 01:30:50 PM UTC, comment #33:
|
Simon Goldschmidt <goldsimon> |
Wed 01 Feb 2017 01:20:50 PM UTC, comment #32:
|
David van Moolenbroek <dcvmoole> |
Wed 01 Feb 2017 01:19:25 PM UTC, comment #31:
|
David van Moolenbroek <dcvmoole> |
Wed 01 Feb 2017 01:11:57 PM UTC, comment #30: Proposal: Make netif->num compliant to RFC -> no complicated mapping needed. Just let netif->num start at 1 and make 0 an invalid value. |
Dirk Ziegelmeier <dziegel> |
Wed 01 Feb 2017 01:07:24 PM UTC, comment #29: Sorry for not seeing this earlier, but I have to comment on this change a bit:
|
Simon Goldschmidt <goldsimon> |
Wed 01 Feb 2017 12:38:21 PM UTC, comment #28: I'm unsure about this. Best solution would be the user has to specify a number at netif_add(), but that would be a breaking change.
|
Dirk Ziegelmeier <dziegel> |
Tue 31 Jan 2017 08:53:56 PM UTC, comment #27: So far we've got a vote for if_api.c from Dirk. This is also my preferred choice. Any other input?
|
Joel Cunningham <jcunningham> |
Mon 30 Jan 2017 08:11:00 PM UTC, comment #26: Sylvain,
|
Simon Goldschmidt <goldsimon> |
Mon 30 Jan 2017 07:53:42 AM UTC, comment #25: I'd vote for if_api.c - maybe there is more that can fit into that file in the future |
Dirk Ziegelmeier <dziegel> |
Fri 27 Jan 2017 07:26:40 PM UTC, comment #24: For PPP the user can't change the init_cb netif callback. I will hit the netif_num overflows in one of "my" product where I am recycling a previously used PPP PCB for a GPRS backup link. (I will also hit it in PPP tests where I am creating/freeing PPP PCB for tests purposes but that's less important.)
|
Sylvain Rochet <gradator> |
Thu 26 Jan 2017 03:15:26 PM UTC, comment #23: I'm not opposed to that. Ideally I'd like the name to convey that the file contains the RFC standard interface name/index APIs. I followed how netdb.c and sockets.h are named after the header file. A couple of ideas:
|
Joel Cunningham <jcunningham> |
Thu 26 Jan 2017 02:47:47 PM UTC, comment #22: Could we change the name of "if.c" to somewhat more lwip-named?
|
Simon Goldschmidt <goldsimon> |
Fri 20 Jan 2017 09:00:36 PM UTC, comment #21: :) perfect, thanks again! |
David van Moolenbroek <dcvmoole> |
Fri 20 Jan 2017 08:59:22 PM UTC, comment #20: Committed in e158f87286512f5a6bba3c52bad06953e8eae6cd |
Joel Cunningham <jcunningham> |
Fri 20 Jan 2017 08:41:11 PM UTC, comment #19: Cool, thank you :) |
David van Moolenbroek <dcvmoole> |
Fri 20 Jan 2017 08:40:40 PM UTC, comment #18: Ok thanks for clarifying that. I'll push a new commit that moves the logic as per comment #3 and documents we also support netifs fully managing the number. Sorry for the mis-understanding :) |
Joel Cunningham <jcunningham> |
Fri 20 Jan 2017 08:37:27 PM UTC, comment #17: Yes :) Specifically I have an array of netif-containing structures, so I use an O(n) lookup to find a free one (a free list would make that O(1), even). Then I can set netif->num based on the array index. As such, in my use case, the netif_num value is completely unused, so it's also not a problem if it overflows or whatever, as long as I can set netif->num myself. |
David van Moolenbroek <dcvmoole> |
Fri 20 Jan 2017 08:36:17 PM UTC, comment #16: I just wanted to say lwIP should not try to resolve that case internally by adding logic that is bloat for "normal embedded systems". David's solution gives the user the opportunity to ensure unique netif numbers, so everything will just work in for the dynamic case.
|
Dirk Ziegelmeier <dziegel> |
Fri 20 Jan 2017 08:31:20 PM UTC, comment #15: David,
|
Joel Cunningham <jcunningham> |
Fri 20 Jan 2017 08:24:38 PM UTC, comment #14: Would you be so kind as to move the new netif_add() assert to after the init() callback call though? As it is now, I can no longer override netif->num from the init() call because the assert will trigger first, so things actually got way worse for me this way.. See also the order I sketched in comment #3. Thanks :) |
David van Moolenbroek <dcvmoole> |
Fri 20 Jan 2017 08:22:11 PM UTC, comment #13: Patches committed in:
|
Joel Cunningham <jcunningham> |
Fri 20 Jan 2017 06:21:32 PM UTC, comment #12: Just add it to Filelists.mk in lwIP main rep, and don't worry, I'll correct it if necessary. |
Dirk Ziegelmeier <dziegel> |
Fri 20 Jan 2017 06:14:52 PM UTC, comment #11: Triggering an LWIP_ASSERT when netif_num overflows is fine for me as well. I can add that to my patches.
|
Joel Cunningham <jcunningham> |
Fri 20 Jan 2017 05:59:01 PM UTC, comment #10: Yes sure! |
David van Moolenbroek <dcvmoole> |
Fri 20 Jan 2017 05:57:11 PM UTC, comment #9: If the LWIP_ASSERT is OK, then Joel can commit his patches, they are OK to me. |
Dirk Ziegelmeier <dziegel> |
Fri 20 Jan 2017 05:55:36 PM UTC, comment #8: Because this is supposed to be a lightweight stack, I'd vote for the LWIP_ASSERT David proposed. People who create/destroy lots of interfaces at runtime need to patch lwIP for that.
|
Dirk Ziegelmeier <dziegel> |
Fri 20 Jan 2017 04:58:35 PM UTC, comment #7: Basic implementation attached, which I hope will allow the rest to move forward as well here. :) |
David van Moolenbroek <dcvmoole> |
Fri 20 Jan 2017 03:32:39 PM UTC, comment #6:
|
David van Moolenbroek <dcvmoole> |
Fri 20 Jan 2017 03:03:33 PM UTC, comment #5: Thanks for the quick feedback and good catch on netif->num not being unique. I wasn't thinking of this issue either because my use case has static netifs (added once during power up). Also the boundary case of netif->num being 255 can't be converted to an index
|
Joel Cunningham <jcunningham> |
Fri 20 Jan 2017 11:50:55 AM UTC, comment #4: ..I should add that such an array of netif pointers would help speed up index-to-netif lookups during routing decisions. |
David van Moolenbroek <dcvmoole> |
Fri 20 Jan 2017 11:48:43 AM UTC, comment #3: Neat, Joel!
|
David van Moolenbroek <dcvmoole> |
Fri 20 Jan 2017 07:09:18 AM UTC, comment #2: /* Interface indexes always start at 1 per RFC 3493, section 4, num starts at 0 */
|
Dirk Ziegelmeier <dziegel> |
Fri 20 Jan 2017 04:53:39 AM UTC, comment #1: Attached patch adds the following APIs:
|
Joel Cunningham <jcunningham> |
Fri 20 Jan 2017 04:36:47 AM UTC, original submission:
Implement the following top level APIs from RFC 3493: Basic Socket Interface Extensions for IPv6, Section 4: Interface Identification
|
Joel Cunningham <jcunningham> |
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 13 latest changes.
Date | Changed by | Updated Field | Previous Value | => | Replaced by |
---|---|---|---|---|---|
2017-02-23 | jcunningham | Status | In Progress | Done | |
Percent Complete | 50% | 90% | |||
Open/Closed | Open | Closed | |||
2017-02-23 | jcunningham | Status | Done | In Progress | |
Percent Complete | 90% | 50% | |||
Open/Closed | Closed | Open | |||
2017-02-23 | goldsimon | Status | In Progress | Done | |
Percent Complete | 50% | 90% | |||
Open/Closed | Open | Closed | |||
2017-01-20 | jcunningham | Percent Complete | 0% | 50% | |
2017-01-20 | dcvmoole | Attached File | - | Added 0001-netif-ensure-that-netif-num-is-unique.patch, #39519 | |
2017-01-20 | jcunningham | Attached File | - | Added 0001-Task-14314-Add-interface-name-index-APIs.patch, #39516 | |
Attached File | - | Added 0001-Add-interface-support-to-shell-and-VS-project-file.patch, #39517 |
Powered by Savane 3.13-3230.
Corresponding source code
Great, thanks for wrapping up the remaining lose end!