1. 19 Sep, 2021 2 commits
    • Vladimir Oltean's avatar
      net: dsa: be compatible with masters which unregister on shutdown · 0650bf52
      Vladimir Oltean authored
      Lino reports that on his system with bcmgenet as DSA master and KSZ9897
      as a switch, rebooting or shutting down never works properly.
      
      What does the bcmgenet driver have special to trigger this, that other
      DSA masters do not? It has an implementation of ->shutdown which simply
      calls its ->remove implementation. Otherwise said, it unregisters its
      network interface on shutdown.
      
      This message can be seen in a loop, and it hangs the reboot process there:
      
      unregister_netdevice: waiting for eth0 to become free. Usage count = 3
      
      So why 3?
      
      A usage count of 1 is normal for a registered network interface, and any
      virtual interface which links itself as an upper of that will increment
      it via dev_hold. In the case of DSA, this is the call path:
      
      dsa_slave_create
      -> netdev_upper_dev_link
         -> __netdev_upper_dev_link
            -> __netdev_adjacent_dev_insert
               -> dev_hold
      
      So a DSA switch with 3 interfaces will result in a usage count elevated
      by two, and netdev_wait_allrefs will wait until they have gone away.
      
      Other stacked interfaces, like VLAN, watch NETDEV_UNREGISTER events and
      delete themselves, but DSA cannot just vanish and go poof, at most it
      can unbind itself from the switch devices, but that must happen strictly
      earlier compared to when the DSA master unregisters its net_device, so
      reacting on the NETDEV_UNREGISTER event is way too late.
      
      It seems that it is a pretty established pattern to have a driver's
      ->shutdown hook redirect to its ->remove hook, so the same code is
      executed regardless of whether the driver is unbound from the device, or
      the system is just shutting down. As Florian puts it, it is quite a big
      hammer for bcmgenet to unregister its net_device during shutdown, but
      having a common code path with the driver unbind helps ensure it is well
      tested.
      
      So DSA, for better or for worse, has to live with that and engage in an
      arms race of implementing the ->shutdown hook too, from all individual
      drivers, and do something sane when paired with masters that unregister
      their net_device there. The only sane thing to do, of course, is to
      unlink from the master.
      
      However, complications arise really quickly.
      
      The pattern of redirecting ->shutdown to ->remove is not unique to
      bcmgenet or even to net_device drivers. In fact, SPI controllers do it
      too (see dspi_shutdown -> dspi_remove), and presumably, I2C controllers
      and MDIO controllers do it too (this is something I have not researched
      too deeply, but even if this is not the case today, it is certainly
      plausible to happen in the future, and must be taken into consideration).
      
      Since DSA switches might be SPI devices, I2C devices, MDIO devices, the
      insane implication is that for the exact same DSA switch device, we
      might have both ->shutdown and ->remove getting called.
      
      So we need to do something with that insane environment. The pattern
      I've come up with is "if this, then not that", so if either ->shutdown
      or ->remove gets called, we set the device's drvdata to NULL, and in the
      other hook, we check whether the drvdata is NULL and just do nothing.
      This is probably not necessary for platform devices, just for devices on
      buses, but I would really insist for consistency among drivers, because
      when code is copy-pasted, it is not always copy-pasted from the best
      sources.
      
      So depending on whether the DSA switch's ->remove or ->shutdown will get
      called first, we cannot really guarantee even for the same driver if
      rebooting will result in the same code path on all platforms. But
      nonetheless, we need to do something minimally reasonable on ->shutdown
      too to fix the bug. Of course, the ->remove will do more (a full
      teardown of the tree, with all data structures freed, and this is why
      the bug was not caught for so long). The new ->shutdown method is kept
      separate from dsa_unregister_switch not because we couldn't have
      unregistered the switch, but simply in the interest of doing something
      quick and to the point.
      
      The big question is: does the DSA switch's ->shutdown get called earlier
      than the DSA master's ->shutdown? If not, there is still a risk that we
      might still trigger the WARN_ON in unregister_netdevice that says we are
      attempting to unregister a net_device which has uppers. That's no good.
      Although the reference to the master net_device won't physically go away
      even if DSA's ->shutdown comes afterwards, remember we have a dev_hold
      on it.
      
      The answer to that question lies in this comment above device_link_add:
      
       * A side effect of the link creation is re-ordering of dpm_list and the
       * devices_kset list by moving the consumer device and all devices depending
       * on it to the ends of these lists (that does not happen to devices that have
       * not been registered when this function is called).
      
      so the fact that DSA uses device_link_add towards its master is not
      exactly for nothing. device_shutdown() walks devices_kset from the back,
      so this is our guarantee that DSA's shutdown happens before the master's
      shutdown.
      
      Fixes: 2f1e8ea7 ("net: dsa: link interfaces with the DSA master to get rid of lockdep warnings")
      Link: https://lore.kernel.org/netdev/20210909095324.12978-1-LinoSanfilippo@gmx.de/Reported-by: default avatarLino Sanfilippo <LinoSanfilippo@gmx.de>
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Tested-by: default avatarAndrew Lunn <andrew@lunn.ch>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      0650bf52
    • Vladimir Oltean's avatar
      net: mdio: introduce a shutdown method to mdio device drivers · cf957997
      Vladimir Oltean authored
      MDIO-attached devices might have interrupts and other things that might
      need quiesced when we kexec into a new kernel. Things are even more
      creepy when those interrupt lines are shared, and in that case it is
      absolutely mandatory to disable all interrupt sources.
      
      Moreover, MDIO devices might be DSA switches, and DSA needs its own
      shutdown method to unlink from the DSA master, which is a new
      requirement that appeared after commit 2f1e8ea7 ("net: dsa: link
      interfaces with the DSA master to get rid of lockdep warnings").
      
      So introduce a ->shutdown method in the MDIO device driver structure.
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: default avatarAndrew Lunn <andrew@lunn.ch>
      Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      cf957997
  2. 17 Sep, 2021 3 commits
  3. 16 Sep, 2021 16 commits
    • Linus Torvalds's avatar
      Merge tag 'net-5.15-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net · fc0c0548
      Linus Torvalds authored
      Pull networking fixes from Jakub Kicinski:
       "Including fixes from bpf.
      
        Current release - regressions:
      
         - vhost_net: fix OoB on sendmsg() failure
      
         - mlx5: bridge, fix uninitialized variable usage
      
         - bnxt_en: fix error recovery regression
      
        Current release - new code bugs:
      
         - bpf, mm: fix lockdep warning triggered by stack_map_get_build_id_offset()
      
        Previous releases - regressions:
      
         - r6040: restore MDIO clock frequency after MAC reset
      
         - tcp: fix tp->undo_retrans accounting in tcp_sacktag_one()
      
         - dsa: flush switchdev workqueue before tearing down CPU/DSA ports
      
        Previous releases - always broken:
      
         - ptp: dp83640: don't define PAGE0, avoid compiler warning
      
         - igc: fix tunnel segmentation offloads
      
         - phylink: update SFP selected interface on advertising changes
      
         - stmmac: fix system hang caused by eee_ctrl_timer during suspend/resume
      
         - mlx5e: fix mutual exclusion between CQE compression and HW TS
      
        Misc:
      
         - bpf, cgroups: fix cgroup v2 fallback on v1/v2 mixed mode
      
         - sfc: fallback for lack of xdp tx queues
      
         - hns3: add option to turn off page pool feature"
      
      * tag 'net-5.15-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net: (67 commits)
        mlxbf_gige: clear valid_polarity upon open
        igc: fix tunnel offloading
        net/{mlx5|nfp|bnxt}: Remove unnecessary RTNL lock assert
        net: wan: wanxl: define CROSS_COMPILE_M68K
        selftests: nci: replace unsigned int with int
        net: dsa: flush switchdev workqueue before tearing down CPU/DSA ports
        Revert "net: phy: Uniform PHY driver access"
        net: dsa: destroy the phylink instance on any error in dsa_slave_phy_setup
        ptp: dp83640: don't define PAGE0
        bnx2x: Fix enabling network interfaces without VFs
        Revert "Revert "ipv4: fix memory leaks in ip_cmsg_send() callers""
        tcp: fix tp->undo_retrans accounting in tcp_sacktag_one()
        net-caif: avoid user-triggerable WARN_ON(1)
        bpf, selftests: Add test case for mixed cgroup v1/v2
        bpf, selftests: Add cgroup v1 net_cls classid helpers
        bpf, cgroups: Fix cgroup v2 fallback on v1/v2 mixed mode
        bpf: Add oversize check before call kvcalloc()
        net: hns3: fix the timing issue of VF clearing interrupt sources
        net: hns3: fix the exception when query imp info
        net: hns3: disable mac in flr process
        ...
      fc0c0548
    • Guenter Roeck's avatar
      net: 6pack: Fix tx timeout and slot time · 3c0d2a46
      Guenter Roeck authored
      tx timeout and slot time are currently specified in units of HZ.  On
      Alpha, HZ is defined as 1024.  When building alpha:allmodconfig, this
      results in the following error message.
      
        drivers/net/hamradio/6pack.c: In function 'sixpack_open':
        drivers/net/hamradio/6pack.c:71:41: error:
        	unsigned conversion from 'int' to 'unsigned char'
        	changes value from '256' to '0'
      
      In the 6PACK protocol, tx timeout is specified in units of 10 ms and
      transmitted over the wire:
      
          https://www.linux-ax25.org/wiki/6PACK
      
      Defining a value dependent on HZ doesn't really make sense, and
      presumably comes from the (very historical) situation where HZ was
      originally 100.
      
      Note that the SIXP_SLOTTIME use explicitly is about 10ms granularity:
      
              mod_timer(&sp->tx_t, jiffies + ((when + 1) * HZ) / 100);
      
      and the SIXP_TXDELAY walue is sent as a byte over the wire.
      Signed-off-by: default avatarGuenter Roeck <linux@roeck-us.net>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      3c0d2a46
    • Arnd Bergmann's avatar
      drm/rockchip: cdn-dp-core: Make cdn_dp_core_resume __maybe_unused · 040b8907
      Arnd Bergmann authored
      With the new static annotation, the compiler warns when the functions
      are actually unused:
      
         drivers/gpu/drm/rockchip/cdn-dp-core.c:1123:12: error: 'cdn_dp_resume' defined but not used [-Werror=unused-function]
          1123 | static int cdn_dp_resume(struct device *dev)
               |            ^~~~~~~~~~~~~
      
      Mark them __maybe_unused to suppress that warning as well.
      
      [ Not so 'new' static annotations any more, and I removed the part of
        the patch that added __maybe_unused to cdn_dp_suspend(), because it's
        used by the shutdown/remove code.
      
        So only the resume function ends up possibly unused if CONFIG_PM isn't
        set     - Linus ]
      
      Fixes: 7c49abb4 ("drm/rockchip: cdn-dp-core: Make cdn_dp_core_suspend/resume static")
      Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
      Reviewed-by: default avatarEnric Balletbo i Serra <enric.balletbo@collabora.com>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      040b8907
    • Guenter Roeck's avatar
      cpufreq: vexpress: Drop unused variable · b60cee5b
      Guenter Roeck authored
      arm:allmodconfig fails to build with the following error.
      
        drivers/cpufreq/vexpress-spc-cpufreq.c:454:13: error:
      					unused variable 'cur_cluster'
      
      Remove the unused variable.
      
      Fixes: bb8c26d9 ("cpufreq: vexpress: Set CPUFREQ_IS_COOLING_DEV flag")
      Cc: Viresh Kumar <viresh.kumar@linaro.org>
      Signed-off-by: default avatarGuenter Roeck <linux@roeck-us.net>
      Reviewed-by: default avatarKees Cook <keescook@chromium.org>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      b60cee5b
    • Guenter Roeck's avatar
      alpha: Declare virt_to_phys and virt_to_bus parameter as pointer to volatile · 35a3f4ef
      Guenter Roeck authored
      Some drivers pass a pointer to volatile data to virt_to_bus() and
      virt_to_phys(), and that works fine.  One exception is alpha.  This
      results in a number of compile errors such as
      
        drivers/net/wan/lmc/lmc_main.c: In function 'lmc_softreset':
        drivers/net/wan/lmc/lmc_main.c:1782:50: error:
      	passing argument 1 of 'virt_to_bus' discards 'volatile'
      	qualifier from pointer target type
      
        drivers/atm/ambassador.c: In function 'do_loader_command':
        drivers/atm/ambassador.c:1747:58: error:
      	passing argument 1 of 'virt_to_bus' discards 'volatile'
      	qualifier from pointer target type
      
      Declare the parameter of virt_to_phys and virt_to_bus as pointer to
      volatile to fix the problem.
      Signed-off-by: default avatarGuenter Roeck <linux@roeck-us.net>
      Acked-by: default avatarArnd Bergmann <arnd@arndb.de>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      35a3f4ef
    • Linus Torvalds's avatar
      3com 3c515: make it compile on 64-bit architectures · db71f8fb
      Linus Torvalds authored
      This driver isn't enabled most places because of the ISA config
      dependency, but alpha still has it.  And I think the 'Jensen' actually
      did have an ISA slot.
      
      However, it doesn't build cleanly, because the "Vortex bus master" code
      just casts the skb->data pointer to 'int':
      
              outl((int) (skb->data), ioaddr + Wn7_MasterAddr);
      
      which is all kinds of broken.  Even on a good old traditional PC/AT it
      would be broken because the high bits will be random kernel address
      bits, but presumably the hardware ignores those bits.  I mean, it's ISA.
      We're talking 16MB dma limits. The "good old days".
      
      Make the build happy with this kind of craziness by using the proper
      isa_virt_to_bus() handling that the full bus master code uses anyway
      (the Vortex bus mastering is a limited special case).
      
      Who knows, this might even work.
      Reported-by: default avatarGuenter Roeck <linux@roeck-us.net>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      db71f8fb
    • Linus Torvalds's avatar
      Merge tag 'for-5.15/parisc-4' of git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux · 5fe983d3
      Linus Torvalds authored
      Pull parisc fix from Helge Deller:
       "Fix a build warning when using the PAGE0 pointer"
      
      * tag 'for-5.15/parisc-4' of git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux:
        parisc: Use absolute_pointer() to define PAGE0
      5fe983d3
    • Linus Torvalds's avatar
      Merge tag 'm68k-for-v5.15-tag2' of git://git.kernel.org/pub/scm/linux/kernel/git/geert/linux-m68k · 077a6ccf
      Linus Torvalds authored
      Pull m68k fixes from Geert Uytterhoeven:
      
       - Warning fixes to mitigate CONFIG_WERROR=y
      
      * tag 'm68k-for-v5.15-tag2' of git://git.kernel.org/pub/scm/linux/kernel/git/geert/linux-m68k:
        m68k: mvme: Remove overdue #warnings in RTC handling
        m68k: Double cast io functions to unsigned long
      077a6ccf
    • David Thompson's avatar
      mlxbf_gige: clear valid_polarity upon open · ee8a9600
      David Thompson authored
      The network interface managed by the mlxbf_gige driver can
      get into a problem state where traffic does not flow.
      In this state, the interface will be up and enabled, but
      will stop processing received packets.  This problem state
      will happen if three specific conditions occur:
          1) driver has received more than (N * RxRingSize) packets but
             less than (N+1 * RxRingSize) packets, where N is an odd number
             Note: the command "ethtool -g <interface>" will display the
             current receive ring size, which currently defaults to 128
          2) the driver's interface was disabled via "ifconfig oob_net0 down"
             during the window described in #1.
          3) the driver's interface is re-enabled via "ifconfig oob_net0 up"
      
      This patch ensures that the driver's "valid_polarity" field is
      cleared during the open() method so that it always matches the
      receive polarity used by hardware.  Without this fix, the driver
      needs to be unloaded and reloaded to correct this problem state.
      
      Fixes: f92e1869 ("Add Mellanox BlueField Gigabit Ethernet driver")
      Reviewed-by: default avatarAsmaa Mnebhi <asmaa@nvidia.com>
      Signed-off-by: default avatarDavid Thompson <davthompson@nvidia.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      ee8a9600
    • Paolo Abeni's avatar
      igc: fix tunnel offloading · 40ee363c
      Paolo Abeni authored
      Checking tunnel offloading, it turns out that offloading doesn't work
      as expected.  The following script allows to reproduce the issue.
      Call it as `testscript DEVICE LOCALIP REMOTEIP NETMASK'
      
      === SNIP ===
      if [ $# -ne 4 ]
      then
        echo "Usage $0 DEVICE LOCALIP REMOTEIP NETMASK"
        exit 1
      fi
      DEVICE="$1"
      LOCAL_ADDRESS="$2"
      REMOTE_ADDRESS="$3"
      NWMASK="$4"
      echo "Driver: $(ethtool -i ${DEVICE} | awk '/^driver:/{print $2}') "
      ethtool -k "${DEVICE}" | grep tx-udp
      echo
      echo "Set up NIC and tunnel..."
      ip addr add "${LOCAL_ADDRESS}/${NWMASK}" dev "${DEVICE}"
      ip link set "${DEVICE}" up
      sleep 2
      ip link add vxlan1 type vxlan id 42 \
      		   remote "${REMOTE_ADDRESS}" \
      		   local "${LOCAL_ADDRESS}" \
      		   dstport 0 \
      		   dev "${DEVICE}"
      ip addr add fc00::1/64 dev vxlan1
      ip link set vxlan1 up
      sleep 2
      rm -f vxlan.pcap
      echo "Running tcpdump and iperf3..."
      ( nohup tcpdump -i any -w vxlan.pcap >/dev/null 2>&1 ) &
      sleep 2
      iperf3 -c fc00::2 >/dev/null
      pkill tcpdump
      echo
      echo -n "Max. Paket Size: "
      tcpdump -r vxlan.pcap -nnle 2>/dev/null \
      | grep "${LOCAL_ADDRESS}.*> ${REMOTE_ADDRESS}.*OTV" \
      | awk '{print $8}' | awk -F ':' '{print $1}' \
      | sort -n | tail -1
      echo
      ip link del vxlan1
      ip addr del ${LOCAL_ADDRESS}/${NWMASK} dev "${DEVICE}"
      === SNAP ===
      
      The expected outcome is
      
        Max. Paket Size: 64904
      
      This is what you see on igb, the code igc has been taken from.
      However, on igc the output is
      
        Max. Paket Size: 1516
      
      so the GSO aggregate packets are segmented by the kernel before calling
      igc_xmit_frame.  Inside the subsequent call to igc_tso, the check for
      skb_is_gso(skb) fails and the function returns prematurely.
      
      It turns out that this occurs because the feature flags aren't set
      entirely correctly in igc_probe.  In contrast to the original code
      from igb_probe, igc_probe neglects to set the flags required to allow
      tunnel offloading.
      
      Setting the same flags as igb fixes the issue on igc.
      
      Fixes: 34428dff ("igc: Add GSO partial support")
      Signed-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
      Tested-by: default avatarCorinna Vinschen <vinschen@redhat.com>
      Acked-by: default avatarSasha Neftin <sasha.neftin@intel.com>
      Tested-by: default avatarNechama Kraus <nechamax.kraus@linux.intel.com>
      Signed-off-by: default avatarTony Nguyen <anthony.l.nguyen@intel.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      40ee363c
    • Eli Cohen's avatar
      net/{mlx5|nfp|bnxt}: Remove unnecessary RTNL lock assert · 7c3a0a01
      Eli Cohen authored
      Remove the assert from the callback priv lookup function since it does
      not require RTNL lock and is already protected by flow_indr_block_lock.
      
      This will avoid warnings from being emitted to dmesg if the driver
      registers its callback after an ingress qdisc was created for a
      netdevice.
      
      The warnings started after the following patch was merged:
      commit 74fc4f82 ("net: Fix offloading indirect devices dependency on qdisc order creation")
      Signed-off-by: default avatarEli Cohen <elic@nvidia.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      7c3a0a01
    • Adam Borowski's avatar
      net: wan: wanxl: define CROSS_COMPILE_M68K · 84fb7dfc
      Adam Borowski authored
      It was used but never set.  The hardcoded value from before the dawn of
      time was non-standard; the usual name for cross-tools is $TRIPLET-$TOOL
      Signed-off-by: default avatarAdam Borowski <kilobyte@angband.pl>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      84fb7dfc
    • Xiang wangx's avatar
      selftests: nci: replace unsigned int with int · 98dc68f8
      Xiang wangx authored
      Should not use comparison of unsigned expressions < 0.
      Signed-off-by: default avatarXiang wangx <wangxiang@cdjrlc.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      98dc68f8
    • Helge Deller's avatar
      parisc: Use absolute_pointer() to define PAGE0 · 90cc7bed
      Helge Deller authored
      Use absolute_pointer() wrapper for PAGE0 to avoid this compiler warning:
      
        arch/parisc/kernel/setup.c: In function 'start_parisc':
        error: '__builtin_memcmp_eq' specified bound 8 exceeds source size 0
      Signed-off-by: default avatarHelge Deller <deller@gmx.de>
      Co-Developed-by: default avatarGuenter Roeck <linux@roeck-us.net>
      Suggested-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      90cc7bed
    • Linus Torvalds's avatar
      Merge tag 'hyperv-fixes-signed-20210915' of... · ff1ffd71
      Linus Torvalds authored
      Merge tag 'hyperv-fixes-signed-20210915' of git://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux
      
      Pull hyperv fixes from Wei Liu:
      
       - Fix kernel crash caused by uio driver (Vitaly Kuznetsov)
      
       - Remove on-stack cpumask from HV APIC code (Wei Liu)
      
      * tag 'hyperv-fixes-signed-20210915' of git://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux:
        x86/hyperv: remove on-stack cpumask from hv_send_ipi_mask_allbutself
        asm-generic/hyperv: provide cpumask_to_vpset_noself
        Drivers: hv: vmbus: Fix kernel crash upon unbinding a device from uio_hv_generic driver
      ff1ffd71
    • Linus Torvalds's avatar
      Merge tag 'rtc-5.15-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux · 453fa43c
      Linus Torvalds authored
      Pull RTC fix from Alexandre Belloni:
       "Fix a locking issue in the cmos rtc driver"
      
      * tag 'rtc-5.15-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux:
        rtc: cmos: Disable irq around direct invocation of cmos_interrupt()
      453fa43c
  4. 15 Sep, 2021 13 commits
    • Vladimir Oltean's avatar
      net: dsa: flush switchdev workqueue before tearing down CPU/DSA ports · a57d8c21
      Vladimir Oltean authored
      Sometimes when unbinding the mv88e6xxx driver on Turris MOX, these error
      messages appear:
      
      mv88e6085 d0032004.mdio-mii:12: port 1 failed to delete be:79:b4:9e:9e:96 vid 1 from fdb: -2
      mv88e6085 d0032004.mdio-mii:12: port 1 failed to delete be:79:b4:9e:9e:96 vid 0 from fdb: -2
      mv88e6085 d0032004.mdio-mii:12: port 1 failed to delete d8:58:d7:00:ca:6d vid 100 from fdb: -2
      mv88e6085 d0032004.mdio-mii:12: port 1 failed to delete d8:58:d7:00:ca:6d vid 1 from fdb: -2
      mv88e6085 d0032004.mdio-mii:12: port 1 failed to delete d8:58:d7:00:ca:6d vid 0 from fdb: -2
      
      (and similarly for other ports)
      
      What happens is that DSA has a policy "even if there are bugs, let's at
      least not leak memory" and dsa_port_teardown() clears the dp->fdbs and
      dp->mdbs lists, which are supposed to be empty.
      
      But deleting that cleanup code, the warnings go away.
      
      => the FDB and MDB lists (used for refcounting on shared ports, aka CPU
      and DSA ports) will eventually be empty, but are not empty by the time
      we tear down those ports. Aka we are deleting them too soon.
      
      The addresses that DSA complains about are host-trapped addresses: the
      local addresses of the ports, and the MAC address of the bridge device.
      
      The problem is that offloading those entries happens from a deferred
      work item scheduled by the SWITCHDEV_FDB_DEL_TO_DEVICE handler, and this
      races with the teardown of the CPU and DSA ports where the refcounting
      is kept.
      
      In fact, not only it races, but fundamentally speaking, if we iterate
      through the port list linearly, we might end up tearing down the shared
      ports even before we delete a DSA user port which has a bridge upper.
      
      So as it turns out, we need to first tear down the user ports (and the
      unused ones, for no better place of doing that), then the shared ports
      (the CPU and DSA ports). In between, we need to ensure that all work
      items scheduled by our switchdev handlers (which only run for user
      ports, hence the reason why we tear them down first) have finished.
      
      Fixes: 161ca59d ("net: dsa: reference count the MDB entries at the cross-chip notifier level")
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
      Link: https://lore.kernel.org/r/20210914134726.2305133-1-vladimir.oltean@nxp.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      a57d8c21
    • Vladimir Oltean's avatar
      Revert "net: phy: Uniform PHY driver access" · 301de697
      Vladimir Oltean authored
      This reverts commit 3ac8eed6, which did
      more than it said on the box, and not only it replaced to_phy_driver
      with phydev->drv, but it also removed the "!drv" check, without actually
      explaining why that is fine.
      
      That patch in fact breaks suspend/resume on any system which has PHY
      devices with no drivers bound.
      
      The stack trace is:
      
      Unable to handle kernel NULL pointer dereference at virtual address 00000000000000e8
      pc : mdio_bus_phy_suspend+0xd8/0xec
      lr : dpm_run_callback+0x38/0x90
      Call trace:
       mdio_bus_phy_suspend+0xd8/0xec
       dpm_run_callback+0x38/0x90
       __device_suspend+0x108/0x3cc
       dpm_suspend+0x140/0x210
       dpm_suspend_start+0x7c/0xa0
       suspend_devices_and_enter+0x13c/0x540
       pm_suspend+0x2a4/0x330
      
      Examples why that assumption is not fine:
      
      - There is an MDIO bus with a PHY device that doesn't have a specific
        PHY driver loaded, because mdiobus_register() automatically creates a
        PHY device for it but there is no specific PHY driver in the system.
        Normally under those circumstances, the generic PHY driver will be
        bound lazily to it (at phy_attach_direct time). But some Ethernet
        drivers attach to their PHY at .ndo_open time. Until then it, the
        to-be-driven-by-genphy PHY device will not have a driver. The blamed
        patch amounts to saying "you need to open all net devices before the
        system can suspend, to avoid the NULL pointer dereference".
      
      - There is any raw MDIO device which has 'plausible' values in the PHY
        ID registers 2 and 3, which is located on an MDIO bus whose driver
        does not set bus->phy_mask = ~0 (which prevents auto-scanning of PHY
        devices). An example could be a MAC's internal MDIO bus with PCS
        devices on it, for serial links such as SGMII. PHY devices will get
        created for those PCSes too, due to that MDIO bus auto-scanning, and
        although those PHY devices are not used, they do not bother anybody
        either. PCS devices are usually managed in Linux as raw MDIO devices.
        Nonetheless, they do not have a PHY driver, nor does anybody attempt
        to connect to them (because they are not a PHY), and therefore this
        patch breaks that.
      
      The goal itself of the patch is questionable, so I am going for a
      straight revert. to_phy_driver does not seem to have a need to be
      replaced by phydev->drv, in fact that might even trigger code paths
      which were not given too deep of a thought.
      
      For instance:
      
      phy_probe populates phydev->drv at the beginning, but does not clean it
      up on any error (including EPROBE_DEFER). So if the phydev driver
      requests probe deferral, phydev->drv will remain populated despite there
      being no driver bound.
      
      If a system suspend starts in between the initial probe deferral request
      and the subsequent probe retry, we will be calling the phydev->drv->suspend
      method, but _before_ any phydev->drv->probe call has succeeded.
      
      That is to say, if the phydev->drv is allocating any driver-private data
      structure in ->probe, it pretty much expects that data structure to be
      available in ->suspend. But it may not. That is a pretty insane
      environment to present to PHY drivers.
      
      In the code structure before the blamed patch, mdio_bus_phy_may_suspend
      would just say "no, don't suspend" to any PHY device which does not have
      a driver pointer _in_the_device_structure_ (not the phydev->drv). That
      would essentially ensure that ->suspend will never get called for a
      device that has not yet successfully completed probe. This is the code
      structure the patch is returning to, via the revert.
      
      Fixes: 3ac8eed6 ("net: phy: Uniform PHY driver access")
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Acked-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
      Link: https://lore.kernel.org/r/20210914140515.2311548-1-vladimir.oltean@nxp.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      301de697
    • Vladimir Oltean's avatar
      net: dsa: destroy the phylink instance on any error in dsa_slave_phy_setup · 6a52e733
      Vladimir Oltean authored
      DSA supports connecting to a phy-handle, and has a fallback to a non-OF
      based method of connecting to an internal PHY on the switch's own MDIO
      bus, if no phy-handle and no fixed-link nodes were present.
      
      The -ENODEV error code from the first attempt (phylink_of_phy_connect)
      is what triggers the second attempt (phylink_connect_phy).
      
      However, when the first attempt returns a different error code than
      -ENODEV, this results in an unbalance of calls to phylink_create and
      phylink_destroy by the time we exit the function. The phylink instance
      has leaked.
      
      There are many other error codes that can be returned by
      phylink_of_phy_connect. For example, phylink_validate returns -EINVAL.
      So this is a practical issue too.
      
      Fixes: aab9c406 ("net: dsa: Plug in PHYLINK support")
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
      Reviewed-by: default avatarRussell King (Oracle) <rmk+kernel@armlinux.org.uk>
      Link: https://lore.kernel.org/r/20210914134331.2303380-1-vladimir.oltean@nxp.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      6a52e733
    • Linus Torvalds's avatar
      qnx4: avoid stringop-overread errors · b7213ffa
      Linus Torvalds authored
      The qnx4 directory entries are 64-byte blocks that have different
      contents depending on the a status byte that is in the last byte of the
      block.
      
      In particular, a directory entry can be either a "link info" entry with
      a 48-byte name and pointers to the real inode information, or an "inode
      entry" with a smaller 16-byte name and the full inode information.
      
      But the code was written to always just treat the directory name as if
      it was part of that "inode entry", and just extend the name to the
      longer case if the status byte said it was a link entry.
      
      That work just fine and gives the right results, but now that gcc is
      tracking data structure accesses much more, the code can trigger a
      compiler error about using up to 48 bytes (the long name) in a structure
      that only has that shorter name in it:
      
         fs/qnx4/dir.c: In function ‘qnx4_readdir’:
         fs/qnx4/dir.c:51:32: error: ‘strnlen’ specified bound 48 exceeds source size 16 [-Werror=stringop-overread]
            51 |                         size = strnlen(de->di_fname, size);
               |                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
         In file included from fs/qnx4/qnx4.h:3,
                          from fs/qnx4/dir.c:16:
         include/uapi/linux/qnx4_fs.h:45:25: note: source object declared here
            45 |         char            di_fname[QNX4_SHORT_NAME_MAX];
               |                         ^~~~~~~~
      
      which is because the source code doesn't really make this whole "one of
      two different types" explicit.
      
      Fix this by introducing a very explicit union of the two types, and
      basically explaining to the compiler what is really going on.
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      b7213ffa
    • Linus Torvalds's avatar
      sparc: avoid stringop-overread errors · fc7c028d
      Linus Torvalds authored
      The sparc mdesc code does pointer games with 'struct mdesc_hdr', but
      didn't describe to the compiler how that header is then followed by the
      data that the header describes.
      
      As a result, gcc is now unhappy since it does stricter pointer range
      tracking, and doesn't understand about how these things work.  This
      results in various errors like:
      
          arch/sparc/kernel/mdesc.c: In function ‘mdesc_node_by_name’:
          arch/sparc/kernel/mdesc.c:647:22: error: ‘strcmp’ reading 1 or more bytes from a region of size 0 [-Werror=stringop-overread]
            647 |                 if (!strcmp(names + ep[ret].name_offset, name))
                |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      
      which are easily avoided by just describing 'struct mdesc_hdr' better,
      and making the node_block() helper function look into that unsized
      data[] that follows the header.
      
      This makes the sparc64 build happy again at least for my cross-compiler
      version (gcc version 11.2.1).
      
      Link: https://lore.kernel.org/lkml/CAHk-=wi4NW3NC0xWykkw=6LnjQD6D_rtRtxY9g8gQAJXtQMi8A@mail.gmail.com/
      Cc: Guenter Roeck <linux@roeck-us.net>
      Cc: David S. Miller <davem@davemloft.net>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      fc7c028d
    • Linus Torvalds's avatar
      Merge branch 'absolute-pointer' (patches from Guenter) · d6efd3f1
      Linus Torvalds authored
      Merge absolute_pointer macro series from Guenter Roeck:
       "Kernel test builds currently fail for several architectures with error
        messages such as the following.
      
        drivers/net/ethernet/i825xx/82596.c: In function 'i82596_probe':
        arch/m68k/include/asm/string.h:72:25: error:
              '__builtin_memcpy' reading 6 bytes from a region of size 0
                      [-Werror=stringop-overread]
      
        Such warnings may be reported by gcc 11.x for string and memory
        operations on fixed addresses if gcc's builtin functions are used for
        those operations.
      
        This series introduces absolute_pointer() to fix the problem.
        absolute_pointer() disassociates a pointer from its originating symbol
        type and context, and thus prevents gcc from making assumptions about
        pointers passed to memory operations"
      
      * emailed patches from Guenter Roeck <linux@roeck-us.net>:
        alpha: Use absolute_pointer to define COMMAND_LINE
        alpha: Move setup.h out of uapi
        net: i825xx: Use absolute_pointer for memcpy from fixed memory location
        compiler.h: Introduce absolute_pointer macro
      d6efd3f1
    • Guenter Roeck's avatar
      alpha: Use absolute_pointer to define COMMAND_LINE · ebdc20d7
      Guenter Roeck authored
      alpha:allmodconfig fails to build with the following error
      when using gcc 11.x.
      
        arch/alpha/kernel/setup.c: In function 'setup_arch':
        arch/alpha/kernel/setup.c:493:13: error:
      	'strcmp' reading 1 or more bytes from a region of size 0
      
      Avoid the problem by declaring COMMAND_LINE as absolute_pointer().
      Signed-off-by: default avatarGuenter Roeck <linux@roeck-us.net>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      ebdc20d7
    • Guenter Roeck's avatar
      alpha: Move setup.h out of uapi · 3cb8b153
      Guenter Roeck authored
      Most of the contents of setup.h have no value for userspace
      applications.  The file was probably moved to uapi accidentally.
      
      Keep the file in uapi to define the alpha-specific COMMAND_LINE_SIZE.
      Move all other defines to arch/alpha/include/asm/setup.h.
      Suggested-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      Signed-off-by: default avatarGuenter Roeck <linux@roeck-us.net>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      3cb8b153
    • Guenter Roeck's avatar
      net: i825xx: Use absolute_pointer for memcpy from fixed memory location · dff2d131
      Guenter Roeck authored
      gcc 11.x reports the following compiler warning/error.
      
        drivers/net/ethernet/i825xx/82596.c: In function 'i82596_probe':
        arch/m68k/include/asm/string.h:72:25: error:
      	'__builtin_memcpy' reading 6 bytes from a region of size 0 [-Werror=stringop-overread]
      
      Use absolute_pointer() to work around the problem.
      
      Cc: Geert Uytterhoeven <geert@linux-m68k.org>
      Signed-off-by: default avatarGuenter Roeck <linux@roeck-us.net>
      Reviewed-by: default avatarGeert Uytterhoeven <geert@linux-m68k.org>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      dff2d131
    • Guenter Roeck's avatar
      compiler.h: Introduce absolute_pointer macro · f6b5f1a5
      Guenter Roeck authored
      absolute_pointer() disassociates a pointer from its originating symbol
      type and context. Use it to prevent compiler warnings/errors such as
      
        drivers/net/ethernet/i825xx/82596.c: In function 'i82596_probe':
        arch/m68k/include/asm/string.h:72:25: error:
      	'__builtin_memcpy' reading 6 bytes from a region of size 0 [-Werror=stringop-overread]
      
      Such warnings may be reported by gcc 11.x for string and memory
      operations on fixed addresses.
      Suggested-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      Signed-off-by: default avatarGuenter Roeck <linux@roeck-us.net>
      Reviewed-by: default avatarGeert Uytterhoeven <geert@linux-m68k.org>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      f6b5f1a5
    • Masami Hiramatsu's avatar
      tools/bootconfig: Define memblock_free_ptr() to fix build error · 80be5998
      Masami Hiramatsu authored
      The lib/bootconfig.c file is shared with the 'bootconfig' tooling, and
      as a result, the changes incommit 77e02cf5 ("memblock: introduce
      saner 'memblock_free_ptr()' interface") need to also be reflected in the
      tooling header file.
      
      So define the new memblock_free_ptr() wrapper, and remove unused __pa()
      and memblock_free().
      
      Fixes: 77e02cf5 ("memblock: introduce saner 'memblock_free_ptr()' interface")
      Signed-off-by: default avatarMasami Hiramatsu <mhiramat@kernel.org>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      80be5998
    • Randy Dunlap's avatar
      ptp: dp83640: don't define PAGE0 · 7366c23f
      Randy Dunlap authored
      Building dp83640.c on arch/parisc/ produces a build warning for
      PAGE0 being redefined. Since the macro is not used in the dp83640
      driver, just make it a comment for documentation purposes.
      
      In file included from ../drivers/net/phy/dp83640.c:23:
      ../drivers/net/phy/dp83640_reg.h:8: warning: "PAGE0" redefined
          8 | #define PAGE0                     0x0000
                       from ../drivers/net/phy/dp83640.c:11:
      ../arch/parisc/include/asm/page.h:187: note: this is the location of the previous definition
        187 | #define PAGE0   ((struct zeropage *)__PAGE_OFFSET)
      
      Fixes: cb646e2b ("ptp: Added a clock driver for the National Semiconductor PHYTER.")
      Signed-off-by: default avatarRandy Dunlap <rdunlap@infradead.org>
      Reported-by: default avatarGeert Uytterhoeven <geert@linux-m68k.org>
      Cc: Richard Cochran <richard.cochran@omicron.at>
      Cc: John Stultz <john.stultz@linaro.org>
      Cc: Heiner Kallweit <hkallweit1@gmail.com>
      Cc: Russell King <linux@armlinux.org.uk>
      Reviewed-by: default avatarAndrew Lunn <andrew@lunn.ch>
      Link: https://lore.kernel.org/r/20210913220605.19682-1-rdunlap@infradead.orgSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      7366c23f
    • Adrian Bunk's avatar
      bnx2x: Fix enabling network interfaces without VFs · 52ce14c1
      Adrian Bunk authored
      This function is called to enable SR-IOV when available,
      not enabling interfaces without VFs was a regression.
      
      Fixes: 65161c35 ("bnx2x: Fix missing error code in bnx2x_iov_init_one()")
      Signed-off-by: default avatarAdrian Bunk <bunk@kernel.org>
      Reported-by: default avatarYunQiang Su <wzssyqa@gmail.com>
      Tested-by: default avatarYunQiang Su <wzssyqa@gmail.com>
      Cc: stable@vger.kernel.org
      Acked-by: default avatarShai Malin <smalin@marvell.com>
      Link: https://lore.kernel.org/r/20210912190523.27991-1-bunk@kernel.orgSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      52ce14c1
  5. 14 Sep, 2021 6 commits