Re: [PATCH bpf-next v1 07/10] bpfilter: Add struct rule

On Wed, Jun 09, 2021 at 05:30:56PM -0700, Yonghong Song wrote:

On 6/3/21 3:14 AM, Dmitrii Banshchikov wrote:

quoted

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 <redacted>
---
  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

[...]

quoted

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