netdev - Re: [PATCH v2 bpf-next 2/2] libbpf: remove qidconf and better support external bpf programs.

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <80213576-3DE9-4FCA-8B9F-D8C1F85C9378@gmail.com>
Date:   Fri, 31 May 2019 09:30:06 -0700
From:   "Jonathan Lemon" <jonathan.lemon@...il.com>
To:     "Björn Töpel" <bjorn.topel@...el.com>
Cc:     netdev@...r.kernel.org, magnus.karlsson@...el.com,
        kernel-team@...com, bpf <bpf@...r.kernel.org>
Subject: Re: [PATCH v2 bpf-next 2/2] libbpf: remove qidconf and better support
 external bpf programs.

On 31 May 2019, at 4:55, Björn Töpel wrote:

> On 2019-05-30 20:57, Jonathan Lemon wrote:
>> Use the recent change to XSKMAP bpf_map_lookup_elem() to test if
>> there is a xsk present in the map instead of duplicating the work
>> with qidconf.
>>
>> Fix things so callers using XSK_LIBBPF_FLAGS__INHIBIT_PROG_LOAD
>> bypass any internal bpf maps, so xsk_socket__{create|delete} works
>> properly.
>>
>> Signed-off-by: Jonathan Lemon <jonathan.lemon@...il.com>
>> ---
>>   tools/lib/bpf/xsk.c | 79 
>> +++++++++------------------------------------
>>   1 file changed, 16 insertions(+), 63 deletions(-)
>>
>> diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
>> index 38667b62f1fe..a150493d51ec 100644
>> --- a/tools/lib/bpf/xsk.c
>> +++ b/tools/lib/bpf/xsk.c
>> @@ -60,10 +60,8 @@ struct xsk_socket {
>>   	struct xsk_umem *umem;
>>   	struct xsk_socket_config config;
>>   	int fd;
>> -	int xsks_map;
>>   	int ifindex;
>>   	int prog_fd;
>> -	int qidconf_map_fd;
>>   	int xsks_map_fd;
>>   	__u32 queue_id;
>>   	char ifname[IFNAMSIZ];
>> @@ -265,15 +263,11 @@ static int xsk_load_xdp_prog(struct xsk_socket 
>> *xsk)
>>   	/* This is the C-program:
>>   	 * SEC("xdp_sock") int xdp_sock_prog(struct xdp_md *ctx)
>>   	 * {
>> -	 *     int *qidconf, index = ctx->rx_queue_index;
>> +	 *     int index = ctx->rx_queue_index;
>>   	 *
>>   	 *     // A set entry here means that the correspnding queue_id
>>   	 *     // has an active AF_XDP socket bound to it.
>> -	 *     qidconf = bpf_map_lookup_elem(&qidconf_map, &index);
>> -	 *     if (!qidconf)
>> -	 *         return XDP_ABORTED;
>> -	 *
>> -	 *     if (*qidconf)
>> +	 *     if (bpf_map_lookup_elem(&xsks_map, &index))
>>   	 *         return bpf_redirect_map(&xsks_map, index, 0);
>>   	 *
>>   	 *     return XDP_PASS;
>> @@ -286,15 +280,10 @@ static int xsk_load_xdp_prog(struct xsk_socket 
>> *xsk)
>>   		BPF_STX_MEM(BPF_W, BPF_REG_10, BPF_REG_1, -4),
>>   		BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
>>   		BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -4),
>> -		BPF_LD_MAP_FD(BPF_REG_1, xsk->qidconf_map_fd),
>> +		BPF_LD_MAP_FD(BPF_REG_1, xsk->xsks_map_fd),
>>   		BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
>>   		BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
>> -		BPF_MOV32_IMM(BPF_REG_0, 0),
>> -		/* if r1 == 0 goto +8 */
>> -		BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 0, 8),
>>   		BPF_MOV32_IMM(BPF_REG_0, 2),
>> -		/* r1 = *(u32 *)(r1 + 0) */
>> -		BPF_LDX_MEM(BPF_W, BPF_REG_1, BPF_REG_1, 0),
>>   		/* if r1 == 0 goto +5 */
>>   		BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 0, 5),
>>   		/* r2 = *(u32 *)(r10 - 4) */
>> @@ -366,18 +355,11 @@ static int xsk_create_bpf_maps(struct 
>> xsk_socket *xsk)
>>   	if (max_queues < 0)
>>   		return max_queues;
>>  -	fd = bpf_create_map_name(BPF_MAP_TYPE_ARRAY, "qidconf_map",
>> +	fd = bpf_create_map_name(BPF_MAP_TYPE_ARRAY, "xsks_map",
>>   				 sizeof(int), sizeof(int), max_queues, 0);
>>   	if (fd < 0)
>>   		return fd;
>> -	xsk->qidconf_map_fd = fd;
>>  -	fd = bpf_create_map_name(BPF_MAP_TYPE_XSKMAP, "xsks_map",
>> -				 sizeof(int), sizeof(int), max_queues, 0);
>> -	if (fd < 0) {
>> -		close(xsk->qidconf_map_fd);
>> -		return fd;
>> -	}
>
> Uhm, you're removing the XSKMAP here, replacing it with an ARRAY. Have
> you run this?

Err... I've been running the code, but this version is wrong.   Let me 
respin this.
-- 
Jonathan