Skip to content

Commit 476772d

Browse files
pavel-shirshovShuotian Cheng
authored and
Shuotian Cheng
committed
[aclorch]: Add support for redirect ACL action (#235)
* Add doc for ACL redirect * Implementation * Add reference counting
1 parent 6fe29db commit 476772d

File tree

6 files changed

+191
-26
lines changed

6 files changed

+191
-26
lines changed

doc/swss-schema.md

+9-4
Original file line numberDiff line numberDiff line change
@@ -444,10 +444,15 @@ Stores rules associated with a specific ACL table on the switch.
444444
priority = 1*3DIGIT ; rule priority. Valid values range
445445
; could be platform dependent
446446

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

452457
mirror_action = 1*255VCHAR ; refer to the mirror session
453458
; (only available to mirror acl

orchagent/aclorch.cpp

+141-9
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,11 @@ acl_rule_attr_lookup_t aclMatchLookup =
3838
{ MATCH_L4_DST_PORT_RANGE, (sai_acl_entry_attr_t)SAI_ACL_RANGE_L4_DST_PORT_RANGE },
3939
};
4040

41-
acl_rule_attr_lookup_t aclActionLookup =
41+
acl_rule_attr_lookup_t aclL3ActionLookup =
4242
{
43-
{ ACTION_PACKET_ACTION, SAI_ACL_ENTRY_ATTR_PACKET_ACTION },
44-
{ ACTION_MIRROR_ACTION, SAI_ACL_ENTRY_ATTR_ACTION_MIRROR_INGRESS }
43+
{ PACKET_ACTION_FORWARD, SAI_ACL_ENTRY_ATTR_PACKET_ACTION },
44+
{ PACKET_ACTION_DROP, SAI_ACL_ENTRY_ATTR_PACKET_ACTION },
45+
{ PACKET_ACTION_REDIRECT, SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT }
4546
};
4647

4748
static acl_table_type_lookup_t aclTableTypeLookUp =
@@ -345,11 +346,42 @@ bool AclRule::create()
345346
{
346347
SWSS_LOG_ERROR("Failed to create ACL rule");
347348
AclRange::remove(range_objects, range_object_list.count);
349+
decreaseNextHopRefCount();
348350
}
349351

350352
return (status == SAI_STATUS_SUCCESS);
351353
}
352354

355+
void AclRule::decreaseNextHopRefCount()
356+
{
357+
if (!m_redirect_target_next_hop.empty())
358+
{
359+
m_pAclOrch->m_neighOrch->decreaseNextHopRefCount(IpAddress(m_redirect_target_next_hop));
360+
m_redirect_target_next_hop.clear();
361+
}
362+
if (!m_redirect_target_next_hop_group.empty())
363+
{
364+
IpAddresses target = IpAddresses(m_redirect_target_next_hop_group);
365+
m_pAclOrch->m_routeOrch->decreaseNextHopRefCount(target);
366+
// remove next hop group in case it's not used by anything else
367+
if (m_pAclOrch->m_routeOrch->isRefCounterZero(target))
368+
{
369+
if (m_pAclOrch->m_routeOrch->removeNextHopGroup(target))
370+
{
371+
SWSS_LOG_DEBUG("Removed acl redirect target next hop group '%s'", m_redirect_target_next_hop_group.c_str());
372+
}
373+
else
374+
{
375+
SWSS_LOG_ERROR("Failed to remove unused next hop group '%s'", m_redirect_target_next_hop_group.c_str());
376+
// FIXME: what else could we do here?
377+
}
378+
}
379+
m_redirect_target_next_hop_group.clear();
380+
}
381+
382+
return;
383+
}
384+
353385
bool AclRule::remove()
354386
{
355387
SWSS_LOG_ENTER();
@@ -363,6 +395,8 @@ bool AclRule::remove()
363395

364396
m_ruleOid = SAI_NULL_OBJECT_ID;
365397

398+
decreaseNextHopRefCount();
399+
366400
res = removeRanges();
367401
res &= removeCounter();
368402

@@ -477,9 +511,6 @@ bool AclRuleL3::validateAddAction(string attr_name, string _attr_value)
477511
string attr_value = toUpper(_attr_value);
478512
sai_attribute_value_t value;
479513

480-
if (aclActionLookup.find(attr_name) == aclActionLookup.end())
481-
return false;
482-
483514
if (attr_name != ACTION_PACKET_ACTION)
484515
{
485516
return false;
@@ -493,17 +524,116 @@ bool AclRuleL3::validateAddAction(string attr_name, string _attr_value)
493524
{
494525
value.aclaction.parameter.s32 = SAI_PACKET_ACTION_DROP;
495526
}
527+
else if (attr_value.find(PACKET_ACTION_REDIRECT) != string::npos)
528+
{
529+
// resize attr_value to remove argument, _attr_value still has the argument
530+
attr_value.resize(string(PACKET_ACTION_REDIRECT).length());
531+
532+
sai_object_id_t param_id = getRedirectObjectId(_attr_value);
533+
if (param_id == SAI_NULL_OBJECT_ID)
534+
{
535+
return false;
536+
}
537+
value.aclaction.parameter.oid = param_id;
538+
}
496539
else
497540
{
498541
return false;
499542
}
500543
value.aclaction.enable = true;
501544

502-
m_actions[aclActionLookup[attr_name]] = value;
545+
m_actions[aclL3ActionLookup[attr_value]] = value;
503546

504547
return true;
505548
}
506549

550+
// This method should return sai attribute id of the redirect destination
551+
sai_object_id_t AclRuleL3::getRedirectObjectId(const string& redirect_value)
552+
{
553+
// check that we have a colon after redirect rule
554+
size_t colon_pos = string(PACKET_ACTION_REDIRECT).length();
555+
if (redirect_value[colon_pos] != ':')
556+
{
557+
SWSS_LOG_ERROR("Redirect action rule must have ':' after REDIRECT");
558+
return SAI_NULL_OBJECT_ID;
559+
}
560+
561+
if (colon_pos + 1 == redirect_value.length())
562+
{
563+
SWSS_LOG_ERROR("Redirect action rule must have a target after 'REDIRECT:' action");
564+
return SAI_NULL_OBJECT_ID;
565+
}
566+
567+
string target = redirect_value.substr(colon_pos + 1);
568+
569+
// Try to parse physical port and LAG first
570+
Port port;
571+
if(m_pAclOrch->m_portOrch->getPort(target, port))
572+
{
573+
if (port.m_type == Port::PHY)
574+
{
575+
return port.m_port_id;
576+
}
577+
else if (port.m_type == Port::LAG)
578+
{
579+
return port.m_lag_id;
580+
}
581+
else
582+
{
583+
SWSS_LOG_ERROR("Wrong port type for REDIRECT action. Only physical ports and LAG ports are supported");
584+
return SAI_NULL_OBJECT_ID;
585+
}
586+
}
587+
588+
// Try to parse nexthop ip address
589+
try
590+
{
591+
IpAddress ip(target);
592+
if (!m_pAclOrch->m_neighOrch->hasNextHop(ip))
593+
{
594+
SWSS_LOG_ERROR("ACL Redirect action target next hop ip: '%s' doesn't exist on the switch", ip.to_string().c_str());
595+
return SAI_NULL_OBJECT_ID;
596+
}
597+
598+
m_redirect_target_next_hop = target;
599+
m_pAclOrch->m_neighOrch->increaseNextHopRefCount(ip);
600+
return m_pAclOrch->m_neighOrch->getNextHopId(ip);
601+
}
602+
catch (...)
603+
{
604+
// no error, just try next variant
605+
}
606+
607+
// try to parse nh group ip addresses
608+
try
609+
{
610+
IpAddresses ips(target);
611+
if (!m_pAclOrch->m_routeOrch->hasNextHopGroup(ips))
612+
{
613+
SWSS_LOG_INFO("ACL Redirect action target next hop group: '%s' doesn't exist on the switch. Creating it.", ips.to_string().c_str());
614+
615+
if(!m_pAclOrch->m_routeOrch->addNextHopGroup(ips))
616+
{
617+
SWSS_LOG_ERROR("Can't create required target next hop group '%s'", ips.to_string().c_str());
618+
return SAI_NULL_OBJECT_ID;
619+
}
620+
SWSS_LOG_DEBUG("Created acl redirect target next hop group '%s'", ips.to_string().c_str());
621+
}
622+
623+
m_redirect_target_next_hop_group = target;
624+
m_pAclOrch->m_routeOrch->increaseNextHopRefCount(ips);
625+
return m_pAclOrch->m_routeOrch->getNextHopGroupId(ips);
626+
}
627+
catch (...)
628+
{
629+
// no error, just try next variant
630+
}
631+
632+
SWSS_LOG_ERROR("ACL Redirect action target '%s' wasn't recognized", target.c_str());
633+
634+
return SAI_NULL_OBJECT_ID;
635+
}
636+
507637
bool AclRuleL3::validateAddMatch(string attr_name, string attr_value)
508638
{
509639
if (attr_name == MATCH_DSCP)
@@ -805,10 +935,12 @@ bool AclRange::remove()
805935
return true;
806936
}
807937

808-
AclOrch::AclOrch(DBConnector *db, vector<string> tableNames, PortsOrch *portOrch, MirrorOrch *mirrorOrch) :
938+
AclOrch::AclOrch(DBConnector *db, vector<string> tableNames, PortsOrch *portOrch, MirrorOrch *mirrorOrch, NeighOrch *neighOrch, RouteOrch *routeOrch) :
809939
Orch(db, tableNames),
810940
m_portOrch(portOrch),
811-
m_mirrorOrch(mirrorOrch)
941+
m_mirrorOrch(mirrorOrch),
942+
m_neighOrch(neighOrch),
943+
m_routeOrch(routeOrch)
812944
{
813945
SWSS_LOG_ENTER();
814946

orchagent/aclorch.h

+14-4
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343

4444
#define PACKET_ACTION_FORWARD "FORWARD"
4545
#define PACKET_ACTION_DROP "DROP"
46+
#define PACKET_ACTION_REDIRECT "REDIRECT"
4647

4748
#define IP_TYPE_ANY "ANY"
4849
#define IP_TYPE_IP "IP"
@@ -163,6 +164,8 @@ class AclRule
163164
virtual bool removeCounter();
164165
virtual bool removeRanges();
165166

167+
void decreaseNextHopRefCount();
168+
166169
static sai_uint32_t m_minPriority;
167170
static sai_uint32_t m_maxPriority;
168171
AclOrch *m_pAclOrch;
@@ -174,6 +177,8 @@ class AclRule
174177
uint32_t m_priority;
175178
map <sai_acl_entry_attr_t, sai_attribute_value_t> m_matches;
176179
map <sai_acl_entry_attr_t, sai_attribute_value_t> m_actions;
180+
string m_redirect_target_next_hop;
181+
string m_redirect_target_next_hop_group;
177182
};
178183

179184
class AclRuleL3: public AclRule
@@ -185,6 +190,8 @@ class AclRuleL3: public AclRule
185190
bool validateAddMatch(string attr_name, string attr_value);
186191
bool validate();
187192
void update(SubjectType, void *);
193+
private:
194+
sai_object_id_t getRedirectObjectId(const string& redirect_param);
188195
};
189196

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

@@ -242,6 +249,12 @@ class AclOrch : public Orch, public Observer
242249
return m_countersTable;
243250
}
244251

252+
// FIXME: Add getters for them? I'd better to add a common directory of orch objects and use it everywhere
253+
PortsOrch *m_portOrch;
254+
MirrorOrch *m_mirrorOrch;
255+
NeighOrch *m_neighOrch;
256+
RouteOrch *m_routeOrch;
257+
245258
private:
246259
void doTask(Consumer &consumer);
247260
void doAclTableTask(Consumer &consumer);
@@ -268,9 +281,6 @@ class AclOrch : public Orch, public Observer
268281
static swss::DBConnector m_db;
269282
static swss::Table m_countersTable;
270283

271-
PortsOrch *m_portOrch;
272-
MirrorOrch *m_mirrorOrch;
273-
274284
thread m_countersThread;
275285
};
276286

orchagent/orchdaemon.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ bool OrchDaemon::init()
7878
APP_ACL_TABLE_NAME,
7979
APP_ACL_RULE_TABLE_NAME
8080
};
81-
AclOrch *acl_orch = new AclOrch(m_applDb, acl_tables, gPortsOrch, mirror_orch);
81+
AclOrch *acl_orch = new AclOrch(m_applDb, acl_tables, gPortsOrch, mirror_orch, neigh_orch, route_orch);
8282

