Skip to content

Several fixes and updates for ecnconfig#2889

Merged
XuChen-MSFT merged 6 commits intosonic-net:masterfrom
rbpittman:update_ecnconfig
Jul 25, 2023
Merged

Several fixes and updates for ecnconfig#2889
XuChen-MSFT merged 6 commits intosonic-net:masterfrom
rbpittman:update_ecnconfig

Conversation

@rbpittman
Copy link
Copy Markdown
Contributor

@rbpittman rbpittman commented Jun 29, 2023 •
edited
Loading

What I did

  • Update DB entry table ref syntax.
  • Allow non-lossless queues to be queried and updated.
  • Remove entire wred_profile entry to activate a change to the SAI layer.
  • If wred_profile was the only value and was removed, delete entire key entry.
  • Check that a port is available before attempting an index access.

How I did it

How to verify it

  • Update UT to reflect new expectations and DB syntax.
  • Add new tests for lossy queues.
  • Manually execute a variety of commands to observe output.

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

ecnconfig -q 3 off

syslog:

Jun 29 16:25:43.287115 sonic NOTICE swss#orchagent: :- handleQueueTable: QUEUE|Ethernet60|3 wred_profile was configured but is not any more. Remove it
Jun 29 16:25:43.294859 sonic NOTICE swss#orchagent: :- handleQueueTable: QUEUE|Ethernet76|3 wred_profile was configured but is not any more. Remove it

sairedis.rec:

2023-06-29.16:25:43.284060|s|SAI_OBJECT_TYPE_SCHEDULER_GROUP:oid:0x170000000002c9|SAI_SCHEDULER_GROUP_ATTR_SCHEDULER_PROFILE_ID=oid:0x160000000004df
2023-06-29.16:25:43.286168|s|SAI_OBJECT_TYPE_QUEUE:oid:0x15000000000128|SAI_QUEUE_ATTR_WRED_PROFILE_ID=oid:0x130000000004e2
2023-06-29.16:25:43.287026|s|SAI_OBJECT_TYPE_SCHEDULER_GROUP:oid:0x170000000002c9|SAI_SCHEDULER_GROUP_ATTR_SCHEDULER_PROFILE_ID=oid:0x160000000004df
2023-06-29.16:25:43.287794|s|SAI_OBJECT_TYPE_QUEUE:oid:0x15000000000128|SAI_QUEUE_ATTR_WRED_PROFILE_ID=oid:0x0
2023-06-29.16:25:43.294791|s|SAI_OBJECT_TYPE_SCHEDULER_GROUP:oid:0x170000000002ed|SAI_SCHEDULER_GROUP_ATTR_SCHEDULER_PROFILE_ID=oid:0x160000000004df
2023-06-29.16:25:43.295568|s|SAI_OBJECT_TYPE_QUEUE:oid:0x15000000000168|SAI_QUEUE_ATTR_WRED_PROFILE_ID=oid:0x0

ecnconfig -q 3 on

sairedis.rec:

2023-06-29.16:27:57.892976|s|SAI_OBJECT_TYPE_SCHEDULER_GROUP:oid:0x170000000002c9|SAI_SCHEDULER_GROUP_ATTR_SCHEDULER_PROFILE_ID=oid:0x160000000004df
2023-06-29.16:27:57.894043|s|SAI_OBJECT_TYPE_QUEUE:oid:0x15000000000128|SAI_QUEUE_ATTR_WRED_PROFILE_ID=oid:0x130000000004e2
2023-06-29.16:27:57.901807|s|SAI_OBJECT_TYPE_SCHEDULER_GROUP:oid:0x170000000002ed|SAI_SCHEDULER_GROUP_ATTR_SCHEDULER_PROFILE_ID=oid:0x160000000004df
2023-06-29.16:27:57.902492|s|SAI_OBJECT_TYPE_QUEUE:oid:0x15000000000168|SAI_QUEUE_ATTR_WRED_PROFILE_ID=oid:0x130000000004e2

Updating non-lossless TCs, specifically a TC that doesn't have a queue entry and ensuring it returns to its original state when turned off:

# redis-dump -d 4 -y -k "QUEUE|Ethernet60|7"
{}# ./ecnconfig -q 7 on
# redis-dump -d 4 -y -k "QUEUE|Ethernet60|7"
{
  "QUEUE|Ethernet60|7": {
    "expireat": 1688056336.0276697,
    "ttl": -0.001,
    "type": "hash",
    "value": {
      "wred_profile": "AZURE_LOSSLESS"
    }
  }
}# ./ecnconfig -q 7 off
# redis-dump -d 4 -y -k "QUEUE|Ethernet60|7"
{}# ./ecnconfig -q 7
ECN status:
Executing queue: 7
queue 7: off
#

