From: Dmitrii Banshchikov <me@ubique.spb.ru>
To: Song Liu <songliubraving@fb.com>
Cc: "open list:BPF (Safe dynamic programs and tools)"
<bpf@vger.kernel.org>, "ast@kernel.org" <ast@kernel.org>,
"davem@davemloft.net" <davem@davemloft.net>,
"daniel@iogearbox.net" <daniel@iogearbox.net>,
"andrii@kernel.org" <andrii@kernel.org>,
Martin Lau <kafai@fb.com>, Yonghong Song <yhs@fb.com>,
"john.fastabend@gmail.com" <john.fastabend@gmail.com>,
"kpsingh@kernel.org" <kpsingh@kernel.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
Andrey Ignatov <rdna@fb.com>
Subject: Re: [PATCH bpf-next 07/11] bpfilter: Add struct target
Date: Thu, 20 May 2021 11:44:36 +0400 [thread overview]
Message-ID: <20210520074436.pxbqvwkvw4gihizj@amnesia> (raw)
In-Reply-To: <1B747351-8336-45FB-918D-5FC3DA18B47D@fb.com>
On Thu, May 20, 2021 at 04:36:46AM +0000, Song Liu wrote:
>
>
> > On May 17, 2021, at 3:53 PM, Dmitrii Banshchikov <me@ubique.spb.ru> wrote:
> >
> > struct target_ops defines polymorphic interface for targets. A target
> > consists of pointers to struct target_ops and struct xt_entry_target
> > which contains a payload for the target's type.
> >
> > All target_ops are kept in map target_ops_map by their name.
> >
> > Signed-off-by: Dmitrii Banshchikov <me@ubique.spb.ru>
> >
>
> [...]
>
> > index 000000000000..6b65241328da
> > --- /dev/null
> > +++ b/net/bpfilter/target-ops-map.h
> > @@ -0,0 +1,49 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (c) 2021 Telegram FZ-LLC
> > + */
> > +
> > +#ifndef NET_BPFILTER_TARGET_OPS_MAP_H
> > +#define NET_BPFILTER_TARGET_OPS_MAP_H
> > +
> > +#include "map-common.h"
> > +
> > +#include <linux/err.h>
> > +
> > +#include <errno.h>
> > +#include <string.h>
> > +
> > +#include "target.h"
> > +
> > +struct target_ops_map {
> > + struct hsearch_data index;
> > +};
>
> Similar to 06/11, target_ops_map seems unnecessary. Also, do we need to
> support non-xt targets?
As with nft matches - probably eventually but not now.
>
> > +
> > +static inline int create_target_ops_map(struct target_ops_map *map, size_t nelem)
> > +{
> > + return create_map(&map->index, nelem);
> > +}
> > +
> > +static inline const struct target_ops *target_ops_map_find(struct target_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 target_ops_map_insert(struct target_ops_map *map,
> > + const struct target_ops *target_ops)
> > +{
> > + return map_insert(&map->index, target_ops->name, (void *)target_ops);
> > +}
> > +
> > +static inline void free_target_ops_map(struct target_ops_map *map)
> > +{
> > + free_map(&map->index);
> > +}
> > +
> > +#endif // NET_BPFILTER_TARGET_OPS_MAP_H
> > diff --git a/net/bpfilter/target.c b/net/bpfilter/target.c
> > new file mode 100644
> > index 000000000000..a18fe477f93c
> > --- /dev/null
> > +++ b/net/bpfilter/target.c
> > @@ -0,0 +1,112 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2021 Telegram FZ-LLC
> > + */
> > +
> > +#define _GNU_SOURCE
> > +
> > +#include "target.h"
> > +
> > +#include <linux/err.h>
> > +#include <linux/netfilter/x_tables.h>
> > +
> > +#include <errno.h>
> > +#include <string.h>
> > +
> > +#include "bflog.h"
> > +#include "context.h"
> > +#include "target-ops-map.h"
> > +
>
> Please add some comments about convert_verdict.
>
>
> > +static int convert_verdict(int verdict)
> > +{
> > + return -verdict - 1;
> > +}
> > +
> > +static int standard_target_check(struct context *ctx, const struct bpfilter_ipt_target *ipt_target)
> > +{
> > + const struct bpfilter_ipt_standard_target *standard_target;
> > +
> > + standard_target = (const struct bpfilter_ipt_standard_target *)ipt_target;
> > +
> > + if (standard_target->verdict > 0)
> > + return 0;
> > +
> > + if (standard_target->verdict < 0) {
> > + if (standard_target->verdict == BPFILTER_RETURN)
> > + return 0;
> > +
> > + switch (convert_verdict(standard_target->verdict)) {
> > + case BPFILTER_NF_ACCEPT:
> > + case BPFILTER_NF_DROP:
> > + case BPFILTER_NF_QUEUE:
> > + return 0;
> > + }
> > + }
> > +
> > + BFLOG_DEBUG(ctx, "invalid verdict: %d\n", standard_target->verdict);
> > +
> > + return -EINVAL;
> > +}
> > +
> > +const struct target_ops standard_target_ops = {
> > + .name = "",
> > + .revision = 0,
> > + .size = sizeof(struct xt_standard_target),
> > + .check = standard_target_check,
> > +};
> > +
> > +static int error_target_check(struct context *ctx, const struct bpfilter_ipt_target *ipt_target)
> > +{
> > + const struct bpfilter_ipt_error_target *error_target;
> > + size_t maxlen;
> > +
> > + error_target = (const struct bpfilter_ipt_error_target *)&ipt_target;
> > + maxlen = sizeof(error_target->error_name);
> > + if (strnlen(error_target->error_name, maxlen) == maxlen) {
> > + BFLOG_DEBUG(ctx, "cannot check error target: too long errorname\n");
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +const struct target_ops error_target_ops = {
> > + .name = "ERROR",
> > + .revision = 0,
> > + .size = sizeof(struct xt_error_target),
> > + .check = error_target_check,
> > +};
> > +
> > +int init_target(struct context *ctx, const struct bpfilter_ipt_target *ipt_target,
> > + struct target *target)
> > +{
> > + const size_t maxlen = sizeof(ipt_target->u.user.name);
> > + const struct target_ops *found;
> > + int err;
> > +
> > + if (strnlen(ipt_target->u.user.name, maxlen) == maxlen) {
> > + BFLOG_DEBUG(ctx, "cannot init target: too long target name\n");
> > + return -EINVAL;
> > + }
> > +
> > + found = target_ops_map_find(&ctx->target_ops_map, ipt_target->u.user.name);
> > + if (IS_ERR(found)) {
> > + BFLOG_DEBUG(ctx, "cannot find target by name: '%s'\n", ipt_target->u.user.name);
> > + return PTR_ERR(found);
> > + }
> > +
> > + if (found->size != ipt_target->u.target_size ||
> > + found->revision != ipt_target->u.user.revision) {
> > + BFLOG_DEBUG(ctx, "invalid target: '%s'\n", ipt_target->u.user.name);
> > + return -EINVAL;
> > + }
> > +
> > + err = found->check(ctx, ipt_target);
> > + if (err)
> > + return err;
> > +
> > + target->target_ops = found;
> > + target->ipt_target = ipt_target;
> > +
> > + return 0;
> > +}
> > diff --git a/net/bpfilter/target.h b/net/bpfilter/target.h
> > new file mode 100644
> > index 000000000000..5d9c4c459c05
> > --- /dev/null
> > +++ b/net/bpfilter/target.h
> > @@ -0,0 +1,34 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (c) 2021 Telegram FZ-LLC
> > + */
> > +
> > +#ifndef NET_BPFILTER_TARGET_H
> > +#define NET_BPFILTER_TARGET_H
> > +
> > +#include "../../include/uapi/linux/bpfilter.h"
> > +
> > +#include <stdint.h>
> > +
> > +struct context;
> > +struct target_ops_map;
> > +
> > +struct target_ops {
> > + char name[BPFILTER_EXTENSION_MAXNAMELEN];
> > + uint16_t size;
>
> Mis-aligned "size".
>
> > + uint8_t revision;
> > + int (*check)(struct context *ctx, const struct bpfilter_ipt_target *ipt_target);
> > +};
> > +
> > +struct target {
> > + const struct target_ops *target_ops;
> > + const struct bpfilter_ipt_target *ipt_target;
> > +};
> > +
> > +extern const struct target_ops standard_target_ops;
> > +extern const struct target_ops error_target_ops;
> > +
> > +int init_target(struct context *ctx, const struct bpfilter_ipt_target *ipt_target,
> > + struct target *target);
> > +
> > +#endif // NET_BPFILTER_TARGET_H
> > diff --git a/tools/testing/selftests/bpf/bpfilter/.gitignore b/tools/testing/selftests/bpf/bpfilter/.gitignore
> > index e5073231f811..1856d0515f49 100644
> > --- a/tools/testing/selftests/bpf/bpfilter/.gitignore
> > +++ b/tools/testing/selftests/bpf/bpfilter/.gitignore
> > @@ -2,3 +2,4 @@
> > test_io
> > test_map
> > test_match
> > +test_target
> > diff --git a/tools/testing/selftests/bpf/bpfilter/Makefile b/tools/testing/selftests/bpf/bpfilter/Makefile
> > index 362c9a28b88d..78da74b9ee68 100644
> > --- a/tools/testing/selftests/bpf/bpfilter/Makefile
> > +++ b/tools/testing/selftests/bpf/bpfilter/Makefile
> > @@ -11,6 +11,7 @@ CFLAGS += -Wall -g -pthread -I$(TOOLSINCDIR) -I$(APIDIR) -I$(BPFILTERSRCDIR)
> > TEST_GEN_PROGS += test_io
> > TEST_GEN_PROGS += test_map
> > TEST_GEN_PROGS += test_match
> > +TEST_GEN_PROGS += test_target
> >
> > KSFT_KHDR_INSTALL := 1
> >
> > @@ -19,4 +20,6 @@ include ../../lib.mk
> > $(OUTPUT)/test_io: test_io.c $(BPFILTERSRCDIR)/io.c
> > $(OUTPUT)/test_map: test_map.c $(BPFILTERSRCDIR)/map-common.c
> > $(OUTPUT)/test_match: test_match.c $(BPFILTERSRCDIR)/match.c $(BPFILTERSRCDIR)/map-common.c \
> > - $(BPFILTERSRCDIR)/context.c $(BPFILTERSRCDIR)/bflog.c
> > + $(BPFILTERSRCDIR)/context.c $(BPFILTERSRCDIR)/bflog.c $(BPFILTERSRCDIR)/target.c
> > +$(OUTPUT)/test_target: test_target.c $(BPFILTERSRCDIR)/target.c $(BPFILTERSRCDIR)/map-common.c \
> > + $(BPFILTERSRCDIR)/context.c $(BPFILTERSRCDIR)/bflog.c $(BPFILTERSRCDIR)/match.c
> > diff --git a/tools/testing/selftests/bpf/bpfilter/bpfilter_util.h b/tools/testing/selftests/bpf/bpfilter/bpfilter_util.h
> > new file mode 100644
> > index 000000000000..d82ff86f280e
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/bpfilter/bpfilter_util.h
> > @@ -0,0 +1,31 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +
> > +#ifndef BPFILTER_UTIL_H
> > +#define BPFILTER_UTIL_H
> > +
> > +#include <linux/bpfilter.h>
> > +#include <linux/netfilter/x_tables.h>
> > +
> > +#include <stdio.h>
> > +
> > +static inline void init_standard_target(struct xt_standard_target *ipt_target, int revision,
> > + int verdict)
> > +{
> > + snprintf(ipt_target->target.u.user.name, sizeof(ipt_target->target.u.user.name), "%s",
> > + BPFILTER_STANDARD_TARGET);
> > + ipt_target->target.u.user.revision = revision;
> > + ipt_target->target.u.user.target_size = sizeof(*ipt_target);
> > + ipt_target->verdict = verdict;
> > +}
> > +
> > +static inline void init_error_target(struct xt_error_target *ipt_target, int revision,
> > + const char *error_name)
> > +{
> > + snprintf(ipt_target->target.u.user.name, sizeof(ipt_target->target.u.user.name), "%s",
> > + BPFILTER_ERROR_TARGET);
> > + ipt_target->target.u.user.revision = revision;
> > + ipt_target->target.u.user.target_size = sizeof(*ipt_target);
> > + snprintf(ipt_target->errorname, sizeof(ipt_target->errorname), "%s", error_name);
> > +}
> > +
> > +#endif // BPFILTER_UTIL_H
> >
> [...]
> >
>
--
Dmitrii Banshchikov
next prev parent reply other threads:[~2021-05-20 7:44 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-17 22:52 [PATCH bpf-next 00/11] bpfilter Dmitrii Banshchikov
2021-05-17 22:52 ` [PATCH bpf-next 01/11] bpfilter: Add types for usermode helper Dmitrii Banshchikov
2021-05-17 22:52 ` [PATCH bpf-next 02/11] bpfilter: Add logging facility Dmitrii Banshchikov
2021-05-19 17:32 ` Song Liu
2021-05-20 7:08 ` Dmitrii Banshchikov
2021-05-20 16:35 ` Song Liu
2021-05-21 6:46 ` Dmitrii Banshchikov
2021-05-17 22:53 ` [PATCH bpf-next 03/11] bpfilter: Add IO functions Dmitrii Banshchikov
2021-05-19 18:47 ` Song Liu
2021-05-17 22:53 ` [PATCH bpf-next 04/11] tools: Add bpfilter usermode helper header Dmitrii Banshchikov
2021-05-17 22:53 ` [PATCH bpf-next 05/11] bpfilter: Add map container Dmitrii Banshchikov
2021-05-17 22:53 ` [PATCH bpf-next 06/11] bpfilter: Add struct match Dmitrii Banshchikov
2021-05-20 4:26 ` Song Liu
2021-05-20 7:31 ` Dmitrii Banshchikov
2021-05-20 17:44 ` Song Liu
2021-05-17 22:53 ` [PATCH bpf-next 07/11] bpfilter: Add struct target Dmitrii Banshchikov
2021-05-20 4:36 ` Song Liu
2021-05-20 7:44 ` Dmitrii Banshchikov [this message]
2021-05-17 22:53 ` [PATCH bpf-next 08/11] bpfilter: Add struct rule Dmitrii Banshchikov
2021-05-17 22:53 ` [PATCH bpf-next 09/11] bpfilter: Add struct table Dmitrii Banshchikov
2021-05-20 18:07 ` Song Liu
2021-05-17 22:53 ` [PATCH bpf-next 10/11] bpfilter: Add handling of setsockopt() calls Dmitrii Banshchikov
2021-05-17 22:53 ` [PATCH bpf-next 11/11] bpfilter: Handle setsockopts Dmitrii Banshchikov
2021-05-20 4:54 ` [PATCH bpf-next 00/11] bpfilter Song Liu
2021-05-20 7:53 ` Dmitrii Banshchikov
2021-05-20 16:55 ` Alexei Starovoitov
2021-05-20 17:56 ` Song Liu
2021-05-21 6:00 ` Dmitrii Banshchikov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210520074436.pxbqvwkvw4gihizj@amnesia \
--to=me@ubique.spb.ru \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=john.fastabend@gmail.com \
--cc=kafai@fb.com \
--cc=kpsingh@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=rdna@fb.com \
--cc=songliubraving@fb.com \
--cc=yhs@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox