Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

icmp: Remove some spurious dropped packet profile hits from the ICMP path

If icmp_rcv() has successfully processed the incoming ICMP datagram, we
should use consume_skb() rather than kfree_skb() because a hit on the likes
of perf -e skb:kfree_skb is not called-for.

Signed-off-by: Rick Jones <rick.jones2@hp.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

authored by

Rick Jones and committed by
David S. Miller
e3e32170 54aeba7f

+42 -21
+1 -1
include/net/ping.h
··· 82 82 int ping_v6_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg, 83 83 size_t len); 84 84 int ping_queue_rcv_skb(struct sock *sk, struct sk_buff *skb); 85 - void ping_rcv(struct sk_buff *skb); 85 + bool ping_rcv(struct sk_buff *skb); 86 86 87 87 #ifdef CONFIG_PROC_FS 88 88 struct ping_seq_afinfo {
+28 -15
net/ipv4/icmp.c
··· 190 190 */ 191 191 192 192 struct icmp_control { 193 - void (*handler)(struct sk_buff *skb); 193 + bool (*handler)(struct sk_buff *skb); 194 194 short error; /* This ICMP is classed as an error message */ 195 195 }; 196 196 ··· 746 746 * ICMP_PARAMETERPROB. 747 747 */ 748 748 749 - static void icmp_unreach(struct sk_buff *skb) 749 + static bool icmp_unreach(struct sk_buff *skb) 750 750 { 751 751 const struct iphdr *iph; 752 752 struct icmphdr *icmph; ··· 839 839 icmp_socket_deliver(skb, info); 840 840 841 841 out: 842 - return; 842 + return true; 843 843 out_err: 844 844 ICMP_INC_STATS_BH(net, ICMP_MIB_INERRORS); 845 - goto out; 845 + return false; 846 846 } 847 847 848 848 ··· 850 850 * Handle ICMP_REDIRECT. 851 851 */ 852 852 853 - static void icmp_redirect(struct sk_buff *skb) 853 + static bool icmp_redirect(struct sk_buff *skb) 854 854 { 855 855 if (skb->len < sizeof(struct iphdr)) { 856 856 ICMP_INC_STATS_BH(dev_net(skb->dev), ICMP_MIB_INERRORS); 857 - return; 857 + return false; 858 858 } 859 859 860 - if (!pskb_may_pull(skb, sizeof(struct iphdr))) 861 - return; 860 + if (!pskb_may_pull(skb, sizeof(struct iphdr))) { 861 + /* there aught to be a stat */ 862 + return false; 863 + } 862 864 863 865 icmp_socket_deliver(skb, icmp_hdr(skb)->un.gateway); 866 + return true; 864 867 } 865 868 866 869 /* ··· 878 875 * See also WRT handling of options once they are done and working. 879 876 */ 880 877 881 - static void icmp_echo(struct sk_buff *skb) 878 + static bool icmp_echo(struct sk_buff *skb) 882 879 { 883 880 struct net *net; 884 881 ··· 894 891 icmp_param.head_len = sizeof(struct icmphdr); 895 892 icmp_reply(&icmp_param, skb); 896 893 } 894 + /* should there be an ICMP stat for ignored echos? */ 895 + return true; 897 896 } 898 897 899 898 /* ··· 905 900 * MUST be accurate to a few minutes. 906 901 * MUST be updated at least at 15Hz. 907 902 */ 908 - static void icmp_timestamp(struct sk_buff *skb) 903 + static bool icmp_timestamp(struct sk_buff *skb) 909 904 { 910 905 struct timespec tv; 911 906 struct icmp_bxm icmp_param; ··· 932 927 icmp_param.data_len = 0; 933 928 icmp_param.head_len = sizeof(struct icmphdr) + 12; 934 929 icmp_reply(&icmp_param, skb); 935 - out: 936 - return; 930 + return true; 931 + 937 932 out_err: 938 933 ICMP_INC_STATS_BH(dev_net(skb_dst(skb)->dev), ICMP_MIB_INERRORS); 939 - goto out; 934 + return false; 940 935 } 941 936 942 - static void icmp_discard(struct sk_buff *skb) 937 + static bool icmp_discard(struct sk_buff *skb) 943 938 { 939 + /* pretend it was a success */ 940 + return true; 944 941 } 945 942 946 943 /* ··· 953 946 struct icmphdr *icmph; 954 947 struct rtable *rt = skb_rtable(skb); 955 948 struct net *net = dev_net(rt->dst.dev); 949 + bool success; 956 950 957 951 if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb)) { 958 952 struct sec_path *sp = skb_sec_path(skb); ··· 1020 1012 } 1021 1013 } 1022 1014 1023 - icmp_pointers[icmph->type].handler(skb); 1015 + success = icmp_pointers[icmph->type].handler(skb); 1016 + 1017 + if (success) { 1018 + consume_skb(skb); 1019 + return 0; 1020 + } 1024 1021 1025 1022 drop: 1026 1023 kfree_skb(skb);
+3 -3
net/ipv4/ping.c
··· 955 955 * All we need to do is get the socket. 956 956 */ 957 957 958 - void ping_rcv(struct sk_buff *skb) 958 + bool ping_rcv(struct sk_buff *skb) 959 959 { 960 960 struct sock *sk; 961 961 struct net *net = dev_net(skb->dev); ··· 974 974 pr_debug("rcv on socket %p\n", sk); 975 975 ping_queue_rcv_skb(sk, skb_get(skb)); 976 976 sock_put(sk); 977 - return; 977 + return true; 978 978 } 979 979 pr_debug("no socket, dropping\n"); 980 980 981 - /* We're called from icmp_rcv(). kfree_skb() is done there. */ 981 + return false; 982 982 } 983 983 EXPORT_SYMBOL_GPL(ping_rcv); 984 984
+10 -2
net/ipv6/icmp.c
··· 679 679 const struct in6_addr *saddr, *daddr; 680 680 struct icmp6hdr *hdr; 681 681 u8 type; 682 + bool success = false; 682 683 683 684 if (!xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb)) { 684 685 struct sec_path *sp = skb_sec_path(skb); ··· 727 726 break; 728 727 729 728 case ICMPV6_ECHO_REPLY: 730 - ping_rcv(skb); 729 + success = ping_rcv(skb); 731 730 break; 732 731 733 732 case ICMPV6_PKT_TOOBIG: ··· 791 790 icmpv6_notify(skb, type, hdr->icmp6_code, hdr->icmp6_mtu); 792 791 } 793 792 794 - kfree_skb(skb); 793 + /* until the v6 path can be better sorted assume failure and 794 + * preserve the status quo behaviour for the rest of the paths to here 795 + */ 796 + if (success) 797 + consume_skb(skb); 798 + else 799 + kfree_skb(skb); 800 + 795 801 return 0; 796 802 797 803 csum_error: