sonic-utilities: Add support for sFlow #592

GarrickHe · 2019-07-30T23:22:18Z

What I did?

Adding CLI to configure sFlow globally or per interface
Adding CLI to show sFlow settings globally or per interface

How I did it?
Added logic into config/main.py to add sFlow related information supplied by the user through a CLI. Also added logic into show/main.py to query configDB to show sFlow related information.

How to verify it?
Manually ran the new CLIs and verified it with the corresponding show command. Also use redis-cli command to verify entries inside the configDB.

Additional Information
Please DO NOT MERGE until these PRs are merged:
sonic-net/sonic-swss-common#299
sonic-net/sonic-swss#1011
sonic-net/sonic-swss#1012
sonic-net/sonic-linux-kernel#94
sonic-net/sonic-sairedis#498

Previous command output?
N/A - added new command

New command output?

admin@sonic:~$ sudo config sflow enable
admin@sonic:~$ sudo config sflow agent-id add Ethernet0
admin@sonic:~$ sudo show sflow

SFlow Global Information:
  SFlow Admin State:         enable
  SFlow Polling Interval:    20
  SFlow AgentID:             Ethernet0

  0 Collectors configured:
admin@sonic:~$ sudo config sflow agent-id del
admin@sonic:~$ sudo show sflow

SFlow Global Information:
  SFlow Admin State:         enable
  SFlow Polling Interval:    20
  SFlow AgentID:             default

  0 Collectors configured:
admin@sonic:~$
admin@sonic:~$ sudo config sflow collector add Collector1 1.1.1.1
admin@sonic:~$ sudo config sflow collector add Collector2 1.1.1.2 --port 3243434343
Collector port number must be between 0 and 65535
admin@sonic:~$ sudo config sflow collector add Collector2 1.1.1.2 --port 7747
admin@sonic:~$ sudo config sflow collector add Collector2 1.1.1.2 --port 7743
Only 2 collectors can be configured, please delete one
admin@sonic:~$
admin@sonic:~$ sudo show sflow

SFlow Global Information:
  SFlow Admin State:         enable
  SFlow Polling Interval:    20
  SFlow AgentID:             default

  2 Collectors configured:
    Collector IP addr: 1.1.1.1   UDP port:6343
    Collector IP addr: 1.1.1.2   UDP port:7747
admin@sonic:~$
admin@sonic:~$ sudo config sflow collector del Collector3
Collector: Collector3 not configured
admin@sonic:~$ sudo config sflow collector del Collector2
admin@sonic:~$ sudo show sflow

SFlow Global Information:
  SFlow Admin State:         enable
  SFlow Polling Interval:    20
  SFlow AgentID:             default

  1 Collectors configured:
    Collector IP addr: 1.1.1.1   UDP port:6343
admin@sonic:~$
admin@sonic:~$ sudo config sflow polling-interval 4545
Polling interval must be between 0-300
admin@sonic:~$ sudo config sflow polling-interval 290
admin@sonic:~$ sudo show sflow

SFlow Global Information:
  SFlow Admin State:         enable
  SFlow Polling Interval:    290
  SFlow AgentID:             default

  1 Collectors configured:
    Collector IP addr: 1.1.1.1   UDP port:6343
admin@sonic:~$
admin@sonic:~$ sudo show sflow interface

sFlow interface configurations
+-------------+---------------+-----------------+
| Interface   | Admin State   |   Sampling Rate |
+=============+===============+=================+
| Ethernet0   | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet4   | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet8   | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet12  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet16  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet20  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet24  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet28  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet32  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet36  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet40  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet44  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet48  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet52  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet56  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet60  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet64  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet68  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet72  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet76  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet80  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet84  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet88  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet92  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet96  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet100 | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet104 | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet108 | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet112 | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet116 | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet120 | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet124 | enable        |            4000 |
+-------------+---------------+-----------------+
admin@sonic:~$
admin@sonic:~$ sudo config sflow polling-interval 4545
Polling interval must be between 0-300
admin@sonic:~$ sudo config sflow polling-interval 290
admin@sonic:~$ sudo show sflow

SFlow Global Information:
  SFlow Admin State:         enable
  SFlow Polling Interval:    290
  SFlow AgentID:             default

  1 Collectors configured:
    Collector IP addr: 1.1.1.1   UDP port:6343
admin@sonic:~$
admin@sonic:~$ sudo config sflow interface disable Ethernet0
admin@sonic:~$ sudo config sflow interface sample-rate Ethernet321 9600
Invalid interface name
admin@sonic:~$ sudo config sflow interface sample-rate Ethernet4 9600
admin@sonic:~$ sudo show sflow interface
admin@sonic:~$ sudo show sflow interface

sFlow interface configurations
+-------------+---------------+-----------------+
| Interface   | Admin State   |   Sampling Rate |
+=============+===============+=================+
| Ethernet0   | disable       |            4000 |
+-------------+---------------+-----------------+
| Ethernet4   | enable        |            9600 |
+-------------+---------------+-----------------+
| Ethernet8   | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet12  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet16  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet20  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet24  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet28  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet32  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet36  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet40  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet44  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet48  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet52  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet56  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet60  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet64  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet68  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet72  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet76  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet80  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet84  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet88  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet92  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet96  | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet100 | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet104 | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet108 | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet112 | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet116 | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet120 | enable        |            4000 |
+-------------+---------------+-----------------+
| Ethernet124 | enable        |            4000 |
+-------------+---------------+-----------------+
admin@sonic:~$

* Add configuration CLI to add/delete collector * Add configuration CLI to set sampling-rate * Add show CLI to display sFlow information Signed-off-by: Garrick He <[email protected]>

* Updated config/show commands to conform with HDL * Updated with code-review suggestions Signed-off-by: Garrick He <[email protected]>

* Added logic to query AppsDB for sample-rate * Updated CLI to remove 'global' and 'all' option * Added logic to handle 'all' as interface name Signed-off-by: Garrick He <[email protected]>

* Fixed a minor display bug in the 'show sflow interface' command where the global information will also be displayed. Signed-off-by: Garrick He <[email protected]>

* Add sflow container to the restart service list Signed-off-by: Garrick He <[email protected]>

* Add comments and some clean-ups Signed-off-by: Garrick He <[email protected]>

* Fix the sFlow related code to ensure PEP8 compliance. Signed-off-by: Garrick He <[email protected]>

* Address code review comments * Re-run PEP8 compliance check Signed-off-by: Garrick He <[email protected]>

msftclas · 2019-07-30T23:22:32Z

All CLA requirements met.

* Fix: Can't configure sFlow when daemon is disabled. * Fix: Interface admin-state are in correct * Fix: Collectors are shown in sorted (by name) order * Fix: Can add lo interface for sFlow agent-id * Fix: Can't ovewrite existing collector with the same name Signed-off-by: Garrick He <[email protected]>

GarrickHe · 2019-08-13T22:14:36Z

Closing this pull-request to fix some issues found during unit-test. Will create a new one soon.

* Fix the polling intervals to be between 5-300 and 0 to disable. Signed-off-by: Garrick He <[email protected]>

* Add sFlow container to support rollback and upgrade. Signed-off-by: Garrick He <[email protected]>

prsunny · 2019-09-28T00:14:45Z

config/main.py

+    else:
+        sflow_tbl['global']['admin_state'] = 'enable'
+
+    config_db.set_entry('SFLOW', 'global', sflow_tbl['global'])


You could just do a config_db.mod_entry for setting the admin_state field

Will change.

prsunny · 2019-09-28T00:14:59Z

config/main.py

+    else:
+        sflow_tbl['global']['admin_state'] = 'disable'
+
+    config_db.set_entry('SFLOW', 'global', sflow_tbl['global'])


Same as above

Will change.

prsunny · 2019-09-28T00:15:46Z

config/main.py

+        sflow_tbl = {'global': {'admin_state': 'disable'}}
+
+    sflow_tbl['global']['polling_interval'] = interval
+    config_db.set_entry('SFLOW', 'global', sflow_tbl['global'])


mod_entry ?

Will change.

prsunny · 2019-09-28T01:03:55Z

config/main.py

+    """Add a sFlow collector"""
+    ipaddr = ipaddr.lower()
+
+    if not is_valid_collector_info(name, ipaddr, port):


Do you see an issue with function being defined after invocation? I see that this is_valid_collector_info function is defined below and not above this.

Didn't notice a problem, but will move it up.

prsunny · 2019-09-28T01:08:14Z

config/main.py

+
+    import socket
+    try:
+        socket.inet_pton(socket.AF_INET, ip)


Please use is_ipaddress here

Will change.

prsunny · 2019-09-28T01:10:34Z

show/main.py

+    """Show sFlow interface information"""
+    show_sflow_interface(ctx.obj['db'])
+
+def sflow_appDB_connect():


What is this function? Looks different from the objective. May be sflow_sample_rate ?

We need to connect to applDB to get default sample-rate per port if the user doesn't configure them manually.

prsunny · 2019-09-28T01:19:34Z

show/main.py

+                body_info.append(sflow_session_tbl[pname]['sample_rate'])
+            else:
+                body_info.append(sflow_sampling_tbl[port_tbl[pname]['speed']])
+        else:


Ok so the session table is always present in APP_DB, right?. Then let show just read the table and display and not have a logic for individual ports. if the table is not present, don't add to the show output. What do you think?

If the user configures the sampling-rate for a port, we will add a separate session for that port with the new sampling-rate. That is why we need to figure out whether there is a session for that particular port (in configDB) and if we do, we will use that information instead of the one in appLDB.

prsunny · 2019-09-28T01:21:37Z

show/main.py

+        if 'admin_state' in sflow_session_tbl['all']:
+            all_session_admin_state = sflow_session_tbl['all']['admin_state']
+
+    if not port_tbl:


This check must be right after port_tbl = config_db.get_table('PORT')

Will change.

prsunny · 2019-09-28T01:23:28Z

show/main.py

+        global_admin_state = sflow_info['global']['admin_state']
+
+    click.echo("\nsFlow Global Information:")
+    click.echo("  sFlow Daemon State:        {}".format(global_admin_state))


This is different from the output in description. What is the intention? Prefer the one in description

SFlow Global Information: SFlow Admin State: enable

Sure, will change it back to match description.

prsunny · 2019-09-28T01:25:31Z

show/main.py

+    click.echo(tabulate(body, header, tablefmt='grid'))
+
+def show_sflow_global(config_db):
+    default_polling = 20


My thinking is CLI should not have a default but read from the APP/CONFIG DBs. Can you check if this is always present, if so, just read from DB and for some reason its missing, let it be an error.

ConfigDB only stores information configured by the user, so the default polling-interval will not be present. According to @padmanarayana, we will just print the default (20) hence that variable. The default is not part of the ConfigDB or AppDB schema hence it will not be present. It will only be in ConfigDB if the user manually it.

@prsunny I just clarified with @padmanarayana - He will make changes to the back-end to include polling-interval in the appDB and I will make changes to read that value.

* Use mod_entry instead of set_entry for configDB changes * Only query applDB for session information * Minor change to show command output to match HDL Signed-off-by: Garrick He <[email protected]>

* Change to deleting agent-id logic Signed-off-by: Garrick He <[email protected]>

* Change admin-state from enable/disable to up/down to match SONiC YANG. Signed-off-by: Garrick He <[email protected]>

prsunny · 2019-10-22T01:54:38Z

retest this please

GarrickHe · 2019-10-22T18:34:25Z

@prsunny - I ran some basic UT with the script and all was good.
@dgsudharsan ran some pytest script and he said all was good as well.

* Fix some minor alignment issues in the show sFlow global output. Signed-off-by: Garrick He <[email protected]>

prsunny · 2019-10-30T18:45:56Z

retest this please

dgsudharsan · 2019-10-31T17:01:01Z

retest this please

dgsudharsan · 2019-10-31T18:50:36Z

retest this please

GarrickHe · 2019-10-31T18:54:20Z

@prsunny @dgsudharsan - not sure if you're asking me to retest or the robot. But here's mine, notice the output is aligned now regardless of the length of the collector name:

sFlow Global Information:
  sFlow Admin State:          enable
  sFlow Polling Interval:     default
  sFlow AgentID:              default

  2 Collectors configured:
    Name: col1                IP addr: 1.1.1.1    UDP port: 4400
    Name: colllectaxy1        IP addr: 1.1.1.2    UDP port: 23456

dgsudharsan · 2019-11-01T19:13:09Z

retest this please

dgsudharsan · 2019-11-04T15:27:50Z

retest this please

prsunny · 2019-11-05T20:35:25Z

retest this please

wendani · 2020-09-01T19:54:22Z

config/main.py

+@click.argument('rate', metavar='<sample_rate>', required=True, type=int)
+@click.pass_context
+def sample_rate(ctx, ifname, rate):
+    if not interface_name_is_valid(ifname) and ifname != 'all':


No need to include key all to be valid here as sflowmgrd only respond to field admin_state under SFLOW_SESSION|all

This check is to ensure the user gives a valid interface name or all before writing to the dB.

wendani · 2020-09-01T20:09:30Z

config/main.py

+    config_db = ctx.obj['db']
+    collector_tbl = config_db.get_table('SFLOW_COLLECTOR')
+
+    if (collector_tbl and name not in collector_tbl.keys() and len(collector_tbl) == 2):


Collectors are limited to 2. Is this a limit from upper-layer hsflowd daemon?

Also confused about the output in the PR description that you were able to configure 4 collectors.

Because you were reading the output from an outdated version of the code. I changed that afterwards:

