lwIP - A Lightweight TCP/IP stack - Patches: patch #9523, MDNS responder should reply after...
You are not allowed to post comments on this tracker with your current authentication level.
patch #9523: MDNS responder should reply after a random timeout
Submitter: | David GIRAULT <dgirault> | ||
Submitted: | Tue 19 Dec 2017 01:39:44 PM UTC | ||
Category: | apps | Priority: | 5 - Normal |
Status: | Done | Privacy: | Public |
Assigned to: | None | Open/Closed: | Closed |
Planned Release: | None |
Jump to the original submission
Wed 14 Nov 2018 08:18:02 AM UTC, comment #28: |
Jasper Verschueren <jasperv> |
Tue 13 Nov 2018 01:59:35 PM UTC, comment #27: Hi Jasper,
|
Axel Lin <axellin> |
Tue 13 Nov 2018 11:24:48 AM UTC, comment #26: Pushed your patches. Thanks you very much for your contributions! |
Dirk Ziegelmeier <dziegel> |
Mon 12 Nov 2018 08:59:29 AM UTC, comment #25: I'm currently working on the probe tiebreaking and found that I made a small mistake with commit:
|
Jasper Verschueren <jasperv> |
Thu 01 Nov 2018 09:13:03 PM UTC, comment #24: netif_mdns_data() and get_mdns_pcb() should go to priv header. The function to get mdns pcb can go away if mdns_out.c would only contain the routines to build a packet and mdns.c contains the routine to send a packet.
|
Dirk Ziegelmeier <dziegel> |
Wed 31 Oct 2018 04:05:50 PM UTC, comment #23: Response to comment #21:
|
Jasper Verschueren <jasperv> |
Wed 31 Oct 2018 11:32:07 AM UTC, comment #22: Do you really need mdns_set_timeout()? AFAICS, you can always call sys_untimeout() even if the timeout is not active. |
Dirk Ziegelmeier <dziegel> |
Wed 31 Oct 2018 09:58:56 AM UTC, comment #21: Regarding your question about NETIF_TO_HOST: Only lwIP CORE functionality should be a direct type-safe netif member. This is because lwIP core should not know about apps, netifs etc., even if they are in main lwIP rep. This is why all apps that need to store something on a netif need to use the netif_client_data mechanism. |
Dirk Ziegelmeier <dziegel> |
Wed 31 Oct 2018 09:53:00 AM UTC, comment #20: No problem at all. I indeed thought you wanted only one patch :).
|
Jasper Verschueren <jasperv> |
Tue 30 Oct 2018 09:38:37 PM UTC, comment #19: I'm happy someone's working on this!
|
Simon Goldschmidt <goldsimon> |
Tue 30 Oct 2018 10:27:40 AM UTC, comment #18: The approach suggested below is not perfect. To implement the random delays we need to transform the code a bit further. With our mind on the other stuff that needs to be implemented.
|
Jasper Verschueren <jasperv> |
Tue 16 Jan 2018 08:46:58 PM UTC, comment #17: The only thing I have to add here currently is: please don't add cyclic timers below 100ms. There are targets where this might hurt. Especially in the 5ms range (in fact, the 100ms cyclic timers already seem too fast for doing near nothing).
|
Simon Goldschmidt <goldsimon> |
Tue 16 Jan 2018 07:09:05 PM UTC, comment #16: Even if many units happen to send at the same time, Ethernet takes care of that for you - either way it will be much better than now. I am happy to discuss such details once the rest of the code is in place.
|
Erik Ekman <yarrick> |
Tue 16 Jan 2018 04:24:20 PM UTC, comment #15: I'm not sure that assuming different devices will have unsynchronized periodic timer will be ok. If sys_now() implementation is base on gettimeofday() for example (well I know, this is a bad thing), and this one in keep sync using NTP, this result in periodic timer sync across all devices.
|
David GIRAULT <dgirault> |
Mon 08 Jan 2018 09:13:50 PM UTC, comment #14: As I understand it, there is no concept of order in MDNS.
|
Erik Ekman <yarrick> |
Mon 08 Jan 2018 01:30:32 PM UTC, comment #13: Douglas, regarding your comment #8, I think it should be randomized even for unicast response. If not, this won't resolve our issue when a device request (with unicast) for a service and multiple device responds at exactly same time (same HW, same code, so same reaction time). Most of the time, 1 or 2 of 4 devices aren't seen in our case.
|
David GIRAULT <dgirault> |
Mon 08 Jan 2018 11:56:58 AM UTC, comment #12: Erik, what's your status? Do you want to work on this or should someone else do it? |
Simon Goldschmidt <goldsimon> |
Mon 08 Jan 2018 10:15:30 AM UTC, comment #11: I think the approach is good, that is building the output pbuf and holding it for a short delay. I tried other options such as building the pbuf only when sending and it did not fit well, it had the problem that the response was not built atomically (the services might change), and for legacy queries the questions need to be added. If no one else is doing a major rewrite then landing this seems a move forward. |
Douglas <ourairquality> |
Mon 08 Jan 2018 09:35:30 AM UTC, comment #10: I've made a new version with a flag that allow to force synchronous send when mdns_send_outpacket() called from mdns_announce() or mdns_search(). See new patch. |
David GIRAULT <dgirault> |
Thu 04 Jan 2018 12:45:54 PM UTC, comment #9: Also should the responses still be sent in order. With only the random delay they might end up being sent out of order. |
Douglas <ourairquality> |
Wed 03 Jan 2018 12:10:42 PM UTC, comment #8: Perhaps the delay should only be applied for multicast, so the test in mdns_send_outpacket() would become:
|
Douglas <ourairquality> |
Wed 03 Jan 2018 09:29:50 AM UTC, comment #7:
|
Douglas <ourairquality> |
Wed 03 Jan 2018 08:20:12 AM UTC, comment #6: Note the patch also delays announcements with a random delay. This may not be a bad thing, if several devices of the same type power up at the same time, this avoids floodeding the net with announcements. |
Dirk Ziegelmeier <dziegel> |
Wed 03 Jan 2018 06:51:11 AM UTC, comment #5: OK, I reverted the patch. Erik, may I ask you to take over the current work on MDNS? You seem to be interested :-) |
Dirk Ziegelmeier <dziegel> |
Wed 03 Jan 2018 02:04:52 AM UTC, comment #4: It seems this broke the build.
|
Erik Ekman <yarrick> |
Tue 02 Jan 2018 09:21:07 PM UTC, comment #3: Applied, thanks! |
Dirk Ziegelmeier <dziegel> |
Sat 23 Dec 2017 07:17:52 PM UTC, comment #2: Ehrm, I haven't looked at the patch, nor do I know the mdns code too well, but RFC 6762, chapter 6.3 includes this:
|
Simon Goldschmidt <goldsimon> |
Fri 22 Dec 2017 10:08:28 AM UTC, comment #1: Thanks for the patch.
|
Erik Ekman <yarrick> |
Tue 19 Dec 2017 01:39:44 PM UTC, original submission:
According to chapter 6.3 in RFC 6762, the reply to a query that result in multiple answers must be randomly delayed from 20 to 120ms.
|
David GIRAULT <dgirault> |
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 11 latest changes.
Date | Changed by | Updated Field | Previous Value | => | Replaced by |
---|---|---|---|---|---|
2018-11-13 | dziegel | Status | None | Done | |
Open/Closed | Open | Closed | |||
2018-11-12 | jasperv | Attached File | - | Added 0001-Revert-mDNS-authority-section-of-probe-message-needs.patch, #45412 | |
2018-10-31 | jasperv | Attached File | - | Added 2018-10-31_mDNS_responder_delayed_refactored.zip, #45336 | |
2018-10-30 | jasperv | Attached File | - | Added 0001-mDNS-responder-upgraded-with-delay-functionality-cle.patch, #45329 | |
2018-01-08 | dgirault | Attached File | - | Added async_mdns_response.patch, #42869 | |
2018-01-03 | dziegel | Status | Done | None | |
Open/Closed | Closed | Open | |||
2018-01-02 | dziegel | Status | None | Done | |
Open/Closed | Open | Closed | |||
2017-12-19 | dgirault | Attached File | - | Added lwip_mdns_async_reply.patch, #42682 |
Powered by Savane 3.13-d3ae.
Corresponding source code
Hi Axel,
You will indeed find some commit logs in "one" commit because that commit originates from multiple commits. For example commit: "mDNS: add support for output delaying" is such a big change that during development I made several commits.
But as asked by Simon, I interactively rebased the commits so I can send fewer commits to you. The commit messages are kept for clarity on what the commit all changes.
I made the revert from my development environment, not from the rebased commit history, sorry :). I will take this in account next time.
Can you apply it?
If you can't, maybe it's easier if you just revert the commit.
Thanks for pushing!