Fw: [Bug 84951] New: 8021q: kernel doesn't take into account ethernet header bytes for received packets

Fw: [Bug 84951] New: 8021q: kernel doesn't take into account ethernet header bytes for received packets

[Stephen Hemminger]

I am inclined to think this is something that is just an incorrect
user expectation.

Begin forwarded message:

Date: Sun, 21 Sep 2014 11:47:22 -0700
From: "bugzilla-daemon@bugzilla.kernel.org" <bugzilla-daemon@bugzilla.kernel.org>
To: "stephen@networkplumber.org" <stephen@networkplumber.org>
Subject: [Bug 84951] New: 8021q: kernel doesn't take into account ethernet header bytes for received packets


https://bugzilla.kernel.org/show_bug.cgi?id=84951

            Bug ID: 84951
           Summary: 8021q: kernel doesn't take into account ethernet
                    header bytes for received packets
           Product: Networking
           Version: 2.5
    Kernel Version: 3.16.1
          Hardware: All
                OS: Linux
              Tree: Mainline
            Status: NEW
          Severity: normal
          Priority: P1
         Component: Other
          Assignee: shemminger@linux-foundation.org
          Reporter: g.djavadyan@gmail.com
        Regression: No

Statistics exported by the kernel for received packets on 802.1q subinterfaces
don't consider Ethernet header bytes (dst MAC, src MAC, type). Statistics for
transmitted packets is not affected. The problem is more prominent on
high-speed links (>100Mb/s), as networking tools (nload, iftop, ...) display
lower bandwidth utilization (>5Mb/s difference) than a report from neighboring
routing device (FreeBSD, Cisco). Also, someone monitoring Linux router
interfaces will find that the router generates more information than it
receives.

Tested on CentOS kernel 2.6.32-431.29.2.el6.x86_64 and vanilla 3.16.1 with
drivers: bnx2, atl1c and igb.

How to reproduce:

1. Create dot1q subinterfaces on two boxes (vconfig add eth0 4040).
2. Assign IP addresses to subinterfaces.
3. Check the connection using ping.
4. Check the output from ifconfig eth0.4040 or 'cat
/sys/class/net/eth0.4040/statistics/rx_{packets,bytes}'.
5. Issue standard 56 byte payload ping using 'ping -c 1 neighboring_ip'.
6. Recheck statistics using step 4.

Expected results:

Received packets value should increase by 1 and received bytes value should
increase by 98.

dst MAC - 6
src MAC - 6
ethertype - 2
IP header - 20
ICMP header - 8
ICMP Payload - 56
Total: 98 bytes.


Actual results:

Received packets value increases by 1 and received bytes value increases by 84
bytes.

-- 
You are receiving this mail because:
You are the assignee for the bug.

Re: Fw: [Bug 84951] New: 8021q: kernel doesn't take into account ethernet header bytes for received packets

[Vlad Yasevich]

On 09/22/2014 12:01 PM, Stephen Hemminger wrote:
> I am inclined to think this is something that is just an incorrect
> user expectation.

Looks like a valid bug.  vlan just uses skb->len which already had
the eth header stripped off, so it will always under-report 12 bytes
per packet.

-vlad

> 
> Begin forwarded message:
> 
> Date: Sun, 21 Sep 2014 11:47:22 -0700
> From: "bugzilla-daemon@bugzilla.kernel.org" <bugzilla-daemon@bugzilla.kernel.org>
> To: "stephen@networkplumber.org" <stephen@networkplumber.org>
> Subject: [Bug 84951] New: 8021q: kernel doesn't take into account ethernet header bytes for received packets
> 
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=84951
> 
>             Bug ID: 84951
>            Summary: 8021q: kernel doesn't take into account ethernet
>                     header bytes for received packets
>            Product: Networking
>            Version: 2.5
>     Kernel Version: 3.16.1
>           Hardware: All
>                 OS: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: Other
>           Assignee: shemminger@linux-foundation.org
>           Reporter: g.djavadyan@gmail.com
>         Regression: No
> 
> Statistics exported by the kernel for received packets on 802.1q subinterfaces
> don't consider Ethernet header bytes (dst MAC, src MAC, type). Statistics for
> transmitted packets is not affected. The problem is more prominent on
> high-speed links (>100Mb/s), as networking tools (nload, iftop, ...) display
> lower bandwidth utilization (>5Mb/s difference) than a report from neighboring
> routing device (FreeBSD, Cisco). Also, someone monitoring Linux router
> interfaces will find that the router generates more information than it
> receives.
> 
> Tested on CentOS kernel 2.6.32-431.29.2.el6.x86_64 and vanilla 3.16.1 with
> drivers: bnx2, atl1c and igb.
> 
> How to reproduce:
> 
> 1. Create dot1q subinterfaces on two boxes (vconfig add eth0 4040).
> 2. Assign IP addresses to subinterfaces.
> 3. Check the connection using ping.
> 4. Check the output from ifconfig eth0.4040 or 'cat
> /sys/class/net/eth0.4040/statistics/rx_{packets,bytes}'.
> 5. Issue standard 56 byte payload ping using 'ping -c 1 neighboring_ip'.
> 6. Recheck statistics using step 4.
> 
> Expected results:
> 
> Received packets value should increase by 1 and received bytes value should
> increase by 98.
> 
> dst MAC - 6
> src MAC - 6
> ethertype - 2
> IP header - 20
> ICMP header - 8
> ICMP Payload - 56
> Total: 98 bytes.
> 
> 
> Actual results:
> 
> Received packets value increases by 1 and received bytes value increases by 84
> bytes.
> 

[PATCH] vlan: Fix receive statistics under-reporting

[Vladislav Yasevich]

Vlan devices uder-report 14 bytes per packet in the recieve statistics.
This is because the ETH_HLEN bytes of data has been pulled off the skb
by the time it gets to the vlan receive code.  When accounting for
receive butes, add ETH_HLEN back.

See also https://bugzilla.kernel.org/show_bug.cgi?id=84951

Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
 net/8021q/vlan_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 90cc2bd..8bc5d46 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -54,7 +54,7 @@ bool vlan_do_receive(struct sk_buff **skbp)
 
 	u64_stats_update_begin(&rx_stats->syncp);
 	rx_stats->rx_packets++;
-	rx_stats->rx_bytes += skb->len;
+	rx_stats->rx_bytes += skb->len + ETH_HLEN;
 	if (skb->pkt_type == PACKET_MULTICAST)
 		rx_stats->rx_multicast++;
 	u64_stats_update_end(&rx_stats->syncp);
-- 
1.9.3

Re: [PATCH] vlan: Fix receive statistics under-reporting

[Eric Dumazet]

On Wed, 2014-09-24 at 10:45 -0400, Vladislav Yasevich wrote:
> Vlan devices uder-report 14 bytes per packet in the recieve statistics.
> This is because the ETH_HLEN bytes of data has been pulled off the skb
> by the time it gets to the vlan receive code.  When accounting for
> receive butes, add ETH_HLEN back.
> 
> See also https://bugzilla.kernel.org/show_bug.cgi?id=84951
> 
> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
> ---
>  net/8021q/vlan_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
> index 90cc2bd..8bc5d46 100644
> --- a/net/8021q/vlan_core.c
> +++ b/net/8021q/vlan_core.c
> @@ -54,7 +54,7 @@ bool vlan_do_receive(struct sk_buff **skbp)
>  
>  	u64_stats_update_begin(&rx_stats->syncp);
>  	rx_stats->rx_packets++;
> -	rx_stats->rx_bytes += skb->len;
> +	rx_stats->rx_bytes += skb->len + ETH_HLEN;
>  	if (skb->pkt_type == PACKET_MULTICAST)
>  		rx_stats->rx_multicast++;
>  	u64_stats_update_end(&rx_stats->syncp);


I do not think this patch is general enough, it doesn't handle GRO for
example.

And should not the vlan tag also be accounted ? 

Re: [PATCH] vlan: Fix receive statistics under-reporting

[Vlad Yasevich]

On 09/24/2014 11:08 AM, Eric Dumazet wrote:
> On Wed, 2014-09-24 at 10:45 -0400, Vladislav Yasevich wrote:
>> Vlan devices uder-report 14 bytes per packet in the recieve statistics.
>> This is because the ETH_HLEN bytes of data has been pulled off the skb
>> by the time it gets to the vlan receive code.  When accounting for
>> receive butes, add ETH_HLEN back.
>>
>> See also https://bugzilla.kernel.org/show_bug.cgi?id=84951
>>
>> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
>> ---
>>  net/8021q/vlan_core.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
>> index 90cc2bd..8bc5d46 100644
>> --- a/net/8021q/vlan_core.c
>> +++ b/net/8021q/vlan_core.c
>> @@ -54,7 +54,7 @@ bool vlan_do_receive(struct sk_buff **skbp)
>>  
>>  	u64_stats_update_begin(&rx_stats->syncp);
>>  	rx_stats->rx_packets++;
>> -	rx_stats->rx_bytes += skb->len;
>> +	rx_stats->rx_bytes += skb->len + ETH_HLEN;
>>  	if (skb->pkt_type == PACKET_MULTICAST)
>>  		rx_stats->rx_multicast++;
>>  	u64_stats_update_end(&rx_stats->syncp);
> 
> 
> I do not think this patch is general enough, it doesn't handle GRO for
> example.

I just looked at more software devices (including encapsulators) and
find that macvlan appears to be the only anomaly.

Everyone appears to be using skb->len directly and dropping ETH_HLEN.
Macvlan tries to be "extra smart" and adds the ETH_HLEN back in.

Definitely this patch isn't enough all by itself.  I might be better
to make it consistent the other way around.


> 
> And should not the vlan tag also be accounted ? 
> 
> 

I don't think so. We don't do it on TX.

-vlad