Consider lifetimes of addresses in kernel before advertising prefixes in Router Advertisments

[samboyles1]

The code that works to add prefix information to outgoing RA messages contained a TODO message for the processing of valid and preferred lifetimes.

Though it is not explicitly stated in an RFC, the prefix valid and preferred lifetime values advertised in an RA should be the lower of:

• the valid and preferred lifetimes of the prefix configured in radvd.conf
• the valid and preferred lifetimes of any address that match that prefix on the outgoing interface (i.e an address statically assigned to the interface with non default lifetimes)

This is to ensure that prefixes are not advertised to clients with lifetimes that may expire before they are expected to (for example in the case of an upstream configuration change) leading to a possible loss of communication.

A new function has been added to netlink.c which does an RTM_GETADDR, and fetches the ifa_cacheinfo of addresses on an interface, before comparing these with any (if there are any) addresses that match the prefix on the interface.

[robbat2]

LGTM, but needs testing

[samboyles1]

How would you like this tested? Creating new test cases?

[Neustradamus]

@fgont: What do you think?

[Neustradamus]

@reubenhwk: Can you look?

[reubenhwk]

I don't have time to maintain radvd anymore. Somebody else will have to take over completely. Whoever wants it let me know.

On Wed, Dec 30, 2020 at 5:13 PM Neustradamus notifications@github.com wrote:

@reubenhwk https://github.com/reubenhwk: Can you look?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/reubenhwk/radvd/pull/127#issuecomment-752808358, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABRG63ANPXXEGG26NM6HOTSXPF4FANCNFSM4SXJSQWQ .

[reubenhwk]

Robin do you want to take over RADVD? I'm ready to retire.

On Sat, Jan 9, 2021 at 12:02 AM Reuben Hawkins reubenhwk@gmail.com wrote:

I don't have time to maintain radvd anymore. Somebody else will have to take over completely. Whoever wants it let me know.

On Wed, Dec 30, 2020 at 5:13 PM Neustradamus notifications@github.com wrote:

@reubenhwk https://github.com/reubenhwk: Can you look?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/reubenhwk/radvd/pull/127#issuecomment-752808358, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABRG63ANPXXEGG26NM6HOTSXPF4FANCNFSM4SXJSQWQ .

[robbat2]

This PR is still valuable and addresses multiple reported issues in the past, but we need to figure out adding test cases for it to consistently validate it.

[robbat2]

@samboyles1 are you still interested in this? Could you help to write a test matrix for it at least?

[stappersg]

And then there was merge request #151, which is merged.

@samboyles1 has git HEAD now what you intented with this MR?

[stappersg]

@samboyles1 does the merge of #151 solve what you wanted to should with this merge request?

Yes consider this update as a retransmit of https://github.com/radvd-project/radvd/pull/127#issuecomment-812337784

Here at transmitting end is hope for recieving messages like:

• Yes, lifetimes are since #151 fine. I closing the MR that I opened.
• Seen the request for feedback. My planning is to do verification in the week of monday 2021-04-12 and report back in the same week.
• Nope, lifetimes are not good enough for me. I'm closing this merge request because I have prepared meanwhile a new one.
• Frankly speaking: I avoid GH as much as possible. My time-out-communication-technique will also close my merge request, hopefully.

[samboyles1]

Hi @stappersg, I apologise for being MIA, I've had a lot on my plate recently. I will take a look at #151 now, although I have my doubts that it achieves what I was wanting to. I've noted your review comment above @robbat2 and will look to use a helper function.

With regards to testing, it is a difficult one given that my patch relies on retrieving address lifetimes from the kernel and the check testing framework doesn't look like it allows for mocking of functions.

I have a few thoughts on how to go about it:

• During the test setup create a temporary interface which we can then use to add addresses to and retrieve lifetimes from without messing with the other interfaces on the host machine, and possibly leave the test off by default.
• Use an #ifdef in the get_prefix_lifetimes function so that during testing we don't attempt to read values from the kernel and instead provide a range of different values to test the logic behind add_ra_options_prefix() is working as intended.
• Attempt to find another test framework that can allow us to mock the netlink function calls (a quick search shows cmocka might be able to do this?)

I am not sold on any of the options but I feel like the second one might be the safest way to go about it, however if you have any other suggestions I am open to them.

[stappersg]

:-)

Thanks for the response.

It is overwhelming.

:-/

[robbat2]

• Can you rebase your changes on top of master, so it doesn't have the extra commits here?
• Find a good way to mock netlink messages would go a long way to testing radvd in general, the other thought I was spinning up tiny VMs for testing

[mcr]

• Find a good way to mock netlink messages would go a long way to testing radvd in general, the other thought I was spinning up tiny VMs for testing

I've thought hard about this in various projects.... see #153

[samboyles1]

• Find a good way to mock netlink messages would go a long way to testing radvd in general, the other thought I was spinning up tiny VMs for testing

I've thought hard about this in various projects.... see #153

@mcr you mention in #153 that you already have some code changes for helping with this, are you planning on creating a PR with those changes?