-
Notifications
You must be signed in to change notification settings - Fork 783
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
[19778][20296] Add netmask filter transport configuration + interface allowlist and blocklist #4241
Conversation
9d1dd95
to
d950405
Compare
f4bfccd
to
f7f1390
Compare
79f136f
to
2c91a08
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, great job in this PR! bringing important corrections and refactor.
I am leaving a partial review here. Still pending review the transport implementations, the xmlparsing related things, and testing that the issue is gone.
I could perfectly reproduce the issue. In the attached image the same data packet was being delivered to different destinations: lan
address (correctly) and localhost
(wrongly, after a failed docker-ip interpretation)
Leaving some questions and suggestions though.
In addition, some fixes should be made to correct windows compilation and I think that the PR should be paired with a Fast DDS Docs
documentation PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left the second part of the review.
I checked that the feature fixes the locator transformation mangling but I still need to properly test the blocklist and netmask filter.
The documentation has been linked so I remove the docs pending label
2c91a08
to
19cffc9
Compare
1660f76
to
76d7338
Compare
@richiprosima please test this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice effort on the review and refactors! looks really nice.
Leaving some minor suggestions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with green CI
@richiprosima please test this |
@richiprosima please test this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with green CI
Signed-off-by: Juan Lopez Fernandez <[email protected]> Signed-off-by: EduPonz <[email protected]>
Signed-off-by: Juan Lopez Fernandez <[email protected]> Signed-off-by: EduPonz <[email protected]>
Signed-off-by: Juan Lopez Fernandez <[email protected]> Signed-off-by: EduPonz <[email protected]>
Signed-off-by: Juan Lopez Fernandez <[email protected]> Signed-off-by: EduPonz <[email protected]>
Signed-off-by: Juan Lopez Fernandez <[email protected]> Signed-off-by: EduPonz <[email protected]>
Signed-off-by: Juan Lopez Fernandez <[email protected]> Signed-off-by: EduPonz <[email protected]>
Signed-off-by: Juan Lopez Fernandez <[email protected]> Signed-off-by: EduPonz <[email protected]>
Signed-off-by: Juan Lopez Fernandez <[email protected]> Signed-off-by: EduPonz <[email protected]>
Signed-off-by: Juan Lopez Fernandez <[email protected]> Signed-off-by: EduPonz <[email protected]>
…o cpp Signed-off-by: Juan Lopez Fernandez <[email protected]> Signed-off-by: EduPonz <[email protected]>
Signed-off-by: Juan Lopez Fernandez <[email protected]> Signed-off-by: EduPonz <[email protected]>
Signed-off-by: Juan Lopez Fernandez <[email protected]> Signed-off-by: EduPonz <[email protected]>
Signed-off-by: Juan Lopez Fernandez <[email protected]> Signed-off-by: EduPonz <[email protected]>
Signed-off-by: Juan Lopez Fernandez <[email protected]> Signed-off-by: EduPonz <[email protected]>
Signed-off-by: Juan Lopez Fernandez <[email protected]> Signed-off-by: EduPonz <[email protected]>
Signed-off-by: Juan Lopez Fernandez <[email protected]> Signed-off-by: EduPonz <[email protected]>
eb9943c
to
98be415
Compare
@richiprosima please test this |
@richiprosima please test windows test mac |
@richiprosima please check style |
Description
This PR adds two new features:
Behaviour changes:
SystemInfo
singleton, in order to reduce the number of system calls performed and promote consistency across separate Fast-DDS code sections.Contributor Checklist
versions.md
file (if applicable).Related documentation PR: eProsima/Fast-DDS-docs# (PR)
Reviewer Checklist