| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 29 Sep 2017 20:06:09 -0700
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Jesper Dangaard Brouer <brouer@...hat.com>
Cc: netdev@...r.kernel.org, jakub.kicinski@...ronome.com,
"Michael S. Tsirkin" <mst@...hat.com>,
Jason Wang <jasowang@...hat.com>, mchan@...adcom.com,
John Fastabend <john.fastabend@...il.com>,
peter.waskiewicz.jr@...el.com,
Daniel Borkmann <borkmann@...earbox.net>,
Andy Gospodarek <andy@...yhouse.net>
Subject: Re: [net-next V2 PATCH 5/5] samples/bpf: add cpumap sample program
xdp_redirect_cpu
On Fri, Sep 29, 2017 at 06:34:32PM +0200, Jesper Dangaard Brouer wrote:
> +
> +static __always_inline
> +u16 get_dest_port_ipv4_udp(struct xdp_md *ctx, u64 nh_off)
> +{
> + void *data_end = (void *)(long)ctx->data_end;
> + void *data = (void *)(long)ctx->data;
> + struct iphdr *iph = data + nh_off;
> + struct udphdr *udph;
> + u16 dport;
formatting is off in the above.
> +
> + if (iph + 1 > data_end)
> + return 0;
> + if (!(iph->protocol == IPPROTO_UDP))
> + return 0;
> +
> + udph = (void *)(iph + 1);
> + if (udph + 1 > data_end)
> + return 0;
> +
> + dport = ntohs(udph->dest);
> + return dport;
> +}
> +
> +static __always_inline
> +int get_proto_ipv4(struct xdp_md *ctx, u64 nh_off)
> +{
> + void *data_end = (void *)(long)ctx->data_end;
> + void *data = (void *)(long)ctx->data;
> + struct iphdr *iph = data + nh_off;
> +
> + if (iph + 1 > data_end)
> + return 0;
> + return iph->protocol;
and here
> +}
> +
> +static __always_inline
> +int get_proto_ipv6(struct xdp_md *ctx, u64 nh_off)
> +{
> + void *data_end = (void *)(long)ctx->data_end;
> + void *data = (void *)(long)ctx->data;
> + struct ipv6hdr *ip6h = data + nh_off;
> +
> + if (ip6h + 1 > data_end)
> + return 0;
and here
> +/*** Trace point code ***/
> +
> +/* Tracepoint format: /sys/kernel/debug/tracing/events/xdp/xdp_redirect/format
> + * Code in: kernel/include/trace/events/xdp.h
> + */
> +struct xdp_redirect_ctx {
> + unsigned short common_type; // offset:0; size:2; signed:0;
> + unsigned char common_flags; // offset:2; size:1; signed:0;
> + unsigned char common_preempt_count;// offset:3; size:1; signed:0;
> + int common_pid; // offset:4; size:4; signed:1;
this part is not right. First 8 bytes are not accessible by bpf code.
Please use __u64 pad; or similar here.
Just noticed that samples/bpf/xdp_monitor_kern.c has the same problem.
> +
> + int prog_id; // offset:8; size:4; signed:1;
> + u32 act; // offset:12 size:4; signed:0;
> + int ifindex; // offset:16 size:4; signed:1;
> + int err; // offset:20 size:4; signed:1;
> + int to_ifindex; // offset:24 size:4; signed:1;
> + u32 map_id; // offset:28 size:4; signed:0;
> + int map_index; // offset:32 size:4; signed:1;
> +}; // offset:36
the second part of fields is correct.
> +/* Tracepoint format: /sys/kernel/debug/tracing/events/xdp/xdp_exception/format
> + * Code in: kernel/include/trace/events/xdp.h
> + */
> +struct xdp_exception_ctx {
> + unsigned short common_type; // offset:0; size:2; signed:0;
> + unsigned char common_flags; // offset:2; size:1; signed:0;
> + unsigned char common_preempt_count;// offset:3; size:1; signed:0;
> + int common_pid; // offset:4; size:4; signed:1;
same problem here.
> + int prog_id; // offset:8; size:4; signed:1;
> + u32 act; // offset:12; size:4; signed:0;
> + int ifindex; // offset:16; size:4; signed:1;
> +};
> +
> +SEC("tracepoint/xdp/xdp_exception")
> +int trace_xdp_exception(struct xdp_exception_ctx *ctx)
> +{
> + struct datarec *rec;
> + u32 key = 0;
> +
> + rec = bpf_map_lookup_elem(&exception_cnt, &key);
> + if (!rec)
> + return 1;
> + rec->dropped += 1;
> +
> + return 0;
> +}
> +
> +/* Tracepoint: /sys/kernel/debug/tracing/events/xdp/xdp_cpumap_enqueue/format
> + * Code in: kernel/include/trace/events/xdp.h
> + */
> +struct cpumap_enqueue_ctx {
> + unsigned short common_type; // offset:0; size:2; signed:0;
> + unsigned char common_flags; // offset:2; size:1; signed:0;
> + unsigned char common_preempt_count;// offset:3; size:1; signed:0;
> + int common_pid; // offset:4; size:4; signed:1;
and here
> +/* Tracepoint: /sys/kernel/debug/tracing/events/xdp/xdp_cpumap_kthread/format
> + * Code in: kernel/include/trace/events/xdp.h
> + */
> +struct cpumap_kthread_ctx {
> + unsigned short common_type; // offset:0; size:2; signed:0;
> + unsigned char common_flags; // offset:2; size:1; signed:0;
> + unsigned char common_preempt_count;// offset:3; size:1; signed:0;
> + int common_pid; // offset:4; size:4; signed:1;
and here.
> +int main(int argc, char **argv)
> +{
> + struct rlimit r = {10 * 1024*1024, RLIM_INFINITY};
extra spaces around * wouldn't hurt
> + /* Parse commands line args */
> + while ((opt = getopt_long(argc, argv, "hSd:",
> + long_options, &longindex)) != -1) {
> + switch (opt) {
> + case 'd':
> + if (strlen(optarg) >= IF_NAMESIZE) {
> + fprintf(stderr, "ERR: --dev name too long\n");
> + goto error;
> + }
> + ifname = (char *)&ifname_buf;
> + strncpy(ifname, optarg, IF_NAMESIZE);
> + ifindex = if_nametoindex(ifname);
> + if (ifindex == 0) {
> + fprintf(stderr,
> + "ERR: --dev name unknown err(%d):%s\n",
> + errno, strerror(errno));
> + goto error;
> + }
> + break;
> + case 's':
> + interval = atoi(optarg);
> + break;
> + case 'S':
> + xdp_flags |= XDP_FLAGS_SKB_MODE;
> + break;
> + case 'D':
> + debug = true;
> + break;
> + case 'z':
> + use_separators = false;
> + break;
> + case 'p':
> + /* Selecting eBPF prog to load */
> + prog_num = atoi(optarg);
> + if (prog_num < 0 || prog_num >= MAX_PROG) {
> + fprintf(stderr,
> + "--prognum too large err(%d):%s\n",
> + errno, strerror(errno));
> + goto error;
> + }
> + break;
> + case 'c':
> + /* Add multiple CPUs */
> + add_cpu = strtoul(optarg, NULL, 0);
> + if (add_cpu > MAX_CPUS) {
> + fprintf(stderr,
> + "--cpu nr too large for cpumap err(%d):%s\n",
> + errno, strerror(errno));
> + goto error;
> + }
> + create_cpu_entry(add_cpu, qsize, added_cpus, true);
> + added_cpus++;
> + break;
> + case 'q':
> + qsize = atoi(optarg);
> + break;
> + case 'h':
> + error:
> + default:
> + usage(argv);
> + return EXIT_FAIL_OPTION;
the rest looks good. Nice that it parses cmdline properly
and even has -h flag :)
Powered by blists - more mailing lists