admin@sonic:~$ admin@sonic:~$ sudo config sflow collector add collect1 1.1.1.1 admin@sonic:~$ sudo config sflow collector add collect2 1.1.1.2 --port 6100 admin@sonic:~$ sudo config sflow collector add collect3 1.1.1.3 --port 6100 Only 2 collectors can be configured, please delete one

#592 (comment)

Is this a limitation from upper-layer hsflowd daemon?

GarrickHe added 8 commits June 11, 2019 15:52

sonic-utilities: Add sFlow CLIs

8913251

* Add configuration CLI to add/delete collector * Add configuration CLI to set sampling-rate * Add show CLI to display sFlow information Signed-off-by: Garrick He <[email protected]>

Update sFlow CLI

6ba881f

* Updated config/show commands to conform with HDL * Updated with code-review suggestions Signed-off-by: Garrick He <[email protected]>

Query AppsDB for sample-rate

d00dddd

* Added logic to query AppsDB for sample-rate * Updated CLI to remove 'global' and 'all' option * Added logic to handle 'all' as interface name Signed-off-by: Garrick He <[email protected]>

Fix minor display bug

0a268af

* Fixed a minor display bug in the 'show sflow interface' command where the global information will also be displayed. Signed-off-by: Garrick He <[email protected]>

Add sflow container to restart list

bb54a87

* Add sflow container to the restart service list Signed-off-by: Garrick He <[email protected]>

Code clean-up

ad4a0af

* Add comments and some clean-ups Signed-off-by: Garrick He <[email protected]>

PEP8 Compliance

54a1f80

* Fix the sFlow related code to ensure PEP8 compliance. Signed-off-by: Garrick He <[email protected]>

Address code-review comments

b5fcb07

* Address code review comments * Re-run PEP8 compliance check Signed-off-by: Garrick He <[email protected]>

jleveque added the Enhancement label Aug 1, 2019

This was referenced Aug 1, 2019

For sflow feature, SAI changes needed for sonic-vs platform sonic-net/sonic-sairedis#498

Merged

[sflow]: backport psample and act_sample drivers, needed for sonic-vs platform sonic-net/sonic-linux-kernel#94

Merged

GarrickHe closed this Aug 13, 2019

GarrickHe reopened this Aug 13, 2019

GarrickHe added 3 commits August 14, 2019 11:09

Fix: Valid polling intervals

7e04e74

* Fix the polling intervals to be between 5-300 and 0 to disable. Signed-off-by: Garrick He <[email protected]>

Merge remote-tracking branch 'origin/master' into sflow-cli

5e316ec

Add sFlow rollback and upgrade

b2e555d

* Add sFlow container to support rollback and upgrade. Signed-off-by: Garrick He <[email protected]>

lguohan assigned prsunny Sep 3, 2019

prsunny reviewed Sep 28, 2019

View reviewed changes

prsunny mentioned this pull request Oct 2, 2019

[sonic-utilities] managementVRF cli support(l3mdev) #463

Merged

GarrickHe added 3 commits October 10, 2019 15:41

fix: Code review comments

6c25969

* Use mod_entry instead of set_entry for configDB changes * Only query applDB for session information * Minor change to show command output to match HDL Signed-off-by: Garrick He <[email protected]>

fix: Minor change to deleting agent-id

0db2868

* Change to deleting agent-id logic Signed-off-by: Garrick He <[email protected]>

Change admin-state

d435dd2

* Change admin-state from enable/disable to up/down to match SONiC YANG. Signed-off-by: Garrick He <[email protected]>

prsunny approved these changes Oct 22, 2019

View reviewed changes

Fix CLI output alignment

21706ac

* Fix some minor alignment issues in the show sFlow global output. Signed-off-by: Garrick He <[email protected]>

prsunny merged commit 0e23e1c into sonic-net:master Nov 5, 2019

wendani reviewed Sep 1, 2020

View reviewed changes

sonic-utilities: Add support for sFlow #592

sonic-utilities: Add support for sFlow #592

Conversation

GarrickHe commented Jul 30, 2019 • edited Loading

msftclas commented Jul 30, 2019 • edited Loading

GarrickHe commented Aug 13, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prsunny commented Oct 22, 2019

GarrickHe commented Oct 22, 2019

prsunny commented Oct 30, 2019

dgsudharsan commented Oct 31, 2019

dgsudharsan commented Oct 31, 2019

GarrickHe commented Oct 31, 2019

dgsudharsan commented Nov 1, 2019

dgsudharsan commented Nov 4, 2019

prsunny commented Nov 5, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wendani Sep 1, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GarrickHe commented Jul 30, 2019 •

edited

Loading

msftclas commented Jul 30, 2019 •

edited

Loading

wendani Sep 1, 2020 •

edited

Loading