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

IPv4 is sometimes empty in found service entries #27

Open
shangsunset opened this issue Aug 16, 2017 · 7 comments
Open

IPv4 is sometimes empty in found service entries #27

shangsunset opened this issue Aug 16, 2017 · 7 comments

Comments

@shangsunset
Copy link

Hi,

I have a question. With provided example code, if I start and stop client.go a couple times, IPv4 array would be empty in found service entry sometimes.

&{{service _foobar._tcp local. } router.home.local. 9000 [txtv=0 lo=1 la=2] 3200 [192.168.1.154] [fe80::ca2:1095:d9ab:c3e3]}

&{{service _foobar._tcp local. } router.home.local. 9000 [txtv=0 lo=1 la=2] 3200 [] [fe80::fe3a:efd5:f3dd:a9b6]}

Is this normal or a bug?

Thank you.

@jlitzingerdev
Copy link

I hit the same issue. The problem (in my case), was the entry with the IPV4 address comes in immediately after the IV6 one, but, since the entry was sent (and is correspondingly in sentEntries), it isn't sent again.

in client.mainloop, the line that prevents seeing the IPV4 entry is:

				if _, ok := sentEntries[k]; ok {
					continue
				}

@grandcat I'm more than willing to work up a patch for this, (this assumes it is something that you'd like to see fixed. FWIW avahi-browse will show both entries separately), but it would probably be best to have some idea of how you'd like to see this handled.

For example, one option is to delete the entry from sentEntries if it is updated. This would result in sending two entries. The IP association loop then becomes:

			for _, answer := range sections {
				switch rr := answer.(type) {
				case *dns.A:
					for k, e := range entries {
						if e.HostName == rr.Hdr.Name {
							entries[k].AddrIPv4 = append(entries[k].AddrIPv4, rr.A)
                                                        delete(sentEntries, k)
						}
					}
				case *dns.AAAA:
					for k, e := range entries {
						if e.HostName == rr.Hdr.Name {
							entries[k].AddrIPv6 = append(entries[k].AddrIPv6, rr.AAAA)
                                                        delete(sentEntries, k)
						}
					}
				}
		}

I'm not sure of a way to wait for both IPV4/6 that isn't brittle. At least if both entries are sent then the user can implement their own wait and merge strategy.

@grandcat
Copy link
Owner

I am aware of this problem. As it is time-critical and dependent on the environment, it might be appear or not.

The idea to tackle this in future would be as follows:
client.go would cache received service entries as usual. Instead of sending only a single update to the listening application (via Browse), this channel would also be used to announce an updated service entry, e.g. for an IPv4 entry that came in later. Like this, it is up to the application layer to wait until its needed fields are there.

As it is an API break, I would like to introduce a versioning/vendoring first, so people could stick to the old API.
Otherwise, I am open for suggestions.

@grandcat grandcat added this to ToDo in Future Releases Oct 29, 2017
@sdumetz
Copy link

sdumetz commented Jan 24, 2018

FWIW, it's currently possible to do such a wait and combine using zeroconf.SelectIPTraffic(iptype ) in parallel with IPv6 and IPv4 and merging the lists as they come.

I think that it's a surprising behavior from a end-user point of view.

Wouldn't introducing a new method to return a slice of ServiceEntry after a timeout be a better way, hiding the merge operation from users? It's what I (most users?) work with in the end anyway.

@grandcat
Copy link
Owner

grandcat commented Jan 25, 2018

Nice workaround :)
I am aware of the problem, but returning after a timeout is no reliable solution either. Depending on the network, delays are quite different.

Instead, ServiceEntrys should be updateable in the sense that any updates coming in though mDNS, should be pushed to the client by keeping a ServiceEntry internally and fill in the new values.
An additional property should state whether an entry was updated.

@vtolstov
Copy link

vtolstov commented Sep 3, 2018

does anybody plan to fix this?

@andig
Copy link

andig commented Jun 6, 2022

We‘re seeing the same problem in evcc-io/evcc#1217 (reply in thread)

@andig
Copy link

andig commented Jun 7, 2022

What is required to reproduce this issue? I'm using examples/register and examples/resolv. Even when I delay the resolv's ip4 query by ~100ms, I always receive the entire response including all IPs/services as advertised by register.

Is it correct to assume, that this issue will only trigger if there are two separate services announced, one for A and one for AAAA?

cpuchip pushed a commit to cpuchip/zeroconf that referenced this issue Jul 27, 2022
)

* Fix compatibility with avahi 0.8-rc1

This fixes browse, lookup and also register not working properly with devices running with avahi 0.8-rc1

* Fix linter warning

* Also set IPv6 HopLimit
cpuchip pushed a commit to cpuchip/zeroconf that referenced this issue Jul 27, 2022
)

* Fix compatibility with avahi 0.8-rc1

This fixes browse, lookup and also register not working properly with devices running with avahi 0.8-rc1

* Fix linter warning

* Also set IPv6 HopLimit
ebiyu added a commit to ebiyu/zeroconf that referenced this issue Sep 30, 2023
In some case, only IPv4 or IPv6 is returned.
For the usecase which requre IPv4 address, made possible to specify
stratedy.
This may resolve grandcat#27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

6 participants