[syseeprom] Add a new daemon to populate syseeprom info to state DB

[keboliu]

syseeprom: Add a new daemon to populate the syseeprom info to state DB

• This daemon intends to collect the syseeprom information and write it to state DB, so CLI can get this info from DB instead of access HW. This daemon will keep monitoring the DB table, if table was deleted, it will write the info to DB again.

There will be a followed PR to add make files and modify start.sh after this one merged.

Signed-off-by: Kebo Liu [email protected]

[lguohan]

is this eeprom information in state db? the state db is clearly after sonic config load or swss restart. we need to deal with such situation.

[lguohan]

need to deal with state clear due to config reload or swss restart.

[keboliu]

@lguohan added '-w' and '-c' options to the scripts to handle write DB and clear DB functions respectively.

[liat-grozovik]

@jleveque @lguohan can you please review? this PR is the last piece of the pmon refactoring to be part of 201904

[lguohan]

it does not address my comments : "need to deal with state clear due to config reload or swss restart."

[nazariig]

it does not address my comments : "need to deal with state clear due to config reload or swss restart."

@lguohan can we consider having an EEPROM daemon?

[henry-ma]

@lguohan could you please approve this or raise your comments? We should get it merged as part of Pmon enhancement work.

[keboliu]

it does not address my comments : "need to deal with state clear due to config reload or swss restart."

@lguohan, as I mentioned in the description there will be a followd PR against sonic-buildimage which will include the code to call this utility to post info to DB during pmon start and clear DB when pmon stopped. I was planning to create it after this one merged.
Now in order to give you a whole picture of this solution, I just submitted that one: sonic-net/sonic-buildimage#2866 you can find the code where to clear DB when pmon stop, please also review.

[keboliu]

it does not address my comments : "need to deal with state clear due to config reload or swss restart."

@lguohan can we consider having an EEPROM daemon?

@nazariig I think it kind of overkill to have a daemon here since eeprom info never changed.

[qiluo-msft]

As comments

Unblock

[keboliu]

@lguohan I have addressed new comments with Qi and Joe, would you please approve if it's OK to you?

[jleveque]

@keboliu: The event listener does not take care of the situation Guohan was concerned about. I think you misunderstood his comment. We are not concerned with clearing the syseeprom data from the DB when PMON exits; syseeprom data is static and should not change. However, the concern Guohan raised was that the STATE_DB could get cleared by other services (config reload, swss restart, etc.). We need a way to recognize the STATE_DB was cleared and repopulate the syseeprom info into the DB.

The SwSS restart scenario shouldn't be a problem because we have configured the PMon service as a dependent of SwSS via systemd. Thus, if SwSS gets restarted, so too should PMon get restarted. However, we still need a way to get notified that the STATE_DB was cleared so we can repopulate the info.

[jleveque]

As comments.

[jleveque]

A couple more small requests.

[jleveque]

@lguohan: Please review the latest changes.

[jleveque]

@keboliu: A couple comments:

1. When you add this daemon to the supervisor config file, make sure you set it to restart always to ensure it's always running.
2. This storage of syseeprom info to STATE_DB should be viewed as a cache to improve performance for non-critical applications (SNMP, streaming telemetry, etc.). However, there are critical applications which rely on obtaining the MAC address. These applications should never fail. Thus, they should rely on calling decode-syseeprom. Therefore, we should not remove the ability for decode-syseeprom to access the hardware directly. At the most, we could modify decode-syseeprom to use the STATE_DB as a cache, and return the data if found, otherwise, it should poll the hardware directly as it currently does.

[keboliu]

@keboliu: A couple comments:

1. When you add this daemon to the supervisor config file, make sure you set it to restart always to ensure it's always running.

updated in another PR.

2. This storage of syseeprom info to STATE_DB should be viewed as a cache to improve performance for non-critical applications (SNMP, streaming telemetry, etc.). However, there are critical applications which rely on obtaining the MAC address. These applications should never fail. Thus, they should rely on calling decode-syseeprom. Therefore, we should not remove the ability for decode-syseeprom to access the hardware directly. At the most, we could modify decode-syseeprom to use the STATE_DB as a cache, and return the data if found, otherwise, it should poll the hardware directly as it currently does.

the current decode-syseeprom logic is implemented as you mentioned, when it failed to get data from DB, it will fall back on access the hardware. So we are ok here.