[yang-models] Add YANG model for SYSTEM_PORT #12689
Conversation
|
@rlhui for awareness |
kenneth-arista
force-pushed
the
master-system-port-yang
branch
2 times, most recently
from
de3d1a6
to
2d93191
Compare
|
@arlakshm @praveen-li would you please help to review this PR? Thanks. |
@arlakshm @praveen-li kindly ping. |
| "SYSTEM_PORT_POSITIVE_CONFIG": { | ||
| "desc": "Configure SYSTEM_PORT positive config." | ||
| }, | ||
| "SYSTEM_PORT_WRONG_SPEED_CONFIG": { |
There was a problem hiding this comment.
SYSTEM_PORT_WRONG_SPEED_CONFIG -> SYSTEM_PORT_WRONG_SPEED_PATTERN
Preferred if test name has condition being tested.
| @@ -1290,6 +1290,56 @@ | |||
| "login": "local" | |||
| } | |||
| }, | |||
| "SYSTEM_PORT": { | |||
There was a problem hiding this comment.
kindly point to the source of truth for config, e.g. HLD, Orchagent processing functions etc. Thx
There was a problem hiding this comment.
Please see PortsOrch::addSystemPorts() in sonic-swss/orchagent/portsorch.cpp
|
|
||
| yang-version 1.1; | ||
|
|
||
| namespace "http://github.com/Azure/sonic-system-port"; |
| } | ||
|
|
||
| leaf asic_name { | ||
| type string; |
There was a problem hiding this comment.
Suggestion: any constraints needed ? e.g. asic[0-9].
| } | ||
|
|
||
| leaf ifname { | ||
| type string; |
There was a problem hiding this comment.
Does it need to be a leafref to PORT\Interface?
There was a problem hiding this comment.
Yes, I can use a leafref. I had it as follows previously but ran into a build problem. Let's try again.
There was a problem hiding this comment.
I encountered the same build failure; e.g.
ERROR:YANG-TEST: Exception >Leafref "/sonic-port:sonic-port/sonic-port:PORT/sonic-port:PORT_LIST/sonic-port:name" of value "Ethernet0" points to a non-existing leaf. in not empty< in /sonic/src/sonic-yang-models/tests/yang_model_tests/test_yang_model.py:208
There was a problem hiding this comment.
You need to add Ethernet0 in your json config. For example:
https://github.com/sonic-net/sonic-buildimage/blob/master/src/sonic-yang-models/tests/yang_model_tests/tests_config/lldp.json#L82-L109
There was a problem hiding this comment.
Thanks @ganglyu for the tip.
| } | ||
|
|
||
| leaf core_index { | ||
| type uint8; |
There was a problem hiding this comment.
8-bits give us flexibility to support more cores in future ASICs. Admittedly, 255 cores is not likely. If you prefer, I can constrain to maximum 8 cores as so:
type uint8 {
range 0..7;
}
| } | ||
|
|
||
| leaf core_port_index { | ||
| type uint16; |
There was a problem hiding this comment.
Yes, similar to core_index we would like flexibility to support logical indices that can exceed 8 bits.
| } | ||
|
|
||
| leaf num_voq { | ||
| type uint8; |
There was a problem hiding this comment.
I am happy to constrain this. The number of VOQs per port is typically 8 but can be more. I'll constrain this to between 1..8 for now.
kenneth-arista
force-pushed
the
master-system-port-yang
branch
from
2d93191
to
d3c0c2e
Compare
|
@praveen-li @kenneth-arista do we still have open questions for this PR? Or just need to fix build failure? Thanks. |
|
I don't have any open questions. /azpw run Azure.sonic-buildimage |
|
/azpw run Azure.sonic-buildimage |
|
/AzurePipelines run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| description "Number of VoQs associated with a port."; | ||
| } | ||
|
|
||
| leaf speed { |
There was a problem hiding this comment.
Can you add description? And what's the unit? kbps?
There was a problem hiding this comment.
A VOQ is a Virtual Output Queue. Typically one VOQ is allocated per traffic class per port. The unit is the number of queues per port. For example, 8 corresponds to 8 VOQs per port. This number is not related to the speed or a port.
There was a problem hiding this comment.
Then why do we have speed for VOQ?
There was a problem hiding this comment.
This is a YANG model for a system port and not for a VOQ. A system port has many attributes, some of which are core_index (which core the port is associated with in a multicore ASIC), number of VOQs associated with the port, and the speed of the port.
Are you asking about this description Number of VoQs associated with a port? Or are you asking about the description for leaf speed?
There was a problem hiding this comment.
I'm asking about leaf speed.
There was a problem hiding this comment.
Oops. I meant to say the units are in kbps. Max speed for future compatibility is 800Gbps.
There was a problem hiding this comment.
If unit is kbps, max speed is 800Mbps. Plese include unit in description.
There was a problem hiding this comment.
Sorry I'm not thinking straight. The listed range is 1..800000. So, units are million bits per second since 800,000 mbps == 800,000,000,000 bps.
There was a problem hiding this comment.
Ok, please add description with unit.
|
@kenneth-arista would you please update doc/Configuration.md? |
|
@kenneth-arista |
|
Hi @ganglyu, I see a number of failures in For example, |
We have fixed a related issue, would you please merge the latest code? |
Add YANG model for SYSTEM_PORT. Signed-off-by: Kenneth Cheung <kennethcheung@arista.com>
Revert leafref for ifname because of build failure.
- ifname should be a leafref - adding missing port info in json test_config - add missing description for speed attribute
kenneth-arista
force-pushed
the
master-system-port-yang
branch
from
0fe0dc9
to
9554f70
Compare
|
@kenneth-arista
|
System ports also includes Cpu0 ports, which are not defined in port_config.ini and thus the YANG was modified accordingly.
|
/azpw run Azure.sonic-buildimage |
|
/AzurePipelines run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@praveen-li are you ok to approve this PR? Thanks. |
|
Can we get this PR back ported to 202205 if needed? @arlakshm |
arlakshm
added
the
Chassis for 202205 branch
gechiang
added
the
Included in Chassis for 202205 Branch
Add YANG model for SYSTEM_PORT. Resolves sonic-net#12458 YANG model for SYSTEM_PORT in CONFIG_DB was missing. Added new YANG model and associated unit tests. Passing unit tests
[yang-models] Add YANG model for SYSTEM_PORT (sonic-net#12689)
Add YANG model for SYSTEM_PORT.
Resolves #12458
Signed-off-by: Kenneth Cheung kennethcheung@arista.com
Why I did it
YANG model for SYSTEM_PORT in CONFIG_DB was missing.
How I did it
Added new YANG model and associated unit tests.
How to verify it
Passing unit tests
Which release branch to backport (provide reason below if selected)