| Message ID | 20240213134205.8705-2-kerneljasonxing@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | introduce drop reasons for cookie check | expand |
On Tue, Feb 13, 2024 at 2:42 PM Jason Xing <kerneljasonxing@gmail.com> wrote: > > From: Jason Xing <kernelxing@tencent.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@tencent.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.
On Tue, Feb 13, 2024 at 11:24 PM Eric Dumazet <edumazet@google.com> wrote: > > On Tue, Feb 13, 2024 at 2:42 PM Jason Xing <kerneljasonxing@gmail.com> wrote: > > > > From: Jason Xing <kernelxing@tencent.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@tencent.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 ? Oh right, I will reuse it. > > > 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. You're right about this. > > > + 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. Let me think about a proper new name. > > Either the reason is deterministic, or just reuse a generic and > existing drop_reason that can be augmented later. I learned that. Thanks, Jason > > You are adding weak or duplicate drop_reasons, we already have them.
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) \ 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 + */ + 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 + */ + SKB_DROP_REASON_COOKIE_NOCHILD, /** * @SKB_DROP_REASON_TCP_ACK_UNSENT_DATA: TCP ACK for data we haven't * sent yet @@ -254,6 +269,8 @@ enum skb_drop_reason { SKB_DROP_REASON_TCP_OFO_DROP, /** @SKB_DROP_REASON_IP_OUTNOROUTES: route lookup failed */ SKB_DROP_REASON_IP_OUTNOROUTES, + /** @SKB_DROP_REASON_IP_ROUTEOUTPUTKEY: route output key failed */ + SKB_DROP_REASON_IP_ROUTEOUTPUTKEY, /** * @SKB_DROP_REASON_BPF_CGROUP_EGRESS: dropped by BPF_PROG_TYPE_CGROUP_SKB * eBPF program @@ -271,6 +288,8 @@ enum skb_drop_reason { SKB_DROP_REASON_NEIGH_DEAD, /** @SKB_DROP_REASON_TC_EGRESS: dropped in TC egress HOOK */ SKB_DROP_REASON_TC_EGRESS, + /** @SKB_DROP_REASON_SECURITY_HOOK: dropped due to security HOOK */ + SKB_DROP_REASON_SECURITY_HOOK, /** * @SKB_DROP_REASON_QDISC_DROP: dropped by qdisc when packet outputting ( * failed to enqueue to current qdisc) @@ -333,6 +352,8 @@ enum skb_drop_reason { * such as a broadcasts ICMP_TIMESTAMP */ SKB_DROP_REASON_INVALID_PROTO, + /** @SKB_DROP_REASON_INVALID_DST: look-up dst entry error */ + SKB_DROP_REASON_INVALID_DST, /** * @SKB_DROP_REASON_IP_INADDRERRORS: host unreachable, corresponding to * IPSTATS_MIB_INADDRERRORS