netdev - Re: [PATCH v3 1/1] net-next: fix IPSTATS_MIB_OUTFORWDATAGRAMS increment after fragment check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <05faefff-181d-3a69-8b74-69381c62cdb7@windriver.com>
Date: Thu, 28 Sep 2023 10:11:59 +0800
From: heng guo <heng.guo@...driver.com>
To: davem@...emloft.net, sahern@...nel.org, edumazet@...gle.com,
        kuba@...nel.org, pabeni@...hat.com
Cc: netdev@...r.kernel.org, filip.pudak@...driver.com
Subject: Re: [PATCH v3 1/1] net-next: fix IPSTATS_MIB_OUTFORWDATAGRAMS
 increment after fragment check

Could you please take time to review this patch?


Thanks a lot,

Heng

On 9/21/23 17:23, Heng Guo wrote:
> According to RFC 4293 "3.2.3. IP Statistics Tables",
>    +-------+------>------+----->-----+----->-----+
>    | InForwDatagrams (6) | OutForwDatagrams (6)  |
>    |                     V                       +->-+ OutFragReqds
>    |                 InNoRoutes                  |   | (packets)
>    / (local packet (3)                           |   |
>    |  IF is that of the address                  |   +--> OutFragFails
>    |  and may not be the receiving IF)           |   |    (packets)
> the IPSTATS_MIB_OUTFORWDATAGRAMS should be counted before fragment
> check.
>
> The existing implementation, instead, would incease the counter after
> fragment check: ip_exceeds_mtu() in ipv4 and ip6_pkt_too_big() in ipv6.
>
> So move IPSTATS_MIB_OUTFORWDATAGRAMS counter to ip_forward() for ipv4 and
> ip6_forward() for ipv6.
>
> Reviewed-by: Filip Pudak <filip.pudak@...driver.com>
> Signed-off-by: Heng Guo <heng.guo@...driver.com>
> ---
>   net/ipv4/ip_forward.c | 4 ++--
>   net/ipv6/ip6_output.c | 6 ++----
>   2 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/net/ipv4/ip_forward.c b/net/ipv4/ip_forward.c
> index 66fac1216d46..8b65f12583eb 100644
> --- a/net/ipv4/ip_forward.c
> +++ b/net/ipv4/ip_forward.c
> @@ -66,8 +66,6 @@ static int ip_forward_finish(struct net *net, struct sock *sk, struct sk_buff *s
>   {
>   	struct ip_options *opt	= &(IPCB(skb)->opt);
>   
> -	__IP_INC_STATS(net, IPSTATS_MIB_OUTFORWDATAGRAMS);
> -
>   #ifdef CONFIG_NET_SWITCHDEV
>   	if (skb->offload_l3_fwd_mark) {
>   		consume_skb(skb);
> @@ -130,6 +128,8 @@ int ip_forward(struct sk_buff *skb)
>   	if (opt->is_strictroute && rt->rt_uses_gateway)
>   		goto sr_failed;
>   
> +	__IP_INC_STATS(net, IPSTATS_MIB_OUTFORWDATAGRAMS);
> +
>   	IPCB(skb)->flags |= IPSKB_FORWARDED;
>   	mtu = ip_dst_mtu_maybe_forward(&rt->dst, true);
>   	if (ip_exceeds_mtu(skb, mtu)) {
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 54fc4c711f2c..8a9199ab97ef 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -448,10 +448,6 @@ static int ip6_forward_proxy_check(struct sk_buff *skb)
>   static inline int ip6_forward_finish(struct net *net, struct sock *sk,
>   				     struct sk_buff *skb)
>   {
> -	struct dst_entry *dst = skb_dst(skb);
> -
> -	__IP6_INC_STATS(net, ip6_dst_idev(dst), IPSTATS_MIB_OUTFORWDATAGRAMS);
> -
>   #ifdef CONFIG_NET_SWITCHDEV
>   	if (skb->offload_l3_fwd_mark) {
>   		consume_skb(skb);
> @@ -619,6 +615,8 @@ int ip6_forward(struct sk_buff *skb)
>   		}
>   	}
>   
> +	__IP6_INC_STATS(net, ip6_dst_idev(dst), IPSTATS_MIB_OUTFORWDATAGRAMS);
> +
>   	mtu = ip6_dst_mtu_maybe_forward(dst, true);
>   	if (mtu < IPV6_MIN_MTU)
>   		mtu = IPV6_MIN_MTU;