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

CPU queues on vslib #732

Merged
merged 1 commit into from
Dec 18, 2020
Merged

Conversation

PrabhuSreenivasan
Copy link
Contributor

Signed-off-by: Prabhu Sreenivasan [email protected]

Add Queues to CPU port on vslib

@@ -91,6 +91,8 @@ sai_status_t SwitchBCM56850::create_qos_queues()
CHECK_STATUS(create_qos_queues_per_port(port_id));
}

CHECK_STATUS(create_qos_queues_per_port(m_cpu_port_id));
Copy link
Collaborator

Choose a reason for hiding this comment

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

if i remember BCM56850 asic didn't have qos queues on cpu port, at least that was broadcom implementation, why this is changing right now ? this can lead to some wired stuff, also are you sure that those cpu queues are exactly the same as on port queue ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not aware of such restrictions. Most of the recent Broadcom devices support 48 CPU queues. copp.json.j2 does reference multiple CPU for different traffic.

Copy link
Collaborator

@kcudnik kcudnik Dec 9, 2020

Choose a reason for hiding this comment

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

Reason im asking, is that when VS switch is created, internally it should reflect SAI objects that are in real hardware, it should be 1 to 1 match, and last time (long time ago, sai 0.9.4 ?) when i did discovery objects on cpu port for brcm, there were no queues associated with it. Maybe brcm didn't implemented them at that time or something else.
You can actually use "/usr/bin/saidiscovery" tool in syncd package to get all objects on that specific asic to what is it right now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I tried the "/usr/bin/saidiscovery" command, I got the below error error on my image Maybe I need to try it on a community image.

root@sonic:/# /usr/bin/saidiscovery
/var/sonicbld/workspace/Build/broadcom/broadcom_sai/20-sai-build-brcm-4.2.1.3/output/x86-xgs5-deb80/saimeta/meta/saimetadata.c:79753 sai_metadata_apis_query: :- failed to query api SAI_API_BMTOR: SAI_STATUS_INVALID_PARAMETER (-5)
/var/sonicbld/workspace/Build/broadcom/broadcom_sai/20-sai-build-brcm-4.2.1.3/output/x86-xgs5-deb80/saimeta/meta/saimetadata.c:79873 sai_metadata_apis_query: :- failed to query api SAI_API_MACSEC: SAI_STATUS_NOT_IMPLEMENTED (-15)

Copy link
Contributor Author

@PrabhuSreenivasan PrabhuSreenivasan Dec 10, 2020

Choose a reason for hiding this comment

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

I checked on a real hardware(BCM56850) and its showing 32 CPU queue.

root@sonic:/home/admin# show platform summary
Platform: x86_64-accton_as5712_54x-r0
HwSKU: Accton-AS5712-54X
ASIC: broadcom
ASIC Count: 1

root@sonic:/home/admin# show queue  counters CPU
  Port    TxQ    Counter/pkts    Counter/bytes    Drop/pkts    Drop/bytes
------  -----  --------------  ---------------  -----------  ------------
   CPU    MC0               0                0            0             0
   CPU    MC1               0                0            0             0
   CPU    MC2               0                0            0             0
   CPU    MC3               0                0            0             0
   CPU    MC4               0                0            0             0
   CPU    MC5               0                0            0             0
   CPU    MC6               0                0            0             0
   CPU    MC7               0                0            0             0
   CPU    MC8               0                0            0             0
   CPU    MC9               0                0            0             0
   CPU   MC10               0                0            0             0
   CPU   MC11               0                0            0             0
   CPU   MC12               0                0            0             0
   CPU   MC13               0                0            0             0
   CPU   MC14               0                0            0             0
   CPU   MC15               0                0            0             0
   CPU   MC16               0                0            0             0
   CPU   MC17               0                0            0             0
   CPU   MC18               0                0            0             0
   CPU   MC19               0                0            0             0
   CPU   MC20               0                0            0             0
   CPU   MC21               0                0            0             0
   CPU   MC22               0                0            0             0
   CPU   MC23               0                0            0             0
   CPU   MC24               0                0            0             0
   CPU   MC25               0                0            0             0
   CPU   MC26               0                0            0             0
   CPU   MC27               0                0            0             0
   CPU   MC28               0                0            0             0
   CPU   MC29               0                0            0             0
   CPU   MC30               0                0            0             0
   CPU   MC31               0                0            0             0

root@sonic:/home/admin# show queue watermark CPU
COUNTERS_BUFFER_POOL_NAME_MAP is empty!
Egress shared pool occupancy per CPU queue
  Queue    Bytes
-------  -------
  CPU:0        0
  CPU:1        0
  CPU:2        0
  CPU:3        0
  CPU:4        0
  CPU:5        0
  CPU:6        0
  CPU:7        0
  CPU:8        0
  CPU:9        0
 CPU:10        0
 CPU:11        0
 CPU:12        0
 CPU:13        0
 CPU:14        0
 CPU:15        0
 CPU:16        0
 CPU:17        0
 CPU:18        0
 CPU:19        0
 CPU:20        0
 CPU:21        0
 CPU:22        0
 CPU:23        0
 CPU:24        0
 CPU:25        0
 CPU:26        0
 CPU:27        0
 CPU:28        0
 CPU:29        0
 CPU:30        0
 CPU:31        0

Copy link
Collaborator

Choose a reason for hiding this comment

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

Already merged, you can use saidiscovery tool to see what those queues actually look like

Copy link
Collaborator

Choose a reason for hiding this comment

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

if you could run "/usr/bin/saidiscoviery -p profile.ini -f -D" and share output i could guide you how to proceed from there

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we have any Jenkins PR image with this change?
Got the below error when I ran it after pulling the changes to my branch, Will the output of saidump help?

Feb 19 21:27:28.159701 sonic NOTICE syncd#saidiscovery: :- main: creating switch                                                          
Feb 19 21:27:28.159733 sonic ERR syncd#saidiscovery: :- create: virtual sai_status_t syncd::VendorSai::create(sai_object_type_t, sai_object_id_t*, sai_object_id_t, uint32_t, const sai_attribute_t*): api not initialized                                                          
Feb 19 21:27:28.159733 sonic ERR syncd#saidiscovery: :- create: create status: SAI_STATUS_FAILURE                                         
Feb 19 21:27:28.159775 sonic ERR syncd#saidiscovery: :- main: failed to create a switch: SAI_STATUS_FAILURE 

Copy link
Contributor Author

@PrabhuSreenivasan PrabhuSreenivasan Dec 15, 2020

Choose a reason for hiding this comment

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

On BRCM switches all CPU queues are multicast. BCM56850 has 32 multicast queues on CPU port whereas newer devices have 48 multicast queues. I have written a new function similar to create_qos_queues_per_port() to populate 32 queues on BCM56850.

Copy link
Collaborator

@kcudnik kcudnik Dec 15, 2020

Choose a reason for hiding this comment

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

at which sairedis commit did you built sairedis/saidiscovery ? at lread at this one 94fc99f ?
this is intersting since initialize is called here: https://github.com/Azure/sonic-sairedis/blob/master/saidiscovery/saidiscovery.cpp#L555
and create switch after that:
https://github.com/Azure/sonic-sairedis/blob/master/saidiscovery/saidiscovery.cpp#L581

so initialize is returning success, since it's passed to create function, interesting
can we make online call with rdp maybe to figure this out ? i wanted to run saidiscovery to see if there are any possible other attributes defined besides those that you implemented

@@ -24,6 +24,9 @@ namespace saivs

protected:

sai_status_t create_cpu_qos_queues(
Copy link
Collaborator

@kcudnik kcudnik Dec 15, 2020

Choose a reason for hiding this comment

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

since this could be the case also in other platforms maybe this could be virtual functions override and not implemented function in base class SwitchStateBase like create_qos_queues_per_port? everything other looks great

Signed-off-by: Prabhu Sreenivasan <[email protected]>
@PrabhuSreenivasan
Copy link
Contributor Author

Can you please merge this PR to help sonic-net/sonic-swss#1544 pass its test.

@kcudnik kcudnik merged commit 560c171 into sonic-net:master Dec 18, 2020
@PrabhuSreenivasan PrabhuSreenivasan deleted the cpu_queues branch December 18, 2020 16:55
lguohan pushed a commit that referenced this pull request Dec 19, 2020
Signed-off-by: Prabhu Sreenivasan <[email protected]>

Co-authored-by: Prabhu Sreenivasan <[email protected]>
pettershao-ragilenetworks pushed a commit to pettershao-ragilenetworks/sonic-sairedis that referenced this pull request Nov 18, 2022
Signed-off-by: Prabhu Sreenivasan <[email protected]>

Co-authored-by: Prabhu Sreenivasan <[email protected]>
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.

3 participants