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

PIM Assert state correction #64

Closed
wants to merge 4 commits into from

Conversation

idismmxiv
Copy link
Contributor

Proposal for assert state handling fixes to issue #63. It might be the case that MRTF_ASSERTED flag was meant to be used for inbound ASSERT detection only, but I took it into outbound handling as well. Had to add check for mrt->upstream pointer as well, as it was fairly easy to get pimd to crash without that.

With these changes ASSERT state is cleared after 180 seconds (hard coded value). Is this the "right" fix; I am not sure.

@troglobit
Copy link
Owner

I think this looks great! Very clean patch indeed. Will give it a spin ASAP in my testbed to see if it breaks something obvious.

@troglobit
Copy link
Owner

@idismmxiv As you know, I usually like to credit people who make significant contributions. Would you care to reveal your first and last name, or remain under this anonymous guise? 😃

@idismmxiv
Copy link
Contributor Author

We are redefining our contribution policy at work at the moment. Looks like after couple of days (next week perhaps) we have clearer guidance how to do open source contributions. So I hope you bare with Idis few more days.

@troglobit
Copy link
Owner

@idismmxiv No problem, I'll just delay the release a while longer! (Which is not a problem anway, still waiting for confirmation of having fixed issue #57 for FreeBSD :)

@troglobit troglobit added this to the 2.3.2 milestone Jan 7, 2016
@troglobit troglobit self-assigned this Jan 7, 2016
@idismmxiv
Copy link
Contributor Author

Added one more commit as there was unnecessary comparison in the case where new MRT entry was not created at all.
Regarding our contribution policy, it will still take little while (days, few weeks, who knows). If @troglobit want to move forward and release next version, then you are free to leave this contribution as anonymous.

@troglobit
Copy link
Owner

Thank you!

I'm in no hurry to release anything yet. The FreeBSD problems we've seen turned out to be simple misconfiguration, and there are a few more issues I'd like to fix before releasing v2.3.2 anyway :)

@troglobit
Copy link
Owner

@idismmxiv Any news on your company's open source policy? Due to the latest PR I'd like to get v2.3.2 released soon, like end of Febuary if possible.

Signed-off-by: Mika Joutsenvirta <mika.joutsenvirta@insta.fi>
@idismmxiv
Copy link
Contributor Author

I made proposed code cleanup and added signed-off-by line to commit.

@troglobit
Copy link
Owner

Alright, great! Hi Mika 😃

I'll rebase and merge later this week, thank you!

@troglobit
Copy link
Owner

Merged to master, with only slight (whitespace) changes.

@troglobit troglobit closed this Feb 10, 2016
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