Bug 263817 - dwc: dwc driver will re-count the number of sent/received uni-cast mac address packets and multicast mac address packets
Summary: dwc: dwc driver will re-count the number of sent/received uni-cast mac addres...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: arm64 Any
: --- Affects Some People
Assignee: Mitchell Horne
URL: https://reviews.freebsd.org/D35499
Keywords:
Depends on:
Blocks:
 
Reported: 2022-05-06 14:52 UTC by Jiahao LI
Modified: 2022-08-11 14:54 UTC (History)
1 user (show)

See Also:


Attachments
remove duplicate packet counts from if_dwc (1.45 KB, patch)
2022-05-09 22:23 UTC, Mitchell Horne
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jiahao LI 2022-05-06 14:52:19 UTC
Overview
--------
The number of unicast packets that the dwc driver sent/received will be recorded by using the IFCOUNTER_IPACKETS/IFCOUNTER_OPACKETS macros when the tx/rx interrupts are invoked.(See if_dwc.c/dwc_rxfinish_one() and if_dwc.c/dwc_txfinish_locked())


The number of multicast packets will also be recorded at the networking stack using IFCOUNTER_IMCASTS/IFCOUNTER_OMCASTS macros.(For example, /trunk/io-sock/sys/net/if_ethersubr.c/ether_input_internal())


However, the dwc driver will register a callout function, dwc_tick(), which will read the driver's registers, dwc_harvest_stats, to count how many unicast/multicast packets are sent/received. The recalculation issue also exists in Freebsd's current release image.

Actual Results
--------------
Steps for reproducing

1. Set up

In dwc driver's Freebsd terminal

root@generic:~ # uname -a
FreeBSD generic 14.0-CURRENT FreeBSD 14.0-CURRENT #0 main-n254961-b91a48693a5: Thu Apr 21 09:35:51 UTC 2022     root@releng1.nyi.freebsd.org:/usr/obj/usr/src/arm64.aarch64/sys/GENERIC arm64

root@generic:~ # ifconfig dwc0
dwc0: flags=8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> metric 0 mtu 1500
        options=8000b<RXCSUM,TXCSUM,VLAN_MTU,LINKSTATE>
        ether fa:97:92:f6:f1:09
        inet 0.0.0.0 netmask 0xff000000 broadcast 255.255.255.255
        media: Ethernet autoselect (1000baseT <full-duplex>)
        status: active
        nd6 options=29<PERFORMNUD,IFDISABLED,AUTO_LINKLOCAL>

root@generic:~ # ifconfig dwc0 192.168.3.129/24
root@generic:~ # ifconfig dwc0
dwc0: flags=8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> metric 0 mtu 1500
        options=8000b<RXCSUM,TXCSUM,VLAN_MTU,LINKSTATE>
        ether fa:97:92:f6:f1:09
        inet 192.168.3.129 netmask 0xffffff00 broadcast 192.168.3.255
        media: Ethernet autoselect (1000baseT <full-duplex>)
        status: active
        nd6 options=29<PERFORMNUD,IFDISABLED,AUTO_LINKLOCAL>

Host Terminal 1

Check interface config
$ ifconfig enp0s31f6
enp0s31f6: flags=4163<UP,BROADCAST,RUNNING,MULTICAST>  mtu 1500
        inet 192.168.3.2  netmask 255.255.255.0  broadcast 192.168.3.255
        inet6 fe80::f897:92ff:fef6:f102  prefixlen 64  scopeid 0x20<link>
        ether 8c:8c:aa:c1:2b:c3  txqueuelen 1000  (Ethernet)
        RX packets 1662  bytes 516194 (516.1 KB)
        RX errors 0  dropped 0  overruns 0  frame 0
        TX packets 10885  bytes 1346113 (1.3 MB)
        TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0
        device interrupt 16  memory 0xae380000-ae3a0000

Run the tcpdump on interface 'enp0s31f6"
sudo tcpdump -U -pi enp0s31f6 -en


2. Test Results
In dwc driver's Freebsd terminal

root@generic:~ # netstat -n -I dwc0
Name    Mtu Network       Address              Ipkts Ierrs Idrop    Opkts Oerrs  Coll
dwc0   1500 <Link#1>      fa:97:92:f6:f1:09       24     0     0       74     0     0
dwc0      - 192.168.3.0/2 192.168.3.129            6     -     -        4     -     -

root@generic:~ # ping -c 1 192.168.3.2
PING 192.168.3.2 (192.168.3.2): 56 data bytes
64 bytes from 192.168.3.2: icmp_seq=0 ttl=64 time=0.426 ms

--- 192.168.3.2 ping statistics ---
1 packets transmitted, 1 packets received, 0.0% packet loss
round-trip min/avg/max/stddev = 0.426/0.426/0.426/0.000 ms

root@generic:~ # netstat -n -I dwc0
Name    Mtu Network       Address              Ipkts Ierrs Idrop    Opkts Oerrs  Coll
dwc0   1500 <Link#1>      fa:97:92:f6:f1:09       26     0     0       78     0     0
dwc0      - 192.168.3.0/2 192.168.3.129            7     -     -        5     -     -


Packets captured by the tcpdump at Host Terminal 1

$ sudo tcpdump -U -pi enp0s31f6 -en
tcpdump: verbose output suppressed, use -v or -vv for full protocol decode
listening on enp0s31f6, link-type EN10MB (Ethernet), capture size 262144 bytes
10:23:53.592175 fa:97:92:f6:f1:09 > ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 60: Request who-has 192.168.3.2 tell 192.168.3.129, length 46
10:23:53.592188 8c:8c:aa:c1:2b:c3 > fa:97:92:f6:f1:09, ethertype ARP (0x0806), length 42: Reply 192.168.3.2 is-at 8c:8c:aa:c1:2b:c3, length 28
10:23:53.592320 fa:97:92:f6:f1:09 > 8c:8c:aa:c1:2b:c3, ethertype IPv4 (0x0800), length 98: 192.168.3.129 > 192.168.3.2: ICMP echo request, id 59653, seq 0, length 64
10:23:53.592342 8c:8c:aa:c1:2b:c3 > fa:97:92:f6:f1:09, ethertype IPv4 (0x0800), length 98: 192.168.3.2 > 192.168.3.129: ICMP echo reply, id 59653, seq 0, length 64
^C
4 packets captured
4 packets received by filter
0 packets dropped by kernel


Analysis
--------------

The change in the netstat results, Ipkts and Opkts, shows that there were 4 packets sent and 4 packets received at the link-layer level, which is doubled the actual number of packets.

I tried to use the gdb debugger to go through the increments of the counters step by step. And I found that IFCOUNTER_IPACKETS, IFCOUNTER_OPACKETS, IFCOUNTER_IMCASTS, and IFCOUNTER_OMCASTS are involved.
Comment 1 Jiahao LI 2022-05-06 21:36:28 UTC
Update the test result for showing the number of counting tx/rx packets is doubled. The previous result did not show the doubled counting in rx, Ipkts.

In dwc driver's Freebsd terminal

root@generic:~ # arp -d 192.168.3.2
192.168.3.2 (192.168.3.2) deleted
root@generic:~ # netstat -nI dwc0
Name    Mtu Network       Address              Ipkts Ierrs Idrop    Opkts Oerrs  Coll
dwc0   1500 <Link#1>      fa:97:92:f6:f1:09       20     0     0       42     0     0
dwc0      - 192.168.3.0/2 192.168.3.129            2     -     -        2     -     -
root@generic:~ # ping -c 1 192.168.3.2
PING 192.168.3.2 (192.168.3.2): 56 data bytes
64 bytes from 192.168.3.2: icmp_seq=0 ttl=64 time=0.606 ms

--- 192.168.3.2 ping statistics ---
1 packets transmitted, 1 packets received, 0.0% packet loss
round-trip min/avg/max/stddev = 0.606/0.606/0.606/0.000 ms
root@generic:~ # netstat -nI dwc0
Name    Mtu Network       Address              Ipkts Ierrs Idrop    Opkts Oerrs  Coll
dwc0   1500 <Link#1>      fa:97:92:f6:f1:09       24     0     0       46     0     0
dwc0      - 192.168.3.0/2 192.168.3.129            3     -     -        3     -     -


Packets captured by the tcpdump at Host Terminal 1

$ sudo tcpdump -U -pi enp0s31f6 -en
tcpdump: verbose output suppressed, use -v or -vv for full protocol decode
listening on enp0s31f6, link-type EN10MB (Ethernet), capture size 262144 bytes
17:33:20.682566 fa:97:92:f6:f1:09 > ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 60: Request who-has 192.168.3.2 tell 192.168.3.129, length 46
17:33:20.682597 8c:8c:aa:c1:2b:c3 > fa:97:92:f6:f1:09, ethertype ARP (0x0806), length 42: Reply 192.168.3.2 is-at 8c:8c:aa:c1:2b:c3, length 28
17:33:20.682807 fa:97:92:f6:f1:09 > 8c:8c:aa:c1:2b:c3, ethertype IPv4 (0x0800), length 98: 192.168.3.129 > 192.168.3.2: ICMP echo request, id 48389, seq 0, length 64
17:33:20.682864 8c:8c:aa:c1:2b:c3 > fa:97:92:f6:f1:09, ethertype IPv4 (0x0800), length 98: 192.168.3.2 > 192.168.3.129: ICMP echo reply, id 48389, seq 0, length 64
^C
4 packets captured
4 packets received by filter
0 packets dropped by kernel
Comment 2 Mitchell Horne freebsd_committer freebsd_triage 2022-05-09 22:23:41 UTC
Created attachment 233828 [details]
remove duplicate packet counts from if_dwc

Hi,

Thanks for the report, I agree with your analysis.

