diff --git a/autotest/units/001_one_port/041_firewall_keepstate_with_sync_multi_state_in_single_packet/autotest.yaml b/autotest/units/001_one_port/041_firewall_keepstate_with_sync_multi_state_in_single_packet/autotest.yaml index 426667a9..d2263f5e 100644 --- a/autotest/units/001_one_port/041_firewall_keepstate_with_sync_multi_state_in_single_packet/autotest.yaml +++ b/autotest/units/001_one_port/041_firewall_keepstate_with_sync_multi_state_in_single_packet/autotest.yaml @@ -26,3 +26,4 @@ steps: - port: kni0 send: 002-send.pcap expect: 002-expect.pcap +- clearFWState: 1 diff --git a/autotest/units/001_one_port/059_firewall_tablearg/autotest.yaml b/autotest/units/001_one_port/059_firewall_tablearg/autotest.yaml index 41862999..f1b4ae9d 100644 --- a/autotest/units/001_one_port/059_firewall_tablearg/autotest.yaml +++ b/autotest/units/001_one_port/059_firewall_tablearg/autotest.yaml @@ -1,7 +1,8 @@ steps: - ipv4Update: "0.0.0.0/0 -> 200.0.0.1" -- clearFWState: +- clearFWState: 1 - sendPackets: - port: kni0 send: 001-send.pcap expect: 001-expect.pcap +- clearFWState: 1 diff --git a/autotest/units/001_one_port/077_state_timeout/001-expect.pcap b/autotest/units/001_one_port/077_state_timeout/001-expect.pcap new file mode 100644 index 00000000..fddb8179 Binary files /dev/null and b/autotest/units/001_one_port/077_state_timeout/001-expect.pcap differ diff --git a/autotest/units/001_one_port/077_state_timeout/001-send.pcap b/autotest/units/001_one_port/077_state_timeout/001-send.pcap new file mode 100644 index 00000000..6bd24cbe Binary files /dev/null and b/autotest/units/001_one_port/077_state_timeout/001-send.pcap differ diff --git a/autotest/units/001_one_port/077_state_timeout/autotest.yaml b/autotest/units/001_one_port/077_state_timeout/autotest.yaml new file mode 100644 index 00000000..c3b65f85 --- /dev/null +++ b/autotest/units/001_one_port/077_state_timeout/autotest.yaml @@ -0,0 +1,49 @@ +steps: +- ipv4Update: "0.0.0.0/0 -> 10.0.0.2" + +- sendPackets: + - port: kni0 + send: 001-send.pcap + expect: 001-expect.pcap + +- cli_check: | + fw list states + id ruleno label rule + -------- -------- ----- -------------------------------------------------------------------------------------------- + 16777215 16777215 allow tcp from 10.0.0.1 80 to 10.0.0.2 12345 [own, last seen: 2s ago flags S:][packets: 0/0] + +- sleep: 6 # Wait for state to expire + +- cli_check: | + fw list states + id ruleno label rule + -- ------ ----- ---- + +- clearFWState: 1 + +- sendPackets: + - port: kni0 + send: 001-send.pcap + expect: 001-expect.pcap + +- cli_check: | + fw list states + id ruleno label rule + -------- -------- ----- -------------------------------------------------------------------------------------------- + 16777215 16777215 allow tcp from 10.0.0.1 80 to 10.0.0.2 12345 [own, last seen: 2s ago flags S:][packets: 0/0] + +- sleep: 3 # Wait but state should still be present + + # note that last seen value changes +- cli_check: | + fw list states + id ruleno label rule + -------- -------- ----- -------------------------------------------------------------------------------------------- + 16777215 16777215 allow tcp from 10.0.0.1 80 to 10.0.0.2 12345 [own, last seen: 5s ago flags S:][packets: 0/0] + +- sleep: 3 # Wait for state to expire + +- cli_check: | + fw list states + id ruleno label rule + -- ------ ----- ---- diff --git a/autotest/units/001_one_port/077_state_timeout/controlplane.conf b/autotest/units/001_one_port/077_state_timeout/controlplane.conf new file mode 100644 index 00000000..7cbab619 --- /dev/null +++ b/autotest/units/001_one_port/077_state_timeout/controlplane.conf @@ -0,0 +1,26 @@ +{ + "modules": { + "lp0": { + "type": "logicalPort", + "physicalPort": "kni0", + "macAddress": "00:11:22:33:44:55", + "nextModule": "acl0" + }, + "acl0": { + "type": "acl", + "firewall": "firewall.txt", + "nextModules": ["vrf0"] + }, + "vrf0": { + "type": "route", + "interfaces": { + "kni0": { + "ipv4Prefix": "10.0.0.1/24", + "neighborIPv4Address": "10.0.0.2", + "neighborMacAddress": "00:00:00:11:11:11", + "nextModule": "lp0" + } + } + } + } +} diff --git a/autotest/units/001_one_port/077_state_timeout/firewall.txt b/autotest/units/001_one_port/077_state_timeout/firewall.txt new file mode 100644 index 00000000..a719ec80 --- /dev/null +++ b/autotest/units/001_one_port/077_state_timeout/firewall.txt @@ -0,0 +1,9 @@ +:BEGIN +add state-timeout 1 ip from any to any +add state-timeout 2 ip from any to any +add state-timeout 3 ip from any to any +add state-timeout 4 ip from any to any +add state-timeout 5 ip from any to any +# only the last occurence matters +add allow ip from any to any keep-state + diff --git a/autotest/units/001_one_port/077_state_timeout/gen.py b/autotest/units/001_one_port/077_state_timeout/gen.py new file mode 100644 index 00000000..1c886159 --- /dev/null +++ b/autotest/units/001_one_port/077_state_timeout/gen.py @@ -0,0 +1,29 @@ +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- +from typing import List + +from scapy.layers.inet import IP, TCP +from scapy.layers.l2 import Ether +from scapy.packet import Packet +from scapy.utils import PcapWriter + +def write_pcap(path: str, packets: List[Packet]) -> None: + with PcapWriter(path, sync=True) as fh: + for p in packets: + fh.write(p) + +def ipv4_send(src: str, dst: str) -> Packet: + return Ether(dst="00:11:22:33:44:55", src="00:00:00:11:11:11") / IP(src=src, dst=dst, ttl=64) + +def ipv4_recv(src: str, dst: str) -> Packet: + return Ether(dst="00:00:00:11:11:11", src="00:11:22:33:44:55") / IP(src=src, dst=dst, ttl=63) + +# Send packet from 10.0.0.2 to 10.0.0.1 +write_pcap("001-send.pcap", [ + ipv4_send("10.0.0.2", "10.0.0.1") / TCP(sport=12345, dport=80, flags="S"), +]) + +# Expect the packet forwarded +write_pcap("001-expect.pcap", [ + ipv4_recv("10.0.0.2", "10.0.0.1") / TCP(sport=12345, dport=80, flags="S"), +]) diff --git a/autotest/units/001_one_port/077_state_timeout_different_groups/001-expect.pcap b/autotest/units/001_one_port/077_state_timeout_different_groups/001-expect.pcap new file mode 100644 index 00000000..0eeb4fa9 Binary files /dev/null and b/autotest/units/001_one_port/077_state_timeout_different_groups/001-expect.pcap differ diff --git a/autotest/units/001_one_port/077_state_timeout_different_groups/001-send.pcap b/autotest/units/001_one_port/077_state_timeout_different_groups/001-send.pcap new file mode 100644 index 00000000..6f39337a Binary files /dev/null and b/autotest/units/001_one_port/077_state_timeout_different_groups/001-send.pcap differ diff --git a/autotest/units/001_one_port/077_state_timeout_different_groups/autotest.yaml b/autotest/units/001_one_port/077_state_timeout_different_groups/autotest.yaml new file mode 100644 index 00000000..5a6d588b --- /dev/null +++ b/autotest/units/001_one_port/077_state_timeout_different_groups/autotest.yaml @@ -0,0 +1,38 @@ +steps: +- ipv4Update: "0.0.0.0/0 -> 10.0.0.2" + +- sendPackets: + - port: kni0 + send: 001-send.pcap + expect: 001-expect.pcap + +- cli_check: | + fw list states + id ruleno label rule + -------- -------- ----- ------------------------------------------------------------------------------------------------ + 16777215 16777215 allow tcp from 10.0.0.1 80 to 192.168.1.10 12345 [own, last seen: 2s ago flags S:][packets: 0/0] + 16777216 16777215 allow tcp from 10.0.0.1 80 to 192.168.2.20 12346 [own, last seen: 2s ago flags S:][packets: 0/0] + +- sleep: 3 # Wait, states should still be present + +- cli_check: | + fw list states + id ruleno label rule + -------- -------- ----- ------------------------------------------------------------------------------------------------ + 16777215 16777215 allow tcp from 10.0.0.1 80 to 192.168.1.10 12345 [own, last seen: 5s ago flags S:][packets: 0/0] + 16777216 16777215 allow tcp from 10.0.0.1 80 to 192.168.2.20 12346 [own, last seen: 5s ago flags S:][packets: 0/0] + +- sleep: 3 # Wait for first state to expire (total sleep 6s) + +- cli_check: | + fw list states + id ruleno label rule + -------- -------- ----- ------------------------------------------------------------------------------------------------ + 16777215 16777215 allow tcp from 10.0.0.1 80 to 192.168.2.20 12346 [own, last seen: 8s ago flags S:][packets: 0/0] + +- sleep: 3 # Wait for second state to expire (total sleep 9s) + +- cli_check: | + fw list states + id ruleno label rule + -- ------ ----- ---- diff --git a/autotest/units/001_one_port/077_state_timeout_different_groups/controlplane.conf b/autotest/units/001_one_port/077_state_timeout_different_groups/controlplane.conf new file mode 100644 index 00000000..7cbab619 --- /dev/null +++ b/autotest/units/001_one_port/077_state_timeout_different_groups/controlplane.conf @@ -0,0 +1,26 @@ +{ + "modules": { + "lp0": { + "type": "logicalPort", + "physicalPort": "kni0", + "macAddress": "00:11:22:33:44:55", + "nextModule": "acl0" + }, + "acl0": { + "type": "acl", + "firewall": "firewall.txt", + "nextModules": ["vrf0"] + }, + "vrf0": { + "type": "route", + "interfaces": { + "kni0": { + "ipv4Prefix": "10.0.0.1/24", + "neighborIPv4Address": "10.0.0.2", + "neighborMacAddress": "00:00:00:11:11:11", + "nextModule": "lp0" + } + } + } + } +} diff --git a/autotest/units/001_one_port/077_state_timeout_different_groups/firewall.txt b/autotest/units/001_one_port/077_state_timeout_different_groups/firewall.txt new file mode 100644 index 00000000..673febc8 --- /dev/null +++ b/autotest/units/001_one_port/077_state_timeout_different_groups/firewall.txt @@ -0,0 +1,4 @@ +:BEGIN +add state-timeout 5 ip from 192.168.1.0/24 to any +add state-timeout 10 ip from 192.168.2.0/24 to any +add allow ip from any to any keep-state diff --git a/autotest/units/001_one_port/077_state_timeout_different_groups/gen.py b/autotest/units/001_one_port/077_state_timeout_different_groups/gen.py new file mode 100644 index 00000000..9b7bc692 --- /dev/null +++ b/autotest/units/001_one_port/077_state_timeout_different_groups/gen.py @@ -0,0 +1,31 @@ +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- +from typing import List + +from scapy.layers.inet import IP, TCP +from scapy.layers.l2 import Ether +from scapy.packet import Packet +from scapy.utils import PcapWriter + +def write_pcap(path: str, packets: List[Packet]) -> None: + with PcapWriter(path, sync=True) as fh: + for p in packets: + fh.write(p) + +def ipv4_send(src: str, dst: str, ttl: int = 64) -> Packet: + return Ether(dst="00:11:22:33:44:55", src="00:00:00:11:11:11") / IP(src=src, dst=dst, ttl=ttl) + +def ipv4_recv(src: str, dst: str, ttl: int = 63) -> Packet: + return Ether(dst="00:00:00:11:11:11", src="00:11:22:33:44:55") / IP(src=src, dst=dst, ttl=ttl) + +# Send packets from two different subnets +write_pcap("001-send.pcap", [ + ipv4_send("192.168.1.10", "10.0.0.1") / TCP(sport=12345, dport=80, flags="S"), + ipv4_send("192.168.2.20", "10.0.0.1") / TCP(sport=12346, dport=80, flags="S"), +]) + +# Expect the same packets forwarded +write_pcap("001-expect.pcap", [ + ipv4_recv("192.168.1.10", "10.0.0.1") / TCP(sport=12345, dport=80, flags="S"), + ipv4_recv("192.168.2.20", "10.0.0.1") / TCP(sport=12346, dport=80, flags="S"), +]) diff --git a/autotest/units/001_one_port/077_state_timeout_with_dump/001-expect-dump-ring1.pcap b/autotest/units/001_one_port/077_state_timeout_with_dump/001-expect-dump-ring1.pcap new file mode 100644 index 00000000..c0cc6615 Binary files /dev/null and b/autotest/units/001_one_port/077_state_timeout_with_dump/001-expect-dump-ring1.pcap differ diff --git a/autotest/units/001_one_port/077_state_timeout_with_dump/001-expect.pcap b/autotest/units/001_one_port/077_state_timeout_with_dump/001-expect.pcap new file mode 100644 index 00000000..535358ac Binary files /dev/null and b/autotest/units/001_one_port/077_state_timeout_with_dump/001-expect.pcap differ diff --git a/autotest/units/001_one_port/077_state_timeout_with_dump/001-send.pcap b/autotest/units/001_one_port/077_state_timeout_with_dump/001-send.pcap new file mode 100644 index 00000000..4ea57dfb Binary files /dev/null and b/autotest/units/001_one_port/077_state_timeout_with_dump/001-send.pcap differ diff --git a/autotest/units/001_one_port/077_state_timeout_with_dump/002-expect-dump-ring1.pcap b/autotest/units/001_one_port/077_state_timeout_with_dump/002-expect-dump-ring1.pcap new file mode 100644 index 00000000..8c9d89d8 Binary files /dev/null and b/autotest/units/001_one_port/077_state_timeout_with_dump/002-expect-dump-ring1.pcap differ diff --git a/autotest/units/001_one_port/077_state_timeout_with_dump/002-expect.pcap b/autotest/units/001_one_port/077_state_timeout_with_dump/002-expect.pcap new file mode 100644 index 00000000..bcd52bf0 Binary files /dev/null and b/autotest/units/001_one_port/077_state_timeout_with_dump/002-expect.pcap differ diff --git a/autotest/units/001_one_port/077_state_timeout_with_dump/002-send.pcap b/autotest/units/001_one_port/077_state_timeout_with_dump/002-send.pcap new file mode 100644 index 00000000..bc24465e Binary files /dev/null and b/autotest/units/001_one_port/077_state_timeout_with_dump/002-send.pcap differ diff --git a/autotest/units/001_one_port/077_state_timeout_with_dump/autotest.yaml b/autotest/units/001_one_port/077_state_timeout_with_dump/autotest.yaml new file mode 100644 index 00000000..267b8761 --- /dev/null +++ b/autotest/units/001_one_port/077_state_timeout_with_dump/autotest.yaml @@ -0,0 +1,63 @@ +steps: +- ipv4Update: "0.0.0.0/0 -> 10.0.0.2" + +- sendPackets: + - port: kni0 + send: 001-send.pcap + expect: 001-expect.pcap + +- dumpPackets: + - ringTag: shm_2_0 + expect: 001-expect-dump-ring1.pcap + +- cli_check: | + fw list states + id ruleno label rule + -------- -------- ----- ------------------------------------------------------------------------------------------- + 16777215 16777215 allow udp from 10.0.0.1 53 to 10.0.0.10 1024 [own, last seen: 2s ago flags :][packets: 0/0] + +- sleep: 3 # Wait, state should still be present + +- cli_check: | + fw list states + id ruleno label rule + -------- -------- ----- ------------------------------------------------------------------------------------------- + 16777215 16777215 allow udp from 10.0.0.1 53 to 10.0.0.10 1024 [own, last seen: 5s ago flags :][packets: 0/0] + +- sleep: 3 # Wait for state to expire (total sleep 6s) + +- cli_check: | + fw list states + id ruleno label rule + -- ------ ----- ---- + +- sendPackets: + - port: kni0 + send: 002-send.pcap + expect: 002-expect.pcap + +- dumpPackets: + - ringTag: shm_2_0 + expect: 002-expect-dump-ring1.pcap + +- cli_check: | + fw list states + id ruleno label rule + -------- -------- ----- ------------------------------------------------------------------------------------------- + 16777215 16777215 allow udp from 10.0.0.1 53 to 10.0.0.10 1024 [own, last seen: 2s ago flags :][packets: 0/0] + +- sleep: 3 # Wait, state should still be present + +- cli_check: | + fw list states + id ruleno label rule + -------- -------- ----- ------------------------------------------------------------------------------------------- + 16777215 16777215 allow udp from 10.0.0.1 53 to 10.0.0.10 1024 [own, last seen: 5s ago flags :][packets: 0/0] + +- sleep: 3 # Wait for state to expire (total sleep 6s) + +- cli_check: | + fw list states + id ruleno label rule + -- ------ ----- ---- + diff --git a/autotest/units/001_one_port/077_state_timeout_with_dump/controlplane.conf b/autotest/units/001_one_port/077_state_timeout_with_dump/controlplane.conf new file mode 100644 index 00000000..7cbab619 --- /dev/null +++ b/autotest/units/001_one_port/077_state_timeout_with_dump/controlplane.conf @@ -0,0 +1,26 @@ +{ + "modules": { + "lp0": { + "type": "logicalPort", + "physicalPort": "kni0", + "macAddress": "00:11:22:33:44:55", + "nextModule": "acl0" + }, + "acl0": { + "type": "acl", + "firewall": "firewall.txt", + "nextModules": ["vrf0"] + }, + "vrf0": { + "type": "route", + "interfaces": { + "kni0": { + "ipv4Prefix": "10.0.0.1/24", + "neighborIPv4Address": "10.0.0.2", + "neighborMacAddress": "00:00:00:11:11:11", + "nextModule": "lp0" + } + } + } + } +} diff --git a/autotest/units/001_one_port/077_state_timeout_with_dump/firewall.txt b/autotest/units/001_one_port/077_state_timeout_with_dump/firewall.txt new file mode 100644 index 00000000..1bec0169 --- /dev/null +++ b/autotest/units/001_one_port/077_state_timeout_with_dump/firewall.txt @@ -0,0 +1,6 @@ +:BEGIN +add state-timeout 5 ip from any to any +add check-state +add dump ring1 ip from any to any +add allow udp from 10.0.0.0/24 to any 53 record-state +add deny ip from any to any diff --git a/autotest/units/001_one_port/077_state_timeout_with_dump/gen.py b/autotest/units/001_one_port/077_state_timeout_with_dump/gen.py new file mode 100644 index 00000000..83ce705a --- /dev/null +++ b/autotest/units/001_one_port/077_state_timeout_with_dump/gen.py @@ -0,0 +1,50 @@ +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- +from typing import List + +from scapy.layers.inet import IP, UDP +from scapy.layers.l2 import Ether +from scapy.packet import Packet +from scapy.utils import PcapWriter + +def write_pcap(path: str, packets: List[Packet]) -> None: + with PcapWriter(path, sync=True) as fh: + for p in packets: + fh.write(p) + +def ipv4_send(src: str, dst: str, ttl: int = 64) -> Packet: + return Ether(dst="00:11:22:33:44:55", src="00:00:00:11:11:11") / IP(src=src, dst=dst, ttl=ttl) + +def ipv4_recv(src: str, dst: str, ttl: int = 63) -> Packet: + return Ether(dst="00:00:00:11:11:11", src="00:11:22:33:44:55") / IP(src=src, dst=dst, ttl=ttl) + +# Initial packet to create state +write_pcap("001-send.pcap", [ + ipv4_send("10.0.0.10", "10.0.0.1") / UDP(sport=1024, dport=53), +]) + +# Expect the packet to be forwarded +write_pcap("001-expect.pcap", [ + ipv4_recv("10.0.0.10", "10.0.0.1") / UDP(sport=1024, dport=53), +]) + +# Expected dump (initial packet) +write_pcap("001-expect-dump-ring1.pcap", [ + ipv4_send("10.0.0.10", "10.0.0.1") / UDP(sport=1024, dport=53), +]) + +# Packet after state expiration +write_pcap("002-send.pcap", [ + ipv4_send("10.0.0.10", "10.0.0.1") / UDP(sport=1024, dport=53), +]) + +# Expect the packet to be forwarded +write_pcap("002-expect.pcap", [ + ipv4_recv("10.0.0.10", "10.0.0.1") / UDP(sport=1024, dport=53), +]) + +# Expected dump after state expiration +write_pcap("002-expect-dump-ring1.pcap", [ + ipv4_send("10.0.0.10", "10.0.0.1") / UDP(sport=1024, dport=53), #result of a first dump (the ring is the same) + ipv4_send("10.0.0.10", "10.0.0.1") / UDP(sport=1024, dport=53), +]) diff --git a/common/actions.h b/common/actions.h index cebbcc5b..ff3ed6de 100644 --- a/common/actions.h +++ b/common/actions.h @@ -1,11 +1,8 @@ #pragma once -#include -#include - #include "common/type.h" -#include "config.release.h" -#include "stream.h" +#include "common/utils.h" +#include "common/variant_trait_map.h" namespace common { @@ -74,6 +71,44 @@ struct check_state_t } }; +struct state_timeout_t +{ + state_timeout_t() : + timeout(0) + {} + + state_timeout_t(uint32_t timeout) : + timeout(timeout) + {} + + bool operator==(const state_timeout_t& o) const + { + return timeout == o.timeout; + } + + bool operator!=(const state_timeout_t& o) const + { + return !operator==(o); + } + + constexpr bool operator<(const state_timeout_t& o) const + { + return timeout < o.timeout; + } + + void pop(stream_in_t& stream) + { + stream.pop(timeout); + } + + void push(stream_out_t& stream) const + { + stream.push(timeout); + } + + uint32_t timeout; +}; + } // namespace acl // TODO: When rewriting the current ACL library into LibFilter, we could consider using inheritance. @@ -131,6 +166,8 @@ struct FlowAction final static constexpr size_t MAX_COUNT = 1; // The flow associated with this action. globalBase::tFlow flow; + // Timeout for the state + std::optional timeout; FlowAction(const globalBase::tFlow& flow) : flow(flow){}; @@ -146,17 +183,21 @@ struct FlowAction final void pop(stream_in_t& stream) { stream.pop(flow); + stream.pop(timeout); } void push(stream_out_t& stream) const { stream.push(flow); + stream.push(timeout); } [[nodiscard]] std::string to_string() const { std::ostringstream oss; - oss << "FlowAction(flow=" << flow.to_string() << ")"; + oss << "FlowAction(flow=" << flow.to_string() << ", timeout = " + << (timeout.has_value() ? std::to_string(timeout.value()) : "not specified") + << ")"; return oss.str(); } }; @@ -194,12 +235,51 @@ struct CheckStateAction final } }; +/** + * @brief Represents an action that sets timeout for the dynamic firewall rule. + */ +struct StateTimeoutAction final +{ + // Maximum count of StateTimeoutActions objects allowed. + // We have one here since only the last timeout matters. + static constexpr size_t MAX_COUNT = 1; + // Timeout in seconds + uint32_t timeout; + + StateTimeoutAction(const acl::state_timeout_t& timeout_action) : + timeout(timeout_action.timeout){}; + + StateTimeoutAction() : + timeout(0){}; + + [[nodiscard]] bool terminating() const { return false; } + + void pop(stream_in_t& stream) + { + stream.pop(timeout); + } + + void push(stream_out_t& stream) const + { + stream.push(timeout); + } + + [[nodiscard]] std::string to_string() const + { + std::ostringstream oss; + oss << "StateTimeoutAction(timeout=" << timeout << ")"; + return oss.str(); + } +}; + +using RawAction = std::variant; + /** * @brief Represents a generic action. */ struct Action { - std::variant raw_action; + RawAction raw_action; Action() : raw_action(FlowAction()) {} @@ -235,42 +315,240 @@ namespace acl * will have slightly different representation of the internal vector based on whether we have * a "check-state" action or not. This way we can reduce a number of branching in the dataplane and * also reduce the size of the object since we will use std::variant to hold either a "check-state"-object, - * or a regular one (see common::Actions definition below) + * or a regular one (see common::Actions definition below). + * Also, it manages the StateTimeoutActions to reduce number of actions in the dataplane by saving only the + * last timeout action. */ -struct Actions +struct IntermediateActions { - std::vector path{}; - std::array> action_counts = {0}; - std::optional check_state_index{}; + template + struct has_max_count_one + { + static constexpr bool value = (T::MAX_COUNT == 1); + }; - Actions() = default; - Actions(const Action& action) { add(action); }; + /** + * Trait to check if we should store the first occurrence + * + * Define first-matters actions here. + */ + template + struct is_first_matters + { + static constexpr bool value = std::is_same_v; + }; + /** + * Trait to check if we should store the last occurrence + * + * Define last-matters actions here. + */ + template + struct is_last_matters + { + static constexpr bool value = std::is_same_v; + }; + + std::vector path{}; + std::array> action_counts = {0}; + // Using VariantTraitMap to store indices of actions with MAX_COUNT=1. + using OptionalIndexMap = utils::VariantTraitMap>; + OptionalIndexMap indices{}; + + IntermediateActions() = default; + IntermediateActions(const Action& action) { add(action); } + + /** + * @brief Adds an action to the current path while adhering to the following: + * + * Either: + * - Only the first occurrence of an action is kept. + * - Only the last occurrence of an action is kept (previous occurrences are removed). + * - Actions are added as long as they don't exceed their defined `MAX_COUNT`. + * + * Adding a new action type is as simple as placing the action type in the corresponding group. + */ void add(const Action& action) { - size_t index = action.raw_action.index(); + size_t variant_index = action.raw_action.index(); - if (std::holds_alternative(action.raw_action)) + // Extract the type info at the beginning. + std::visit([&](auto&& actual_action) { + using T = std::decay_t; + + if constexpr (has_max_count_one::value) + { + handle_unique_action(action, variant_index); + } + else if (action_counts[variant_index] < T::MAX_COUNT) + { + add_to_path(action, variant_index); + } + }, + action.raw_action); + } + + /** + * @brief Retrieves a pointer to a unique action of type T from the path. + * + * This method returns a pointer of type T if it exists in the path. + * The action type T must have MAX_COUNT == 1. + * + * @tparam T The action type to retrieve. + * @return Pointer to the action of type T. + */ + template + T* get() + { + static_assert(has_max_count_one::value, "Can get only unique actions from path"); + + return indices.get().has_value() ? &get_action() : nullptr; + } + + /** + * @brief Retrieves a const reference to a unique action of type T from the path. + * + * This method returns a const pointer to the action of type T + * if it exists in the path. The action type T must have MAX_COUNT == 1. + * + * @tparam T The action type to retrieve. + * @return A const pointer to the action of type T. + */ + template + [[nodiscard]] const T* get() const + { + static_assert(has_max_count_one::value, "Can get only unique actions from path"); + + return indices.get().has_value() ? &get_action() : nullptr; + } + + /** + * @brief Removes a unique action of type T from the path. + * + * This method removes the action of type T from the path if it exists. + * The action type T must have MAX_COUNT == 1. + * + * @tparam T The action type to remove. + */ + template + void remove() + { + static_assert(has_max_count_one::value, "Can remove only unique actions from path"); + + if (auto& path_index = indices.get()) { - assert(action_counts[index] == 0 && "Incorrectly requested to add more than one FlowAction"); + remove_action_at(path_index.value()); } - else + } + +private: + // We're interested in storing only the first or last occurrence + template + void handle_unique_action(const Action& action, size_t variant_index) + { + if constexpr (is_first_matters::value) + { + handle_first_matters_action(action, variant_index); + } + else if constexpr (is_last_matters::value) { - size_t max_count = std::visit([](auto&& arg) { return std::decay_t::MAX_COUNT; }, - action.raw_action); - if (action_counts[index] >= max_count) + handle_last_matters_action(action, variant_index); + } + else if constexpr (std::is_same_v) + { + /* + * FlowAction should only appear once in the `path`. If a second + * FlowAction is added, this indicates an error in YANET's + * `total_table_t::compile()`. Ideally, this should not happen, + * but if it does, we log an error rather than crashing the application. + * In that case, we will use the last occurrence of FlowAction and proceed. + */ + if (indices.get().has_value()) { - return; + YANET_LOG_ERROR("Multiple FlowAction instances detected in the " + "path. Check total_table_t::compile(). Will use " + "the last occurrence.\n"); } + + handle_last_matters_action(action, variant_index); + } + else + { + static_assert(utils::always_false::value, "Not all unique actions with MAX_COUNT = 1 are properly categorized. " + "Please add the missing actions to either `is_first_matters` or `is_last_matters` " + "to ensure their index is tracked. Tracking the index of such actions could " + "enhance dataplane performance if this information is utilized in " + "`value_t::compile()`."); } + } + + // Only store the first occurrence. + template + void handle_first_matters_action(const Action& action, size_t variant_index) + { + auto& path_index = indices.get(); - if (std::holds_alternative(action.raw_action)) + if (!path_index.has_value()) { - check_state_index = path.size(); + add_to_path(action, variant_index); + path_index = path.size() - 1; } + // Ignore subsequent occurrences as we're only interested in the first. + } - action_counts[index]++; + // Only store the last occurrence. + template + void handle_last_matters_action(const Action& action, size_t variant_index) + { + // Remove the previous occurrence + remove(); + + // Add the new occurrence + auto& path_index = indices.get(); + + add_to_path(action, variant_index); + path_index = path.size() - 1; + } + + // Add the action to the path and increment its count. + void add_to_path(const Action& action, size_t variant_index) + { path.push_back(action); + action_counts[variant_index]++; + } + + // Retrieves reference to an action of type T. Action should exist. + template + T& get_action() + { + size_t path_index = indices.get().value(); + return std::get(path[path_index].raw_action); + } + + // Remove action at index and adjust saved indices + void remove_action_at(std::ptrdiff_t path_index) + { + path.erase(path.begin() + path_index); + adjust_indices_after_removal(path_index); + } + + // Loop through each type in FilteredTypes and adjust the corresponding index + void adjust_indices_after_removal(std::ptrdiff_t removed_index) + { + std::apply([&](auto&&... types) { + (adjust_index>(removed_index), ...); + }, + OptionalIndexMap::Types{}); + } + + template + void adjust_index(std::ptrdiff_t removed_index) + { + auto& path_index = indices.get(); + if (path_index && *path_index > removed_index) + { + *path_index -= 1; + } } }; @@ -288,7 +566,7 @@ class BaseActions public: BaseActions() = default; - BaseActions(acl::Actions&& actions) : + BaseActions(acl::IntermediateActions&& actions) : path_(std::move(actions.path)) {} [[nodiscard]] const Action& get_last() const @@ -346,10 +624,10 @@ class BaseActions public: BaseActions() = default; - BaseActions(acl::Actions&& actions) + BaseActions(acl::IntermediateActions&& actions) { - assert(actions.check_state_index.has_value()); - auto check_state_index = static_cast(actions.check_state_index.value()); + assert(actions.indices.get().has_value()); + auto check_state_index = actions.indices.get().value(); path_ = std::move(actions.path); diff --git a/common/tuple.h b/common/tuple.h new file mode 100644 index 00000000..68058c1c --- /dev/null +++ b/common/tuple.h @@ -0,0 +1,129 @@ +#pragma once + +#include +#include +#include + +namespace utils +{ + +/** + * @brief Helper structure to extract types from a std::variant. + * + * This template specialization will convert a std::variant into a std::tuple. + * + * Example: + * using Variant = std::variant; + * using TupleTypes = VariantTypes::type; // std::tuple + * + * @tparam Variant The std::variant type from which to extract types. + */ +template +struct VariantTypes; + +template +struct VariantTypes> +{ + using type = std::tuple; +}; + +/** + * @brief Helper structure to filter types in a parameter pack according to a trait. + * + * Filters the types in a parameter pack based on whether they satisfy a given trait. + * The result is a std::tuple containing only the types that satisfy the trait. + * + * Example: + * using Filtered = FilteredTuple::type; // std::tuple + * + * @tparam Trait The trait to filter types by. + * @tparam Ts The types to filter. + */ +template class Trait, typename... Ts> +struct FilteredTuple +{ + using type = decltype(std::tuple_cat( + std::conditional_t::value, std::tuple, std::tuple<>>{}...)); +}; + +/** + * @brief Helper structure to filter types in a std::tuple according to a trait. + * + * This specialization allows you to pass a std::tuple of types to FilteredTuple. + * + * Example: + * using MyTuple = std::tuple; + * using Filtered = FilteredTupleFromTuple::type; // std::tuple + * + * @tparam Trait The trait to filter types by. + * @tparam Tuple The std::tuple containing types to filter. + */ +template class Trait, typename... Ts> +struct FilteredTuple> +{ + using type = typename FilteredTuple::type; +}; + +/** + * @brief Helper structure to check if a type is present in a std::tuple. + * + * Example: + * using MyTuple = std::tuple; + * constexpr bool isInTuple = IsInTuple::value; // true + * + * @tparam T The type to check for. + * @tparam Tuple The std::tuple type to check within. + */ +template +struct IsInTuple; + +template +struct IsInTuple> : std::disjunction...> +{}; + +/** + * @brief Helper structure to get the index of a first occurence of a type in a std::tuple. + * + * This structure calculates the zero-based index of type T within a std::tuple. + * If the type is not found, a compile-time error is raised. + * + * Example: + * using MyTuple = std::tuple; + * constexpr std::size_t index = IndexOf::value; // index will be 1 + * + * @tparam T The type to find the index of. + * @tparam Tuple The std::tuple type containing the types. + */ +template +struct IndexOf; + +template +struct IndexOf> +{ +private: + template + struct Helper + { + static constexpr std::size_t value() + { + if constexpr (Index >= sizeof...(Types)) + { + static_assert(Index < sizeof...(Types), "Type T not found in tuple"); + return 0; // This line will never be reached + } + else if constexpr (std::is_same_v>>) + { + return Index; + } + else + { + return Helper::value(); + } + } + }; + +public: + static constexpr std::size_t value = Helper<0>::value(); +}; + +} // namespace utils diff --git a/common/unittest/meson.build b/common/unittest/meson.build index 53458d48..da6ace33 100644 --- a/common/unittest/meson.build +++ b/common/unittest/meson.build @@ -9,6 +9,8 @@ common_sources = files() sources = files('unittest.cpp', 'static_vector.cpp', 'shared_memory.cpp', + 'tuple.cpp', + 'variant_trait_map.cpp', ) arch = 'corei7' diff --git a/common/unittest/tuple.cpp b/common/unittest/tuple.cpp new file mode 100644 index 00000000..906d2d3c --- /dev/null +++ b/common/unittest/tuple.cpp @@ -0,0 +1,137 @@ +#include +#include + +#include "common/tuple.h" + +namespace +{ + +using utils::VariantTypes; + +TEST(VariantTypes, BasicExtraction) +{ + using Variant = std::variant; + using ExpectedTuple = std::tuple; + using ExtractedTuple = typename VariantTypes::type; + + static_assert(std::is_same_v, "Extracted tuple doesn't match expected"); +} + +TEST(VariantTypes, EmptyVariant) +{ + using Variant = std::variant<>; + using ExpectedTuple = std::tuple<>; + using ExtractedTuple = typename VariantTypes::type; + + static_assert(std::is_same_v, "Extracted empty tuple doesn't match expected"); +} + +using utils::FilteredTuple; + +TEST(FilteredTuple, FilteredTypes) +{ + using Filtered = typename FilteredTuple::type; + using Expected = std::tuple; + + static_assert(std::is_same_v, "Filtered tuple does not match expected"); + + using InputTuple = std::tuple; + using Filtered = typename FilteredTuple::type; + + static_assert(std::is_same_v, "Filtered tuple does not match expected"); +} + +TEST(FilteredTuple, NoMatch) +{ + using Filtered = typename FilteredTuple::type; + using Expected = std::tuple<>; + + static_assert(std::is_same_v, "Expected empty tuple when no types match"); + + using InputTuple = std::tuple; + using Filtered = typename FilteredTuple::type; + + static_assert(std::is_same_v, "Expected empty tuple when no types match"); +} + +TEST(FilteredTuple, AllMatch) +{ + using Filtered = typename FilteredTuple::type; + using Expected = std::tuple; + + static_assert(std::is_same_v, "Filtered tuple should include all types"); + + using InputTuple = std::tuple; + using Filtered = typename FilteredTuple::type; + + static_assert(std::is_same_v, "Filtered tuple should include all types"); +} + +using utils::IsInTuple; + +TEST(IsInTuple, TypeInTuple) +{ + using MyTuple = std::tuple; + + constexpr bool is_int = IsInTuple::value; + EXPECT_TRUE(is_int); + + constexpr bool is_float = IsInTuple::value; + EXPECT_FALSE(is_float); +} + +TEST(IsInTuple, EmptyTuple) +{ + using EmptyTuple = std::tuple<>; + + constexpr bool is_int = IsInTuple::value; + EXPECT_FALSE(is_int); +} + +using utils::IndexOf; + +TEST(IndexOf, IndexCalculation) +{ + using MyTuple = std::tuple; + + constexpr std::size_t index1 = IndexOf::value; + constexpr std::size_t index2 = IndexOf::value; + constexpr std::size_t index3 = IndexOf::value; + + EXPECT_EQ(index1, 0); + EXPECT_EQ(index2, 1); + EXPECT_EQ(index3, 2); +} + +TEST(IndexOf, MultipleOccurrences) +{ + using MyTuple = std::tuple; + + constexpr std::size_t index1 = IndexOf::value; + constexpr std::size_t index2 = IndexOf::value; + constexpr std::size_t index3 = IndexOf::value; + + EXPECT_EQ(index1, 0); // IndexOf should return the first occurrence of 'int' + EXPECT_EQ(index2, 1); // 'double' appears at index 1 + EXPECT_EQ(index3, 3); // 'char' appears at index 3 +} + +TEST(IndexOf, TypeNotFound) +{ + using MyTuple [[maybe_unused]] = std::tuple; + + // Expect compile-time failure for a type not present in the tuple + // The following line should trigger a compile-time error + // constexpr std::size_t invalidIndex = IndexOf::value; +} + +TEST(IndexOf, EmptyTuple) +{ + using EmptyTuple [[maybe_unused]] = std::tuple<>; + + // Expect a compile-time failure if trying to get index from an empty tuple + // Uncommenting the following line should trigger a compile-time error: + // constexpr std::size_t invalidIndex = IndexOf::value; +} + +} // namespace diff --git a/common/unittest/variant_trait_map.cpp b/common/unittest/variant_trait_map.cpp new file mode 100644 index 00000000..0aa76df2 --- /dev/null +++ b/common/unittest/variant_trait_map.cpp @@ -0,0 +1,110 @@ +#include +#include +#include + +#include "common/variant_trait_map.h" + +struct A +{}; +struct B +{}; +struct C +{}; +struct D +{}; + +// Trait that holds for A and C +template +struct SomeTrait : std::false_type +{}; + +template<> +struct SomeTrait : std::true_type +{}; + +template<> +struct SomeTrait : std::true_type +{}; + +// Trait that holds for all types +template +struct AllTrait : std::true_type +{}; + +// Trait that holds for no types +template +struct NoTrait : std::false_type +{}; + +using utils::VariantTraitMap; + +TEST(VariantTraitMap, BasicUsage) +{ + using MyVariant = std::variant; + VariantTraitMap map; + + map.get() = 42; + EXPECT_EQ(map.get(), 42); + + map.get() = 100; + EXPECT_EQ(map.get(), 100); + + ++map.get(); + EXPECT_EQ(map.get(), 43); + + map.set(200); + EXPECT_EQ(map.get(), 200); + + MyVariant runtime_variant(A{}); + + std::visit([&map](auto&& actual_variant) { + using T = std::decay_t; + + if constexpr (SomeTrait::value) + { + EXPECT_EQ(map.get(), 43); + } + }, + runtime_variant); + + // The following lines should cause compile-time errors if uncommented + // map.get(); // B does not satisfy SomeTrait + // map.get(); // D is not in MyVariant +} + +TEST(VariantTraitMap, NoTypesTrait) +{ + using MyVariant [[maybe_unused]] = std::variant; + // The following line should cause a compile-time error because no types satisfy the trait + // VariantTraitMap map; +} + +TEST(VariantTraitMap, SingleTypeVariant) +{ + using MyVariant = std::variant; + VariantTraitMap map; + + map.get() = 10; + EXPECT_EQ(map.get(), 10); +} + +TEST(VariantTraitMap, AllTypesTrait) +{ + using MyVariant = std::variant; + VariantTraitMap> map; + + map.get() = 1; + map.get() = -2; + + EXPECT_EQ(map.get().value(), 1); + EXPECT_EQ(map.get().value(), -2); + // will be default constructed + EXPECT_EQ(map.get(), std::nullopt); +} + +TEST(VariantTraitMap, EmptyVariant) +{ + using MyVariant [[maybe_unused]] = std::variant<>; + // The following line should cause a compile-time error because the variant is empty + // VariantTraitMap map; +} diff --git a/common/utils.h b/common/utils.h new file mode 100644 index 00000000..d979370f --- /dev/null +++ b/common/utils.h @@ -0,0 +1,14 @@ + +#pragma once + +#include + +namespace utils +{ + +template +struct always_false : std::false_type +{}; + +} +// namespace utils diff --git a/common/variant_trait_map.h b/common/variant_trait_map.h new file mode 100644 index 00000000..16fee0b4 --- /dev/null +++ b/common/variant_trait_map.h @@ -0,0 +1,126 @@ +#pragma once + +#include "tuple.h" + +namespace utils +{ + +/** + * @brief A class that maps types in a variant to a value V, based on a trait. + * + * This class uses a std::tuple to hold values of type V for each type in the + * variant that satisfies the trait. + * + * Example Usage: + * struct A {}; + * struct B {}; + * struct C {}; + * + * template + * struct IsSpecial : std::false_type {}; + * + * template<> + * struct IsSpecial : std::true_type {}; + * + * template<> + * struct IsSpecial : std::true_type {}; + * + * using Variant = std::variant; + * using MyMap = VariantTraitMap>; + * + * MyMap map; + * map.set(42); // Works because A satisfies IsSpecial + * map.set(99); // Works because C satisfies IsSpecial + * auto val = map.get(); // Returns 42 + * auto val = map.get(); // Compile-time error because B does not satisfy IsSpecial + * + * @tparam Variant The std::variant type containing different types. + * @tparam Trait The trait that defines which types are mapped to a value. + * @tparam V The value type to map the types to + */ +template class Trait, typename V> +class VariantTraitMap +{ +private: + using AllTypes = typename VariantTypes::type; + static_assert(std::tuple_size::value > 0, "Variant is empty"); + + using FilteredTypes = typename FilteredTuple::type; + static_assert(std::tuple_size::value > 0, "No types satisfy the trait"); + + template + struct StorageFromTuple; + + template + struct StorageFromTuple> + { + using type = std::tuple; + }; + + using Storage = typename StorageFromTuple::type; + + // Storage is a tuple of V types repeated for each type in FilteredTypes + Storage storage_; + +public: + using Types = FilteredTypes; + + VariantTraitMap() = default; + + /** + * @brief Retrieves the value associated with a specific type. + * + * This function retrieves the value of type V that is associated with a type T in the variant. + * + * Example: + * map.get() = 42; + * size_t value = map.get(); + * + * @tparam T The type to retrieve the value for. + * @return V& The value of type V associated with type T. + */ + template + V& get() + { + static_assert(IsInTuple::value, "Type T is not in Variant"); + static_assert(Trait::value, "Type T does not satisfy Trait"); + constexpr std::size_t index = IndexOf::value; + return std::get(storage_); + } + + /** + * @brief Retrieves the value associated with a specific type (const version). + * + * This function retrieves the const value of type V that is associated with a type T in the variant. + * + * @tparam T The type to retrieve the value for. + * @return const V& The const value of type V associated with type T. + */ + template + const V& get() const + { + static_assert(IsInTuple::value, "Type T is not in Variant"); + static_assert(Trait::value, "Type T does not satisfy Trait"); + constexpr std::size_t index = IndexOf::value; + return std::get(storage_); + } + + /** + * @brief Sets the value associated with a specific type. + * + * This function sets the value of type V for a specific type T in the variant. + * + * Example: + * map.set(42); + * + * @tparam T The type to set the value for. + * @param val The value of type V to set. + */ + template + void set(const V& val) + { + get() = val; + } +}; + +} // namespace utils diff --git a/controlplane/acl.cpp b/controlplane/acl.cpp index d767dbf9..0ceef7a5 100644 --- a/controlplane/acl.cpp +++ b/controlplane/acl.cpp @@ -325,6 +325,7 @@ struct firewall_rules_t case ipfw::rule_action_t::DUMP: case ipfw::rule_action_t::DENY: case ipfw::rule_action_t::CHECKSTATE: + case ipfw::rule_action_t::STATETIMEOUT: { // handle only meaning rules auto& ruleref = yanet_rules.emplace_back(rulep, configp); @@ -497,6 +498,14 @@ static bool unwind(int64_t start_from, firewall_rules_t& fw, const dispatcher_ru log || rule.log); ACL_DBGMSG("check_state gathered..."); } + else if (std::holds_alternative(rule.action)) + { + rules.emplace_back(std::move(result_filter), + std::get(rule.action), + ids, + log || rule.log); + ACL_DBGMSG("state_timeout gathered..."); + } else { rules.emplace_back(std::move(result_filter), diff --git a/controlplane/acl/rule.h b/controlplane/acl/rule.h index ea0beccd..598dd77e 100644 --- a/controlplane/acl/rule.h +++ b/controlplane/acl/rule.h @@ -1035,7 +1035,7 @@ const int64_t DISPATCHER = -1; // sense. // // Additionally, we might have another variant for representing rules that are suitable for execution in the dataplane. -using rule_action = std::variant; +using rule_action = std::variant; struct rule_t { @@ -1066,6 +1066,10 @@ struct rule_t rule_t(_filter, rule_action(action), ids, log) {} + rule_t(const ref_t& _filter, common::acl::state_timeout_t action, const ids_t& ids, bool log) : + rule_t(_filter, rule_action(action), ids, log) + {} + rule_t(const ref_t& _filter, int64_t num, int64_t skipto) : filter(_filter), action(skipto), @@ -1113,6 +1117,9 @@ struct rule_t case ipfw::rule_action_t::DUMP: action = common::acl::dump_t(std::get(rulep->action_arg)); break; + case ipfw::rule_action_t::STATETIMEOUT: + action = common::acl::state_timeout_t(std::get(rulep->action_arg)); + break; default: YANET_LOG_WARNING("unexpected rule action in rule '%s'\n", rulep->text.data()); return; @@ -1190,6 +1197,11 @@ struct rule_t { text = "check-state"; } + else if (std::holds_alternative(action)) + { + auto rule_action = std::get(action); + text = "state-timeout(" + std::to_string(rule_action.timeout) + ")"; + } else { auto arg = std::get(action); @@ -1375,11 +1387,17 @@ struct hash // predefined static constant as a unique identifier. hash_combine(h, common::acl::check_state_t::HASH_IDENTIFIER); } + else if (std::holds_alternative(r.action)) + { + auto action = std::get(r.action); + hash_combine(h, action.timeout); + } else { auto action = std::get(r.action); hash_combine(h, action.dump_id); } + if (r.filter) { hash_combine(h, **r.filter); diff --git a/controlplane/acl_compiler.cpp b/controlplane/acl_compiler.cpp index 599421e9..a3d07aa9 100644 --- a/controlplane/acl_compiler.cpp +++ b/controlplane/acl_compiler.cpp @@ -287,6 +287,10 @@ void compiler_t::collect(const std::vector& unwind_rules) { rule.value_filter_id = value.collect_initial_rule(*check_state); } + else if (auto state_timeout = std::get_if(&unwind_rule.action)) + { + rule.value_filter_id = value.collect_initial_rule(*state_timeout); + } } /// terminating diff --git a/controlplane/acl_value.cpp b/controlplane/acl_value.cpp index 7ecf6085..beabebb5 100644 --- a/controlplane/acl_value.cpp +++ b/controlplane/acl_value.cpp @@ -1,5 +1,7 @@ #include "acl_value.h" +#include "common/actions.h" + using namespace acl::compiler; value_t::value_t() @@ -32,25 +34,47 @@ void value_t::append_to_last(unsigned int rule_action_id) intermediate_vector.back().add(rule_actions[rule_action_id]); } +void value_t::ensure_termination(IntermediateActions& actions) +{ + auto& last_action = actions.path.back(); + + if (!std::holds_alternative(last_action.raw_action)) + { + // Adding default "drop" rule to the end + actions.add(rule_actions[0]); + } +} + +void value_t::move_timeout_from_state_timeout_to_flow(IntermediateActions& actions) +{ + if (const auto* state_timeout_action = actions.get()) + { + // Flow action should exist at this point + auto* flow_action = actions.get(); + flow_action->timeout = state_timeout_action->timeout; + + actions.remove(); + } +} + +void value_t::finalize_actions(IntermediateActions&& actions) +{ + if (actions.indices.get().has_value()) + { + vector.emplace_back(common::BaseActions(std::move(actions))); + } + else + { + vector.emplace_back(common::BaseActions(std::move(actions))); + } +} + void value_t::compile() { for (auto& intermediate_actions : intermediate_vector) { - auto last_action = intermediate_actions.path.back(); - - if (!std::visit([](const auto& act) { return act.terminating(); }, last_action.raw_action)) - { - // Adding default "drop" rule to the end - intermediate_actions.add(rule_actions[0]); - } - - if (intermediate_actions.check_state_index.has_value()) - { - vector.emplace_back(common::BaseActions(std::move(intermediate_actions))); - } - else - { - vector.emplace_back(common::BaseActions(std::move(intermediate_actions))); - } + ensure_termination(intermediate_actions); + move_timeout_from_state_timeout_to_flow(intermediate_actions); + finalize_actions(std::move(intermediate_actions)); } } diff --git a/controlplane/acl_value.h b/controlplane/acl_value.h index 0aff3e4e..1b152a47 100644 --- a/controlplane/acl_value.h +++ b/controlplane/acl_value.h @@ -7,6 +7,24 @@ namespace acl::compiler class value_t { + using IntermediateActions = common::acl::IntermediateActions; + + /** + * Ensures that the last action in each path is terminating. If not, a default "drop" rule is added. + */ + void ensure_termination(IntermediateActions& actions); + + /** + * Removes the StateTimeoutAction to reduce dataplane load + * by moving the timeout value directly into the FlowAction. + */ + void move_timeout_from_state_timeout_to_flow(IntermediateActions& actions); + + /** + * Finalizes the intermediate actions into a vector of BaseActions objects. + */ + void finalize_actions(IntermediateActions&& actions); + public: value_t(); @@ -18,7 +36,8 @@ class value_t { static_assert(std::is_same_v, common::globalBase::tFlow> || std::is_same_v, common::acl::dump_t> || - std::is_same_v, common::acl::check_state_t>, + std::is_same_v, common::acl::check_state_t> || + std::is_same_v, common::acl::state_timeout_t>, "Unsupported type in rule_action"); rule_actions.emplace_back(std::forward(rule)); @@ -31,14 +50,16 @@ class value_t public: // FIXME: I don't like this name.. Why was it called like that previously? - std::vector intermediate_vector; + std::vector intermediate_vector; // FIXME: I don't like this name.. Why was it called like that previously? std::vector vector; - // FIXME: The initial objects are created using collect_inintial_rule and stored here. - // Then we create copies in vector. I've tried to use shared pointers and/or real pointers, - // but then a bunch of issues arise when serializing a `vector` with stream.push/pop + // FIXME: + // The initial objects are created using collect_inintial_rule and stored here. + // Then we create copies in intermediate_vector. + // I've tried to use shared pointers and/or real pointers, but then a bunch + // of issues arise when serializing a `vector` with stream.push/pop std::vector rule_actions; void append_to_last(unsigned int rule_action_id); diff --git a/controlplane/unittest/acl.cpp b/controlplane/unittest/acl.cpp index a02e0bb0..a80b6fd6 100644 --- a/controlplane/unittest/acl.cpp +++ b/controlplane/unittest/acl.cpp @@ -700,4 +700,120 @@ add allow ip from any to any keep-state } } +TEST(ACL, StateTimeout_Basic) +{ + auto fw = make_default_acl(R"IPFW( +:BEGIN +add state-timeout 5000 ip from any to any +add allow ip from any to any keep-state +)IPFW"); + + std::map acls{{"acl0", std::move(fw)}}; + acl::result_t result; + acl::compile(acls, {{1, {{true, "vlan1"}}}}, result); + + ASSERT_EQ(result.acl_total_table.size(), 1); + + for (const auto& [total_table_key, total_table_value] : result.acl_total_table) + { + (void)total_table_key; + + const auto& value = result.acl_values[total_table_value]; + std::visit([&](const auto& actions) { + if constexpr (std::is_same_v, common::BaseActions>) + { + // Check that the regular path includes the allow action with the correct timeout + EXPECT_THAT(actions.get_actions().size(), 1); + const auto& flow_action = std::get(actions.get_actions()[0].raw_action); + EXPECT_EQ(flow_action.timeout, 5000); + + // Check that the check-state path includes only check-state action + EXPECT_THAT(actions.get_check_state_actions().size(), 1); + EXPECT_TRUE(std::holds_alternative(actions.get_check_state_actions().back().raw_action)); + } + }, + value); + } +} + +TEST(ACL, StateTimeout_RestrictiveSubset) +{ + auto fw = make_default_acl(R"IPFW( +:BEGIN +add state-timeout 8000 ip from any to any +add state-timeout 3000 ip from 192.168.1.0/24 to any +add allow ip from any to any keep-state +)IPFW"); + + std::map acls{{"acl0", std::move(fw)}}; + acl::result_t result; + acl::compile(acls, {{1, {{true, "vlan1"}}}}, result); + + ASSERT_EQ(result.acl_total_table.size(), 2); + + { + // First group, ip not in 192.168.1.0/24 + auto total_table_value = std::get<1>(result.acl_total_table[0]); + const auto& value = result.acl_values[total_table_value]; + + std::visit([&](const auto& actions) { + if constexpr (std::is_same_v, common::BaseActions>) + { + // Check that the regular path includes the allow action with the correct timeout + EXPECT_THAT(actions.get_actions().size(), 1); + const auto& flow_action = std::get(actions.get_actions()[0].raw_action); + EXPECT_EQ(flow_action.timeout, 8000); + + // Check that the check-state path includes only check-state action + EXPECT_THAT(actions.get_check_state_actions().size(), 1); + EXPECT_TRUE(std::holds_alternative(actions.get_check_state_actions().back().raw_action)); + } + }, + value); + } + { + // Second group, ip is in 192.168.1.0/24 + auto total_table_value = std::get<1>(result.acl_total_table[1]); + const auto& value = result.acl_values[total_table_value]; + + std::visit([&](const auto& actions) { + if constexpr (std::is_same_v, common::BaseActions>) + { + // Check that the regular path includes the allow action with the correct timeout + EXPECT_THAT(actions.get_actions().size(), 1); + + const auto& flow_action = std::get(actions.get_actions()[0].raw_action); + EXPECT_EQ(flow_action.timeout, 3000); // Verify that the timeout for 192.168.1.0/24 is 3000 + + // Check that the check-state path includes only check-state action + EXPECT_THAT(actions.get_check_state_actions().size(), 1); + EXPECT_TRUE(std::holds_alternative(actions.get_check_state_actions().back().raw_action)); + } + }, + value); + } +} + +TEST(ACL, StateTimeout_NoTimeout) +{ + auto fw = make_default_acl(R"IPFW( +:BEGIN +add allow ip from any to any +)IPFW"); + + std::map acls{{"acl0", std::move(fw)}}; + acl::result_t result; + acl::compile(acls, {{1, {{true, "vlan1"}}}}, result); + + ASSERT_EQ(result.acl_total_table.size(), 1); + + auto total_table_value = std::get<1>(result.acl_total_table[0]); + const auto& value = result.acl_values[total_table_value]; + + const auto& last_action = std::visit([&](const auto& actions) { return actions.get_last(); }, value); + const auto& flow_action = std::get(last_action.raw_action); + // Check that by default timeout in flow_action is std::nullopt + EXPECT_EQ(flow_action.timeout, std::nullopt); +} + } // namespace diff --git a/dataplane/action_dispatcher.h b/dataplane/action_dispatcher.h index ee399965..7ac0118d 100644 --- a/dataplane/action_dispatcher.h +++ b/dataplane/action_dispatcher.h @@ -89,6 +89,11 @@ struct ActionDispatcher ring.write(args.mbuf, flow.type); } + static void execute(const common::StateTimeoutAction& action, const common::globalBase::tFlow& flow, const ActionDispatcherArgs& args) + { + YANET_LOG_DEBUG("Asked to execute StateTimeoutAction, which should not occur. Check value_t::compile()\n"); + } + static void execute(const common::FlowAction& action, [[maybe_unused]] const common::globalBase::tFlow& flow, const ActionDispatcherArgs& args) { auto worker = args.worker; @@ -113,7 +118,7 @@ struct ActionDispatcher } if (action.flow.flags & (uint8_t)common::globalBase::eFlowFlags::recordstate) { - worker->acl_create_state(mbuf, acl_id, action.flow); + worker->acl_create_state(mbuf, acl_id, action.flow, action.timeout); } if constexpr (Direction == FlowDirection::Egress) diff --git a/dataplane/worker.cpp b/dataplane/worker.cpp index aa5b09ce..2f2a2875 100644 --- a/dataplane/worker.cpp +++ b/dataplane/worker.cpp @@ -4833,7 +4833,7 @@ inline cWorker::FlowFromState cWorker::acl_checkstate(rte_mbuf* mbuf, return {flow}; } -inline void cWorker::acl_create_state(rte_mbuf* mbuf, tAclId aclId, const common::globalBase::tFlow& flow) +inline void cWorker::acl_create_state(rte_mbuf* mbuf, tAclId aclId, const common::globalBase::tFlow& flow, std::optional timeout) { dataplane::metadata* metadata = YADECAP_METADATA(mbuf); @@ -4876,10 +4876,11 @@ inline void cWorker::acl_create_state(rte_mbuf* mbuf, tAclId aclId, const common key.src_port = 0; } - dataplane::globalBase::fw_state_value_t value; + dataplane::globalBase::fw_state_value_t value{}; value.type = static_cast(metadata->transport_headerType); value.owner = dataplane::globalBase::fw_state_owner_e::internal; acl_touch_state(mbuf, metadata, &value); + acl_fill_state_timeout(mbuf, metadata, &value, timeout); value.flow = flow; value.acl_id = aclId; value.last_sync = basePermanently.globalBaseAtomic->currentTime; @@ -4973,10 +4974,11 @@ inline void cWorker::acl_create_state(rte_mbuf* mbuf, tAclId aclId, const common key.src_port = 0; } - dataplane::globalBase::fw_state_value_t value; + dataplane::globalBase::fw_state_value_t value{}; value.type = static_cast(metadata->transport_headerType); value.owner = dataplane::globalBase::fw_state_owner_e::internal; acl_touch_state(mbuf, metadata, &value); + acl_fill_state_timeout(mbuf, metadata, &value, timeout); value.flow = flow; value.acl_id = aclId; value.last_sync = basePermanently.globalBaseAtomic->currentTime; @@ -5908,7 +5910,11 @@ YANET_NEVER_INLINE void cWorker::slowWorkerFarmHandleFragment(rte_mbuf* mbuf) inline void cWorker::acl_touch_state(rte_mbuf* mbuf, dataplane::metadata* metadata, dataplane::globalBase::fw_state_value_t* value) { value->last_seen = basePermanently.globalBaseAtomic->currentTime; - value->state_timeout = get_state_timeout(mbuf, metadata, acl_state_config); +} + +inline void cWorker::acl_fill_state_timeout(rte_mbuf* mbuf, dataplane::metadata* metadata, dataplane::globalBase::fw_state_value_t* value, std::optional timeout) +{ + value->state_timeout = timeout.has_value() ? timeout.value() : get_state_timeout(mbuf, metadata, acl_state_config); } inline void cWorker::balancer_touch_state(rte_mbuf* mbuf, dataplane::metadata* metadata, dataplane::globalBase::balancer_state_value_t* value) diff --git a/dataplane/worker.h b/dataplane/worker.h index 8595206c..e158641e 100644 --- a/dataplane/worker.h +++ b/dataplane/worker.h @@ -194,7 +194,7 @@ class cWorker inline FlowFromState acl_checkstate(rte_mbuf* mbuf, dataplane::globalBase::fw_state_value_t* value, dataplane::spinlock_nonrecursive_t* locker); inline FlowFromState acl_egress_checkstate(rte_mbuf* mbuf); inline FlowFromState acl_egress_checkstate(rte_mbuf* mbuf, dataplane::globalBase::fw_state_value_t* value, dataplane::spinlock_nonrecursive_t* locker); - inline void acl_create_state(rte_mbuf* mbuf, tAclId aclId, const common::globalBase::tFlow& flow); + inline void acl_create_state(rte_mbuf* mbuf, tAclId aclId, const common::globalBase::tFlow& flow, std::optional timeout); inline void acl_state_emit(tAclId aclId, const dataplane::globalBase::fw_state_sync_frame_t& frame); inline void acl_egress_entry(rte_mbuf* mbuf, tAclId aclId); @@ -203,6 +203,7 @@ class cWorker inline void acl_egress_flow(rte_mbuf* mbuf, const common::globalBase::tFlow& flow); inline void acl_log(rte_mbuf* mbuf, const common::globalBase::tFlow& flow, tAclId aclId); inline void acl_touch_state(rte_mbuf* mbuf, dataplane::metadata* metadata, dataplane::globalBase::fw_state_value_t* value); + inline void acl_fill_state_timeout(rte_mbuf* mbuf, dataplane::metadata* metadata, dataplane::globalBase::fw_state_value_t* value, std::optional timeout); inline void dregress_entry(rte_mbuf* mbuf); diff --git a/libfwparser/fw_config.cpp b/libfwparser/fw_config.cpp index a0c21485..0af9df32 100644 --- a/libfwparser/fw_config.cpp +++ b/libfwparser/fw_config.cpp @@ -1147,6 +1147,7 @@ bool fw_config_t::validate_rule(rule_ptr_t rulep) case rule_action_t::SKIPTO: case rule_action_t::DUMP: case rule_action_t::CHECKSTATE: + case rule_action_t::STATETIMEOUT: break; default: return true; diff --git a/libfwparser/fw_config.h b/libfwparser/fw_config.h index fd711d79..6f6b002f 100644 --- a/libfwparser/fw_config.h +++ b/libfwparser/fw_config.h @@ -170,6 +170,7 @@ enum class rule_action_t SRCPRJID, DSTPRJID, DUMP, + STATETIMEOUT, }; enum class rule_action_modifier_t diff --git a/libfwparser/fw_parser.y b/libfwparser/fw_parser.y index eb977810..f5e68008 100644 --- a/libfwparser/fw_parser.y +++ b/libfwparser/fw_parser.y @@ -131,6 +131,7 @@ SRCPRJID DSTPRJID RED ALL LMAX DSTIP6 SRCIP6 TCPSETMSS NAT64CLAT NAT64LSN NAT64STL NPTV6 SRCADDR QM DSTADDR SRCPORT DSTPORT SRCIP DSTIP EQUAL COMMA MINUS EOL M4LQ M4RQ DUMP + STATETIMEOUT // QUEUE could be an argument to *MASK %precedence QUEUE @@ -505,6 +506,12 @@ action: cfg.set_rule_action(rule_action_t::DUMP); } | + STATETIMEOUT NUMBER + { + cfg.set_rule_action(rule_action_t::STATETIMEOUT); + cfg.set_rule_action_arg($2); + } + | T_REJECT { cfg.set_rule_action(rule_action_t::UNREACH); diff --git a/libfwparser/token.l b/libfwparser/token.l index 0164da45..1cb0c44f 100644 --- a/libfwparser/token.l +++ b/libfwparser/token.l @@ -110,6 +110,7 @@ permit return ipfw::fw_parser_t::make_ALLOW(*ploc); deny_in return ipfw::fw_parser_t::make_DENY_IN(*ploc); deny|drop return ipfw::fw_parser_t::make_DENY(*ploc); dump return ipfw::fw_parser_t::make_DUMP(*ploc); +state-timeout return ipfw::fw_parser_t::make_STATETIMEOUT(*ploc); reject return ipfw::fw_parser_t::make_T_REJECT(*ploc); unreach return ipfw::fw_parser_t::make_UNREACH(*ploc); unreach6 return ipfw::fw_parser_t::make_UNREACH6(*ploc);