On Thu, May 20, 2021 at 04:26:28AM +0000, Song Liu wrote:

quoted

On May 17, 2021, at 3:53 PM, Dmitrii Banshchikov [off-list ref] 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 <redacted>

[...]

quoted

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.

quoted

+
+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.

quoted

+
+#include <errno.h>
+#include <string.h>
+
+#include "bflog.h"
+#include "context.h"
+#include "match-ops-map.h"
+
+#define BPFILTER_ALIGN(__X) __ALIGN_KERNEL(__X, __alignof__(__u64))
+#define MATCH_SIZE(type) (sizeof(struct bpfilter_ipt_match) + BPFILTER_ALIGN(sizeof(type)))
+
+static int udp_match_check(struct context *ctx, const struct bpfilter_ipt_match *ipt_match)
+{
+	const struct xt_udp *udp;
+
+	udp = (const struct xt_udp *)&ipt_match->data;
+
+	if (udp->invflags & XT_UDP_INV_MASK) {
+		BFLOG_DEBUG(ctx, "cannot check match 'udp': invalid flags\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+const struct match_ops udp_match_ops = { .name = "udp",

And maybe we should name this one "xt_udp"? 

Agreed.

quoted

+					 .size = MATCH_SIZE(struct xt_udp),
+					 .revision = 0,
+					 .check = udp_match_check };
+
+int init_match(struct context *ctx, const struct bpfilter_ipt_match *ipt_match, struct match *match)
+{
+	const size_t maxlen = sizeof(ipt_match->u.user.name);
+	const struct match_ops *found;
+	int err;
+
+	if (strnlen(ipt_match->u.user.name, maxlen) == maxlen) {
+		BFLOG_DEBUG(ctx, "cannot init match: too long match name\n");
+		return -EINVAL;
+	}
+
+	found = match_ops_map_find(&ctx->match_ops_map, ipt_match->u.user.name);
+	if (IS_ERR(found)) {
+		BFLOG_DEBUG(ctx, "cannot find match by name: '%s'\n", ipt_match->u.user.name);
+		return PTR_ERR(found);
+	}
+
+	if (found->size != ipt_match->u.match_size ||
+	    found->revision != ipt_match->u.user.revision) {
+		BFLOG_DEBUG(ctx, "invalid match: '%s'\n", ipt_match->u.user.name);
+		return -EINVAL;
+	}
+
+	err = found->check(ctx, ipt_match);
+	if (err)
+		return err;
+
+	match->match_ops = found;
+	match->ipt_match = ipt_match;
+
+	return 0;
+}

diff --git a/net/bpfilter/match.h b/net/bpfilter/match.h
new file mode 100644
index 000000000000..79b7c87016d4
--- /dev/null
+++ b/net/bpfilter/match.h

@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2021 Telegram FZ-LLC
+ */
+
+#ifndef NET_BPFILTER_MATCH_H
+#define NET_BPFILTER_MATCH_H
+
+#include "../../include/uapi/linux/bpfilter.h"
+
+#include <stdint.h>
+
+struct bpfilter_ipt_match;
+struct context;
+struct match_ops_map;
+
+struct match_ops {
+	char name[BPFILTER_EXTENSION_MAXNAMELEN];

BPFILTER_EXTENSION_MAXNAMELEN is 29, so "size" below is mis-aligned. I guess
we can swap size and revision. 

Agreed.

quoted

+	uint16_t size;
+	uint8_t revision;
+	int (*check)(struct context *ctx, const struct bpfilter_ipt_match *ipt_match);
+};
+
+extern const struct match_ops udp_match_ops;
+
+struct match {
+	const struct match_ops *match_ops;
+	const struct bpfilter_ipt_match *ipt_match;
+};

[...]

-- 

Dmitrii Banshchikov