From: Dmitrii Banshchikov <me@ubique.spb.ru>
To: Yonghong Song <yhs@fb.com>
Cc: bpf@vger.kernel.org, ast@kernel.org, davem@davemloft.net,
daniel@iogearbox.net, andrii@kernel.org, kafai@fb.com,
songliubraving@fb.com, john.fastabend@gmail.com,
kpsingh@kernel.org, netdev@vger.kernel.org, rdna@fb.com
Subject: Re: [PATCH bpf-next v1 07/10] bpfilter: Add struct rule
Date: Thu, 10 Jun 2021 17:16:35 +0400 [thread overview]
Message-ID: <20210610131635.w5pshflih4che74s@amnesia> (raw)
In-Reply-To: <8040518a-572a-18d8-5a50-fd3e82f13f5c@fb.com>
On Wed, Jun 09, 2021 at 05:30:56PM -0700, Yonghong Song wrote:
>
>
> On 6/3/21 3:14 AM, Dmitrii Banshchikov wrote:
> > struct rule is an equivalent of struct ipt_entry. A rule consists of
> > zero or more matches and a target. A rule has a pointer to its ipt_entry
> > in entries blob. struct rule should simplify iteration over a blob and
> > avoid blob's guts in code generation.
> >
> > Signed-off-by: Dmitrii Banshchikov <me@ubique.spb.ru>
> > ---
> > net/bpfilter/Makefile | 2 +-
> > net/bpfilter/rule.c | 163 ++++++++++++++++++
> > net/bpfilter/rule.h | 32 ++++
> > .../testing/selftests/bpf/bpfilter/.gitignore | 1 +
> > tools/testing/selftests/bpf/bpfilter/Makefile | 5 +-
> > .../selftests/bpf/bpfilter/bpfilter_util.h | 8 +
> > .../selftests/bpf/bpfilter/test_rule.c | 55 ++++++
> > 7 files changed, 264 insertions(+), 2 deletions(-)
> > create mode 100644 net/bpfilter/rule.c
> > create mode 100644 net/bpfilter/rule.h
> > create mode 100644 tools/testing/selftests/bpf/bpfilter/test_rule.c
> >
> [...]
> > +
> > +bool rule_has_standard_target(const struct rule *rule);
> > +bool is_rule_unconditional(const struct rule *rule);
> > +int init_rule(struct context *ctx, const struct bpfilter_ipt_entry *ipt_entry, struct rule *rule);
> > +void free_rule(struct rule *rule);
> > +
> > +#endif // NET_BPFILTER_RULE_H
> > diff --git a/tools/testing/selftests/bpf/bpfilter/.gitignore b/tools/testing/selftests/bpf/bpfilter/.gitignore
> > index 7e077f506af1..4d7c5083d980 100644
> > --- a/tools/testing/selftests/bpf/bpfilter/.gitignore
> > +++ b/tools/testing/selftests/bpf/bpfilter/.gitignore
> > @@ -2,3 +2,4 @@
> > test_map
> > test_match
> > test_target
> > +test_rule
> > diff --git a/tools/testing/selftests/bpf/bpfilter/Makefile b/tools/testing/selftests/bpf/bpfilter/Makefile
> > index a11775e8b5af..27a1ddcb6dc9 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_map
> > TEST_GEN_PROGS += test_match
> > TEST_GEN_PROGS += test_target
> > +TEST_GEN_PROGS += test_rule
> > KSFT_KHDR_INSTALL := 1
> > @@ -19,9 +20,11 @@ include ../../lib.mk
> > BPFILTER_MATCH_SRCS := $(BPFILTERSRCDIR)/match.c $(BPFILTERSRCDIR)/xt_udp.c
> > BPFILTER_TARGET_SRCS := $(BPFILTERSRCDIR)/target.c
> > -BPFILTER_COMMON_SRCS := $(BPFILTERSRCDIR)/map-common.c $(BPFILTERSRCDIR)/context.c
> > +BPFILTER_COMMON_SRCS := $(BPFILTERSRCDIR)/map-common.c $(BPFILTERSRCDIR)/context.c \
> > + $(BPFILTERSRCDIR)/rule.c
> > BPFILTER_COMMON_SRCS += $(BPFILTER_MATCH_SRCS) $(BPFILTER_TARGET_SRCS)
> > $(OUTPUT)/test_map: test_map.c $(BPFILTERSRCDIR)/map-common.c
> > $(OUTPUT)/test_match: test_match.c $(BPFILTER_COMMON_SRCS)
> > $(OUTPUT)/test_target: test_target.c $(BPFILTER_COMMON_SRCS)
> > +$(OUTPUT)/test_rule: test_rule.c $(BPFILTER_COMMON_SRCS)
> > diff --git a/tools/testing/selftests/bpf/bpfilter/bpfilter_util.h b/tools/testing/selftests/bpf/bpfilter/bpfilter_util.h
> > index d82ff86f280e..55fb0e959fca 100644
> > --- a/tools/testing/selftests/bpf/bpfilter/bpfilter_util.h
> > +++ b/tools/testing/selftests/bpf/bpfilter/bpfilter_util.h
> > @@ -7,6 +7,7 @@
> > #include <linux/netfilter/x_tables.h>
> > #include <stdio.h>
> > +#include <string.h>
> > static inline void init_standard_target(struct xt_standard_target *ipt_target, int revision,
> > int verdict)
> > @@ -28,4 +29,11 @@ static inline void init_error_target(struct xt_error_target *ipt_target, int rev
> > snprintf(ipt_target->errorname, sizeof(ipt_target->errorname), "%s", error_name);
> > }
> > +static inline void init_standard_entry(struct ipt_entry *entry, __u16 matches_size)
> > +{
> > + memset(entry, 0, sizeof(*entry));
> > + entry->target_offset = sizeof(*entry) + matches_size;
> > + entry->next_offset = sizeof(*entry) + matches_size + sizeof(struct xt_standard_target);
> > +}
> > +
> > #endif // BPFILTER_UTIL_H
> > diff --git a/tools/testing/selftests/bpf/bpfilter/test_rule.c b/tools/testing/selftests/bpf/bpfilter/test_rule.c
> > new file mode 100644
> > index 000000000000..fe12adf32fe5
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/bpfilter/test_rule.c
> > @@ -0,0 +1,55 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +#define _GNU_SOURCE
> > +
> > +#include "rule.h"
> > +
> > +#include <linux/bpfilter.h>
> > +#include <linux/err.h>
> > +
> > +#include <linux/netfilter_ipv4/ip_tables.h>
> > +
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +
> > +#include "../../kselftest_harness.h"
> > +
> > +#include "context.h"
> > +#include "rule.h"
> > +
> > +#include "bpfilter_util.h"
> > +
> > +FIXTURE(test_standard_rule)
> > +{
> > + struct context ctx;
> > + struct {
> > + struct ipt_entry entry;
> > + struct xt_standard_target target;
> > + } entry;
> > + struct rule rule;
> > +};
> > +
> > +FIXTURE_SETUP(test_standard_rule)
> > +{
> > + const int verdict = BPFILTER_NF_ACCEPT;
> > +
> > + ASSERT_EQ(create_context(&self->ctx), 0);
> > + self->ctx.log_file = stderr;
> > +
> > + init_standard_entry(&self->entry.entry, 0);
> > + init_standard_target(&self->entry.target, 0, -verdict - 1);
> > +}
> > +
> > +FIXTURE_TEARDOWN(test_standard_rule)
> > +{
> > + free_rule(&self->rule);
> > + free_context(&self->ctx);
> > +}
> > +
> > +TEST_F(test_standard_rule, init)
> > +{
> > + ASSERT_EQ(0, init_rule(&self->ctx, (const struct bpfilter_ipt_entry *)&self->entry.entry,
> > + &self->rule));
> > +}
> > +
> > +TEST_HARNESS_MAIN
>
> When compiling selftests/bpf/bpfilter, I got the following compilation
> warning:
>
> gcc -Wall -g -pthread -I/home/yhs/work/bpf-next/tools/include
> -I/home/yhs/work/bpf-next/tools/include/uapi -I../../../../../net/bpfilter
> test_rule.c ../../../../../net/bpfilter/map-common.c
> ../../../../../net/bpfilter/context.c ../../../../../net/bpfilter/rule.c
> ../../../../../net/bpfilter/table.c ../../../../../net/bpfilter/match.c
> ../../../../../net/bpfilter/xt_udp.c ../../../../../net/bpfilter/target.c
> -o /home/yhs/work/bpf-next/tools/testing/selftests/bpf/bpfilter/test_rule
> In file included from test_rule.c:15:
> ../../kselftest_harness.h:674: warning: "ARRAY_SIZE" redefined
> #define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))
>
> In file included from /usr/include/linux/netfilter/x_tables.h:4,
> from /usr/include/linux/netfilter_ipv4/ip_tables.h:24,
> from test_rule.c:10:
> /home/yhs/work/bpf-next/tools/include/linux/kernel.h:105: note: this is the
> location of the previous definition
> #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) +
> __must_be_array(arr))
>
Hmm. I cannot reproduce it locally now though I saw this error and
fixed it by removing some of the includes from header files.
I will double check it.
--
Dmitrii Banshchikov
next prev parent reply other threads:[~2021-06-10 13:17 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-03 10:14 [PATCH bpf-next v1 00/10] bpfilter Dmitrii Banshchikov
2021-06-03 10:14 ` [PATCH bpf-next v1 01/10] bpfilter: Add types for usermode helper Dmitrii Banshchikov
2021-06-03 10:14 ` [PATCH bpf-next v1 02/10] bpfilter: Add logging facility Dmitrii Banshchikov
2021-06-03 10:14 ` [PATCH bpf-next v1 03/10] tools: Add bpfilter usermode helper header Dmitrii Banshchikov
2021-06-08 16:20 ` Yonghong Song
2021-06-09 10:07 ` Dmitrii Banshchikov
2021-06-03 10:14 ` [PATCH bpf-next v1 04/10] bpfilter: Add map container Dmitrii Banshchikov
2021-06-03 10:14 ` [PATCH bpf-next v1 05/10] bpfilter: Add struct match Dmitrii Banshchikov
2021-06-03 10:14 ` [PATCH bpf-next v1 06/10] bpfilter: Add struct target Dmitrii Banshchikov
2021-06-03 16:47 ` Dmitrii Banshchikov
2021-06-03 10:14 ` [PATCH bpf-next v1 07/10] bpfilter: Add struct rule Dmitrii Banshchikov
2021-06-10 0:30 ` Yonghong Song
2021-06-10 13:16 ` Dmitrii Banshchikov [this message]
2021-06-03 10:14 ` [PATCH bpf-next v1 08/10] bpfilter: Add struct table Dmitrii Banshchikov
2021-06-03 10:14 ` [PATCH bpf-next v1 09/10] bpfilter: Add handling of setsockopt() calls Dmitrii Banshchikov
2021-06-03 10:14 ` [PATCH bpf-next v1 10/10] bpfilter: Handle setsockopts Dmitrii Banshchikov
2021-06-10 0:50 ` [PATCH bpf-next v1 00/10] bpfilter Yonghong Song
2021-06-10 13:36 ` Dmitrii Banshchikov
2021-06-10 13:58 ` Daniel Borkmann
2021-06-10 14:56 ` Yonghong Song
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=20210610131635.w5pshflih4che74s@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