Skip to content

Commit

Permalink
[aclorch]: Add support for redirect ACL action (sonic-net#235)
Browse files Browse the repository at this point in the history
* Add doc for ACL redirect
* Implementation
* Add reference counting
  • Loading branch information
pavel-shirshov authored and Shuotian Cheng committed Jun 9, 2017
1 parent 6fe29db commit 476772d
Show file tree
Hide file tree
Showing 6 changed files with 191 additions and 26 deletions.
13 changes: 9 additions & 4 deletions doc/swss-schema.md
Original file line number Diff line number Diff line change
Expand Up @@ -444,10 +444,15 @@ Stores rules associated with a specific ACL table on the switch.
priority = 1*3DIGIT ; rule priority. Valid values range
; could be platform dependent

packet_action = "forward"/"drop"/"mirror" ; action when the fields are
; matched (mirror action only
; available to mirror acl table
; type)
packet_action = "forward"/"drop"/"redirect:"redirect_parameter
; an action when the fields are matched
; we have a parameter in case of packet_action="redirect"
; This parameter defines a destination for redirected packets
; it could be:
: name of physical port. Example: "Ethernet10"
: name of LAG port Example: "PortChannel5"
: next-hop ip address Example: "10.0.0.1"
: next-hop group set of addresses Example: "10.0.0.1,10.0.0.3"

mirror_action = 1*255VCHAR ; refer to the mirror session
; (only available to mirror acl
Expand Down
150 changes: 141 additions & 9 deletions orchagent/aclorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,11 @@ acl_rule_attr_lookup_t aclMatchLookup =
{ MATCH_L4_DST_PORT_RANGE, (sai_acl_entry_attr_t)SAI_ACL_RANGE_L4_DST_PORT_RANGE },
};

acl_rule_attr_lookup_t aclActionLookup =
acl_rule_attr_lookup_t aclL3ActionLookup =
{
{ ACTION_PACKET_ACTION, SAI_ACL_ENTRY_ATTR_PACKET_ACTION },
{ ACTION_MIRROR_ACTION, SAI_ACL_ENTRY_ATTR_ACTION_MIRROR_INGRESS }
{ PACKET_ACTION_FORWARD, SAI_ACL_ENTRY_ATTR_PACKET_ACTION },
{ PACKET_ACTION_DROP, SAI_ACL_ENTRY_ATTR_PACKET_ACTION },
{ PACKET_ACTION_REDIRECT, SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT }
};

static acl_table_type_lookup_t aclTableTypeLookUp =
Expand Down Expand Up @@ -345,11 +346,42 @@ bool AclRule::create()
{
SWSS_LOG_ERROR("Failed to create ACL rule");
AclRange::remove(range_objects, range_object_list.count);
decreaseNextHopRefCount();
}

return (status == SAI_STATUS_SUCCESS);
}

void AclRule::decreaseNextHopRefCount()
{
if (!m_redirect_target_next_hop.empty())
{
m_pAclOrch->m_neighOrch->decreaseNextHopRefCount(IpAddress(m_redirect_target_next_hop));
m_redirect_target_next_hop.clear();
}
if (!m_redirect_target_next_hop_group.empty())
{
IpAddresses target = IpAddresses(m_redirect_target_next_hop_group);
m_pAclOrch->m_routeOrch->decreaseNextHopRefCount(target);
// remove next hop group in case it's not used by anything else
if (m_pAclOrch->m_routeOrch->isRefCounterZero(target))
{
if (m_pAclOrch->m_routeOrch->removeNextHopGroup(target))
{
SWSS_LOG_DEBUG("Removed acl redirect target next hop group '%s'", m_redirect_target_next_hop_group.c_str());
}
else
{
SWSS_LOG_ERROR("Failed to remove unused next hop group '%s'", m_redirect_target_next_hop_group.c_str());
// FIXME: what else could we do here?
}
}
m_redirect_target_next_hop_group.clear();
}

return;
}

bool AclRule::remove()
{
SWSS_LOG_ENTER();
Expand All @@ -363,6 +395,8 @@ bool AclRule::remove()

m_ruleOid = SAI_NULL_OBJECT_ID;

decreaseNextHopRefCount();

res = removeRanges();
res &= removeCounter();

Expand Down Expand Up @@ -477,9 +511,6 @@ bool AclRuleL3::validateAddAction(string attr_name, string _attr_value)
string attr_value = toUpper(_attr_value);
sai_attribute_value_t value;

if (aclActionLookup.find(attr_name) == aclActionLookup.end())
return false;

if (attr_name != ACTION_PACKET_ACTION)
{
return false;
Expand All @@ -493,17 +524,116 @@ bool AclRuleL3::validateAddAction(string attr_name, string _attr_value)
{
value.aclaction.parameter.s32 = SAI_PACKET_ACTION_DROP;
}
else if (attr_value.find(PACKET_ACTION_REDIRECT) != string::npos)
{
// resize attr_value to remove argument, _attr_value still has the argument
attr_value.resize(string(PACKET_ACTION_REDIRECT).length());

sai_object_id_t param_id = getRedirectObjectId(_attr_value);
if (param_id == SAI_NULL_OBJECT_ID)
{
return false;
}
value.aclaction.parameter.oid = param_id;
}
else
{
return false;
}
value.aclaction.enable = true;

m_actions[aclActionLookup[attr_name]] = value;
m_actions[aclL3ActionLookup[attr_value]] = value;

return true;
}

// This method should return sai attribute id of the redirect destination
sai_object_id_t AclRuleL3::getRedirectObjectId(const string& redirect_value)
{
// check that we have a colon after redirect rule
size_t colon_pos = string(PACKET_ACTION_REDIRECT).length();
if (redirect_value[colon_pos] != ':')
{
SWSS_LOG_ERROR("Redirect action rule must have ':' after REDIRECT");
return SAI_NULL_OBJECT_ID;
}

if (colon_pos + 1 == redirect_value.length())
{
SWSS_LOG_ERROR("Redirect action rule must have a target after 'REDIRECT:' action");
return SAI_NULL_OBJECT_ID;
}

string target = redirect_value.substr(colon_pos + 1);

// Try to parse physical port and LAG first
Port port;
if(m_pAclOrch->m_portOrch->getPort(target, port))
{
if (port.m_type == Port::PHY)
{
return port.m_port_id;
}
else if (port.m_type == Port::LAG)
{
return port.m_lag_id;
}
else
{
SWSS_LOG_ERROR("Wrong port type for REDIRECT action. Only physical ports and LAG ports are supported");
return SAI_NULL_OBJECT_ID;
}
}

// Try to parse nexthop ip address
try
{
IpAddress ip(target);
if (!m_pAclOrch->m_neighOrch->hasNextHop(ip))
{
SWSS_LOG_ERROR("ACL Redirect action target next hop ip: '%s' doesn't exist on the switch", ip.to_string().c_str());
return SAI_NULL_OBJECT_ID;
}

m_redirect_target_next_hop = target;
m_pAclOrch->m_neighOrch->increaseNextHopRefCount(ip);
return m_pAclOrch->m_neighOrch->getNextHopId(ip);
}
catch (...)
{
// no error, just try next variant
}

// try to parse nh group ip addresses
try
{
IpAddresses ips(target);
if (!m_pAclOrch->m_routeOrch->hasNextHopGroup(ips))
{
SWSS_LOG_INFO("ACL Redirect action target next hop group: '%s' doesn't exist on the switch. Creating it.", ips.to_string().c_str());

if(!m_pAclOrch->m_routeOrch->addNextHopGroup(ips))
{
SWSS_LOG_ERROR("Can't create required target next hop group '%s'", ips.to_string().c_str());
return SAI_NULL_OBJECT_ID;
}
SWSS_LOG_DEBUG("Created acl redirect target next hop group '%s'", ips.to_string().c_str());
}

m_redirect_target_next_hop_group = target;
m_pAclOrch->m_routeOrch->increaseNextHopRefCount(ips);
return m_pAclOrch->m_routeOrch->getNextHopGroupId(ips);
}
catch (...)
{
// no error, just try next variant
}

SWSS_LOG_ERROR("ACL Redirect action target '%s' wasn't recognized", target.c_str());

return SAI_NULL_OBJECT_ID;
}

bool AclRuleL3::validateAddMatch(string attr_name, string attr_value)
{
if (attr_name == MATCH_DSCP)
Expand Down Expand Up @@ -805,10 +935,12 @@ bool AclRange::remove()
return true;
}

AclOrch::AclOrch(DBConnector *db, vector<string> tableNames, PortsOrch *portOrch, MirrorOrch *mirrorOrch) :
AclOrch::AclOrch(DBConnector *db, vector<string> tableNames, PortsOrch *portOrch, MirrorOrch *mirrorOrch, NeighOrch *neighOrch, RouteOrch *routeOrch) :
Orch(db, tableNames),
m_portOrch(portOrch),
m_mirrorOrch(mirrorOrch)
m_mirrorOrch(mirrorOrch),
m_neighOrch(neighOrch),
m_routeOrch(routeOrch)
{
SWSS_LOG_ENTER();

Expand Down
18 changes: 14 additions & 4 deletions orchagent/aclorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@

#define PACKET_ACTION_FORWARD "FORWARD"
#define PACKET_ACTION_DROP "DROP"
#define PACKET_ACTION_REDIRECT "REDIRECT"

#define IP_TYPE_ANY "ANY"
#define IP_TYPE_IP "IP"
Expand Down Expand Up @@ -163,6 +164,8 @@ class AclRule
virtual bool removeCounter();
virtual bool removeRanges();

void decreaseNextHopRefCount();

static sai_uint32_t m_minPriority;
static sai_uint32_t m_maxPriority;
AclOrch *m_pAclOrch;
Expand All @@ -174,6 +177,8 @@ class AclRule
uint32_t m_priority;
map <sai_acl_entry_attr_t, sai_attribute_value_t> m_matches;
map <sai_acl_entry_attr_t, sai_attribute_value_t> m_actions;
string m_redirect_target_next_hop;
string m_redirect_target_next_hop_group;
};

class AclRuleL3: public AclRule
Expand All @@ -185,6 +190,8 @@ class AclRuleL3: public AclRule
bool validateAddMatch(string attr_name, string attr_value);
bool validate();
void update(SubjectType, void *);
private:
sai_object_id_t getRedirectObjectId(const string& redirect_param);
};

class AclRuleMirror: public AclRule
Expand Down Expand Up @@ -231,7 +238,7 @@ inline void split(string str, Iterable& out, char delim = ' ')
class AclOrch : public Orch, public Observer
{
public:
AclOrch(DBConnector *db, vector<string> tableNames, PortsOrch *portOrch, MirrorOrch *mirrorOrch);
AclOrch(DBConnector *db, vector<string> tableNames, PortsOrch *portOrch, MirrorOrch *mirrorOrch, NeighOrch *neighOrch, RouteOrch *routeOrch);
~AclOrch();
void update(SubjectType, void *);

Expand All @@ -242,6 +249,12 @@ class AclOrch : public Orch, public Observer
return m_countersTable;
}

// FIXME: Add getters for them? I'd better to add a common directory of orch objects and use it everywhere
PortsOrch *m_portOrch;
MirrorOrch *m_mirrorOrch;
NeighOrch *m_neighOrch;
RouteOrch *m_routeOrch;

private:
void doTask(Consumer &consumer);
void doAclTableTask(Consumer &consumer);
Expand All @@ -268,9 +281,6 @@ class AclOrch : public Orch, public Observer
static swss::DBConnector m_db;
static swss::Table m_countersTable;

PortsOrch *m_portOrch;
MirrorOrch *m_mirrorOrch;

