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

[syncd-rpc]: The port is hard coded and the mapping between the index and the port is wrong #1625

Closed
stcheng opened this issue Apr 20, 2018 · 3 comments

Comments

@pavel-shirshov
Copy link
Contributor

Can you please elaborate? Why the hardcoded port is a bug and why the mappings are wrong?

@pavel-shirshov
Copy link
Contributor

I've fixed the copp test case by this.
sonic-net/sonic-mgmt@a0b77da

What do you mean the mapping between the index and the port is wrong?

@pavel-shirshov
Copy link
Contributor

Closing it for now.

lguohan pushed a commit that referenced this issue Jun 17, 2021
19615e3 Fixing db_migrator for Feature table (#1674)
d1c1c61 [tests]: skip some dynamic port breakout unit tests (#1677)
25669c3 [CI] sonic-config-engine now depends on SONiC YANG packages (#1675)
3ff68c4 [neighbor-advertiser] delete the tunnel maps appropriately (#1663)
a425ca2 [config] support for configuring muxcable to manual mode of operation  (#1642)
25e17de [show platform summary] Add chassis hardware info to platform summary and version (#1624)
f5f2a00 [db_migrator] fix old 1911 feature config migration to a new one. (#1635)
56db162 [config] Fix config int add incorrect ip (#1414)
1da879c [db_migrator][Mellanox] Update Mellanox buffer migrator with 2km-cable supported (#1564)
c2b760f [sonic_package_manager] flush once finished saving docker image into temporary file (#1638)
cd69473 Replace swsssdk.ConfigDBConnector and SonicDBConfig with swsscommon implementation (#1620)
5f20365 Change to use rvtysh when calling the show commands (#1572)
51d6bf5 Fix Aboot breakage in sonic package manager in sonic-installer (#1625)
18bed46 [console][show] Force refresh all lines status during show line (#1641)
b616cd9 [TPID CONFIG] Added TPID configuration CLI support (#1618)
01eb4b1 [show] support for show muxcable firmware version of only active banks (#1629)
7744c8d [fdb]cli: fdb entries are cleared according to vlan or port or vlan&&port (#657)
e23c5ee Add psu hardware revision to psushow table (#1601)
f1726fe Make advance_version_for_expected_database available for other db migrator test cases as well (#1614)
5d1ad05 [show] add support for muxcable metrics (#1615)
feeab29 [config] Sort Config Db When Saving (#1623)
carl-nokia pushed a commit to carl-nokia/sonic-buildimage that referenced this issue Aug 7, 2021
19615e3 Fixing db_migrator for Feature table (sonic-net#1674)
d1c1c61 [tests]: skip some dynamic port breakout unit tests (sonic-net#1677)
25669c3 [CI] sonic-config-engine now depends on SONiC YANG packages (sonic-net#1675)
3ff68c4 [neighbor-advertiser] delete the tunnel maps appropriately (sonic-net#1663)
a425ca2 [config] support for configuring muxcable to manual mode of operation  (sonic-net#1642)
25e17de [show platform summary] Add chassis hardware info to platform summary and version (sonic-net#1624)
f5f2a00 [db_migrator] fix old 1911 feature config migration to a new one. (sonic-net#1635)
56db162 [config] Fix config int add incorrect ip (sonic-net#1414)
1da879c [db_migrator][Mellanox] Update Mellanox buffer migrator with 2km-cable supported (sonic-net#1564)
c2b760f [sonic_package_manager] flush once finished saving docker image into temporary file (sonic-net#1638)
cd69473 Replace swsssdk.ConfigDBConnector and SonicDBConfig with swsscommon implementation (sonic-net#1620)
5f20365 Change to use rvtysh when calling the show commands (sonic-net#1572)
51d6bf5 Fix Aboot breakage in sonic package manager in sonic-installer (sonic-net#1625)
18bed46 [console][show] Force refresh all lines status during show line (sonic-net#1641)
b616cd9 [TPID CONFIG] Added TPID configuration CLI support (sonic-net#1618)
01eb4b1 [show] support for show muxcable firmware version of only active banks (sonic-net#1629)
7744c8d [fdb]cli: fdb entries are cleared according to vlan or port or vlan&&port (sonic-net#657)
e23c5ee Add psu hardware revision to psushow table (sonic-net#1601)
f1726fe Make advance_version_for_expected_database available for other db migrator test cases as well (sonic-net#1614)
5d1ad05 [show] add support for muxcable metrics (sonic-net#1615)
feeab29 [config] Sort Config Db When Saving (sonic-net#1623)
taras-keryk pushed a commit to taras-keryk/sonic-buildimage that referenced this issue Apr 28, 2022
…-net#1625)

> Failure cause

The `get_rootfs_path` contextmanager was repurposed to implement
`get_file_in_image` and later used as a function by leveraging some
python complexity to bypass the restrictions coming with the
contextmanager which were added purposefuly.
It was then called multiple times to compute paths though a simple
path join using `new_image_dir` could have been performed.

The `get_rootfs_path` implementation for Aboot behaved differently
when a rootfs was extracted or kept within the SWI image. It also
behaved differently on secureboot systems.
The updated method was then called on non-existing files for which
the method was never meant to process.

> Context around the failure

Over time, the installation and boot process has slightly diverged from
the ONIE one. There are 3 scenarios to consider.

1) Regular boot similar to ONIE
This one should just work the same as the filesystem layout is
unchanged.

2) docker_inram where dockerfs.tar.gz is extracted in tmpfs at boot
In this scenario the boot is similar to the regular one beside that
dockerfs.tar.gz is preserved intact on the flash and not extracted.
By design this does not fit the sonic-package-manager requierements and
the migration should be skipped which is what I did in this review.
In the coming month this boot mode will look closer to 3) below.

3) Secureboot on Arista devices
In this scenario the SWI image is kept intact and nothing extracted
from it. By ensuring the integrity of the SWI we can guarantee that no
code/data has been tampered with. This mode also relies on
`docker_inram` at the moment.
It could be enhanced when sonic-package-manager can guarantee the
integrity of code and data that is both installed and migrated.

> Solution provided

The method `get_file_in_image` was reverted to its original meaning
`get_rootfs_path` as there is no point in keeping the new one.
It doesn't have the necessary logic to handle more than the rootfs and
the logic can be easily be achieved by the following line.
`os.path.join(bootloader.get_image_path(binary_image_name), 'something')`

A new Bootloader method called `support_package_migration` is
introduced to allow the bootloader to skip the package migration based
on the image (docker_inram) or its own configuration (secureboot).
By default all bootloaders enable the package migration.

That change leads to 1) above running package migration while 2) and 3)
skip it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants