Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mdns] update mDNSResponder to 1790.80.10 #1919

Merged
merged 2 commits into from
Aug 24, 2023

Conversation

agners
Copy link
Contributor

@agners agners commented Jun 21, 2023

No description provided.

@agners agners force-pushed the update-mdnsresponder-to-1790.80.10 branch 4 times, most recently from 160e814 to a0d814b Compare June 21, 2023 12:13
@agners
Copy link
Contributor Author

agners commented Jun 22, 2023

It seems that some tests fail despite not using mDNSResponder. The reason is that the tests use mDNSResponder (mdnsd and dns-sd CLI utility) to find services. It seems that this new version causes problems at times, e.g. it seems to discover the OTBR twice (e.g. from the Border Router TREL (Avahi) test):

 ======================================================================
FAIL: test (__main__.PublishMeshCopService)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/ot-br-posix/ot-br-posix/third_party/openthread/repo/tests/scripts/thread-cert/border_router/test_publish_meshcop_service.py", line 117, in test
    self.assertEqual(len(service_instances), 2)
AssertionError: 4 != 2

It could be just timing, or maybe it is really a bug in that new mDNSResponder version.

I am currently not setup to run these tests/didn't investigate on how to run them. I probably can look into these issues at one point, but it doesn't seem to be a quick/easy thing from what I can tell.

If someone else has a good idea I am happy to hear it.

@agners agners force-pushed the update-mdnsresponder-to-1790.80.10 branch 7 times, most recently from 7b7211f to 0280fb1 Compare June 26, 2023 15:10
@agners
Copy link
Contributor Author

agners commented Jun 26, 2023

With the patches from apple-oss-distributions/mDNSResponder#2 and apple-oss-distributions/mDNSResponder#6 the tests seem to run through nicely (the first time there were some errors, but they were unrelated. With this second run it seems everything is green now.

I'll update the branch again to trigger another run, if that returns green again I think it shows some certainty that this the update is solid.

@agners agners force-pushed the update-mdnsresponder-to-1790.80.10 branch from eace363 to 4c442c7 Compare August 14, 2023 09:09
@agners
Copy link
Contributor Author

agners commented Aug 23, 2023

Any thoughts on this PR? It does address issue we have seen where mdnsd suddenly used 100% CPU. We haven't observed that issue with this version so far.

Copy link
Contributor

@superwhd superwhd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thanks!

Copy link
Member

@wgtdkp wgtdkp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Copy link
Member

@jwhui jwhui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! 👍🏼

@jwhui jwhui merged commit 9e50efa into openthread:main Aug 24, 2023
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants