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

[console speed] Inherit console speed from install environment #1987

Merged
merged 3 commits into from
Sep 1, 2018
Merged

[console speed] Inherit console speed from install environment #1987

merged 3 commits into from
Sep 1, 2018

Conversation

keboliu
Copy link
Collaborator

@keboliu keboliu commented Aug 27, 2018

- What I did
In the current installer the console speed is set to 9600 regardless the original value of the install enviroment. If the install enviroment was using another value, eg. 115200, after installed SONiC, user have to make some chage to adapt the new console speed. To give user a more smooth experience, suggest to keep using the original console speed instead of overwrite it.

- How I did it
in the install.sh add code to pick up console speed from install enviroment instead of hardcode to 9600.

- How to verify it
tested by installing SONiC from ONIE and SONiC itself.

- Description for the changelog

installer/x86_64/install.sh

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

CONSOLE_SPEED=9600

# Pick up console speed from install enviroment, if failed, set it to 9600
CONSOLE_SPEED=$(stty -F /dev/ttyS0 | grep speed | cut -d " " -f2)
Copy link
Collaborator

Choose a reason for hiding this comment

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

not all platform use ttyS0, need to get the the console_port first, check following grep.

lgh@gulv-vm1:/data2/sonic/stretch/sonic-buildimage$ git grep CONSOLE_PORT
device/accton/x86_64-accton_as5712_54x-r0/installer.conf:CONSOLE_PORT=0x2f8
device/accton/x86_64-accton_as7116_54x-r0/installer.conf:CONSOLE_PORT=0x3f8
device/accton/x86_64-accton_as7212_54x-r0/installer.conf:CONSOLE_PORT=0x2f8
device/accton/x86_64-accton_as7312_54x-r0/installer.conf:CONSOLE_PORT=0x2f8
device/accton/x86_64-accton_as7312_54xs-r0/installer.conf:CONSOLE_PORT=0x2f8
device/accton/x86_64-accton_as7512_32x-r0/installer.conf:CONSOLE_PORT=0x2f8
device/accton/x86_64-accton_as7712_32x-r0/installer.conf:CONSOLE_PORT=0x2f8
device/accton/x86_64-accton_as7716_32x-r0/installer.conf:CONSOLE_PORT=0x3f8
device/accton/x86_64-accton_as7716_32xb-r0/installer.conf:CONSOLE_PORT=0x3f8
device/celestica/x86_64-cel_e1031-r0/installer.conf:CONSOLE_PORT=0x2f8
device/celestica/x86_64-cel_seastone-r0/installer.conf:CONSOLE_PORT=0x3f8
device/dell/x86_64-dell_s6100_c2538-r0/installer.conf:CONSOLE_PORT=0x2f8
device/dell/x86_64-dell_z9100_c2538-r0/installer.conf:CONSOLE_PORT=0x2f8
device/dell/x86_64-dellemc_z9264f_c3538-r0/installer.conf:CONSOLE_PORT=0x3f8
device/delta/x86_64-delta_ag5648-r0/installer.conf:CONSOLE_PORT=0x3f8
device/delta/x86_64-delta_ag9032v1-r0/installer.conf:CONSOLE_PORT=0x3f8
device/delta/x86_64-delta_ag9064-r0/installer.conf:CONSOLE_PORT=0x3f8
device/delta/x86_64-delta_et-6248brb-r0/installer.conf:CONSOLE_PORT=0x2f8
device/ingrasys/x86_64-ingrasys_s8810_32q-r0/installer.conf:CONSOLE_PORT=0x2f8
device/ingrasys/x86_64-ingrasys_s8900_54xc-r0/installer.conf:CONSOLE_PORT=0x2f8
device/ingrasys/x86_64-ingrasys_s8900_64xc-r0/installer.conf:CONSOLE_PORT=0x2f8
device/ingrasys/x86_64-ingrasys_s9100-r0/installer.conf:CONSOLE_PORT=0x2f8
device/ingrasys/x86_64-ingrasys_s9130_32x-r0/installer.conf:CONSOLE_PORT=0x3f8
device/ingrasys/x86_64-ingrasys_s9180_32x-r0/installer.conf:CONSOLE_PORT=0x3f8
device/ingrasys/x86_64-ingrasys_s9200_64x-r0/installer.conf:CONSOLE_PORT=0x3f8
device/ingrasys/x86_64-ingrasys_s9230_64x-r0/installer.conf:CONSOLE_PORT=0x3f8
device/ingrasys/x86_64-ingrasys_s9280_64x-r0/installer.conf:CONSOLE_PORT=0x3f8
device/inventec/x86_64-inventec_d7032q28b-r0/installer.conf:CONSOLE_PORT=0x2f8
device/inventec/x86_64-inventec_d7054q28b-r0/installer.conf:CONSOLE_PORT=0x3f8
device/inventec/x86_64-inventec_d7264q28b-r0/installer.conf:CONSOLE_PORT=0x3f8
device/marvell/x86_64-marvell_slm5401_54x-r0/installer.conf:CONSOLE_PORT=0x2f8
device/mitac/x86_64-mitac_ly1200_b32h0_c3-r0/installer.conf:CONSOLE_PORT=0x3f8
device/quanta/x86_64-quanta_ix1b_32x-r0/installer.conf:CONSOLE_PORT=0x2f8
device/wnc/x86_64-wnc_osw1800-r0/installer.conf:CONSOLE_PORT=0x2f8

