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.

Sign up for GitHub

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

Jump to bottom

More mDNSPosix Fixes #6

Closed
wants to merge 10 commits into from
Closed

More mDNSPosix Fixes #6

wants to merge 10 commits into from

Conversation

agners
Copy link

@agners agners commented Jun 23, 2023

This builds on #2, but contains an additional fix to address a SIGSEGV.

nkarstens and others added 10 commits March 23, 2023 12:11
@nkarstens
Fix Linux build
e136dcd 
The __block qualifier is not used in Linux builds.

Signed-off-by: Nate Karstens <nate.karstens@garmin.com>
@nkarstens
Create subroutine for cleaning recent interfaces
4f7970a 
Moves functionality for cleaning the list of recent
interfaces into its own subroutine.

Upstream-Status: Submitted [dts@apple.com]

Signed-off-by: Nate Karstens <nate.karstens@garmin.com>
Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
@nkarstens
Create subroutine for tearing down an interface
f7ab91f 
Creates a subroutine for tearing down an interface.

Upstream-Status: Submitted [dts@apple.com]

Signed-off-by: Nate Karstens <nate.karstens@garmin.com>
Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
@nkarstens
Track interface socket family
542c1b2 
Tracks the socket family associated with the interface.

Upstream-Status: Submitted [dts@apple.com]

Signed-off-by: Nate Karstens <nate.karstens@garmin.com>
Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
@nkarstens
Indicate loopback interface to mDNS core
4438577 
Tells the mDNS core if an interface is a loopback interface,
similar to AddInterfaceToList() in the MacOS implementation.

Upstream-Status: Submitted [dts@apple.com]

Signed-off-by: Nate Karstens <nate.karstens@garmin.com>
Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
@nkarstens
Use list for changed interfaces
2a0f873 
Uses a linked list to store the index of changed network interfaces
instead of a bitfield. This allows for network interfaces with an
index greater than 31 (an index of 36 was seen on Android).

Upstream-Status: Submitted [dts@apple.com]

Signed-off-by: Nate Karstens <nate.karstens@garmin.com>
Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
Change-Id: Ibeab0ec68ca0d21da8384d4362e59afd2951f138
@nkarstens
Handle noisy netlink sockets
00289e8 
The POSIX implementation currently clears all network interfaces
when netlink indicates that there has been a change. This causes
the following problems:

  1) Applications are informed that all of the services they are
     tracking have been removed.
  2) Increases network load because the client must re-query for
     all records it is interested in.

This changes netlink notification handling by:

  1) Always comparing with the latest interface list returned
     by the OS.
  2) Confirming that the interface has been changed in a way
     that we care about.

Upstream-Status: Submitted [dts@apple.com]

Signed-off-by: Nate Karstens <nate.karstens@garmin.com>
Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
@nkarstens
Mark deleted interfaces as being changed
8ebfeaf 
Netlink notification handling ignores messages for deleted links,
RTM_DELLINK. It does handle RTM_GETLINK. According to libnl docu-
mentation (http://www.infradead.org/~tgr/libnl/doc/route.html)
RTM_DELLINK can be sent by the kernel, but RTM_GETLINK cannot.
There was likely a mixup in the original implementation, so this
change replaces handling for RTM_GETLINK with RTM_DELLINK.

Testing and Verification Instructions:
  1. Use ip-link to add and remove a VLAN interface and verify
     that mDNSResponder handles the deleted link.

Upstream-Status: Submitted [dts@apple.com]

Signed-off-by: Nate Karstens <nate.karstens@garmin.com>
Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
@nkarstens
Handle errors from socket calls
dae89c4 
Adds handling for socket() or read() returning a
negative value (indicating an error has occurred).

Upstream-Status: Submitted [dts@apple.com]

Signed-off-by: Nate Karstens <nate.karstens@garmin.com>
Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
@agners
Handle interface without ifa_addr
e501d58 
It seems that certain interface types may have `ifa_addr` set to null.
Handle this case gracefully.

Signed-off-by: Stefan Agner <stefan@agner.ch>
@agners agners mentioned this pull request Jun 23, 2023
Open
@agners agners mentioned this pull request Jun 26, 2023
Merged
@nkarstens
Copy link

@agners Thanks for finding that. To make it easier for the maintainers, would you mind instead if we bring that into #2 and close this PR? Three ways I can think of doing that:

  1. I cherry-pick your change
  2. You create a PR on my fork
  3. I amend 00289e8 and add "Co-authored-by" and "Signed-off-by" tags for you.

What is your preference?

@agners
Copy link
Author

agners commented Aug 3, 2023

@nkarstens thanks for your work!

What is your preference?

Either way works for me, I think the simplest would be if you just cherry-pick my change (so 1).

I've noticed that the latest version (1807.101.2) doesn't seem to have a working POSIX implementation at all? 😕

$ cd mDNSPosix
$ make os=linux
make: *** No rule to make target 'objects/prod/PosixDaemon.c.o', needed by 'build/prod/mdnsd'.  Stop.

@akiernan
Copy link

akiernan commented Aug 3, 2023

I've noticed that the latest version (1807.101.2) doesn't seem to have a working POSIX implementation at all? 😕

$ cd mDNSPosix
$ make os=linux
make: *** No rule to make target 'objects/prod/PosixDaemon.c.o', needed by 'build/prod/mdnsd'.  Stop.

It pretty much has no source at all...

~/mDNSResponder$ find . -name '*.c' | wc -l
0

@nkarstens
Copy link

Thanks, @agners, I cherry-picked your change into #2. You may consider closing this now.

I also noticed that most of the source files have disappeared in their latest push. I suspect that was an oversight but will try to follow up.

@dhgutteridge
Copy link

Thanks, @agners, I cherry-picked your change into #2. You may consider closing this now.

I also noticed that most of the source files have disappeared in their latest push. I suspect that was an oversight but will try to follow up.

Yes, it appears to be a merge botch where much of the project is lost, so there'd be nothing relevant left to apply changes to. There doesn't seem to be any means of communicating this I can see other than via PRs like this one, since I can find no bug reporting contact details, nor is issue reporting enabled for this project. (This is relevant in my case as this is the upstream source for integration into NetBSD, which we're, well, lagging behind on.)

@agners
Copy link
Author

agners commented Aug 4, 2023

Close in favor of #2.

@agners agners closed this Aug 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Goldindollar Goldindollar Goldindollar approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@agners @nkarstens @akiernan @dhgutteridge @Goldindollar