-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[cfggen] Add tool to translate openconfig acl into sonic format #388
Conversation
rule_props["PACKET_ACTION"] = "FORWARD" | ||
elif rule.actions.config.forwarding_action == "DROP": | ||
rule_props["PACKET_ACTION"] = "DROP" | ||
elif rule.actions.config.forwarding_action == "REJECT": |
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'm just curious what they do for the reject action. any difference from the drop? #Closed
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.
By openconfig definition, REJECT is to "Drop the packet and send an ICMP error message to the source". We don't support ICMP error message right now, of course. #Closed
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.
aha thanks. #Closed
07cd100
to
38d19ce
Compare
38d19ce
to
b7486c2
Compare
if flag == "TCP_SYN": | ||
tcp_flags = tcp_flags | 0x02 | ||
if flag == "TCP_FIN": | ||
tcp_flags = tcp_flags | 0x01 |
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.
probably swap the order of SYN and FIN? #Resolved
table_props = {} | ||
table_props["policy_desc"] = table_name | ||
table_props["type"] = "L3" | ||
table_props["ports"] = "Ethernet0" |
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.
why is this Ethernet0? #Resolved
|
||
|
||
def main(): | ||
translate(sys.argv[1]) |
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.
rename to translate_acl for better clarity? #Resolved
|
||
{% if docker_config_engine_debs != '' %} |
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.
-> {% if docker_config_engine_debs.strip() -%} #Resolved
RUN dpkg -i \ | ||
{% for deb in docker_config_engine_debs.split(' ') -%} | ||
debs/{{ deb }}{{' '}} | ||
{%- endfor %} | ||
{%- endif -%} | ||
|
||
{% if docker_config_engine_whls != '' %} |
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.
see above #Resolved
sudo LANG=C DEBIAN_FRONTEND=noninteractive chroot $FILESYSTEM_ROOT apt-get -y install \ | ||
python-dev \ |
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.
also need to remove python-dev later? #Resolved
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.
It's in base image. Do we also care a lot about size here? I prefer we keep it for easier usage of pip.
In reply to: 105694773 [](ancestors = 105694773)
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.
we care about size since some switch only have 2G disk, what's the size we are talking about? #Resolved
rule_data["ACL_RULE_TABLE:"+table_name+":Rule_"+str(rule_idx)] = rule_props | ||
rule_data["OP"] = "SET" | ||
|
||
rule_props["priority"] = 10000 - rule_idx |
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.
make 10000 as a parameter with default value, it should not be hardcoded in the code. #Resolved
elif rule.ip.config.protocol == "IP_AUTH": | ||
rule_props["IP_PROTOCOL"] = "51" | ||
elif rule.ip.config.protocol == "IP_L2TP": | ||
rule_props["IP_PROTOCOL"] = "115" |
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.
better to create a map for such translation? #Resolved
print "Unknown rule action %s in table %s, rule %d!" % (rule.actions.config.forwarding_action, table_name, rule_idx) | ||
return {} | ||
|
||
if rule.ip.config.protocol == "": |
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.
compare with "" does not look like a good practice. check below
table_data = [{}] | ||
table_data[0]["ACL_TABLE:"+table_name] = table_props | ||
table_data[0]["OP"] = "SET" | ||
dump_json("table_"+table_name+".json", table_data) |
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.
which directory do we put them into? can you add an option to specify the output directory? #Resolved
what's the license for openconfig_acl.py? can we import the whole github instead of this one file? How do we plan to maintain this file? #Resolved |
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.
see the comments
This file is not in openconfig github. It is an auto-generated file by pyangbind taking openconfig and ietf-config github as input. We can also rerun the generation process during every build, but I doubt if we really have the need to do that as it will quite complicate the build process. In reply to: 286154088 [](ancestors = 286154088) |
ok, can you put comments in the file, to describe how this file is generated? #Resolved |
8e18b73
to
4a3c0e1
Compare
4a3c0e1
to
fa2ff78
Compare
* cd97c60 2018-12-03 | Add support for recreate host interfaces tap devices on warm start (sonic-net#392) [Kamil Cudnik] * b4a7160 2018-12-03 | Drain asic queue before processing shutdown request (sonic-net#388) [Kamil Cudnik] Signed-off-by: Guohan Lu <[email protected]>
- [warm boot] introduce command line options to warm/fast reboot scripts (sonic-net#399) - Use -d instead of -m in config qos (sonic-net#388) Signed-off-by: Ying Xie <[email protected]>
- [warm boot] introduce command line options to warm/fast reboot scripts (#399) - Use -d instead of -m in config qos (#388) Signed-off-by: Ying Xie <[email protected]>
…onic-net#358)" (sonic-net#371)" (sonic-net#388) This reverts commit 8fc09d0.
Update the sonic-swss-common submodule. The following are the commits in the submodule. ``` 95f9e11 2020-11-19 | [pyext] allow to catch exceptions raised in python (sonic-net#415) [Stepan Blyshchak] 5a718f9 2020-11-18 | [swig] translate C++ `del` to python `delete` (sonic-net#416) [Qi Luo] 40b255b 2020-11-12 | Fix: SWIG dict.get() should have optional default value parameter (sonic-net#413) [Qi Luo] 91e484d 2020-11-07 | Reduce notice logging (sonic-net#412) [Qi Luo] f5945ae 2020-11-05 | Mux Cable schema definitions for interaction between linkmanager and xcvrd (sonic-net#411) [vdahiya12] 602f9c2 2020-11-05 | [lua] load lua script on demand (sonic-net#409) [Dong Zhang] d88412b 2020-11-04 | Rename hdel to del when using multiple keys as param (sonic-net#410) [Kamil Cudnik] e0c229a 2020-11-04 | CHASSIS_STATE_DB on control-card for chassis state (sonic-net#395) [mprabhu-nokia] a4e3ac8 2020-11-04 | Chassisd config table to store admin state (sonic-net#388) [mprabhu-nokia] ```
Update the sonic-swss-common submodule. The following are the commits in the submodule. ``` 95f9e11 2020-11-19 | [pyext] allow to catch exceptions raised in python (#415) [Stepan Blyshchak] 5a718f9 2020-11-18 | [swig] translate C++ `del` to python `delete` (#416) [Qi Luo] 40b255b 2020-11-12 | Fix: SWIG dict.get() should have optional default value parameter (#413) [Qi Luo] 91e484d 2020-11-07 | Reduce notice logging (#412) [Qi Luo] f5945ae 2020-11-05 | Mux Cable schema definitions for interaction between linkmanager and xcvrd (#411) [vdahiya12] 602f9c2 2020-11-05 | [lua] load lua script on demand (#409) [Dong Zhang] d88412b 2020-11-04 | Rename hdel to del when using multiple keys as param (#410) [Kamil Cudnik] e0c229a 2020-11-04 | CHASSIS_STATE_DB on control-card for chassis state (#395) [mprabhu-nokia] a4e3ac8 2020-11-04 | Chassisd config table to store admin state (#388) [mprabhu-nokia] ```
Update the sonic-swss-common submodule. The following are the commits in the submodule. ``` 95f9e11 2020-11-19 | [pyext] allow to catch exceptions raised in python (sonic-net#415) [Stepan Blyshchak] 5a718f9 2020-11-18 | [swig] translate C++ `del` to python `delete` (sonic-net#416) [Qi Luo] 40b255b 2020-11-12 | Fix: SWIG dict.get() should have optional default value parameter (sonic-net#413) [Qi Luo] 91e484d 2020-11-07 | Reduce notice logging (sonic-net#412) [Qi Luo] f5945ae 2020-11-05 | Mux Cable schema definitions for interaction between linkmanager and xcvrd (sonic-net#411) [vdahiya12] 602f9c2 2020-11-05 | [lua] load lua script on demand (sonic-net#409) [Dong Zhang] d88412b 2020-11-04 | Rename hdel to del when using multiple keys as param (sonic-net#410) [Kamil Cudnik] e0c229a 2020-11-04 | CHASSIS_STATE_DB on control-card for chassis state (sonic-net#395) [mprabhu-nokia] a4e3ac8 2020-11-04 | Chassisd config table to store admin state (sonic-net#388) [mprabhu-nokia] ```
… automatically (#16676) #### Why I did it src/sonic-platform-common ``` * c63abc0 - (HEAD -> master, origin/master, origin/HEAD) [Credo][Ycable] Remove the thread locker protection from the thread-safe APIs (#388) (21 hours ago) [Xinyu Lin] ``` #### How I did it #### How to verify it #### Description for the changelog
… automatically (#17084) #### Why I did it src/sonic-platform-common ``` * e7325db - (HEAD -> 202305, origin/202305) Fix SSD health percentage issue for vendor Virtium (#407) (#408) (11 hours ago) [Stephen Sun] * 87e33ab - [Credo][Ycable] Remove the thread locker protection from the thread-safe APIs (#388) (11 hours ago) [Xinyu Lin] ``` #### How I did it #### How to verify it #### Description for the changelog
No description provided.