Skip to content
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

Handling Control Plane ACLs so that IPv4 rules and IP… #1

Closed
wants to merge 273 commits into from

Conversation

madhanmellanox
Copy link
Owner

@madhanmellanox madhanmellanox commented Mar 22, 2020

…v6 rules are not added to the same ACL table

**-What I did? I added condition to check whether a new IPv6 rule is getting added to a IPv4 rule based ACL table and log a syslog and abort. And, the same for IPv4 based rule in a IPv6 based table.

**- How I did it? I added condition with matching table_ip_version against the rule props.

**-How did I verify? I added an IPv6 rule in a ACL table which contains all IPv4 rules and verified it throws a SYSLOG error and aborts the caclmgrd daemon.

- Description for the changelog
A condition check to verify only either IPv4 rules are added or IPv6 rules are added to the Control Plane ACL table.

The previous behavior of the caclmgrd was with both IPV4 rules and IPV6 rules added to the same control plane ACL table, inconsistent ACL rules were getting added and iptables and ip6tables commands were invoked with such inconsistent ACL rules. That is why this restriction of ACL tables can have only IPV4 rules or IPV6 rules.

SYSLOG if an IPv4 rule is added to IPv6 based ACL table or vice versa.

- A picture of a cute animal (not mandatory but encouraged)

madhanmellanox pushed a commit that referenced this pull request Mar 23, 2020
* Fix the stack-overflow issue in AclOrch::getTableById()

In case the table_id and m_mirrorTableId/m_mirrorTableId are empty
The function would be called recursively without ending.

```
If we have some rule that does not have table defined, it will trigger this issue.

(gdb) bt
#0  0x00007f77ac5af3a9 in swss::Logger::write (this=0x7f77ac801ec0 <swss::Logger::getInstance()::m_logger>, prio=swss::Logger::SWSS_DEBUG, fmt=0x7f77ac5f1690 ":> %s: enter")
    at logger.cpp:209
#1  0x00000000004a5792 in AclOrch::getTableById (this=this@entry=0x1c722a0, table_id="") at aclorch.cpp:2617
#2  0x00000000004a59d1 in AclOrch::getTableById (this=this@entry=0x1c722a0, table_id="") at aclorch.cpp:2634
#3  0x00000000004a59d1 in AclOrch::getTableById (this=this@entry=0x1c722a0, table_id="") at aclorch.cpp:2634
...
#20944 0x00000000004a59d1 in AclOrch::getTableById (this=this@entry=0x1c722a0, table_id="") at aclorch.cpp:2634
#20945 0x00000000004ad3ce in AclOrch::doAclRuleTask (this=this@entry=0x1c722a0, consumer=...) at aclorch.cpp:2437
#20946 0x00000000004b04cd in AclOrch::doTask (this=0x1c722a0, consumer=...) at aclorch.cpp:2141
#20947 0x00000000004231b2 in Orch::doTask (this=0x1c722a0) at orch.cpp:369
#20948 0x000000000041c4e9 in OrchDaemon::start (this=this@entry=0x1c19960) at orchdaemon.cpp:376
#20949 0x0000000000409ffc in main (argc=<optimized out>, argv=0x7ffe2e392d68) at main.cpp:295

(gdb) p table_id
$1 = ""
(gdb) p m_mirrorTableId
$2 = ""
(gdb) p m_mirrorV6TableId
$3 = ""
```

Signed-off-by: Zhenggen Xu <zxu@linkedin.com>
Copy link

@stepanblyschak stepanblyschak left a comment

Choose a reason for hiding this comment

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

As commented.
Please remove "Defect 2082949" - noone from community will understand this, please refer to github issue if exists.
Please, add a description of behaviour observed when IPv6 rule is added to IPv4 CP table and describe why this is unacceptable behaviour.

@@ -204,6 +204,19 @@ class ControlPlaneAclManager(object):
("DST_IP" in rule_props and rule_props["DST_IP"])):
table_ip_version = 4

