Avoid redundant query, announcement and unregistration overhaul #239

hrzlgnm · 2024-08-12T22:44:27Z

I had a second thought about the "already sent query or announcemnet in the subnet approach".

Using a "subnet" to filter already notified multicast groups on IPv4 or IPv6 is not a effective method in a multicast context.

The socket used to send multicast traffic is acutally bound to the "any" address associated with the respective address family and is joined on the mDNS multicast_v(4|6) address using the network interface index.

If we have multiple IPv6 or IPv4 addresses on the same network interface in different subnets, this will result in redundant traffic to be sent when sending queries, unregistration messages or announcements, as we still sent those to the same multicast group on the same interface.

I've came up with another approach. We use a tuple(interface index , adress family ) in a set to track already "used" notifications.

This reduces the number of messages sent drastically for example on my windows machine, i'm getting 3 IPv6 addresses and one IPv4 address. Before this change this would result in 4 messages being send on that interface. With this change we only send 2.

@keepsimple1 Please, let me know if there are any problems with the approach.

keepsimple1 · 2024-08-13T04:05:03Z

It's a good question, and I need to dig more as well. Here is my thoughts so far:

The socket used to send multicast traffic is acutally bound to the "any" address associated with the respective address family and is joined on the mDNS multicast_v(4|6) address using the network interface index.

The "bound" part is true, although for "join" it's not using the network interface index for IPv4. It's using the IP address. IPv6 seems different.

If we have multiple IPv6 or IPv4 addresses on the same network interface in different subnets, this will result in redundant traffic to be sent when sending queries, unregistration messages or announcements, as we still sent those to the same multicast group on the same interface.

That's true. However the source address will be different. Suppose we only send out one message for both subnet A and subnet B from a source address in subnet A, the receiver on subnet B would see it with the source address from a different subnet, which I'm not sure if its valid, or possible at all. I don't think the mdns message is routable.

I've came up with another approach. We use a tuple(interface index , adress family ) in a set to track already "used" notifications.

This reduces the number of messages sent drastically for example on my windows machine, i'm getting 3 IPv6 addresses and one IPv4 address. Before this change this would result in 4 messages being send on that interface. With this change we only send 2.

Do you mind paste the ipconfig or ifconfig or equivalent thing for these interfaces / addresses?

hrzlgnm · 2024-08-13T06:52:22Z

It's a good question, and I need to dig more as well. Here is my thoughts so far:

The socket used to send multicast traffic is acutally bound to the "any" address associated with the respective address family and is joined on the mDNS multicast_v(4|6) address using the network interface index.

The "bound" part is true, although for "join" it's not using the network interface index for IPv4. It's using the IP address. IPv6 seems different.

Yes your are right about IPv4, we are using the IP Address to join. But this has the same effect as binding to the interface index from my understanding.

If we have multiple IPv6 or IPv4 addresses on the same network interface in different subnets, this will result in redundant traffic to be sent when sending queries, unregistration messages or announcements, as we still sent those to the same multicast group on the same interface.

That's true. However the source address will be different. Suppose we only send out one message for both subnet A and subnet B from a source address in subnet A, the receiver on subnet B would see it with the source address from a different subnet, which I'm not sure if its valid, or possible at all. I don't think the mdns message is routable.

I don't think the subnet matters at all, when receiving message. No-one will look at the senders address and do something with it in regards of mDNS. And yes, according to the standard, the IPv4 mDNS Multicast address is not routable. Source. IPv6 is a different matter.

I've came up with another approach. We use a tuple(interface index , adress family ) in a set to track already "used" notifications.
This reduces the number of messages sent drastically for example on my windows machine, i'm getting 3 IPv6 addresses and one IPv4 address. Before this change this would result in 4 messages being send on that interface. With this change we only send 2.

Do you mind paste the ipconfig or ifconfig or equivalent thing for these interfaces / addresses?

ip addr show from my Linux machine, where I'm using a bridge with multicast setup, to be able to do networking stuff from virtual machines:

