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.

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

Jump to bottom

[ACL]: Optimize DASH ACL by introducing cluster tag #336

Merged
merged 13 commits into from Apr 12, 2023
Merged

[ACL]: Optimize DASH ACL by introducing cluster tag #336

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
9048551
Init tag
Pterosaur Feb 13, 2023
e9a38b2
Merge branch 'main' into dash_acl_tag
Pterosaur Feb 13, 2023
e62ac0a
Polish doc
Pterosaur Feb 14, 2023
ee67783
Fix code and add test
Pterosaur Feb 15, 2023
a245bf5
Increasing wait time for verify packet
Pterosaur Feb 15, 2023
54556af
polish doc
Pterosaur Feb 15, 2023
8a1ef51
Add optional mark in doc
Pterosaur Feb 16, 2023
b84c4ef
Polish doc
Pterosaur Feb 24, 2023
16457a5
Polish doc
Pterosaur Mar 17, 2023
f993969
Polish doc
Pterosaur Feb 24, 2023
f27ec5f
Merge branch 'main' into dash_acl_tag
Pterosaur Mar 19, 2023
8cc3eb8
Merge branch 'dash_acl_tag' of github.com:Pterosaur/DASH into dash_ac…
Pterosaur Mar 20, 2023
afa951d
Merge branch 'main' into dash_acl_tag
Pterosaur Apr 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions dash-pipeline/SAI/templates/saiapi.cpp.j2
View file
Open in desktop
Expand Up @@ -100,6 +100,12 @@ sai_status_t sai_create_{{ table.name }}(
{% elif key.match_type == 'optional' %}
auto mf_optional = mf->mutable_optional();
{{key.sai_key_field}}SetVal(attr_list[i].value, mf_optional, {{key.bitwidth}});
{% elif key.match_type == 'ternary' %}
auto mf_ternary = mf->mutable_ternary();
{{key.sai_key_field}}SetVal(attr_list[i].value, mf_ternary, {{key.bitwidth}});
auto mask = getMaskAttr(SAI_{{ table.name | upper }}_ATTR_{{ key.sai_key_name | upper }}_MASK, attr_count, attr_list);
assert(mask && "SAI_{{ table.name | upper }}_ATTR_{{ key.sai_key_name | upper }}_MASK isn't provided");
{{key.sai_key_field}}SetMask(mask->value, mf_ternary, {{key.bitwidth}});
{% endif %}
{% if 'v4_or_v6_id' in key %}
{
Expand Down
16 changes: 16 additions & 0 deletions dash-pipeline/SAI/templates/saiapi.h.j2
View file
Open in desktop
Expand Up @@ -155,6 +155,22 @@ typedef enum _sai_{{ table.name }}_attr_t
SAI_{{ table.name | upper }}_ATTR_{{ key.sai_key_name | upper }},
{% endif %}

{% if key.match_type == 'ternary' %}
/**
* @brief Ternary matched mask {{ key.sai_key_name }}
*
{% if key.match_type == 'list' %}
* @type {{ key.sai_list_type }}
{% elif key.match_type == 'range_list' %}
* @type {{ key.sai_range_list_type }}
{% else %}
* @type {{ key.sai_key_type }}
{% endif %}
* @flags MANDATORY_ON_CREATE | CREATE_ONLY
*/
SAI_{{ table.name | upper }}_ATTR_{{ key.sai_key_name | upper }}_MASK,

{% endif %}
{% endfor %}
{% endif %}
{% endif %}
Expand Down
19 changes: 19 additions & 0 deletions dash-pipeline/SAI/templates/utils.cpp.j2
View file
Open in desktop
Expand Up @@ -40,6 +40,16 @@ static std::mutex tableLock;
static atomic<sai_object_id_t> nextId;
static std::unique_ptr<p4::v1::P4Runtime::Stub> stub;

void correctIpPrefix(void *ip, const void *mask, size_t length)
{
auto _ip = reinterpret_cast<uint8_t *>(ip);
auto _mask = reinterpret_cast<const uint8_t *>(mask);
for (size_t i = 0; i < length; i++)
{
_ip[i] = _ip[i] & _mask[i];
}
}

int leadingNonZeroBits(const uint32_t ipv4) {
auto firstSetBit = __builtin_ffs(ipv4);
if (0==firstSetBit) {
Expand Down Expand Up @@ -137,6 +147,15 @@ string updateTypeStr(p4::v1::Update_Type updateType) {
return descriptor->FindValueByNumber(updateType)->name();
}

const sai_attribute_t *getMaskAttr(sai_attr_id_t id, uint32_t attr_count, const sai_attribute_t *attr_list) {
for (uint32_t i = 0; i < attr_count; i++) {
if (attr_list[i].id == id) {
return &attr_list[i];
}
}
return nullptr;
}

grpc::StatusCode MutateTableEntry(p4::v1::TableEntry *entry, p4::v1::Update_Type updateType) {

p4::v1::WriteRequest request;
Expand Down
63 changes: 60 additions & 3 deletions dash-pipeline/SAI/templates/utils.h.j2
View file
Open in desktop
Expand Up @@ -140,6 +140,8 @@ void macSetVal(const sai_mac_t &value, T &t, int bits = -1){
t->set_value(const_cast<uint8_t*>(&value[0]), 6);
}

void correctIpPrefix(void *ip, const void *mask, size_t length);

int leadingNonZeroBits(const uint32_t ipv4);

int leadingNonZeroBits(const sai_ip6_t ipv6);
Expand All @@ -154,14 +156,20 @@ void ipPrefixSetVal(const sai_ip_prefix_t &value, T &t, int bits = -1){
switch(value.addr_family) {
case SAI_IP_ADDR_FAMILY_IPV4: {
uint32_t val = value.addr.ip4;
correctIpPrefix(&val, &value.mask.ip4, 4);
t->set_value(&val, 4);
val = htonl(val);
val = htonl(value.mask.ip4);
// LPM entry match field prefix length calculation needs to be fixed to accomodate 128 bit size.
// So the 96 is added to the prefix length.
t->set_prefix_len(leadingNonZeroBits(val)+96);
}
break;
case SAI_IP_ADDR_FAMILY_IPV6: {
t->set_value(const_cast<uint8_t*>(&value.addr.ip6[0]), 16);
t->set_prefix_len(leadingNonZeroBits(value.addr.ip6));
uint8_t ip[16];
std::copy(const_cast<uint8_t*>(&value.addr.ip6[0]), const_cast<uint8_t*>(&value.addr.ip6[0])+16, ip);
correctIpPrefix(ip, value.mask.ip6, 16);
t->set_value(ip, 16);
t->set_prefix_len(leadingNonZeroBits(value.mask.ip6));
}
break;
default: assert(0 && "unrecognzed value.ipaddr.addr_family");
Expand Down Expand Up @@ -218,6 +226,55 @@ void ipaddrrangelistSetVal(const sai_attribute_value_t &value, T &t, int bits =
assert (0 && "NYI");
}

const sai_attribute_t *getMaskAttr(sai_attr_id_t id, uint32_t attr_count, const sai_attribute_t *attr_list);

template<typename T>
void u32SetMask(const sai_attribute_value_t &value, T &t, int bits = 32){
assert(bits <= 32);
uint32_t val = value.u32;
val = htonl(val);
val = val >> (32 - bits);
int bytes = (bits + 7) / 8;
t->set_mask(&val, bytes);
}

template<typename T>
void u32SetMask(const sai_uint32_t &value, T &t, int bits = 32){
assert(bits <= 32);
uint32_t val = value;
val = htonl(val);
val = val >> (32 - bits);
int bytes = (bits + 7) / 8;
t->set_mask(&val, bytes);
}

template<typename T>
void u64SetMask(const sai_attribute_value_t &value, T &t, int bits = 64){
assert(bits <= 64);
uint64_t val = value.u64;
if (*reinterpret_cast<const char*>("\0\x01") == 0) { // Little Endian
const uint32_t high_part = htonl(static_cast<uint32_t>(val >> 32));
const uint32_t low_part = htonl(static_cast<uint32_t>(val & 0xFFFFFFFFLL));
val = (static_cast<uint64_t>(low_part) << 32) | high_part;
val = val >> (64-bits);
}
int bytes = (bits + 7) / 8;
t->set_value(&val, bytes);}

template<typename T>
void u64SetMask(const sai_uint64_t &value, T &t, int bits = 64) {
assert(bits <= 64);
uint64_t val = value;
if (*reinterpret_cast<const char*>("\0\x01") == 0) { // Little Endian
const uint32_t high_part = htonl(static_cast<uint32_t>(val >> 32));
const uint32_t low_part = htonl(static_cast<uint32_t>(val & 0xFFFFFFFFLL));
val = (static_cast<uint64_t>(low_part) << 32) | high_part;
val = val >> (64-bits);
}
int bytes = (bits + 7) / 8;
t->set_mask(&val, bytes);
}

grpc::StatusCode MutateTableEntry(p4::v1::TableEntry *entry, p4::v1::Update_Type updateType);

sai_object_id_t NextObjIndex();
Expand Down
3 changes: 3 additions & 0 deletions dash-pipeline/bmv2/dash_acl.p4
View file
Open in desktop
Expand Up @@ -13,6 +13,7 @@ match_kind {
range_list
}

// #define DASH_MATCH
#ifdef DASH_MATCH
#define LIST_MATCH list
#define RANGE_LIST_MATCH range_list
Expand All @@ -36,6 +37,8 @@ match_kind {
table table_name { \
key = { \
meta. ## table_name ##_dash_acl_group_id : exact @name("meta.dash_acl_group_id:dash_acl_group_id"); \
meta.dst_tag_map : ternary @name("meta.dst_tag_map:dst_tag"); \
Copy link
Collaborator

Choose a reason for hiding this comment

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

match type: optional

Copy link
Collaborator Author

@Pterosaur Pterosaur Feb 16, 2023 •
edited

Choose a reason for hiding this comment

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

Why is here optional?

  1. We would like a mask operation.
    E.G. If the source IP of packet belong to tag 1 and 2, and this ACL rule is applied to tag 2 and 3, it should be matched. If another ACL rule is applied to tag 3 and 4, it should be mismatched.
  2. If the tag was not provided, the target rule could match all tags.

I'm not sure whether the optional match_kind can meet above behaviors.

Copy link
Collaborator Author

@Pterosaur Pterosaur Feb 24, 2023 •
edited

Choose a reason for hiding this comment

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

A case is if a prefix belongs to tags, A, B, C, D (1111) and an ACL rule is applied to tags A, C(1010).
If my understanding is correct, to the match_kind, optional. the mask should be 1111, which implies tag (1111) & mask(1111) != target_tag(1010).
But to the match_kind, ternary, the mask will be 1010. So the rule will be matched due to tag(1111) & mask(1010) = target_tag(1010).

meta.src_tag_map : ternary @name("meta.src_tag_map:src_tag"); \
meta.dst_ip_addr : LIST_MATCH @name("meta.dst_ip_addr:dip"); \
meta.src_ip_addr : LIST_MATCH @name("meta.src_ip_addr:sip"); \
meta.ip_protocol : LIST_MATCH @name("meta.ip_protocol:protocol"); \
Expand Down
4 changes: 4 additions & 0 deletions dash-pipeline/bmv2/dash_metadata.p4
View file
Open in desktop
Expand Up @@ -3,6 +3,8 @@

#include "dash_headers.p4"

typedef bit<32> tag_map_t;

struct encap_data_t {
bit<24> vni;
bit<24> dest_vnet_vni;
Expand Down Expand Up @@ -56,6 +58,8 @@ struct metadata_t {
bit<16> stage3_dash_acl_group_id;
bit<16> stage4_dash_acl_group_id;
bit<16> stage5_dash_acl_group_id;
tag_map_t src_tag_map;
tag_map_t dst_tag_map;
}

#endif /* _SIRIUS_METADATA_P4_ */
32 changes: 32 additions & 0 deletions dash-pipeline/bmv2/dash_pipeline.p4
View file
Open in desktop
Expand Up @@ -268,6 +268,34 @@ control dash_ingress(
}
}

action set_src_tag(tag_map_t tag_map) {
meta.src_tag_map = tag_map;
}

@name("src_tag|dash_tag")
table src_tag {
key = {
meta.src_ip_addr : lpm @name("meta.src_ip_addr:sip");
Copy link
Collaborator

Choose a reason for hiding this comment

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

match type: prefix_list

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think here shouldn't be a list. AFAIK, one prefix can belong to multiple clusters with different tags, So the prefix list will be separated into the tag table. Otherwise if it's prefix_list, it's hard to solve the following case:
prefix(A, B, C)=>tag1
prefix(C,D,E)=>tag2

Because we have to dynamically create a new entry for prefix(C)=>tag1, tag2. And if the C was updated, we need also update all entries that include prefix(C).

Copy link

Choose a reason for hiding this comment

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

Could you have a scenario where eg. 10.0.0.0/8 -> tag1, 10.1.0.0/16 -> tag2.. How do you expect lpm match here to resolve and assign tags?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Assumption of 1 prefix matching and 1 tag assignment. What if there are multiple prefixes matching to 1 tag (overlapping prefixes matching to 1 tag)? (A tag is simply a list of prefixes).
Tags will be in the Control Plane and could be in the Data Plane. Whether or not this tag is represented in the Fastpath is an 'implementation choice'. If a Data Plane chooses to enumerate, for example.
Assumption is that it would be pushed to the SouthBound.
Tag is just a name for a list to be pushed.
Indirection for the Control Plane to use.
@prsunny - we do want to optimize from the Data Plane side as well. Scale limits are in the HLD, however this is possible.
@vincent-xs suggests the API is flexible enough for the supplier to control
@jfingerh - remember that the bit-factor must be big enough :) but we do have scale

}
actions = {
set_src_tag;
}
}

action set_dst_tag(tag_map_t tag_map) {
meta.dst_tag_map = tag_map;
}

@name("dst_tag|dash_tag")
table dst_tag {
key = {
meta.dst_ip_addr : lpm @name("meta.dst_ip_addr:dip");
}
actions = {
set_dst_tag;
}
}

apply {

/* Send packet on same port it arrived (echo) by default */
Expand Down Expand Up @@ -348,6 +376,10 @@ control dash_ingress(
}
acl_group.apply();

src_tag.apply();
dst_tag.apply();


if (meta.direction == direction_t.OUTBOUND) {
outbound.apply(hdr, meta);
} else if (meta.direction == direction_t.INBOUND) {
Expand Down