Skip to content

[DX010] fix possible cpld race read issue#15339

Merged
qiluo-msft merged 1 commit intosonic-net:202012from
qnos:cel_dx010_cpld_parallel_read_fix
Jun 20, 2023
Merged

[DX010] fix possible cpld race read issue#15339
qiluo-msft merged 1 commit intosonic-net:202012from
qnos:cel_dx010_cpld_parallel_read_fix

Conversation

@qnos
Copy link
Copy Markdown
Contributor

@qnos qnos commented Jun 5, 2023

Why I did it

fix possible cpld race read issue between watchdog and reboot cause process

Work item tracking
  • Microsoft ADO (number only):

How I did it

Use flock to limit parallel access to cpld sys file

How to verify it

It can be simulate and verified with following python script

import signal
import subprocess
import threading

exit_flag = False

def run_command(cmd):
    status = True
    result = ""
    try:
        p = subprocess.Popen(
            cmd, shell=True, universal_newlines=True,
stdout=subprocess.PIPE, stderr=subprocess.PIPE)
        raw_data, err = p.communicate()
        if err == '':
            result = raw_data.strip()
    except:
        status = False
    return status, result

def get_cpld_reg_value(getreg_path, register):
    #cmd = "echo {1} > {0}; cat {0}".format(getreg_path, register)
    cmd = "flock {0} -c 'echo {1} > {0}; cat {0}'".format(getreg_path,
register)
    status, result = run_command(cmd)
    return result if status else None

def cpld_read(thread_num, cpld_reg):
    while not exit_flag:
        val
= get_cpld_reg_value("/sys/devices/platform/dx010_cpld/getreg",
cpld_reg)
        print(f"Thread {thread_num}: get cpld reg {cpld_reg}, value
{val}")

def signal_handler(sig, frame):
    global exit_flag
    print("Ctrl+C detected. Quitting...")
    exit_flag = True

if __name__ == '__main__':
    # Register the signal handler for Ctrl+C
    signal.signal(signal.SIGINT, signal_handler)

    t1 = threading.Thread(target=cpld_read, args=(1, '0x103',))
    t2 = threading.Thread(target=cpld_read, args=(2, '0x141',))
    t1.start()
    t2.start()
    t1.join()
    t2.join()

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

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

Why I did it
fix possible cpld race read issue between watchdog and reboot cause
process

How I did it
Use flock to limit parallel access to cpld sys file

How to verify it
It can be simulate and verified with following python script

```python3
import signal
import subprocess
import threading

exit_flag = False

def run_command(cmd):
    status = True
    result = ""
    try:
        p = subprocess.Popen(
            cmd, shell=True, universal_newlines=True,
stdout=subprocess.PIPE, stderr=subprocess.PIPE)
        raw_data, err = p.communicate()
        if err == '':
            result = raw_data.strip()
    except:
        status = False
    return status, result

def get_cpld_reg_value(getreg_path, register):
    #cmd = "echo {1} > {0}; cat {0}".format(getreg_path, register)
    cmd = "flock {0} -c 'echo {1} > {0}; cat {0}'".format(getreg_path,
register)
    status, result = run_command(cmd)
    return result if status else None

def cpld_read(thread_num, cpld_reg):
    while not exit_flag:
        val
= get_cpld_reg_value("/sys/devices/platform/dx010_cpld/getreg",
cpld_reg)
        print(f"Thread {thread_num}: get cpld reg {cpld_reg}, value
{val}")

def signal_handler(sig, frame):
    global exit_flag
    print("Ctrl+C detected. Quitting...")
    exit_flag = True

if __name__ == '__main__':
    # Register the signal handler for Ctrl+C
    signal.signal(signal.SIGINT, signal_handler)

    t1 = threading.Thread(target=cpld_read, args=(1, '0x103',))
    t2 = threading.Thread(target=cpld_read, args=(2, '0x141',))
    t1.start()
    t2.start()
    t1.join()
    t2.join()
```
@prgeor
Copy link
Copy Markdown
Contributor

prgeor commented Jun 6, 2023

@qnos please raise a master branch PR

@prgeor
Copy link
Copy Markdown
Contributor

prgeor commented Jun 6, 2023

@qiluo-msft Please merge for 202012

@qiluo-msft
Copy link
Copy Markdown
Collaborator

Is it also applicable to master and recent release branches? why only 202012?

@qiluo-msft qiluo-msft merged commit db12b8c into sonic-net:202012 Jun 20, 2023
@qnos
Copy link
Copy Markdown
Contributor Author

qnos commented Jun 20, 2023

Is it also applicable to master and recent release branches? why only 202012?

Is it also applicable to master and recent release branches? why only 202012?

PR #15371 already been raised for master branch.

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.

4 participants