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

[Song Liu <songliubraving@fb.com>]

> On May 20, 2021, at 12:31 AM, Dmitrii Banshchikov <me@ubique.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@ubique.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@ubique.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