the logic should be move below line 83.

check the console_port, if it is 0x3f8 then it is ttyS0, if it is 0x2f8, then it is ttyS1. then need to check if CONSOLE_SPEED is already defined or not, if not, then automatically set the console speed. otherwise, use the CONSOLE_SPEED setting in the installer.conf.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@lguohan thanks for the comments, will do further change.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@lguohan I decide to get the console speed setting from '/proc/cmdline' instead of from ttyS*, to avoid to check which ttyS is using. please see the modification in the latest commit.

Copy link
Collaborator

@lguohan lguohan left a comment

Choose a reason for hiding this comment

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

good idea, need to improve

@lguohan
Copy link
Collaborator

lguohan commented Aug 28, 2018

retest this please

# so we can use pattern 'console=ttyS[0-9]+,[0-9]+' to match it
if [ -z "$CONSOLE_SPEED" ]; then
CONSOLE_SPEED=$(cat /proc/cmdline | grep -Eo 'console=ttyS[0-9]+,[0-9]+' | cut -d "," -f2)
fi
Copy link
Collaborator

Choose a reason for hiding this comment

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

we should also set the CONSOLE_PORT here if CONSOLE_PORT is not set.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@lguohan there is a default CONSOLE_PORT definition in line 68, are you suggesting to remove it and apply the same procedure to it with CONSOLE_SPEED?

Copy link
Collaborator

Choose a reason for hiding this comment

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

good question, do you know if line 83 will always be successful? will the /proc/cmdline always match and get you the CONSOLE_SPEED? if line 83 fails to match, what should be our CONSOLE_SPEED? Can you add a fall-back, say if CONSOLE_SPEED is null, then we set to the default value 9600?

I had thought twice. We can use same approach for CONSOLE_PORT as this one won't change and each platform vendor can hard-code it. we can probably derive this in ONIE environment as well, but it needs more testing.

Copy link
Collaborator

Choose a reason for hiding this comment

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

you use stty approach in the initial commit, why change to read from /proc/cmdline?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@guohan I have tested to install from the different version of ONIE as well as SONiC. Line 83 works well and always can get the speed and port, anyway I added fallback for both port and speed in case not able to get them from cmdline, they will be set to 9600 and ttyS0.

I switch to use cmdline just because of it much easier to get the current ttyS and speed of the installing enviroment. If use stty need to find out which ttyS it is using first.

please chek my latest commit.

@lguohan lguohan merged commit fd5a3cf into sonic-net:master Sep 1, 2018
@keboliu keboliu deleted the serial-port-baudrate branch September 13, 2018 01:29
stephenxs added a commit to stephenxs/sonic-buildimage that referenced this pull request Nov 30, 2021
f6f6f86 [mclaglink] fix acl out ports (sonic-net#2026)
fd887bf [Reclaim buffer] Reclaim unused buffer for dynamic buffer model (sonic-net#1910)
9258978 [orchagent, cfgmgr] Add response publisher and state recording (sonic-net#1992)
3d862a7 Fixing subport vs test script for subport under VNET (sonic-net#2048)
fb0a5fd Don't handle buffer pool watermark during warm reboot reconciling (sonic-net#1987)
16d4bcd Routed subinterface enhancements (sonic-net#1907)
9639db7 [vstest/subintf] Add vs test to validate sub interface ingress to a vnet (sonic-net#1642)

Signed-off-by: Stephen Sun <[email protected]>
judyjoseph added a commit that referenced this pull request Dec 1, 2021
    [8522f4f] Don't handle buffer pool watermark during warm reboot reconciling (#1987)
stephenxs added a commit to stephenxs/sonic-buildimage that referenced this pull request Dec 2, 2021
9258978 [orchagent, cfgmgr] Add response publisher and state recording (sonic-net#1992)
3d862a7 Fixing subport vs test script for subport under VNET (sonic-net#2048)
fb0a5fd Don't handle buffer pool watermark during warm reboot reconciling (sonic-net#1987)

Signed-off-by: Stephen Sun <[email protected]>
vivekrnv pushed a commit to vivekrnv/sonic-buildimage that referenced this pull request Jan 6, 2022
Signed-off-by: Vivek Reddy Karri <[email protected]>

30f5dd6 Update the example for pfcwd start command (sonic-net#1984)
9e30871 [Auto Techsupport] Event driven Techsupport Bug Fixes (sonic-net#1986)
fbd565d Fix wrong help message for cable length setting (sonic-net#1978)
b3a5052 [GCU] Using simulated config instead of target config when validating replace operation in NoDependencyMoveValidator (sonic-net#1987)
35cb524 [GCU] Copying config_db before callding sonic_yang.loadData (sonic-net#1983)
a98858d [GCU] Different apply-patch runs should produce same sorted steps (sonic-net#1988)
8c81ae3 [breakout] Fix the check  when port is not present in BREAKOUT_CFG table (sonic-net#1765)
bc8fe7c [doc][DPB] Update DPB related interface breakout command Info (sonic-net#1438)
1a2a9a3 [config] Fix 'config reload -l' command to get filename by default (sonic-net#1611)
ed2fa69 [debug dump util] FDB debug dump util changes (sonic-net#1968)
3b642c9 [GCU] Loading yang-models only once (sonic-net#1981)
bb56fc2 Update swss_ready check to check per namespace swss service (sonic-net#1974)
4f39f9f [GCU] Moving PatchSorter unit-test to json file to make it easier to read/maintain (sonic-net#1977)
1a75870 [CLI][Help string] Changed the show command help text to be more consistent with each other.
818dcbf [GCU] Implementing DryRun by printing patch-sorter steps/imitating config_db (sonic-net#1973)
judyjoseph added a commit that referenced this pull request Jan 9, 2022
4236bc4 [config reload] Fixing config reload when timer based delayed services are disabled (#1967)
d2514e4 [GCU] Different apply-patch runs should produce same sorted steps (#1988)
2878adb [GCU] Using simulated config instead of target config when validating replace operation in NoDependencyMoveValidator (#1987)
fb8ca98 [GCU] Loading yang-models only once (#1981)
f88ee92 [GCU] Copying config_db before callding sonic_yang.loadData (#1983)
9ed0e91 [GCU] Implementing DryRun by printing patch-sorter steps/imitating config_db (#1973)
b36b5e3 [GCU] Moving PatchSorter unit-test to json file to make it easier to read/maintain (#1977)
c0fa28b [generic-config-updater] Improving CreateOnly validator and marking /LOOPBACK_INTERFACE/LOOPBACK#/vrf_name as create-only (#1969)
0559d04 [generic-config-updater] Adding non-strict mode (#1929)
b07f477 [debug dump util] FDB debug dump util changes (#1968)
6d8757a [warm/fast-reboot] Fix kexec portion to support platforms based on Device Tree (#1966)
cc1409e [Auto Techsupport] Event driven Techsupport Bug Fixes (#1986)
6c48bd5 Fix wrong help message for cable length setting (#1978)
c0bbbe3 [breakout] Fix the check  when port is not present in BREAKOUT_CFG table (#1765)
5bb8cad [doc][DPB] Update DPB related interface breakout command Info (#1438)
e6fd990 [config] Fix 'config reload -l' command to get filename by default (#1611)
bd8f7bb Update swss_ready check to check per namespace swss service (#1974)
5439f94 [soft-reboot] Add support for platforms based on Device Tree (#1963)
7c5810a [config] Add portchannel support  for static route  (#1857)
7cb6a1b preserve old order for config reload (#1964)
20bddbd [Auto-Techsupport] Issues related to Multiple Cores crashing handled (#1948)
@xumia xumia mentioned this pull request Jan 17, 2022
5 tasks
qiluo-msft pushed a commit that referenced this pull request Jan 19, 2022
includes:

320591a [DualToR] Handle race condition between tunnel_decap and mux orchestrator (#2114)
5027a8f Handling Invalid CRM configuration gracefully (#2109)
0b120fa [ci]: use native arm64 and armhf pool (#2013)
394e88a Don't handle buffer pool watermark during warm reboot reconciling (#1987)
9008a01 patch for issue #1971 - enable Rx Drop handling for cisco-8000 (#2041)
2723ee3 create debug_shell_enable config to enable debug shell (#2060)
d7be0b9 [request parser] Add unit tests for request parser for multiple values (#1766)
theasianpianist pushed a commit to theasianpianist/sonic-buildimage that referenced this pull request Feb 5, 2022
…nic-net#1987)

- What I did
Don't handle buffer pool watermark during warm reboot reconciling

- Why I did it
This is to fix the community issue sonic-net/sonic-sairedis#862 and sonic-net#8722

- How I verified it
Perform a warm reboot. Check whether

buffer pool watermark handling is skipped during reconciling and handled after it.
other watermark handling is handled during reconciling as it was before.
Details if related
The warm reboot flow is like this:

System starts. Orchagent fetches the items from database stored before warm reboot and pushes them into m_toSync of all orchagents. This is done by bake, which can be overridden by sub orchagent.
All sub orchagents handle the items in m_toSync. At this point, any notification from redis-db is blocked.
Warm reboot converges.
Orchagent starts to handle notifications from redis-db.
The fix is like this: in FlexCounterOrch::bake. the buffer pool watermark handling is skipped.

Signed-off-by: Stephen Sun <[email protected]>
taras-keryk pushed a commit to taras-keryk/sonic-buildimage that referenced this pull request Apr 28, 2022
… replace operation in NoDependencyMoveValidator (sonic-net#1987)

#### What I did
Using `SimulatedConfig` instead of `TargetConfig` for validating a move using `NoDependencyMoveValidator`

SimulatedConfig: config after applying the given move
TargetConfig: the final config state that's required by the patch

The problem is if the moves is to update a list item, the list item location in the `TargetConfig` might have different location in the `CurrentConfig`.

The reason for that is the `TargetConfig` has the final outcome after applying the `patch`, but the move might be just making a small change towards this goal.

Example:
Assume current_config
```
{
    "VLAN": {
        "Vlan100": {
            "vlanid": "100",
            "dhcp_servers": [
                "192.0.0.1",
                "192.0.0.2"
            ]
        }
    }
}
```
TargetConfig:
```
{
    "VLAN": {
        "Vlan100": {
            "vlanid": "100",
            "dhcp_servers": [
                "192.0.0.3"
            ]
        }
    }
}
```
Move:
```python
ps.JsonMove(diff, OperationType.REPLACE, ["VLAN", "Vlan100", "dhcp_servers", 1], ["VLAN", "Vlan100", "dhcp_servers", 0])
```


The move means:
```
Replace the value in CurrentConfig that has path `/VLAN/Vlan100/dhcp_servers/1` with
the value from the TargetConfig that has the path `/VLAN/Vlan100/dhcp_servers/0`
```
Notice how the array index in CurrentConfig does not exist in TargetConfig

Instead of using TargetConfig to validate, use SimulatedConfig which is the config after applying the move. In this case it would be:
```
{
    "VLAN": {
        "Vlan100": {
            "vlanid": "100",
            "dhcp_servers": [
                "192.0.0.1",
                "192.0.0.3"
            ]
        }
    }
}
```

#### How I did it
Replace `diff.target_config` with `simulated_config`

#### How to verify it
added a unit-test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants