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

Make Stratum-bfrt QoS configurable with singleton port IDs #808

Merged
merged 8 commits into from
Aug 13, 2021

Conversation

pudelkoM
Copy link
Member

@pudelkoM pudelkoM commented Aug 10, 2021

This PR add the option to use SingletonPort IDs instead of sdk ports in the QoS config for Tofino.

TODO:

  • test on HW

@codecov
Copy link

codecov bot commented Aug 10, 2021

Codecov Report

Merging #808 (7e0e6f6) into main (3bcaec5) will decrease coverage by 0.03%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #808      +/-   ##
==========================================
- Coverage   78.63%   78.60%   -0.04%     
==========================================
  Files         334      334              
  Lines       30006    30019      +13     
==========================================
  Hits        23595    23595              
- Misses       6411     6424      +13     
Impacted Files Coverage Δ
stratum/hal/lib/barefoot/bf_chassis_manager.cc 72.65% <0.00%> (-1.04%) ⬇️

@pudelkoM
Copy link
Member Author

The latest commit is an experiment with adding a common TofinoPort. Has the drawback that the oneof does not directly embed in the parent message anymore, thus needing an extra port { sdk_port: 1 } wrapper in the text config.

ccascone
ccascone previously approved these changes Aug 12, 2021
Copy link
Member

@ccascone ccascone left a comment

Choose a reason for hiding this comment

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

The proto changes look good to me

@pudelkoM
Copy link
Member Author

pudelkoM commented Aug 12, 2021

The common tofino port message did not really save any code once I found a way to shorten the namespace for the port type case, so I reverted it back to direct embed, which makes the config itself easier to read and write.

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.

None yet

3 participants