Generic enabled profile confirming 'on' is reported:

# ./ecnconfig -q 3
ECN status:
queue 3: on

Lossless TC 3 toggling with DB output:

# ./ecnconfig -q 3
ECN status:
queue 3: on
# redis-dump -d 4 -y -k "QUEUE|Ethernet60|3"
{
  "QUEUE|Ethernet60|3": {
    "expireat": 1688062188.101967,
    "ttl": -0.001,
    "type": "hash",
    "value": {
      "scheduler": "scheduler.1",
      "wred_profile": "AZURE_LOSSLESS"
    }
  }
# ./ecnconfig -q 3 off
# redis-dump -d 4 -y -k "QUEUE|Ethernet60|3"
{
  "QUEUE|Ethernet60|3": {
    "expireat": 1688062199.867536,
    "ttl": -0.001,
    "type": "hash",
    "value": {
      "scheduler": "scheduler.1"
    }
  }
# ./ecnconfig -q 3 on
# redis-dump -d 4 -y -k "QUEUE|Ethernet60|3"
{
  "QUEUE|Ethernet60|3": {
    "expireat": 1688062221.2048986,
    "ttl": -0.001,
    "type": "hash",
    "value": {
      "scheduler": "scheduler.1",
      "wred_profile": "AZURE_LOSSLESS"
    }
  }

@yxieca yxieca requested review from XuChen-MSFT and neethajohn June 29, 2023 22:45
@XuChen-MSFT
Copy link
Copy Markdown

XuChen-MSFT commented Jul 3, 2023

Did you checked ecnconfig status for queue 2 and queue 6, when enable gemini in minigraph?

@rbpittman
Copy link
Copy Markdown
Contributor Author

rbpittman commented Jul 5, 2023

@XuChen-MSFT
Output for queues 2,6,5 in Gemini mode:

# show pfc priority

Interface    Lossless priorities
-----------  ---------------------
Ethernet0    2,3,4,6
Ethernet4    2,3,4,6
Ethernet16   2,3,4,6
Ethernet20   2,3,4,6
Ethernet64   2,3,4,6
Ethernet68   2,3,4,6
Ethernet80   2,3,4,6
Ethernet84   2,3,4,6
Ethernet96   3,4
Ethernet104  3,4
Ethernet112  3,4
Ethernet120  3,4
Ethernet128  3,4
Ethernet136  3,4
Ethernet144  3,4
Ethernet152  3,4
Ethernet168  2,3,4,6
Ethernet180  2,3,4,6
Ethernet200  2,3,4,6
Ethernet204  2,3,4,6
Ethernet212  2,3,4,6
Ethernet216  2,3,4,6
Ethernet220  2,3,4,6
Ethernet232  2,3,4,6
Ethernet240  2,3,4,6
Ethernet244  2,3,4,6
Ethernet248  2,3,4,6
Ethernet252  2,3,4,6

# ./ecnconfig -q 2
ECN status:
queue 2: on
# ./ecnconfig -q 6
ECN status:
queue 6: on
# ./ecnconfig -q 5
ECN status:
queue 5: off

Output in non-gemini mode:

root@chu-dut:/home/cisco# show pfc priority

Interface    Lossless priorities
-----------  ---------------------
Ethernet0    3,4
Ethernet4    3,4
Ethernet16   3,4
Ethernet20   3,4
Ethernet64   3,4
Ethernet68   3,4
Ethernet80   3,4
Ethernet84   3,4
Ethernet96   3,4
Ethernet104  3,4
Ethernet112  3,4
Ethernet120  3,4
Ethernet128  3,4
Ethernet136  3,4
Ethernet144  3,4
Ethernet152  3,4
Ethernet168  3,4
Ethernet180  3,4
Ethernet200  3,4
Ethernet204  3,4
Ethernet212  3,4
Ethernet216  3,4
Ethernet220  3,4
Ethernet232  3,4
Ethernet240  3,4
Ethernet244  3,4
Ethernet248  3,4
Ethernet252  3,4

# ./ecnconfig -q 2
ECN status:
queue 2: off
# ./ecnconfig -q 6
ECN status:
queue 6: off
# ./ecnconfig -q 5
ECN status:
queue 5: off

XuChen-MSFT
XuChen-MSFT previously approved these changes Jul 5, 2023
View reviewed changes
Copy link
Copy Markdown

@XuChen-MSFT XuChen-MSFT left a comment

Choose a reason for hiding this comment

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

looks good to me

@rbpittman @XuChen-MSFT
Update DB entry table ref syntax.
b3e1978 
Allow non-lossless queues to be queried and updated.

Remove entire wred_profile entry to activate a change to the SAI layer.

If wred_profile was the only value and was removed, delete entire key entry.

Check that a port is available before attempting an index access.
rbpittman added 2 commits July 21, 2023 15:24
@rbpittman
Update mock DB syntax, fix UT.
6025c4f 
Update mock DB syntax for wred_profile entries.

Add UT support for expecting no wred profile to exist.

Fix UT expectations.
@rbpittman
Add new lossy queue tests.
c4aa385
XuChen-MSFT
XuChen-MSFT approved these changes Jul 25, 2023
View reviewed changes
Copy link
Copy Markdown

@XuChen-MSFT XuChen-MSFT left a comment

Choose a reason for hiding this comment

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

LGTM

@XuChen-MSFT XuChen-MSFT merged commit 44c1108 into sonic-net:master Jul 25, 2023
pdhruv-marvell pushed a commit to pdhruv-marvell/sonic-utilities that referenced this pull request Aug 23, 2023
@rbpittman @kevinskwang @pdhruv-marvell
Several fixes and updates for ecnconfig (sonic-net#2889)
083e536 
* Update DB entry table ref syntax.

Allow non-lossless queues to be queried and updated.

Remove entire wred_profile entry to activate a change to the SAI layer.

If wred_profile was the only value and was removed, delete entire key entry.

Check that a port is available before attempting an index access.

* Update mock DB syntax, fix UT.

Update mock DB syntax for wred_profile entries.

Add UT support for expecting no wred profile to exist.

Fix UT expectations.

* Add new lossy queue tests.

* rm comment.

* rm another comment.

---------

Co-authored-by: Kevin Wang <65380078+kevinskwang@users.noreply.github.com>
@neethajohn neethajohn mentioned this pull request Mar 4, 2024
Open
8 tasks
@StormLiangMS
Copy link
Copy Markdown
Contributor

StormLiangMS commented Mar 6, 2024

ADO:26804399

mssonicbld pushed a commit to mssonicbld/sonic-utilities that referenced this pull request Mar 6, 2024
@rbpittman @kevinskwang @mssonicbld
Several fixes and updates for ecnconfig (sonic-net#2889)
53d0c8d 
* Update DB entry table ref syntax.

Allow non-lossless queues to be queried and updated.

Remove entire wred_profile entry to activate a change to the SAI layer.

If wred_profile was the only value and was removed, delete entire key entry.

Check that a port is available before attempting an index access.

* Update mock DB syntax, fix UT.

Update mock DB syntax for wred_profile entries.

Add UT support for expecting no wred profile to exist.

Fix UT expectations.

* Add new lossy queue tests.

* rm comment.

* rm another comment.

---------

Co-authored-by: Kevin Wang <65380078+kevinskwang@users.noreply.github.com>
@mssonicbld mssonicbld mentioned this pull request Mar 6, 2024
Merged
@mssonicbld
Copy link
Copy Markdown
Collaborator

mssonicbld commented Mar 6, 2024

Cherry-pick PR to 202305: #3193

mssonicbld pushed a commit that referenced this pull request Mar 6, 2024
@rbpittman @kevinskwang @mssonicbld
Several fixes and updates for ecnconfig (#2889)
49af6df 
* Update DB entry table ref syntax.

Allow non-lossless queues to be queried and updated.

Remove entire wred_profile entry to activate a change to the SAI layer.

If wred_profile was the only value and was removed, delete entire key entry.

Check that a port is available before attempting an index access.

* Update mock DB syntax, fix UT.

Update mock DB syntax for wred_profile entries.

Add UT support for expecting no wred profile to exist.

Fix UT expectations.

* Add new lossy queue tests.

* rm comment.

* rm another comment.

---------

Co-authored-by: Kevin Wang <65380078+kevinskwang@users.noreply.github.com>
@yxieca
Copy link
Copy Markdown
Contributor

yxieca commented Aug 28, 2024

@rbpittman are you able to help creating PR for 202311 branch?

@rbpittman rbpittman deleted the update_ecnconfig branch August 28, 2024 13:00
@rbpittman
Copy link
Copy Markdown
Contributor Author

rbpittman commented Aug 28, 2024

@yxieca Since this was merged in July of 2023, it's already in 202311 natively.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@XuChen-MSFT XuChen-MSFT XuChen-MSFT approved these changes

@neethajohn neethajohn Awaiting requested review from neethajohn

Assignees

No one assigned

Labels

Approved for 202305 Branch Included in 202305 Branch Request for 202205 Branch Request for 202305 Branch

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@rbpittman @XuChen-MSFT @StormLiangMS @mssonicbld @yxieca @neethajohn @kevinskwang