5: mn-bridge1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
    link/ether 22:06:d3:db:c1:02 brd ff:ff:ff:ff:ff:ff
    inet 192.168.42.92/24 brd 192.168.42.255 scope global noprefixroute mn-bridge1
       valid_lft forever preferred_lft forever
    inet 192.168.43.92/24 brd 192.168.43.255 scope global noprefixroute mn-bridge1
       valid_lft forever preferred_lft forever
    inet 192.168.44.92/24 brd 192.168.44.255 scope global noprefixroute mn-bridge1
       valid_lft forever preferred_lft forever
    inet 192.168.70.92/24 brd 192.168.70.255 scope global noprefixroute mn-bridge1
       valid_lft forever preferred_lft forever
    inet6 fe80::4aa5:3359:1346:2865/64 scope link noprefixroute 
       valid_lft forever preferred_lft forever

In this case we would send 5 messages instead of just 2.

Avahi for example randomly uses one of those IPv4 addresses and happily sends queries for example using 192.168.70.92 and is albe to discover services of the other subnets without issues.

ipconfig output from the windows machine, sorry about the German:

Windows-IP-Konfiguration

Ethernet-Adapter Ethernet:

IPv6-Adresse. . . . . . . . . . . : 2003:e8:bf4e:c900:860d:98d5:d90b:7d 
Temporäre IPv6-Adresse. . . . . . : 2003:e8:bf4e:c900:9532:fd36:933f:ea3d 
Verbindungslokale IPv6-Adresse  . : fe80::f900:996d:50d1:8349%5 
IPv4-Adresse  . . . . . . . . . . : 192.168.178.25 
Subnetzmaske  . . . . . . . . . . : 255.255.255.0

In this case we would send 4 messages instead of 2 as mentioned above.

I'm not having any apple products, therefore I can't say anything about macOS. I guess it behaves like Linux.

I also can provide network traces as evidence if needed.

keepsimple1 · 2024-08-14T05:39:39Z

I don't think the subnet matters at all, when receiving message. No-one will look at the senders address and do something with it in regards of mDNS.

In most cases, I agree it probably doesn't matter. But what if we need to support Unicast Responses in future?

And in Known-Answers multi-packet, we also need to know about the source address although we cannot support it now.

Of course, one option is to ignore such incoming message if the source is in a different subnet not in the current interface.

(In general I felt your approach is nice, just wanted to understand all the impacts)

hrzlgnm · 2024-08-14T13:50:38Z

I don't think the subnet matters at all, when receiving message. No-one will look at the senders address and do something with it in regards of mDNS.

In most cases, I agree it probably doesn't matter. But what if we need to support Unicast Responses in future?

And in Known-Answers multi-packet, we also need to know about the source address although we cannot support it now.

Of course, one option is to ignore such incoming message if the source is in a different subnet not in the current interface.

(In general I felt your approach is nice, just wanted to understand all the impacts)

Ah ok, I was not aware of those two future use cases. But in general, I think my approach does not put any obstacles in the way of these two points, as its main intent is to reduce the outgoing multicast traffic.

…gration_success test

keepsimple1

Overall looks good. A couple of comments inline.

src/service_daemon.rs

keepsimple1 · 2024-08-15T04:02:02Z

Ah ok, I was not aware of those two future use cases. But in general, I think my approach does not put any obstacles in the way of these two points, as its main intent is to reduce the outgoing multicast traffic.

I think so too. Given that currently we don't specify / limit the source address when browsing services, I think it's worth to go with this optimization.

I submitted the review. And thanks for providing the details about the IP interface and addresses info previously.

src/service_daemon.rs

keepsimple1

Thanks for the update! A few minor comments inline.

keepsimple1 · 2024-08-16T04:01:11Z

src/service_daemon.rs

