On 9/30/20 9:20 PM, Alexei Starovoitov wrote:
On Wed, Sep 30, 2020 at 05:18:20PM +0200, Daniel Borkmann wrote:
+
+#ifndef barrier_data
+# define barrier_data(ptr) asm volatile("": :"r"(ptr) :"memory")
+#endif
+
+#ifndef ctx_ptr
+# define ctx_ptr(field) (void *)(long)(field)
+#endif
+static __always_inline bool is_remote_ep_v4(struct __sk_buff *skb,
+ __be32 addr)
+{
+ void *data_end = ctx_ptr(skb->data_end);
+ void *data = ctx_ptr(skb->data);
please consider adding:
__bpf_md_ptr(void *, data);
__bpf_md_ptr(void *, data_end);
to struct __sk_buff in a followup to avoid this casting headache.
You mean also for the other ctx types? I can take a look, yeah.
+SEC("dst_ingress") int tc_dst(struct __sk_buff *skb)
+{
+ int idx = dst_to_src_tmp;
+ __u8 zero[ETH_ALEN * 2];
+ bool redirect = false;
+
+ switch (skb->protocol) {
+ case __bpf_constant_htons(ETH_P_IP):
+ redirect = is_remote_ep_v4(skb, __bpf_constant_htonl(ip4_src));
+ break;
+ case __bpf_constant_htons(ETH_P_IPV6):
+ redirect = is_remote_ep_v6(skb, (struct in6_addr)ip6_src);
+ break;
+ }
+
+ if (!redirect)
+ return TC_ACT_OK;
+
+ barrier_data(&idx);
+ idx = bpf_ntohl(idx);
I don't follow. Why force that constant into a register and force
actual swap instruction?
+
+ __builtin_memset(&zero, 0, sizeof(zero));
+ if (bpf_skb_store_bytes(skb, 0, &zero, sizeof(zero), 0) < 0)
+ return TC_ACT_SHOT;
+
+ return bpf_redirect_neigh(idx, 0);
+}
+xxd -p < test_tc_neigh.o | sed "s/eeddddee/$veth_src/g" | xxd -r -p > test_tc_neigh.x.o
+xxd -p < test_tc_neigh.x.o | sed "s/eeffffee/$veth_dst/g" | xxd -r -p > test_tc_neigh.y.o
So the inline asm is because of the above?
So after compiling you're hacking elf binary for this pattern ?
Ouch. Please use global data or something. This is fragile.
This type of hacks should be discouraged and having selftests do them
goes as counter example.
Yeah, so the barrier_data() was to avoid compiler to optimize, and the bpf_ntohl()
to load target ifindex which was stored in big endian. Thanks for applying the set,
I'll look into reworking this to have a loader application w/ the global data and
then to pin it and have iproute2 pick this up from the pinned location, for example
(or directly interact with netlink wrt attaching ... I'll see which is better).
Thanks,
Daniel
