• Vladimir Oltean's avatar
    net: dsa: tag_ocelot: call only the relevant portion of __skb_vlan_pop() on TX · 0bcf2e4a
    Vladimir Oltean authored
    ocelot_xmit_get_vlan_info() calls __skb_vlan_pop() as the most
    appropriate helper I could find which strips away a VLAN header.
    That's all I need it to do, but __skb_vlan_pop() has more logic, which
    will become incompatible with the future revert of commit 6d1ccff6
    ("net: reset mac header in dev_start_xmit()").
    
    Namely, it performs a sanity check on skb_mac_header(), which will stop
    being set after the above revert, so it will return an error instead of
    removing the VLAN tag.
    
    ocelot_xmit_get_vlan_info() gets called in 2 circumstances:
    
    (1) the port is under a VLAN-aware bridge and the bridge sends
        VLAN-tagged packets
    
    (2) the port is under a VLAN-aware bridge and somebody else (an 8021q
        upper) sends VLAN-tagged packets (using a VID that isn't in the
        bridge vlan tables)
    
    In case (1), there is actually no bug to defend against, because
    br_dev_xmit() calls skb_reset_mac_header() and things continue to work.
    
    However, in case (2), illustrated using the commands below, it can be
    seen that our intervention is needed, since __skb_vlan_pop() complains:
    
    $ ip link add br0 type bridge vlan_filtering 1 && ip link set br0 up
    $ ip link set $eth master br0 && ip link set $eth up
    $ ip link add link $eth name $eth.100 type vlan id 100 && ip link set $eth.100 up
    $ ip addr add 192.168.100.1/24 dev $eth.100
    
    I could fend off the checks in __skb_vlan_pop() with some
    skb_mac_header_was_set() calls, but seeing how few callers of
    __skb_vlan_pop() there are from TX paths, that seems rather
    unproductive.
    
    As an alternative solution, extract the bare minimum logic to strip a
    VLAN header, and move it to a new helper named vlan_remove_tag(), close
    to the definition of vlan_insert_tag(). Document it appropriately and
    make ocelot_xmit_get_vlan_info() call this smaller helper instead.
    
    Seeing that it doesn't appear illegal to test skb->protocol in the TX
    path, I guess it would be a good for vlan_remove_tag() to also absorb
    the vlan_set_encap_proto() function call.
    Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
    Reviewed-by: default avatarSimon Horman <simon.horman@corigine.com>
    Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
    Reviewed-by: default avatarEric Dumazet <edumazet@google.com>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    0bcf2e4a
skbuff.c 173 KB