1. 21 Apr, 2023 16 commits
    • Jakub Kicinski's avatar
      Merge branch 'ethtool-mm-api-consolidation' · b7b871f5
      Jakub Kicinski authored
      Vladimir Oltean says:
      
      ====================
      ethtool mm API consolidation
      
      This series consolidates the behavior of the 2 drivers that implement
      the ethtool MAC Merge layer by making NXP ENETC commit its preemptible
      traffic classes to hardware only when MM TX is active (same as Ocelot).
      
      Then, after resolving an issue with the ENETC driver, it restricts user
      space from entering 2 states which don't make sense:
      
      - pmac-enabled off tx-enabled on  verify-enabled *
      - pmac-enabled *   tx-enabled off verify-enabled on
      
      Then, it introduces a selftest (ethtool_mm.sh) which puts everything
      together and tests all valid configurations known to me.
      
      This is simultaneously the v2 of "[PATCH net-next 0/2] ethtool mm API
      improvements":
      https://lore.kernel.org/netdev/20230415173454.3970647-1-vladimir.oltean@nxp.com/
      which had caused some problems to openlldp. Those were solved in the
      meantime, see:
      https://github.com/intel/openlldp/commit/11171b474f6f3cbccac5d608b7f26b32ff72c651
      
      and of "[RFC PATCH net-next] selftests: forwarding: add a test for MAC
      Merge layer":
      https://lore.kernel.org/netdev/20230210221243.228932-1-vladimir.oltean@nxp.com/
      ====================
      
      Link: https://lore.kernel.org/r/20230418111459.811553-1-vladimir.oltean@nxp.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      b7b871f5
    • Vladimir Oltean's avatar
      selftests: forwarding: add a test for MAC Merge layer · e6991384
      Vladimir Oltean authored
      The MAC Merge layer (IEEE 802.3-2018 clause 99) does all the heavy
      lifting for Frame Preemption (IEEE 802.1Q-2018 clause 6.7.2), a TSN
      feature for minimizing latency.
      
      Preemptible traffic is different on the wire from normal traffic in
      incompatible ways. If we send a preemptible packet and the link partner
      doesn't support preemption, it will drop it as an error frame and we
      will never know. The MAC Merge layer has a control plane of its own,
      which can be manipulated (using ethtool) in order to negotiate this
      capability with the link partner (through LLDP).
      
      Actually the TLV format for LLDP solves this problem only partly,
      because both partners only advertise:
      - if they support preemption (RX and TX)
      - if they have enabled preemption (TX)
      so we cannot tell the link partner what to do - we cannot force it to
      enable reception of our preemptible packets.
      
      That is fully solved by the verification feature, where the local device
      generates some small probe frames which look like preemptible frames
      with no useful content, and the link partner is obliged to respond to
      them if it supports the standard. If the verification times out, we know
      that preemption isn't active in our TX direction on the link.
      
      Having clarified the definition, this selftest exercises the manual
      (ethtool) configuration path of 2 link partners (with and without
      verification), and the LLDP code path, using the openlldp project.
      
      The test also verifies the TX activity of the MAC Merge layer by
      sending traffic through a traffic class configured as preemptible
      (using mqprio). There isn't a good way to make this really portable
      (user space cannot find out how many traffic classes there are for
      a device), but I chose num_tc 4 here, that should work reasonably well.
      I also know that some devices (stmmac) only permit TXQ0 to be
      preemptible, so this is why PREEMPTIBLE_PRIO was strategically chosen
      as 0. Even if other hardware is more configurable, this test should
      cover the baseline.
      
      This is not really a "forwarding" selftest, but I put it near the other
      "ethtool" selftests.
      
      $ ./ethtool_mm.sh eno0 swp0
      TEST: Manual configuration with verification: eno0 to swp0          [ OK ]
      TEST: Manual configuration with verification: swp0 to eno0          [ OK ]
      TEST: Manual configuration without verification: eno0 to swp0       [ OK ]
      TEST: Manual configuration without verification: swp0 to eno0       [ OK ]
      TEST: Manual configuration with failed verification: eno0 to swp0   [ OK ]
      TEST: Manual configuration with failed verification: swp0 to eno0   [ OK ]
      TEST: LLDP                                                          [ OK ]
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      e6991384
    • Vladimir Oltean's avatar
      selftests: forwarding: introduce helper for standard ethtool counters · b5bf7126
      Vladimir Oltean authored
      Counters for the MAC Merge layer and preemptible MAC have standardized
      so far on using structured ethtool stats as opposed to the driver
      specific names and meanings.
      
      Benefit from that rare opportunity and introduce a helper to lib.sh for
      querying standardized counters, in the hope that these will take off for
      other uses as well.
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      b5bf7126
    • Petr Machata's avatar
      selftests: forwarding: generalize bail_on_lldpad from mlxsw · 8fcac792
      Petr Machata authored
      mlxsw selftests often invoke a bail_on_lldpad() helper to make sure LLDPAD
      is not running, to prevent conflicts between the QoS configuration applied
      through TC or DCB command line tool, and the DCB configuration that LLDPAD
      might apply. This helper might be useful to others. Move the function to
      lib.sh, and parameterize to make reusable in other contexts.
      Signed-off-by: default avatarPetr Machata <petrm@nvidia.com>
      Reviewed-by: default avatarDanielle Ratson <danieller@nvidia.com>
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      8fcac792
    • Petr Machata's avatar
      selftests: forwarding: sch_tbf_*: Add a pre-run hook · 54e906f1
      Petr Machata authored
      The driver-specific wrappers of these selftests invoke bail_on_lldpad to
      make sure that LLDPAD doesn't trample the configuration. The function
      bail_on_lldpad is going to move to lib.sh in the next patch. With that, it
      won't be visible for the wrappers before sourcing the framework script. And
      after sourcing it, it is too late: the selftest will have run by then.
      
      One option might be to source NUM_NETIFS=0 lib.sh from the wrapper, but
      even if that worked (it might, it might not), that seems cumbersome. lib.sh
      is doing fair amount of stuff, and even if it works today, it does not look
      particularly solid as a solution.
      
      Instead, introduce a hook, sch_tbf_pre_hook(), that when available, gets
      invoked. Move the bail to the hook.
      Signed-off-by: default avatarPetr Machata <petrm@nvidia.com>
      Reviewed-by: default avatarDanielle Ratson <danieller@nvidia.com>
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      54e906f1
    • Vladimir Oltean's avatar
      net: ethtool: mm: sanitize some UAPI configurations · 35b288d6
      Vladimir Oltean authored
      The verify-enabled boolean (ETHTOOL_A_MM_VERIFY_ENABLED) was intended to
      be a sub-setting of tx-enabled (ETHTOOL_A_MM_TX_ENABLED). IOW, MAC Merge
      TX can be enabled with or without verification, but verification with TX
      disabled makes no sense.
      
      The pmac-enabled boolean (ETHTOOL_A_MM_PMAC_ENABLED) was intended to be
      a global toggle from an API perspective, whereas tx-enabled just handles
      the TX direction. IOW, the pMAC can be enabled with or without TX, but
      it doesn't make sense to enable TX if the pMAC is not enabled.
      
      Add two checks which sanitize and reject these invalid cases.
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: default avatarSimon Horman <simon.horman@corigine.com>
      Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      35b288d6
    • Vladimir Oltean's avatar
      net: enetc: include MAC Merge / FP registers in register dump · 16a2c763
      Vladimir Oltean authored
      These have been useful in debugging various problems related to frame
      preemption, so make them available through ethtool --register-dump for
      later too.
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: default avatarSimon Horman <simon.horman@corigine.com>
      Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      16a2c763
    • Vladimir Oltean's avatar
      net: enetc: only commit preemptible TCs to hardware when MM TX is active · 82714539
      Vladimir Oltean authored
      This was left as TODO in commit 01e23b2b ("net: enetc: add support
      for preemptible traffic classes") since it's relatively complicated.
      
      Where this makes a difference is with a configuration as follows:
      
      ethtool --set-mm eno0 pmac-enabled on tx-enabled on verify-enabled on
      
      Preemptible packets should only be sent when the MAC Merge TX direction
      becomes active (i.o.w. when the verification process succeeds, aka when
      the link partner confirms it can process preemptible traffic). But the
      tc qdisc with the preemptible traffic classes is offloaded completely
      asynchronously w.r.t. the MM becoming active.
      
      The ENETC manual does suggest that this should be handled in the driver:
      "On startup, software should wait for the verification process to
      complete (MMCSR[VSTS]=011) before initiating traffic".
      
      Adding the necessary logic allows future selftests to uphold the claim
      that an inactive or disabled MAC Merge layer should never send data
      packets through the pMAC.
      
      This change moves enetc_set_ptcfpr() from enetc.c to enetc_ethtool.c,
      where its only caller is now - enetc_mm_commit_preemptible_tcs().
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: default avatarSimon Horman <simon.horman@corigine.com>
      Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      82714539
    • Vladimir Oltean's avatar
      net: enetc: report mm tx-active based on tx-enabled and verify-status · 153b5b1d
      Vladimir Oltean authored
      The MMCSR register contains 2 fields with overlapping meaning:
      
      - LPA (Local preemption active):
      This read-only status bit indicates whether preemption is active for
      this port. This bit will be set if preemption is both enabled and has
      completed the verification process.
      - TXSTS (Merge status):
      This read-only status field provides the state of the MAC Merge sublayer
      transmit status as defined in IEEE Std 802.3-2018 Clause 99.
      00 Transmit preemption is inactive
      01 Transmit preemption is active
      10 Reserved
      11 Reserved
      
      However none of these 2 fields offer reliable reporting to software.
      
      When connecting ENETC to a link partner which is not capable of Frame
      Preemption, the expectation is that ENETC's verification should fail
      (VSTS=4) and its MM TX direction should be inactive (LPA=0, TXSTS=00)
      even though the MM TX is enabled (ME=1). But surprise, the LPA bit of
      MMCSR stays set even if VSTS=4 and ME=1.
      
      OTOH, the TXSTS field has the opposite problem. I cannot get its value
      to change from 0, even when connecting to a link partner capable of
      frame preemption, which does respond to its verification frames (ME=1
      and VSTS=3, "SUCCEEDED").
      
      The only option with such buggy hardware seems to be to reimplement the
      formula for calculating tx-active in software, which is for tx-enabled
      to be true, and for the verify-status to be either SUCCEEDED, or
      DISABLED.
      
      Without reliable tx-active reporting, we have no good indication when
      to commit the preemptible traffic classes to hardware, which makes it
      possible (but not desirable) to send preemptible traffic to a link
      partner incapable of receiving it. However, currently we do not have the
      logic to wait for TX to be active yet, so the impact is limited.
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: default avatarSimon Horman <simon.horman@corigine.com>
      Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      153b5b1d
    • Vladimir Oltean's avatar
      net: enetc: fix MAC Merge layer remaining enabled until a link down event · 59be75db
      Vladimir Oltean authored
      Current enetc_set_mm() is designed to set the priv->active_offloads bit
      ENETC_F_QBU for enetc_mm_link_state_update() to act on, but if the link
      is already up, it modifies the ENETC_MMCSR_ME ("Merge Enable") bit
      directly.
      
      The problem is that it only *sets* ENETC_MMCSR_ME if the link is up, it
      doesn't *clear* it if needed. So subsequent enetc_get_mm() calls still
      see tx-enabled as true, up until a link down event, which is when
      enetc_mm_link_state_update() will get called.
      
      This is not a functional issue as far as I can assess. It has only come
      up because I'd like to uphold a simple API rule in core ethtool code:
      the pMAC cannot be disabled if TX is going to be enabled. Currently,
      the fact that TX remains enabled for longer than expected (after the
      enetc_set_mm() call that disables it) is going to violate that rule,
      which is how it was caught.
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: default avatarSimon Horman <simon.horman@corigine.com>
      Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      59be75db
    • Slark Xiao's avatar
      wwan: core: add print for wwan port attach/disconnect · 787e6144
      Slark Xiao authored
      Refer to USB serial device or net device, there is a notice to
      let end user know the status of device, like attached or
      disconnected. Add attach/disconnect print for wwan device as
      well.
      Signed-off-by: default avatarSlark Xiao <slark_xiao@163.com>
      Reviewed-by: default avatarLoic Poulain <loic.poulain@linaro.org>
      Link: https://lore.kernel.org/r/20230420023617.3919569-1-slark_xiao@163.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      787e6144
    • Jakub Kicinski's avatar
      net: skbuff: update and rename __kfree_skb_defer() · 8fa66e4a
      Jakub Kicinski authored
      __kfree_skb_defer() uses the old naming where "defer" meant
      slab bulk free/alloc APIs. In the meantime we also made
      __kfree_skb_defer() feed the per-NAPI skb cache, which
      implies bulk APIs. So take away the 'defer' and add 'napi'.
      
      While at it add a drop reason. This only matters on the
      tx_action path, if the skb has a frag_list. But getting
      rid of a SKB_DROP_REASON_NOT_SPECIFIED seems like a net
      benefit so why not.
      Reviewed-by: default avatarAlexander Lobakin <aleksander.lobakin@intel.com>
      Link: https://lore.kernel.org/r/20230420020005.815854-1-kuba@kernel.orgSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      8fa66e4a
    • Jakub Kicinski's avatar
      eth: mlx5: avoid iterator use outside of a loop · 61718206
      Jakub Kicinski authored
      Fix the following warning about risky iterator use:
      
      drivers/net/ethernet/mellanox/mlx5/core/eq.c:1010 mlx5_comp_irq_get_affinity_mask() warn: iterator used outside loop: 'eq'
      Acked-by: default avatarSaeed Mahameed <saeed@kernel.org>
      Link: https://lore.kernel.org/r/20230420015802.815362-1-kuba@kernel.orgSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      61718206
    • Simon Horman's avatar
      flow_dissector: Address kdoc warnings · 8c966a10
      Simon Horman authored
      Address a number of warnings flagged by
      ./scripts/kernel-doc -none include/net/flow_dissector.h
      
       include/net/flow_dissector.h:23: warning: Function parameter or member 'addr_type' not described in 'flow_dissector_key_control'
       include/net/flow_dissector.h:23: warning: Function parameter or member 'flags' not described in 'flow_dissector_key_control'
       include/net/flow_dissector.h:46: warning: Function parameter or member 'padding' not described in 'flow_dissector_key_basic'
       include/net/flow_dissector.h:145: warning: Function parameter or member 'tipckey' not described in 'flow_dissector_key_addrs'
       include/net/flow_dissector.h:157: warning: cannot understand function prototype: 'struct flow_dissector_key_arp '
       include/net/flow_dissector.h:171: warning: cannot understand function prototype: 'struct flow_dissector_key_ports '
       include/net/flow_dissector.h:203: warning: cannot understand function prototype: 'struct flow_dissector_key_icmp '
      
      Also improve indentation on adjacent lines to those changed
      to address the above.
      
      No functional changes intended.
      Signed-off-by: default avatarSimon Horman <horms@kernel.org>
      Link: https://lore.kernel.org/r/20230419-flow-dissector-kdoc-v1-1-1aa0cca1118b@kernel.orgSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      8c966a10
    • Jakub Kicinski's avatar
      page_pool: unlink from napi during destroy · dd64b232
      Jakub Kicinski authored
      Jesper points out that we must prevent recycling into cache
      after page_pool_destroy() is called, because page_pool_destroy()
      is not synchronized with recycling (some pages may still be
      outstanding when destroy() gets called).
      
      I assumed this will not happen because NAPI can't be scheduled
      if its page pool is being destroyed. But I missed the fact that
      NAPI may get reused. For instance when user changes ring configuration
      driver may allocate a new page pool, stop NAPI, swap, start NAPI,
      and then destroy the old pool. The NAPI is running so old page
      pool will think it can recycle to the cache, but the consumer
      at that point is the destroy() path, not NAPI.
      
      To avoid extra synchronization let the drivers do "unlinking"
      during the "swap" stage while NAPI is indeed disabled.
      
      Fixes: 8c48eea3 ("page_pool: allow caching from safely localized NAPI")
      Reported-by: default avatarJesper Dangaard Brouer <jbrouer@redhat.com>
      Link: https://lore.kernel.org/all/e8df2654-6a5b-3c92-489d-2fe5e444135f@redhat.com/Acked-by: default avatarJesper Dangaard Brouer <brouer@redhat.com>
      Link: https://lore.kernel.org/r/20230419182006.719923-1-kuba@kernel.orgSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      dd64b232
    • Arnd Bergmann's avatar
      net: phy: fix circular LEDS_CLASS dependencies · 4bb7aac7
      Arnd Bergmann authored
      The CONFIG_PHYLIB symbol is selected by a number of device drivers that
      need PHY support, but it now has a dependency on CONFIG_LEDS_CLASS,
      which may not be enabled, causing build failures.
      
      Avoid the risk of missing and circular dependencies by guarding the
      phylib LED support itself in another Kconfig symbol that can only be
      enabled if the dependency is met.
      
      This could be made a hidden symbol and always enabled when both CONFIG_OF
      and CONFIG_LEDS_CLASS are reachable from the phylib, but there may be an
      advantage in having users see this option when they have a misconfigured
      kernel without built-in LED support.
      
      Fixes: 01e5b728 ("net: phy: Add a binding for PHY LEDs")
      Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
      Reviewed-by: default avatarAndrew Lunn <andrew@lunn.ch>
      Link: https://lore.kernel.org/r/20230420084624.3005701-1-arnd@kernel.orgSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      4bb7aac7
  2. 20 Apr, 2023 24 commits