+
+/// A struct to track multicast notification status for a network interface.
+#[derive(Debug, Eq, Hash, PartialEq)]
+struct MulticastNotificationTracker {


I think IfIndexTracker (or MulticastSendTracker) would be easier to understand / map to what it does. It's up to you to decide though.

Went with MulticastSendTracker, thx for the suggestions.

src/service_daemon.rs

keepsimple1

LGTM, thanks!

This integratest the following two changes: - keepsimple1/mdns-sd#239 - keepsimple1/mdns-sd#240

hrzlgnm added 3 commits August 13, 2024 00:04

Experiment with suppressions

04bb623

remove counters

88ebebb

Cargo fmt

1778db1

hrzlgnm changed the title ~~Redundant query, announcement and unregistration overhaul~~ Avoid redundant query, announcement and unregistration overhaul Aug 12, 2024

hrzlgnm added 7 commits August 14, 2024 19:22

Deduplicate interface index and ip version retrieval and clarify inte…

5ef1149

…gration_success test

clarify integration_success test

e3537fe

clarify integration_success test

dc38e0b

Cargo fmt

a540e57

Revert unintended change

b961e03

Remove accidentally added test batch file

4825f37

Remove notion of address family, as address family is ambiguous.

910d2c1

keepsimple1 reviewed Aug 15, 2024

View reviewed changes

src/service_daemon.rs Outdated Show resolved Hide resolved

src/service_daemon.rs Outdated Show resolved Hide resolved

src/service_daemon.rs Show resolved Hide resolved

src/service_daemon.rs Show resolved Hide resolved

src/service_daemon.rs Outdated Show resolved Hide resolved

hrzlgnm added 3 commits August 15, 2024 19:35

Changes after review

7df87ab

Cargo fmt

e2eb8f1

Minor naming cleanup and use Option instead of is_valid...

b25fcc8

hrzlgnm requested a review from keepsimple1 August 15, 2024 17:49

hrzlgnm commented Aug 15, 2024

View reviewed changes

src/service_daemon.rs Outdated Show resolved Hide resolved

Revert unintended change

2cad3c2

keepsimple1 reviewed Aug 16, 2024

View reviewed changes

Chages after review 2.

1dfd963

hrzlgnm requested a review from keepsimple1 August 16, 2024 06:26

Cargo fmt

4130136

keepsimple1 approved these changes Aug 16, 2024

View reviewed changes

keepsimple1 merged commit 0453030 into keepsimple1:main Aug 16, 2024
3 checks passed

hrzlgnm deleted the redundant-query-announcement-overhaul branch August 17, 2024 06:41

hrzlgnm added a commit to hrzlgnm/mdns-browser that referenced this pull request Aug 18, 2024

feat: integrate latest changes of dependency mdns-sd

89fcd17

This integratest the following two changes: - keepsimple1/mdns-sd#239 - keepsimple1/mdns-sd#240

hrzlgnm mentioned this pull request Aug 18, 2024

feat: integrate latest changes of dependency mdns-sd hrzlgnm/mdns-browser#226

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Avoid redundant query, announcement and unregistration overhaul #239

Avoid redundant query, announcement and unregistration overhaul #239

hrzlgnm commented Aug 12, 2024 •

edited

Loading

keepsimple1 commented Aug 13, 2024

hrzlgnm commented Aug 13, 2024 •

edited

Loading

keepsimple1 commented Aug 14, 2024

hrzlgnm commented Aug 14, 2024

keepsimple1 left a comment

keepsimple1 commented Aug 15, 2024

keepsimple1 left a comment

keepsimple1 Aug 16, 2024

hrzlgnm Aug 16, 2024

keepsimple1 left a comment

Avoid redundant query, announcement and unregistration overhaul #239

Avoid redundant query, announcement and unregistration overhaul #239

Conversation

hrzlgnm commented Aug 12, 2024 • edited Loading

keepsimple1 commented Aug 13, 2024

hrzlgnm commented Aug 13, 2024 • edited Loading

keepsimple1 commented Aug 14, 2024

hrzlgnm commented Aug 14, 2024

keepsimple1 left a comment

Choose a reason for hiding this comment

keepsimple1 commented Aug 15, 2024

keepsimple1 left a comment

Choose a reason for hiding this comment

keepsimple1 Aug 16, 2024

Choose a reason for hiding this comment

hrzlgnm Aug 16, 2024

Choose a reason for hiding this comment

keepsimple1 left a comment

Choose a reason for hiding this comment

hrzlgnm commented Aug 12, 2024 •

edited

Loading

hrzlgnm commented Aug 13, 2024 •

edited

Loading