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

Misleading comment in add_rp_grp_entry(), suggests missing required processing #108

Open
jp-t opened this issue Jul 10, 2017 · 2 comments
Open

Comments

@jp-t
Copy link
Contributor

jp-t commented Jul 10, 2017

Since the RP hold time was fixed in RP #106, this comment in rp.c around line 380:

        /* TODO: We shoudn't have old existing entry, because with the
         * current implementation all of them will be deleted
         * (different fragment_tag). Debug and check and eventually
         * delete.
         */

is not true anymore. A debug log message in that place will appear very often.

  • Contrary to the suggestion in the comment, the code here should not be removed.
  • Since the current behavior is new, the code is at best untested and might be incomplete or buggy.
    However a simple test with two routers linking a MC sender and a receiver, do work properly.

Need to investigate and correct/remove the comment.

@jp-t
Copy link
Contributor Author

jp-t commented Jul 10, 2017

Reading the whole function, it seems that the RP priority is not updated when an old rp_grp_entry is found, and if priority changed, the RP<=>group remapping does not take place.
This might become an issue in corner cases where RP priorities change while running.

@troglobit
Copy link
Owner

Thank you for following up and reporting this!

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

No branches or pull requests

2 participants