* [PATCH 0/1] Issue description and debug
@ 2023-10-08 0:59 Heng Guo
2023-10-08 0:59 ` [PATCH 1/1] net-next: fix IPSTATS_MIB_OUTFORWDATAGRAMS increment after fragment check Heng Guo
0 siblings, 1 reply; 6+ messages in thread
From: Heng Guo @ 2023-10-08 0:59 UTC (permalink / raw)
To: davem, dsahern, edumazet, kuba, pabeni; +Cc: netdev, filip.pudak, heng.guo
Hi maintainers,
The IPSTATS_MIB_OUTFORWDATAGRAMS is counted after fragment check.
Reproduce environment:
network with 3 VM linuxs is connected as below:
VM1<---->VM2(latest kernel 6.5.0-rc7)<---->VM3
VM1: eth0 ip: 192.168.122.207 MTU 1800
VM2: eth0 ip: 192.168.122.208, eth1 ip: 192.168.123.224 MTU 1500
VM3: eth0 ip: 192.168.123.240 MTU 1800
Reproduce:
VM1 send 1600 bytes UDP data to VM3 using tools scapy with flags='DF'.
scapy command:
send(IP(dst="192.168.123.240",flags='DF')/UDP()/str('0'*1600),count=1,
inter=1.000000)
Result:
Before IP data is sent.
----------------------------------------------------------------------
root@qemux86-64:~# cat /proc/net/snmp
Ip: Forwarding DefaultTTL InReceives InHdrErrors InAddrErrors
ForwDatagrams InUnknownProtos InDiscards InDelivers OutRequests
OutDiscards OutNoRoutes ReasmTimeout ReasmReqdss
Ip: 1 64 6 0 2 2 0 0 2 4 0 0 0 0 0 0 0 0 0
......
root@qemux86-64:~#
----------------------------------------------------------------------
After IP data is sent.
----------------------------------------------------------------------
root@qemux86-64:~# cat /proc/net/snmp
Ip: Forwarding DefaultTTL InReceives InHdrErrors InAddrErrors
ForwDatagrams InUnknownProtos InDiscards InDelivers OutRequests
OutDiscards OutNoRoutes ReasmTimeout ReasmReqdss
Ip: 1 64 7 0 2 2 0 0 2 5 0 0 0 0 0 0 0 1 0
......
root@qemux86-64:~#
----------------------------------------------------------------------
ForwDatagrams is always keeping 2 without increment.
Issue description and patch:
ip_exceeds_mtu() in ip_forward() drops this IP datagram because skb len
(1600 sending by scapy) is over MTU(1500 in VM2) and "DF" is set.
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 do patch to move IPSTATS_MIB_OUTFORWDATAGRAMS counter to ip_forward()
for ipv4 and ip6_forward() for ipv6.
Test result with patch:
Before IP data is sent.
----------------------------------------------------------------------
root@qemux86-64:~# cat /proc/net/snmp
Ip: Forwarding DefaultTTL InReceives InHdrErrors InAddrErrors
ForwDatagrams InUnknownProtos InDiscards InDelivers OutRequests
OutDiscards OutNoRoutes ReasmTimeout ReasmReqdss
Ip: 1 64 6 0 2 2 0 0 2 4 0 0 0 0 0 0 0 0 0
......
root@qemux86-64:~#
----------------------------------------------------------------------
After IP data is sent.
----------------------------------------------------------------------
root@qemux86-64:~# cat /proc/net/snmp
Ip: Forwarding DefaultTTL InReceives InHdrErrors InAddrErrors
ForwDatagrams InUnknownProtos InDiscards InDelivers OutRequests
OutDiscards OutNoRoutes ReasmTimeout ReasmReqdss
Ip: 1 64 7 0 2 3 0 0 2 5 0 0 0 0 0 0 0 1 0
......
root@qemux86-64:~#
----------------------------------------------------------------------
ForwDatagrams is updated from 2 to 3.
Thanks,
Heng
This is a new email thread, last one is:
https://lore.kernel.org/all/20230914051623.2180843-2-heng.guo@windriver.com
Heng Guo (1):
net-next: fix IPSTATS_MIB_OUTFORWDATAGRAMS increment after fragment
check
net/ipv4/ip_forward.c | 4 ++--
net/ipv6/ip6_output.c | 6 ++----
2 files changed, 4 insertions(+), 6 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/1] net-next: fix IPSTATS_MIB_OUTFORWDATAGRAMS increment after fragment check 2023-10-08 0:59 [PATCH 0/1] Issue description and debug Heng Guo @ 2023-10-08 0:59 ` Heng Guo 2023-10-10 16:26 ` David Ahern 2023-10-11 1:51 ` [PATCH] " Heng Guo 0 siblings, 2 replies; 6+ messages in thread From: Heng Guo @ 2023-10-08 0:59 UTC (permalink / raw) To: davem, dsahern, edumazet, kuba, pabeni; +Cc: netdev, filip.pudak, heng.guo 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@windriver.com> Signed-off-by: Heng Guo <heng.guo@windriver.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; -- 2.35.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] net-next: fix IPSTATS_MIB_OUTFORWDATAGRAMS increment after fragment check 2023-10-08 0:59 ` [PATCH 1/1] net-next: fix IPSTATS_MIB_OUTFORWDATAGRAMS increment after fragment check Heng Guo @ 2023-10-10 16:26 ` David Ahern 2023-10-11 1:51 ` [PATCH] " Heng Guo 1 sibling, 0 replies; 6+ messages in thread From: David Ahern @ 2023-10-10 16:26 UTC (permalink / raw) To: Heng Guo, davem, edumazet, kuba, pabeni; +Cc: netdev, filip.pudak On 10/7/23 6:59 PM, 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@windriver.com> > Signed-off-by: Heng Guo <heng.guo@windriver.com> > --- > net/ipv4/ip_forward.c | 4 ++-- > net/ipv6/ip6_output.c | 6 ++---- > 2 files changed, 4 insertions(+), 6 deletions(-) > Reviewed-by: David Ahern <dsahern@kernel.org> Please repost as just a single patch and add in the details from the cover letter. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] net-next: fix IPSTATS_MIB_OUTFORWDATAGRAMS increment after fragment check 2023-10-08 0:59 ` [PATCH 1/1] net-next: fix IPSTATS_MIB_OUTFORWDATAGRAMS increment after fragment check Heng Guo 2023-10-10 16:26 ` David Ahern @ 2023-10-11 1:51 ` Heng Guo 2023-10-13 17:01 ` Jakub Kicinski 2023-10-13 17:10 ` patchwork-bot+netdevbpf 1 sibling, 2 replies; 6+ messages in thread From: Heng Guo @ 2023-10-11 1:51 UTC (permalink / raw) To: davem, dsahern, edumazet, kuba, pabeni; +Cc: netdev, filip.pudak, heng.guo Reproduce environment: network with 3 VM linuxs is connected as below: VM1<---->VM2(latest kernel 6.5.0-rc7)<---->VM3 VM1: eth0 ip: 192.168.122.207 MTU 1800 VM2: eth0 ip: 192.168.122.208, eth1 ip: 192.168.123.224 MTU 1500 VM3: eth0 ip: 192.168.123.240 MTU 1800 Reproduce: VM1 send 1600 bytes UDP data to VM3 using tools scapy with flags='DF'. scapy command: send(IP(dst="192.168.123.240",flags='DF')/UDP()/str('0'*1600),count=1, inter=1.000000) Result: Before IP data is sent. ---------------------------------------------------------------------- root@qemux86-64:~# cat /proc/net/snmp Ip: Forwarding DefaultTTL InReceives InHdrErrors InAddrErrors ForwDatagrams InUnknownProtos InDiscards InDelivers OutRequests OutDiscards OutNoRoutes ReasmTimeout ReasmReqdss Ip: 1 64 6 0 2 2 0 0 2 4 0 0 0 0 0 0 0 0 0 ...... root@qemux86-64:~# ---------------------------------------------------------------------- After IP data is sent. ---------------------------------------------------------------------- root@qemux86-64:~# cat /proc/net/snmp Ip: Forwarding DefaultTTL InReceives InHdrErrors InAddrErrors ForwDatagrams InUnknownProtos InDiscards InDelivers OutRequests OutDiscards OutNoRoutes ReasmTimeout ReasmReqdss Ip: 1 64 7 0 2 2 0 0 2 5 0 0 0 0 0 0 0 1 0 ...... root@qemux86-64:~# ---------------------------------------------------------------------- ForwDatagrams is always keeping 2 without increment. Issue description and patch: ip_exceeds_mtu() in ip_forward() drops this IP datagram because skb len (1600 sending by scapy) is over MTU(1500 in VM2) if "DF" is set. 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 do patch to move IPSTATS_MIB_OUTFORWDATAGRAMS counter to ip_forward() for ipv4 and ip6_forward() for ipv6. Test result with patch: Before IP data is sent. ---------------------------------------------------------------------- root@qemux86-64:~# cat /proc/net/snmp Ip: Forwarding DefaultTTL InReceives InHdrErrors InAddrErrors ForwDatagrams InUnknownProtos InDiscards InDelivers OutRequests OutDiscards OutNoRoutes ReasmTimeout ReasmReqdss Ip: 1 64 6 0 2 2 0 0 2 4 0 0 0 0 0 0 0 0 0 ...... root@qemux86-64:~# ---------------------------------------------------------------------- After IP data is sent. ---------------------------------------------------------------------- root@qemux86-64:~# cat /proc/net/snmp Ip: Forwarding DefaultTTL InReceives InHdrErrors InAddrErrors ForwDatagrams InUnknownProtos InDiscards InDelivers OutRequests OutDiscards OutNoRoutes ReasmTimeout ReasmReqdss Ip: 1 64 7 0 2 3 0 0 2 5 0 0 0 0 0 0 0 1 0 ...... root@qemux86-64:~# ---------------------------------------------------------------------- ForwDatagrams is updated from 2 to 3. Reviewed-by: Filip Pudak <filip.pudak@windriver.com> Signed-off-by: Heng Guo <heng.guo@windriver.com> Reviewed-by: David Ahern <dsahern@kernel.org> --- 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; -- 2.35.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] net-next: fix IPSTATS_MIB_OUTFORWDATAGRAMS increment after fragment check 2023-10-11 1:51 ` [PATCH] " Heng Guo @ 2023-10-13 17:01 ` Jakub Kicinski 2023-10-13 17:10 ` patchwork-bot+netdevbpf 1 sibling, 0 replies; 6+ messages in thread From: Jakub Kicinski @ 2023-10-13 17:01 UTC (permalink / raw) To: Heng Guo; +Cc: davem, dsahern, edumazet, pabeni, netdev, filip.pudak On Wed, 11 Oct 2023 09:51:37 +0800 Heng Guo wrote: > Subject: [PATCH] net-next: fix IPSTATS_MIB_OUTFORWDATAGRAMS increment after fragment check Applied, thank you! For future reference - the net-next goes into the prefix (git format-patch --subject-prefix="PATCH net-next") and does not replace the commit prefix. So: [PATCH net-next] net: fix ^ \ net-next here this stays as net ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net-next: fix IPSTATS_MIB_OUTFORWDATAGRAMS increment after fragment check 2023-10-11 1:51 ` [PATCH] " Heng Guo 2023-10-13 17:01 ` Jakub Kicinski @ 2023-10-13 17:10 ` patchwork-bot+netdevbpf 1 sibling, 0 replies; 6+ messages in thread From: patchwork-bot+netdevbpf @ 2023-10-13 17:10 UTC (permalink / raw) To: Heng Guo; +Cc: davem, dsahern, edumazet, kuba, pabeni, netdev, filip.pudak Hello: This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Wed, 11 Oct 2023 09:51:37 +0800 you wrote: > Reproduce environment: > network with 3 VM linuxs is connected as below: > VM1<---->VM2(latest kernel 6.5.0-rc7)<---->VM3 > VM1: eth0 ip: 192.168.122.207 MTU 1800 > VM2: eth0 ip: 192.168.122.208, eth1 ip: 192.168.123.224 MTU 1500 > VM3: eth0 ip: 192.168.123.240 MTU 1800 > > [...] Here is the summary with links: - net-next: fix IPSTATS_MIB_OUTFORWDATAGRAMS increment after fragment check https://git.kernel.org/netdev/net-next/c/cf8b49fbd041 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-10-13 17:10 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-10-08 0:59 [PATCH 0/1] Issue description and debug Heng Guo 2023-10-08 0:59 ` [PATCH 1/1] net-next: fix IPSTATS_MIB_OUTFORWDATAGRAMS increment after fragment check Heng Guo 2023-10-10 16:26 ` David Ahern 2023-10-11 1:51 ` [PATCH] " Heng Guo 2023-10-13 17:01 ` Jakub Kicinski 2023-10-13 17:10 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).