24 Nov, 2019
1 commit
-
Rename the mac_link_state() method to mac_pcs_get_state() to make it
clear that it should be returning the MACs PCS current state, which
is used for inband negotiation rather than just reading back what the
MAC has been configured for. Update the documentation to explicitly
mention that this is for inband.We drop the return value as well; most of phylink doesn't check the
return value and it is not clear what it should do on error - instead
arrange for state->link to be false.Signed-off-by: Russell King
Signed-off-by: Jakub Kicinski
22 Nov, 2019
1 commit
-
This patch is to reuse ocelot functions as possible to enable PTP
clock and to support hardware timestamping on Felix.
On TX path, timestamping works on packet which requires timestamp.
The injection header will be configured accordingly, and skb clone
requires timestamp will be added into a list. The TX timestamp
is final handled in threaded interrupt handler when PTP timestamp
FIFO is ready.
On RX path, timestamping is always working. The RX timestamp could
be got from extraction header.Signed-off-by: Yangbo Lu
Signed-off-by: David S. Miller
17 Nov, 2019
2 commits
-
Lots of overlapping changes and parallel additions, stuff
like that.Signed-off-by: David S. Miller
-
This sequence of operations:
ip link set dev br0 type bridge vlan_filtering 1
bridge vlan del dev swp2 vid 1
ip link set dev br0 type bridge vlan_filtering 1
ip link set dev br0 type bridge vlan_filtering 0apparently fails with the message:
[ 31.305716] sja1105 spi0.1: Reset switch and programmed static config. Reason: VLAN filtering
[ 31.322161] sja1105 spi0.1: Couldn't determine PVID attributes (pvid 0)
[ 31.328939] sja1105 spi0.1: Failed to setup VLAN tagging for port 1: -2
[ 31.335599] ------------[ cut here ]------------
[ 31.340215] WARNING: CPU: 1 PID: 194 at net/switchdev/switchdev.c:157 switchdev_port_attr_set_now+0x9c/0xa4
[ 31.349981] br0: Commit of attribute (id=6) failed.
[ 31.354890] Modules linked in:
[ 31.357942] CPU: 1 PID: 194 Comm: ip Not tainted 5.4.0-rc6-01792-gf4f632e07665-dirty #2062
[ 31.366167] Hardware name: Freescale LS1021A
[ 31.370437] [] (unwind_backtrace) from [] (show_stack+0x10/0x14)
[ 31.378153] [] (show_stack) from [] (dump_stack+0xe0/0x10c)
[ 31.385437] [] (dump_stack) from [] (__warn+0xf4/0x10c)
[ 31.392373] [] (__warn) from [] (warn_slowpath_fmt+0x74/0xb8)
[ 31.399827] [] (warn_slowpath_fmt) from [] (switchdev_port_attr_set_now+0x9c/0xa4)
[ 31.409097] [] (switchdev_port_attr_set_now) from [] (__br_vlan_filter_toggle+0x6c/0x118)
[ 31.418971] [] (__br_vlan_filter_toggle) from [] (br_changelink+0xf8/0x518)
[ 31.427637] [] (br_changelink) from [] (__rtnl_newlink+0x3f4/0x76c)
[ 31.435613] [] (__rtnl_newlink) from [] (rtnl_newlink+0x44/0x60)
[ 31.443329] [] (rtnl_newlink) from [] (rtnetlink_rcv_msg+0x2cc/0x51c)
[ 31.451477] [] (rtnetlink_rcv_msg) from [] (netlink_rcv_skb+0xb8/0x110)
[ 31.459796] [] (netlink_rcv_skb) from [] (netlink_unicast+0x17c/0x1f8)
[ 31.468026] [] (netlink_unicast) from [] (netlink_sendmsg+0x2bc/0x3b4)
[ 31.476261] [] (netlink_sendmsg) from [] (___sys_sendmsg+0x230/0x250)
[ 31.484408] [] (___sys_sendmsg) from [] (__sys_sendmsg+0x50/0x8c)
[ 31.492209] [] (__sys_sendmsg) from [] (ret_fast_syscall+0x0/0x28)
[ 31.500090] Exception stack(0xedf47fa8 to 0xedf47ff0)
[ 31.505122] 7fa0: 00000002 b6f2e060 00000003 beabd6a4 00000000 00000000
[ 31.513265] 7fc0: 00000002 b6f2e060 5d6e3213 00000128 00000000 00000001 00000006 000619c4
[ 31.521405] 7fe0: 00086078 beabd658 0005edbc b6e7ce68The reason is the implementation of br_get_pvid:
static inline u16 br_get_pvid(const struct net_bridge_vlan_group *vg)
{
if (!vg)
return 0;smp_rmb();
return vg->pvid;
}Since VID 0 is an invalid pvid from the bridge's point of view, let's
add this check in dsa_8021q_restore_pvid to avoid restoring a pvid that
doesn't really exist.Fixes: 5f33183b7fdf ("net: dsa: tag_8021q: Restore bridge VLANs when enabling vlan_filtering")
Signed-off-by: Vladimir Oltean
Reviewed-by: Andrew Lunn
Signed-off-by: David S. Miller
16 Nov, 2019
1 commit
-
While it is entirely possible that this tagger format is in fact more
generic than just these 2 switch families, I don't have that knowledge.
The Seville switch in NXP T1040 has a similar frame format, but there
are enough differences (e.g. DEST field starts at bit 57 instead of 56)
that calling this file tag_vitesse.c is a bit of a stretch at the
moment. The frame format has been listed in a comment so that people who
add support for further Vitesse switches can rework this tagger while
keeping compatibility with Felix.The "ocelot" name was chosen instead of "felix" because even the Ocelot
switch can act as a DSA device when it is used in NPI mode, and the Felix
tagger format is almost identical. Currently it is only used for the
Felix switch embedded in the NXP LS1028A chip.The ABI for this tagger should be considered "not stable" at the moment.
The DSA tag is always placed before the Ethernet header and therefore,
we are using the long prefix for RX tags to avoid putting the DSA master
port in promiscuous mode. Once there will be an API in DSA for drivers
to request DSA masters to be in promiscuous mode unconditionally, we
will switch to the "no prefix" extraction frame header, which will save
16 padding bytes for each RX frame.Signed-off-by: Vladimir Oltean
Reviewed-by: Florian Fainelli
Signed-off-by: David S. Miller
13 Nov, 2019
1 commit
-
It is possible for a switch driver to use NET_DSA_TAG_8021Q as a valid
DSA tagging protocol since it registers itself as such, unfortunately
since there are not xmit or rcv functions provided, the lack of a xmit()
function will lead to a NPD in dsa_slave_xmit() to start with.net/dsa/tag_8021q.c is only comprised of a set of helper functions at
the moment, but is not a fully autonomous or functional tagging "driver"
(though it could become later on). We do not have any users of
NET_DSA_TAG_8021Q so now is a good time to make sure there are not
issues being encountered by making this file strictly a place holder for
helper functions.Reviewed-by: Vladimir Oltean
Signed-off-by: Florian Fainelli
Signed-off-by: David S. Miller
06 Nov, 2019
2 commits
-
Add wrappers around the devlink resource API, so that DSA drivers can
register and unregister devlink resources.Signed-off-by: Andrew Lunn
Acked-by: Jiri Pirko
Signed-off-by: David S. Miller -
The order in which the ports are deleted from the list and freed and the
call to dsa_switch_remove() is done is reversed, which leads to an
use after free condition. Reverse the two: first tear down the ports and
switch from the fabric, then free the ports associated with that switch
fabric.Fixes: 05f294a85235 ("net: dsa: allocate ports on touch")
Signed-off-by: Florian Fainelli
Reviewed-by: Vivien Didelot
Signed-off-by: David S. Miller
05 Nov, 2019
1 commit
-
Before this change of_get_phy_mode() returned an enum,
phy_interface_t. On error, -ENODEV etc, is returned. If the result of
the function is stored in a variable of type phy_interface_t, and the
compiler has decided to represent this as an unsigned int, comparision
with -ENODEV etc, is a signed vs unsigned comparision.Fix this problem by changing the API. Make the function return an
error, or 0 on success, and pass a pointer, of type phy_interface_t,
where the phy mode should be stored.v2:
Return with *interface set to PHY_INTERFACE_MODE_NA on error.
Add error checks to all users of of_get_phy_mode()
Fixup a few reverse christmas tree errors
Fixup a few slightly malformed reverse christmas treesv3:
Fix 0-day reported errors.Reported-by: Dan Carpenter
Signed-off-by: Andrew Lunn
Signed-off-by: David S. Miller
03 Nov, 2019
1 commit
-
The only slightly tricky merge conflict was the netdevsim because the
mutex locking fix overlapped a lot of driver reload reorganization.The rest were (relatively) trivial in nature.
Signed-off-by: David S. Miller
01 Nov, 2019
7 commits
-
Now that there's no restriction from the DSA core side regarding
the switch IDs and port numbers, only tag_8021q which is currently
reserving 3 bits for the switch ID and 4 bits for the port number, has
limitation for these values. Update their descriptions to reflect that.Signed-off-by: Vivien Didelot
Signed-off-by: David S. Miller -
Because there is no static array describing the links between switches
anymore, we have no reason to force a limitation of the index value
set by the device tree.Signed-off-by: Vivien Didelot
Signed-off-by: David S. Miller -
The DSA fabric setup code has been simplified a lot so get rid of
the dsa_tree_remove_switch, dsa_tree_add_switch and dsa_switch_add
helpers, and keep the code simple with only the dsa_switch_probe and
dsa_switch_remove functions.Signed-off-by: Vivien Didelot
Signed-off-by: David S. Miller -
Now that the DSA ports are listed in the switch fabric, there is
no need to store the dsa_switch structures from the drivers in the
fabric anymore. So get rid of the dst->ds static array.Signed-off-by: Vivien Didelot
Signed-off-by: David S. Miller -
The dsa_switch structure has no routing table specific data to setup,
so the switch fabric can directly walk its ports and initialize its
routing table from them.This allows us to remove the dsa_switch_setup_routing_table function.
Signed-off-by: Vivien Didelot
Signed-off-by: David S. Miller -
Drivers do not use the ds->rtable static arrays anymore, get rid of it.
Signed-off-by: Vivien Didelot
Signed-off-by: David S. Miller -
Implement a new list of DSA links in the switch fabric itself, to
provide an alterative to the ds->rtable static arrays.At the same time, provide a new dsa_routing_port() helper to abstract
the usage of ds->rtable in drivers. If there's no port to reach a
given device, return the first invalid port, ds->num_ports. This avoids
potential signedness errors or the need to define special values.Signed-off-by: Vivien Didelot
Signed-off-by: David S. Miller
31 Oct, 2019
1 commit
-
This patch adds glue logic to make pause settings per port
configurable vie ethtool.Signed-off-by: Heiner Kallweit
Signed-off-by: David S. Miller
29 Oct, 2019
2 commits
-
Add plumbing to allow DSA drivers to register parameters with devlink.
To keep with the abstraction, the DSA drivers pass the ds structure to
these helpers, and the DSA core then translates that to the devlink
structure associated to the device.Signed-off-by: Andrew Lunn
Signed-off-by: David S. Miller -
Currently ds->dev is dereferenced on the assignments of pdata and
np before ds->dev is null checked, hence there is a potential null
pointer dereference on ds->dev. Fix this by assigning pdata and
np after the ds->dev null pointer sanity check.Addresses-Coverity: ("Dereference before null check")
Fixes: 7e99e3470172 ("net: dsa: remove dsa_switch_alloc helper")
Signed-off-by: Colin Ian King
Reviewed-by: Andrew Lunn
Reviewed-by: Florian Fainelli
Reported-by: kbuild test robot
Reported-by: Dan Carpenter
Reviewed-by: Vivien Didelot
Signed-off-by: David S. Miller
25 Oct, 2019
1 commit
-
Some interface types could be nested.
(VLAN, BONDING, TEAM, MACSEC, MACVLAN, IPVLAN, VIRT_WIFI, VXLAN, etc..)
These interface types should set lockdep class because, without lockdep
class key, lockdep always warn about unexisting circular locking.In the current code, these interfaces have their own lockdep class keys and
these manage itself. So that there are so many duplicate code around the
/driver/net and /net/.
This patch adds new generic lockdep keys and some helper functions for it.This patch does below changes.
a) Add lockdep class keys in struct net_device
- qdisc_running, xmit, addr_list, qdisc_busylock
- these keys are used as dynamic lockdep key.
b) When net_device is being allocated, lockdep keys are registered.
- alloc_netdev_mqs()
c) When net_device is being free'd llockdep keys are unregistered.
- free_netdev()
d) Add generic lockdep key helper function
- netdev_register_lockdep_key()
- netdev_unregister_lockdep_key()
- netdev_update_lockdep_key()
e) Remove unnecessary generic lockdep macro and functions
f) Remove unnecessary lockdep code of each interfaces.After this patch, each interface modules don't need to maintain
their lockdep keys.Signed-off-by: Taehee Yoo
Signed-off-by: David S. Miller
23 Oct, 2019
11 commits
-
Now that ports are dynamically listed in the fabric, there is no need
to provide a special helper to allocate the dsa_switch structure. This
will give more flexibility to drivers to embed this structure as they
wish in their private structure.Signed-off-by: Vivien Didelot
Reviewed-by: Florian Fainelli
Signed-off-by: Jakub Kicinski -
Allocate the struct dsa_port the first time it is accessed with
dsa_port_touch, and remove the static dsa_port array from the
dsa_switch structure.Signed-off-by: Vivien Didelot
Reviewed-by: Florian Fainelli
Signed-off-by: Jakub Kicinski -
Use the new ports list instead of iterating over switches and their
ports when setting up the default CPU port. Unassign it on teardown.Now that we can iterate over multiple CPU ports, remove dst->cpu_dp.
At the same time, provide a better error message for CPU-less tree.
Signed-off-by: Vivien Didelot
Reviewed-by: Florian Fainelli
Signed-off-by: Jakub Kicinski -
Use the new ports list instead of iterating over switches and their
ports when looking up the first CPU port in the tree.Signed-off-by: Vivien Didelot
Reviewed-by: Florian Fainelli
Signed-off-by: Jakub Kicinski -
Now that we have a potential list of CPU ports, make use of it instead
of only configuring the master device of an unique CPU port.Signed-off-by: Vivien Didelot
Signed-off-by: Jakub Kicinski -
Use the new ports list instead of iterating over switches and their
ports to find a port from a given node.Signed-off-by: Vivien Didelot
Reviewed-by: Florian Fainelli
Signed-off-by: Jakub Kicinski -
Use the new ports list instead of accessing the dsa_switch array
of ports when iterating over DSA ports of a switch to set up the
routing table.Signed-off-by: Vivien Didelot
Reviewed-by: Florian Fainelli
Reviewed-by: Andrew Lunn
Signed-off-by: Jakub Kicinski -
Use the new ports list instead of iterating over switches and their
ports when setting up the switches and their ports.At the same time, provide setup states and messages for ports and
switches as it is done for the trees.Signed-off-by: Vivien Didelot
Signed-off-by: Jakub Kicinski -
Use the new ports list instead of iterating over switches and their
ports when looking for a slave device from a given master interface.Signed-off-by: Vivien Didelot
Reviewed-by: Florian Fainelli
Reviewed-by: Andrew Lunn
Signed-off-by: Jakub Kicinski -
Add a list of switch ports within the switch fabric. This will help the
lookup of a port inside the whole fabric, and it is the first step
towards supporting multiple CPU ports, before deprecating the usage of
the unique dst->cpu_dp pointer.In preparation for a future allocation of the dsa_port structures,
return -ENOMEM in case no structure is returned, even though this
error cannot be reached yet.Signed-off-by: Vivien Didelot
Reviewed-by: Florian Fainelli
Reviewed-by: Andrew Lunn
Signed-off-by: Jakub Kicinski -
Do not let the drivers access the ds->ports static array directly
while there is a dsa_to_port helper for this purpose.At the same time, un-const this helper since the SJA1105 driver
assigns the priv member of the returned dsa_port structure.Signed-off-by: Vivien Didelot
Reviewed-by: Florian Fainelli
Reviewed-by: Andrew Lunn
Signed-off-by: Jakub Kicinski
20 Oct, 2019
1 commit
-
If there are multiple switch trees on the device, only the last one
will be listed, because the arguments of list_add_tail are swapped.Fixes: 83c0afaec7b7 ("net: dsa: Add new binding implementation")
Signed-off-by: Vivien Didelot
Reviewed-by: Florian Fainelli
Signed-off-by: David S. Miller
03 Oct, 2019
1 commit
-
Currently this stack trace can be seen with CONFIG_DEBUG_ATOMIC_SLEEP=y:
[ 41.568348] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:909
[ 41.576757] in_atomic(): 1, irqs_disabled(): 0, pid: 208, name: ptp4l
[ 41.583212] INFO: lockdep is turned off.
[ 41.587123] CPU: 1 PID: 208 Comm: ptp4l Not tainted 5.3.0-rc6-01445-ge950f2d4bc7f-dirty #1827
[ 41.599873] [] (unwind_backtrace) from [] (show_stack+0x10/0x14)
[ 41.607584] [] (show_stack) from [] (dump_stack+0xd4/0x100)
[ 41.614863] [] (dump_stack) from [] (___might_sleep+0x1c8/0x2b4)
[ 41.622574] [] (___might_sleep) from [] (__mutex_lock+0x48/0xab8)
[ 41.630368] [] (__mutex_lock) from [] (mutex_lock_nested+0x1c/0x24)
[ 41.638340] [] (mutex_lock_nested) from [] (sja1105_static_config_reload+0x30/0x27c)
[ 41.647779] [] (sja1105_static_config_reload) from [] (sja1105_hwtstamp_set+0x108/0x1cc)
[ 41.657562] [] (sja1105_hwtstamp_set) from [] (dev_ifsioc+0x18c/0x330)
[ 41.665788] [] (dev_ifsioc) from [] (dev_ioctl+0x320/0x6e8)
[ 41.673064] [] (dev_ioctl) from [] (sock_ioctl+0x334/0x5e8)
[ 41.680340] [] (sock_ioctl) from [] (do_vfs_ioctl+0xb0/0xa10)
[ 41.687789] [] (do_vfs_ioctl) from [] (ksys_ioctl+0x34/0x58)
[ 41.695151] [] (ksys_ioctl) from [] (ret_fast_syscall+0x0/0x28)
[ 41.702768] Exception stack(0xe8495fa8 to 0xe8495ff0)
[ 41.707796] 5fa0: beff4a8c 00000001 00000011 000089b0 beff4a8c beff4a80
[ 41.715933] 5fc0: beff4a8c 00000001 0000000c 00000036 b6fa98c8 004e19c1 00000001 00000000
[ 41.724069] 5fe0: 004dcedc beff4a6c 004c0738 b6e7af4c
[ 41.729860] BUG: scheduling while atomic: ptp4l/208/0x00000002
[ 41.735682] INFO: lockdep is turned off.Enabling RX timestamping will logically disturb the fastpath (processing
of meta frames). Replace bool hwts_rx_en with a bit that is checked
atomically from the fastpath and temporarily unset from the sleepable
context during a change of the RX timestamping process (a destructive
operation anyways, requires switch reset).
If found unset, the fastpath (net/dsa/tag_sja1105.c) will just drop any
received meta frame and not take the meta_lock at all.Fixes: a602afd200f5 ("net: dsa: sja1105: Expose PTP timestamping ioctls to userspace")
Signed-off-by: Vladimir Oltean
Signed-off-by: David S. Miller
18 Sep, 2019
1 commit
-
Pull in bug fixes from 'net' tree for the merge window.
Signed-off-by: David S. Miller
17 Sep, 2019
2 commits
-
This is a preparation patch for the tc-taprio offload (and potentially
for other future offloads such as tc-mqprio).Instead of looking directly at skb->priority during xmit, let's get the
netdev queue and the queue-to-traffic-class mapping, and put the
resulting traffic class into the dsa_8021q PCP field. The switch is
configured with a 1-to-1 PCP-to-ingress-queue-to-egress-queue mapping
(see vlan_pmap in sja1105_main.c), so the effect is that we can inject
into a front-panel's egress traffic class through VLAN tagging from
Linux, completely transparently.Unfortunately the switch doesn't look at the VLAN PCP in the case of
management traffic to/from the CPU (link-local frames at
01-80-C2-xx-xx-xx or 01-1B-19-xx-xx-xx) so we can't alter the
transmission queue of this type of traffic on a frame-by-frame basis. It
is only selected through the "hostprio" setting which ATM is harcoded in
the driver to 7.Signed-off-by: Vladimir Oltean
Reviewed-by: Florian Fainelli
Signed-off-by: David S. Miller -
DSA currently handles shared block filters (for the classifier-action
qdisc) in the core due to what I believe are simply pragmatic reasons -
hiding the complexity from drivers and offerring a simple API for port
mirroring.Extend the dsa_slave_setup_tc function by passing all other qdisc
offloads to the driver layer, where the driver may choose what it
implements and how. DSA is simply a pass-through in this case.Signed-off-by: Vladimir Oltean
Acked-by: Kurt Kanzenbach
Reviewed-by: Florian Fainelli
Acked-by: Ilias Apalodimas
Signed-off-by: David S. Miller
16 Sep, 2019
1 commit
-
The DSA core, DSA taggers and DSA drivers all make use of
module_init(). Hence they get initialised at device_initcall() time.
The ordering is non-deterministic. It can be a DSA driver is bound to
a device before the needed tag driver has been initialised, resulting
in the message:No tagger for this switch
Rather than have this be fatal, return -EPROBE_DEFER so that it is
tried again later once all the needed drivers have been loaded.Fixes: d3b8c04988ca ("dsa: Add boilerplate helper to register DSA tag driver modules")
Signed-off-by: Andrew Lunn
Signed-off-by: David S. Miller
12 Sep, 2019
1 commit
-
Remove the superfluous NET_DSA_TAG_KSZ_COMMON and just use the existing
NET_DSA_TAG_KSZ. Update the description to mention the three switch
families it supports. No functional change.Signed-off-by: George McCollister
Reviewed-by: Marek Vasut
Reviewed-by: Florian Fainelli
Reviewed-by: Andrew Lunn
Signed-off-by: David S. Miller
03 Sep, 2019
1 commit
-
When a function such as dsa_slave_create fails, currently the following
stack trace can be seen:[ 2.038342] sja1105 spi0.1: Probed switch chip: SJA1105T
[ 2.054556] sja1105 spi0.1: Reset switch and programmed static config
[ 2.063837] sja1105 spi0.1: Enabled switch tagging
[ 2.068706] fsl-gianfar soc:ethernet@2d90000 eth2: error -19 setting up slave phy
[ 2.076371] ------------[ cut here ]------------
[ 2.080973] WARNING: CPU: 1 PID: 21 at net/core/devlink.c:6184 devlink_free+0x1b4/0x1c0
[ 2.088954] Modules linked in:
[ 2.092005] CPU: 1 PID: 21 Comm: kworker/1:1 Not tainted 5.3.0-rc6-01360-g41b52e38d2b6-dirty #1746
[ 2.100912] Hardware name: Freescale LS1021A
[ 2.105162] Workqueue: events deferred_probe_work_func
[ 2.110287] [] (unwind_backtrace) from [] (show_stack+0x10/0x14)
[ 2.117992] [] (show_stack) from [] (dump_stack+0xb4/0xc8)
[ 2.125180] [] (dump_stack) from [] (__warn+0xe0/0xf8)
[ 2.132018] [] (__warn) from [] (warn_slowpath_null+0x40/0x48)
[ 2.139549] [] (warn_slowpath_null) from [] (devlink_free+0x1b4/0x1c0)
[ 2.147772] [] (devlink_free) from [] (dsa_switch_teardown+0x60/0x6c)
[ 2.155907] [] (dsa_switch_teardown) from [] (dsa_register_switch+0x8e4/0xaa8)
[ 2.164821] [] (dsa_register_switch) from [] (sja1105_probe+0x21c/0x2ec)
[ 2.173216] [] (sja1105_probe) from [] (spi_drv_probe+0x80/0xa4)
[ 2.180920] [] (spi_drv_probe) from [] (really_probe+0x108/0x400)
[ 2.188711] [] (really_probe) from [] (driver_probe_device+0x78/0x1bc)
[ 2.196933] [] (driver_probe_device) from [] (bus_for_each_drv+0x58/0xb8)
[ 2.205414] [] (bus_for_each_drv) from [] (__device_attach+0xd0/0x168)
[ 2.213637] [] (__device_attach) from [] (bus_probe_device+0x84/0x8c)
[ 2.221772] [] (bus_probe_device) from [] (deferred_probe_work_func+0x84/0xc4)
[ 2.230686] [] (deferred_probe_work_func) from [] (process_one_work+0x218/0x510)
[ 2.239772] [] (process_one_work) from [] (worker_thread+0x2a8/0x5c0)
[ 2.247908] [] (worker_thread) from [] (kthread+0x148/0x150)
[ 2.255265] [] (kthread) from [] (ret_from_fork+0x14/0x2c)
[ 2.262444] Exception stack(0xea965fb0 to 0xea965ff8)
[ 2.267466] 5fa0: 00000000 00000000 00000000 00000000
[ 2.275598] 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 2.283729] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[ 2.290333] ---[ end trace ca5d506728a0581a ]---devlink_free is complaining right here:
WARN_ON(!list_empty(&devlink->port_list));
This happens because devlink_port_unregister is no longer done right
away in dsa_port_setup when a DSA_PORT_TYPE_USER has failed.
Vivien said about this change that:Also no need to call devlink_port_unregister from within dsa_port_setup
as this step is inconditionally handled by dsa_port_teardown on error.which is not really true. The devlink_port_unregister function _is_
being called unconditionally from within dsa_port_setup, but not for
this port that just failed, just for the previous ones which were set
up.ports_teardown:
for (i = 0; i < port; i++)
dsa_port_teardown(&ds->ports[i]);Initially I was tempted to fix this by extending the "for" loop to also
cover the port that failed during setup. But this could have potentially
unforeseen consequences unrelated to devlink_port or even other types of
ports than user ports, which I can't really test for. For example, if
for some reason devlink_port_register itself would fail, then
unconditionally unregistering it in dsa_port_teardown would not be a
smart idea. The list might go on.So just make dsa_port_setup undo the setup it had done upon failure, and
let the for loop undo the work of setting up the previous ports, which
are guaranteed to be brought up to a consistent state.Fixes: 955222ca5281 ("net: dsa: use a single switch statement for port setup")
Signed-off-by: Vladimir Oltean
Reviewed-by: Vivien Didelot
Signed-off-by: David S. Miller