Skip to content

Conversation

@ColinMcInnes
Copy link
Contributor

Sends route lifetime to NETLINK RTA_EXPIRES

Partially addresses #428

Still need to send hoplimit

@rsmarples
Copy link
Member

This looks nice! Can we also extract the lifetime in if_copyrt in if-linux.c please?

@ColinMcInnes
Copy link
Contributor Author

This looks nice! Can we also extract the lifetime in if_copyrt in if-linux.c please?

Yes, I can do that.

@ColinMcInnes ColinMcInnes marked this pull request as draft January 9, 2025 14:54
@ColinMcInnes ColinMcInnes marked this pull request as ready for review January 9, 2025 15:07
@ColinMcInnes ColinMcInnes requested a review from rsmarples January 9, 2025 15:50
Copy link
Member

@rsmarples rsmarples left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gah, and I forgot one last thing.

Before setting each rt_expires, check that the lifetime (before working out elapsed) is not ND6_INFINITE_LIFETIME.

@ColinMcInnes
Copy link
Contributor Author

ColinMcInnes commented Jan 9, 2025

ND6_INFINITE_LIFETIME

Something like this?

		if (rap->lifetime != ND6_INFINITE_LIFETIME)
		{
			elapsed = (uint32_t)eloop_timespec_diff(&now, &rap->acquired, NULL);
			rt->rt_expires = rap->lifetime - elapsed;
		}
		else
			rt->rt_expires = ND6_INFINITE_LIFETIME;

@rsmarples
Copy link
Member

Exactly! We have a lot of that code, so maybe we benefit from a helper function in common.c?

uint32_t
lifetime_left(uint32_t lifetime, const struct timespec *aquired, const struct timespec *now)
{
    uint32_t elpased;

    if (lifetime == INFINITE_LIFETIME)
        return lifetime;

    elapsed = (uint32_t)eloop_timespec_diff(now, &acquired, NULL);
    if (elapsed > lifetime)
        return 0;
    return lifetime - elapsed;
}

then we can simply write:

rt->rt_expires = lifetime_left(rap->lifetime, &rap->aquired, &now);

Thoughts? If you like it, #define INFINITE_LIFETIME (~0U) please in common.h
A future patch could then replace bits where we do this to reduce code even more.

If not set to infinite, lifetime is shortened
by time elapsed since acquired
Copy link
Member

@rsmarples rsmarples left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last round of changes, then I think we're good to merge!

Copy link
Member

@rsmarples rsmarples left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CI picked up a warning we need to address, but this is looking really good now.

@rsmarples rsmarples merged commit 7745034 into NetworkConfiguration:master Jan 15, 2025
2 of 5 checks passed
@rsmarples
Copy link
Member

@ColinMcInnes thanks for the excellent work here.

@rsmarples
Copy link
Member

@ColinMcInnes I used the new function to reduce more code in 7b94a2e

Hopefully everything still works!

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.

2 participants