[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANn89i+iELpsoea6+C-08m6+=JkneEEM=nAj-28eNtcOCkwQjw@mail.gmail.com>
Date: Tue, 13 Feb 2024 16:23:53 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: Jason Xing <kerneljasonxing@...il.com>
Cc: davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com,
dsahern@...nel.org, kuniyu@...zon.com, netdev@...r.kernel.org,
Jason Xing <kernelxing@...cent.com>
Subject: Re: [PATCH net-next v4 1/5] tcp: add dropreasons definitions and
prepare for cookie check
On Tue, Feb 13, 2024 at 2:42 PM Jason Xing <kerneljasonxing@...il.com> wrote:
>
> From: Jason Xing <kernelxing@...cent.com>
>
> Only add five drop reasons to detect the condition of skb dropped
> in cookie check for later use.
>
> Signed-off-by: Jason Xing <kernelxing@...cent.com>
> --
> v2
> Link: https://lore.kernel.org/netdev/20240212172302.3f95e454@kernel.org/
> 1. fix misspelled name in kdoc as Jakub said
> ---
> include/net/dropreason-core.h | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/include/net/dropreason-core.h b/include/net/dropreason-core.h
> index 6d3a20163260..065caba42b0b 100644
> --- a/include/net/dropreason-core.h
> +++ b/include/net/dropreason-core.h
> @@ -6,6 +6,7 @@
> #define DEFINE_DROP_REASON(FN, FNe) \
> FN(NOT_SPECIFIED) \
> FN(NO_SOCKET) \
> + FN(NO_REQSK_ALLOC) \
> FN(PKT_TOO_SMALL) \
> FN(TCP_CSUM) \
> FN(SOCKET_FILTER) \
> @@ -43,10 +44,12 @@
> FN(TCP_FASTOPEN) \
> FN(TCP_OLD_ACK) \
> FN(TCP_TOO_OLD_ACK) \
> + FN(COOKIE_NOCHILD) \
> FN(TCP_ACK_UNSENT_DATA) \
> FN(TCP_OFO_QUEUE_PRUNE) \
> FN(TCP_OFO_DROP) \
> FN(IP_OUTNOROUTES) \
> + FN(IP_ROUTEOUTPUTKEY) \
> FN(BPF_CGROUP_EGRESS) \
> FN(IPV6DISABLED) \
> FN(NEIGH_CREATEFAIL) \
> @@ -54,6 +57,7 @@
> FN(NEIGH_QUEUEFULL) \
> FN(NEIGH_DEAD) \
> FN(TC_EGRESS) \
> + FN(SECURITY_HOOK) \
> FN(QDISC_DROP) \
> FN(CPU_BACKLOG) \
> FN(XDP) \
> @@ -71,6 +75,7 @@
> FN(TAP_TXFILTER) \
> FN(ICMP_CSUM) \
> FN(INVALID_PROTO) \
> + FN(INVALID_DST) \
We already have SKB_DROP_REASON_IP_OUTNOROUTES ?
> FN(IP_INADDRERRORS) \
> FN(IP_INNOROUTES) \
> FN(PKT_TOO_BIG) \
> @@ -107,6 +112,11 @@ enum skb_drop_reason {
> SKB_DROP_REASON_NOT_SPECIFIED,
> /** @SKB_DROP_REASON_NO_SOCKET: socket not found */
> SKB_DROP_REASON_NO_SOCKET,
> + /**
> + * @SKB_DROP_REASON_NO_REQSK_ALLOC: request socket allocation failed
> + * probably because of no available memory for now
> + */
We have SKB_DROP_REASON_NOMEM, I do not think we need to be very precise.
REQSK are implementation details.
> + SKB_DROP_REASON_NO_REQSK_ALLOC,
> /** @SKB_DROP_REASON_PKT_TOO_SMALL: packet size is too small */
> SKB_DROP_REASON_PKT_TOO_SMALL,
> /** @SKB_DROP_REASON_TCP_CSUM: TCP checksum error */
> @@ -243,6 +253,11 @@ enum skb_drop_reason {
> SKB_DROP_REASON_TCP_OLD_ACK,
> /** @SKB_DROP_REASON_TCP_TOO_OLD_ACK: TCP ACK is too old */
> SKB_DROP_REASON_TCP_TOO_OLD_ACK,
> + /**
> + * @SKB_DROP_REASON_COOKIE_NOCHILD: no child socket in cookie mode
> + * reason could be the failure of child socket allocation
This makes no sense to me. There are many reasons for this.
Either the reason is deterministic, or just reuse a generic and
existing drop_reason that can be augmented later.
You are adding weak or duplicate drop_reasons, we already have them.
Powered by blists - more mailing lists