• Vladimir Oltean's avatar
    net: dsa: report rx_bytes unadjusted for ETH_HLEN · a8eff035
    Vladimir Oltean authored
    We collect the software statistics counters for RX bytes (reported to
    /proc/net/dev and to ethtool -S $dev | grep 'rx_bytes: ") at a time when
    skb->len has already been adjusted by the eth_type_trans() ->
    skb_pull_inline(skb, ETH_HLEN) call to exclude the L2 header.
    
    This means that when connecting 2 DSA interfaces back to back and
    sending 1 packet with length 100, the sending interface will report
    tx_bytes as incrementing by 100, and the receiving interface will report
    rx_bytes as incrementing by 86.
    
    Since accounting for that in scripts is quirky and is something that
    would be DSA-specific behavior (requiring users to know that they are
    running on a DSA interface in the first place), the proposal is that we
    treat it as a bug and fix it.
    
    This design bug has always existed in DSA, according to my analysis:
    commit 91da11f8 ("net: Distributed Switch Architecture protocol
    support") also updates skb->dev->stats.rx_bytes += skb->len after the
    eth_type_trans() call. Technically, prior to Florian's commit
    a86d8bec ("net: dsa: Factor bottom tag receive functions"), each and
    every vendor-specific tagging protocol driver open-coded the same bug,
    until the buggy code was consolidated into something resembling what can
    be seen now. So each and every driver should have its own Fixes: tag,
    because of their different histories until the convergence point.
    I'm not going to do that, for the sake of simplicity, but just blame the
    oldest appearance of buggy code.
    
    There are 2 ways to fix the problem. One is the obvious way, and the
    other is how I ended up doing it. Obvious would have been to move
    dev_sw_netstats_rx_add() one line above eth_type_trans(), and below
    skb_push(skb, ETH_HLEN). But DSA processing is not as simple as that.
    We count the bytes after removing everything DSA-related from the
    packet, to emulate what the packet's length was, on the wire, when the
    user port received it.
    
    When eth_type_trans() executes, dsa_untag_bridge_pvid() has not run yet,
    so in case the switch driver requests this behavior - commit
    412a1526 ("net: dsa: untag the bridge pvid from rx skbs") has the
    details - the obvious variant of the fix wouldn't have worked, because
    the positioning there would have also counted the not-yet-stripped VLAN
    header length, something which is absent from the packet as seen on the
    wire (there it may be untagged, whereas software will see it as
    PVID-tagged).
    
    Fixes: f613ed66 ("net: dsa: Add support for 64-bit statistics")
    Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    a8eff035
tag.c 5.64 KB