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

implement IntoIterator for SliceCache #11

Closed
wants to merge 5 commits into from

Conversation

little-dude
Copy link
Contributor

I'd like to have the ability to dump the arp cache. One way to do this is to iterate over the cache, which is what this PR implement. If you are ok with this change, I'd like to add an iter_arp_cache method on EthernetInterface.

While implementing this, I noticed that the ARP cache could have entries with non unicast ip addresses (like 0.0.0.0, 0.0.0.1 or 255.255.255.255). Is this intentional? Shouldn't the insert fail when trying to insert such entries? If that's intentional, I'll remove 26b78ae and make SliceCacheIterator work for these entries. Otherwise, I can work on a fix.

I also noticed that the lru counter was not incremented on insert. I took the liberty to add this but I can revert if that was intentional.

@whitequark
Copy link
Contributor

First, I've cherry-picked fabcbd9 and 380bfd5, thanks.

Regarding IntoIterator, I'm not sure I see the point over implementing that trait instead of an .iter() method. I don't think ArpCache is a collection in a traditional sense.

Regarding non-unicast addresses in the ARP cache, yes, that's troublesome. I think it warrants an assert in the ARP cache, and a branch in EthernetInterface.

Regarding the iterator itself, sure, that's reasonable. But you'll encounter a problem, namely that you don't have the concrete SliceArpCache type in EthernetInterface. I think you'll have to make the iterator an associated type of ArpCache trait and SliceArpCache.

@whitequark
Copy link
Contributor

See also 4e364d8.

@whitequark
Copy link
Contributor

See also 15cf0cc.

@whitequark
Copy link
Contributor

I'm going to closing this PR because it seems like everything except the actual ARP cache iteration is already done, and iteration itself is something I would like to do quite differently. Feel free to open an issue requesting that.

@whitequark whitequark closed this Sep 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants