Skip to content

Support for sonic-mgmt ACL testing on VPP #1599

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Support for sonic-mgmt ACL testing on VPP #1599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
27cf0a4
Enabling sonic-mgmt ACL testing for Sonic-VPP
AkeelAli May 12, 2025
11fbf87
Use retval in debugs
AkeelAli May 12, 2025
5121a86
Merge branch 'master' into acl_sonic_mgmt
AkeelAli Jun 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 103 additions & 18 deletions vslib/vpp/SwitchVppAcl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ static sai_status_t acl_icmp_field_to_vpp_acl_rule(
{
SWSS_LOG_ENTER();

uint16_t data = 0, mask = 0;
uint16_t first = 0, last = 0;
uint16_t new_data, new_mask;

assert((SAI_ACL_ENTRY_ATTR_FIELD_ICMP_CODE == attr_id) ||
Expand All @@ -196,25 +196,21 @@ static sai_status_t acl_icmp_field_to_vpp_acl_rule(
new_data = (value->aclfield.enable) ? value->aclfield.data.u8 : 0;
new_mask = (value->aclfield.enable) ? value->aclfield.mask.u8 : 0;

first = new_data & new_mask;
last = new_data | (~new_mask & 0xFF);

switch (attr_id) {
case SAI_ACL_ENTRY_ATTR_FIELD_ICMP_CODE:
case SAI_ACL_ENTRY_ATTR_FIELD_ICMPV6_CODE:
data = (uint16_t) ((data & 0xFF) | (new_data << 8));
mask = (uint16_t) ((mask & 0xFF) | (new_mask << 8));

rule->dstport_or_icmpcode_first = data;
rule->srcport_or_icmptype_last = mask;
rule->dstport_or_icmpcode_first = first;
rule->dstport_or_icmpcode_last = last;

break;

case SAI_ACL_ENTRY_ATTR_FIELD_ICMP_TYPE:
case SAI_ACL_ENTRY_ATTR_FIELD_ICMPV6_TYPE:
data = (uint16_t) ((data & 0xFF00) | new_data);
mask = (uint16_t) ((mask & 0xFF00) | new_mask);

rule->srcport_or_icmptype_first = data;
rule->srcport_or_icmptype_last = mask;
rule->srcport_or_icmptype_first = first;
rule->srcport_or_icmptype_last = last;

break;

Expand Down Expand Up @@ -266,15 +262,29 @@ static sai_status_t acl_rule_port_range_vpp_acl_set(

switch (type) {
case SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE:
if (rule->proto != 0 && rule->proto != IPPROTO_TCP) {
SWSS_LOG_ERROR(
"Conflicting protocol settings: "
"src port range requires TCP, but proto is already set to %u",
rule->proto);
return SAI_STATUS_FAILURE;
}
rule->proto = IPPROTO_TCP;
rule->srcport_or_icmptype_first = (uint16_t) range->min;
rule->srcport_or_icmptype_last = (uint16_t) range->max;
SWSS_LOG_INFO("SRC port range %u-%u", range->min, range->max);
break;

case SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE:
if (rule->proto != 0 && rule->proto != IPPROTO_TCP) {
SWSS_LOG_ERROR(
"Conflicting protocol settings: "
"dst port range requires TCP, but proto is already set to %u",
rule->proto);
return SAI_STATUS_FAILURE;
}
rule->proto = IPPROTO_TCP;
rule->dstport_or_icmpcode_first = (uint16_t) range->min;
rule->dstport_or_icmpcode_last = (uint16_t) range->max;
SWSS_LOG_INFO("DST port range %u-%u", range->min, range->max);
break;

default:
Expand Down Expand Up @@ -342,9 +352,9 @@ static void acl_rule_set_action(
{
SWSS_LOG_ENTER();

switch (value->aclaction.parameter.s32) {
switch (value->aclaction.parameter.s32) {
case SAI_PACKET_ACTION_FORWARD:
rule->action = VPP_ACL_ACTION_API_PERMIT_STFULL;
rule->action = VPP_ACL_ACTION_API_PERMIT;
break;

case SAI_PACKET_ACTION_DROP:
Expand Down Expand Up @@ -381,30 +391,95 @@ sai_status_t acl_rule_field_update(
status = acl_ip_type_field_to_vpp_acl_rule(attr_id, value, rule);
break;


case SAI_ACL_ENTRY_ATTR_FIELD_ICMP_CODE:
case SAI_ACL_ENTRY_ATTR_FIELD_ICMP_TYPE:
if (rule->proto != 0 && rule->proto != IPPROTO_ICMP) {
SWSS_LOG_ERROR(
"Conflicting protocol settings: "
"ICMP requires ICMP protocol, but proto is already set to %u",
rule->proto);
return SAI_STATUS_FAILURE;
}
rule->proto = IPPROTO_ICMP;
status = acl_icmp_field_to_vpp_acl_rule(attr_id, value, rule);
break;

case SAI_ACL_ENTRY_ATTR_FIELD_ICMPV6_CODE:
case SAI_ACL_ENTRY_ATTR_FIELD_ICMPV6_TYPE:
status = acl_icmp_field_to_vpp_acl_rule(attr_id,
value, rule);
if (rule->proto != 0 && rule->proto != IPPROTO_ICMPV6) {
SWSS_LOG_ERROR(
"Conflicting protocol settings: "
"ICMPv6 requires ICMPv6 protocol, but proto is already set to %u",
rule->proto);
return SAI_STATUS_FAILURE;
}
rule->proto = IPPROTO_ICMPV6;
status = acl_icmp_field_to_vpp_acl_rule(attr_id, value, rule);
break;

case SAI_ACL_ENTRY_ATTR_FIELD_L4_SRC_PORT:
case SAI_ACL_ENTRY_ATTR_FIELD_L4_DST_PORT:
if (rule->proto != 0 && rule->proto != IPPROTO_TCP) {
SWSS_LOG_ERROR(
"Conflicting protocol settings: "
"src/dst port requires TCP, but proto is already set to %u",
rule->proto);
return SAI_STATUS_FAILURE;
}
rule->proto = IPPROTO_TCP;
status = acl_entry_port_to_vpp_acl_rule(attr_id, value, rule);
break;

case SAI_ACL_ENTRY_ATTR_FIELD_IP_PROTOCOL:
if (rule->proto != 0 && rule->proto != (value->aclfield.data.u8 & value->aclfield.mask.u8)) {
SWSS_LOG_ERROR(
"Conflicting protocol settings: "
"IP_PROTOCOL specified but proto is already set to %u",
rule->proto);
return SAI_STATUS_FAILURE;
}
rule->proto = value->aclfield.data.u8 & value->aclfield.mask.u8;
status = SAI_STATUS_SUCCESS;
break;

case SAI_ACL_ENTRY_ATTR_FIELD_TCP_FLAGS:
if (rule->proto != 0 && rule->proto != IPPROTO_TCP) {
SWSS_LOG_ERROR(
"Conflicting protocol settings: "
"TCP flags require TCP, but proto is already set to %u",
rule->proto);
return SAI_STATUS_FAILURE;
}
rule->proto = IPPROTO_TCP;
rule->tcp_flags_mask = value->aclfield.mask.u8;
rule->tcp_flags_value = value->aclfield.data.u8;
break;

case SAI_ACL_ENTRY_ATTR_FIELD_IPV6_NEXT_HEADER:
if (rule->proto != 0 && rule->proto != (value->aclfield.data.u8 & value->aclfield.mask.u8)) {
SWSS_LOG_ERROR(
"Conflicting protocol settings: "
"IPV6_NEXT_HEADER specified but proto is already set to %u",
rule->proto);
return SAI_STATUS_FAILURE;
}
rule->proto = value->aclfield.data.u8 & value->aclfield.mask.u8;
break;

case SAI_ACL_ENTRY_ATTR_ACTION_PACKET_ACTION:
acl_rule_set_action(value, rule);
break;

case SAI_ACL_ENTRY_ATTR_PRIORITY:
case SAI_ACL_ENTRY_ATTR_TABLE_ID:
case SAI_ACL_ENTRY_ATTR_ADMIN_STATE:
case SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE:
case SAI_ACL_ENTRY_ATTR_ACTION_COUNTER:
// NOOP here - these are either handled elsewhere or not currently applicable
break;

default:
SWSS_LOG_ERROR("Unhandled ACL entry attribute ID: %d", attr_id);
break;
}

Expand Down Expand Up @@ -837,6 +912,16 @@ sai_status_t SwitchVpp::fill_acl_rules(
status = acl_rule_field_update((sai_acl_entry_attr_t) attr->id, &attr->value, rule);
}

if (rule && (rule->srcport_or_icmptype_first != 0 || rule->dstport_or_icmpcode_first != 0)) {
SWSS_LOG_DEBUG(
"Attribute %d ranges: "
"srcport_or_icmptype = %u - %u, "
"dstport_or_icmpcode = %u - %u",
attr->id,
rule->srcport_or_icmptype_first, rule->srcport_or_icmptype_last,
rule->dstport_or_icmpcode_first, rule->dstport_or_icmpcode_last);
}

if(status != SAI_STATUS_SUCCESS) {
SWSS_LOG_ERROR("Failed to fill acl rule, status: %d", status);
return SAI_STATUS_FAILURE;
Expand Down
Loading
Loading