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

[intfmgr] Fix OA crash issue due to link local configurations #1195

Merged
merged 2 commits into from
Aug 18, 2020

Conversation

zhenggen-xu
Copy link
Collaborator

@zhenggen-xu zhenggen-xu commented Feb 12, 2020

[intfmgr] Fix OA crash issue due to link local configurations

Signed-off-by: Zhenggen Xu [email protected]

This should be backported to 201811 and 201911.

What I did
Fix OA crash issue due to link local configurations.
In this PR, it will not send the link local configurations towards AppDB as it was configured on HW by default.

Why I did it
In case we have ipv6 link local configuration in configDB, it will crash OA.

How I verified it
configuration:

    "VLAN_INTERFACE": {
        "Vlan100|10.11.10.1/26": {
            "family": "IPv4", 
            "scope": "global"
        }, 
        "Vlan100|2000:11:10::1/64": {
            "family": "IPv6", 
            "scope": "global"
        }, 
        "Vlan100|fe80::1/10": {
            "family": "IPv6", 
            "scope": "local"
        }, 
        "Vlan777|10.11.77.1/26": {
            "family": "IPv4", 
            "scope": "global"
        }, 
        "Vlan777|2000:11:77::1/64": {
            "family": "IPv6", 
            "scope": "global"
        }, 
        "Vlan777|fe80::1/10": {
            "family": "IPv6", 
            "scope": "local"
        }
    }, 

Before fix:

Feb 12 00:15:56.615576 sonic ERR swss#orchagent: :- meta_sai_validate_route_entry: object key SAI_OBJECT_TYPE_ROUTE_ENTRY:{"dest":"fe80::/10","switch_id":"oid:0x21000000000000","vr":"oid:0x300000000
0022"} already exists
Feb 12 00:15:56.615682 sonic ERR swss#orchagent: :- addSubnetRoute: Failed to create subnet route to fe80::/10 from Vlan100, rv:-6
Feb 12 00:15:56.616068 sonic INFO swss#supervisord: orchagent terminate called after throwing an instance of '
Feb 12 00:15:56.621355 sonic INFO swss#supervisord: orchagent std::runtime_error'
Feb 12 00:15:56.621534 sonic INFO swss#supervisord: orchagent   what():  Failed to create subnet route.
Feb 12 00:15:56.621625 sonic INFO swss#supervisord: orchagent 
admin@sonic:~$ redis-cli 
127.0.0.1:6379> keys INT*
1) "INTF_TABLE:Vlan100:10.11.10.1/26"
2) "INTF_TABLE:Vlan100:2000:11:10::1/64"
3) "INTF_TABLE:Vlan777:2000:11:77::1/64"
4) "INTF_TABLE:Vlan100:fe80::1/10"
5) "INTF_TABLE:lo:2000:0:1::1/128"
6) "INTF_TABLE:Vlan777:fe80::1/10"
7) "INTF_TABLE:lo:10.0.1.1/32"
8) "INTF_TABLE_KEY_SET"
9) "INTF_TABLE:Vlan777:10.11.77.1/26"
127.0.0.1:6379> 

After fix:
No crash was seen.

127.0.0.1:6379> keys INTF*
 1) "INTF_TABLE:Vlan100:10.11.10.1/26"
 2) "INTF_TABLE:Vlan100:2000:11:10::1/64"
 3) "INTF_TABLE:Vlan777:2000:11:77::1/64"
 4) "INTF_TABLE:Ethernet116:10.2.1.1/31"
 5) "INTF_TABLE:lo:2000:0:1::1/128"
 6) "INTF_TABLE:Ethernet112:2000:1:1::2/126"
 7) "INTF_TABLE:lo:10.0.1.1/32"
 8) "INTF_TABLE:Vlan777:10.11.77.1/26"
 9) "INTF_TABLE:Ethernet116:2000:2:1::2/126"
10) "INTF_TABLE:Ethernet112:10.1.1.1/31"

Details if related

@prsunny
Copy link
Collaborator

prsunny commented Feb 12, 2020

Doesn't this fail in kernel as well as it would already have a LL address?

