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

Fix 5.10+ complexity issue with kubeProxyReplacement=disabled #16084

Merged
merged 3 commits into from
May 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 1 addition & 2 deletions bpf/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,12 @@ MAX_BASE_OPTIONS += -DHAVE_LPM_TRIE_MAP_TYPE=1 -DHAVE_LRU_HASH_MAP_TYPE=1 \
ifeq ("$(KERNEL)","54")
MAX_BASE_OPTIONS += -DENABLE_BANDWIDTH_MANAGER=1
else ifeq ("$(KERNEL)","netnext")
# We need to define ENABLE_HOST_SERVICES_FULL to work around complexity issue #14234.
# We define ENABLE_CUSTOM_CALLS only for net-next (vs. net-next and 4.19) to work
# around program size issue #15539.
# We define ETH_HLEN only for net-next, as bpf_skb_change_head is non-available
# on 4.{9,19}.
MAX_BASE_OPTIONS += -DENABLE_TPROXY=1 -DENABLE_REDIRECT_FAST=1 -DENABLE_BANDWIDTH_MANAGER=1 \
-DENABLE_HOST_SERVICES_FULL=1 -DENABLE_CUSTOM_CALLS=1 -DETH_HLEN=0
-DENABLE_CUSTOM_CALLS=1 -DETH_HLEN=0
endif
endif

Expand Down
2 changes: 2 additions & 0 deletions bpf/Makefile.bpf
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ LLC_FLAGS := -march=bpf -mattr=dwarfris
# Mimics the mcpu values set by cilium-agent. See GetBPFCPU().
ifeq ("$(KERNEL)","49")
LLC_FLAGS += -mcpu=v1
else ifeq ("$(KERNEL)","netnext")
LLC_FLAGS += -mcpu=v3
else
LLC_FLAGS += -mcpu=v2
endif
Expand Down
2 changes: 1 addition & 1 deletion bpf/include/bpf/ctx/skb.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ ctx_adjust_troom(struct __sk_buff *ctx, const __s32 len_diff)
return skb_change_tail(ctx, ctx->len + len_diff, 0);
}

static __always_inline __maybe_unused __u32
static __always_inline __maybe_unused __u64
ctx_full_len(const struct __sk_buff *ctx)
{
return ctx->len;
Expand Down
2 changes: 1 addition & 1 deletion bpf/include/bpf/ctx/xdp.h
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ ctx_redirect(const struct xdp_md *ctx, int ifindex, const __u32 flags)
return XDP_TX;
}

static __always_inline __maybe_unused __u32
static __always_inline __maybe_unused __u64
ctx_full_len(const struct xdp_md *ctx)
pchaigno marked this conversation as resolved.
Show resolved Hide resolved
{
/* No non-linear section in XDP. */
Expand Down
2 changes: 1 addition & 1 deletion bpf/lib/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ ____revalidate_data_pull(struct __ctx_buff *ctx, void **data_, void **data_end_,
void **l3, const __u32 l3_len, const bool pull,
__u8 eth_hlen)
{
const __u32 tot_len = eth_hlen + l3_len;
const __u64 tot_len = eth_hlen + l3_len;
void *data_end;
void *data;

Expand Down
2 changes: 1 addition & 1 deletion bpf/lib/csum.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ static __always_inline void csum_l4_offset_and_flags(__u8 nexthdr,
* @arg to To value or a csum diff
* @arg flags Additional flags to be passed to l4_csum_replace()
*/
static __always_inline int csum_l4_replace(struct __ctx_buff *ctx, int l4_off,
static __always_inline int csum_l4_replace(struct __ctx_buff *ctx, __u64 l4_off,
const struct csum_offset *csum,
__be32 from, __be32 to, int flags)
{
Expand Down
3 changes: 2 additions & 1 deletion bpf/lib/encap.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,11 @@ encap_remap_v6_host_address(struct __ctx_buff *ctx __maybe_unused,
void *data, *data_end;
struct ipv6hdr *ip6;
union v6addr *which;
__u32 off, noff;
__u8 nexthdr;
__u16 proto;
__be32 sum;
__u32 noff;
__u64 off;
int ret;

validate_ethertype(ctx, &proto);
Expand Down
6 changes: 3 additions & 3 deletions bpf/lib/metrics.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
* is the drop error code.
* Update the metrics map.
*/
static __always_inline void update_metrics(__u32 bytes, __u8 direction,
static __always_inline void update_metrics(__u64 bytes, __u8 direction,
__u8 reason)
{
struct metrics_value *entry, newEntry = {};
Expand All @@ -34,10 +34,10 @@ static __always_inline void update_metrics(__u32 bytes, __u8 direction,
entry = map_lookup_elem(&METRICS_MAP, &key);
if (entry) {
entry->count += 1;
entry->bytes += (__u64)bytes;
entry->bytes += bytes;
} else {
newEntry.count = 1;
newEntry.bytes = (__u64)bytes;
newEntry.bytes = bytes;
michi-covalent marked this conversation as resolved.
Show resolved Hide resolved
map_update_elem(&METRICS_MAP, &key, &newEntry, 0);
}
}
Expand Down
2 changes: 1 addition & 1 deletion bpf/lib/nat.h
Original file line number Diff line number Diff line change
Expand Up @@ -512,8 +512,8 @@ static __always_inline __maybe_unused int snat_v4_process(struct __ctx_buff *ctx
__be16 sport;
__be16 dport;
} l4hdr;
__u32 off;
bool icmp_echoreply = false;
__u64 off;
int ret;

build_bug_on(sizeof(struct ipv4_nat_entry) > 64);
Expand Down
4 changes: 2 additions & 2 deletions bpf/lib/nodeport.h
Original file line number Diff line number Diff line change
Expand Up @@ -442,8 +442,8 @@ static __always_inline int dsr_reply_icmp6(struct __ctx_buff *ctx,
const __s32 orig_dgram = 64, off = ETH_HLEN;
const __u32 l3_max = sizeof(*ip6) + orig_dgram;
__be16 type = bpf_htons(ETH_P_IPV6);
__s32 len_new = off + sizeof(*ip6) + orig_dgram;
__s32 len_old = ctx_full_len(ctx);
__u64 len_new = off + sizeof(*ip6) + orig_dgram;
__u64 len_old = ctx_full_len(ctx);
void *data_end = ctx_data_end(ctx);
pchaigno marked this conversation as resolved.
Show resolved Hide resolved
void *data = ctx_data(ctx);
__wsum wsum;
Expand Down
15 changes: 13 additions & 2 deletions pkg/datapath/loader/compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,20 @@ var (
func GetBPFCPU() string {
probeCPUOnce.Do(func() {
if !option.Config.DryMode {
manager := probes.NewProbeManager()
// We can probe the availability of BPF instructions indirectly
// based on what kernel helpers are available since both were
// added in the same release, that is, 4.14.
// based on what kernel helpers are available when both were
// added in the same release.
// We want to enable v3 only on kernels 5.10+ where we have
// tested it and need it to work around complexity issues.
if h := manager.GetHelpers("sched_cls"); h != nil {
if _, ok := h["bpf_redirect_neigh"]; ok {
nameBPFCPU = "v3"
return
}
}
// We want to enable v2 on all kernels that support it, that is,
// kernels 4.14+.
if h := probes.NewProbeManager().GetHelpers("xdp"); h != nil {
Copy link
Member

Choose a reason for hiding this comment

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

nit: saw too late, but could you do a small cleanup in that the 2nd GetHelpers() could also use prior defined manager?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, ***. I defined that manager variable for nothing 🤦

TBH, I don't think it matters much: probes.NewProbeManager() is a singleton ;-)

if _, ok := h["bpf_redirect_map"]; ok {
nameBPFCPU = "v2"
Expand Down