netdev - Re: [PATCH bpf-next 06/11] bpfilter: Add struct match

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: <CB97A458-90FA-46AF-96C2-6FB3F9139570@fb.com>
Date:   Thu, 20 May 2021 17:44:37 +0000
From:   Song Liu <songliubraving@...com>
To:     Dmitrii Banshchikov <me@...que.spb.ru>
CC:     "open list:BPF (Safe dynamic programs and tools)" 
        <bpf@...r.kernel.org>, Alexei Starovoitov <ast@...nel.org>,
        "David S . Miller" <davem@...emloft.net>,
        Daniel Borkmann <daniel@...earbox.net>,
        "Andrii Nakryiko" <andrii@...nel.org>, Martin Lau <kafai@...com>,
        Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>,
        "open list:BPF (Safe dynamic programs and tools)" 
        <netdev@...r.kernel.org>, Andrey Ignatov <rdna@...com>
Subject: Re: [PATCH bpf-next 06/11] bpfilter: Add struct match



> On May 20, 2021, at 12:31 AM, Dmitrii Banshchikov <me@...que.spb.ru> wrote:
> 
> On Thu, May 20, 2021 at 04:26:28AM +0000, Song Liu wrote:
>> 
>> 
>>> On May 17, 2021, at 3:53 PM, Dmitrii Banshchikov <me@...que.spb.ru> wrote:
>>> 
>>> struct match_ops defines polymorphic interface for matches. A match
>>> consists of pointers to struct match_ops and struct xt_entry_match which
>>> contains a payload for the match's type.
>>> 
>>> All match_ops are kept in map match_ops_map by their name.
>>> 
>>> Signed-off-by: Dmitrii Banshchikov <me@...que.spb.ru>
>>> 
>> [...]
>> 
>>> diff --git a/net/bpfilter/match-ops-map.h b/net/bpfilter/match-ops-map.h
>>> new file mode 100644
>>> index 000000000000..0ff57f2d8da8
>>> --- /dev/null
>>> +++ b/net/bpfilter/match-ops-map.h
>>> @@ -0,0 +1,48 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +/*
>>> + * Copyright (c) 2021 Telegram FZ-LLC
>>> + */
>>> +
>>> +#ifndef NET_BPFILTER_MATCH_OPS_MAP_H
>>> +#define NET_BPFILTER_MATCH_OPS_MAP_H
>>> +
>>> +#include "map-common.h"
>>> +
>>> +#include <linux/err.h>
>>> +
>>> +#include <errno.h>
>>> +#include <string.h>
>>> +
>>> +#include "match.h"
>>> +
>>> +struct match_ops_map {
>>> +	struct hsearch_data index;
>>> +};
>> 
>> Do we plan to extend match_ops_map? Otherwise, we can just use 
>> hsearch_data in struct context. 
> 
> Agreed.
> 
>> 
>>> +
>>> +static inline int create_match_ops_map(struct match_ops_map *map, size_t nelem)
>>> +{
>>> +	return create_map(&map->index, nelem);
>>> +}
>>> +
>>> +static inline const struct match_ops *match_ops_map_find(struct match_ops_map *map,
>>> +							 const char *name)
>>> +{
>>> +	const size_t namelen = strnlen(name, BPFILTER_EXTENSION_MAXNAMELEN);
>>> +
>>> +	if (namelen < BPFILTER_EXTENSION_MAXNAMELEN)
>>> +		return map_find(&map->index, name);
>>> +
>>> +	return ERR_PTR(-EINVAL);
>>> +}
>>> +
>>> +static inline int match_ops_map_insert(struct match_ops_map *map, const struct match_ops *match_ops)
>>> +{
>>> +	return map_insert(&map->index, match_ops->name, (void *)match_ops);
>>> +}
>>> +
>>> +static inline void free_match_ops_map(struct match_ops_map *map)
>>> +{
>>> +	free_map(&map->index);
>>> +}
>>> +
>>> +#endif // NET_BPFILTER_MATCT_OPS_MAP_H
>>> diff --git a/net/bpfilter/match.c b/net/bpfilter/match.c
>>> new file mode 100644
>>> index 000000000000..aeca1b93cd2d
>>> --- /dev/null
>>> +++ b/net/bpfilter/match.c
>>> @@ -0,0 +1,73 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (c) 2021 Telegram FZ-LLC
>>> + */
>>> +
>>> +#define _GNU_SOURCE
>>> +
>>> +#include "match.h"
>>> +
>>> +#include <linux/err.h>
>>> +#include <linux/netfilter/xt_tcpudp.h>
>> 
>> Besides xt_ filters, do we plan to support others? If so, we probably 
>> want separate files for each of them. 
> 
> Do you mean nft filters?
> They use nfilter API and currently we cannot hook into it - so
> probably eventually.
> 

The comment was mostly about how we name variables/marcos. If we plan to 
support more than xt_ matches, we should prefix variables properly. 

Song