if ((("SRC_IPV6" in rule_props and rule_props["SRC_IPV6"]) or

Choose a reason for hiding this comment

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

Suggest to create utility functions is_v4_rule(rule_props), is_v6_rule(rule_props) to make this condition more readable

Copy link
Owner Author

Choose a reason for hiding this comment

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

addressed in the next commit.

(table_ip_version == 4)):
log_error("CtrlPlane ACL table {} is a IPv4 based table and rule {} is a IPV6 rule!"
.format(table_name, rule_id))
sys.exit(1)

Choose a reason for hiding this comment

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

Try to raise exception instead of exit(1). In this case the caller of this method will be able to catch the exception to perfrom additional operations before shutting down (e.g. some cleanup)

Copy link
Owner Author

Choose a reason for hiding this comment

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

addressed in the next commit.

@madhanmellanox madhanmellanox changed the title Defect 2082949: Handling Control Plane ACLs so that IPv4 rules and IP… Handling Control Plane ACLs so that IPv4 rules and IP… Mar 25, 2020
lguohan and others added 27 commits April 16, 2020 10:26
Signed-off-by: Guohan Lu <lguohan@gmail.com>
Signed-off-by: Guohan Lu <lguohan@gmail.com>
1. undefine led_classdev_register as it is defined in leds.h
2. header file location change
   a. linux/i2c/pmbus.h -> linux/pmbus.h
   b. linux/i2c-mux-gpio.h -> linux/platform_data/i2c-mux-gpio.h
   c. linux/i2c/pca954x.h -> linux/platform_data/pca954x.h
Signed-off-by: Guohan Lu <lguohan@gmail.com>
Signed-off-by: Guohan Lu <lguohan@gmail.com>
Signed-off-by: Guohan Lu <lguohan@gmail.com>
do mount/umount in the chroot environment
install cron explicitly
install rasdaemon as a replacement for mcelog
switch python package docker-py to docker
ebtables is not enabled by default in buster

Signed-off-by: Guohan Lu <lguohan@gmail.com>
explicitly install dependency linux-base

Signed-off-by: Guohan Lu <lguohan@gmail.com>
change depends to linux-image-4.19.0-6-amd64 in debian/control

Signed-off-by: Guohan Lu <lguohan@gmail.com>
/usr/sbin/mke2fs is now rom busybox which does not support
the operation we need
Signed-off-by: Guohan Lu <lguohan@gmail.com>
…date

due to the upgrade from docker-py (1.6.0) to docker (4.1.0)
Signed-off-by: Guohan Lu <lguohan@gmail.com>
Currently we port SONiC to buster in a way that base image is on buster and
other dockers based on stretch. The benefit is that tasks can be carried out
simultaneously.

The build procedure can be treated as 2 stages.

The first stage is to build the stretch-based debs and dockers and the second
stage is to build the buster-based ones.

One thing we have to pay attention to is some debs depend on kernel should not
be built at stretch stage because the kernel isn't available at that time.
The idea is to move that kind of debs out of SONIC_STRETCH_DEBS. Meanwhile,
any dependency explicitly put on the stretch based dockers on kernel should be
removed.
The initialization of /dev/random (crng init) is responsible for
random number generation. On some devices, crng took very long to
finish.
Co-authored-by: Bing Sun <Bing_Sun@dell.com>
- create a file in files/image_config/ntp/ntp-systemd-wrapper to add mgmt vrf related start cmd for ntp service. So that the default /usr/lib/ntp/ntp-systemd-wrapper can be overriden during build time.

- modify build_debian.sh to cp files/image_config/ntp/ntp-systemd-wrapper to /usr/lib/ntp/ntp-systemd-wrapper during build time.

Co-authored-by: Bing Sun <Bing_Sun@dell.com>
in buster, by default, rsyslog service does not create pidfile which
breaks monit configuration

Signed-off-by: Guohan Lu <lguohan@gmail.com>
Signed-off-by: Guohan Lu <lguohan@gmail.com>
Signed-off-by: Guohan Lu <lguohan@gmail.com>
Signed-off-by: Guohan Lu <lguohan@gmail.com>
Signed-off-by: Guohan Lu <lguohan@gmail.com>
iMasaruOki and others added 20 commits June 4, 2020 13:29
Cleanup description string
First port (management port) are excluded from general port naming scheme.

Management port are excluded from general port naming scheme.

before:
|on GNS3  |in SONiC |
|---------|---------|
|Ethernet0|eth0     |
|Ethernet1|Ethernet0|
|Ethernet2|Ethernet4|
|Ethernet3|Ethernet8|

after:
|on GNS3  |in SONiC |
|---------|---------|
|eth0   |eth0       |
|Ethernet0|Ethernet0|
|Ethernet1|Ethernet4|
|Ethernet2|Ethernet8|

Signed-off-by: Masaru OKI <masaru.oki@gmail.com>
- Sensor and Fan information added to primary platforms for thermal API.
- Refactors involving better abstractions, code reuse and dead code removal.
- Improvements to the diag capabilities
- Pylintrc added to improve code quality. Will become fatal at a later time.

Co-authored-by: Baptiste Covolato <baptiste@arista.com>
)

