<feed xmlns='http://www.w3.org/2005/Atom'>
<title>user/sven/linux.git/net/dsa, branch v5.10.212</title>
<subtitle>Linux Kernel
</subtitle>
<id>https://git.stealer.net/cgit.cgi/user/sven/linux.git/atom?h=v5.10.212</id>
<link rel='self' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/atom?h=v5.10.212'/>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/'/>
<updated>2023-07-27T06:44:10Z</updated>
<entry>
<title>net: dsa: tag_sja1105: fix MAC DA patching from meta frames</title>
<updated>2023-07-27T06:44:10Z</updated>
<author>
<name>Vladimir Oltean</name>
<email>vladimir.oltean@nxp.com</email>
</author>
<published>2023-07-03T22:05:44Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=9f5548e4214d260ed0c9ec0394d7b4bae4015029'/>
<id>urn:sha1:9f5548e4214d260ed0c9ec0394d7b4bae4015029</id>
<content type='text'>
[ Upstream commit 1dcf6efd5f0c1f4496b3ef7ec5a7db104a53b38c ]

The SJA1105 manual says that at offset 4 into the meta frame payload we
have "MAC destination byte 2" and at offset 5 we have "MAC destination
byte 1". These are counted from the LSB, so byte 1 is h_dest[ETH_HLEN-2]
aka h_dest[4] and byte 2 is h_dest[ETH_HLEN-3] aka h_dest[3].

The sja1105_meta_unpack() function decodes these the other way around,
so a frame with MAC DA 01:80:c2:11:22:33 is received by the network
stack as having 01:80:c2:22:11:33.

Fixes: e53e18a6fe4d ("net: dsa: sja1105: Receive and decode meta frames")
Signed-off-by: Vladimir Oltean &lt;vladimir.oltean@nxp.com&gt;
Reviewed-by: Simon Horman &lt;simon.horman@corigine.com&gt;
Reviewed-by: Florian Fainelli &lt;florian.fainelli@broadcom.com&gt;
Signed-off-by: David S. Miller &lt;davem@davemloft.net&gt;
Signed-off-by: Sasha Levin &lt;sashal@kernel.org&gt;
</content>
</entry>
<entry>
<title>net: dsa: ksz: Check return value</title>
<updated>2022-12-14T10:32:01Z</updated>
<author>
<name>Artem Chernyshev</name>
<email>artem.chernyshev@red-soft.ru</email>
</author>
<published>2022-12-01T14:00:30Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=bccda3ad0748958473eb730e29c0487007a98832'/>
<id>urn:sha1:bccda3ad0748958473eb730e29c0487007a98832</id>
<content type='text'>
[ Upstream commit 3d8fdcbf1f42e2bb9ae8b8c0b6f202278c788a22 ]

Return NULL if we got unexpected value from skb_trim_rcsum()
in ksz_common_rcv()

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: bafe9ba7d908 ("net: dsa: ksz: Factor out common tag code")
Signed-off-by: Artem Chernyshev &lt;artem.chernyshev@red-soft.ru&gt;
Reviewed-by: Vladimir Oltean &lt;olteanv@gmail.com&gt;
Reviewed-by: Florian Fainelli &lt;f.fainelli@gmail.com&gt;
Link: https://lore.kernel.org/r/20221201140032.26746-1-artem.chernyshev@red-soft.ru
Signed-off-by: Jakub Kicinski &lt;kuba@kernel.org&gt;
Signed-off-by: Sasha Levin &lt;sashal@kernel.org&gt;
</content>
</entry>
<entry>
<title>net: dsa: Add missing of_node_put() in dsa_port_link_register_of</title>
<updated>2022-05-09T07:05:02Z</updated>
<author>
<name>Miaoqian Lin</name>
<email>linmq006@gmail.com</email>
</author>
<published>2022-04-20T11:04:08Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=72ae15d5ce5fd37f3c36fd0a5a3de3cf266c11e8'/>
<id>urn:sha1:72ae15d5ce5fd37f3c36fd0a5a3de3cf266c11e8</id>
<content type='text'>
[ Upstream commit fc06b2867f4cea543505acfb194c2be4ebf0c7d3 ]