@zhenggen-xu
Copy link
Collaborator Author

zhenggen-xu commented Feb 13, 2020

Doesn't this fail in kernel as well as it would already have a LL address?

Thanks for reviewing. We need the LL configuration goes to kernel for the interfaces and it won't fail since it is just configuring the interfaces. The OA crash issue was due to fe80::/10 on HW we added by default, and we don't want to do that again in OA by LL configurations.

@prsunny
Copy link
Collaborator

prsunny commented Feb 18, 2020

@zhenggen-xu , my question is for the interface in kernel, it would already have a link-local address right? So are you trying to configure the same thing? or is it a different link-local address?

@zhenggen-xu
Copy link
Collaborator Author

@zhenggen-xu , my question is for the interface in kernel, it would already have a link-local address right? So are you trying to configure the same thing? or is it a different link-local address?

The link local generated by kernel is already there. We are configuring additional link local address like this "fe80::1/10" for other purpose. Kernel has no issues to handle multiple link local addresses on the same interface.

sudo ifconfig Vlan100
Vlan100: flags=4163<UP,BROADCAST,RUNNING,MULTICAST>  mtu 1500
        inet 10.11.10.1  netmask 255.255.255.192  broadcast 10.11.10.63
        inet6 2000:11:10::1  prefixlen 64  scopeid 0x0<global>
        inet6 fe80::2e0:ecff:fe51:5c61  prefixlen 64  scopeid 0x20<link>
        inet6 fe80::1  prefixlen 10  scopeid 0x20<link>
        ether 00:e0:ec:51:5c:61  txqueuelen 1000  (Ethernet)
        RX packets 1807664  bytes 594255986 (566.7 MiB)
        RX errors 0  dropped 0  overruns 0  frame 0
        TX packets 1900896  bytes 2106398723 (1.9 GiB)
        TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0

prsunny
prsunny previously approved these changes Feb 19, 2020
@zhenggen-xu
Copy link
Collaborator Author

@prsunny Can we get this merged?

@zhenggen-xu
Copy link
Collaborator Author

retest vs please

@prsunny
Copy link
Collaborator

prsunny commented Aug 15, 2020

Yes, @zhenggen-xu , will wait for tests to be complete

@prsunny
Copy link
Collaborator

prsunny commented Aug 17, 2020

@zhenggen-xu , can you rebase to latest, not sure why some checks are not completing

@zhenggen-xu
Copy link
Collaborator Author

zhenggen-xu commented Aug 17, 2020

@zhenggen-xu , can you rebase to latest, not sure why some checks are not completing

I think that must be a different issue. My take is, if no conflicts, the CI pipeline should just pick the diff and apply to the latest code anyways, so rebase from forked repo should not be needed.

@zhenggen-xu
Copy link
Collaborator Author

retest this please

@zhenggen-xu
Copy link
Collaborator Author

@prsunny

@prsunny prsunny merged commit e3ed57b into sonic-net:master Aug 18, 2020
oleksandrivantsiv pushed a commit to oleksandrivantsiv/sonic-swss that referenced this pull request Mar 1, 2023
What I did:

