Skip to content

Commit

Permalink
[acl] Expand VLAN into VLAN members when creating an ACL table (#1475)
Browse files Browse the repository at this point in the history
Signed-off-by: Danny Allen <daall@microsoft.com>
  • Loading branch information
daall authored Mar 4, 2021
1 parent 10de91d commit 64604db
Show file tree
Hide file tree
Showing 5 changed files with 190 additions and 13 deletions.
74 changes: 61 additions & 13 deletions config/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -3291,37 +3291,85 @@ def get_acl_bound_ports():

return list(ports)

#
# 'table' subcommand ('config acl add table ...')
#

@add.command()
@click.argument("table_name", metavar="<table_name>")
@click.argument("table_type", metavar="<table_type>")
@click.option("-d", "--description")
@click.option("-p", "--ports")
@click.option("-s", "--stage", type=click.Choice(["ingress", "egress"]), default="ingress")
def table(table_name, table_type, description, ports, stage):
def expand_vlan_ports(port_name):
"""
Add ACL table
Expands a given VLAN interface into its member ports.
If the provided interface is a VLAN, then this method will return its member ports.
If the provided interface is not a VLAN, then this method will return a list with only
the provided interface in it.
"""
config_db = ConfigDBConnector()
config_db.connect()

if port_name not in config_db.get_keys("VLAN"):
return [port_name]

vlan_members = config_db.get_keys("VLAN_MEMBER")

members = [member for vlan, member in vlan_members if port_name == vlan]

if not members:
raise ValueError("Cannot bind empty VLAN {}".format(port_name))

return members


def parse_acl_table_info(table_name, table_type, description, ports, stage):
table_info = {"type": table_type}

if description:
table_info["policy_desc"] = description
else:
table_info["policy_desc"] = table_name

if not ports and ports != None:
raise ValueError("Cannot bind empty list of ports")

port_list = []
valid_acl_ports = get_acl_bound_ports()
if ports:
table_info["ports@"] = ports
for port in ports.split(","):
port_list += expand_vlan_ports(port)
port_list = set(port_list)
else:
table_info["ports@"] = ",".join(get_acl_bound_ports())
port_list = valid_acl_ports

for port in port_list:
if port not in valid_acl_ports:
raise ValueError("Cannot bind ACL to specified port {}".format(port))

table_info["ports@"] = ",".join(port_list)

table_info["stage"] = stage

return table_info

#
# 'table' subcommand ('config acl add table ...')
#

@add.command()
@click.argument("table_name", metavar="<table_name>")
@click.argument("table_type", metavar="<table_type>")
@click.option("-d", "--description")
@click.option("-p", "--ports")
@click.option("-s", "--stage", type=click.Choice(["ingress", "egress"]), default="ingress")
@click.pass_context
def table(ctx, table_name, table_type, description, ports, stage):
"""
Add ACL table
"""
config_db = ConfigDBConnector()
config_db.connect()

try:
table_info = parse_acl_table_info(table_name, table_type, description, ports, stage)
except ValueError as e:
ctx.fail("Failed to parse ACL table config: exception={}".format(e))

config_db.set_entry("ACL_TABLE", table_name, table_info)

#
Expand Down
29 changes: 29 additions & 0 deletions doc/Command-Reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -1371,6 +1371,35 @@ When the optional argument "max_priority" is specified, each rule’s priority

Go Back To [Beginning of the document](#) or [Beginning of this section](#acl)

**config acl add table**

This command is used to create new ACL tables.

- Usage:
```
config acl add table [OPTIONS] <table_name> <table_type> [-d <description>] [-p <ports>] [-s (ingress | egress)]
```

- Parameters:
- table_name: The name of the ACL table to create.
- table_type: The type of ACL table to create (e.g. "L3", "L3V6", "MIRROR")
- description: A description of the table for the user. (default is the table_name)
- ports: A comma-separated list of ports/interfaces to add to the table. The behavior is as follows:
- Physical ports will be bound as physical ports
- Portchannels will be bound as portchannels - passing a portchannel member is invalid
- VLANs will be expanded into their members (e.g. "Vlan1000" will become "Ethernet0,Ethernet2,Ethernet4...")
- stage: The stage this ACL table will be applied to, either ingress or egress. (default is ingress)

- Examples:
```
admin@sonic:~$ sudo config acl add table EXAMPLE L3 -p Ethernet0,Ethernet4 -s ingress
```
```
admin@sonic:~$ sudo config acl add table EXAMPLE_2 L3V6 -p Vlan1000,PortChannel0001,Ethernet128 -s egress
```

Go Back To [Beginning of the document](#) or [Beginning of this section](#acl)


## ARP & NDP

Expand Down
83 changes: 83 additions & 0 deletions tests/acl_config_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import pytest

import config.main as config

from click.testing import CliRunner
from config.main import expand_vlan_ports, parse_acl_table_info


class TestConfigAcl(object):
def test_expand_vlan(self):
assert set(expand_vlan_ports("Vlan1000")) == {"Ethernet4", "Ethernet8", "Ethernet12", "Ethernet16"}

def test_expand_lag(self):
assert set(expand_vlan_ports("PortChannel1001")) == {"PortChannel1001"}

def test_expand_physical_interface(self):
assert set(expand_vlan_ports("Ethernet4")) == {"Ethernet4"}

def test_expand_empty_vlan(self):
with pytest.raises(ValueError):
expand_vlan_ports("Vlan3000")

def test_parse_table_with_vlan_expansion(self):
table_info = parse_acl_table_info("TEST", "L3", None, "Vlan1000", "egress")
assert table_info["type"] == "L3"
assert table_info["policy_desc"] == "TEST"
assert table_info["stage"] == "egress"

port_list = table_info["ports@"].split(",")
assert set(port_list) == {"Ethernet4", "Ethernet8", "Ethernet12", "Ethernet16"}

def test_parse_table_with_vlan_and_duplicates(self):
table_info = parse_acl_table_info("TEST", "L3", None, "Ethernet4,Vlan1000", "egress")
assert table_info["type"] == "L3"
assert table_info["policy_desc"] == "TEST"
assert table_info["stage"] == "egress"

# Since Ethernet4 is a member of Vlan1000 we should not include it twice in the output
port_list = table_info["ports@"].split(",")
assert len(port_list) == 4
assert set(port_list) == {"Ethernet4", "Ethernet8", "Ethernet12", "Ethernet16"}

def test_parse_table_with_empty_vlan(self):
with pytest.raises(ValueError):
parse_acl_table_info("TEST", "L3", None, "Ethernet4,Vlan3000", "egress")

def test_parse_table_with_invalid_ports(self):
with pytest.raises(ValueError):
parse_acl_table_info("TEST", "L3", None, "Ethernet200", "egress")

def test_parse_table_with_empty_ports(self):
with pytest.raises(ValueError):
parse_acl_table_info("TEST", "L3", None, "", "egress")

def test_acl_add_table_nonexistent_port(self):
runner = CliRunner()

result = runner.invoke(
config.config.commands["acl"].commands["add"].commands["table"],
["TEST", "L3", "-p", "Ethernet200"])

assert result.exit_code != 0
assert "Cannot bind ACL to specified port Ethernet200" in result.output

def test_acl_add_table_empty_string_port_list(self):
runner = CliRunner()

result = runner.invoke(
config.config.commands["acl"].commands["add"].commands["table"],
["TEST", "L3", "-p", ""])

assert result.exit_code != 0
assert "Cannot bind empty list of ports" in result.output

def test_acl_add_table_empty_vlan(self):
runner = CliRunner()

result = runner.invoke(
config.config.commands["acl"].commands["add"].commands["table"],
["TEST", "L3", "-p", "Vlan3000"])

assert result.exit_code != 0
assert "Cannot bind empty VLAN Vlan3000" in result.output
3 changes: 3 additions & 0 deletions tests/mock_tables/config_db.json
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,9 @@
"dhcp_servers@": "192.0.0.1,192.0.0.2,192.0.0.3,192.0.0.4",
"vlanid": "2000"
},
"VLAN|Vlan3000": {
"vlanid": "3000"
},
"VLAN_INTERFACE|Vlan1000": {
"NULL": "NULL"
},
Expand Down
14 changes: 14 additions & 0 deletions tests/vlan_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
| | | | | 192.0.0.3 | |
| | | | | 192.0.0.4 | |
+-----------+-----------------+------------+----------------+-----------------------+-------------+
| 3000 | | | | | disabled |
+-----------+-----------------+------------+----------------+-----------------------+-------------+
"""

show_vlan_brief_in_alias_mode_output="""\
Expand All @@ -38,6 +40,8 @@
| | | | | 192.0.0.3 | |
| | | | | 192.0.0.4 | |
+-----------+-----------------+---------+----------------+-----------------------+-------------+
| 3000 | | | | | disabled |
+-----------+-----------------+---------+----------------+-----------------------+-------------+
"""

show_vlan_brief_empty_output="""\
Expand All @@ -49,6 +53,8 @@
| | | | | 192.0.0.3 | |
| | | | | 192.0.0.4 | |
+-----------+-----------------+------------+----------------+-----------------------+-------------+
| 3000 | | | | | disabled |
+-----------+-----------------+------------+----------------+-----------------------+-------------+
"""

show_vlan_brief_with_portchannel_output="""\
Expand All @@ -66,6 +72,8 @@
| | | | | 192.0.0.3 | |
| | | | | 192.0.0.4 | |
+-----------+-----------------+-----------------+----------------+-----------------------+-------------+
| 3000 | | | | | disabled |
+-----------+-----------------+-----------------+----------------+-----------------------+-------------+
"""

show_vlan_config_output="""\
Expand Down Expand Up @@ -115,6 +123,8 @@
| | | | | 192.0.0.3 | |
| | | | | 192.0.0.4 | |
+-----------+-----------------+------------+----------------+-----------------------+-------------+
| 3000 | | | | | disabled |
+-----------+-----------------+------------+----------------+-----------------------+-------------+
"""

config_add_del_vlan_and_vlan_member_output="""\
Expand All @@ -133,6 +143,8 @@
| | | | | 192.0.0.3 | |
| | | | | 192.0.0.4 | |
+-----------+-----------------+------------+----------------+-----------------------+-------------+
| 3000 | | | | | disabled |
+-----------+-----------------+------------+----------------+-----------------------+-------------+
"""

config_add_del_vlan_and_vlan_member_in_alias_mode_output="""\
Expand All @@ -151,6 +163,8 @@
| | | | | 192.0.0.3 | |
| | | | | 192.0.0.4 | |
+-----------+-----------------+---------+----------------+-----------------------+-------------+
| 3000 | | | | | disabled |
+-----------+-----------------+---------+----------------+-----------------------+-------------+
"""
class TestVlan(object):
@classmethod
Expand Down

0 comments on commit 64604db

Please sign in to comment.