The device_node pointer is returned by of_parse_phandle()  with refcount
incremented. We should use of_node_put() on it when done.
of_node_put() will check for NULL value.

Fixes: a20f997010c4 ("net: dsa: Don't instantiate phylink for CPU/DSA ports unless needed")
Signed-off-by: Miaoqian Lin &lt;linmq006@gmail.com&gt;
Signed-off-by: David S. Miller &lt;davem@davemloft.net&gt;
Signed-off-by: Sasha Levin &lt;sashal@kernel.org&gt;
</content>
</entry>
<entry>
<title>net: dsa: Add missing of_node_put() in dsa_port_parse_of</title>
<updated>2022-03-23T08:13:28Z</updated>
<author>
<name>Miaoqian Lin</name>
<email>linmq006@gmail.com</email>
</author>
<published>2022-03-16T08:26:02Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=ba50073cf4af619f8cec3b50622adbf3ff5e9abd'/>
<id>urn:sha1:ba50073cf4af619f8cec3b50622adbf3ff5e9abd</id>
<content type='text'>
[ Upstream commit cb0b430b4e3acc88c85e0ad2e25f2a25a5765262 ]

The device_node pointer is returned by of_parse_phandle()  with refcount
incremented. We should use of_node_put() on it when done.

Fixes: 6d4e5c570c2d ("net: dsa: get port type at parse time")
Signed-off-by: Miaoqian Lin &lt;linmq006@gmail.com&gt;
Link: https://lore.kernel.org/r/20220316082602.10785-1-linmq006@gmail.com
Signed-off-by: Paolo Abeni &lt;pabeni@redhat.com&gt;
Signed-off-by: Sasha Levin &lt;sashal@kernel.org&gt;
</content>
</entry>
<entry>
<title>net: dsa: don't allocate the slave_mii_bus using devres</title>
<updated>2021-09-30T08:11:02Z</updated>
<author>
<name>Vladimir Oltean</name>
<email>vladimir.oltean@nxp.com</email>
</author>
<published>2021-09-20T21:42:08Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=43c880b860c7a7dbc8361e2e4a124cd5337d4c5f'/>
<id>urn:sha1:43c880b860c7a7dbc8361e2e4a124cd5337d4c5f</id>
<content type='text'>
[ Upstream commit 5135e96a3dd2f4555ae6981c3155a62bcf3227f6 ]

The Linux device model permits both the -&gt;shutdown and -&gt;remove driver
methods to get called during a shutdown procedure. Example: a DSA switch
which sits on an SPI bus, and the SPI bus driver calls this on its
-&gt;shutdown method:

spi_unregister_controller
-&gt; device_for_each_child(&amp;ctlr-&gt;dev, NULL, __unregister);
   -&gt; spi_unregister_device(to_spi_device(dev));
      -&gt; device_del(&amp;spi-&gt;dev);

So this is a simple pattern which can theoretically appear on any bus,
although the only other buses on which I've been able to find it are
I2C:

i2c_del_adapter
-&gt; device_for_each_child(&amp;adap-&gt;dev, NULL, __unregister_client);
   -&gt; i2c_unregister_device(client);
      -&gt; device_unregister(&amp;client-&gt;dev);

The implication of this pattern is that devices on these buses can be
unregistered after having been shut down. The drivers for these devices
might choose to return early either from -&gt;remove or -&gt;shutdown if the
other callback has already run once, and they might choose that the
-&gt;shutdown method should only perform a subset of the teardown done by
-&gt;remove (to avoid unnecessary delays when rebooting).

