[load_minigraph]: Add support to load default_config.json file #1228

mlorrillere · 2020-11-11T02:11:32Z

This change add support for a per-device/per-npu default_config.json
file that contains platform-specific or hwsku-specific configuration to
be used when generating config_db*.json

An example of such file in a VoQ chassis could be:

{
    "DEVICE_METADATA": {
        "localhost": {
            "asic_id": "06:00.0"
        }
    }
}

- What I did

Added support for a default_config.json file present in platform directory for each NPU and for the host. This file will be loaded when config load_minigraph is called.

- Why I did it

Some VoQ devices require per-platform/per-NPU configurations to be present in CONFIG_DB. default_config.json provides a way for a vendor to give the required initial configuration of a device.

- How to verify it

Verified manually on a single and multi-NPU platform with and without a default_config.json present in the platform directory.

anshuv-mfst · 2021-02-03T17:18:50Z

Veda (Nokia) , @arlakshm - could you please review, thanks.

arlakshm

Please add unit test for this

arlakshm · 2021-02-05T19:30:45Z

config/main.py

@@ -294,6 +294,22 @@ def _get_device_type():

    return device_type

+def _get_hwsku(config_db):


We can remove this function here. Same function available in sonic_py_common get_hwsku

get_hwsku in sonic_py_common does not support namespaces, it will always connect to the host redis instance.

Also, get_hwsku in sonic_py_common will be stuck because it will try to open a new connection to CONFIG_DB.

This can be revisited when sonic_py_common provides support for namespaces and existing connection.

hwsku is for the whole device, wouldn't getting the hwsku form the host redis work will it change for each namespace/asic ?

This would be the same hwsku for all namespace/asic. However we can't get it before minigraph.xml is parsed and CONFIG_DB is populated, so calling get_hwsku from sonic_py_common beforehand is not possible since CONFIG_DB will be empty.

if config_db is empty then _get_hwsku won't return the correct hwsku also. I am trying to understand what the difference between _get_hwsku define here and get_hwsku in sonic_py_common

_get_hwsku is called after minigraph.xml is parsed and CONFIG_DB is populated for the corresponding namespace CONFIG_DB initialized at line 1274, _get_hwsku is called at line 1287).

The difference with get_hwsku from sonic_py_common is that _get_hwsku here reuse the existing connection to CONFIG_DB. If we cann get_hwsku from sonic_py_common instead it will get stuck because we can't open multiple connections the same time.

arlakshm · 2021-02-05T19:32:52Z

config/main.py

+            npu_id = device_info.get_npu_id_from_name(namespace)
+
+        # load default_config.json file from platform directory
+        default_config_file = os.path.join('/usr/share/sonic/device/', platform, npu_id, 'default_config.json')


can you give an example what is present in 'default_config.json'

I added the following example in the description:

{ "DEVICE_METADATA": { "localhost": { "instance_name": "ASIC0", "chassis_db_address" : "240.127.1.1", "start_chassis_db" : "1", "asic_id": "06:00.0" } } }

As per PR sonic-net/sonic-buildimage#5991, for VOQ chassis systems the "asic_id" and "instance_name" are configured via minigraph.xml. Will these values from default_config.json be preferred over what is configured through minigraph?

