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

[BUG] GCU Field validation for Interface fields fec,speed fails for Multi-ASIC platform #3444

Open
okaravasi opened this issue Jul 24, 2024 · 3 comments
Assignees

Comments

@okaravasi
Copy link

Description

GCU Field validation for interface fields fec and speed is not properly done with as a result to result with not-allowed configuartion.
The check is applied in CLI command thus prevents the faulty config, while in config apply-patch check fails and faulty value is allowed to be configured.

Analysis of Failure/Debugging

Based on debugging it seems that there is a failure in function read_statedb_entry for multi-asic duthost:

def read_statedb_entry(table, key, field):

The code results there in below order:

def port_config_update_validator
|
|___ def validate_field
|
|
___ def read_statedb_entry

The issue is that statedb read is failed and function read_statedb_entry returns empty for supported_fecs, thus the code in _validate_field sets as supported_fecs_list the DEFAULT_SUPPORTED_FECS_LIST. However the DEFAULT_SUPPORTED_FECS_LIST include the value 'fc' that is not supported for the platform. This results to accept the value 'fc; when applying patch and eventually the confoig change to erronneously succeed.

The empty value from the read in statedb might indicate an attempt to read wrong namespace, please see below the manual fetches from database for the supported_fec values. You will see the success values returned when using correct namespace (asic0 for Ethernet0) while getting empty value "" when erronneously trying to fetch value from namespace localhost.

$ sudo ip netns exec asic0 sonic-db-cli STATE_DB hget "PORT_TABLE|Ethernet0" supported_fecs
rs
admin@ixre-egl-board41:~$ sonic-db-cli STATE_DB hget "PORT_TABLE|Ethernet0" supported_fecs

admin@ixre-egl-board41:~$

Steps to reproduce the issue

  1. Apply below steps in multi-ASIC duthost that does not support fec: fc
  2. Create a file fec_fc.json with below contents:
[
    {
        "op": "add",
        "path": "/PORT/Ethernet0/fec",
        "value": "fc"
    }
]
  1. sudo config apply-patch fec_fc.json

Describe the results you received

$ sudo config apply-patch fec.json
Patch Applier: asic0: Patch application starting.
Patch Applier: asic0: Patch: [{"op": "add", "path": "/PORT/Ethernet0/fec", "value": "fc"}]
Patch Applier: asic0 getting current config db.
Patch Applier: asic0: simulating the target full config after applying the patch.
Patch Applier: asic0: validating all JsonPatch operations are permitted on the specified fields
Patch Applier: asic0: validating target config does not have empty tables,
                            since they do not show up in ConfigDb.
Patch Applier: asic0: sorting patch updates.
Patch Applier: The asic0 patch was converted into 1 change:
Patch Applier: asic0: applying 1 change in order:
Patch Applier:   * [{"op": "add", "path": "/PORT/Ethernet0/fec", "value": "fc"}]
Patch Applier: asic0: verifying patch updates are reflected on ConfigDB.
Patch Applier: asic0 patch application completed.
Patch applied successfully.
$ show int status Ethernet0
  Interface                    Lanes    Speed    MTU    FEC        Alias            Vlan    Oper    Admin                                             Type    Asym PFC
-----------  -----------------------  -------  -----  -----  -----------  --------------  ------  -------  -----------------------------------------------  ----------
  Ethernet0  72,73,74,75,76,77,78,79     400G   9100     fc  Ethernet1/1  PortChannel101      up       up  QSFP-DD Double Density 8X Pluggable Transceiver         off
$


Describe the results you expected

Apply-patch should have failed as value "fec" is not in supported values for the duthost.

Please check the output when trying to apply the same config change via CLI:

$ sudo config interface -n asic0 fec Ethernet0 fc
fec fc is not in ['rs']

Additional information you deem important (e.g. issue happens only occasionally)

Output of show version

$ show version

SONiC Software Version: SONiC.HEAD.776732-nokia-master-3ec6570e3
SONiC OS Version: 12
Distribution: Debian 12.6
Kernel: 6.1.0-11-2-amd64
Build commit: 3ec6570e3
Build date: Tue Jul 23 05:07:09 UTC 2024
Built by: gitlab-runner@sonic-build-server04

Platform: x86_64-nokia_ixr7250e_36x400g-r0
HwSKU: Nokia-IXR7250E-36x400G
ASIC: broadcom
ASIC Count: 2
Serial Number: EAG2-02-143
Model Number: N/A
Hardware Revision: 56
Uptime: 16:24:47 up  8:10,  1 user,  load average: 1.52, 1.54, 1.63
Date: Wed 24 Jul 2024 16:24:47

Docker images:
REPOSITORY                    TAG                                  IMAGE ID       SIZE
docker-macsec                 latest                               9e29cb1c37d4   407MB
docker-orchagent              HEAD.776732-nokia-master-3ec6570e3   a2b9868a3489   417MB
docker-orchagent              latest                               a2b9868a3489   417MB
docker-teamd                  HEAD.776732-nokia-master-3ec6570e3   782ce07ee61f   404MB
docker-teamd                  latest                               782ce07ee61f   404MB
docker-sflow                  HEAD.776732-nokia-master-3ec6570e3   ad213de87143   405MB
docker-sflow                  latest                               ad213de87143   405MB
docker-fpm-frr                HEAD.776732-nokia-master-3ec6570e3   3bda43fbde3f   436MB
docker-fpm-frr                latest                               3bda43fbde3f   436MB
docker-nat                    HEAD.776732-nokia-master-3ec6570e3   426b5d45703a   407MB
docker-nat                    latest                               426b5d45703a   407MB
docker-dhcp-relay             latest                               c0cdefa5b8ff   385MB
docker-platform-monitor       HEAD.776732-nokia-master-3ec6570e3   6dfd76a1b485   461MB
docker-platform-monitor       latest                               6dfd76a1b485   461MB
docker-eventd                 HEAD.776732-nokia-master-3ec6570e3   698e5b6ead38   376MB
docker-eventd                 latest                               698e5b6ead38   376MB
docker-snmp                   HEAD.776732-nokia-master-3ec6570e3   29cfeaab0506   415MB
docker-snmp                   latest                               29cfeaab0506   415MB
docker-sonic-mgmt-framework   HEAD.776732-nokia-master-3ec6570e3   83fdb85a9c4a   425MB
docker-sonic-mgmt-framework   latest                               83fdb85a9c4a   425MB
docker-database               HEAD.776732-nokia-master-3ec6570e3   6a9db15a6e1c   384MB
docker-database               latest                               6a9db15a6e1c   384MB
docker-router-advertiser      HEAD.776732-nokia-master-3ec6570e3   475ae601b4b2   376MB
docker-router-advertiser      latest                               475ae601b4b2   376MB
docker-mux                    HEAD.776732-nokia-master-3ec6570e3   ad8171033b5a   387MB
docker-mux                    latest                               ad8171033b5a   387MB
docker-lldp                   HEAD.776732-nokia-master-3ec6570e3   9c1699c165a3   384MB
docker-lldp                   latest                               9c1699c165a3   384MB
docker-sonic-gnmi             HEAD.776732-nokia-master-3ec6570e3   8c46dd708fb5   460MB
docker-sonic-gnmi             latest                               8c46dd708fb5   460MB
docker-syncd-brcm-dnx         HEAD.776732-nokia-master-3ec6570e3   0f8e8b77e9ab   666MB
docker-syncd-brcm-dnx         latest                               0f8e8b77e9ab   666MB
docker-gbsyncd-broncos        HEAD.776732-nokia-master-3ec6570e3   2477fe9a9c48   411MB
docker-gbsyncd-broncos        latest                               2477fe9a9c48   411MB
docker-gbsyncd-credo          HEAD.776732-nokia-master-3ec6570e3   6187c2253b0f   384MB
docker-gbsyncd-credo          latest                               6187c2253b0f   384MB


@judyjoseph
Copy link
Contributor

@xincunli-sonic can you check this

@xincunli-sonic
Copy link
Contributor

As @prgeor pointed out, @judyjoseph would you mind help checking?

OA reads the suppoted speed and supported fec during init 
 
https://github.com/sonic-net/sonic-swss/blob/465391d1825ea3906d27a1285d2da5876f763cd7/orchagent/portsorch.cpp#L2638
https://github.com/sonic-net/sonic-swss/blob/465391d1825ea3906d27a1285d2da5876f763cd7/orchagent/portsorch.cpp#L2581
 
if the platform does not support this, then these fields will remain empty

@abdosi
Copy link
Contributor

abdosi commented Aug 28, 2024

@xincunli-sonic : This should not be applicable . Can you recheck ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

4 participants