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

ARP table sensor #76

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

mdrichardson
Copy link

@mdrichardson mdrichardson commented Apr 10, 2023

Related: #73

This is an attempt to get a sensor listing all of the IPs in the ARP table.

@travisghansen, I've tried a lot of different variations of dict_get(state, "arp_table") and they always return None. Via some error logging through _LOGGER.error(...), I can see that the data is retrieved and set successfully here, but if I print out the whole state object in my added native_value() method, it never has an "arp_table" property. Any idea what's going on?

@mdrichardson mdrichardson changed the title getArp ARP table sensor Apr 10, 2023
@mdrichardson mdrichardson marked this pull request as draft April 10, 2023 22:41
@travisghansen
Copy link
Owner

Great!

The integration uses 2 separate data coordinator instances, one for the device tracker and another for everything else. The reason you never see the value is because the coordinator you’re using is not for device tracker. This is primarily for performance reasons and being able to set different intervals. For testing just copy the try/catch into the ‘else’ block below.

Fetching the arp data probably needs to be ‘backgrounded’ to prevent it from holding everything else up. I can do that later though and not really something you need to worry about.

I am not sure if an entity can return an array value like that. Give it a try and let me know how it goes!

@mdrichardson mdrichardson marked this pull request as ready for review April 11, 2023 02:21
@mdrichardson
Copy link
Author

How's this? Been awhile since I did much in Python, so I'm real open to changing things

@travisghansen
Copy link
Owner

How well is it working in your setup?

There are some improvements to be made to make it more efficient but I can clean those up. I’ll need to rework the coordinator stuff as well.

The subnet logic doesn’t seem valid to me (just reading the code). It looks like you’re taking the first octet and just using that as the subnet?

@mdrichardson
Copy link
Author

How well is it working in your setup?
The subnet logic doesn’t seem valid to me (just reading the code). It looks like you’re taking the first octet and just using that as the subnet?

It's working great!

image
image

I'm totally fine dropping the subnet part. My thought was that it might make it a little easier to build a template sensor for my original use case, but it really wouldn't change it much.

@travisghansen
Copy link
Owner

I see, you were assuming a /24 subnet basically. Regarding the IPs we probably need to structure the data such that it includes the actual mac address etc and not just a list of IPs. Not exactly sure the best way to approach that :(

@mdrichardson
Copy link
Author

mdrichardson commented Apr 11, 2023

I see, you were assuming a /24 subnet basically

Yeah. I didn't see a great way to add separate subnets without also adding a bunch of user config options. Maybe the better approach is to have the additional attributes just be a list of tuples containing the ip and mac addresses:

clients: [
  (<ip-address1>, <mac-address1>),
  (<ip-address2>, <mac-address2>),
]

@travisghansen Thoughts?

@travisghansen
Copy link
Owner

Not sure honestly how the data for an arp table is stored. At the end of the day you’re looking for a mac not an ip right? Technically a single mac could have more than 1 ip but I think generally the value of this feature is more focused on macs vs IPs (for examples, for ip there are ping sensors etc that already exist).

@mdrichardson
Copy link
Author

mdrichardson commented Apr 15, 2023 via email

@travisghansen
Copy link
Owner

I think there was some discussion about getting dhcp stats on a per-interface basis. Is that an easier route?

@alexdelprete
Copy link
Contributor

Technically a single mac could have more than 1 ip

In some cases you can also have a single IP for multiple MACs.

@travisghansen
Copy link
Owner

travisghansen commented Apr 15, 2023

What (valid) scenario would that happen?

@alexdelprete
Copy link
Contributor

alexdelprete commented Apr 15, 2023

What (valid) scenario would that happen?

Devices with multiple cards (wifi + ethernet, or double NICs not pooled).

@travisghansen
Copy link
Owner

Each device in such cases has their own mac. I don’t think there’s a valid scenario where a single IP has multiple macs.

@alexdelprete
Copy link
Contributor

Each device in such cases has their own mac. I don’t think there’s a valid scenario where a single IP has multiple macs.

There is: my media players have eth + wifi, and I want my DHCP server to assign the same address whatever interface the device is using.

Same goes for my network printer, it has eth+wifi, and I have the same IP for both interfaces.

@alexdelprete
Copy link
Contributor

alexdelprete commented Apr 15, 2023

image

This is a dnsmasq DHCP config for multiple MACs for 1 reserved IP:

config 'host'
    option 'name' 'printer'
    option 'mac' '99:88:77:66:55:44 ff:ee:dd:cc:bb:aa'
    option 'ip' '192.168.1.104'

@travisghansen
Copy link
Owner

Purposely using arp poisoning is not valid :) Even in failover scenarios (ie: carp) the mac isn’t shared but rather is advertised by the active device. The shared screenshot makes my point. You shouldn’t have both wifi and eth enabled simultaneously…and even if you do the arp table of any peer will only respect 1 at any given time.

@alexdelprete
Copy link
Contributor

alexdelprete commented Apr 15, 2023

There's no arp poisoning, only 1 interface of the two is active. :)

@travisghansen
Copy link
Owner

Then we’re back where we started, there’s only 1 mac per IP :)

@alexdelprete
Copy link
Contributor

alexdelprete commented Apr 15, 2023

Then we’re back where we started, there’s only 1 mac per IP :)

Active on the network, yes, not on the DHCP server though. :)

@travisghansen
Copy link
Owner

Even then, the lease is ‘abandoned’ so only 1 lease :)

@alexdelprete
Copy link
Contributor

Even then, the lease is ‘abandoned’ so only 1 lease :)

I don't know if at low level the IP is released and re-registered, maybe it's not necessary, the DHCP could simply refresh the ARP table with the new MAC. I didn't sniff it...:)

I added this use-case if maybe there was the idea to scrape DHCP config to get MACs, just be aware that there could be multiple MACs for the same reserved IP.

@travisghansen
Copy link
Owner

As far as the arp table is concerned (on the firewall and any peer) no. As far as dhcp leases no as well.

It is a good point about scraping the config. In practice the dhcp details in the api don’t come from the config but rather the state/leases file.

Each peer may have stale data (ie: arp is pointing to ‘old’ mac address), but peers generally clean this up pretty fast if the arp is no longer valid.

@alexdelprete
Copy link
Contributor

ARP tables are refreshed through gratuitous ARP when the interface comes up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants