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.

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

[SAIP4] Add vlan match to acl_pre_ingress_vlan_table, Disallow matching on reserved VLAN ids & Add fixed action id for no_action action in routing.p4 #571

Closed
1 change: 1 addition & 0 deletions sai_p4/fixed/ids.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
// --- Actions -----------------------------------------------------------------

// IDs of fixed SAI actions (8 most significant bits = 0x01).
#define ROUTING_NO_ACTION_ACTION_ID 0x01798B9E // 24742814
#define ROUTING_SET_DST_MAC_ACTION_ID 0x01000001 // 16777217
#define ROUTING_SET_PORT_AND_SRC_MAC_ACTION_ID 0x01000002 // 16777218
#define ROUTING_SET_PORT_AND_SRC_MAC_AND_VLAN_ID_ACTION_ID \
Expand Down
1 change: 1 addition & 0 deletions sai_p4/fixed/routing.p4
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,7 @@ control routing(in headers_t headers,
// Google's naming conventions.
// TODO: Add support for CamlCase actions to the PD generator,
// so we can use `NoAction` throughout.
@id(ROUTING_NO_ACTION_ACTION_ID)
action no_action() {}

// Programming this table does not affect packet forwarding directly -- the
Expand Down
2 changes: 2 additions & 0 deletions sai_p4/instantiations/google/acl_pre_ingress.p4
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,8 @@ control acl_pre_ingress(in headers_t headers,
@sai_field(SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE/IPV6ANY);
headers.ethernet.ether_type : ternary @name("ether_type") @id(4)
@sai_field(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE);
local_metadata.vlan_id : ternary @id(5) @name("vlan_id")
@sai_field(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID);
}
actions = {
@proto_id(1) set_outer_vlan_id;
Expand Down
1 change: 0 additions & 1 deletion sai_p4/instantiations/google/ids.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@
#define ACL_WBB_INGRESS_TRAP_ACTION_ID 0x01000108 // 16777480
#define ACL_DROP_ACTION_ID 0x01000109 // 16777481


// NOLINTEND
// --- Meters ------------------------------------------------------------------
#define ACL_INGRESS_METER_ID 0x15000100 // 352321792
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ tables {
alias: "acl_pre_ingress_table"
annotations: "@p4runtime_role(\"sdn_controller\")"
annotations: "@sai_acl(PRE_INGRESS)"
annotations: "@entry_restriction(\"\n // Only allow IP field matches for IP packets.\n dscp::mask != 0 -> (is_ip == 1 || is_ipv4 == 1 || is_ipv6 == 1);\n ecn::mask != 0 -> (is_ip == 1 || is_ipv4 == 1 || is_ipv6 == 1);\n dst_ip::mask != 0 -> is_ipv4 == 1;\n dst_ipv6::mask != 0 -> is_ipv6 == 1;\n // Forbid illegal combinations of IP_TYPE fields.\n is_ip::mask != 0 -> (is_ipv4::mask == 0 && is_ipv6::mask == 0);\n is_ipv4::mask != 0 -> (is_ip::mask == 0 && is_ipv6::mask == 0);\n is_ipv6::mask != 0 -> (is_ip::mask == 0 && is_ipv4::mask == 0);\n // Forbid unsupported combinations of IP_TYPE fields.\n is_ipv4::mask != 0 -> (is_ipv4 == 1);\n is_ipv6::mask != 0 -> (is_ipv6 == 1);\n // Reserve high priorities for switch-internal use.\n // TODO: Remove once inband workaround is obsolete.\n ::priority < 0x7fffffff;\n \")"
annotations: "@entry_restriction(\"\n // Only allow IP field matches for IP packets.\n dscp::mask != 0 -> (is_ip == 1 || is_ipv4 == 1 || is_ipv6 == 1);\n ecn::mask != 0 -> (is_ip == 1 || is_ipv4 == 1 || is_ipv6 == 1);\n dst_ip::mask != 0 -> is_ipv4 == 1;\n dst_ipv6::mask != 0 -> is_ipv6 == 1;\n // Forbid illegal combinations of IP_TYPE fields.\n is_ip::mask != 0 -> (is_ipv4::mask == 0 && is_ipv6::mask == 0);\n is_ipv4::mask != 0 -> (is_ip::mask == 0 && is_ipv6::mask == 0);\n is_ipv6::mask != 0 -> (is_ip::mask == 0 && is_ipv4::mask == 0);\n // Forbid unsupported combinations of IP_TYPE fields.\n is_ipv4::mask != 0 -> (is_ipv4 == 1);\n is_ipv6::mask != 0 -> (is_ipv6 == 1);\n\n\n\n\n // Reserve high priorities for switch-internal use.\n // TODO: Remove once inband workaround is obsolete.\n ::priority < 0x7fffffff;\n \")"
}
match_fields {
id: 1
Expand Down
4 changes: 4 additions & 0 deletions sai_p4/instantiations/google/sai_pd.proto
Original file line number Diff line number Diff line change
Expand Up @@ -452,12 +452,16 @@ message AclEgressTableEntry {
// ## Forbid unsupported combinations of IP_TYPE fields.
// is_ipv4::mask != 0 -> (is_ipv4 == 1);
// is_ipv6::mask != 0 -> (is_ipv6 == 1);
// ## Disallow match on reserved VLAN IDs to rule out vendor specific behavior.
// vlan_id::mask != 0 -> (vlan_id != 4095 && vlan_id != 0);
// ## TODO: Disallow setting to reserved VLAN IDs when supported.
message AclPreIngressVlanTableEntry {
message Match {
Optional is_ip = 1; // optional match / Format::HEX_STRING / 1 bits
Optional is_ipv4 = 2; // optional match / Format::HEX_STRING / 1 bits
Optional is_ipv6 = 3; // optional match / Format::HEX_STRING / 1 bits
Ternary ether_type = 4; // ternary match / Format::HEX_STRING / 16 bits
Ternary vlan_id = 5; // ternary match / Format::HEX_STRING / 12 bits
}
Match match = 1;
message Action {
Expand Down
16 changes: 16 additions & 0 deletions sai_p4/instantiations/google/test_tools/test_entries.cc
Original file line number Diff line number Diff line change
Expand Up @@ -504,4 +504,20 @@ EntryBuilder& EntryBuilder::AddEntrySettingVrfForAllPackets(
return *this;
}

EntryBuilder& EntryBuilder::AddEntrySettingVlanIdInPreIngress(
absl::string_view set_vlan_id_hexstr,
std::optional<absl::string_view> match_vlan_id_hexstr) {
sai::AclPreIngressVlanTableEntry& entry =
*entries_.add_entries()->mutable_acl_pre_ingress_vlan_table_entry();
if (match_vlan_id_hexstr.has_value()) {
entry.mutable_match()->mutable_vlan_id()->set_value(*match_vlan_id_hexstr);
entry.mutable_match()->mutable_vlan_id()->set_mask("0xfff");
}
entry.mutable_action()->mutable_set_outer_vlan_id()->set_vlan_id(
set_vlan_id_hexstr);
entry.set_priority(1);

return *this;
}

} // namespace sai
3 changes: 3 additions & 0 deletions sai_p4/instantiations/google/test_tools/test_entries.h
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,9 @@ class EntryBuilder {
EntryBuilder& AddEntrySettingVrfBasedOnVlanId(
absl::string_view vlan_id_hexstr, absl::string_view vrf);
EntryBuilder& AddEntrySettingVrfForAllPackets(absl::string_view vrf);
EntryBuilder& AddEntrySettingVlanIdInPreIngress(
absl::string_view set_vlan_id_hexstr,
std::optional<absl::string_view> match_vlan_id_hexstr = std::nullopt);

private:
sai::TableEntries entries_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,6 @@ absl::Status InstallEntriesForwardingAndRewritingVlanInRifTable(
.AddDefaultRouteForwardingAllPacketsToGivenPort(
egress_port, sai::IpVersion::kIpv4, "vrf-forward",
{.disable_vlan_rewrite = disable_vlan_rewrite}, vlan_id_hexstr)));

if (disable_vlan_checks) {
return InstallEntries(bmv2, ir_p4info,
sai::EntryBuilder().AddDisableVlanChecksEntry());
Expand Down Expand Up @@ -442,6 +441,7 @@ TEST_P(VlanTest, IngressVidGetCarriedOverToEgressWhenVlanRewriteIsDisabled) {

ASSERT_OK(InstallEntriesForwardingAndRewritingVlanInRifTable(
bmv2, kIrP4Info, /*vlan_id_hexstr=*/std::nullopt, kEgressPortProto,
/*disable_vlan_checks=*/true, /*disable_vlan_rewrite=*/true));
{
// Inject packet without a VLAN tag.
ASSERT_OK_AND_ASSIGN(PacketsByPort output_by_port,
Expand Down
7 changes: 7 additions & 0 deletions sai_p4/instantiations/google/tor.p4info.pb.txt
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,13 @@ tables {
bitwidth: 16
match_type: TERNARY
}
match_fields {
id: 5
name: "vlan_id"
annotations: "@sai_field(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID)"
bitwidth: 12
match_type: TERNARY
}
action_refs {
id: 16777482
annotations: "@proto_id(1)"
Expand Down
9 changes: 8 additions & 1 deletion sai_p4/instantiations/google/unioned_p4info.pb.txt
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ tables {
alias: "acl_pre_ingress_table"
annotations: "@p4runtime_role(\"sdn_controller\")"
annotations: "@sai_acl(PRE_INGRESS)"
annotations: "@entry_restriction(\"\n // Only allow IP field matches for IP packets.\n dscp::mask != 0 -> (is_ip == 1 || is_ipv4 == 1 || is_ipv6 == 1);\n ecn::mask != 0 -> (is_ip == 1 || is_ipv4 == 1 || is_ipv6 == 1);\n dst_ip::mask != 0 -> is_ipv4 == 1;\n dst_ipv6::mask != 0 -> is_ipv6 == 1;\n // Forbid illegal combinations of IP_TYPE fields.\n is_ip::mask != 0 -> (is_ipv4::mask == 0 && is_ipv6::mask == 0);\n is_ipv4::mask != 0 -> (is_ip::mask == 0 && is_ipv6::mask == 0);\n is_ipv6::mask != 0 -> (is_ip::mask == 0 && is_ipv4::mask == 0);\n // Forbid unsupported combinations of IP_TYPE fields.\n is_ipv4::mask != 0 -> (is_ipv4 == 1);\n is_ipv6::mask != 0 -> (is_ipv6 == 1);\n // Reserve high priorities for switch-internal use.\n // TODO: Remove once inband workaround is obsolete.\n ::priority < 0x7fffffff;\n \")"
annotations: "@entry_restriction(\"\n // Only allow IP field matches for IP packets.\n dscp::mask != 0 -> (is_ip == 1 || is_ipv4 == 1 || is_ipv6 == 1);\n ecn::mask != 0 -> (is_ip == 1 || is_ipv4 == 1 || is_ipv6 == 1);\n dst_ip::mask != 0 -> is_ipv4 == 1;\n dst_ipv6::mask != 0 -> is_ipv6 == 1;\n // Forbid illegal combinations of IP_TYPE fields.\n is_ip::mask != 0 -> (is_ipv4::mask == 0 && is_ipv6::mask == 0);\n is_ipv4::mask != 0 -> (is_ip::mask == 0 && is_ipv6::mask == 0);\n is_ipv6::mask != 0 -> (is_ip::mask == 0 && is_ipv4::mask == 0);\n // Forbid unsupported combinations of IP_TYPE fields.\n is_ipv4::mask != 0 -> (is_ipv4 == 1);\n is_ipv6::mask != 0 -> (is_ipv6 == 1);\n\n\n\n\n // Reserve high priorities for switch-internal use.\n // TODO: Remove once inband workaround is obsolete.\n ::priority < 0x7fffffff;\n \")"
}
match_fields {
id: 1
Expand Down Expand Up @@ -989,6 +989,13 @@ tables {
bitwidth: 16
match_type: TERNARY
}
match_fields {
id: 5
name: "vlan_id"
annotations: "@sai_field(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID)"
bitwidth: 12
match_type: TERNARY
}
action_refs {
id: 16777482
annotations: "@proto_id(1)"
Expand Down
Loading