So in other words, the device driver may choose on -&gt;remove to not
do anything (therefore to not unregister an MDIO bus it has registered
on -&gt;probe), because this -&gt;remove is actually triggered by the
device_shutdown path, and its -&gt;shutdown method has already run and done
the minimally required cleanup.

This used to be fine until the blamed commit, but now, the following
BUG_ON triggers:

void mdiobus_free(struct mii_bus *bus)
{
	/* For compatibility with error handling in drivers. */
	if (bus-&gt;state == MDIOBUS_ALLOCATED) {
		kfree(bus);
		return;
	}

	BUG_ON(bus-&gt;state != MDIOBUS_UNREGISTERED);
	bus-&gt;state = MDIOBUS_RELEASED;

	put_device(&amp;bus-&gt;dev);
}

In other words, there is an attempt to free an MDIO bus which was not
unregistered. The attempt to free it comes from the devres release
callbacks of the SPI device, which are executed after the device is
unregistered.

I'm not saying that the fact that MDIO buses allocated using devres
would automatically get unregistered wasn't strange. I'm just saying
that the commit didn't care about auditing existing call paths in the
kernel, and now, the following code sequences are potentially buggy:

(a) devm_mdiobus_alloc followed by plain mdiobus_register, for a device
    located on a bus that unregisters its children on shutdown. After
    the blamed patch, either both the alloc and the register should use
    devres, or none should.

(b) devm_mdiobus_alloc followed by plain mdiobus_register, and then no
    mdiobus_unregister at all in the remove path. After the blamed
    patch, nobody unregisters the MDIO bus anymore, so this is even more
    buggy than the previous case which needs a specific bus
    configuration to be seen, this one is an unconditional bug.

In this case, DSA falls into category (a), it tries to be helpful and
registers an MDIO bus on behalf of the switch, which might be on such a
bus. I've no idea why it does it under devres.

It does this on probe:

	if (!ds-&gt;slave_mii_bus &amp;&amp; ds-&gt;ops-&gt;phy_read)
		alloc and register mdio bus

and this on remove:

	if (ds-&gt;slave_mii_bus &amp;&amp; ds-&gt;ops-&gt;phy_read)
		unregister mdio bus

I _could_ imagine using devres because the condition used on remove is
different than the condition used on probe. So strictly speaking, DSA
cannot determine whether the ds-&gt;slave_mii_bus it sees on remove is the
ds-&gt;slave_mii_bus that _it_ has allocated on probe. Using devres would
have solved that problem. But nonetheless, the existing code already
proceeds to unregister the MDIO bus, even though it might be
unregistering an MDIO bus it has never registered. So I can only guess
that no driver that implements ds-&gt;ops-&gt;phy_read also allocates and
registers ds-&gt;slave_mii_bus itself.

So in that case, if unregistering is fine, freeing must be fine too.

Stop using devres and free the MDIO bus manually. This will make devres
stop attempting to free a still registered MDIO bus on -&gt;shutdown.

Fixes: ac3a68d56651 ("net: phy: don't abuse devres in devm_mdiobus_register()")
Reported-by: Lino Sanfilippo &lt;LinoSanfilippo@gmx.de&gt;
Signed-off-by: Vladimir Oltean &lt;vladimir.oltean@nxp.com&gt;
Reviewed-by: Florian Fainelli &lt;f.fainelli@gmail.com&gt;
Tested-by: Lino Sanfilippo &lt;LinoSanfilippo@gmx.de&gt;
Reviewed-by: Andrew Lunn &lt;andrew@lunn.ch&gt;
Signed-off-by: David S. Miller &lt;davem@davemloft.net&gt;
Signed-off-by: Sasha Levin &lt;sashal@kernel.org&gt;
</content>
</entry>
<entry>
<title>net: dsa: tag_rtl4_a: Fix egress tags</title>
<updated>2021-09-22T10:28:04Z</updated>
<author>
<name>Linus Walleij</name>
<email>linus.walleij@linaro.org</email>
</author>
<published>2021-08-31T18:50:50Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=5711ced58eb4ad37b3b71409294505abe572e26b'/>
<id>urn:sha1:5711ced58eb4ad37b3b71409294505abe572e26b</id>
<content type='text'>
[ Upstream commit 0e90dfa7a8d817db755c7b5d89d77b9c485e4180 ]

