Skip to content

Perf eval #1404

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Perf eval #1404

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
e4aa296
tidy fix
jrajahalme Jun 19, 2025
1f74b63
accesslog: Add trace Envoy logging
jrajahalme Jun 18, 2025
9cea468
metadata: Eliminate separate 'ingress_source_identity'
jrajahalme Jun 18, 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
2 changes: 2 additions & 0 deletions cilium/accesslog.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ void AccessLog::log(AccessLog::Entry& log_entry, ::cilium::EntryType entry_type)
log_entry.request_logged_ = true;
}

ENVOY_LOG_MISC(trace, "Cilium Access log entry: {}", entry.DebugString());

// encode protobuf
std::string msg;
entry.SerializeToString(&msg);
Expand Down
27 changes: 14 additions & 13 deletions cilium/bpf_metadata.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,16 +403,18 @@ Config::extractSocketMetadata(Network::ConnectionSocket& socket) {
// Use a pointer as we may need to change the policy in the case of "North/South L7 LB" below.
const auto* policy = &getPolicy(pod_ip);

// Resolve the source security ID from conntrack map, or from ip cache
uint32_t source_identity = resolveSourceIdentity(*policy, sip, dip, is_ingress_, is_l7lb_);
// original_source_identity is set to the original source identity and is used in policy logs and
// access logging. This is typically the same as 'source_identity', but for north/south L7 LB,
// where the upstream source address is set to an Ingress IP, the 'source_identity' will be that
// if the Ingress IP. This original source identity is still used in policy logs and access
// logging even in that case.
uint32_t original_source_identity =
resolveSourceIdentity(*policy, sip, dip, is_ingress_, is_l7lb_);
uint32_t source_identity = original_source_identity;

// Resolve the destination security ID for egress traffic
uint32_t destination_identity = is_ingress_ ? 0 : resolvePolicyId(dip);

// ingress_source_identity is non-zero when the egress path l7 LB should also enforce
// the ingress path policy using the original source identity.
uint32_t ingress_source_identity = 0;

// Use the configured IPv4/IPv6 Ingress IPs as starting point for the sources addresses
IpAddressPair source_addresses(ipv4_source_address_, ipv6_source_address_);

Expand Down Expand Up @@ -463,7 +465,6 @@ Config::extractSocketMetadata(Network::ConnectionSocket& socket) {

// Enforce Ingress policy?
if (enforce_policy_on_l7lb_) {
ingress_source_identity = source_identity;
ingress_policy_name =
l7lb_policy_name_.empty() ? ingress_ip->addressAsString() : l7lb_policy_name_;
}
Expand Down Expand Up @@ -519,15 +520,15 @@ Config::extractSocketMetadata(Network::ConnectionSocket& socket) {
}

ENVOY_LOG(trace,
"cilium.bpf_metadata: mark {}, ingress_source_identity {}, source_identity {}, "
"cilium.bpf_metadata: mark {}, original_source_identity {}, source_identity {}, "
"is_ingress {}, is_l7lb_ {}, ingress_policy_name {}, port {}, pod_ip {}",
mark, ingress_source_identity, source_identity, is_ingress_, is_l7lb_,
mark, original_source_identity, source_identity, is_ingress_, is_l7lb_,
ingress_policy_name, dip->port(), pod_ip);
return {Cilium::BpfMetadata::SocketMetadata(
mark, ingress_source_identity, source_identity, is_ingress_, is_l7lb_, dip->port(),
std::move(pod_ip), std::move(ingress_policy_name), std::move(src_address),
std::move(source_addresses.ipv4_), std::move(source_addresses.ipv6_), std::move(dst_address),
shared_from_this(), proxy_id_, std::move(proxylib_l7proto), sni)};
mark, original_source_identity, is_ingress_, is_l7lb_, dip->port(), std::move(pod_ip),
std::move(ingress_policy_name), std::move(src_address), std::move(source_addresses.ipv4_),
std::move(source_addresses.ipv6_), std::move(dst_address), shared_from_this(), proxy_id_,
std::move(proxylib_l7proto), sni)};
}

Network::FilterStatus Instance::onAccept(Network::ListenerFilterCallbacks& cb) {
Expand Down
17 changes: 7 additions & 10 deletions cilium/bpf_metadata.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,17 @@ namespace BpfMetadata {
#define DEFAULT_CACHE_ENTRY_TTL_MS 3

struct SocketMetadata : public Logger::Loggable<Logger::Id::filter> {
SocketMetadata(uint32_t mark, uint32_t ingress_source_identity, uint32_t source_identity,
bool ingress, bool l7lb, uint16_t port, std::string&& pod_ip,
std::string&& ingress_policy_name,
SocketMetadata(uint32_t mark, uint32_t source_identity, bool ingress, bool l7lb, uint16_t port,
std::string&& pod_ip, std::string&& ingress_policy_name,
Network::Address::InstanceConstSharedPtr original_source_address,
Network::Address::InstanceConstSharedPtr source_address_ipv4,
Network::Address::InstanceConstSharedPtr source_address_ipv6,
Network::Address::InstanceConstSharedPtr original_dest_address,
const PolicyResolverSharedPtr& policy_resolver, uint32_t proxy_id,
std::string&& proxylib_l7_proto, absl::string_view sni)
: ingress_source_identity_(ingress_source_identity), source_identity_(source_identity),
ingress_(ingress), is_l7lb_(l7lb), port_(port), pod_ip_(std::move(pod_ip)),
ingress_policy_name_(std::move(ingress_policy_name)), proxy_id_(proxy_id),
proxylib_l7_proto_(std::move(proxylib_l7_proto)), sni_(sni),
: source_identity_(source_identity), ingress_(ingress), is_l7lb_(l7lb), port_(port),
pod_ip_(std::move(pod_ip)), ingress_policy_name_(std::move(ingress_policy_name)),
proxy_id_(proxy_id), proxylib_l7_proto_(std::move(proxylib_l7_proto)), sni_(sni),
policy_resolver_(policy_resolver), mark_(mark),
original_source_address_(std::move(original_source_address)),
source_address_ipv4_(std::move(source_address_ipv4)),
Expand All @@ -56,7 +54,7 @@ struct SocketMetadata : public Logger::Loggable<Logger::Id::filter> {

std::shared_ptr<Envoy::Cilium::CiliumPolicyFilterState> buildCiliumPolicyFilterState() {
return std::make_shared<Envoy::Cilium::CiliumPolicyFilterState>(
ingress_source_identity_, source_identity_, ingress_, is_l7lb_, port_, std::move(pod_ip_),
source_identity_, ingress_, is_l7lb_, port_, std::move(pod_ip_),
std::move(ingress_policy_name_), policy_resolver_, proxy_id_, sni_);
};

Expand Down Expand Up @@ -111,7 +109,6 @@ struct SocketMetadata : public Logger::Loggable<Logger::Id::filter> {
socket.connectionInfoProvider().restoreLocalAddress(original_dest_address_);
}

uint32_t ingress_source_identity_;
uint32_t source_identity_;
bool ingress_;
bool is_l7lb_;
Expand Down Expand Up @@ -165,8 +162,8 @@ class Config : public Cilium::PolicyResolver,
bool enforce_policy_on_l7lb_;
std::string l7lb_policy_name_;
std::chrono::milliseconds ipcache_entry_ttl_;
Random::RandomGenerator& random_;

Random::RandomGenerator& random_;
std::shared_ptr<const Cilium::NetworkPolicyMap> npmap_{};
Cilium::CtMapSharedPtr ct_maps_{};
Cilium::IpCacheSharedPtr ipcache_{};
Expand Down
52 changes: 24 additions & 28 deletions cilium/filter_state_cilium_policy.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,13 @@ bool CiliumPolicyFilterState::enforceNetworkPolicy(const Network::Connection& co
const auto& policy = policy_resolver_->getPolicy(ingress_policy_name_);

// Enforce ingress policy for Ingress, on the original destination port
if (ingress_source_identity_ != 0) {
auto ingress_port_policy = policy.findPortPolicy(true, port_);
if (!ingress_port_policy.allowed(proxy_id_, ingress_source_identity_, sni)) {
ENVOY_CONN_LOG(debug,
"Ingress network policy {} DROP for source identity and destination "
"reserved ingress identity: {} proxy_id: {} port: {} sni: \"{}\"",
conn, ingress_policy_name_, ingress_source_identity_, proxy_id_, port_, sni);
return false;
}
auto ingress_port_policy = policy.findPortPolicy(true, port_);
if (!ingress_port_policy.allowed(proxy_id_, source_identity_, sni)) {
ENVOY_CONN_LOG(debug,
"Ingress network policy {} DROP for source identity and destination "
"reserved ingress identity: {} proxy_id: {} port: {} sni: \"{}\"",
conn, ingress_policy_name_, source_identity_, proxy_id_, port_, sni);
return false;
}

// Enforce egress policy for Ingress
Expand Down Expand Up @@ -122,36 +120,34 @@ bool CiliumPolicyFilterState::enforceIngressHTTPPolicy(
const auto& policy = policy_resolver_->getPolicy(ingress_policy_name_);

// Enforce ingress policy for Ingress, on the original destination port
if (ingress_source_identity_ != 0) {
const auto port_policy = policy.findPortPolicy(true, port_);
if (!port_policy.hasHttpRules()) {
ENVOY_CONN_LOG(debug,
"cilium.l7policy: Ingress {} HTTP ingress policy enforcement skipped (no HTTP "
"rules) on proxy_id: {} id: {} port: {}",
conn, ingress_policy_name_, proxy_id_, ingress_source_identity_, port_);
return true;
}
const auto ingress_policy = policy.findPortPolicy(true, port_);
if (!ingress_policy.hasHttpRules()) {
ENVOY_CONN_LOG(debug,
"cilium.l7policy: Ingress {} HTTP ingress policy enforcement skipped (no HTTP "
"rules) on proxy_id: {} id: {} port: {}",
conn, ingress_policy_name_, proxy_id_, source_identity_, port_);
return true;
}

if (!port_policy.allowed(proxy_id_, ingress_source_identity_, headers, log_entry)) {
ENVOY_CONN_LOG(
debug,
"cilium.l7policy: Ingress {} HTTP ingress policy DROP on proxy_id: {} id: {} port: {}",
conn, ingress_policy_name_, proxy_id_, ingress_source_identity_, port_);
return false;
}
if (!ingress_policy.allowed(proxy_id_, source_identity_, headers, log_entry)) {
ENVOY_CONN_LOG(
debug,
"cilium.l7policy: Ingress {} HTTP ingress policy DROP on proxy_id: {} id: {} port: {}",
conn, ingress_policy_name_, proxy_id_, source_identity_, port_);
return false;
}

// Enforce egress policy for Ingress on the upstream destination identity and port
const auto port_policy = policy.findPortPolicy(false, destination_port);
if (!port_policy.hasHttpRules()) {
const auto egress_policy = policy.findPortPolicy(false, destination_port);
if (!egress_policy.hasHttpRules()) {
ENVOY_CONN_LOG(debug,
"cilium.l7policy: Ingress {} HTTP egress policy enforcement skipped (no HTTP "
"rules) on proxy_id: {} id: {} port: {}",
conn, ingress_policy_name_, proxy_id_, destination_identity, destination_port);
return true;
}

if (!port_policy.allowed(proxy_id_, destination_identity, headers, log_entry)) {
if (!egress_policy.allowed(proxy_id_, destination_identity, headers, log_entry)) {
ENVOY_CONN_LOG(
debug,
"cilium.l7policy: Ingress {} HTTP egress policy DROP on proxy_id: {} id: {} port: {}",
Expand Down
13 changes: 5 additions & 8 deletions cilium/filter_state_cilium_policy.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,13 @@ using PolicyResolverSharedPtr = std::shared_ptr<PolicyResolver>;
class CiliumPolicyFilterState : public StreamInfo::FilterState::Object,
public Logger::Loggable<Logger::Id::filter> {
public:
CiliumPolicyFilterState(uint32_t ingress_source_identity, uint32_t source_identity, bool ingress,
bool l7lb, uint16_t port, std::string&& pod_ip,
std::string&& ingress_policy_name,
CiliumPolicyFilterState(uint32_t source_identity, bool ingress, bool l7lb, uint16_t port,
std::string&& pod_ip, std::string&& ingress_policy_name,
const PolicyResolverSharedPtr& policy_resolver, uint32_t proxy_id,
absl::string_view sni)
: ingress_source_identity_(ingress_source_identity), source_identity_(source_identity),
ingress_(ingress), is_l7lb_(l7lb), port_(port), pod_ip_(std::move(pod_ip)),
ingress_policy_name_(std::move(ingress_policy_name)), proxy_id_(proxy_id), sni_(sni),
policy_resolver_(policy_resolver) {
: source_identity_(source_identity), ingress_(ingress), is_l7lb_(l7lb), port_(port),
pod_ip_(std::move(pod_ip)), ingress_policy_name_(std::move(ingress_policy_name)),
proxy_id_(proxy_id), sni_(sni), policy_resolver_(policy_resolver) {
ENVOY_LOG(
debug,
"Cilium CiliumPolicyFilterState(): source_identity: {}, "
Expand Down Expand Up @@ -79,7 +77,6 @@ class CiliumPolicyFilterState : public StreamInfo::FilterState::Object,
static const std::string& key();

// Additional ingress policy enforcement is performed if ingress_source_identity is non-zero
uint32_t ingress_source_identity_;
uint32_t source_identity_;
bool ingress_;
bool is_l7lb_;
Expand Down
2 changes: 1 addition & 1 deletion cilium/l7policy.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ Http::FilterHeadersStatus AccessFilter::decodeHeaders(Http::RequestHeaderMap& he
}

// Is there an Ingress policy?
if (policy_fs->ingress_policy_name_.length() > 0) {
if (!policy_fs->ingress_policy_name_.empty()) {
allowed = policy_fs->enforceIngressHTTPPolicy(conn.ref(), destination_identity,
destination_port, headers, *log_entry_);

Expand Down
1 change: 0 additions & 1 deletion cilium/network_policy.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
#include <openssl/mem.h>

#include <algorithm>
#include <cstdint>
#include <functional>
#include <memory>
#include <string>
Expand Down
2 changes: 1 addition & 1 deletion tests/bpf_metadata.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ TestConfig::extractSocketMetadata(Network::ConnectionSocket& socket) {
port, l7proto);

return {Cilium::BpfMetadata::SocketMetadata(
0, 0, source_identity, is_ingress_, is_l7lb_, port, std::move(pod_ip), "", nullptr, nullptr,
0, source_identity, is_ingress_, is_l7lb_, port, std::move(pod_ip), "", nullptr, nullptr,
nullptr, original_dst_address, shared_from_this(), 0, std::move(l7proto), "")};
}

Expand Down
15 changes: 5 additions & 10 deletions tests/metadata_config_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,13 +300,12 @@ TEST_F(MetadataConfigTest, NorthSouthL7LbMetadata) {
auto policy_fs = socket_metadata->buildCiliumPolicyFilterState();
EXPECT_NE(nullptr, policy_fs);

EXPECT_EQ(8, policy_fs->source_identity_);
EXPECT_EQ(12345678, policy_fs->source_identity_);
EXPECT_EQ(false, policy_fs->ingress_);
EXPECT_EQ(true, policy_fs->is_l7lb_);
EXPECT_EQ(80, policy_fs->port_);
EXPECT_EQ("", policy_fs->pod_ip_);
EXPECT_EQ("", policy_fs->ingress_policy_name_);
EXPECT_EQ(0, policy_fs->ingress_source_identity_);

auto source_addresses_socket_option = socket_metadata->buildSourceAddressSocketOption(-1);
EXPECT_NE(nullptr, source_addresses_socket_option);
Expand Down Expand Up @@ -341,13 +340,12 @@ TEST_F(MetadataConfigTest, NorthSouthL7LbIngressEnforcedMetadata) {
auto policy_fs = socket_metadata->buildCiliumPolicyFilterState();
EXPECT_NE(nullptr, policy_fs);

EXPECT_EQ(8, policy_fs->source_identity_);
EXPECT_EQ(12345678, policy_fs->source_identity_);
EXPECT_EQ(false, policy_fs->ingress_);
EXPECT_EQ(true, policy_fs->is_l7lb_);
EXPECT_EQ(80, policy_fs->port_);
EXPECT_EQ("", policy_fs->pod_ip_);
EXPECT_EQ("10.1.1.42", policy_fs->ingress_policy_name_);
EXPECT_EQ(12345678, policy_fs->ingress_source_identity_);

AccessLog::Entry log_entry;
log_entry.entry_.set_policy_name("pod");
Expand Down Expand Up @@ -411,13 +409,12 @@ TEST_F(MetadataConfigTest, NorthSouthL7LbPodAndIngressEnforcedMetadata) {
auto policy_fs = socket_metadata->buildCiliumPolicyFilterState();
EXPECT_NE(nullptr, policy_fs);

EXPECT_EQ(8, policy_fs->source_identity_);
EXPECT_EQ(9999, policy_fs->source_identity_);
EXPECT_EQ(false, policy_fs->ingress_);
EXPECT_EQ(true, policy_fs->is_l7lb_);
EXPECT_EQ(80, policy_fs->port_);
EXPECT_EQ("192.168.1.1", policy_fs->pod_ip_);
EXPECT_EQ("10.1.1.42", policy_fs->ingress_policy_name_);
EXPECT_EQ(9999, policy_fs->ingress_source_identity_);

AccessLog::Entry log_entry;
log_entry.entry_.set_policy_name("pod");
Expand Down Expand Up @@ -482,13 +479,12 @@ TEST_F(MetadataConfigTest, NorthSouthL7LbIngressEnforcedCIDRMetadata) {
auto policy_fs = socket_metadata->buildCiliumPolicyFilterState();
EXPECT_NE(nullptr, policy_fs);

EXPECT_EQ(8, policy_fs->source_identity_);
EXPECT_EQ(2, policy_fs->source_identity_);
EXPECT_EQ(false, policy_fs->ingress_);
EXPECT_EQ(true, policy_fs->is_l7lb_);
EXPECT_EQ(80, policy_fs->port_);
EXPECT_EQ("", policy_fs->pod_ip_);
EXPECT_EQ("10.1.1.42", policy_fs->ingress_policy_name_);
EXPECT_EQ(2, policy_fs->ingress_source_identity_);

AccessLog::Entry log_entry;
log_entry.entry_.set_policy_name("pod");
Expand Down Expand Up @@ -590,13 +586,12 @@ TEST_F(MetadataConfigTest, EastWestL7LbMetadataNoOriginalSource) {
auto policy_fs = socket_metadata->buildCiliumPolicyFilterState();
EXPECT_NE(nullptr, policy_fs);

EXPECT_EQ(8, policy_fs->source_identity_);
EXPECT_EQ(111, policy_fs->source_identity_);
EXPECT_EQ(false, policy_fs->ingress_);
EXPECT_EQ(true, policy_fs->is_l7lb_);
EXPECT_EQ(80, policy_fs->port_);
EXPECT_EQ("10.1.1.1", policy_fs->pod_ip_);
EXPECT_EQ("", policy_fs->ingress_policy_name_);
EXPECT_EQ(0, policy_fs->ingress_source_identity_);

auto source_addresses_socket_option = socket_metadata->buildSourceAddressSocketOption(-1);
EXPECT_NE(nullptr, source_addresses_socket_option);
Expand Down
Loading