-
Notifications
You must be signed in to change notification settings - Fork 54
Add NEXT verdict for BPF program chaining #474
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
17c21dc
e1207d1
8414730
4ce74d0
0ba579e
982fc4e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -340,7 +340,13 @@ With: | |||||
| - ``BF_HOOK_NF_LOCAL_OUT``: similar to ``nftables`` and ``iptables`` output hook. | ||||||
| - ``BF_HOOK_NF_POST_ROUTING``: similar to ``nftables`` and ``iptables`` postrouting hook. | ||||||
| - ``BF_HOOK_TC_EGRESS``: egress TC hook. | ||||||
| - ``$POLICY``: action taken if no rule matches the packet, either ``ACCEPT`` forward the packet to the kernel, or ``DROP`` to discard it. Note while ``CONTINUE`` is a valid verdict for rules, it is not supported for chain policy. | ||||||
| - ``$POLICY``: action taken if no rule matches the packet: | ||||||
|
|
||||||
| - ``ACCEPT``: forward the packet to the kernel. | ||||||
| - ``DROP``: discard the packet. | ||||||
| - ``NEXT``: pass the packet to the next BPF program. For TC hooks, this maps to ``TCX_NEXT``, deferring the decision to the next program in the TCX link. For NF, XDP, and cgroup_skb hooks, ``NEXT`` behaves identically to ``ACCEPT`` since these hooks do not distinguish between "accept" and "pass to next program." | ||||||
|
|
||||||
| Note: ``CONTINUE`` and ``REDIRECT`` are not valid chain policies. | ||||||
|
|
||||||
| ``$OPTIONS`` are hook-specific comma separated key value pairs. A given hook option can only be specified once: | ||||||
|
|
||||||
|
|
@@ -391,13 +397,14 @@ With: | |||||
| - ``log``: optional. If set, log the requested protocol headers. ``link`` will log the link (layer 2) header, ``internet`` with log the internet (layer 3) header, and ``transport`` will log the transport (layer 4) header. At least one header type is required. | ||||||
| - ``counter``: optional literal. If set, the filter will counter the number of packets and bytes matched by the rule. | ||||||
| - ``mark``: optional, ``$MARK`` must be a valid decimal or hexadecimal 32-bits value. If set, write the packet's marker value. This marker can be used later on in a rule (see ``meta.mark``) or with a TC filter. | ||||||
| - ``$VERDICT``: action taken by the rule if the packet is matched against **all** the criteria: either ``ACCEPT``, ``DROP``, ``CONTINUE``, or ``REDIRECT``. | ||||||
| - ``$VERDICT``: action taken by the rule if the packet is matched against **all** the criteria: either ``ACCEPT``, ``DROP``, ``CONTINUE``, ``NEXT``, or ``REDIRECT``. | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit:
Suggested change
Not sure why you want to list the verdicts and then list them again with descriptions |
||||||
| - ``ACCEPT``: forward the packet to the kernel. | ||||||
| - ``DROP``: discard the packet. | ||||||
| - ``CONTINUE``: continue processing subsequent rules. | ||||||
| - ``CONTINUE``: continue processing subsequent rules (non-terminal). | ||||||
| - ``NEXT``: stop rule processing and pass the packet to the next BPF program. For TC hooks, this returns ``TCX_NEXT``. For NF, XDP, and cgroup_skb hooks, this is equivalent to ``ACCEPT``. | ||||||
| - ``REDIRECT $IFACE $DIR``: redirect the packet to interface ``$IFACE`` in direction ``$DIR``. ``$IFACE`` can be an interface name (e.g., "eth0") or an interface index (e.g., "2"). ``$DIR`` is either ``in`` for ingress or ``out`` for egress. | ||||||
|
|
||||||
| In a chain, as soon as a rule matches a packet, its verdict is applied. If the verdict is ``ACCEPT``, ``DROP``, or ``REDIRECT``, the subsequent rules are not processed. Hence, the rules' order matters. If no rule matches the packet, the chain's policy is applied. | ||||||
| In a chain, as soon as a rule matches a packet, its verdict is applied. If the verdict is ``ACCEPT``, ``DROP``, ``NEXT``, or ``REDIRECT``, the subsequent rules are not processed. Hence, the rules' order matters. If no rule matches the packet, the chain's policy is applied. | ||||||
|
|
||||||
| Note ``CONTINUE`` means a packet can be counted more than once if multiple rules specify ``CONTINUE`` and ``counter``. | ||||||
|
|
||||||
|
|
@@ -409,6 +416,10 @@ Note ``CONTINUE`` means a packet can be counted more than once if multiple rules | |||||
|
|
||||||
| ``REDIRECT`` is **not** supported by Netfilter (``BF_HOOK_NF_*``) or cgroup_skb (``BF_HOOK_CGROUP_SKB_*``) hooks. | ||||||
|
|
||||||
| .. note:: | ||||||
|
|
||||||
| ``NEXT`` has distinct behavior only for TC hooks (``BF_HOOK_TC_INGRESS``, ``BF_HOOK_TC_EGRESS``), where it maps to ``TCX_NEXT`` and defers to the next BPF program in the TCX link. For all other hooks (Netfilter, XDP, cgroup_skb), ``NEXT`` produces the same return code as ``ACCEPT``. | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit
Suggested change
|
||||||
|
|
||||||
| Sets | ||||||
| ~~~~ | ||||||
|
|
||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,6 @@ | |
| #include <bpfilter/matcher.h> | ||
| #include <bpfilter/verdict.h> | ||
|
|
||
| #include "cgen/cgen.h" | ||
| #include "cgen/matcher/cmp.h" | ||
| #include "cgen/matcher/packet.h" | ||
| #include "cgen/program.h" | ||
|
|
@@ -141,14 +140,20 @@ static int _bf_cgroup_skb_gen_inline_matcher(struct bf_program *program, | |
| * Convert a standard verdict into a return value. | ||
| * | ||
| * @param verdict Verdict to convert. Must be valid. | ||
| * @return Cgroup return code corresponding to the verdict, as an integer. | ||
| * @param ret_code Cgroup return code. Can't be NULL. | ||
| * @return 0 on success, or a negative errno value on failure. | ||
| */ | ||
| static int _bf_cgroup_skb_get_verdict(enum bf_verdict verdict) | ||
| static int _bf_cgroup_skb_get_verdict(enum bf_verdict verdict, int *ret_code) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since these are internal APIs, I think you could have kept the same return mechanism without adding the |
||
| { | ||
| assert(ret_code); | ||
|
|
||
| switch (verdict) { | ||
| case BF_VERDICT_ACCEPT: | ||
| return 1; | ||
| case BF_VERDICT_NEXT: | ||
| *ret_code = 1; | ||
| return 0; | ||
| case BF_VERDICT_DROP: | ||
| *ret_code = 0; | ||
| return 0; | ||
| default: | ||
| return -ENOTSUP; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to call out that if there are no other BPF programs then this acts like
ACCEPT?