* [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).