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

[multi-asic]Fix minigraph_png template to generate all DeviceInterfaceLink #5538

Merged
merged 2 commits into from
May 2, 2022

Conversation

SuvarnaMeenakshi
Copy link
Contributor

@SuvarnaMeenakshi SuvarnaMeenakshi commented Apr 20, 2022

Signed-off-by: Suvarna Meenakshi sumeenak@microsoft.com

Description of PR

Summary:
Fixes #5368

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Back port request

  • 201911
  • 202012

Approach

What is the motivation for this PR?

DeviceInterfaceLink for front panel interface to ASIC interface is not getting generated for multi-asic platform.
#4420 introduced a change in minigraph_png.j2 template which caused this change.

How did you do it?

Fix png template so that DeviceInterfaceLink with front panel interface to asic interface is generated for all platforms.

How did you verify/test it?

Verified on multi-asic platform, DeviceInterfaceLink with front panel interface to asic interface mapping is generated correctly.
In multi-asic linecard minigraph will nclude all DeviceInterfaceLinks.
In multi-asic linecard, DEVICE_NEIGHBOR table in default config_db gets populated.

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

front panel interface in multi-asic platform and skip
for chassis platform.

Signed-off-by: Suvarna Meenakshi <sumeenak@microsoft.com>
@@ -76,8 +76,8 @@
{% endfor %}
{% endfor %}
{% endif %}
{% if switch_type is not defined or (switch_type != 'voq' and switch_type != 'chassis-packet') %}
Copy link
Contributor

Choose a reason for hiding this comment

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

DeviceInterfaceLink is also required for multi asic linecards, right?

Copy link
Contributor Author

@SuvarnaMeenakshi SuvarnaMeenakshi Apr 21, 2022

Choose a reason for hiding this comment

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

Right, I agree. We can remove this check completely so that for all multi-asic scenario, we see the DeviceInterfaceLink updated correctly.
There will be no change in the supervisor minigraph or config_db.
There will be no change in minigraph or config_db for single asic linecard as well.
There will be no change in namespace config (config_dbX.json) for multi-asic linecard.
The only change I see in multi-asic linecard is that, DEVICE_NEIGHBOR table in default config_db gets populated.
Sample change:

    "DEVICE_NEIGHBOR": {
        "Ethernet1/5": {
            "name": "ASIC1",
            "port": "Eth0-ASIC1"
        },
        "Ethernet1/1": {
            "name": "ASIC0",
            "port": "Eth0-ASIC0"
        },
        "Ethernet1/2": {
            "name": "ASIC0",
            "port": "Eth1-ASIC0"
        },
        "Ethernet1/6": {
            "name": "ASIC1",
            "port": "Eth1-ASIC1"
        },

In VoQ chassis multi-asic Linecard, in config_db.json for default namespace, I see that DEVICE_NEIGHBOR table gets populated, and PORT table gets populated with all ports and not just the ports with external neighbors.
These changes should not cause any functionality impact.

Copy link
Contributor

Choose a reason for hiding this comment

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

@SuvarnaMeenakshi - Sorry for not testing this out earlier.

With your changes incorporated, the generated minigraph on a multi-asic linecard does not have the DeviceInterfaceLink with ChassisInternal as true. As an example, with your changes we are missing

<DeviceLinkBase i:type="DeviceInterfaceLink">
  <ElementType>DeviceInterfaceLink</ElementType>
  <Bandwidth>400000</Bandwidth>
  <ChassisInternal>true</ChassisInternal>
  <EndDevice>ASIC0</EndDevice>
  <EndPort>Eth2-ASIC0</EndPort>
  <FlowControl>true</FlowControl>
  <StartDevice>ixre-egl-board4</StartDevice>
  <StartPort>Ethernet2</StartPort>
  <Validate>true</Validate>
</DeviceLinkBase>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, so we could go ahead and remove this check
{% if inventory_hostname not in device_conn or port_alias[loop.index - 1] in device_conn[inventory_hostname] %} or the new check that I added {% if switch_type is not defined or (switch_type != 'voq' and switch_type != 'chassis-packet') %}
completely from the template so we generate the internal links.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, we can generate the DeviceInterfaceLink for all platforms.
@sanmalho-git , can you confirm.

Copy link
Contributor

Choose a reason for hiding this comment

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

Make sense

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated diff as discussed.

@wenyiz2021
Copy link
Contributor

the check failed at macsec which was fixed by sonic-net/sonic-buildimage#10618

panel interfaces are created for all platforms.

Signed-off-by: Suvarna Meenakshi <sumeenak@microsoft.com>
@wenyiz2021 wenyiz2021 self-requested a review May 2, 2022 17:52
@SuvarnaMeenakshi SuvarnaMeenakshi merged commit 7719b6c into sonic-net:master May 2, 2022
wangxin pushed a commit that referenced this pull request May 5, 2022
…eLink (#5538)

What is the motivation for this PR?
DeviceInterfaceLink for front panel interface to ASIC interface is not getting generated for multi-asic platform.
#4420 introduced a change in minigraph_png.j2 template which caused this change.

How did you do it?
Fix png template so that DeviceInterfaceLink with front panel interface to asic interface is generated for all platforms.

How did you verify/test it?
Verified on multi-asic platform, DeviceInterfaceLink with front panel interface to asic interface mapping is generated correctly.
In multi-asic linecard minigraph will nclude all DeviceInterfaceLinks.
In multi-asic linecard, DEVICE_NEIGHBOR table in default config_db gets populated.
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.

[MASIC] [202012] [201911] ACL tests failure
5 participants