-
Notifications
You must be signed in to change notification settings - Fork 228
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
iptables fixes #488
iptables fixes #488
Conversation
* Add test rig that shows this
This partially resolves #485, but I believe that there are further issues that need to be investigated by someone else. |
* Also includes iptables for routers, this should make sure we dont break this feature again going forward * Fixes batfish#464
We've added another commit with a new iptables testrig that does more comprehensive tests. |
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 think it's good, but I'm not ready to approve until we can clarify the expected result of the reachability unit tests.
"answerElements" : [ | ||
{ | ||
"class" : "org.batfish.datamodel.FlowHistory" | ||
} |
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.
Is this supposed to be empty? What about the other reachability answers?
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.
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.
Yes all three questions are supposed to be empty. A non empty result is a violation of the policy. The questions are crafted so they return no results.
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.
OK
This PR fixes two things:
:FORWARD ACCEPT [0:0]
style of defaults (iptables-save doesn't require it, so neither should batfish)