Skip to content

Junos: parse input-vlan-map and output-vlan-map#8885

Merged
dhalperi merged 3 commits intobatfish:masterfrom
0x-0ddc0de:master
Dec 18, 2023
Merged

Junos: parse input-vlan-map and output-vlan-map#8885
dhalperi merged 3 commits intobatfish:masterfrom
0x-0ddc0de:master

Conversation

@0x-0ddc0de
Copy link
Copy Markdown
Contributor

No description provided.

@batfish-bot
Copy link
Copy Markdown

This change is Reviewable

Copy link
Copy Markdown
Member

@dhalperi dhalperi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @zergling-aws)


projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniper_interfaces.g4 line 220 at r1 (raw file):

i_input_vlan_map
:
   INPUT_VLAN_MAP PUSH

Given the simplicity of this grammar, can you please add all the options: https://www.juniper.net/documentation/us/en/software/junos/multicast-l2/topics/ref/statement/output-vlan-map-edit-interfaces-ge.html

i_input_vlan_map: INPUT_VLAN_MAP action = (PUSH | ...);

Can do this for both input and output.


projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniper_interfaces.g4 line 290 at r1 (raw file):

;

i_unit

FYI confirmed these are only valid under units. Nice work.


projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniper_interfaces.g4 line 300 at r1 (raw file):

      i_common
      | i_bandwidth
      | i_input_vlan_map

please warn on both of these, they related to L2/L3 inference that we don't yet support.

(FYI no need to add a test that the warning is present, codecov will show us it got hit.)

Copy link
Copy Markdown
Contributor Author

@0x-0ddc0de 0x-0ddc0de left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 3 of 5 files reviewed, all discussions resolved (waiting on @dhalperi)


projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniper_interfaces.g4 line 220 at r1 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

Given the simplicity of this grammar, can you please add all the options: https://www.juniper.net/documentation/us/en/software/junos/multicast-l2/topics/ref/statement/output-vlan-map-edit-interfaces-ge.html

i_input_vlan_map: INPUT_VLAN_MAP action = (PUSH | ...);

Can do this for both input and output.

done


projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniper_interfaces.g4 line 300 at r1 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

please warn on both of these, they related to L2/L3 inference that we don't yet support.

(FYI no need to add a test that the warning is present, codecov will show us it got hit.)

done

Copy link
Copy Markdown
Member

@dhalperi dhalperi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @zergling-aws)


projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniper_interfaces.g4 line 220 at r2 (raw file):

i_input_vlan_map
:
   INPUT_VLAN_MAP (POP | PUSH)

please include all the action alternatives: https://www.juniper.net/documentation/us/en/software/junos/multicast-l2/topics/ref/statement/input-vlan-map-edit-interfaces-ge.html

(pop | pop-pop | pop-swap | push | push-push | swap | swap-push | swap-swap);

projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniper_interfaces.g4 line 266 at r2 (raw file):

i_output_vlan_map
:
   OUTPUT_VLAN_MAP (POP | PUSH)

please include all the action alternative:

(pop | pop-pop | pop-swap | push | push-push | swap | swap-push | swap-swap);

https://www.juniper.net/documentation/us/en/software/junos/multicast-l2/topics/ref/statement/output-vlan-map-edit-interfaces-ge.html


projects/batfish/src/main/java/org/batfish/grammar/flatjuniper/ConfigurationBuilder.java line 4940 at r2 (raw file):

  @Override
  public void exitI_output_vlan_map(I_output_vlan_mapContext ctx) {
    warn(ctx, "Batfish does not support output-vlan-map");

Please follow the standard format for unsupported:

todo(ctx);

(on both)

@codecov
Copy link
Copy Markdown

codecov bot commented Dec 18, 2023

Codecov Report

Merging #8885 (22ad5d2) into master (d9b6865) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head 22ad5d2 differs from pull request most recent head 64efb36. Consider uploading reports for the commit 64efb36 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8885      +/-   ##
==========================================
- Coverage   72.51%   72.50%   -0.01%     
==========================================
  Files        3319     3319              
  Lines      169433   169437       +4     
  Branches    19878    19878              
==========================================
- Hits       122861   122852       -9     
- Misses      37426    37436      +10     
- Partials     9146     9149       +3     
Files Coverage Δ
...fish/grammar/flatjuniper/ConfigurationBuilder.java 67.63% <100.00%> (+0.03%) ⬆️

... and 4 files with indirect coverage changes

Copy link
Copy Markdown
Contributor Author

@0x-0ddc0de 0x-0ddc0de left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @zergling-aws)


projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniper_interfaces.g4 line 220 at r2 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

please include all the action alternatives: https://www.juniper.net/documentation/us/en/software/junos/multicast-l2/topics/ref/statement/input-vlan-map-edit-interfaces-ge.html

(pop | pop-pop | pop-swap | push | push-push | swap | swap-push | swap-swap);

done


projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniper_interfaces.g4 line 266 at r2 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

please include all the action alternative:

(pop | pop-pop | pop-swap | push | push-push | swap | swap-push | swap-swap);

https://www.juniper.net/documentation/us/en/software/junos/multicast-l2/topics/ref/statement/output-vlan-map-edit-interfaces-ge.html

done


projects/batfish/src/main/java/org/batfish/grammar/flatjuniper/ConfigurationBuilder.java line 4940 at r2 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

Please follow the standard format for unsupported:

todo(ctx);

(on both)

done

Copy link
Copy Markdown
Member

@dhalperi dhalperi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 3 of 3 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @zergling-aws)

@dhalperi dhalperi enabled auto-merge (squash) December 18, 2023 18:40
@dhalperi dhalperi merged commit 4ea5d1b into batfish:master Dec 18, 2023
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.

3 participants