I noticed that only port 0 worked on the RTL8366RB since we
started to use custom tags.

It turns out that the format of egress custom tags is actually
different from ingress custom tags. While the lower bits just
contain the port number in ingress tags, egress tags need to
indicate destination port by setting the bit for the
corresponding port.

It was working on port 0 because port 0 added 0x00 as port
number in the lower bits, and if you do this the packet appears
at all ports, including the intended port. Ooops.

Fix this and all ports work again. Use the define for shifting
the "type A" into place while we're at it.

Tested on the D-Link DIR-685 by sending traffic to each of
the ports in turn. It works.

Fixes: 86dd9868b878 ("net: dsa: tag_rtl4_a: Support also egress tags")
Cc: DENG Qingfang &lt;dqfext@gmail.com&gt;
Cc: Mauri Sandberg &lt;sandberg@mailfence.com&gt;
Signed-off-by: Linus Walleij &lt;linus.walleij@linaro.org&gt;
Reviewed-by: Florian Fainelli &lt;f.fainelli@gmail.com&gt;
Signed-off-by: David S. Miller &lt;davem@davemloft.net&gt;
Signed-off-by: Sasha Levin &lt;sashal@kernel.org&gt;
</content>
</entry>
<entry>
<title>net: dsa: destroy the phylink instance on any error in dsa_slave_phy_setup</title>
<updated>2021-09-22T10:27:58Z</updated>
<author>
<name>Vladimir Oltean</name>
<email>vladimir.oltean@nxp.com</email>
</author>
<published>2021-09-14T13:43:31Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=cf6f29bb2c18be0aaa54d85b5d58f6a9759bcbf1'/>
<id>urn:sha1:cf6f29bb2c18be0aaa54d85b5d58f6a9759bcbf1</id>
<content type='text'>
commit 6a52e73368038f47f6618623d75061dc263b26ae upstream.

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: aab9c4067d23 ("net: dsa: Plug in PHYLINK support")
Signed-off-by: Vladimir Oltean &lt;vladimir.oltean@nxp.com&gt;
Reviewed-by: Florian Fainelli &lt;f.fainelli@gmail.com&gt;
Reviewed-by: Russell King (Oracle) &lt;rmk+kernel@armlinux.org.uk&gt;
Link: https://lore.kernel.org/r/20210914134331.2303380-1-vladimir.oltean@nxp.com
Signed-off-by: Jakub Kicinski &lt;kuba@kernel.org&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</content>
</entry>
<entry>
<title>net: dsa: properly check for the bridge_leave methods in dsa_switch_bridge_leave()</title>
<updated>2021-07-25T12:36:20Z</updated>
<author>
<name>Vladimir Oltean</name>
<email>vladimir.oltean@nxp.com</email>
</author>
<published>2021-07-13T09:40:21Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=66c73f187d1ecfd8ac24802f5edbc1f994f9a97c'/>
<id>urn:sha1:66c73f187d1ecfd8ac24802f5edbc1f994f9a97c</id>
<content type='text'>
commit bcb9928a155444dbd212473e60241ca0a7f641e1 upstream.

This was not caught because there is no switch driver which implements
the .port_bridge_join but not .port_bridge_leave method, but it should
nonetheless be fixed, as in certain conditions (driver development) it
might lead to NULL pointer dereference.