thread m_countersThread;
};

Expand Down
2 changes: 1 addition & 1 deletion orchagent/orchdaemon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ bool OrchDaemon::init()
APP_ACL_TABLE_NAME,
APP_ACL_RULE_TABLE_NAME
};
AclOrch *acl_orch = new AclOrch(m_applDb, acl_tables, gPortsOrch, mirror_orch);
AclOrch *acl_orch = new AclOrch(m_applDb, acl_tables, gPortsOrch, mirror_orch, neigh_orch, route_orch);

m_orchList = { gPortsOrch, intfs_orch, neigh_orch, route_orch, copp_orch, tunnel_decap_orch, qos_orch, buffer_orch, mirror_orch, acl_orch, gFdbOrch};
m_select = new Select();
Expand Down
18 changes: 17 additions & 1 deletion orchagent/routeorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,17 @@ RouteOrch::RouteOrch(DBConnector *db, string tableName, NeighOrch *neighOrch) :
SWSS_LOG_NOTICE("Create IPv6 default route with packet action drop");
}

bool RouteOrch::hasNextHopGroup(IpAddresses ipAddresses)
bool RouteOrch::hasNextHopGroup(const IpAddresses& ipAddresses) const
{
return m_syncdNextHopGroups.find(ipAddresses) != m_syncdNextHopGroups.end();
}

sai_object_id_t RouteOrch::getNextHopGroupId(const IpAddresses& ipAddresses)
{
assert(hasNextHopGroup(ipAddresses));
return m_syncdNextHopGroups[ipAddresses].next_hop_group_id;
}

void RouteOrch::attach(Observer *observer, const IpAddress& dstAddr)
{
SWSS_LOG_ENTER();
Expand Down Expand Up @@ -393,6 +399,16 @@ void RouteOrch::decreaseNextHopRefCount(IpAddresses ipAddresses)
}
}

bool RouteOrch::isRefCounterZero(const IpAddresses& ipAddresses) const
{
if (!hasNextHopGroup(ipAddresses))
{
return true;
}

return m_syncdNextHopGroups.at(ipAddresses).ref_count == 0;
}

bool RouteOrch::addNextHopGroup(IpAddresses ipAddresses)
{
SWSS_LOG_ENTER();
Expand Down
16 changes: 9 additions & 7 deletions orchagent/routeorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,19 @@ class RouteOrch : public Orch, public Subject
public:
RouteOrch(DBConnector *db, string tableName, NeighOrch *neighOrch);

bool hasNextHopGroup(IpAddresses);
bool hasNextHopGroup(const IpAddresses&) const;
sai_object_id_t getNextHopGroupId(const IpAddresses&);

void attach(Observer *, const IpAddress&);
void detach(Observer *, const IpAddress&);

void increaseNextHopRefCount(IpAddresses);
void decreaseNextHopRefCount(IpAddresses);
bool isRefCounterZero(const IpAddresses&) const;

bool addNextHopGroup(IpAddresses);
bool removeNextHopGroup(IpAddresses);

private:
NeighOrch *m_neighOrch;

Expand All @@ -64,12 +72,6 @@ class RouteOrch : public Orch, public Subject

NextHopObserverTable m_nextHopObservers;

void increaseNextHopRefCount(IpAddresses);
void decreaseNextHopRefCount(IpAddresses);

bool addNextHopGroup(IpAddresses);
bool removeNextHopGroup(IpAddresses);

void addTempRoute(IpPrefix, IpAddresses);
bool addRoute(IpPrefix, IpAddresses);
bool removeRoute(IpPrefix);
Expand Down

0 comments on commit 476772d

Please sign in to comment.