Moved the SAI header to v1.8.1.
   7cd3a7ed84db3fc9cec13496a5339b6fe1888bb7 (HEAD, tag: v1.8.1, origin/v1.8) Update SAI version to V1.8.1 (sonic-net#1218)
   5913e4cdd0c9c7ae859baa2e18086327b39a94da Fix error when compiling Broadcom SAI with v1.8.0 (sonic-net#1216)
   5a98bc3c7e86c01f3cf702054f9af7c7c5ca6daf (HEAD, tag: v1.8.0, origin/master, origin/HEAD, master) Update version to 1.8.0 (sonic-net#1207)
   b3244ceceb45184ffe37da55bb9a98ef126050ce saineighbor.h: Updated SAI_NEIGHBOR_ENTRY_ATTR_ENCAP_INDEX and deprecated SAI_NEIGHBOR_ENTRY_ATTR_ENCAP_IMPOSE_INDEX (sonic-net#1202)
   8731ca6e09c7ba99b0b009e5821d80598e216756 Add source/dest/double NAPT entry available attributes (sonic-net#1194)
   f053d899feb9517f2db43ee462589a30572b5ed1 Add switch attributes for hash offset configuration. (sonic-net#1195)
   13e5cd6940f9a0da1878d00f08e5941e09f16e7f PRBS RX State Data Type (sonic-net#1179)
   9755845a06525a3c17f03e7b936a70783e8ef068 Packet header based VRF classification (sonic-net#1185)
   2369ecb59fff1a5cae948d41eea06bf8b71330b2 SAI versioning (sonic-net#1183)
   744279839c176e68b19734657975e3f5ec6f1a32 Replaced SAI_SWITCH_ATTR_MACSEC_OBJECT_ID with SAI_SWITCH_ATTR_MACSEC_OBJECT_LIST (sonic-net#1199)
   584c724864fe565357e82d097ddcc7363bddefac [CI] Set up CI&PR with Azure Pipelines (sonic-net#1200)
   08192237963174cc60edae9b4812a39c43b291fd Add attribute to query available packet DMA pool size (sonic-net#1198)
   f092ef1e3ce695fc3f9552721025695312b961a2 Add IPv6 flow label hash attribute. (sonic-net#1192)
   cbc9562bb7a8f2c3a79702b99be55f3b3afa6957 Override VRF (sonic-net#1186)
   1eb35afdb2146baf40e6c2b8f2f8bfe99075eaee Add SAI_SWITCH_ATTR_SWITCH_HARDWARE_INFO format for GB MDIO sysfs access   (sonic-net#1171)
   b2d4c9a57c7f00b2632c35ca5eb3dd6480f7916a Switch scoped tunnel attributes (sonic-net#1173)
   96adc95bf8316e1905143d9ecd21f32a43e80d7f Enhancements for MPLS support (sonic-net#1181)
   3dcf1f2028da4060b345ad78e8a0c87d225bf5d0 Support for ACL extensions in metadata (sonic-net#1178)
   24076be95b871e8f82ecaeb908cad951dc68896c [meta] Add support for allow empty list tag (sonic-net#1190)
   a2b3344cdde0bf5a4f8e98e1c676a658c0c615b0 spell check fixes (sonic-net#1189)
   bf4271bab6e8884bd96050bcba44e3134adaaec3 Do not call sai_metadata_sai get APIs before checking if they are allocated (sonic-net#1182)
   5d5784dc3dbfc713c92ae7d2c472680b837bb002 [macsec]: Separate XPN configuration attribute from read-only attribute (sonic-net#1169)
   6d5a9bf5ad17cb82621cabbe2449524320930606 [macsec]: add SAI_MACSEC_ATTR_SUPPORTED_CIPHER_SUITE_LIST (sonic-net#1172)
   e72c8f3a0cc543cb228554be82c97a63db917740 [meta] Print each tool version in Makefile (sonic-net#1177)
   8f19677da88c7494d563ef7c5acb0529ecbd0b6e [meta] Add check for START, END and RANBE_BASE enums (sonic-net#1175)
   24ad7906f145930b2e25682b6248909289d39e72 [meta] Create sai_switch_pointers_t struct (sonic-net#1174)
   4f5f84df3fcd0e146707df41d3e2837c48f7c760 Tunnel loopback packet action as resource (sonic-net#1163)
   8a0e82c57aa0e22e696158735516904e7dc14052 [meta] Add create only oid attribute check on switch object (sonic-net#1170)
   14cf50772e478551920963ecf11f4fd019a0c106 Remove obsolete stub folder (sonic-net#1168)
   f14f406340e4f5f1b1d674f6fdd5fd861a54c877 [meta] Use safer calloc for integer overflow check (sonic-net#1166)

Also this PR include changes of this sonic-net#815

SAI commit b2d4c9a57c7f00b2632c35ca5eb3dd6480f7916a Switch scoped tunnel attributes (sonic-net#1173) needed change in sai_redis_switch.cpp and sai_vs_switch.cpp for compilation.

How I verify:

Verify Build is fine of libsairedis*.deb, syncd*.deb, swss*.deb

Co-authored-by: Ann Pokora <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants