Re: [PATCHv2 bpf-next] samples/bpf: add xdp program on egress for xdp_redirect_map
From: Yonghong Song <hidden>
Date: 2020-11-27 06:32:39
Also in:
bpf
On 11/26/20 12:43 AM, Hangbin Liu wrote:
quoted hunk ↗ jump to hunk
Current sample test xdp_redirect_map only count pkts on ingress. But we can't know whether the pkts are redirected or dropped. So add a counter on egress interface so we could know how many pkts are redirect in fact. sample result: $ ./xdp_redirect_map -X veth1 veth2 input: 5 output: 6 libbpf: elf: skipping unrecognized data section(9) .rodata.str1.16 libbpf: elf: skipping unrecognized data section(23) .eh_frame libbpf: elf: skipping relo section(24) .rel.eh_frame for section(23) .eh_frame in ifindex 5: 1 pkt/s, out ifindex 6: 1 pkt/s in ifindex 5: 1 pkt/s, out ifindex 6: 1 pkt/s in ifindex 5: 0 pkt/s, out ifindex 6: 0 pkt/s in ifindex 5: 68 pkt/s, out ifindex 6: 68 pkt/s in ifindex 5: 91 pkt/s, out ifindex 6: 91 pkt/s in ifindex 5: 91 pkt/s, out ifindex 6: 91 pkt/s in ifindex 5: 66 pkt/s, out ifindex 6: 66 pkt/s Signed-off-by: Hangbin Liu <redacted> --- v2: a) use pkt counter instead of IP ttl modification on egress program b) make the egress program selectable by option -X --- samples/bpf/xdp_redirect_map_kern.c | 26 +++-- samples/bpf/xdp_redirect_map_user.c | 142 ++++++++++++++++++---------- 2 files changed, 113 insertions(+), 55 deletions(-)diff --git a/samples/bpf/xdp_redirect_map_kern.c b/samples/bpf/xdp_redirect_map_kern.c index 6489352ab7a4..fd6704a4f7e2 100644 --- a/samples/bpf/xdp_redirect_map_kern.c +++ b/samples/bpf/xdp_redirect_map_kern.c@@ -22,19 +22,19 @@ struct { __uint(type, BPF_MAP_TYPE_DEVMAP); __uint(key_size, sizeof(int)); - __uint(value_size, sizeof(int)); + __uint(value_size, sizeof(struct bpf_devmap_val)); __uint(max_entries, 100); } tx_port SEC(".maps"); -/* Count RX packets, as XDP bpf_prog doesn't get direct TX-success - * feedback. Redirect TX errors can be caught via a tracepoint. +/* Count RX/TX packets, use key 0 for rx pkt count, key 1 for tx + * pkt count. */ struct { __uint(type, BPF_MAP_TYPE_PERCPU_ARRAY); __type(key, u32); __type(value, long); - __uint(max_entries, 1); -} rxcnt SEC(".maps"); + __uint(max_entries, 2); +} pktcnt SEC(".maps"); static void swap_src_dst_mac(void *data) {@@ -72,7 +72,7 @@ int xdp_redirect_map_prog(struct xdp_md *ctx) vport = 0; /* count packet in global counter */ - value = bpf_map_lookup_elem(&rxcnt, &key); + value = bpf_map_lookup_elem(&pktcnt, &key); if (value) *value += 1;@@ -82,6 +82,20 @@ int xdp_redirect_map_prog(struct xdp_md *ctx) return bpf_redirect_map(&tx_port, vport, 0); } +SEC("xdp_devmap/map_prog") +int xdp_devmap_prog(struct xdp_md *ctx) +{ + long *value; + u32 key = 1; + + /* count packet in global counter */ + value = bpf_map_lookup_elem(&pktcnt, &key); + if (value) + *value += 1; + + return XDP_PASS; +} + /* Redirect require an XDP bpf_prog loaded on the TX device */ SEC("xdp_redirect_dummy") int xdp_redirect_dummy_prog(struct xdp_md *ctx)diff --git a/samples/bpf/xdp_redirect_map_user.c b/samples/bpf/xdp_redirect_map_user.c index 35e16dee613e..8bdec0865e1d 100644 --- a/samples/bpf/xdp_redirect_map_user.c +++ b/samples/bpf/xdp_redirect_map_user.c@@ -21,12 +21,13 @@ static int ifindex_in; static int ifindex_out; -static bool ifindex_out_xdp_dummy_attached = true; +static bool ifindex_out_xdp_dummy_attached = false; +static bool xdp_prog_attached = false;
Maybe xdp_devmap_prog_attached? Feel xdp_prog_attached is too generic since actually it controls xdp_devmap program attachment.
quoted hunk ↗ jump to hunk
static __u32 prog_id; static __u32 dummy_prog_id; static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST; -static int rxcnt_map_fd; +static int pktcnt_map_fd; static void int_exit(int sig) {@@ -60,26 +61,46 @@ static void int_exit(int sig) exit(0); } -static void poll_stats(int interval, int ifindex) +static void poll_stats(int interval, int if_ingress, int if_egress) { unsigned int nr_cpus = bpf_num_possible_cpus(); - __u64 values[nr_cpus], prev[nr_cpus]; + __u64 values[nr_cpus], in_prev[nr_cpus], e_prev[nr_cpus]; + __u64 sum; + __u32 key; + int i; - memset(prev, 0, sizeof(prev)); + memset(in_prev, 0, sizeof(in_prev)); + memset(e_prev, 0, sizeof(e_prev)); while (1) { - __u64 sum = 0; - __u32 key = 0; - int i; + sum = 0; + key = 0; sleep(interval); - assert(bpf_map_lookup_elem(rxcnt_map_fd, &key, values) == 0); - for (i = 0; i < nr_cpus; i++) - sum += (values[i] - prev[i]); - if (sum) - printf("ifindex %i: %10llu pkt/s\n", - ifindex, sum / interval); - memcpy(prev, values, sizeof(values)); + if (bpf_map_lookup_elem(pktcnt_map_fd, &key, values) == 0) {
When we could have a failure here? If it indeed failed maybe it signals something wrong and the process should fail?
+ for (i = 0; i < nr_cpus; i++)
+ sum += (values[i] - in_prev[i]);
+ if (sum)
+ printf("in ifindex %i: %10llu pkt/s",
+ if_ingress, sum / interval);
+ memcpy(in_prev, values, sizeof(values));
+ }
+
+ if (!xdp_prog_attached) {
+ printf("\n");
+ continue;
+ }
+
+ sum = 0;
+ key = 1;
+ if (bpf_map_lookup_elem(pktcnt_map_fd, &key, values) == 0) {same as the above, if bpf_map_lookup_elem() failed, maybe we should signal a failure?
+ for (i = 0; i < nr_cpus; i++)
+ sum += (values[i] - e_prev[i]);
+ if (sum)
+ printf(", out ifindex %i: %10llu pkt/s\n",
+ if_egress, sum / interval);
+ memcpy(e_prev, values, sizeof(values));
+ }
}
}
[...]