From: "Marek Behún" <kabel@kernel.org>
To: netdev@vger.kernel.org, Andrew Lunn <andrew@lunn.ch>,
Florian Fainelli <f.fainelli@gmail.com>,
Vladimir Oltean <olteanv@gmail.com>
Cc: "Marek Behún" <kabel@kernel.org>
Subject: [PATCH net-next 2/2] net: dsa: update the unicast MAC address when changing conduit
Date: Mon, 29 Apr 2024 18:36:27 +0200 [thread overview]
Message-ID: <20240429163627.16031-3-kabel@kernel.org> (raw)
In-Reply-To: <20240429163627.16031-1-kabel@kernel.org>
DSA exhibits different behavior regarding the primary unicast MAC
address stored in port standalone FDB and the conduit device UC
database while the interface is down vs up.
If we put a switch port down while changing the conduit with
ip link set sw0p0 down
ip link set sw0p0 type dsa conduit conduit1
ip link set sw0p0 up
we delete the address in dsa_user_close() and install the (possibly
different) address dsa_user_open().
But when changing the conduit on the fly, the old address is not
deleted and the new one is not installed.
Since we explicitly want to support live-changing the conduit, uninstall
the old address before the dsa_port_change_conduit() call and install
the (possibly different) new one afterwards.
Because the dsa_user_change_conduit() call tries to smoothly restore the
old conduit if anything fails while setting new one (except the MTU
change), this leaves us with the question about what to do if the
installation of the new address fails. Since we have already deleted the
old address, we can expect that restoring the old address would also fail,
and thus we can't revert the conduit change correctly. I have therefore
decided to treat it as a fatal error printed into the kernel log.
Fixes: 95f510d0b792 ("net: dsa: allow the DSA master to be seen and changed through rtnetlink")
Signed-off-by: Marek Behún <kabel@kernel.org>
---
net/dsa/user.c | 45 +++++++++++++++++++++++++++++++++++++--------
1 file changed, 37 insertions(+), 8 deletions(-)
diff --git a/net/dsa/user.c b/net/dsa/user.c
index b1d8d1827f91..70d7be1b6a79 100644
--- a/net/dsa/user.c
+++ b/net/dsa/user.c
@@ -2767,9 +2767,37 @@ int dsa_user_change_conduit(struct net_device *dev, struct net_device *conduit,
if (err)
goto out_revert_old_conduit_unlink;
+ /* If live-changing, we also need to uninstall the user device address
+ * from the port FDB and the conduit interface. This has to be done
+ * before the conduit is changed.
+ */
+ if (dev->flags & IFF_UP)
+ dsa_user_host_uc_uninstall(dev);
+
err = dsa_port_change_conduit(dp, conduit, extack);
if (err)
- goto out_revert_conduit_link;
+ goto out_revert_host_address;
+
+ /* If the port doesn't have its own MAC address and relies on the DSA
+ * conduit's one, inherit it again from the new DSA conduit.
+ */
+ if (is_zero_ether_addr(dp->mac))
+ eth_hw_addr_inherit(dev, conduit);
+
+ /* If live-changing, we need to install the user device address to the
+ * port FDB and the conduit interface. Since the device address needs to
+ * be installed towards the new conduit in the port FDB, this needs to
+ * be done after the conduit is changed.
+ */
+ if (dev->flags & IFF_UP) {
+ err = dsa_user_host_uc_install(dev, dev->dev_addr);
+ if (err) {
+ netdev_err(dev,
+ "fatal error installing new host address: %pe\n",
+ ERR_PTR(err));
+ return err;
+ }
+ }
/* Update the MTU of the new CPU port through cross-chip notifiers */
err = dsa_user_change_mtu(dev, dev->mtu);
@@ -2779,15 +2807,16 @@ int dsa_user_change_conduit(struct net_device *dev, struct net_device *conduit,
ERR_PTR(err));
}
- /* If the port doesn't have its own MAC address and relies on the DSA
- * conduit's one, inherit it again from the new DSA conduit.
- */
- if (is_zero_ether_addr(dp->mac))
- eth_hw_addr_inherit(dev, conduit);
-
return 0;
-out_revert_conduit_link:
+out_revert_host_address:
+ if (dev->flags & IFF_UP) {
+ err = dsa_user_host_uc_install(dev, dev->dev_addr);
+ if (err)
+ netdev_err(dev,
+ "fatal error restoring old host address: %pe\n",
+ ERR_PTR(err));
+ }
netdev_upper_dev_unlink(conduit, dev);
out_revert_old_conduit_unlink:
netdev_upper_dev_link(old_conduit, dev, NULL);
--
2.43.2
next prev parent reply other threads:[~2024-04-29 16:36 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-29 16:36 [PATCH net-next 0/2] Fix changing DSA conduit Marek Behún
2024-04-29 16:36 ` [PATCH net-next 1/2] net: dsa: Deduplicate code adding / deleting the port address to fdb Marek Behún
2024-04-29 16:36 ` Marek Behún [this message]
2024-04-30 0:31 ` [PATCH net-next 2/2] net: dsa: update the unicast MAC address when changing conduit Vladimir Oltean
2024-04-30 14:22 ` Marek Behún
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240429163627.16031-3-kabel@kernel.org \
--to=kabel@kernel.org \
--cc=andrew@lunn.ch \
--cc=f.fainelli@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=olteanv@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).