**- Why I did it**
When I tested auto-restart feature of swss container by manually killing one of critical processes in it, swss will be stopped. Then syncd container as the peer container should also be
stopped as expected. However, I found sometimes syncd container can be stopped, sometimes
it can not be stopped. The reason why syncd container can not be stopped is the process
(/usr/local/bin/syncd.sh stop) to execute the stop() function will be stuck between the lines 164 –167. Systemd will wait for 90 seconds and then kill this process.

164 # wait until syncd quit gracefully
165 while docker top syncd$DEV | grep -q /usr/bin/syncd; do
166 sleep 0.1
167 done

The first thing I did is to profile how long this while loop will spin if syncd container can be
normally stopped after swss container is stopped. The result is 5 seconds or 6 seconds. If syncd
container can be normally stopped, two messages will be written into syslog:

str-a7050-acs-3 NOTICE syncd#dsserve: child /usr/bin/syncd exited status: 134
str-a7050-acs-3 INFO syncd#supervisord: syncd [5] child /usr/bin/syncd exited status: 134

The second thing I did was to add a timer in the condition of while loop to ensure this while loop will be forced to exit after 20 seconds:

After that, the testing result is that syncd container can be normally stopped if swss is stopped
first. One more thing I want to mention is that if syncd container is stopped during 5 seconds or 6 seconds, then the two log messages can be still seen in syslog. However, if the execution 
time of while loop is longer than 20 seconds and is forced to exit, although syncd container can be stopped, I did not see these two messages in syslog. Further, although I observed the auto-restart feature of swss container can work correctly right now, I can not make sure the issue which syncd container can not stopped will occur in future.

**- How I did it**
I added a timer around the while loop in stop() function. This while loop will exit after spinning
20 seconds.

Signed-off-by: Yong Zhao <yozhao@microsoft.com>
)

- What I did
In order to allow the SONiC community to check in platform capability file i.e. platform.json
file directly under device folder. We need to add this test to make sure the contents of the this file is compliant with platform capability design specified in DPB HLD doc

- How I did it
Added platformJson_checker.py file in Test folder.

Signed-off-by: Sangita Maity <sangitamaity0211@gmail.com>
…ic-net#4722)

[sonic-sairedis]
* 322dd01 2020-06-05 | Fix debian/rules makefile: use shell commands instead of dollar replacements (sonic-net#621) (HEAD -> master, origin/master, origin/HEAD) [Qi Luo]
* 6d55a75 2020-06-04 | Update SAI pointer to 1.6.1 (sonic-net#620) [Mahesh Maddikayala]
* d6c40e5 2020-06-01 | [MultiDB] use get API to obtain dbid instead of hardcode value (sonic-net#618) [Dong Zhang]
* bd132ec 2020-06-01 | Add synchronous mode to sairedis library (sonic-net#617) [Kamil Cudnik]
* 0a77a09 2020-06-01 | [meta] Fix tests to be backward compatible (sonic-net#619) [Kamil Cudnik]

[sonic-swss-common]
* 35bc01a 2020-06-05 | EVPN VXLAN DB support (sonic-net#339) (HEAD -> master, origin/master, origin/HEAD) [Rajesh Sankaran]
* 2c7354b 2020-05-30 | Add modifyRedis flag to consumer table object (sonic-net#344) [Kamil Cudnik]
* 5a32636 2020-05-27 | Fix memory leak in pyext when Selectable is returned to Python (sonic-net#343) [pavel-shirshov]
* [MultiDB] daemon base should use multiDB DBConnector
* [sonic-platform-daemon] update submodule for multiDB changes
* [secure boot] Support to sign swi image

* Fix build issue

* fix tab format issue

* Fix typing issue

* Change the sign_image.sh command line

* Remove SONIC_CETIFICATE_PATH

* Fix bugs
Signed-off-by: Joyas Joseph <joyas_joseph@dell.com>
**- Why I did it**

For decoding system EEPROM of S6000 based on Dell offset format and S6000-ON’s system EEPROM in ONIE TLV format.

**- How I did it**

- Differentiate between S6000 and S6000-ON using the product name available in ‘dmi’  ( “/sys/class/dmi/id/product_name” )
- For decoding S6000 system EEPROM in Dell offset format and updating the redis DB with the EEPROM contents, added a new class ‘EepromS6000’ in eeprom.py, 
- Renamed certain methods in both Eeprom, EepromS6000 classes to accommodate the plugin-specific methods.

**- How to verify it**

- Use 'decode-syseeprom' command to list the system EEPROM details.
- Wrote a python script to load chassis class and call the appropriate methods.

UT Logs: [S6000_eeprom_logs.txt](https://github.com/Azure/sonic-buildimage/files/4735515/S6000_eeprom_logs.txt), [S6000-ON_eeprom_logs.txt](https://github.com/Azure/sonic-buildimage/files/4735461/S6000-ON_eeprom_logs.txt)
Test script: [eeprom_test_py.txt](https://github.com/Azure/sonic-buildimage/files/4735509/eeprom_test_py.txt)
…ic-net#4713)

Update sonic-snmpagent submodule with PRs:
89b7b2c  [Multi-asic]: Namespace support for LLDP and Sensor tables (sonic-net#131)
fcb8955 Simplify test code (sonic-net#132)
a677876 [Multi-asic]: Support multi-asic platform (sonic-net#126)

update sonic-py-swsssdk submodule with PRs:
132f8d5  [MultiDB]: use python class composition to avoid confusion in base class (sonic-net#74)

Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>
The -sv2 suffix was used to differentiate SNMP Dockers when we transitioned from "SONiCv1" to "SONiCv2", about four years ago. The old Docker materials were removed long ago; there is no need to keep this suffix. Removing it aligns the name with all the other Dockers.
* Enable to build osw1800
* Modify wnc-eeprom which base on debian buster's eeprom.c

Co-authored-by: Brand.huang <brand.huang@wnc.com.tw>
)

Signed-off-by: Sabareesh Kumar Anandan <sanandan@marvell.com>
…sonic-net#4702)

Update AS7312-54X,AS7312-54XS,AS7315-27XB config.bcm file to make sure there is no the following error message.

configuration: format error in /usr/share/sonic/hwsku/th-as7312-48x25G+6x100G.config.bcm on line 110 (ignored)#15
**How I did it**

- Modified port_config.ini, TD2 settings to bring the ports UP.

**How to verify it**

- Check LLDP neighbors,LLDP table, interface status,EEPROM and other show commands.
- Do OIR, LED, Traffic testings.
**Why I did it**

- Added support for S6000 new HWSKU-Q24S32

**How I did it**

- Modified port_config.ini, TD2 settings to bring the ports UP.

**How to verify it**

- Check LLDP neighbors,LLDP table, interface status,EEPROM and other show commands.
- Do OIR, LED, Traffic testings.
…v6 rules are not added to the same ACL table
…_rule and is_ipv6_rule is addressed and also raising Exceptions instead of simply aborting when the conflict occurs is handled
@madhanmellanox
Copy link
Owner Author

This is an internal PR and the corresponding Azure PR got merged, so closing this one.

madhanmellanox pushed a commit that referenced this pull request Jan 17, 2021
This update brings in the following commits.

86c1108 Enable arm architecture to build in addition to amd64 (sonic-net#37)
4acb2c3 fix bugs and enhance Transformer (sonic-net#35)
49e5a22 ygot related enhancements and fixes (sonic-net#34)
51224de Fix ietf yang search path for cvl schema builds (sonic-net#32)
3c6cdb3 CVL Changes #8: 'must' and 'when' expression evaluation (sonic-net#31)
dabf231 CVL Changes #7: 'leafref' evaluation (sonic-net#28)
6f9535f CVL Changes #6: Customized Xpath Engine integration (sonic-net#27)
5e2466b DB-Layer fixes/enhancements (sonic-net#26)
9a27302 CVL Changes #4: Implementation of new CVL APIs (sonic-net#22)
dbf1093 Translib support for authorization, yang versioning and Delete flag (sonic-net#21)
80f369e CVL Changes #5: YParser enhancement (sonic-net#23)
904ce18 CVL Changes #3: Multi-db instance support (sonic-net#20)
9d24a34 CVL Changes #2:  YValidator infra changes for evaluating xpath expression (sonic-net#19)
f3fc40f CVL Changes #1: Initial CVL code reorganization and common infra changes (sonic-net#18)
4922601 Bulk and RPC API support in translib (#16)
1d730df RFC7895 yang module library implementation (#15)
madhanmellanox pushed a commit that referenced this pull request Jan 17, 2021
…ebian (sonic-net#6114)

Sonic devices advertise meaningful system description along with Debian package information.

before the fix:

-------------
admin@sonic:~$ show lldp neighbors
-------------------------------------------------------------------------------
LLDP neighbors:
-------------------------------------------------------------------------------
Interface: Ethernet0, via: LLDP, RID: 3, Time: 0 day, 16:36:30
SysName: sonic
SysDescr: Debian GNU/Linux 9 (stretch) Linux 4.9.0-11-2-amd64 #1 SMP Debian 4.9.189-3+deb9u2 (2019-11-11) x86_64
-------------------------------------------------------------------------------

After the fix:

root@sonic:~# show lldp neighbors Ethernet16
-------------------------------------------------------------------------------
LLDP neighbors:
-------------------------------------------------------------------------------
Interface:    Ethernet16, via: LLDP, RID: 10, Time: 0 day, 00:01:00
    SysName:      sonic
    SysDescr:     SONiC Software Version: SONiC.sonic_upstream_1.0_daily_201130_1501_62-dirty-20201130.203529 - HwSku: Accton-AS7816-64X - Distribution: Debian 10.6 - Kernel: 4.19.0-9-2-amd64
-------------------------------------------------------------------------------

Signed-off-by: sudhanshukumar22 <sudhanshu.kumar@broadcom.com>
madhanmellanox pushed a commit that referenced this pull request Apr 28, 2021
…ebian (sonic-net#6114)

Sonic devices advertise meaningful system description along with Debian package information.

before the fix:

-------------
admin@sonic:~$ show lldp neighbors
-------------------------------------------------------------------------------
LLDP neighbors:
-------------------------------------------------------------------------------
Interface: Ethernet0, via: LLDP, RID: 3, Time: 0 day, 16:36:30
SysName: sonic
SysDescr: Debian GNU/Linux 9 (stretch) Linux 4.9.0-11-2-amd64 #1 SMP Debian 4.9.189-3+deb9u2 (2019-11-11) x86_64
-------------------------------------------------------------------------------

After the fix:

root@sonic:~# show lldp neighbors Ethernet16
-------------------------------------------------------------------------------
LLDP neighbors:
-------------------------------------------------------------------------------
Interface:    Ethernet16, via: LLDP, RID: 10, Time: 0 day, 00:01:00
    SysName:      sonic
    SysDescr:     SONiC Software Version: SONiC.sonic_upstream_1.0_daily_201130_1501_62-dirty-20201130.203529 - HwSku: Accton-AS7816-64X - Distribution: Debian 10.6 - Kernel: 4.19.0-9-2-amd64
-------------------------------------------------------------------------------

Signed-off-by: sudhanshukumar22 <sudhanshu.kumar@broadcom.com>
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.