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

sonic-utilities: Add support for sFlow #592

Merged
merged 16 commits into from
Nov 5, 2019
273 changes: 272 additions & 1 deletion config/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import netaddr
import re
import syslog
import netifaces

import sonic_device_util
import ipaddress
Expand Down Expand Up @@ -312,6 +313,7 @@ def _restart_services():
'pmon',
'lldp',
'hostcfgd',
'sflow',
]
for service in services:
try:
Expand Down Expand Up @@ -1066,7 +1068,7 @@ def remove(ctx, interface_name, ip_addr):
ctx.fail("'interface_name' is not valid. Valid names [Ethernet/PortChannel/Vlan/Loopback]")
except ValueError:
ctx.fail("'ip_addr' is not valid.")

exists = False
if if_table:
interfaces = config_db.get_table(if_table)
Expand Down Expand Up @@ -1313,5 +1315,274 @@ def naming_mode_alias():
set_interface_naming_mode('alias')


#
# 'sflow' group ('config sflow ...')
#
@config.group()
@click.pass_context
def sflow(ctx):
"""sFlow-related configuration tasks"""
config_db = ConfigDBConnector()
config_db.connect()
ctx.obj = {'db': config_db}
pass

#
# 'sflow' command ('config sflow enable')
#
@sflow.command()
@click.pass_context
def enable(ctx):
"""Enable sFlow"""
config_db = ctx.obj['db']
sflow_tbl = config_db.get_table('SFLOW')

if not sflow_tbl:
sflow_tbl = {'global': {'admin_state': 'up'}}
else:
sflow_tbl['global']['admin_state'] = 'up'

config_db.mod_entry('SFLOW', 'global', sflow_tbl['global'])

#
# 'sflow' command ('config sflow disable')
#
@sflow.command()
@click.pass_context
def disable(ctx):
"""Disable sFlow"""
config_db = ctx.obj['db']
sflow_tbl = config_db.get_table('SFLOW')

if not sflow_tbl:
sflow_tbl = {'global': {'admin_state': 'down'}}
else:
sflow_tbl['global']['admin_state'] = 'down'

config_db.mod_entry('SFLOW', 'global', sflow_tbl['global'])

#
# 'sflow' command ('config sflow polling-interval ...')
#
@sflow.command('polling-interval')
@click.argument('interval', metavar='<polling_interval>', required=True,
type=int)
@click.pass_context
def polling_int(ctx, interval):
"""Set polling-interval for counter-sampling (0 to disable)"""
if interval not in range(5, 301) and interval != 0:
click.echo("Polling interval must be between 5-300 (0 to disable)")

config_db = ctx.obj['db']
sflow_tbl = config_db.get_table('SFLOW')

if not sflow_tbl:
sflow_tbl = {'global': {'admin_state': 'down'}}

sflow_tbl['global']['polling_interval'] = interval
config_db.mod_entry('SFLOW', 'global', sflow_tbl['global'])

def is_valid_sample_rate(rate):
return rate in range(256, 8388608 + 1)


#
# 'sflow interface' group
#
@sflow.group()
@click.pass_context
def interface(ctx):
"""Configure sFlow settings for an interface"""
pass

#
# 'sflow' command ('config sflow interface enable ...')
#
@interface.command()
@click.argument('ifname', metavar='<interface_name>', required=True, type=str)
@click.pass_context
def enable(ctx, ifname):
if not interface_name_is_valid(ifname) and ifname != 'all':
click.echo("Invalid interface name")
return

config_db = ctx.obj['db']
intf_dict = config_db.get_table('SFLOW_SESSION')

if intf_dict and ifname in intf_dict.keys():
intf_dict[ifname]['admin_state'] = 'up'
config_db.mod_entry('SFLOW_SESSION', ifname, intf_dict[ifname])
else:
config_db.mod_entry('SFLOW_SESSION', ifname, {'admin_state': 'up'})

#
# 'sflow' command ('config sflow interface disable ...')
#
@interface.command()
@click.argument('ifname', metavar='<interface_name>', required=True, type=str)
@click.pass_context
def disable(ctx, ifname):
if not interface_name_is_valid(ifname) and ifname != 'all':
click.echo("Invalid interface name")
return

config_db = ctx.obj['db']
intf_dict = config_db.get_table('SFLOW_SESSION')

if intf_dict and ifname in intf_dict.keys():
intf_dict[ifname]['admin_state'] = 'down'
config_db.mod_entry('SFLOW_SESSION', ifname, intf_dict[ifname])
else:
config_db.mod_entry('SFLOW_SESSION', ifname,
{'admin_state': 'down'})

#
# 'sflow' command ('config sflow interface sample-rate ...')
#
@interface.command('sample-rate')
@click.argument('ifname', metavar='<interface_name>', required=True, type=str)
@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':
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

click.echo('Invalid interface name')
return
if not is_valid_sample_rate(rate):
click.echo('Error: Sample rate must be between 256 and 8388608')
return

config_db = ctx.obj['db']
sess_dict = config_db.get_table('SFLOW_SESSION')

if sess_dict and ifname in sess_dict.keys():
sess_dict[ifname]['sample_rate'] = rate
config_db.mod_entry('SFLOW_SESSION', ifname, sess_dict[ifname])
else:
config_db.mod_entry('SFLOW_SESSION', ifname, {'sample_rate': rate})


#
# 'sflow collector' group
#
@sflow.group()
@click.pass_context
def collector(ctx):
"""Add/Delete a sFlow collector"""
pass

def is_valid_collector_info(name, ip, port):
if len(name) > 16:
click.echo("Collector name must not exceed 16 characters")
return False

if port not in range(0, 65535 + 1):
click.echo("Collector port number must be between 0 and 65535")
return False

if not is_ipaddress(ip):
click.echo("Invalid IP address")
return False

return True

#
# 'sflow' command ('config sflow collector add ...')
#
@collector.command()
@click.option('--port', required=False, type=int, default=6343,
help='Collector port number')
@click.argument('name', metavar='<collector_name>', required=True)
@click.argument('ipaddr', metavar='<IPv4/v6_address>', required=True)
@click.pass_context
def add(ctx, name, ipaddr, port):
"""Add a sFlow collector"""
ipaddr = ipaddr.lower()

if not is_valid_collector_info(name, ipaddr, port):
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

return

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):
Copy link
Contributor

@wendani wendani Sep 1, 2020

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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)

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a limitation from upper-layer hsflowd daemon?

click.echo("Only 2 collectors can be configured, please delete one")
return

config_db.mod_entry('SFLOW_COLLECTOR', name,
{"collector_ip": ipaddr, "collector_port": port})
return

#
# 'sflow' command ('config sflow collector del ...')
#
@collector.command('del')
@click.argument('name', metavar='<collector_name>', required=True)
@click.pass_context
def del_collector(ctx, name):
"""Delete a sFlow collector"""
config_db = ctx.obj['db']
collector_tbl = config_db.get_table('SFLOW_COLLECTOR')

if name not in collector_tbl.keys():
click.echo("Collector: {} not configured".format(name))
return

config_db.mod_entry('SFLOW_COLLECTOR', name, None)

#
# 'sflow agent-id' group
#
@sflow.group('agent-id')
@click.pass_context
def agent_id(ctx):
"""Add/Delete a sFlow agent"""
pass

#
# 'sflow' command ('config sflow agent-id add ...')
#
@agent_id.command()
@click.argument('ifname', metavar='<interface_name>', required=True)
@click.pass_context
def add(ctx, ifname):
"""Add sFlow agent information"""
if ifname not in netifaces.interfaces():
click.echo("Invalid interface name")
return

config_db = ctx.obj['db']
sflow_tbl = config_db.get_table('SFLOW')

if not sflow_tbl:
sflow_tbl = {'global': {'admin_state': 'down'}}

if 'agent_id' in sflow_tbl['global'].keys():
click.echo("Agent already configured. Please delete it first.")
return

sflow_tbl['global']['agent_id'] = ifname
config_db.mod_entry('SFLOW', 'global', sflow_tbl['global'])

#
# 'sflow' command ('config sflow agent-id del')
#
@agent_id.command('del')
@click.pass_context
def delete(ctx):
"""Delete sFlow agent information"""
config_db = ctx.obj['db']
sflow_tbl = config_db.get_table('SFLOW')

if not sflow_tbl:
sflow_tbl = {'global': {'admin_state': 'down'}}

if 'agent_id' not in sflow_tbl['global'].keys():
click.echo("sFlow agent not configured.")
return

sflow_tbl['global'].pop('agent_id')
config_db.set_entry('SFLOW', 'global', sflow_tbl['global'])


if __name__ == '__main__':
config()
Loading