I have attached a patch that removes the duplicate if_inc_counter() calls in dwc_harvest_stats(). If you are able, please test that it gives the correct result.
Comment 3 Jiahao LI 2022-05-10 13:38:01 UTC
(In reply to Mitchell Horne from comment #2)
Hi,

Thanks for the patch. The re-counting problem can be fixed using this patch. I have tested in my development environment, which is an older version, FreeBSD 13.0-RELEASE-p11. The result is as follows

In the host machine, send a ping packet to the dec0 interface

$ sudo arp -d 192.168.3.129 
$ ping -c 1 192.168.3.129
PING 192.168.3.129 (192.168.3.129) 56(84) bytes of data.
64 bytes from 192.168.3.129: icmp_seq=1 ttl=64 time=0.360 ms

--- 192.168.3.129 ping statistics ---
1 packets transmitted, 1 received, 0% packet loss, time 0ms
rtt min/avg/max/mdev = 0.360/0.360/0.360/0.000 ms

In dwc driver's Freebsd terminal

# netstat -n -I dwc0 
Name    Mtu Network       Address              Ipkts Ierrs Idrop   Imcast    Opkts Oerrs   Omcast  Coll
dwc0   1500 <Link#5>      fa:97:92:f6:f1:09        9     0     0        4       11     0        3     0
dwc0      - 192.168.3.0/2 192.168.3.129            5     -     -        0        3     -        0     -
dwc0      - fe80::%dwc0/6 fe80::f897:92ff:f        0     -     -        0        1     -        0     -
# arp -d 192.168.3.2
192.168.3.2 (192.168.3.2) deleted
# ping -c 1 192.168.3.2 
PING 192.168.3.2 (192.168.3.2): 56 data bytes
64 bytes from 192.168.3.2: icmp_seq=0 ttl=64 time=1.001 ms

--- 192.168.3.2 ping statistics ---
1 packets transmitted, 1 packets received, 0.0% packet loss
round-trip min/avg/max/stddev = 1.001/1.001/1.001/nan ms
# netstat -n -I dwc0    
Name    Mtu Network       Address              Ipkts Ierrs Idrop   Imcast    Opkts Oerrs   Omcast  Coll
dwc0   1500 <Link#5>      fa:97:92:f6:f1:09       14     0     0        5       16     0        3     0
dwc0      - 192.168.3.0/2 192.168.3.129            7     -     -        0        5     -        0     -
dwc0      - fe80::%dwc0/6 fe80::f897:92ff:f        0     -     -        0        1     -        0     -



Packets captured by the tcpdump at Host Terminal 2

$ sudo tcpdump -U -pi enp0s31f6 -en
tcpdump: verbose output suppressed, use -v or -vv for full protocol decode
listening on enp0s31f6, link-type EN10MB (Ethernet), capture size 262144 bytes
09:25:48.672938 8c:8c:aa:c1:2b:c3 > ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 42: Request who-has 192.168.3.129 tell 192.168.3.2, length 28
09:25:48.673101 fa:97:92:f6:f1:09 > 8c:8c:aa:c1:2b:c3, ethertype ARP (0x0806), length 60: Reply 192.168.3.129 is-at fa:97:92:f6:f1:09, length 46
09:25:48.673116 8c:8c:aa:c1:2b:c3 > fa:97:92:f6:f1:09, ethertype IPv4 (0x0800), length 98: 192.168.3.2 > 192.168.3.129: ICMP echo request, id 240, seq 1, length 64
09:25:48.673248 fa:97:92:f6:f1:09 > 8c:8c:aa:c1:2b:c3, ethertype IPv4 (0x0800), length 98: 192.168.3.129 > 192.168.3.2: ICMP echo reply, id 240, seq 1, length 64
09:26:11.242928 fa:97:92:f6:f1:09 > ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 60: Request who-has 192.168.3.2 tell 192.168.3.129, length 46
09:26:11.242944 8c:8c:aa:c1:2b:c3 > fa:97:92:f6:f1:09, ethertype ARP (0x0806), length 42: Reply 192.168.3.2 is-at 8c:8c:aa:c1:2b:c3, length 28
09:26:11.243124 fa:97:92:f6:f1:09 > 8c:8c:aa:c1:2b:c3, ethertype IPv4 (0x0800), length 98: 192.168.3.129 > 192.168.3.2: ICMP echo request, id 2144, seq 0, length 64
09:26:11.243150 8c:8c:aa:c1:2b:c3 > fa:97:92:f6:f1:09, ethertype IPv4 (0x0800), length 98: 192.168.3.2 > 192.168.3.129: ICMP echo reply, id 2144, seq 0, length 64
09:26:16.496592 8c:8c:aa:c1:2b:c3 > fa:97:92:f6:f1:09, ethertype ARP (0x0806), length 42: Request who-has 192.168.3.129 tell 192.168.3.2, length 28
09:26:16.496800 fa:97:92:f6:f1:09 > 8c:8c:aa:c1:2b:c3, ethertype ARP (0x0806), length 60: Reply 192.168.3.129 is-at fa:97:92:f6:f1:09, length 46
^C
10 packets captured
10 packets received by filter
0 packets dropped by kernel



5 packets are sent from the host to dwc0 interface and 5 packets are sent from dwc0 interface to the host. The Ipkts and Opkts are incremented by 5. The dwc0 interface received one broadcast packet, which increments Imcast by 1. The broadcast packet sent from the dwc0 interface to the host is not counted because IFCOUNTER_OMCASTS counter is not used in the code.
Comment 4 Jiahao LI 2022-05-13 20:30:45 UTC
(In reply to Jiahao LI from comment #3)
A follow-up to the previous test result.

IFCOUNTER_OMCASTS counter is checked when the dwc0 sends the ARP request packet, the first packet in the tcpdump log, to the host interface. The re-counting problem can be fixed using your patch.

The ARP request mbuf's m_flags is set as 0x12 in FreeBSD 13.0-RELEASE-p11, which won't be interpreted as a link-level multicast packet but as a link-level broadcast, see sys/net/if.c/if_transmit() function. However, the IFCOUNTER_IMCASTS counter will be incremented when the first bit of the MAC address is "1".
Comment 5 commit-hook freebsd_committer freebsd_triage 2022-06-23 18:16:48 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=9718759043ec2ef36f12b15963194b866d731b5b

commit 9718759043ec2ef36f12b15963194b866d731b5b
Author:     Mitchell Horne <mhorne@FreeBSD.org>
AuthorDate: 2022-06-21 13:24:25 +0000
Commit:     Mitchell Horne <mhorne@FreeBSD.org>
CommitDate: 2022-06-23 18:15:10 +0000

    if_dwc: avoid duplicate packet counts

    We already increment the unicast IPACKETS and OPACKETS counters in the
    rx/tx paths, respectively. Multicast packets are counted in the generic
    ethernet code. Therefore, we shouldn't increment these counters in
    dwc_harvest_stats().

    Drop the early return from dwc_rxfinish_one() so that we still count
    received packets with e.g. a checksum error.

    PR:             263817
    Reported by:    Jiahao LI <jiahali@blackberry.com>
    Reviewed by:    manu
    MFC after:      1 week
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D35499

 sys/dev/dwc/if_dwc.c | 12 ------------
 1 file changed, 12 deletions(-)
Comment 6 Mitchell Horne freebsd_committer freebsd_triage 2022-06-23 18:25:48 UTC
(In reply to Jiahao LI from comment #4)

Sorry, I don't think I completely understood this last point. You are saying there is some separate issue in accounting of ARP broadcast packets that have the Individual/Group bit of the MAC address set to 1? If so, this should probably be solved in a separate PR.
Comment 7 Jiahao LI 2022-06-23 18:42:28 UTC
(In reply to Mitchell Horne from comment #6)
Thanks for the follow-up.

I just notice that an ARP request sent from a host interface to this Freebsd interface will increment the number of received multicast packets by 1 in netstat, which is recorded at the driver's code. But an ARP request sent from this Freebsd interface to a host interface does not increment the number of transmitted multicast packets by 1 in netstat.

sys/net/if.c/if_transmit() will invoke IFQ_HANDOFF_ADJ macro function to do the TX statistic, which is irrelevant to any driver.

I agree that it could be a separate issue if it is an issue for netstat. The issue in this bug is fixed.
Comment 8 commit-hook freebsd_committer freebsd_triage 2022-07-04 16:41:17 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=3a72965a25f96e0719fef098c6c78c3d34a6e808

commit 3a72965a25f96e0719fef098c6c78c3d34a6e808
Author:     Mitchell Horne <mhorne@FreeBSD.org>
AuthorDate: 2022-06-21 13:24:25 +0000
Commit:     Mitchell Horne <mhorne@FreeBSD.org>
CommitDate: 2022-07-04 16:34:57 +0000

    if_dwc: avoid duplicate packet counts

    We already increment the unicast IPACKETS and OPACKETS counters in the
    rx/tx paths, respectively. Multicast packets are counted in the generic
    ethernet code. Therefore, we shouldn't increment these counters in
    dwc_harvest_stats().

    Drop the early return from dwc_rxfinish_one() so that we still count
    received packets with e.g. a checksum error.

    PR:             263817
    Reported by:    Jiahao LI <jiahali@blackberry.com>
    Reviewed by:    manu
    MFC after:      1 week
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D35499

    (cherry picked from commit 9718759043ec2ef36f12b15963194b866d731b5b)

 sys/dev/dwc/if_dwc.c | 12 ------------
 1 file changed, 12 deletions(-)
Comment 9 commit-hook freebsd_committer freebsd_triage 2022-08-11 14:53:26 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=c7c8b80659f5e03a6a1c87a643e18e0ab929e286

commit c7c8b80659f5e03a6a1c87a643e18e0ab929e286
Author:     Mitchell Horne <mhorne@FreeBSD.org>
AuthorDate: 2022-06-21 13:24:25 +0000
Commit:     Mitchell Horne <mhorne@FreeBSD.org>
CommitDate: 2022-08-11 14:52:03 +0000

    if_dwc: avoid duplicate packet counts

    We already increment the unicast IPACKETS and OPACKETS counters in the
    rx/tx paths, respectively. Multicast packets are counted in the generic
    ethernet code. Therefore, we shouldn't increment these counters in
    dwc_harvest_stats().

    Drop the early return from dwc_rxfinish_one() so that we still count
    received packets with e.g. a checksum error.

    PR:             263817
    Reported by:    Jiahao LI <jiahali@blackberry.com>
    Reviewed by:    manu
    MFC after:      1 week
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D35499

    (cherry picked from commit 9718759043ec2ef36f12b15963194b866d731b5b)

 sys/dev/dwc/if_dwc.c | 12 ------------
 1 file changed, 12 deletions(-)