IOS: Add session info to interfaces #6653
Conversation
|
This change is |
Codecov Report
@@ Coverage Diff @@
## master #6653 +/- ##
============================================
- Coverage 73.52% 73.52% -0.01%
Complexity 36665 36665
============================================
Files 2934 2934
Lines 147488 147504 +16
Branches 17778 17780 +2
============================================
+ Hits 108442 108446 +4
- Misses 30508 30517 +9
- Partials 8538 8541 +3
|
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1, 1 of 1 files at r2.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @corinaminer)
projects/batfish/src/main/java/org/batfish/representation/cisco/CiscoConfiguration.java, line 1936 at r2 (raw file):
Assume each interface has its own session info (sessions are not shared by interfaces).
Is this a safe assumption? It would seem reasonable to allow return traffic to enter any interface on the same (in/out)side as newIface.
projects/batfish/src/main/java/org/batfish/representation/cisco/CiscoConfiguration.java, line 1951 at r2 (raw file):
Action.PRE_NAT_FIB_LOOKUP
Adding the FirewallSessionInteraceInfo to this (inside) interface will cause sessions to be created for all flows exiting this interface. Presumably we only want to create sessions for flows that entered an outside interface. Can we assume that?
If session was set up on inside interface, return flow can match it on any inside interface (same for setup/match on outside ifaces).
There was a problem hiding this comment.
Reviewable status: 0 of 4 files reviewed, 3 unresolved discussions (waiting on @anothermattbrown and @corinaminer)
projects/batfish/src/main/java/org/batfish/representation/cisco/CiscoConfiguration.java, line 1936 at r2 (raw file):
Previously, anothermattbrown (Matt Brown) wrote…
Assume each interface has its own session info (sessions are not shared by interfaces).Is this a safe assumption? It would seem reasonable to allow return traffic to enter any interface on the same (in/out)side as
newIface.
Good call -- i just checked in GNS3 and you're right. Fixed.
tests/parsing-tests/unit-tests-vimodel.ref, line 0 at r3 (raw file):
It's upsetting that an interface's session action changed, but it's just because that interface's config has both ip nat outside and ip nat inside. ip nat outside comes second, so POST_NAT_FIB_LOOKUP seems like the more correct option. (real interface configs can't have both and configuring one will overwrite the other.)
There was a problem hiding this comment.
Reviewed 4 of 4 files at r3.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @corinaminer)
projects/batfish/src/main/java/org/batfish/representation/cisco/CiscoConfiguration.java, line 1936 at r2 (raw file):
Previously, corinaminer (Corina Miner) wrote…
Good call -- i just checked in GNS3 and you're right. Fixed.
Does this comment need to be updated?
There was a problem hiding this comment.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @corinaminer)
projects/batfish/src/main/java/org/batfish/representation/cisco/CiscoConfiguration.java, line 1936 at r2 (raw file):
Previously, anothermattbrown (Matt Brown) wrote…
Does this comment need to be updated?
no, this comment is specific to ASA. There's a comment addressing non-ASA behavior down with the new conversion code
There was a problem hiding this comment.
Reviewable status: 1 of 4 files reviewed, 2 unresolved discussions (waiting on @anothermattbrown)
projects/batfish/src/main/java/org/batfish/representation/cisco/CiscoConfiguration.java, line 1951 at r2 (raw file):
Previously, anothermattbrown (Matt Brown) wrote…
Action.PRE_NAT_FIB_LOOKUPAdding the FirewallSessionInteraceInfo to this (inside) interface will cause sessions to be created for all flows exiting this interface. Presumably we only want to create sessions for flows that entered an outside interface. Can we assume that?
done.
There was a problem hiding this comment.
Reviewed 3 of 3 files at r4.
Reviewable status: all files reviewed, 1 unresolved discussion
No description provided.