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

fix host to self without ctlb #8189

Merged
merged 6 commits into from Nov 9, 2023
Merged

fix host to self without ctlb #8189

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
da6bfc1
[BPF] when BPFConnectTimeLoadBalancing=TCP do not process TCP on lo
tomastigera Nov 7, 2023
de949ac
[BPF] do not look at policy status at dev lo
tomastigera Nov 7, 2023
e7d6573
[BPF] Fix overriding incorrect settings for BPFConnectTimeLoadBalancing
tomastigera Nov 7, 2023
57d2f46
fix defaulting ctlb on 1+ nodes in ebpf runs
tomastigera Nov 8, 2023
4c5912b
[BPF] do not do hep_rpf_check() on lo in conntrack
tomastigera Nov 8, 2023
bfad43d
fix a ctlb config warning message
tomastigera Nov 9, 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: 3 additions & 3 deletions felix/bpf-gpl/conntrack.h
View file
Open in desktop
Expand Up @@ -827,7 +827,7 @@ static CALI_BPF_INLINE struct calico_ct_result calico_ct_lookup(struct cali_tc_c
CALI_DEBUG("Packet returned from tunnel %x\n", debug_ip(ctx->state->tun_ip));
} else if (CALI_F_TO_HOST || (skb_from_host(ctx->skb) && result.flags & CALI_CT_FLAG_HOST_PSNAT)) {
/* Source of the packet is the endpoint, so check the src approval flag. */
if (src_to_dst->approved) {
if (CALI_F_LO || src_to_dst->approved) {
CALI_CT_VERB("Packet approved by this workload's policy.\n");
} else {
/* Only approved by the other side (so far)? Unlike
Expand All @@ -840,7 +840,7 @@ static CALI_BPF_INLINE struct calico_ct_result calico_ct_lookup(struct cali_tc_c
}
} else if (CALI_F_FROM_HOST) {
/* Dest of the packet is the endpoint, so check the dest approval flag. */
if (dst_to_src->approved) {
if (CALI_F_LO || dst_to_src->approved) {
// Packet was approved by the policy attached to this endpoint.
CALI_CT_VERB("Packet approved by this workload's policy.\n");
} else {
Expand Down Expand Up @@ -893,7 +893,7 @@ static CALI_BPF_INLINE struct calico_ct_result calico_ct_lookup(struct cali_tc_c
*
* Do not check if packets are returning from the NP vxlan tunnel.
*/
if (!same_if && !ret_from_tun && !hep_rpf_check(ctx) && !CALI_F_NAT_IF) {
if (!same_if && !ret_from_tun && !hep_rpf_check(ctx) && !CALI_F_NAT_IF && !CALI_F_LO) {
ct_result_set_flag(result.rc, CT_RES_RPF_FAILED);
} else {
src_to_dst->ifindex = ifindex;
Expand Down
1 change: 1 addition & 0 deletions felix/bpf-gpl/globals.h
View file
Open in desktop
Expand Up @@ -49,6 +49,7 @@ enum cali_globals_flags {
CALI_GLOBALS_RPF_OPTION_STRICT = 0x00000020,
CALI_GLOBALS_RESERVED7 = 0x00000040,
CALI_GLOBALS_NO_DSR_CIDRS = 0x00000080,
CALI_GLOBALS_LO_UDP_ONLY = 0x00000100,
};

struct cali_ctlb_globals {
Expand Down
9 changes: 5 additions & 4 deletions felix/bpf-gpl/tc.c
View file
Open in desktop
Expand Up @@ -74,11 +74,7 @@ int calico_tc_main(struct __sk_buff *skb)
struct cali_tc_ctx *ctx = &_ctx;

CALI_DEBUG("New packet at ifindex=%d; mark=%x\n", skb->ifindex, skb->mark);
#ifdef IPVER6
parse_packet_ip_v6(ctx);
#else
parse_packet_ip(ctx);
#endif
CALI_DEBUG("Final result=ALLOW (%d). Bypass mark set.\n", CALI_REASON_BYPASS);
}
return TC_ACT_UNSPEC;
Expand Down Expand Up @@ -195,6 +191,11 @@ static CALI_BPF_INLINE int pre_policy_processing(struct cali_tc_ctx *ctx)
/* Copy fields that are needed by downstream programs from the packet to the state. */
tc_state_fill_from_iphdr(ctx);

if (CALI_F_LO && (GLOBAL_FLAGS & CALI_GLOBALS_LO_UDP_ONLY) && ctx->state->ip_proto != IPPROTO_UDP) {
CALI_DEBUG("Allowing because it is not UDP\n");
goto allow;
}

/* Parse out the source/dest ports (or type/code for ICMP). */
switch (tc_state_fill_from_nexthdr(ctx, dnat_should_decap())) {
case PARSING_ERROR:
Expand Down
1 change: 1 addition & 0 deletions felix/bpf/libbpf/libbpf.go
View file
Open in desktop
Expand Up @@ -382,6 +382,7 @@ const (
GlobalsRPFOptionEnabled uint32 = C.CALI_GLOBALS_RPF_OPTION_ENABLED
GlobalsRPFOptionStrict uint32 = C.CALI_GLOBALS_RPF_OPTION_STRICT
GlobalsNoDSRCidrs uint32 = C.CALI_GLOBALS_NO_DSR_CIDRS
GlobalsLoUDPOnly uint32 = C.CALI_GLOBALS_LO_UDP_ONLY
)

func TcSetGlobals(
Expand Down
1 change: 1 addition & 0 deletions felix/bpf/libbpf/libbpf_stub.go
View file
Open in desktop
Expand Up @@ -135,6 +135,7 @@ const (
GlobalsRPFOptionEnabled uint32 = 16
GlobalsRPFOptionStrict uint32 = 32
GlobalsNoDSRCidrs uint32 = 12345
GlobalsLoUDPOnly uint32 = 12345
)

func TcSetGlobals(_ *Map, globalData *TcGlobalData) error {
Expand Down
5 changes: 5 additions & 0 deletions felix/bpf/tc/attach.go
View file
Open in desktop
Expand Up @@ -59,6 +59,7 @@ type AttachPoint struct {
RPFEnforceOption uint8
NATin uint32
NATout uint32
UDPOnly bool
}

var ErrDeviceNotFound = errors.New("device not found")
Expand Down Expand Up @@ -430,6 +431,10 @@ func (ap *AttachPoint) ConfigureProgram(m *libbpf.Map) error {
globalData.Flags |= libbpf.GlobalsRPFOptionEnabled
}

if ap.UDPOnly {
globalData.Flags |= libbpf.GlobalsLoUDPOnly
}

globalData.HostTunnelIP = globalData.HostIP

if ap.HostTunnelIP != nil {
Expand Down
10 changes: 6 additions & 4 deletions felix/dataplane/linux/bpf_ep_mgr.go
View file
Open in desktop
Expand Up @@ -472,11 +472,10 @@ func newBPFEndpointManager(
m.dp = m
}

if config.BPFHostNetworkedNAT == string(apiv3.BPFHostNetworkedNATEnabled) {
if config.BPFConnTimeLB == string(apiv3.BPFConnectTimeLBTCP) {
m.hostNetworkedNATMode = hostNetworkedNATUDPOnly
} else if config.BPFHostNetworkedNAT == string(apiv3.BPFHostNetworkedNATEnabled) {
m.hostNetworkedNATMode = hostNetworkedNATEnabled
if config.BPFConnTimeLB == string(apiv3.BPFConnectTimeLBTCP) {
m.hostNetworkedNATMode = hostNetworkedNATUDPOnly
}
}

if m.hostNetworkedNATMode != hostNetworkedNATDisabled {
Expand Down Expand Up @@ -2110,6 +2109,9 @@ func (m *bpfEndpointManager) calculateTCAttachPoint(policyDirection PolDirection
endpointType = tcdefs.EpTypeLO
ap.HostTunnelIP = m.tunnelIP
log.Debugf("Setting tunnel ip %s on ap %s", m.tunnelIP, ifaceName)
if m.hostNetworkedNATMode == hostNetworkedNATUDPOnly {
ap.UDPOnly = true
}
} else if ifaceName == "tunl0" {
if m.Features.IPIPDeviceIsL3 {
endpointType = tcdefs.EpTypeL3Device
Expand Down
19 changes: 12 additions & 7 deletions felix/dataplane/linux/int_dataplane.go
View file
Open in desktop
Expand Up @@ -718,8 +718,8 @@ func NewIntDataplaneDriver(config Config) *InternalDataplane {

if config.BPFConnTimeLB == string(apiv3.BPFConnectTimeLBDisabled) &&
config.BPFHostNetworkedNAT == string(apiv3.BPFHostNetworkedNATDisabled) {
log.Warn("Access to services from host networked process wont work, forcing hostnetworked NAT to Enabled")
config.BPFHostNetworkedNAT = string(apiv3.BPFHostNetworkedNATEnabled)
log.Warn("Host-networked access to services from host networked process won't properly " +
Copy link
Member

Choose a reason for hiding this comment

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

won't work properly? If both are disabled, we are going to warn and leave it as it is?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, this is a full rollback to the original behaviour and might be desired by the user if things regress

"- BPFHostNetworkedNAT is disabled.")
}

bpfEndpointManager, err = newBPFEndpointManager(
Expand Down Expand Up @@ -799,14 +799,18 @@ func NewIntDataplaneDriver(config Config) *InternalDataplane {
// The above cases are invalid configuration. Revert to CTLB enabled.
if config.BPFHostNetworkedNAT == string(apiv3.BPFHostNetworkedNATEnabled) {
if config.BPFConnTimeLB == string(apiv3.BPFConnectTimeLBEnabled) {
log.Warn("Access to services may not work properly, reverting to default CTLB configuration")
config.BPFConnTimeLB = string(apiv3.BPFConnectTimeLBTCP)
log.Warn("Both BPFConnectTimeLoadBalancing and BPFHostNetworkedNATWithoutCTLB are enabled. " +
"Disabling BPFHostNetworkedNATWithoutCTLB. " +
"Set BPFConnectTimeLoadBalancing=TCP if you want disable it for other protocols.")
config.BPFHostNetworkedNAT = string(apiv3.BPFHostNetworkedNATDisabled)
}
} else {
if config.BPFConnTimeLB != string(apiv3.BPFConnectTimeLBEnabled) {
log.Warn("Access to services may not work properly, reverting to default CTLB configuration")
config.BPFConnTimeLB = string(apiv3.BPFConnectTimeLBTCP)
config.BPFHostNetworkedNAT = string(apiv3.BPFHostNetworkedNATEnabled)
if config.BPFHostNetworkedNAT == string(apiv3.BPFHostNetworkedNATDisabled) {
log.Warnf("Access to (some) services from host may not work properly because "+
"BPFConnectTimeLoadBalancing is %s and BPFHostNetworkedNATWithoutCTLB is disabled",
config.BPFConnTimeLB)
}
}
}

Expand All @@ -832,6 +836,7 @@ func NewIntDataplaneDriver(config Config) *InternalDataplane {
if err != nil {
log.WithError(err).Panic("BPFConnTimeLBEnabled but failed to attach connect-time load balancer, bailing out.")
}
log.Infof("Connect time load balancer enabled: %s", config.BPFConnTimeLB)
} else {
// Deactivate the connect-time load balancer.
err = nat.RemoveConnectTimeLoadBalancer(config.BPFCgroupV2)
Expand Down
6 changes: 5 additions & 1 deletion felix/fv/bpf_test.go
View file
Open in desktop
Expand Up @@ -395,14 +395,17 @@ func describeBPFTests(opts ...bpfTestOpt) bool {
}

if !testOpts.connTimeEnabled {
options.ExtraEnvVars["FELIX_BPFHostNetworkedNATWithoutCTLB"] = string(api.BPFHostNetworkedNATEnabled)
options.ExtraEnvVars["FELIX_BPFConnectTimeLoadBalancing"] = string(api.BPFConnectTimeLBDisabled)
options.ExtraEnvVars["FELIX_BPFHostNetworkedNATWithoutCTLB"] = string(api.BPFHostNetworkedNATEnabled)
if testOpts.protocol == "udp" {
options.ExtraEnvVars["FELIX_BPFConnectTimeLoadBalancing"] = string(api.BPFConnectTimeLBTCP)
}
} else {
options.ExtraEnvVars["FELIX_BPFConnectTimeLoadBalancing"] = string(api.BPFConnectTimeLBEnabled)
options.ExtraEnvVars["FELIX_BPFHostNetworkedNATWithoutCTLB"] = string(api.BPFHostNetworkedNATDisabled)
if testOpts.protocol == "tcp" {
options.ExtraEnvVars["FELIX_BPFConnectTimeLoadBalancing"] = string(api.BPFConnectTimeLBTCP)
}
}

})
Expand Down Expand Up @@ -1193,6 +1196,7 @@ func describeBPFTests(opts ...bpfTestOpt) bool {
It("should allow host -> host", func() {
// XXX as long as there is no HEP policy
// using hostW as a sink
cc.Expect(Some, tc.Felixes[0], hostW[0])
cc.Expect(Some, tc.Felixes[0], hostW[1])
cc.Expect(Some, tc.Felixes[1], hostW[0])
cc.CheckConnectivity()
Expand Down
4 changes: 3 additions & 1 deletion felix/fv/infrastructure/topology.go
View file
Open in desktop
Expand Up @@ -290,7 +290,9 @@ func StartNNodeTopology(n int, opts TopologyOptions, infra DatastoreInfra) (tc T
if i > 0 {
optsPerFelix[i].ExtraEnvVars["FELIX_BPFConnectTimeLoadBalancingEnabled"] = "false"
optsPerFelix[i].ExtraEnvVars["FELIX_BPFConnectTimeLoadBalancing"] = string(api.BPFConnectTimeLBDisabled)
optsPerFelix[i].ExtraEnvVars["FELIX_BPFHostNetworkedNATWithoutCTLB"] = string(api.BPFHostNetworkedNATDisabled)
if optsPerFelix[i].ExtraEnvVars["FELIX_BPFHostNetworkedNATWithoutCTLB"] == "" {
optsPerFelix[i].ExtraEnvVars["FELIX_BPFHostNetworkedNATWithoutCTLB"] = string(api.BPFHostNetworkedNATDisabled)
}
optsPerFelix[i].ExtraEnvVars["FELIX_DebugSkipCTLBCleanup"] = "true"
}
}
Expand Down