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

[pfcwd] Convert polling interval from ms to us in LUA scripts #1908

Merged
merged 1 commit into from
Sep 16, 2021

Conversation

volodymyrsamotiy
Copy link
Collaborator

Signed-off-by: Volodymyr Samotiy volodymyrs@nvidia.com

What I did
Converted polling interval from milliseconds to microseconds in PFCWD LUA scripts.

Why I did it
PFCWD storm detection and restoration LUA scripts require values in microseconds.
Due to recent changes polling interval is now passed in milliseconds by "FlexCounter".

So need to convert values to microseconds (as it was before) in order to make PFCWD working,

How I verified it
Ran PFCWD tests from sonic-mgmt.

Details if related
N/A

PFCWD storm detection and restoration LUA scripts require values in microseconds.
Due to recent changes polling interval is now passed in milliseconds by "FlexCounter".
* sonic-net/sonic-sairedis#878

Signed-off-by: Volodymyr Samotiy <volodymyrs@nvidia.com>
@volodymyrsamotiy
Copy link
Collaborator Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

qiluo-msft pushed a commit that referenced this pull request Sep 16, 2021
…#1909)

**What I did**
Converted polling interval from milliseconds to microseconds in PFCWD LUA scripts.
Same as PR #1908 but created directly against ```202012``` release branch

**Why I did it**
PFCWD storm detection and restoration LUA scripts require values in microseconds.
Due to recent changes polling interval is now passed in milliseconds by "FlexCounter".
* sonic-net/sonic-sairedis#878

So need to convert values to microseconds (as it was before) in order to make PFCWD working,

**How I verified it**
Ran PFCWD tests from sonic-mgmt.
@qiluo-msft qiluo-msft merged commit 57d21e7 into sonic-net:master Sep 16, 2021
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-swss that referenced this pull request Oct 5, 2021
…net#1908)

**What I did**
Converted polling interval from milliseconds to microseconds in PFCWD LUA scripts.

**Why I did it**
PFCWD storm detection and restoration LUA scripts require values in microseconds.
Due to recent changes polling interval is now passed in milliseconds by "FlexCounter".
* sonic-net/sonic-sairedis#878

So need to convert values to microseconds (as it was before) in order to make PFCWD working,

**How I verified it**
Ran PFCWD tests from sonic-mgmt.
@dgsudharsan
Copy link
Collaborator

Required for 202106

judyjoseph pushed a commit that referenced this pull request Oct 14, 2021
**What I did**
Converted polling interval from milliseconds to microseconds in PFCWD LUA scripts.

**Why I did it**
PFCWD storm detection and restoration LUA scripts require values in microseconds.
Due to recent changes polling interval is now passed in milliseconds by "FlexCounter".
* sonic-net/sonic-sairedis#878

So need to convert values to microseconds (as it was before) in order to make PFCWD working,

**How I verified it**
Ran PFCWD tests from sonic-mgmt.
EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
…onic-net#1923)

#### What I did
Fixing issue sonic-net#1908 

#### How I did it
- When the given patch produces empty-table, reject it since ConfigDb cannot show empty tables. Achieved that by:
  - Adding a validation step before patch-sorting
- The patch-sorter should not produce steps which result in empty-tables because it is not possible in ConfigDb, we should instead delete the whole table. Achieved that by:
  - Adding a new validator to reject moves producing empty tables
  - No need to add a generator for deleting the whole table, current generators take care of that. They do by the following:
    - The move to empty a table is rejected by `NoEmptyTableMoveValidator`
    - The previous rejected move is used to generate moves using `UpperLevelMoveExtender` which produces either
      - Remove move for parent -- This is good, we are removing the table
      - Replace move for parent -- This later on will be extended to a Delete move using `DeleteInsteadOfReplaceMoveExtender`

The generators/validators are documented in the `PatchSorter.py`, check the documentation out.

#### How to verify it
unit-test

#### Previous command output (if the output of a command-line utility has changed)
```sh
admin@vlab-01:~$ sudo config apply-patch empty-a-table.json-patch -v
Patch Applier: Patch application starting.
Patch Applier: Patch: [{"op": "replace", "path": "/DEVICE_METADATA", "value": {}}]
Patch Applier: Validating patch is not making changes to tables without YANG models.
Patch Applier: Getting current config db.
Patch Applier: Simulating the target full config after applying the patch.
Patch Applier: Validating target config according to YANG models.
... sonig-yang-mgmt logs
Patch Applier: Sorting patch updates.
... sonig-yang-mgmt logs
Patch Applier: The patch was sorted into 11 changes:
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/bgp_asn"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/buffer_model"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/default_bgp_status"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/default_pfcwd_status"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/deployment_id"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/docker_routing_config_mode"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/hostname"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/hwsku"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/mac"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/platform"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost"}]
Patch Applier: Applying changes in order.
Patch Applier: Verifying patch updates are reflected on ConfigDB.
Failed to apply patch
Usage: config apply-patch [OPTIONS] PATCH_FILE_PATH
Try "config apply-patch -h" for help.

Error: After applying patch to config, there are still some parts not updated
admin@vlab-01:~$ 
```

The above error occurs because post the update, the whole `DEVICE_METADATA` table is deleted, not just showing as empty i.e. `"DEVICE_METADATA": {}`

#### New command output (if the output of a command-line utility has changed)
```
admin@vlab-01:~$ sudo config apply-patch empty-a-table.json-patch -v
Patch Applier: Patch application starting.
Patch Applier: Patch: [{"op": "replace", "path": "/DEVICE_METADATA", "value": {}}]
Patch Applier: Validating patch is not making changes to tables without YANG models.
Patch Applier: Getting current config db.
Patch Applier: Simulating the target full config after applying the patch.
Patch Applier: Validating target config does not have empty tables, since they do not show up in ConfigDb.
Failed to apply patch
Usage: config apply-patch [OPTIONS] PATCH_FILE_PATH
Try "config apply-patch -h" for help.

Error: Given patch is not valid because it will result in empty tables which is not allowed in ConfigDb. Table: DEVICE_METADATA
admin@vlab-01:~$ 
```
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.

7 participants