mbox series

[net-next,v4,0/4] net: icmp: add skb drop reasons to icmp

Message ID 20220322025220.190568-1-imagedong@tencent.com (mailing list archive)
Headers show
Series net: icmp: add skb drop reasons to icmp | expand

Message

Menglong Dong March 22, 2022, 2:52 a.m. UTC
From: Menglong Dong <imagedong@tencent.com>

In the commit c504e5c2f964 ("net: skb: introduce kfree_skb_reason()"),
we added the support of reporting the reasons of skb drops to kfree_skb
tracepoint. And in this series patches, reasons for skb drops are added
to ICMP protocol.

In order to report the reasons of skb drops in 'sock_queue_rcv_skb()',
the function 'sock_queue_rcv_skb_reason()' is introduced in the 1th
patch, which is used in the 3th patch.

As David Ahern suggested, the reasons for skb drops should be more
general and not be code based. Therefore, in the 2th patch,
SKB_DROP_REASON_PTYPE_ABSENT is renamed to
SKB_DROP_REASON_UNHANDLED_PROTO, which is used for the cases of no
L3 protocol handler, no L4 protocol handler, version extensions, etc.

In the 3th patch, we introduce the new function __ping_queue_rcv_skb()
to report drop reasons by its return value and keep the return value of
ping_queue_rcv_skb() still.

In the 4th patch, we make ICMP message handler functions return drop
reasons, which means we change the return type of 'handler()' in
'struct icmp_control' from 'bool' to 'enum skb_drop_reason'. This
changed its original intention, as 'false' means failure, but
'SKB_NOT_DROPPED_YET', which is 0, means success now. Therefore, we
have to change all usages of these handler. Following "handler"
functions are involved:

icmp_unreach()
icmp_redirect()
icmp_echo()
icmp_timestamp()
icmp_discard()

And following drop reasons are added(what they mean can be see
in the document for them):

SKB_DROP_REASON_ICMP_CSUM
SKB_DROP_REASON_RFC_1122

The reason 'RFC_1122' is introduced for the case that the packet doesn't
follow rfc 1122 and is dropped. Maybe the name 'INVALID_PROTO' is more
suitable? I think this reason is different from the 'UNHANDLED_PROTO',
as the 'UNHANDLED_PROTO' means the packet is fine, and it is just not
supported. This is not a common case, and I believe we can locate the
problem from the data in the packet. For now, this 'RFC_1122' is used
for the icmp broadcasts with wrong types.

Maybe there should be a document file for these reasons. For example,
list all the case that causes the 'RFC_1122' drop reason. Therefore,
users can locate their problems according to the document.

Changes since v3:
- rename SKB_DROP_REASON_PTYPE_ABSENT to SKB_DROP_REASON_UNHANDLED_PROTO
  in the 2th patch
- fix the return value problem of ping_queue_rcv_skb() in the 3th patch
- remove SKB_DROP_REASON_ICMP_TYPE and SKB_DROP_REASON_ICMP_BROADCAST
  and introduce the SKB_DROP_REASON_RFC_1122 in the 4th patch

Changes since v2:
- fix aliegnment problem in the 2th patch

Changes since v1:
- introduce __ping_queue_rcv_skb() instead of change the return value
  of ping_queue_rcv_skb() in the 2th patch, as Paolo suggested

Menglong Dong (4):
  net: sock: introduce sock_queue_rcv_skb_reason()
  net: skb: rename SKB_DROP_REASON_PTYPE_ABSENT
  net: icmp: introduce __ping_queue_rcv_skb() to report drop reasons
  net: icmp: add skb drop reasons to icmp protocol

 include/linux/skbuff.h     | 10 ++---
 include/net/ping.h         |  2 +-
 include/net/sock.h         |  9 ++++-
 include/trace/events/skb.h |  4 +-
 net/core/dev.c             |  8 ++--
 net/core/sock.c            | 30 ++++++++++++---
 net/ipv4/icmp.c            | 75 ++++++++++++++++++++++----------------
 net/ipv4/ping.c            | 32 ++++++++++------
 net/ipv6/icmp.c            | 24 +++++++-----
 9 files changed, 124 insertions(+), 70 deletions(-)