8383
m_orchList = { gPortsOrch, intfs_orch, neigh_orch, route_orch, copp_orch, tunnel_decap_orch, qos_orch, buffer_orch, mirror_orch, acl_orch, gFdbOrch};
8484
m_select = new Select();

orchagent/routeorch.cpp

+17-1
Original file line numberDiff line numberDiff line change
@@ -95,11 +95,17 @@ RouteOrch::RouteOrch(DBConnector *db, string tableName, NeighOrch *neighOrch) :
9595
SWSS_LOG_NOTICE("Create IPv6 default route with packet action drop");
9696
}
9797

98-
bool RouteOrch::hasNextHopGroup(IpAddresses ipAddresses)
98+
bool RouteOrch::hasNextHopGroup(const IpAddresses& ipAddresses) const
9999
{
100100
return m_syncdNextHopGroups.find(ipAddresses) != m_syncdNextHopGroups.end();
101101
}
102102

103+
sai_object_id_t RouteOrch::getNextHopGroupId(const IpAddresses& ipAddresses)
104+
{
105+
assert(hasNextHopGroup(ipAddresses));
106+
return m_syncdNextHopGroups[ipAddresses].next_hop_group_id;
107+
}
108+
103109
void RouteOrch::attach(Observer *observer, const IpAddress& dstAddr)
104110
{
105111
SWSS_LOG_ENTER();
@@ -393,6 +399,16 @@ void RouteOrch::decreaseNextHopRefCount(IpAddresses ipAddresses)
393399
}
394400
}
395401

402+
bool RouteOrch::isRefCounterZero(const IpAddresses& ipAddresses) const
403+
{
404+
if (!hasNextHopGroup(ipAddresses))
405+
{
406+
return true;
407+
}
408+
409+
return m_syncdNextHopGroups.at(ipAddresses).ref_count == 0;
410+
}
411+
396412
bool RouteOrch::addNextHopGroup(IpAddresses ipAddresses)
397413
{
398414
SWSS_LOG_ENTER();

orchagent/routeorch.h

+9-7
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,19 @@ class RouteOrch : public Orch, public Subject
4747
public:
4848
RouteOrch(DBConnector *db, string tableName, NeighOrch *neighOrch);
4949

50-
bool hasNextHopGroup(IpAddresses);
50+
bool hasNextHopGroup(const IpAddresses&) const;
51+
sai_object_id_t getNextHopGroupId(const IpAddresses&);
5152

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

56+
void increaseNextHopRefCount(IpAddresses);
57+
void decreaseNextHopRefCount(IpAddresses);
58+
bool isRefCounterZero(const IpAddresses&) const;
59+
60+
bool addNextHopGroup(IpAddresses);
61+
bool removeNextHopGroup(IpAddresses);
62+
5563
private:
5664
NeighOrch *m_neighOrch;
5765

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

6573
NextHopObserverTable m_nextHopObservers;
6674

67-
void increaseNextHopRefCount(IpAddresses);
68-
void decreaseNextHopRefCount(IpAddresses);
69-
70-
bool addNextHopGroup(IpAddresses);
71-
bool removeNextHopGroup(IpAddresses);
72-
7375
void addTempRoute(IpPrefix, IpAddresses);
7476
bool addRoute(IpPrefix, IpAddresses);
7577
bool removeRoute(IpPrefix);

0 commit comments

Comments
 (0)