Fixes: f66a6a69f97a ("net: dsa: permit cross-chip bridging between all trees in the system")
Signed-off-by: Vladimir Oltean &lt;vladimir.oltean@nxp.com&gt;
Signed-off-by: David S. Miller &lt;davem@davemloft.net&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</content>
</entry>
<entry>
<title>net: dsa: tag_8021q: fix the VLAN IDs used for encoding sub-VLANs</title>
<updated>2021-06-10T11:39:17Z</updated>
<author>
<name>Vladimir Oltean</name>
<email>vladimir.oltean@nxp.com</email>
</author>
<published>2021-05-31T10:20:45Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=09fdb6747b7ed3bc4da720301de52ac7b159214a'/>
<id>urn:sha1:09fdb6747b7ed3bc4da720301de52ac7b159214a</id>
<content type='text'>
[ Upstream commit 4ef8d857b5f494e62bce9085031563fda35f9563 ]

When using sub-VLANs in the range of 1-7, the resulting value from:

	rx_vid = dsa_8021q_rx_vid_subvlan(ds, port, subvlan);

is wrong according to the description from tag_8021q.c:

 | 11  | 10  |  9  |  8  |  7  |  6  |  5  |  4  |  3  |  2  |  1  |  0  |
 +-----------+-----+-----------------+-----------+-----------------------+
 |    DIR    | SVL |    SWITCH_ID    |  SUBVLAN  |          PORT         |
 +-----------+-----+-----------------+-----------+-----------------------+

For example, when ds-&gt;index == 0, port == 3 and subvlan == 1,
dsa_8021q_rx_vid_subvlan() returns 1027, same as it returns for
subvlan == 0, but it should have returned 1043.

This is because the low portion of the subvlan bits are not masked
properly when writing into the 12-bit VLAN value. They are masked into
bits 4:3, but they should be masked into bits 5:4.

Fixes: 3eaae1d05f2b ("net: dsa: tag_8021q: support up to 8 VLANs per port using sub-VLANs")
Signed-off-by: Vladimir Oltean &lt;vladimir.oltean@nxp.com&gt;
Signed-off-by: David S. Miller &lt;davem@davemloft.net&gt;
Signed-off-by: Sasha Levin &lt;sashal@kernel.org&gt;
</content>
</entry>
<entry>
<title>net: dsa: fix error code getting shifted with 4 in dsa_slave_get_sset_count</title>
<updated>2021-06-03T07:00:45Z</updated>
<author>
<name>Vladimir Oltean</name>
<email>vladimir.oltean@nxp.com</email>
</author>
<published>2021-05-09T19:33:38Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=d7932e68520e337be97192b41f320f710996ec77'/>
<id>urn:sha1:d7932e68520e337be97192b41f320f710996ec77</id>
<content type='text'>
[ Upstream commit b94cbc909f1d80378a1f541968309e5c1178c98b ]

DSA implements a bunch of 'standardized' ethtool statistics counters,
namely tx_packets, tx_bytes, rx_packets, rx_bytes. So whatever the
hardware driver returns in .get_sset_count(), we need to add 4 to that.

That is ok, except that .get_sset_count() can return a negative error
code, for example:

b53_get_sset_count
-&gt; phy_ethtool_get_sset_count
   -&gt; return -EIO

-EIO is -5, and with 4 added to it, it becomes -1, aka -EPERM. One can
imagine that certain error codes may even become positive, although
based on code inspection I did not see instances of that.

Check the error code first, if it is negative return it as-is.

Based on a similar patch for dsa_master_get_strings from Dan Carpenter:
https://patchwork.kernel.org/project/netdevbpf/patch/YJaSe3RPgn7gKxZv@mwanda/

Fixes: 91da11f870f0 ("net: Distributed Switch Architecture protocol support")
Signed-off-by: Vladimir Oltean &lt;vladimir.oltean@nxp.com&gt;
Reviewed-by: Florian Fainelli &lt;f.fainelli@gmail.com&gt;
Reviewed-by: Andrew Lunn &lt;andrew@lunn.ch&gt;
Signed-off-by: David S. Miller &lt;davem@davemloft.net&gt;
Signed-off-by: Sasha Levin &lt;sashal@kernel.org&gt;
</content>
</entry>
</feed>
