Skip to content

Commit

Permalink
bpf: remap MARK_MAGIC_SNAT_DONE marker to avoid conflicts
Browse files Browse the repository at this point in the history
[ upstream commit f2bcb69 ]

Commit f25d8b9 ("bpf: Preserve source identity for hairpin via stack")
recently broke BPF NodePort since the use of MARK_MAGIC_SNAT_DONE is now in
conflict/overlapping with MARK_MAGIC_IDENTITY.

In tc egress hooks of bpf_{netdev,overlay} we have a check whether the SNAT
is not needed via if ((ctx->mark & MARK_MAGIC_SNAT_DONE) == MARK_MAGIC_SNAT_DONE).

MARK_MAGIC_SNAT_DONE is 0x0500 whereas MARK_MAGIC_IDENTITY is 0x0F00. So far
it was never a problem since MARK_MAGIC_IDENTITY was not used in a path where
MARK_MAGIC_SNAT_DONE was tested until this changed via f25d8b9 where now
SNAT was skipped for Pod traffic.

As a result, we've seen various flakes in SNAT or Hybrid NodePort setups mostly
on the tftp/UDP tests. Reverting f25d8b9 confirmed to fix these flakes. As
a workaround/fix, remap MARK_MAGIC_SNAT_DONE to 0x1500, so that setting the
MARK_MAGIC_IDENTITY will get a mismatch on above mentioned test and hence we'll
end up doing the SNAT.

This reaches into MARK_MAGIC_KEY_ID bit space, but the agent today cannot be
configured to run with both BPF NodePort and IPSec at the same time.

Fixes: #10942
Fixes: f25d8b9 ("bpf: Preserve source identity for hairpin via stack")
Reported-by: Martynas Pumputis <m@lambda.lt>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
  • Loading branch information
borkmann authored and qmonnet committed May 13, 2020
1 parent 82d8006 commit 13d1bbd
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 8 deletions.
5 changes: 4 additions & 1 deletion bpf/lib/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,10 @@ enum {
#define MARK_MAGIC_KEY_ID 0xF000
#define MARK_MAGIC_KEY_MASK 0xFF00

#define MARK_MAGIC_SNAT_DONE 0x0500
/* IPSec cannot be configured with NodePort BPF today, hence non-conflicting
* overlap with MARK_MAGIC_KEY_ID.
*/
#define MARK_MAGIC_SNAT_DONE 0x1500

/**
* get_identity - returns source identity from the mark field
Expand Down
2 changes: 1 addition & 1 deletion daemon/bpf.sha
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
GO_BINDATA_SHA1SUM=90133683d65986545167b57ad820f7fbe674c975
GO_BINDATA_SHA1SUM=d03d8900221291eb9f1080707e28776960944277
BPF_FILES=../bpf/COPYING ../bpf/Makefile ../bpf/Makefile.bpf ../bpf/bpf_alignchecker.c ../bpf/bpf_features.h ../bpf/bpf_hostdev_ingress.c ../bpf/bpf_ipsec.c ../bpf/bpf_lb.c ../bpf/bpf_lxc.c ../bpf/bpf_netdev.c ../bpf/bpf_network.c ../bpf/bpf_overlay.c ../bpf/bpf_sock.c ../bpf/bpf_xdp.c ../bpf/cilium-map-migrate.c ../bpf/filter_config.h ../bpf/include/bpf/api.h ../bpf/include/elf/elf.h ../bpf/include/elf/gelf.h ../bpf/include/elf/libelf.h ../bpf/include/iproute2/bpf_elf.h ../bpf/include/linux/bpf.h ../bpf/include/linux/bpf_common.h ../bpf/include/linux/byteorder.h ../bpf/include/linux/byteorder/big_endian.h ../bpf/include/linux/byteorder/little_endian.h ../bpf/include/linux/icmp.h ../bpf/include/linux/icmpv6.h ../bpf/include/linux/if_arp.h ../bpf/include/linux/if_ether.h ../bpf/include/linux/if_packet.h ../bpf/include/linux/in.h ../bpf/include/linux/in6.h ../bpf/include/linux/ioctl.h ../bpf/include/linux/ip.h ../bpf/include/linux/ipv6.h ../bpf/include/linux/perf_event.h ../bpf/include/linux/swab.h ../bpf/include/linux/tcp.h ../bpf/include/linux/type_mapper.h ../bpf/include/linux/udp.h ../bpf/init.sh ../bpf/lib/arp.h ../bpf/lib/common.h ../bpf/lib/config.h ../bpf/lib/conntrack.h ../bpf/lib/conntrack_map.h ../bpf/lib/csum.h ../bpf/lib/dbg.h ../bpf/lib/drop.h ../bpf/lib/encap.h ../bpf/lib/eps.h ../bpf/lib/eth.h ../bpf/lib/events.h ../bpf/lib/icmp6.h ../bpf/lib/ipv4.h ../bpf/lib/ipv6.h ../bpf/lib/l3.h ../bpf/lib/l4.h ../bpf/lib/lb.h ../bpf/lib/lxc.h ../bpf/lib/maps.h ../bpf/lib/metrics.h ../bpf/lib/nat.h ../bpf/lib/nat46.h ../bpf/lib/nodeport.h ../bpf/lib/policy.h ../bpf/lib/signal.h ../bpf/lib/tailcall.h ../bpf/lib/trace.h ../bpf/lib/utils.h ../bpf/lib/xdp.h ../bpf/lxc_config.h ../bpf/netdev_config.h ../bpf/node_config.h ../bpf/probes/raw_change_tail.t ../bpf/probes/raw_fib_lookup.t ../bpf/probes/raw_insn.h ../bpf/probes/raw_invalidate_hash.t ../bpf/probes/raw_lpm_map.t ../bpf/probes/raw_lru_map.t ../bpf/probes/raw_main.c ../bpf/probes/raw_map_val_adj.t ../bpf/probes/raw_mark_map_val.t ../bpf/probes/raw_max_insn.t ../bpf/probes/raw_sock_cookie.t ../bpf/run_probes.sh ../bpf/sockops/Makefile ../bpf/sockops/bpf_redir.c ../bpf/sockops/bpf_sockops.c ../bpf/sockops/bpf_sockops.h ../bpf/sockops/sockops_config.h
7 changes: 1 addition & 6 deletions pkg/datapath/linux/linux_defaults/mark.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,7 @@ package linux_defaults
//
// Cilium Mark (4 bits):
// M M M M
// 1 0 1 0 Ingress proxy
// 1 0 1 1 Egress proxy
// 1 1 0 0 From host
// 0 0 1 0 To Ingress Proxy
// 0 0 1 1 To Egress proxy
// 0 1 0 1 BPF SNAT done
// (see MARK_MAGIC_* in bpf/lib/common.h)
const (
// MagicMarkHostMask can be used to fetch the host/proxy-relevant magic
// bits from a mark.
Expand Down

0 comments on commit 13d1bbd

Please sign in to comment.