For VOQ chassis systems, we get "start_chassis_db" and "chassis_db_address" from chassisdb.conf from device specific directory (for example: https://github.com/Azure/sonic-buildimage/blob/master/device/arista/x86_64-arista_7800_sup/chassisdb.conf). These are needed before the database service is started in supervisor card. Having these in config_db is of no use. Also, if these are not used in multi-asic we should not have these in default_config.json.

As per PR Azure/sonic-buildimage#5991, for VOQ chassis systems the "asic_id" and "instance_name" are configured via minigraph.xml. Will these values from default_config.json be preferred over what is configured through minigraph?

Both are still valid. If the platform has configuration in default_config.json it has to be static. If it is present in both files then default_config.json will override the value configured via minigraph.xml, which is fine.

The example above is probably incorrect once VoQ config using minigraph.xml is complete (PR5991), however asic_id is still required in this file as it is platform-dependent.

For VOQ chassis systems, we get "start_chassis_db" and "chassis_db_address" from chassisdb.conf from device specific directory (for example: https://github.com/Azure/sonic-buildimage/blob/master/device/arista/x86_64-arista_7800_sup/chassisdb.conf). These are needed before the database service is started in supervisor card. Having these in config_db is of no use. Also, if these are not used in multi-asic we should not have these in default_config.json.

Right. Actually in the above example only asic_id is required as it is tied to each asic physically on the hardware. Other configurations can easily be set using minigraph.xml

mlorrillere · 2021-03-01T17:56:17Z

Please add unit test for this

I've updated config_test.py to get coverage for default_config.json. I also added similar test for init_cfg.json since coverage was missing.

sanmalho-git · 2021-03-08T16:06:20Z

@vganesan-nokia - please review.

vganesan-nokia · 2021-03-08T16:16:18Z

config/main.py

+            npu_id = device_info.get_npu_id_from_name(namespace)
+
+        # load default_config.json file from platform directory
+        default_config_file = os.path.join('/usr/share/sonic/device/', platform, npu_id, 'default_config.json')


As per PR sonic-net/sonic-buildimage#5991, for VOQ chassis systems the "asic_id" and "instance_name" are configured via minigraph.xml. Will these values from default_config.json be preferred over what is configured through minigraph?

vganesan-nokia · 2021-03-08T16:32:15Z

config/main.py

+            npu_id = device_info.get_npu_id_from_name(namespace)
+
+        # load default_config.json file from platform directory
+        default_config_file = os.path.join('/usr/share/sonic/device/', platform, npu_id, 'default_config.json')


For VOQ chassis systems, we get "start_chassis_db" and "chassis_db_address" from chassisdb.conf from device specific directory (for example: https://github.com/Azure/sonic-buildimage/blob/master/device/arista/x86_64-arista_7800_sup/chassisdb.conf). These are needed before the database service is started in supervisor card. Having these in config_db is of no use. Also, if these are not used in multi-asic we should not have these in default_config.json.

mlorrillere · 2021-03-23T23:44:45Z

Hi @arlakshm and @vganesan-nokia ,

Can you approve this review if there is no further comments?

This change add support for a per-device/per-npu default_config.json file that contains platform-specific or hwsku-specific configuration to be used when generating config_db*.json.

arlakshm · 2021-05-12T23:49:12Z

@mlorrillere, @vganesan-nokia This PR sonic-net/sonic-buildimage#7513 adds support to populate the DEVICE_METADATA.asic_id from the Pci_id present in asic.conf. Do we still need default_config.json

mlorrillere · 2021-05-13T00:00:47Z

@mlorrillere, @vganesan-nokia This PR Azure/sonic-buildimage#7513 adds support to populate the DEVICE_METADATA.asic_id from the Pci_id present in asic.conf. Do we still need default_config.json

I found out about this PR yesterday. We might not need default_config.json after all if asic.conf includes the PCI addresses of the ASICs.

mlorrillere force-pushed the pr-default-config branch from deb489e to cf46687 Compare December 9, 2020 18:16

mlorrillere force-pushed the pr-default-config branch from cf46687 to 028ee25 Compare January 7, 2021 21:33

mlorrillere force-pushed the pr-default-config branch from 028ee25 to 612d39a Compare January 19, 2021 22:14

arlakshm requested changes Feb 5, 2021

View reviewed changes

mlorrillere force-pushed the pr-default-config branch 4 times, most recently from ad84e94 to 00536b9 Compare February 26, 2021 23:27

mlorrillere closed this Feb 26, 2021

mlorrillere reopened this Feb 26, 2021

mlorrillere force-pushed the pr-default-config branch from 00536b9 to 338d0d1 Compare February 26, 2021 23:44

mlorrillere force-pushed the pr-default-config branch 2 times, most recently from 42fdb03 to 65d9313 Compare March 1, 2021 18:20

mlorrillere requested a review from arlakshm March 6, 2021 01:00

vganesan-nokia suggested changes Mar 8, 2021

View reviewed changes

vganesan-nokia mentioned this pull request Mar 9, 2021

VoQ configuration using minigraph.xml file sonic-net/sonic-buildimage#5991

Merged

mlorrillere force-pushed the pr-default-config branch 3 times, most recently from fea0649 to 7ccfb2a Compare March 16, 2021 17:45

mlorrillere force-pushed the pr-default-config branch from 7ccfb2a to 546e782 Compare March 22, 2021 20:35

vganesan-nokia previously approved these changes Mar 24, 2021

View reviewed changes

mlorrillere dismissed vganesan-nokia’s stale review via 6cb68c2 April 13, 2021 17:21

mlorrillere force-pushed the pr-default-config branch from 546e782 to 6cb68c2 Compare April 13, 2021 17:21

[load_minigraph]: Add support to load default_config.json file

1f7de72

This change add support for a per-device/per-npu default_config.json file that contains platform-specific or hwsku-specific configuration to be used when generating config_db*.json.

mlorrillere force-pushed the pr-default-config branch from 6cb68c2 to 1f7de72 Compare April 27, 2021 17:28

arlakshm approved these changes Apr 28, 2021

View reviewed changes

vganesan-nokia approved these changes May 12, 2021

View reviewed changes

mlorrillere closed this May 18, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[load_minigraph]: Add support to load default_config.json file #1228

[load_minigraph]: Add support to load default_config.json file #1228

mlorrillere commented Nov 11, 2020 •

edited

Loading

anshuv-mfst commented Feb 3, 2021

arlakshm left a comment

arlakshm Feb 5, 2021

mlorrillere Feb 19, 2021 •

edited

Loading

arlakshm Feb 26, 2021

mlorrillere Feb 26, 2021

arlakshm Feb 26, 2021

mlorrillere Feb 27, 2021

arlakshm Feb 5, 2021

mlorrillere Feb 20, 2021

vganesan-nokia Mar 8, 2021

vganesan-nokia Mar 8, 2021

mlorrillere Mar 8, 2021

mlorrillere Mar 8, 2021

mlorrillere commented Mar 1, 2021

sanmalho-git commented Mar 8, 2021

vganesan-nokia Mar 8, 2021

vganesan-nokia Mar 8, 2021

mlorrillere commented Mar 23, 2021

arlakshm commented May 12, 2021

mlorrillere commented May 13, 2021

		@@ -294,6 +294,22 @@ def _get_device_type():

		return device_type

		def _get_hwsku(config_db):

[load_minigraph]: Add support to load default_config.json file #1228

[load_minigraph]: Add support to load default_config.json file #1228

Conversation

mlorrillere commented Nov 11, 2020 • edited Loading

anshuv-mfst commented Feb 3, 2021

arlakshm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mlorrillere Feb 19, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mlorrillere commented Mar 1, 2021

sanmalho-git commented Mar 8, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mlorrillere commented Mar 23, 2021

arlakshm commented May 12, 2021

mlorrillere commented May 13, 2021

mlorrillere commented Nov 11, 2020 •

edited

Loading

mlorrillere Feb 19, 2021 •

edited

Loading