* [PATCH nf-next 0/4] netfilter: use skb_drop_reason in more places
@ 2024-10-02 15:55 Florian Westphal
2024-10-02 15:55 ` [PATCH nf-next 1/4] netfilter: xt_nat: compact nf_nat_setup_info calls Florian Westphal
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Florian Westphal @ 2024-10-02 15:55 UTC (permalink / raw)
To: netfilter-devel; +Cc: Florian Westphal
Provide more precise drop information rather than doing the freeing
from core.c:nf_hook_slow().
First patch is a small preparation patch, rest coverts NF_DROP
locations of NF_DROP_REASON().
Florian Westphal (4):
netfilter: xt_nat: compact nf_nat_setup_info calls
netfilter: xt_nat: drop packet earlier
netfilter: nf_nat: use skb_drop_reason
netfilter: nf_tables: use skb_drop_reason
include/linux/netfilter.h | 5 +-
net/bridge/netfilter/nft_reject_bridge.c | 2 +-
net/ipv4/netfilter/nft_reject_ipv4.c | 2 +-
net/ipv6/netfilter/nf_defrag_ipv6_hooks.c | 5 +-
net/netfilter/nf_nat_masquerade.c | 23 +++++--
net/netfilter/nf_nat_proto.c | 18 +++---
net/netfilter/nf_nat_redirect.c | 4 +-
net/netfilter/nf_synproxy_core.c | 16 ++---
net/netfilter/nft_chain_filter.c | 4 +-
net/netfilter/nft_compat.c | 8 +--
net/netfilter/nft_connlimit.c | 4 +-
net/netfilter/nft_ct.c | 14 ++--
net/netfilter/nft_exthdr.c | 2 +-
net/netfilter/nft_fib_inet.c | 2 +-
net/netfilter/nft_fwd_netdev.c | 4 +-
net/netfilter/nft_nat.c | 8 ++-
net/netfilter/nft_reject_inet.c | 2 +-
net/netfilter/nft_reject_netdev.c | 2 +-
net/netfilter/nft_synproxy.c | 10 +--
net/netfilter/xt_nat.c | 78 ++++++++++-------------
20 files changed, 112 insertions(+), 101 deletions(-)
--
2.45.2
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH nf-next 1/4] netfilter: xt_nat: compact nf_nat_setup_info calls
2024-10-02 15:55 [PATCH nf-next 0/4] netfilter: use skb_drop_reason in more places Florian Westphal
@ 2024-10-02 15:55 ` Florian Westphal
2024-10-02 15:55 ` [PATCH nf-next 2/4] netfilter: xt_nat: drop packet earlier Florian Westphal
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Florian Westphal @ 2024-10-02 15:55 UTC (permalink / raw)
To: netfilter-devel; +Cc: Florian Westphal
Collapse some common code, this will allow to use
kfree_skb_drop_reason in the next patch.
No semantic change intended.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/netfilter/xt_nat.c | 73 ++++++++++++++++--------------------------
1 file changed, 28 insertions(+), 45 deletions(-)
diff --git a/net/netfilter/xt_nat.c b/net/netfilter/xt_nat.c
index b4f7bbc3f3ca..d04f7cf6b94d 100644
--- a/net/netfilter/xt_nat.c
+++ b/net/netfilter/xt_nat.c
@@ -13,6 +13,26 @@
#include <linux/netfilter/x_tables.h>
#include <net/netfilter/nf_nat.h>
+static unsigned int
+xt_nat_setup_info(struct sk_buff *skb,
+ const struct nf_nat_range2 *range,
+ enum nf_nat_manip_type maniptype)
+{
+ enum ip_conntrack_info ctinfo;
+ struct nf_conn *ct;
+
+ ct = nf_ct_get(skb, &ctinfo);
+ if (WARN_ON(!ct))
+ return NF_ACCEPT;
+
+ if (WARN_ON(!(ctinfo == IP_CT_NEW ||
+ ctinfo == IP_CT_RELATED ||
+ (ctinfo == IP_CT_RELATED_REPLY && maniptype == NF_NAT_MANIP_SRC))))
+ return NF_ACCEPT;
+
+ return nf_nat_setup_info(ct, range, maniptype);
+}
+
static int xt_nat_checkentry_v0(const struct xt_tgchk_param *par)
{
const struct nf_nat_ipv4_multi_range_compat *mr = par->targinfo;
@@ -53,16 +73,10 @@ xt_snat_target_v0(struct sk_buff *skb, const struct xt_action_param *par)
{
const struct nf_nat_ipv4_multi_range_compat *mr = par->targinfo;
struct nf_nat_range2 range;
- enum ip_conntrack_info ctinfo;
- struct nf_conn *ct;
-
- ct = nf_ct_get(skb, &ctinfo);
- WARN_ON(!(ct != NULL &&
- (ctinfo == IP_CT_NEW || ctinfo == IP_CT_RELATED ||
- ctinfo == IP_CT_RELATED_REPLY)));
xt_nat_convert_range(&range, &mr->range[0]);
- return nf_nat_setup_info(ct, &range, NF_NAT_MANIP_SRC);
+
+ return xt_nat_setup_info(skb, &range, NF_NAT_MANIP_SRC);
}
static unsigned int
@@ -70,15 +84,10 @@ xt_dnat_target_v0(struct sk_buff *skb, const struct xt_action_param *par)
{
const struct nf_nat_ipv4_multi_range_compat *mr = par->targinfo;
struct nf_nat_range2 range;
- enum ip_conntrack_info ctinfo;
- struct nf_conn *ct;
-
- ct = nf_ct_get(skb, &ctinfo);
- WARN_ON(!(ct != NULL &&
- (ctinfo == IP_CT_NEW || ctinfo == IP_CT_RELATED)));
xt_nat_convert_range(&range, &mr->range[0]);
- return nf_nat_setup_info(ct, &range, NF_NAT_MANIP_DST);
+
+ return xt_nat_setup_info(skb, &range, NF_NAT_MANIP_DST);
}
static unsigned int
@@ -86,18 +95,11 @@ xt_snat_target_v1(struct sk_buff *skb, const struct xt_action_param *par)
{
const struct nf_nat_range *range_v1 = par->targinfo;
struct nf_nat_range2 range;
- enum ip_conntrack_info ctinfo;
- struct nf_conn *ct;
-
- ct = nf_ct_get(skb, &ctinfo);
- WARN_ON(!(ct != NULL &&
- (ctinfo == IP_CT_NEW || ctinfo == IP_CT_RELATED ||
- ctinfo == IP_CT_RELATED_REPLY)));
memcpy(&range, range_v1, sizeof(*range_v1));
memset(&range.base_proto, 0, sizeof(range.base_proto));
- return nf_nat_setup_info(ct, &range, NF_NAT_MANIP_SRC);
+ return xt_nat_setup_info(skb, &range, NF_NAT_MANIP_SRC);
}
static unsigned int
@@ -105,46 +107,27 @@ xt_dnat_target_v1(struct sk_buff *skb, const struct xt_action_param *par)
{
const struct nf_nat_range *range_v1 = par->targinfo;
struct nf_nat_range2 range;
- enum ip_conntrack_info ctinfo;
- struct nf_conn *ct;
-
- ct = nf_ct_get(skb, &ctinfo);
- WARN_ON(!(ct != NULL &&
- (ctinfo == IP_CT_NEW || ctinfo == IP_CT_RELATED)));
memcpy(&range, range_v1, sizeof(*range_v1));
memset(&range.base_proto, 0, sizeof(range.base_proto));
- return nf_nat_setup_info(ct, &range, NF_NAT_MANIP_DST);
+ return xt_nat_setup_info(skb, &range, NF_NAT_MANIP_DST);
}
static unsigned int
xt_snat_target_v2(struct sk_buff *skb, const struct xt_action_param *par)
{
const struct nf_nat_range2 *range = par->targinfo;
- enum ip_conntrack_info ctinfo;
- struct nf_conn *ct;
-
- ct = nf_ct_get(skb, &ctinfo);
- WARN_ON(!(ct != NULL &&
- (ctinfo == IP_CT_NEW || ctinfo == IP_CT_RELATED ||
- ctinfo == IP_CT_RELATED_REPLY)));
- return nf_nat_setup_info(ct, range, NF_NAT_MANIP_SRC);
+ return xt_nat_setup_info(skb, range, NF_NAT_MANIP_SRC);
}
static unsigned int
xt_dnat_target_v2(struct sk_buff *skb, const struct xt_action_param *par)
{
const struct nf_nat_range2 *range = par->targinfo;
- enum ip_conntrack_info ctinfo;
- struct nf_conn *ct;
-
- ct = nf_ct_get(skb, &ctinfo);
- WARN_ON(!(ct != NULL &&
- (ctinfo == IP_CT_NEW || ctinfo == IP_CT_RELATED)));
- return nf_nat_setup_info(ct, range, NF_NAT_MANIP_DST);
+ return xt_nat_setup_info(skb, range, NF_NAT_MANIP_DST);
}
static struct xt_target xt_nat_target_reg[] __read_mostly = {
--
2.45.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH nf-next 2/4] netfilter: xt_nat: drop packet earlier
2024-10-02 15:55 [PATCH nf-next 0/4] netfilter: use skb_drop_reason in more places Florian Westphal
2024-10-02 15:55 ` [PATCH nf-next 1/4] netfilter: xt_nat: compact nf_nat_setup_info calls Florian Westphal
@ 2024-10-02 15:55 ` Florian Westphal
2024-10-02 15:55 ` [PATCH nf-next 3/4] netfilter: nf_nat: use skb_drop_reason Florian Westphal
` (2 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Florian Westphal @ 2024-10-02 15:55 UTC (permalink / raw)
To: netfilter-devel; +Cc: Florian Westphal
Let net dropmonitor pick up a more specific location rather than the
catchall core.c:nf_hook_slow drop point.
This isn't moved into nf_nat_setup_info() because we do not pass
the skb to it.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/netfilter/xt_nat.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/net/netfilter/xt_nat.c b/net/netfilter/xt_nat.c
index d04f7cf6b94d..aaf31bd8381b 100644
--- a/net/netfilter/xt_nat.c
+++ b/net/netfilter/xt_nat.c
@@ -20,6 +20,7 @@ xt_nat_setup_info(struct sk_buff *skb,
{
enum ip_conntrack_info ctinfo;
struct nf_conn *ct;
+ int ret;
ct = nf_ct_get(skb, &ctinfo);
if (WARN_ON(!ct))
@@ -30,7 +31,11 @@ xt_nat_setup_info(struct sk_buff *skb,
(ctinfo == IP_CT_RELATED_REPLY && maniptype == NF_NAT_MANIP_SRC))))
return NF_ACCEPT;
- return nf_nat_setup_info(ct, range, maniptype);
+ ret = nf_nat_setup_info(ct, range, maniptype);
+ if (ret != NF_DROP)
+ return ret;
+
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
}
static int xt_nat_checkentry_v0(const struct xt_tgchk_param *par)
--
2.45.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH nf-next 3/4] netfilter: nf_nat: use skb_drop_reason
2024-10-02 15:55 [PATCH nf-next 0/4] netfilter: use skb_drop_reason in more places Florian Westphal
2024-10-02 15:55 ` [PATCH nf-next 1/4] netfilter: xt_nat: compact nf_nat_setup_info calls Florian Westphal
2024-10-02 15:55 ` [PATCH nf-next 2/4] netfilter: xt_nat: drop packet earlier Florian Westphal
@ 2024-10-02 15:55 ` Florian Westphal
2024-10-02 15:55 ` [PATCH nf-next 4/4] netfilter: nf_tables: " Florian Westphal
2024-10-12 14:09 ` [PATCH nf-next 0/4] netfilter: use skb_drop_reason in more places Pablo Neira Ayuso
4 siblings, 0 replies; 11+ messages in thread
From: Florian Westphal @ 2024-10-02 15:55 UTC (permalink / raw)
To: netfilter-devel; +Cc: Florian Westphal
same as previous patch: extend nftables nat and masquerade functions to
indicate more precise drop locations.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/netfilter/nf_nat_masquerade.c | 23 ++++++++++++++++-------
net/netfilter/nft_nat.c | 8 +++++++-
2 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/net/netfilter/nf_nat_masquerade.c b/net/netfilter/nf_nat_masquerade.c
index 1a506b0c6511..06f5968dbc15 100644
--- a/net/netfilter/nf_nat_masquerade.c
+++ b/net/netfilter/nf_nat_masquerade.c
@@ -35,6 +35,7 @@ nf_nat_masquerade_ipv4(struct sk_buff *skb, unsigned int hooknum,
struct nf_nat_range2 newrange;
const struct rtable *rt;
__be32 newsrc, nh;
+ int ret;
WARN_ON(hooknum != NF_INET_POST_ROUTING);
@@ -52,10 +53,8 @@ nf_nat_masquerade_ipv4(struct sk_buff *skb, unsigned int hooknum,
rt = skb_rtable(skb);
nh = rt_nexthop(rt, ip_hdr(skb)->daddr);
newsrc = inet_select_addr(out, nh, RT_SCOPE_UNIVERSE);
- if (!newsrc) {
- pr_info("%s ate my IP address\n", out->name);
- return NF_DROP;
- }
+ if (!newsrc)
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EADDRNOTAVAIL);
nat = nf_ct_nat_ext_add(ct);
if (nat)
@@ -71,7 +70,12 @@ nf_nat_masquerade_ipv4(struct sk_buff *skb, unsigned int hooknum,
newrange.max_proto = range->max_proto;
/* Hand modified range to generic setup. */
- return nf_nat_setup_info(ct, &newrange, NF_NAT_MANIP_SRC);
+ ret = nf_nat_setup_info(ct, &newrange, NF_NAT_MANIP_SRC);
+ if (ret == NF_DROP)
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP,
+ EPERM);
+
+ return ret;
}
EXPORT_SYMBOL_GPL(nf_nat_masquerade_ipv4);
@@ -246,6 +250,7 @@ nf_nat_masquerade_ipv6(struct sk_buff *skb, const struct nf_nat_range2 *range,
struct in6_addr src;
struct nf_conn *ct;
struct nf_nat_range2 newrange;
+ int ret;
ct = nf_ct_get(skb, &ctinfo);
WARN_ON(!(ct && (ctinfo == IP_CT_NEW || ctinfo == IP_CT_RELATED ||
@@ -253,7 +258,7 @@ nf_nat_masquerade_ipv6(struct sk_buff *skb, const struct nf_nat_range2 *range,
if (nat_ipv6_dev_get_saddr(nf_ct_net(ct), out,
&ipv6_hdr(skb)->daddr, 0, &src) < 0)
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EADDRNOTAVAIL);
nat = nf_ct_nat_ext_add(ct);
if (nat)
@@ -265,7 +270,11 @@ nf_nat_masquerade_ipv6(struct sk_buff *skb, const struct nf_nat_range2 *range,
newrange.min_proto = range->min_proto;
newrange.max_proto = range->max_proto;
- return nf_nat_setup_info(ct, &newrange, NF_NAT_MANIP_SRC);
+ ret = nf_nat_setup_info(ct, &newrange, NF_NAT_MANIP_SRC);
+ if (ret == NF_DROP)
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
+
+ return ret;
}
EXPORT_SYMBOL_GPL(nf_nat_masquerade_ipv6);
diff --git a/net/netfilter/nft_nat.c b/net/netfilter/nft_nat.c
index 6e21f72c5b57..bd2bda5c2b13 100644
--- a/net/netfilter/nft_nat.c
+++ b/net/netfilter/nft_nat.c
@@ -108,6 +108,7 @@ static void nft_nat_eval(const struct nft_expr *expr,
enum ip_conntrack_info ctinfo;
struct nf_conn *ct = nf_ct_get(pkt->skb, &ctinfo);
struct nf_nat_range2 range;
+ int verdict;
memset(&range, 0, sizeof(range));
@@ -122,7 +123,12 @@ static void nft_nat_eval(const struct nft_expr *expr,
range.flags = priv->flags;
- regs->verdict.code = nf_nat_setup_info(ct, &range, priv->type);
+ verdict = nf_nat_setup_info(ct, &range, priv->type);
+ if (verdict == NF_DROP)
+ verdict = NF_DROP_REASON(pkt->skb,
+ SKB_DROP_REASON_NETFILTER_DROP,
+ EPERM);
+ regs->verdict.code = verdict;
}
static const struct nla_policy nft_nat_policy[NFTA_NAT_MAX + 1] = {
--
2.45.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH nf-next 4/4] netfilter: nf_tables: use skb_drop_reason
2024-10-02 15:55 [PATCH nf-next 0/4] netfilter: use skb_drop_reason in more places Florian Westphal
` (2 preceding siblings ...)
2024-10-02 15:55 ` [PATCH nf-next 3/4] netfilter: nf_nat: use skb_drop_reason Florian Westphal
@ 2024-10-02 15:55 ` Florian Westphal
2024-10-12 14:09 ` [PATCH nf-next 0/4] netfilter: use skb_drop_reason in more places Pablo Neira Ayuso
4 siblings, 0 replies; 11+ messages in thread
From: Florian Westphal @ 2024-10-02 15:55 UTC (permalink / raw)
To: netfilter-devel; +Cc: Florian Westphal
same as previous patch: extend nftables nat and masquerade functions to
indicate more precise drop locations.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
include/linux/netfilter.h | 5 ++++-
net/bridge/netfilter/nft_reject_bridge.c | 2 +-
net/ipv4/netfilter/nft_reject_ipv4.c | 2 +-
net/ipv6/netfilter/nf_defrag_ipv6_hooks.c | 5 ++++-
net/netfilter/nf_nat_proto.c | 18 +++++++++---------
net/netfilter/nf_nat_redirect.c | 4 ++--
net/netfilter/nf_synproxy_core.c | 16 ++++++++--------
net/netfilter/nft_chain_filter.c | 4 ++--
net/netfilter/nft_compat.c | 8 ++++----
net/netfilter/nft_connlimit.c | 4 ++--
net/netfilter/nft_ct.c | 14 ++++++++------
net/netfilter/nft_exthdr.c | 2 +-
net/netfilter/nft_fib_inet.c | 2 +-
net/netfilter/nft_fwd_netdev.c | 4 ++--
net/netfilter/nft_reject_inet.c | 2 +-
net/netfilter/nft_reject_netdev.c | 2 +-
net/netfilter/nft_synproxy.c | 10 +++++-----
17 files changed, 56 insertions(+), 48 deletions(-)
diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
index 2683b2b77612..5b350de9b4c7 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -25,7 +25,10 @@ static inline int NF_DROP_GETERR(int verdict)
static __always_inline int
NF_DROP_REASON(struct sk_buff *skb, enum skb_drop_reason reason, u32 err)
{
- BUILD_BUG_ON(err > 0xffff);
+ if (__builtin_constant_p(err))
+ BUILD_BUG_ON(err > 0xffff);
+ else if (WARN_ON_ONCE(err > 0xffff))
+ err = 0;
kfree_skb_reason(skb, reason);
diff --git a/net/bridge/netfilter/nft_reject_bridge.c b/net/bridge/netfilter/nft_reject_bridge.c
index 1cb5c16e97b7..09da1f4459c9 100644
--- a/net/bridge/netfilter/nft_reject_bridge.c
+++ b/net/bridge/netfilter/nft_reject_bridge.c
@@ -166,7 +166,7 @@ static void nft_reject_bridge_eval(const struct nft_expr *expr,
break;
}
out:
- regs->verdict.code = NF_DROP;
+ regs->verdict.code = NF_DROP_REASON(pkt->skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
}
static int nft_reject_bridge_validate(const struct nft_ctx *ctx,
diff --git a/net/ipv4/netfilter/nft_reject_ipv4.c b/net/ipv4/netfilter/nft_reject_ipv4.c
index 6cb213bb7256..76d1f89b594a 100644
--- a/net/ipv4/netfilter/nft_reject_ipv4.c
+++ b/net/ipv4/netfilter/nft_reject_ipv4.c
@@ -34,7 +34,7 @@ static void nft_reject_ipv4_eval(const struct nft_expr *expr,
break;
}
- regs->verdict.code = NF_DROP;
+ regs->verdict.code = NF_DROP_REASON(pkt->skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
}
static struct nft_expr_type nft_reject_ipv4_type;
diff --git a/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c b/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c
index be7817fbc024..41f2a7906a5a 100644
--- a/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c
+++ b/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c
@@ -70,7 +70,10 @@ static unsigned int ipv6_defrag(void *priv,
if (err == -EINPROGRESS)
return NF_STOLEN;
- return err == 0 ? NF_ACCEPT : NF_DROP;
+ if (err == 0)
+ return NF_ACCEPT;
+
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, -err);
}
static const struct nf_hook_ops ipv6_defrag_ops[] = {
diff --git a/net/netfilter/nf_nat_proto.c b/net/netfilter/nf_nat_proto.c
index dc450cc81222..7458c8b59ed2 100644
--- a/net/netfilter/nf_nat_proto.c
+++ b/net/netfilter/nf_nat_proto.c
@@ -439,7 +439,7 @@ unsigned int nf_nat_manip_pkt(struct sk_buff *skb, struct nf_conn *ct,
break;
}
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
}
static void nf_nat_ipv4_csum_update(struct sk_buff *skb,
@@ -636,7 +636,7 @@ nf_nat_ipv4_fn(void *priv, struct sk_buff *skb,
if (ip_hdr(skb)->protocol == IPPROTO_ICMP) {
if (!nf_nat_icmp_reply_translation(skb, ct, ctinfo,
state->hook))
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
else
return NF_ACCEPT;
}
@@ -781,7 +781,7 @@ nf_nat_ipv4_out(void *priv, struct sk_buff *skb,
ct->tuplehash[!dir].tuple.dst.u.all)) {
err = nf_xfrm_me_harder(state->net, skb, AF_INET);
if (err < 0)
- ret = NF_DROP_ERR(err);
+ ret = NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, -err);
}
}
#endif
@@ -809,7 +809,7 @@ nf_nat_ipv4_local_fn(void *priv, struct sk_buff *skb,
ct->tuplehash[!dir].tuple.src.u3.ip) {
err = ip_route_me_harder(state->net, state->sk, skb, RTN_UNSPEC);
if (err < 0)
- ret = NF_DROP_ERR(err);
+ ret = NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, -err);
}
#ifdef CONFIG_XFRM
else if (!(IPCB(skb)->flags & IPSKB_XFRM_TRANSFORMED) &&
@@ -818,7 +818,7 @@ nf_nat_ipv4_local_fn(void *priv, struct sk_buff *skb,
ct->tuplehash[!dir].tuple.src.u.all) {
err = nf_xfrm_me_harder(state->net, skb, AF_INET);
if (err < 0)
- ret = NF_DROP_ERR(err);
+ ret = NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, -err);
}
#endif
}
@@ -965,7 +965,7 @@ nf_nat_ipv6_fn(void *priv, struct sk_buff *skb,
if (!nf_nat_icmpv6_reply_translation(skb, ct, ctinfo,
state->hook,
hdrlen))
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
else
return NF_ACCEPT;
}
@@ -1040,7 +1040,7 @@ nf_nat_ipv6_out(void *priv, struct sk_buff *skb,
ct->tuplehash[!dir].tuple.dst.u.all)) {
err = nf_xfrm_me_harder(state->net, skb, AF_INET6);
if (err < 0)
- ret = NF_DROP_ERR(err);
+ ret = NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, -err);
}
}
#endif
@@ -1069,7 +1069,7 @@ nf_nat_ipv6_local_fn(void *priv, struct sk_buff *skb,
&ct->tuplehash[!dir].tuple.src.u3)) {
err = nf_ip6_route_me_harder(state->net, state->sk, skb);
if (err < 0)
- ret = NF_DROP_ERR(err);
+ ret = NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, -err);
}
#ifdef CONFIG_XFRM
else if (!(IP6CB(skb)->flags & IP6SKB_XFRM_TRANSFORMED) &&
@@ -1078,7 +1078,7 @@ nf_nat_ipv6_local_fn(void *priv, struct sk_buff *skb,
ct->tuplehash[!dir].tuple.src.u.all) {
err = nf_xfrm_me_harder(state->net, skb, AF_INET6);
if (err < 0)
- ret = NF_DROP_ERR(err);
+ ret = NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, -err);
}
#endif
}
diff --git a/net/netfilter/nf_nat_redirect.c b/net/netfilter/nf_nat_redirect.c
index 5b37487d9d11..5adb648538f0 100644
--- a/net/netfilter/nf_nat_redirect.c
+++ b/net/netfilter/nf_nat_redirect.c
@@ -71,7 +71,7 @@ nf_nat_redirect_ipv4(struct sk_buff *skb, const struct nf_nat_range2 *range,
}
if (!newdst.ip)
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
}
return nf_nat_redirect(skb, range, &newdst);
@@ -130,7 +130,7 @@ nf_nat_redirect_ipv6(struct sk_buff *skb, const struct nf_nat_range2 *range,
}
if (!addr)
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
}
return nf_nat_redirect(skb, range, &newdst);
diff --git a/net/netfilter/nf_synproxy_core.c b/net/netfilter/nf_synproxy_core.c
index 5b140c12b7df..08a650fc6ee7 100644
--- a/net/netfilter/nf_synproxy_core.c
+++ b/net/netfilter/nf_synproxy_core.c
@@ -664,7 +664,7 @@ ipv4_synproxy_hook(void *priv, struct sk_buff *skb,
thoff = ip_hdrlen(skb);
th = skb_header_pointer(skb, thoff, sizeof(_th), &_th);
if (!th)
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
state = &ct->proto.tcp;
switch (state->state) {
@@ -689,7 +689,7 @@ ipv4_synproxy_hook(void *priv, struct sk_buff *skb,
fallthrough;
case TCP_CONNTRACK_SYN_SENT:
if (!synproxy_parse_options(skb, thoff, th, &opts))
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
if (!th->syn && th->ack &&
CTINFO2DIR(ctinfo) == IP_CT_DIR_ORIGINAL) {
@@ -703,7 +703,7 @@ ipv4_synproxy_hook(void *priv, struct sk_buff *skb,
consume_skb(skb);
return NF_STOLEN;
} else {
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
}
}
@@ -718,7 +718,7 @@ ipv4_synproxy_hook(void *priv, struct sk_buff *skb,
break;
if (!synproxy_parse_options(skb, thoff, th, &opts))
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
if (opts.options & NF_SYNPROXY_OPT_TIMESTAMP) {
synproxy->tsoff = opts.tsval - synproxy->its;
@@ -1087,7 +1087,7 @@ ipv6_synproxy_hook(void *priv, struct sk_buff *skb,
th = skb_header_pointer(skb, thoff, sizeof(_th), &_th);
if (!th)
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
state = &ct->proto.tcp;
switch (state->state) {
@@ -1112,7 +1112,7 @@ ipv6_synproxy_hook(void *priv, struct sk_buff *skb,
fallthrough;
case TCP_CONNTRACK_SYN_SENT:
if (!synproxy_parse_options(skb, thoff, th, &opts))
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
if (!th->syn && th->ack &&
CTINFO2DIR(ctinfo) == IP_CT_DIR_ORIGINAL) {
@@ -1126,7 +1126,7 @@ ipv6_synproxy_hook(void *priv, struct sk_buff *skb,
consume_skb(skb);
return NF_STOLEN;
} else {
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
}
}
@@ -1141,7 +1141,7 @@ ipv6_synproxy_hook(void *priv, struct sk_buff *skb,
break;
if (!synproxy_parse_options(skb, thoff, th, &opts))
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
if (opts.options & NF_SYNPROXY_OPT_TIMESTAMP) {
synproxy->tsoff = opts.tsval - synproxy->its;
diff --git a/net/netfilter/nft_chain_filter.c b/net/netfilter/nft_chain_filter.c
index 7010541fcca6..b0a90b49ada9 100644
--- a/net/netfilter/nft_chain_filter.c
+++ b/net/netfilter/nft_chain_filter.c
@@ -175,7 +175,7 @@ static unsigned int nft_do_chain_inet_ingress(void *priv, struct sk_buff *skb,
nft_set_pktinfo(&pkt, skb, &ingress_state);
if (nft_set_pktinfo_ipv4_ingress(&pkt) < 0)
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
break;
case htons(ETH_P_IPV6):
ingress_state.pf = NFPROTO_IPV6;
@@ -183,7 +183,7 @@ static unsigned int nft_do_chain_inet_ingress(void *priv, struct sk_buff *skb,
nft_set_pktinfo(&pkt, skb, &ingress_state);
if (nft_set_pktinfo_ipv6_ingress(&pkt) < 0)
- return NF_DROP;
+ return NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
break;
default:
return NF_ACCEPT;
diff --git a/net/netfilter/nft_compat.c b/net/netfilter/nft_compat.c
index 52cdfee17f73..0ee10c7f7a4f 100644
--- a/net/netfilter/nft_compat.c
+++ b/net/netfilter/nft_compat.c
@@ -85,7 +85,7 @@ static void nft_target_eval_xt(const struct nft_expr *expr,
ret = target->target(skb, &xt);
if (xt.hotdrop)
- ret = NF_DROP;
+ ret = NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
switch (ret) {
case XT_CONTINUE:
@@ -112,14 +112,14 @@ static void nft_target_eval_bridge(const struct nft_expr *expr,
ret = target->target(skb, &xt);
if (xt.hotdrop)
- ret = NF_DROP;
+ ret = NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
switch (ret) {
case EBT_ACCEPT:
regs->verdict.code = NF_ACCEPT;
break;
case EBT_DROP:
- regs->verdict.code = NF_DROP;
+ regs->verdict.code = NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
break;
case EBT_CONTINUE:
regs->verdict.code = NFT_CONTINUE;
@@ -403,7 +403,7 @@ static void __nft_match_eval(const struct nft_expr *expr,
ret = match->match(skb, &xt);
if (xt.hotdrop) {
- regs->verdict.code = NF_DROP;
+ regs->verdict.code = NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
return;
}
diff --git a/net/netfilter/nft_connlimit.c b/net/netfilter/nft_connlimit.c
index 92b984fa8175..3cfa9295001a 100644
--- a/net/netfilter/nft_connlimit.c
+++ b/net/netfilter/nft_connlimit.c
@@ -39,12 +39,12 @@ static inline void nft_connlimit_do_eval(struct nft_connlimit *priv,
zone = nf_ct_zone(ct);
} else if (!nf_ct_get_tuplepr(pkt->skb, skb_network_offset(pkt->skb),
nft_pf(pkt), nft_net(pkt), &tuple)) {
- regs->verdict.code = NF_DROP;
+ regs->verdict.code = NF_DROP_REASON(pkt->skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
return;
}
if (nf_conncount_add(nft_net(pkt), priv->list, tuple_ptr, zone)) {
- regs->verdict.code = NF_DROP;
+ regs->verdict.code = NF_DROP_REASON(pkt->skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
return;
}
diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c
index 67a41cd2baaf..d705077ccaaa 100644
--- a/net/netfilter/nft_ct.c
+++ b/net/netfilter/nft_ct.c
@@ -259,7 +259,7 @@ static void nft_ct_set_zone_eval(const struct nft_expr *expr,
*/
ct = nf_ct_tmpl_alloc(nft_net(pkt), &zone, GFP_ATOMIC);
if (!ct) {
- regs->verdict.code = NF_DROP;
+ regs->verdict.code = NF_DROP_REASON(skb, SKB_DROP_REASON_NOMEM, ENOMEM);
return;
}
__set_bit(IPS_CONFIRMED_BIT, &ct->status);
@@ -917,7 +917,7 @@ static void nft_ct_timeout_obj_eval(struct nft_object *obj,
if (!timeout) {
timeout = nf_ct_timeout_ext_add(ct, priv->timeout, GFP_ATOMIC);
if (!timeout) {
- regs->verdict.code = NF_DROP;
+ regs->verdict.code = NF_DROP_REASON(pkt->skb, SKB_DROP_REASON_NOMEM, ENOMEM);
return;
}
}
@@ -1316,6 +1316,7 @@ static void nft_ct_expect_obj_eval(struct nft_object *obj,
enum ip_conntrack_dir dir;
u16 l3num = priv->l3num;
struct nf_conn *ct;
+ int err;
ct = nf_ct_get(pkt->skb, &ctinfo);
if (!ct || nf_ct_is_confirmed(ct) || nf_ct_is_template(ct)) {
@@ -1328,7 +1329,7 @@ static void nft_ct_expect_obj_eval(struct nft_object *obj,
if (!help)
help = nf_ct_helper_ext_add(ct, GFP_ATOMIC);
if (!help) {
- regs->verdict.code = NF_DROP;
+ regs->verdict.code = NF_DROP_REASON(pkt->skb, SKB_DROP_REASON_NOMEM, ENOMEM);
return;
}
@@ -1341,7 +1342,7 @@ static void nft_ct_expect_obj_eval(struct nft_object *obj,
exp = nf_ct_expect_alloc(ct);
if (exp == NULL) {
- regs->verdict.code = NF_DROP;
+ regs->verdict.code = NF_DROP_REASON(pkt->skb, SKB_DROP_REASON_NOMEM, ENOMEM);
return;
}
nf_ct_expect_init(exp, NF_CT_EXPECT_CLASS_DEFAULT, l3num,
@@ -1350,8 +1351,9 @@ static void nft_ct_expect_obj_eval(struct nft_object *obj,
priv->l4proto, NULL, &priv->dport);
exp->timeout.expires = jiffies + priv->timeout * HZ;
- if (nf_ct_expect_related(exp, 0) != 0)
- regs->verdict.code = NF_DROP;
+ err = nf_ct_expect_related(exp, 0);
+ if (err != 0)
+ regs->verdict.code = NF_DROP_REASON(pkt->skb, SKB_DROP_REASON_NETFILTER_DROP, -err);
}
static const struct nla_policy nft_ct_expect_policy[NFTA_CT_EXPECT_MAX + 1] = {
diff --git a/net/netfilter/nft_exthdr.c b/net/netfilter/nft_exthdr.c
index 6bfd33516241..9e7fedcb2c16 100644
--- a/net/netfilter/nft_exthdr.c
+++ b/net/netfilter/nft_exthdr.c
@@ -365,7 +365,7 @@ static void nft_exthdr_tcp_strip_eval(const struct nft_expr *expr,
return;
drop:
/* can't remove, no choice but to drop */
- regs->verdict.code = NF_DROP;
+ regs->verdict.code = NF_DROP_REASON(pkt->skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
}
static void nft_exthdr_sctp_eval(const struct nft_expr *expr,
diff --git a/net/netfilter/nft_fib_inet.c b/net/netfilter/nft_fib_inet.c
index 666a3741d20b..a15f40729201 100644
--- a/net/netfilter/nft_fib_inet.c
+++ b/net/netfilter/nft_fib_inet.c
@@ -38,7 +38,7 @@ static void nft_fib_inet_eval(const struct nft_expr *expr,
break;
}
- regs->verdict.code = NF_DROP;
+ regs->verdict.code = NF_DROP_REASON(pkt->skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
}
static struct nft_expr_type nft_fib_inet_type;
diff --git a/net/netfilter/nft_fwd_netdev.c b/net/netfilter/nft_fwd_netdev.c
index 152a9fb4d23a..ecf48b5e8de1 100644
--- a/net/netfilter/nft_fwd_netdev.c
+++ b/net/netfilter/nft_fwd_netdev.c
@@ -112,7 +112,7 @@ static void nft_fwd_neigh_eval(const struct nft_expr *expr,
goto out;
}
if (skb_try_make_writable(skb, sizeof(*iph))) {
- verdict = NF_DROP;
+ verdict = NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
goto out;
}
iph = ip_hdr(skb);
@@ -128,7 +128,7 @@ static void nft_fwd_neigh_eval(const struct nft_expr *expr,
goto out;
}
if (skb_try_make_writable(skb, sizeof(*ip6h))) {
- verdict = NF_DROP;
+ verdict = NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
goto out;
}
ip6h = ipv6_hdr(skb);
diff --git a/net/netfilter/nft_reject_inet.c b/net/netfilter/nft_reject_inet.c
index 49020e67304a..6ba969e94afa 100644
--- a/net/netfilter/nft_reject_inet.c
+++ b/net/netfilter/nft_reject_inet.c
@@ -57,7 +57,7 @@ static void nft_reject_inet_eval(const struct nft_expr *expr,
break;
}
- regs->verdict.code = NF_DROP;
+ regs->verdict.code = NF_DROP_REASON(pkt->skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
}
static int nft_reject_inet_validate(const struct nft_ctx *ctx,
diff --git a/net/netfilter/nft_reject_netdev.c b/net/netfilter/nft_reject_netdev.c
index 2558ce1505d9..ba39bf9274b1 100644
--- a/net/netfilter/nft_reject_netdev.c
+++ b/net/netfilter/nft_reject_netdev.c
@@ -141,7 +141,7 @@ static void nft_reject_netdev_eval(const struct nft_expr *expr,
break;
}
out:
- regs->verdict.code = NF_DROP;
+ regs->verdict.code = NF_DROP_REASON(pkt->skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
}
static int nft_reject_netdev_validate(const struct nft_ctx *ctx,
diff --git a/net/netfilter/nft_synproxy.c b/net/netfilter/nft_synproxy.c
index 5d3e51825985..fe96f6fc4d3c 100644
--- a/net/netfilter/nft_synproxy.c
+++ b/net/netfilter/nft_synproxy.c
@@ -66,7 +66,7 @@ static void nft_synproxy_eval_v4(const struct nft_synproxy *priv,
consume_skb(skb);
regs->verdict.code = NF_STOLEN;
} else {
- regs->verdict.code = NF_DROP;
+ regs->verdict.code = NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
}
}
}
@@ -97,7 +97,7 @@ static void nft_synproxy_eval_v6(const struct nft_synproxy *priv,
consume_skb(skb);
regs->verdict.code = NF_STOLEN;
} else {
- regs->verdict.code = NF_DROP;
+ regs->verdict.code = NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
}
}
}
@@ -119,7 +119,7 @@ static void nft_synproxy_do_eval(const struct nft_synproxy *priv,
}
if (nf_ip_checksum(skb, nft_hook(pkt), thoff, IPPROTO_TCP)) {
- regs->verdict.code = NF_DROP;
+ regs->verdict.code = NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
return;
}
@@ -127,12 +127,12 @@ static void nft_synproxy_do_eval(const struct nft_synproxy *priv,
sizeof(struct tcphdr),
&_tcph);
if (!tcp) {
- regs->verdict.code = NF_DROP;
+ regs->verdict.code = NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
return;
}
if (!synproxy_parse_options(skb, thoff, tcp, &opts)) {
- regs->verdict.code = NF_DROP;
+ regs->verdict.code = NF_DROP_REASON(skb, SKB_DROP_REASON_NETFILTER_DROP, EPERM);
return;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH nf-next 0/4] netfilter: use skb_drop_reason in more places
2024-10-02 15:55 [PATCH nf-next 0/4] netfilter: use skb_drop_reason in more places Florian Westphal
` (3 preceding siblings ...)
2024-10-02 15:55 ` [PATCH nf-next 4/4] netfilter: nf_tables: " Florian Westphal
@ 2024-10-12 14:09 ` Pablo Neira Ayuso
2024-10-12 14:42 ` Florian Westphal
4 siblings, 1 reply; 11+ messages in thread
From: Pablo Neira Ayuso @ 2024-10-12 14:09 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel
Hi Florian,
On Wed, Oct 02, 2024 at 05:55:38PM +0200, Florian Westphal wrote:
> Provide more precise drop information rather than doing the freeing
> from core.c:nf_hook_slow().
>
> First patch is a small preparation patch, rest coverts NF_DROP
> locations of NF_DROP_REASON().
One question regarding this series.
Most spots still rely on EPERM which is the default reason for
NF_DROP.
I wonder if it is worth updating all these spots to use NF_DROP_REASON
with EPERM. I think patchset becomes smaller if it is only used to
provide a better reason than EPERM.
Thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH nf-next 0/4] netfilter: use skb_drop_reason in more places
2024-10-12 14:09 ` [PATCH nf-next 0/4] netfilter: use skb_drop_reason in more places Pablo Neira Ayuso
@ 2024-10-12 14:42 ` Florian Westphal
2024-10-12 15:42 ` Pablo Neira Ayuso
0 siblings, 1 reply; 11+ messages in thread
From: Florian Westphal @ 2024-10-12 14:42 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: Florian Westphal, netfilter-devel
Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> One question regarding this series.
>
> Most spots still rely on EPERM which is the default reason for
> NF_DROP.
core converts NF_DROP to EPERM if no errno value is set, correct.
> I wonder if it is worth updating all these spots to use NF_DROP_REASON
> with EPERM. I think patchset becomes smaller if it is only used to
> provide a better reason than EPERM.
I'm not following, sorry. What do you mean?
This is not about errno. NF_DROP_REASON() calls kfree_skb, so tooling
can show location other than nf_hook_slow().
Or do you mean using a different macro that always sets EPERM?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH nf-next 0/4] netfilter: use skb_drop_reason in more places
2024-10-12 14:42 ` Florian Westphal
@ 2024-10-12 15:42 ` Pablo Neira Ayuso
2024-10-12 15:54 ` Florian Westphal
0 siblings, 1 reply; 11+ messages in thread
From: Pablo Neira Ayuso @ 2024-10-12 15:42 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel
On Sat, Oct 12, 2024 at 04:42:16PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > One question regarding this series.
> >
> > Most spots still rely on EPERM which is the default reason for
> > NF_DROP.
>
> core converts NF_DROP to EPERM if no errno value is set, correct.
>
> > I wonder if it is worth updating all these spots to use NF_DROP_REASON
> > with EPERM. I think patchset becomes smaller if it is only used to
> > provide a better reason than EPERM.
>
> I'm not following, sorry. What do you mean?
>
> This is not about errno. NF_DROP_REASON() calls kfree_skb, so tooling
> can show location other than nf_hook_slow().
Right.
> Or do you mean using a different macro that always sets EPERM?
Maybe remove SKB_DROP_REASON_NETFILTER_DROP from macro, so line is
shorter?
NF_DROP_REASON(pkt->skb, -EPERM)
And add a new macro for br_netfilter NF_BR_DROP_REASON which does not
always sets SKB_DROP_REASON_NETFILTER_DROP? (Pick a better name for
this new macro if you like).
Or you think the existing generic long version of NF_DROP_REASON is
convenient to have?
Thanks
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH nf-next 0/4] netfilter: use skb_drop_reason in more places
2024-10-12 15:42 ` Pablo Neira Ayuso
@ 2024-10-12 15:54 ` Florian Westphal
2024-10-12 16:45 ` Pablo Neira Ayuso
0 siblings, 1 reply; 11+ messages in thread
From: Florian Westphal @ 2024-10-12 15:54 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: Florian Westphal, netfilter-devel
Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > Or do you mean using a different macro that always sets EPERM?
>
> Maybe remove SKB_DROP_REASON_NETFILTER_DROP from macro, so line is
> shorter?
>
> NF_DROP_REASON(pkt->skb, -EPERM)
>
> And add a new macro for br_netfilter NF_BR_DROP_REASON which does not
> always sets SKB_DROP_REASON_NETFILTER_DROP? (Pick a better name for
> this new macro if you like).
NF_DROP_REASON is already in the tree and currently most users use
something other than SKB_DROP_REASON_NETFILTER_DROP.
I did not yet add new enum values or a dedicated nf namespace
(enum skb_drop_reason_subsys), because I did not see a reason and
wasn't sure if we'd need sub-subsystems (nf_tables, conntrack, nat,
whatever).
If you like, I can add NF_FREE_SKB(skb, errno) and rework this
set to use that?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH nf-next 0/4] netfilter: use skb_drop_reason in more places
2024-10-12 15:54 ` Florian Westphal
@ 2024-10-12 16:45 ` Pablo Neira Ayuso
2024-10-12 20:38 ` Florian Westphal
0 siblings, 1 reply; 11+ messages in thread
From: Pablo Neira Ayuso @ 2024-10-12 16:45 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel
On Sat, Oct 12, 2024 at 05:54:48PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > Or do you mean using a different macro that always sets EPERM?
> >
> > Maybe remove SKB_DROP_REASON_NETFILTER_DROP from macro, so line is
> > shorter?
> >
> > NF_DROP_REASON(pkt->skb, -EPERM)
> >
> > And add a new macro for br_netfilter NF_BR_DROP_REASON which does not
> > always sets SKB_DROP_REASON_NETFILTER_DROP? (Pick a better name for
> > this new macro if you like).
>
> NF_DROP_REASON is already in the tree and currently most users use
> something other than SKB_DROP_REASON_NETFILTER_DROP.
>
> I did not yet add new enum values or a dedicated nf namespace
> (enum skb_drop_reason_subsys), because I did not see a reason and
> wasn't sure if we'd need sub-subsystems (nf_tables, conntrack, nat,
> whatever).
Does this mean values exposed through tracing infrastructure can
change or these are part of uapi? From what I read from you, I
understand it is possible to change SKB_DROP_REASON_NETFILTER_DROP to
a more specific sub-subsystem tag in the future without issues.
> If you like, I can add NF_FREE_SKB(skb, errno) and rework this
> set to use that?
Not strong about this. I was exploring if it should be possible to
remove (repetitive) information in the code that can be assumed to be
implicit, I still like the word "REASON" in the macro for grepping.
I think we can just move on with this series as-is if you prefer and
add new macros incrementally to refine.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH nf-next 0/4] netfilter: use skb_drop_reason in more places
2024-10-12 16:45 ` Pablo Neira Ayuso
@ 2024-10-12 20:38 ` Florian Westphal
0 siblings, 0 replies; 11+ messages in thread
From: Florian Westphal @ 2024-10-12 20:38 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: Florian Westphal, netfilter-devel
Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > I did not yet add new enum values or a dedicated nf namespace
> > (enum skb_drop_reason_subsys), because I did not see a reason and
> > wasn't sure if we'd need sub-subsystems (nf_tables, conntrack, nat,
> > whatever).
>
> Does this mean values exposed through tracing infrastructure can
> change or these are part of uapi?
The enum has had values added (not just appended), so its not considered
uapi.
> From what I read from you, I
> understand it is possible to change SKB_DROP_REASON_NETFILTER_DROP to
> a more specific sub-subsystem tag in the future without issues.
Thats correct.
> > If you like, I can add NF_FREE_SKB(skb, errno) and rework this
> > set to use that?
>
> Not strong about this. I was exploring if it should be possible to
> remove (repetitive) information in the code that can be assumed to be
> implicit, I still like the word "REASON" in the macro for grepping.
Yes, common name is good.
> I think we can just move on with this series as-is if you prefer and
> add new macros incrementally to refine.
I think we can refine later if there are use-cases for that.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-10-12 20:38 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-02 15:55 [PATCH nf-next 0/4] netfilter: use skb_drop_reason in more places Florian Westphal
2024-10-02 15:55 ` [PATCH nf-next 1/4] netfilter: xt_nat: compact nf_nat_setup_info calls Florian Westphal
2024-10-02 15:55 ` [PATCH nf-next 2/4] netfilter: xt_nat: drop packet earlier Florian Westphal
2024-10-02 15:55 ` [PATCH nf-next 3/4] netfilter: nf_nat: use skb_drop_reason Florian Westphal
2024-10-02 15:55 ` [PATCH nf-next 4/4] netfilter: nf_tables: " Florian Westphal
2024-10-12 14:09 ` [PATCH nf-next 0/4] netfilter: use skb_drop_reason in more places Pablo Neira Ayuso
2024-10-12 14:42 ` Florian Westphal
2024-10-12 15:42 ` Pablo Neira Ayuso
2024-10-12 15:54 ` Florian Westphal
2024-10-12 16:45 ` Pablo Neira Ayuso
2024-10-12 20:38 ` Florian Westphal
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).