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

[aclshow] output only counters per table/rule #442

Merged
merged 3 commits into from
Feb 11, 2019

Conversation

romankachur-mlnx
Copy link
Contributor

@romankachur-mlnx romankachur-mlnx commented Jan 23, 2019

- What I did
Modified aclshow utility.

  1. Removed three unneeded columns (Type / Priority / Action) from the output
    (as this information is duplicated by acl-loader utility).
  2. Removed unneeded arguments (details and ports) passed to the utility.
  3. Added handling of arguments (rules and tables), which were accepted by the utility but not handled
    (now the output is filtered based on the passed arguments).

- How I did it
Modified scripts/aclshow
- How to verify it
Run aclshow utility with all possible options:
aclshow
aclshow -h
aclshow -a -vv
aclshow -t <existing_table,unexisting_table> -a
aclshow -r <existing_rule,unexisting_rule> -a
- Previous command output (if the output of a command-line utility has changed)

aclshow   -a -t DATAACL -r RULE_2,RULE_3,DEFAULT_RULE -vv
Reading ACL info...
ACL Tables to show: 4
ACL Rules to show: 14
ACL Counters found: 14

RULE NAME     TABLE NAME    TYPE      PRIO  ACTION      PACKETS COUNT    BYTES COUNT
------------  ------------  ------  ------  --------  ---------------  -------------
RULE_1        DATAACL       L3        9999  FORWARD                 0              0
RULE_2        DATAACL       L3        9998  FORWARD                 0              0
RULE_3        DATAACL       L3        9997  FORWARD                 0              0
RULE_4        DATAACL       L3        9996  FORWARD                 0              0
RULE_5        DATAACL       L3        9995  FORWARD                 0              0
RULE_6        DATAACL       L3        9994  FORWARD                 0              0
RULE_7        DATAACL       L3        9993  DROP                    0              0
RULE_8        DATAACL       L3        9992  FORWARD                 0              0
RULE_9        DATAACL       L3        9991  FORWARD                 0              0
RULE_10       DATAACL       L3        9990  FORWARD                 0              0
RULE_11       DATAACL       L3        9989  FORWARD                 0              0
RULE_12       DATAACL       L3        9988  FORWARD                 0              0
RULE_13       DATAACL       L3        9987  FORWARD                 0              0
DEFAULT_RULE  DATAACL       L3           1  DROP                 5157         402246

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

aclshow   -a -t DATAACL -r RULE_2,RULE_3,DEFAULT_RULE -vv
Reading ACL info...
Total number of ACL Tables: 4
Total number of ACL Rules: 14

RULE NAME     TABLE NAME      PACKETS COUNT    BYTES COUNT
------------  ------------  ---------------  -------------
DEFAULT_RULE  DATAACL                  5177         403806
RULE_2        DATAACL                     0              0
RULE_3        DATAACL                     0              0

-->

@romankachur-mlnx
Copy link
Contributor Author

retest this please

Fixed output  "..ACL Tables: %d"
@romankachur-mlnx romankachur-mlnx changed the title [aclshow] improvement [aclshow] shorten command output. remove unneeded arguments. add handler for tables and rules Jan 24, 2019
@liat-grozovik
Copy link
Collaborator

@romankachur-mlnx please fix commit typo 'acs- to acl-'
The output of the new aclshow only provides counters' information per table/per rule. The info about the rule/table should be retrieved from 'acl-loader show'.
@lguohan now that the aclshow only handles ACL counters should we change the name?

@romankachur-mlnx it might be a good idea to send in the mailing list that the UI is going to change. we dont know if SONiC users has any automation based on it which is not exposed and the community need some time to get ready for it.

@romankachur-mlnx romankachur-mlnx changed the title [aclshow] shorten command output. remove unneeded arguments. add handler for tables and rules [aclshow] output only counters per table/rule Jan 24, 2019
@jleveque
Copy link
Contributor

now that the aclshow only handles ACL counters should we change the name?

Maybe we should consider combining acl-loader and aclshow logic into one utility, "aclutil"?

@romankachur-mlnx
Copy link
Contributor Author

retest this please

Fixed a case when counters, once reset to 0, are not showed untill they are increased ('>0').
Now counters, if reset to 0, are shown as 0 ('>=0'.)
@liat-grozovik
Copy link
Collaborator

now that the aclshow only handles ACL counters should we change the name?

Maybe we should consider combining acl-loader and aclshow logic into one utility, "aclutil"?

Following offline discussion it was agreed to have this fix regardless on the suggestion raised.
The idea is to have an existing tool working (fixes few detected issues) and then work on new UI for ACL show and config. From my understanding such UI will be defined as part of the everflow improvements.

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