• Ivan Vecera's avatar
    ice: Fix broken IFF_ALLMULTI handling · 1273f895
    Ivan Vecera authored
    Handling of all-multicast flag and associated multicast promiscuous
    mode is broken in ice driver. When an user switches allmulticast
    flag on or off the driver checks whether any VLANs are configured
    over the interface (except default VLAN 0).
    
    If any extra VLANs are registered it enables multicast promiscuous
    mode for all these VLANs (including default VLAN 0) using
    ICE_SW_LKUP_PROMISC_VLAN look-up type. In this situation all
    multicast packets tagged with known VLAN ID or untagged are received
    and multicast packets tagged with unknown VLAN ID ignored.
    
    If no extra VLANs are registered (so only VLAN 0 exists) it enables
    multicast promiscuous mode for VLAN 0 and uses ICE_SW_LKUP_PROMISC
    look-up type. In this situation any multicast packets including
    tagged ones are received.
    
    The driver handles IFF_ALLMULTI in ice_vsi_sync_fltr() this way:
    
    ice_vsi_sync_fltr() {
      ...
      if (changed_flags & IFF_ALLMULTI) {
        if (netdev->flags & IFF_ALLMULTI) {
          if (vsi->num_vlans > 1)
            ice_set_promisc(..., ICE_MCAST_VLAN_PROMISC_BITS);
          else
            ice_set_promisc(..., ICE_MCAST_PROMISC_BITS);
        } else {
          if (vsi->num_vlans > 1)
            ice_clear_promisc(..., ICE_MCAST_VLAN_PROMISC_BITS);
          else
            ice_clear_promisc(..., ICE_MCAST_PROMISC_BITS);
        }
      }
      ...
    }
    
    The code above depends on value vsi->num_vlan that specifies number
    of VLANs configured over the interface (including VLAN 0) and
    this is problem because that value is modified in NDO callbacks
    ice_vlan_rx_add_vid() and ice_vlan_rx_kill_vid().
    
    Scenario 1:
    1. ip link set ens7f0 allmulticast on
    2. ip link add vlan10 link ens7f0 type vlan id 10
    3. ip link set ens7f0 allmulticast off
    4. ip link set ens7f0 allmulticast on
    
    [1] In this scenario IFF_ALLMULTI is enabled and the driver calls
        ice_set_promisc(..., ICE_MCAST_PROMISC_BITS) that installs
        multicast promisc rule with non-VLAN look-up type.
    [2] Then VLAN with ID 10 is added and vsi->num_vlan incremented to 2
    [3] Command switches IFF_ALLMULTI off and the driver calls
        ice_clear_promisc(..., ICE_MCAST_VLAN_PROMISC_BITS) but this
        call is effectively NOP because it looks for multicast promisc
        rules for VLAN 0 and VLAN 10 with VLAN look-up type but no such
        rules exist. So the all-multicast remains enabled silently
        in hardware.
    [4] Command tries to switch IFF_ALLMULTI on and the driver calls
        ice_clear_promisc(..., ICE_MCAST_PROMISC_BITS) but this call
        fails (-EEXIST) because non-VLAN multicast promisc rule already
        exists.
    
    Scenario 2:
    1. ip link add vlan10 link ens7f0 type vlan id 10
    2. ip link set ens7f0 allmulticast on
    3. ip link add vlan20 link ens7f0 type vlan id 20
    4. ip link del vlan10 ; ip link del vlan20
    5. ip link set ens7f0 allmulticast off
    
    [1] VLAN with ID 10 is added and vsi->num_vlan==2
    [2] Command switches IFF_ALLMULTI on and driver installs multicast
        promisc rules with VLAN look-up type for VLAN 0 and 10
    [3] VLAN with ID 20 is added and vsi->num_vlan==3 but no multicast
        promisc rules is added for this new VLAN so the interface does
        not receive MC packets from VLAN 20
    [4] Both VLANs are removed but multicast rule for VLAN 10 remains
        installed so interface receives multicast packets from VLAN 10
    [5] Command switches IFF_ALLMULTI off and because vsi->num_vlan is 1
        the driver tries to remove multicast promisc rule for VLAN 0
        with non-VLAN look-up that does not exist.
        All-multicast looks disabled from user point of view but it
        is partially enabled in HW (interface receives all multicast
        packets either untagged or tagged with VLAN ID 10)
    
    To resolve these issues the patch introduces these changes:
    1. Adds handling for IFF_ALLMULTI to ice_vlan_rx_add_vid() and
       ice_vlan_rx_kill_vid() callbacks. So when VLAN is added/removed
       and IFF_ALLMULTI is enabled an appropriate multicast promisc
       rule for that VLAN ID is added/removed.
    2. In ice_vlan_rx_add_vid() when first VLAN besides VLAN 0 is added
       so (vsi->num_vlan == 2) and IFF_ALLMULTI is enabled then look-up
       type for existing multicast promisc rule for VLAN 0 is updated
       to ICE_MCAST_VLAN_PROMISC_BITS.
    3. In ice_vlan_rx_kill_vid() when last VLAN besides VLAN 0 is removed
       so (vsi->num_vlan == 1) and IFF_ALLMULTI is enabled then look-up
       type for existing multicast promisc rule for VLAN 0 is updated
       to ICE_MCAST_PROMISC_BITS.
    4. Both ice_vlan_rx_{add,kill}_vid() have to run under ICE_CFG_BUSY
       bit protection to avoid races with ice_vsi_sync_fltr() that runs
       in ice_service_task() context.
    5. Bit ICE_VSI_VLAN_FLTR_CHANGED is use-less and can be removed.
    6. Error messages added to ice_fltr_*_vsi_promisc() helper functions
       to avoid them in their callers
    7. Small improvements to increase readability
    
    Fixes: 5eda8afd ("ice: Add support for PF/VF promiscuous mode")
    Signed-off-by: default avatarIvan Vecera <ivecera@redhat.com>
    Reviewed-by: default avatarJacob Keller <jacob.e.keller@intel.com>
    Signed-off-by: default avatarAlice Michael <alice.michael@intel.com>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    1273f895
ice_main.c 238 KB