* [PATCH net-next 01/15] net: dsa: reorder PHY initialization with MTU setup in slave.c
2022-01-04 17:13 [PATCH net-next 00/15] DSA miscellaneous cleanups Vladimir Oltean
@ 2022-01-04 17:13 ` Vladimir Oltean
2022-01-04 17:14 ` [PATCH net-next 02/15] net: dsa: merge rtnl_lock sections in dsa_slave_create Vladimir Oltean
` (13 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Vladimir Oltean @ 2022-01-04 17:13 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
Florian Fainelli
In dsa_slave_create() there are 2 sections that take rtnl_lock():
MTU change and netdev registration. They are separated by PHY
initialization.
There isn't any strict ordering requirement except for the fact that
netdev registration should be last. Therefore, we can perform the MTU
change a bit later, after the PHY setup. A future change will then be
able to merge the two rtnl_lock sections into one.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
net/dsa/slave.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 88f7b8686dac..88bcdba92fa7 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -2011,13 +2011,6 @@ int dsa_slave_create(struct dsa_port *port)
port->slave = slave_dev;
dsa_slave_setup_tagger(slave_dev);
- rtnl_lock();
- ret = dsa_slave_change_mtu(slave_dev, ETH_DATA_LEN);
- rtnl_unlock();
- if (ret && ret != -EOPNOTSUPP)
- dev_warn(ds->dev, "nonfatal error %d setting MTU to %d on port %d\n",
- ret, ETH_DATA_LEN, port->index);
-
netif_carrier_off(slave_dev);
ret = dsa_slave_phy_setup(slave_dev);
@@ -2028,6 +2021,13 @@ int dsa_slave_create(struct dsa_port *port)
goto out_gcells;
}
+ rtnl_lock();
+ ret = dsa_slave_change_mtu(slave_dev, ETH_DATA_LEN);
+ rtnl_unlock();
+ if (ret && ret != -EOPNOTSUPP)
+ dev_warn(ds->dev, "nonfatal error %d setting MTU to %d on port %d\n",
+ ret, ETH_DATA_LEN, port->index);
+
rtnl_lock();
ret = register_netdevice(slave_dev);
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH net-next 02/15] net: dsa: merge rtnl_lock sections in dsa_slave_create
2022-01-04 17:13 [PATCH net-next 00/15] DSA miscellaneous cleanups Vladimir Oltean
2022-01-04 17:13 ` [PATCH net-next 01/15] net: dsa: reorder PHY initialization with MTU setup in slave.c Vladimir Oltean
@ 2022-01-04 17:14 ` Vladimir Oltean
2022-01-04 17:14 ` [PATCH net-next 03/15] net: dsa: stop updating master MTU from master.c Vladimir Oltean
` (12 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Vladimir Oltean @ 2022-01-04 17:14 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
Florian Fainelli
Currently dsa_slave_create() has two sequences of rtnl_lock/rtnl_unlock
in a row. Remove the rtnl_unlock() and rtnl_lock() in between, such that
the operation can execute slighly faster.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
net/dsa/slave.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 88bcdba92fa7..22241afcac81 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -2022,14 +2022,12 @@ int dsa_slave_create(struct dsa_port *port)
}
rtnl_lock();
+
ret = dsa_slave_change_mtu(slave_dev, ETH_DATA_LEN);
- rtnl_unlock();
if (ret && ret != -EOPNOTSUPP)
dev_warn(ds->dev, "nonfatal error %d setting MTU to %d on port %d\n",
ret, ETH_DATA_LEN, port->index);
- rtnl_lock();
-
ret = register_netdevice(slave_dev);
if (ret) {
netdev_err(master, "error %d registering interface %s\n",
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH net-next 03/15] net: dsa: stop updating master MTU from master.c
2022-01-04 17:13 [PATCH net-next 00/15] DSA miscellaneous cleanups Vladimir Oltean
2022-01-04 17:13 ` [PATCH net-next 01/15] net: dsa: reorder PHY initialization with MTU setup in slave.c Vladimir Oltean
2022-01-04 17:14 ` [PATCH net-next 02/15] net: dsa: merge rtnl_lock sections in dsa_slave_create Vladimir Oltean
@ 2022-01-04 17:14 ` Vladimir Oltean
2022-01-04 17:14 ` [PATCH net-next 04/15] net: dsa: hold rtnl_mutex when calling dsa_master_{setup,teardown} Vladimir Oltean
` (11 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Vladimir Oltean @ 2022-01-04 17:14 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
Florian Fainelli
At present there are two paths for changing the MTU of the DSA master.
The first is:
dsa_tree_setup
-> dsa_tree_setup_ports
-> dsa_port_setup
-> dsa_slave_create
-> dsa_slave_change_mtu
-> dev_set_mtu(master)
The second is:
dsa_tree_setup
-> dsa_tree_setup_master
-> dsa_master_setup
-> dev_set_mtu(dev)
So the dev_set_mtu() call from dsa_master_setup() has been effectively
superseded by the dsa_slave_change_mtu(slave_dev, ETH_DATA_LEN) that is
done from dsa_slave_create() for each user port. The later function also
updates the master MTU according to the largest user port MTU from the
tree. Therefore, updating the master MTU through a separate code path
isn't needed.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
net/dsa/master.c | 25 +------------------------
1 file changed, 1 insertion(+), 24 deletions(-)
diff --git a/net/dsa/master.c b/net/dsa/master.c
index e8e19857621b..f4efb244f91d 100644
--- a/net/dsa/master.c
+++ b/net/dsa/master.c
@@ -330,28 +330,13 @@ static const struct attribute_group dsa_group = {
.attrs = dsa_slave_attrs,
};
-static void dsa_master_reset_mtu(struct net_device *dev)
-{
- int err;
-
- rtnl_lock();
- err = dev_set_mtu(dev, ETH_DATA_LEN);
- if (err)
- netdev_dbg(dev,
- "Unable to reset MTU to exclude DSA overheads\n");
- rtnl_unlock();
-}
-
static struct lock_class_key dsa_master_addr_list_lock_key;
int dsa_master_setup(struct net_device *dev, struct dsa_port *cpu_dp)
{
- const struct dsa_device_ops *tag_ops = cpu_dp->tag_ops;
struct dsa_switch *ds = cpu_dp->ds;
struct device_link *consumer_link;
- int mtu, ret;
-
- mtu = ETH_DATA_LEN + dsa_tag_protocol_overhead(tag_ops);
+ int ret;
/* The DSA master must use SET_NETDEV_DEV for this to work. */
consumer_link = device_link_add(ds->dev, dev->dev.parent,
@@ -361,13 +346,6 @@ int dsa_master_setup(struct net_device *dev, struct dsa_port *cpu_dp)
"Failed to create a device link to DSA switch %s\n",
dev_name(ds->dev));
- rtnl_lock();
- ret = dev_set_mtu(dev, mtu);
- rtnl_unlock();
- if (ret)
- netdev_warn(dev, "error %d setting MTU to %d to include DSA overhead\n",
- ret, mtu);
-
/* If we use a tagging format that doesn't have an ethertype
* field, make sure that all packets from this point on get
* sent to the tag format's receive function.
@@ -405,7 +383,6 @@ void dsa_master_teardown(struct net_device *dev)
sysfs_remove_group(&dev->dev.kobj, &dsa_group);
dsa_netdev_ops_set(dev, NULL);
dsa_master_ethtool_teardown(dev);
- dsa_master_reset_mtu(dev);
dsa_master_set_promiscuity(dev, -1);
dev->dsa_ptr = NULL;
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH net-next 04/15] net: dsa: hold rtnl_mutex when calling dsa_master_{setup,teardown}
2022-01-04 17:13 [PATCH net-next 00/15] DSA miscellaneous cleanups Vladimir Oltean
` (2 preceding siblings ...)
2022-01-04 17:14 ` [PATCH net-next 03/15] net: dsa: stop updating master MTU from master.c Vladimir Oltean
@ 2022-01-04 17:14 ` Vladimir Oltean
2022-01-04 17:14 ` [PATCH net-next 05/15] net: dsa: first set up shared ports, then non-shared ports Vladimir Oltean
` (10 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Vladimir Oltean @ 2022-01-04 17:14 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
Florian Fainelli
DSA needs to simulate master tracking events when a binding is first
with a DSA master established and torn down, in order to give drivers
the simplifying guarantee that ->master_state_change calls are made
only when the master's readiness state to pass traffic changes.
master_state_change() provide a operational bool that DSA driver can use
to understand if DSA master is operational or not.
To avoid races, we need to block the reception of
NETDEV_UP/NETDEV_CHANGE/NETDEV_GOING_DOWN events in the netdev notifier
chain while we are changing the master's dev->dsa_ptr (this changes what
netdev_uses_dsa(dev) reports).
The dsa_master_setup() and dsa_master_teardown() functions optionally
require the rtnl_mutex to be held, if the tagger needs the master to be
promiscuous, these functions call dev_set_promiscuity(). Move the
rtnl_lock() from that function and make it top-level.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
net/dsa/dsa2.c | 8 ++++++++
net/dsa/master.c | 4 ++--
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index c18b22c0bf55..f044136f3625 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -1038,6 +1038,8 @@ static int dsa_tree_setup_master(struct dsa_switch_tree *dst)
struct dsa_port *dp;
int err;
+ rtnl_lock();
+
list_for_each_entry(dp, &dst->ports, list) {
if (dsa_port_is_cpu(dp)) {
err = dsa_master_setup(dp->master, dp);
@@ -1046,6 +1048,8 @@ static int dsa_tree_setup_master(struct dsa_switch_tree *dst)
}
}
+ rtnl_unlock();
+
return 0;
}
@@ -1053,9 +1057,13 @@ static void dsa_tree_teardown_master(struct dsa_switch_tree *dst)
{
struct dsa_port *dp;
+ rtnl_lock();
+
list_for_each_entry(dp, &dst->ports, list)
if (dsa_port_is_cpu(dp))
dsa_master_teardown(dp->master);
+
+ rtnl_unlock();
}
static int dsa_tree_setup_lags(struct dsa_switch_tree *dst)
diff --git a/net/dsa/master.c b/net/dsa/master.c
index f4efb244f91d..2199104ca7df 100644
--- a/net/dsa/master.c
+++ b/net/dsa/master.c
@@ -267,9 +267,9 @@ static void dsa_master_set_promiscuity(struct net_device *dev, int inc)
if (!ops->promisc_on_master)
return;
- rtnl_lock();
+ ASSERT_RTNL();
+
dev_set_promiscuity(dev, inc);
- rtnl_unlock();
}
static ssize_t tagging_show(struct device *d, struct device_attribute *attr,
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH net-next 05/15] net: dsa: first set up shared ports, then non-shared ports
2022-01-04 17:13 [PATCH net-next 00/15] DSA miscellaneous cleanups Vladimir Oltean
` (3 preceding siblings ...)
2022-01-04 17:14 ` [PATCH net-next 04/15] net: dsa: hold rtnl_mutex when calling dsa_master_{setup,teardown} Vladimir Oltean
@ 2022-01-04 17:14 ` Vladimir Oltean
2022-01-04 17:14 ` [PATCH net-next 06/15] net: dsa: setup master before ports Vladimir Oltean
` (9 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Vladimir Oltean @ 2022-01-04 17:14 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
Florian Fainelli
After commit a57d8c217aad ("net: dsa: flush switchdev workqueue before
tearing down CPU/DSA ports"), the port setup and teardown procedure
became asymmetric.
The fact of the matter is that user ports need the shared ports to be up
before they can be used for CPU-initiated termination. And since we
register net devices for the user ports, those won't be functional until
we also call the setup for the shared (CPU, DSA) ports. But we may do
that later, depending on the port numbering scheme of the hardware we
are dealing with.
It just makes sense that all shared ports are brought up before any user
port is. I can't pinpoint any issue due to the current behavior, but
let's change it nonetheless, for consistency's sake.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
net/dsa/dsa2.c | 50 +++++++++++++++++++++++++++++++++++++-------------
1 file changed, 37 insertions(+), 13 deletions(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index f044136f3625..1ca78d83fa39 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -1003,23 +1003,28 @@ static void dsa_tree_teardown_switches(struct dsa_switch_tree *dst)
dsa_switch_teardown(dp->ds);
}
-static int dsa_tree_setup_switches(struct dsa_switch_tree *dst)
+/* Bring shared ports up first, then non-shared ports */
+static int dsa_tree_setup_ports(struct dsa_switch_tree *dst)
{
struct dsa_port *dp;
- int err;
+ int err = 0;
list_for_each_entry(dp, &dst->ports, list) {
- err = dsa_switch_setup(dp->ds);
- if (err)
- goto teardown;
+ if (dsa_port_is_dsa(dp) || dsa_port_is_cpu(dp)) {
+ err = dsa_port_setup(dp);
+ if (err)
+ goto teardown;
+ }
}
list_for_each_entry(dp, &dst->ports, list) {
- err = dsa_port_setup(dp);
- if (err) {
- err = dsa_port_reinit_as_unused(dp);
- if (err)
- goto teardown;
+ if (dsa_port_is_user(dp) || dsa_port_is_unused(dp)) {
+ err = dsa_port_setup(dp);
+ if (err) {
+ err = dsa_port_reinit_as_unused(dp);
+ if (err)
+ goto teardown;
+ }
}
}
@@ -1028,7 +1033,21 @@ static int dsa_tree_setup_switches(struct dsa_switch_tree *dst)
teardown:
dsa_tree_teardown_ports(dst);
- dsa_tree_teardown_switches(dst);
+ return err;
+}
+
+static int dsa_tree_setup_switches(struct dsa_switch_tree *dst)
+{
+ struct dsa_port *dp;
+ int err = 0;
+
+ list_for_each_entry(dp, &dst->ports, list) {
+ err = dsa_switch_setup(dp->ds);
+ if (err) {
+ dsa_tree_teardown_switches(dst);
+ break;
+ }
+ }
return err;
}
@@ -1115,10 +1134,14 @@ static int dsa_tree_setup(struct dsa_switch_tree *dst)
if (err)
goto teardown_cpu_ports;
- err = dsa_tree_setup_master(dst);
+ err = dsa_tree_setup_ports(dst);
if (err)
goto teardown_switches;
+ err = dsa_tree_setup_master(dst);
+ if (err)
+ goto teardown_ports;
+
err = dsa_tree_setup_lags(dst);
if (err)
goto teardown_master;
@@ -1131,8 +1154,9 @@ static int dsa_tree_setup(struct dsa_switch_tree *dst)
teardown_master:
dsa_tree_teardown_master(dst);
-teardown_switches:
+teardown_ports:
dsa_tree_teardown_ports(dst);
+teardown_switches:
dsa_tree_teardown_switches(dst);
teardown_cpu_ports:
dsa_tree_teardown_cpu_ports(dst);
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH net-next 06/15] net: dsa: setup master before ports
2022-01-04 17:13 [PATCH net-next 00/15] DSA miscellaneous cleanups Vladimir Oltean
` (4 preceding siblings ...)
2022-01-04 17:14 ` [PATCH net-next 05/15] net: dsa: first set up shared ports, then non-shared ports Vladimir Oltean
@ 2022-01-04 17:14 ` Vladimir Oltean
2022-01-04 17:14 ` [PATCH net-next 07/15] net: dsa: remove cross-chip support for MRP Vladimir Oltean
` (8 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Vladimir Oltean @ 2022-01-04 17:14 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
Florian Fainelli
It is said that as soon as a network interface is registered, all its
resources should have already been prepared, so that it is available for
sending and receiving traffic. One of the resources needed by a DSA
slave interface is the master.
dsa_tree_setup
-> dsa_tree_setup_ports
-> dsa_port_setup
-> dsa_slave_create
-> register_netdevice
-> dsa_tree_setup_master
-> dsa_master_setup
-> sets up master->dsa_ptr, which enables reception
Therefore, there is a short period of time after register_netdevice()
during which the master isn't prepared to pass traffic to the DSA layer
(master->dsa_ptr is checked by eth_type_trans). Same thing during
unregistration, there is a time frame in which packets might be missed.
Note that this change opens us to another race: dsa_master_find_slave()
will get invoked potentially earlier than the slave creation, and later
than the slave deletion. Since dp->slave starts off as a NULL pointer,
the earlier calls aren't a problem, but the later calls are. To avoid
use-after-free, we should zeroize dp->slave before calling
dsa_slave_destroy().
In practice I cannot really test real life improvements brought by this
change, since in my systems, netdevice creation races with PHY autoneg
which takes a few seconds to complete, and that masks quite a few races.
Effects might be noticeable in a setup with fixed links all the way to
an external system.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
net/dsa/dsa2.c | 23 +++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 1ca78d83fa39..c1da813786a4 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -561,6 +561,7 @@ static void dsa_port_teardown(struct dsa_port *dp)
struct devlink_port *dlp = &dp->devlink_port;
struct dsa_switch *ds = dp->ds;
struct dsa_mac_addr *a, *tmp;
+ struct net_device *slave;
if (!dp->setup)
return;
@@ -582,9 +583,11 @@ static void dsa_port_teardown(struct dsa_port *dp)
dsa_port_link_unregister_of(dp);
break;
case DSA_PORT_TYPE_USER:
- if (dp->slave) {
- dsa_slave_destroy(dp->slave);
+ slave = dp->slave;
+
+ if (slave) {
dp->slave = NULL;
+ dsa_slave_destroy(slave);
}
break;
}
@@ -1134,17 +1137,17 @@ static int dsa_tree_setup(struct dsa_switch_tree *dst)
if (err)
goto teardown_cpu_ports;
- err = dsa_tree_setup_ports(dst);
+ err = dsa_tree_setup_master(dst);
if (err)
goto teardown_switches;
- err = dsa_tree_setup_master(dst);
+ err = dsa_tree_setup_ports(dst);
if (err)
- goto teardown_ports;
+ goto teardown_master;
err = dsa_tree_setup_lags(dst);
if (err)
- goto teardown_master;
+ goto teardown_ports;
dst->setup = true;
@@ -1152,10 +1155,10 @@ static int dsa_tree_setup(struct dsa_switch_tree *dst)
return 0;
-teardown_master:
- dsa_tree_teardown_master(dst);
teardown_ports:
dsa_tree_teardown_ports(dst);
+teardown_master:
+ dsa_tree_teardown_master(dst);
teardown_switches:
dsa_tree_teardown_switches(dst);
teardown_cpu_ports:
@@ -1173,10 +1176,10 @@ static void dsa_tree_teardown(struct dsa_switch_tree *dst)
dsa_tree_teardown_lags(dst);
- dsa_tree_teardown_master(dst);
-
dsa_tree_teardown_ports(dst);
+ dsa_tree_teardown_master(dst);
+
dsa_tree_teardown_switches(dst);
dsa_tree_teardown_cpu_ports(dst);
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH net-next 07/15] net: dsa: remove cross-chip support for MRP
2022-01-04 17:13 [PATCH net-next 00/15] DSA miscellaneous cleanups Vladimir Oltean
` (5 preceding siblings ...)
2022-01-04 17:14 ` [PATCH net-next 06/15] net: dsa: setup master before ports Vladimir Oltean
@ 2022-01-04 17:14 ` Vladimir Oltean
2022-01-05 10:01 ` Horatiu Vultur
2022-01-04 17:14 ` [PATCH net-next 08/15] net: dsa: remove cross-chip support for HSR Vladimir Oltean
` (7 subsequent siblings)
14 siblings, 1 reply; 20+ messages in thread
From: Vladimir Oltean @ 2022-01-04 17:14 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Horatiu Vultur
The cross-chip notifiers for MRP are bypass operations, meaning that
even though all switches in a tree are notified, only the switch
specified in the info structure is targeted.
We can eliminate the unnecessary complexity by deleting the cross-chip
notifier logic and calling the ds->ops straight from port.c.
Cc: Horatiu Vultur <horatiu.vultur@microchip.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
net/dsa/dsa_priv.h | 4 ---
net/dsa/port.c | 44 +++++++++++++++----------------
net/dsa/switch.c | 64 ----------------------------------------------
3 files changed, 20 insertions(+), 92 deletions(-)
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index b5ae21f172a8..54c23479b9ba 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -40,10 +40,6 @@ enum {
DSA_NOTIFIER_TAG_PROTO,
DSA_NOTIFIER_TAG_PROTO_CONNECT,
DSA_NOTIFIER_TAG_PROTO_DISCONNECT,
- DSA_NOTIFIER_MRP_ADD,
- DSA_NOTIFIER_MRP_DEL,
- DSA_NOTIFIER_MRP_ADD_RING_ROLE,
- DSA_NOTIFIER_MRP_DEL_RING_ROLE,
DSA_NOTIFIER_TAG_8021Q_VLAN_ADD,
DSA_NOTIFIER_TAG_8021Q_VLAN_DEL,
};
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 05677e016982..5c72f890c6a2 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -907,49 +907,45 @@ int dsa_port_vlan_del(struct dsa_port *dp,
int dsa_port_mrp_add(const struct dsa_port *dp,
const struct switchdev_obj_mrp *mrp)
{
- struct dsa_notifier_mrp_info info = {
- .sw_index = dp->ds->index,
- .port = dp->index,
- .mrp = mrp,
- };
+ struct dsa_switch *ds = dp->ds;
+
+ if (!ds->ops->port_mrp_add)
+ return -EOPNOTSUPP;
- return dsa_port_notify(dp, DSA_NOTIFIER_MRP_ADD, &info);
+ return ds->ops->port_mrp_add(ds, dp->index, mrp);
}
int dsa_port_mrp_del(const struct dsa_port *dp,
const struct switchdev_obj_mrp *mrp)
{
- struct dsa_notifier_mrp_info info = {
- .sw_index = dp->ds->index,
- .port = dp->index,
- .mrp = mrp,
- };
+ struct dsa_switch *ds = dp->ds;
+
+ if (!ds->ops->port_mrp_del)
+ return -EOPNOTSUPP;
- return dsa_port_notify(dp, DSA_NOTIFIER_MRP_DEL, &info);
+ return ds->ops->port_mrp_del(ds, dp->index, mrp);
}
int dsa_port_mrp_add_ring_role(const struct dsa_port *dp,
const struct switchdev_obj_ring_role_mrp *mrp)
{
- struct dsa_notifier_mrp_ring_role_info info = {
- .sw_index = dp->ds->index,
- .port = dp->index,
- .mrp = mrp,
- };
+ struct dsa_switch *ds = dp->ds;
+
+ if (!ds->ops->port_mrp_add)
+ return -EOPNOTSUPP;
- return dsa_port_notify(dp, DSA_NOTIFIER_MRP_ADD_RING_ROLE, &info);
+ return ds->ops->port_mrp_add_ring_role(ds, dp->index, mrp);
}
int dsa_port_mrp_del_ring_role(const struct dsa_port *dp,
const struct switchdev_obj_ring_role_mrp *mrp)
{
- struct dsa_notifier_mrp_ring_role_info info = {
- .sw_index = dp->ds->index,
- .port = dp->index,
- .mrp = mrp,
- };
+ struct dsa_switch *ds = dp->ds;
+
+ if (!ds->ops->port_mrp_del)
+ return -EOPNOTSUPP;
- return dsa_port_notify(dp, DSA_NOTIFIER_MRP_DEL_RING_ROLE, &info);
+ return ds->ops->port_mrp_del_ring_role(ds, dp->index, mrp);
}
void dsa_port_set_tag_protocol(struct dsa_port *cpu_dp,
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index 393f2d8a860a..a164ec02b4e9 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -701,58 +701,6 @@ dsa_switch_disconnect_tag_proto(struct dsa_switch *ds,
return 0;
}
-static int dsa_switch_mrp_add(struct dsa_switch *ds,
- struct dsa_notifier_mrp_info *info)
-{
- if (!ds->ops->port_mrp_add)
- return -EOPNOTSUPP;
-
- if (ds->index == info->sw_index)
- return ds->ops->port_mrp_add(ds, info->port, info->mrp);
-
- return 0;
-}
-
-static int dsa_switch_mrp_del(struct dsa_switch *ds,
- struct dsa_notifier_mrp_info *info)
-{
- if (!ds->ops->port_mrp_del)
- return -EOPNOTSUPP;
-
- if (ds->index == info->sw_index)
- return ds->ops->port_mrp_del(ds, info->port, info->mrp);
-
- return 0;
-}
-
-static int
-dsa_switch_mrp_add_ring_role(struct dsa_switch *ds,
- struct dsa_notifier_mrp_ring_role_info *info)
-{
- if (!ds->ops->port_mrp_add)
- return -EOPNOTSUPP;
-
- if (ds->index == info->sw_index)
- return ds->ops->port_mrp_add_ring_role(ds, info->port,
- info->mrp);
-
- return 0;
-}
-
-static int
-dsa_switch_mrp_del_ring_role(struct dsa_switch *ds,
- struct dsa_notifier_mrp_ring_role_info *info)
-{
- if (!ds->ops->port_mrp_del)
- return -EOPNOTSUPP;
-
- if (ds->index == info->sw_index)
- return ds->ops->port_mrp_del_ring_role(ds, info->port,
- info->mrp);
-
- return 0;
-}
-
static int dsa_switch_event(struct notifier_block *nb,
unsigned long event, void *info)
{
@@ -826,18 +774,6 @@ static int dsa_switch_event(struct notifier_block *nb,
case DSA_NOTIFIER_TAG_PROTO_DISCONNECT:
err = dsa_switch_disconnect_tag_proto(ds, info);
break;
- case DSA_NOTIFIER_MRP_ADD:
- err = dsa_switch_mrp_add(ds, info);
- break;
- case DSA_NOTIFIER_MRP_DEL:
- err = dsa_switch_mrp_del(ds, info);
- break;
- case DSA_NOTIFIER_MRP_ADD_RING_ROLE:
- err = dsa_switch_mrp_add_ring_role(ds, info);
- break;
- case DSA_NOTIFIER_MRP_DEL_RING_ROLE:
- err = dsa_switch_mrp_del_ring_role(ds, info);
- break;
case DSA_NOTIFIER_TAG_8021Q_VLAN_ADD:
err = dsa_switch_tag_8021q_vlan_add(ds, info);
break;
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH net-next 07/15] net: dsa: remove cross-chip support for MRP
2022-01-04 17:14 ` [PATCH net-next 07/15] net: dsa: remove cross-chip support for MRP Vladimir Oltean
@ 2022-01-05 10:01 ` Horatiu Vultur
2022-01-05 11:51 ` Vladimir Oltean
0 siblings, 1 reply; 20+ messages in thread
From: Horatiu Vultur @ 2022-01-05 10:01 UTC (permalink / raw)
To: Vladimir Oltean
Cc: netdev, David S. Miller, Jakub Kicinski, Andrew Lunn,
Vivien Didelot, Florian Fainelli
The 01/04/2022 19:14, Vladimir Oltean wrote:
>
> The cross-chip notifiers for MRP are bypass operations, meaning that
> even though all switches in a tree are notified, only the switch
> specified in the info structure is targeted.
>
> We can eliminate the unnecessary complexity by deleting the cross-chip
> notifier logic and calling the ds->ops straight from port.c.
It looks like structs dsa_notifier_mrp_info and
dsa_notifier_mrp_ring_role_info are not used anywhere anymore. So they
should also be deleted.
>
> Cc: Horatiu Vultur <horatiu.vultur@microchip.com>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> net/dsa/dsa_priv.h | 4 ---
> net/dsa/port.c | 44 +++++++++++++++----------------
> net/dsa/switch.c | 64 ----------------------------------------------
> 3 files changed, 20 insertions(+), 92 deletions(-)
>
> diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
> index b5ae21f172a8..54c23479b9ba 100644
> --- a/net/dsa/dsa_priv.h
> +++ b/net/dsa/dsa_priv.h
> @@ -40,10 +40,6 @@ enum {
> DSA_NOTIFIER_TAG_PROTO,
> DSA_NOTIFIER_TAG_PROTO_CONNECT,
> DSA_NOTIFIER_TAG_PROTO_DISCONNECT,
> - DSA_NOTIFIER_MRP_ADD,
> - DSA_NOTIFIER_MRP_DEL,
> - DSA_NOTIFIER_MRP_ADD_RING_ROLE,
> - DSA_NOTIFIER_MRP_DEL_RING_ROLE,
> DSA_NOTIFIER_TAG_8021Q_VLAN_ADD,
> DSA_NOTIFIER_TAG_8021Q_VLAN_DEL,
> };
> diff --git a/net/dsa/port.c b/net/dsa/port.c
> index 05677e016982..5c72f890c6a2 100644
> --- a/net/dsa/port.c
> +++ b/net/dsa/port.c
> @@ -907,49 +907,45 @@ int dsa_port_vlan_del(struct dsa_port *dp,
> int dsa_port_mrp_add(const struct dsa_port *dp,
> const struct switchdev_obj_mrp *mrp)
> {
> - struct dsa_notifier_mrp_info info = {
> - .sw_index = dp->ds->index,
> - .port = dp->index,
> - .mrp = mrp,
> - };
> + struct dsa_switch *ds = dp->ds;
> +
> + if (!ds->ops->port_mrp_add)
> + return -EOPNOTSUPP;
>
> - return dsa_port_notify(dp, DSA_NOTIFIER_MRP_ADD, &info);
> + return ds->ops->port_mrp_add(ds, dp->index, mrp);
> }
>
> int dsa_port_mrp_del(const struct dsa_port *dp,
> const struct switchdev_obj_mrp *mrp)
> {
> - struct dsa_notifier_mrp_info info = {
> - .sw_index = dp->ds->index,
> - .port = dp->index,
> - .mrp = mrp,
> - };
> + struct dsa_switch *ds = dp->ds;
> +
> + if (!ds->ops->port_mrp_del)
> + return -EOPNOTSUPP;
>
> - return dsa_port_notify(dp, DSA_NOTIFIER_MRP_DEL, &info);
> + return ds->ops->port_mrp_del(ds, dp->index, mrp);
> }
>
> int dsa_port_mrp_add_ring_role(const struct dsa_port *dp,
> const struct switchdev_obj_ring_role_mrp *mrp)
> {
> - struct dsa_notifier_mrp_ring_role_info info = {
> - .sw_index = dp->ds->index,
> - .port = dp->index,
> - .mrp = mrp,
> - };
> + struct dsa_switch *ds = dp->ds;
> +
> + if (!ds->ops->port_mrp_add)
> + return -EOPNOTSUPP;
>
> - return dsa_port_notify(dp, DSA_NOTIFIER_MRP_ADD_RING_ROLE, &info);
> + return ds->ops->port_mrp_add_ring_role(ds, dp->index, mrp);
> }
>
> int dsa_port_mrp_del_ring_role(const struct dsa_port *dp,
> const struct switchdev_obj_ring_role_mrp *mrp)
> {
> - struct dsa_notifier_mrp_ring_role_info info = {
> - .sw_index = dp->ds->index,
> - .port = dp->index,
> - .mrp = mrp,
> - };
> + struct dsa_switch *ds = dp->ds;
> +
> + if (!ds->ops->port_mrp_del)
> + return -EOPNOTSUPP;
>
> - return dsa_port_notify(dp, DSA_NOTIFIER_MRP_DEL_RING_ROLE, &info);
> + return ds->ops->port_mrp_del_ring_role(ds, dp->index, mrp);
> }
>
> void dsa_port_set_tag_protocol(struct dsa_port *cpu_dp,
> diff --git a/net/dsa/switch.c b/net/dsa/switch.c
> index 393f2d8a860a..a164ec02b4e9 100644
> --- a/net/dsa/switch.c
> +++ b/net/dsa/switch.c
> @@ -701,58 +701,6 @@ dsa_switch_disconnect_tag_proto(struct dsa_switch *ds,
> return 0;
> }
>
> -static int dsa_switch_mrp_add(struct dsa_switch *ds,
> - struct dsa_notifier_mrp_info *info)
> -{
> - if (!ds->ops->port_mrp_add)
> - return -EOPNOTSUPP;
> -
> - if (ds->index == info->sw_index)
> - return ds->ops->port_mrp_add(ds, info->port, info->mrp);
> -
> - return 0;
> -}
> -
> -static int dsa_switch_mrp_del(struct dsa_switch *ds,
> - struct dsa_notifier_mrp_info *info)
> -{
> - if (!ds->ops->port_mrp_del)
> - return -EOPNOTSUPP;
> -
> - if (ds->index == info->sw_index)
> - return ds->ops->port_mrp_del(ds, info->port, info->mrp);
> -
> - return 0;
> -}
> -
> -static int
> -dsa_switch_mrp_add_ring_role(struct dsa_switch *ds,
> - struct dsa_notifier_mrp_ring_role_info *info)
> -{
> - if (!ds->ops->port_mrp_add)
> - return -EOPNOTSUPP;
> -
> - if (ds->index == info->sw_index)
> - return ds->ops->port_mrp_add_ring_role(ds, info->port,
> - info->mrp);
> -
> - return 0;
> -}
> -
> -static int
> -dsa_switch_mrp_del_ring_role(struct dsa_switch *ds,
> - struct dsa_notifier_mrp_ring_role_info *info)
> -{
> - if (!ds->ops->port_mrp_del)
> - return -EOPNOTSUPP;
> -
> - if (ds->index == info->sw_index)
> - return ds->ops->port_mrp_del_ring_role(ds, info->port,
> - info->mrp);
> -
> - return 0;
> -}
> -
> static int dsa_switch_event(struct notifier_block *nb,
> unsigned long event, void *info)
> {
> @@ -826,18 +774,6 @@ static int dsa_switch_event(struct notifier_block *nb,
> case DSA_NOTIFIER_TAG_PROTO_DISCONNECT:
> err = dsa_switch_disconnect_tag_proto(ds, info);
> break;
> - case DSA_NOTIFIER_MRP_ADD:
> - err = dsa_switch_mrp_add(ds, info);
> - break;
> - case DSA_NOTIFIER_MRP_DEL:
> - err = dsa_switch_mrp_del(ds, info);
> - break;
> - case DSA_NOTIFIER_MRP_ADD_RING_ROLE:
> - err = dsa_switch_mrp_add_ring_role(ds, info);
> - break;
> - case DSA_NOTIFIER_MRP_DEL_RING_ROLE:
> - err = dsa_switch_mrp_del_ring_role(ds, info);
> - break;
> case DSA_NOTIFIER_TAG_8021Q_VLAN_ADD:
> err = dsa_switch_tag_8021q_vlan_add(ds, info);
> break;
> --
> 2.25.1
>
--
/Horatiu
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH net-next 07/15] net: dsa: remove cross-chip support for MRP
2022-01-05 10:01 ` Horatiu Vultur
@ 2022-01-05 11:51 ` Vladimir Oltean
0 siblings, 0 replies; 20+ messages in thread
From: Vladimir Oltean @ 2022-01-05 11:51 UTC (permalink / raw)
To: Horatiu Vultur
Cc: netdev, David S. Miller, Jakub Kicinski, Andrew Lunn,
Vivien Didelot, Florian Fainelli
On Wed, Jan 05, 2022 at 11:01:52AM +0100, Horatiu Vultur wrote:
> The 01/04/2022 19:14, Vladimir Oltean wrote:
> >
> > The cross-chip notifiers for MRP are bypass operations, meaning that
> > even though all switches in a tree are notified, only the switch
> > specified in the info structure is targeted.
> >
> > We can eliminate the unnecessary complexity by deleting the cross-chip
> > notifier logic and calling the ds->ops straight from port.c.
>
> It looks like structs dsa_notifier_mrp_info and
> dsa_notifier_mrp_ring_role_info are not used anywhere anymore. So they
> should also be deleted.
Well spotted, thanks.
> >
> > Cc: Horatiu Vultur <horatiu.vultur@microchip.com>
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > ---
> > net/dsa/dsa_priv.h | 4 ---
> > net/dsa/port.c | 44 +++++++++++++++----------------
> > net/dsa/switch.c | 64 ----------------------------------------------
> > 3 files changed, 20 insertions(+), 92 deletions(-)
> >
> > diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
> > index b5ae21f172a8..54c23479b9ba 100644
> > --- a/net/dsa/dsa_priv.h
> > +++ b/net/dsa/dsa_priv.h
> > @@ -40,10 +40,6 @@ enum {
> > DSA_NOTIFIER_TAG_PROTO,
> > DSA_NOTIFIER_TAG_PROTO_CONNECT,
> > DSA_NOTIFIER_TAG_PROTO_DISCONNECT,
> > - DSA_NOTIFIER_MRP_ADD,
> > - DSA_NOTIFIER_MRP_DEL,
> > - DSA_NOTIFIER_MRP_ADD_RING_ROLE,
> > - DSA_NOTIFIER_MRP_DEL_RING_ROLE,
> > DSA_NOTIFIER_TAG_8021Q_VLAN_ADD,
> > DSA_NOTIFIER_TAG_8021Q_VLAN_DEL,
> > };
> > diff --git a/net/dsa/port.c b/net/dsa/port.c
> > index 05677e016982..5c72f890c6a2 100644
> > --- a/net/dsa/port.c
> > +++ b/net/dsa/port.c
> > @@ -907,49 +907,45 @@ int dsa_port_vlan_del(struct dsa_port *dp,
> > int dsa_port_mrp_add(const struct dsa_port *dp,
> > const struct switchdev_obj_mrp *mrp)
> > {
> > - struct dsa_notifier_mrp_info info = {
> > - .sw_index = dp->ds->index,
> > - .port = dp->index,
> > - .mrp = mrp,
> > - };
> > + struct dsa_switch *ds = dp->ds;
> > +
> > + if (!ds->ops->port_mrp_add)
> > + return -EOPNOTSUPP;
> >
> > - return dsa_port_notify(dp, DSA_NOTIFIER_MRP_ADD, &info);
> > + return ds->ops->port_mrp_add(ds, dp->index, mrp);
> > }
> >
> > int dsa_port_mrp_del(const struct dsa_port *dp,
> > const struct switchdev_obj_mrp *mrp)
> > {
> > - struct dsa_notifier_mrp_info info = {
> > - .sw_index = dp->ds->index,
> > - .port = dp->index,
> > - .mrp = mrp,
> > - };
> > + struct dsa_switch *ds = dp->ds;
> > +
> > + if (!ds->ops->port_mrp_del)
> > + return -EOPNOTSUPP;
> >
> > - return dsa_port_notify(dp, DSA_NOTIFIER_MRP_DEL, &info);
> > + return ds->ops->port_mrp_del(ds, dp->index, mrp);
> > }
> >
> > int dsa_port_mrp_add_ring_role(const struct dsa_port *dp,
> > const struct switchdev_obj_ring_role_mrp *mrp)
> > {
> > - struct dsa_notifier_mrp_ring_role_info info = {
> > - .sw_index = dp->ds->index,
> > - .port = dp->index,
> > - .mrp = mrp,
> > - };
> > + struct dsa_switch *ds = dp->ds;
> > +
> > + if (!ds->ops->port_mrp_add)
> > + return -EOPNOTSUPP;
> >
> > - return dsa_port_notify(dp, DSA_NOTIFIER_MRP_ADD_RING_ROLE, &info);
> > + return ds->ops->port_mrp_add_ring_role(ds, dp->index, mrp);
> > }
> >
> > int dsa_port_mrp_del_ring_role(const struct dsa_port *dp,
> > const struct switchdev_obj_ring_role_mrp *mrp)
> > {
> > - struct dsa_notifier_mrp_ring_role_info info = {
> > - .sw_index = dp->ds->index,
> > - .port = dp->index,
> > - .mrp = mrp,
> > - };
> > + struct dsa_switch *ds = dp->ds;
> > +
> > + if (!ds->ops->port_mrp_del)
> > + return -EOPNOTSUPP;
> >
> > - return dsa_port_notify(dp, DSA_NOTIFIER_MRP_DEL_RING_ROLE, &info);
> > + return ds->ops->port_mrp_del_ring_role(ds, dp->index, mrp);
> > }
> >
> > void dsa_port_set_tag_protocol(struct dsa_port *cpu_dp,
> > diff --git a/net/dsa/switch.c b/net/dsa/switch.c
> > index 393f2d8a860a..a164ec02b4e9 100644
> > --- a/net/dsa/switch.c
> > +++ b/net/dsa/switch.c
> > @@ -701,58 +701,6 @@ dsa_switch_disconnect_tag_proto(struct dsa_switch *ds,
> > return 0;
> > }
> >
> > -static int dsa_switch_mrp_add(struct dsa_switch *ds,
> > - struct dsa_notifier_mrp_info *info)
> > -{
> > - if (!ds->ops->port_mrp_add)
> > - return -EOPNOTSUPP;
> > -
> > - if (ds->index == info->sw_index)
> > - return ds->ops->port_mrp_add(ds, info->port, info->mrp);
> > -
> > - return 0;
> > -}
> > -
> > -static int dsa_switch_mrp_del(struct dsa_switch *ds,
> > - struct dsa_notifier_mrp_info *info)
> > -{
> > - if (!ds->ops->port_mrp_del)
> > - return -EOPNOTSUPP;
> > -
> > - if (ds->index == info->sw_index)
> > - return ds->ops->port_mrp_del(ds, info->port, info->mrp);
> > -
> > - return 0;
> > -}
> > -
> > -static int
> > -dsa_switch_mrp_add_ring_role(struct dsa_switch *ds,
> > - struct dsa_notifier_mrp_ring_role_info *info)
> > -{
> > - if (!ds->ops->port_mrp_add)
> > - return -EOPNOTSUPP;
> > -
> > - if (ds->index == info->sw_index)
> > - return ds->ops->port_mrp_add_ring_role(ds, info->port,
> > - info->mrp);
> > -
> > - return 0;
> > -}
> > -
> > -static int
> > -dsa_switch_mrp_del_ring_role(struct dsa_switch *ds,
> > - struct dsa_notifier_mrp_ring_role_info *info)
> > -{
> > - if (!ds->ops->port_mrp_del)
> > - return -EOPNOTSUPP;
Upon further self-review I found a bug here and in dsa_switch_mrp_add_ring_role,
that the incorrect function pointer is checked. If a driver implements
ds->ops->port_mrp_del but not ds->ops->port_mrp_del_ring_role, this will
cause a NULL pointer dereference. I'll submit a separate patch to
net-next for this (it's a bug but an inconsequential one, since there
isn't any DSA driver in that situation).
> > -
> > - if (ds->index == info->sw_index)
> > - return ds->ops->port_mrp_del_ring_role(ds, info->port,
> > - info->mrp);
> > -
> > - return 0;
> > -}
> > -
> > static int dsa_switch_event(struct notifier_block *nb,
> > unsigned long event, void *info)
> > {
> > @@ -826,18 +774,6 @@ static int dsa_switch_event(struct notifier_block *nb,
> > case DSA_NOTIFIER_TAG_PROTO_DISCONNECT:
> > err = dsa_switch_disconnect_tag_proto(ds, info);
> > break;
> > - case DSA_NOTIFIER_MRP_ADD:
> > - err = dsa_switch_mrp_add(ds, info);
> > - break;
> > - case DSA_NOTIFIER_MRP_DEL:
> > - err = dsa_switch_mrp_del(ds, info);
> > - break;
> > - case DSA_NOTIFIER_MRP_ADD_RING_ROLE:
> > - err = dsa_switch_mrp_add_ring_role(ds, info);
> > - break;
> > - case DSA_NOTIFIER_MRP_DEL_RING_ROLE:
> > - err = dsa_switch_mrp_del_ring_role(ds, info);
> > - break;
> > case DSA_NOTIFIER_TAG_8021Q_VLAN_ADD:
> > err = dsa_switch_tag_8021q_vlan_add(ds, info);
> > break;
> > --
> > 2.25.1
> >
>
> --
> /Horatiu
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH net-next 08/15] net: dsa: remove cross-chip support for HSR
2022-01-04 17:13 [PATCH net-next 00/15] DSA miscellaneous cleanups Vladimir Oltean
` (6 preceding siblings ...)
2022-01-04 17:14 ` [PATCH net-next 07/15] net: dsa: remove cross-chip support for MRP Vladimir Oltean
@ 2022-01-04 17:14 ` Vladimir Oltean
2022-01-04 22:03 ` George McCollister
2022-01-04 17:14 ` [PATCH net-next 09/15] net: dsa: move dsa_port :: stp_state near dsa_port :: mac Vladimir Oltean
` (6 subsequent siblings)
14 siblings, 1 reply; 20+ messages in thread
From: Vladimir Oltean @ 2022-01-04 17:14 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
Florian Fainelli, George McCollister
The cross-chip notifiers for HSR are bypass operations, meaning that
even though all switches in a tree are notified, only the switch
specified in the info structure is targeted.
We can eliminate the unnecessary complexity by deleting the cross-chip
notifier logic and calling the ds->ops straight from port.c.
Cc: George McCollister <george.mccollister@gmail.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
net/dsa/dsa_priv.h | 2 --
net/dsa/port.c | 20 ++++++--------------
net/dsa/switch.c | 24 ------------------------
3 files changed, 6 insertions(+), 40 deletions(-)
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 54c23479b9ba..b3386d408fc6 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -25,8 +25,6 @@ enum {
DSA_NOTIFIER_FDB_DEL,
DSA_NOTIFIER_HOST_FDB_ADD,
DSA_NOTIFIER_HOST_FDB_DEL,
- DSA_NOTIFIER_HSR_JOIN,
- DSA_NOTIFIER_HSR_LEAVE,
DSA_NOTIFIER_LAG_CHANGE,
DSA_NOTIFIER_LAG_JOIN,
DSA_NOTIFIER_LAG_LEAVE,
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 5c72f890c6a2..9e7c421c47b9 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -1317,16 +1317,12 @@ EXPORT_SYMBOL_GPL(dsa_port_get_phy_sset_count);
int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr)
{
- struct dsa_notifier_hsr_info info = {
- .sw_index = dp->ds->index,
- .port = dp->index,
- .hsr = hsr,
- };
+ struct dsa_switch *ds = dp->ds;
int err;
dp->hsr_dev = hsr;
- err = dsa_port_notify(dp, DSA_NOTIFIER_HSR_JOIN, &info);
+ err = ds->ops->port_hsr_join(ds, dp->index, hsr);
if (err)
dp->hsr_dev = NULL;
@@ -1335,20 +1331,16 @@ int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr)
void dsa_port_hsr_leave(struct dsa_port *dp, struct net_device *hsr)
{
- struct dsa_notifier_hsr_info info = {
- .sw_index = dp->ds->index,
- .port = dp->index,
- .hsr = hsr,
- };
+ struct dsa_switch *ds = dp->ds;
int err;
dp->hsr_dev = NULL;
- err = dsa_port_notify(dp, DSA_NOTIFIER_HSR_LEAVE, &info);
+ err = ds->ops->port_hsr_leave(ds, dp->index, hsr);
if (err)
dev_err(dp->ds->dev,
- "port %d failed to notify DSA_NOTIFIER_HSR_LEAVE: %pe\n",
- dp->index, ERR_PTR(err));
+ "port %d failed to leave HSR %s: %pe\n",
+ dp->index, hsr->name, ERR_PTR(err));
}
int dsa_port_tag_8021q_vlan_add(struct dsa_port *dp, u16 vid, bool broadcast)
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index a164ec02b4e9..e3c7d2627a61 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -437,24 +437,6 @@ static int dsa_switch_fdb_del(struct dsa_switch *ds,
return dsa_port_do_fdb_del(dp, info->addr, info->vid);
}
-static int dsa_switch_hsr_join(struct dsa_switch *ds,
- struct dsa_notifier_hsr_info *info)
-{
- if (ds->index == info->sw_index && ds->ops->port_hsr_join)
- return ds->ops->port_hsr_join(ds, info->port, info->hsr);
-
- return -EOPNOTSUPP;
-}
-
-static int dsa_switch_hsr_leave(struct dsa_switch *ds,
- struct dsa_notifier_hsr_info *info)
-{
- if (ds->index == info->sw_index && ds->ops->port_hsr_leave)
- return ds->ops->port_hsr_leave(ds, info->port, info->hsr);
-
- return -EOPNOTSUPP;
-}
-
static int dsa_switch_lag_change(struct dsa_switch *ds,
struct dsa_notifier_lag_info *info)
{
@@ -729,12 +711,6 @@ static int dsa_switch_event(struct notifier_block *nb,
case DSA_NOTIFIER_HOST_FDB_DEL:
err = dsa_switch_host_fdb_del(ds, info);
break;
- case DSA_NOTIFIER_HSR_JOIN:
- err = dsa_switch_hsr_join(ds, info);
- break;
- case DSA_NOTIFIER_HSR_LEAVE:
- err = dsa_switch_hsr_leave(ds, info);
- break;
case DSA_NOTIFIER_LAG_CHANGE:
err = dsa_switch_lag_change(ds, info);
break;
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH net-next 08/15] net: dsa: remove cross-chip support for HSR
2022-01-04 17:14 ` [PATCH net-next 08/15] net: dsa: remove cross-chip support for HSR Vladimir Oltean
@ 2022-01-04 22:03 ` George McCollister
2022-01-04 22:20 ` Vladimir Oltean
0 siblings, 1 reply; 20+ messages in thread
From: George McCollister @ 2022-01-04 22:03 UTC (permalink / raw)
To: Vladimir Oltean
Cc: netdev, David S. Miller, Jakub Kicinski, Andrew Lunn,
Vivien Didelot, Florian Fainelli
On Tue, Jan 4, 2022 at 11:14 AM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> The cross-chip notifiers for HSR are bypass operations, meaning that
> even though all switches in a tree are notified, only the switch
> specified in the info structure is targeted.
>
> We can eliminate the unnecessary complexity by deleting the cross-chip
> notifier logic and calling the ds->ops straight from port.c.
>
> Cc: George McCollister <george.mccollister@gmail.com>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> net/dsa/dsa_priv.h | 2 --
> net/dsa/port.c | 20 ++++++--------------
> net/dsa/switch.c | 24 ------------------------
> 3 files changed, 6 insertions(+), 40 deletions(-)
>
> diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
> index 54c23479b9ba..b3386d408fc6 100644
> --- a/net/dsa/dsa_priv.h
> +++ b/net/dsa/dsa_priv.h
> @@ -25,8 +25,6 @@ enum {
> DSA_NOTIFIER_FDB_DEL,
> DSA_NOTIFIER_HOST_FDB_ADD,
> DSA_NOTIFIER_HOST_FDB_DEL,
> - DSA_NOTIFIER_HSR_JOIN,
> - DSA_NOTIFIER_HSR_LEAVE,
> DSA_NOTIFIER_LAG_CHANGE,
> DSA_NOTIFIER_LAG_JOIN,
> DSA_NOTIFIER_LAG_LEAVE,
> diff --git a/net/dsa/port.c b/net/dsa/port.c
> index 5c72f890c6a2..9e7c421c47b9 100644
> --- a/net/dsa/port.c
> +++ b/net/dsa/port.c
> @@ -1317,16 +1317,12 @@ EXPORT_SYMBOL_GPL(dsa_port_get_phy_sset_count);
>
> int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr)
> {
> - struct dsa_notifier_hsr_info info = {
> - .sw_index = dp->ds->index,
> - .port = dp->index,
> - .hsr = hsr,
> - };
> + struct dsa_switch *ds = dp->ds;
> int err;
>
> dp->hsr_dev = hsr;
>
> - err = dsa_port_notify(dp, DSA_NOTIFIER_HSR_JOIN, &info);
> + err = ds->ops->port_hsr_join(ds, dp->index, hsr);
> if (err)
> dp->hsr_dev = NULL;
>
> @@ -1335,20 +1331,16 @@ int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr)
>
> void dsa_port_hsr_leave(struct dsa_port *dp, struct net_device *hsr)
> {
> - struct dsa_notifier_hsr_info info = {
> - .sw_index = dp->ds->index,
> - .port = dp->index,
> - .hsr = hsr,
> - };
> + struct dsa_switch *ds = dp->ds;
> int err;
>
> dp->hsr_dev = NULL;
>
> - err = dsa_port_notify(dp, DSA_NOTIFIER_HSR_LEAVE, &info);
> + err = ds->ops->port_hsr_leave(ds, dp->index, hsr);
> if (err)
> dev_err(dp->ds->dev,
> - "port %d failed to notify DSA_NOTIFIER_HSR_LEAVE: %pe\n",
> - dp->index, ERR_PTR(err));
> + "port %d failed to leave HSR %s: %pe\n",
> + dp->index, hsr->name, ERR_PTR(err));
> }
>
> int dsa_port_tag_8021q_vlan_add(struct dsa_port *dp, u16 vid, bool broadcast)
> diff --git a/net/dsa/switch.c b/net/dsa/switch.c
> index a164ec02b4e9..e3c7d2627a61 100644
> --- a/net/dsa/switch.c
> +++ b/net/dsa/switch.c
> @@ -437,24 +437,6 @@ static int dsa_switch_fdb_del(struct dsa_switch *ds,
> return dsa_port_do_fdb_del(dp, info->addr, info->vid);
> }
>
> -static int dsa_switch_hsr_join(struct dsa_switch *ds,
> - struct dsa_notifier_hsr_info *info)
> -{
> - if (ds->index == info->sw_index && ds->ops->port_hsr_join)
> - return ds->ops->port_hsr_join(ds, info->port, info->hsr);
> -
> - return -EOPNOTSUPP;
> -}
> -
> -static int dsa_switch_hsr_leave(struct dsa_switch *ds,
> - struct dsa_notifier_hsr_info *info)
> -{
> - if (ds->index == info->sw_index && ds->ops->port_hsr_leave)
> - return ds->ops->port_hsr_leave(ds, info->port, info->hsr);
> -
> - return -EOPNOTSUPP;
> -}
> -
> static int dsa_switch_lag_change(struct dsa_switch *ds,
> struct dsa_notifier_lag_info *info)
> {
> @@ -729,12 +711,6 @@ static int dsa_switch_event(struct notifier_block *nb,
> case DSA_NOTIFIER_HOST_FDB_DEL:
> err = dsa_switch_host_fdb_del(ds, info);
> break;
> - case DSA_NOTIFIER_HSR_JOIN:
> - err = dsa_switch_hsr_join(ds, info);
> - break;
> - case DSA_NOTIFIER_HSR_LEAVE:
> - err = dsa_switch_hsr_leave(ds, info);
> - break;
> case DSA_NOTIFIER_LAG_CHANGE:
> err = dsa_switch_lag_change(ds, info);
> break;
> --
> 2.25.1
>
Looks good.
Reviewed-by: George McCollister <george.mccollister@gmail.com>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH net-next 08/15] net: dsa: remove cross-chip support for HSR
2022-01-04 22:03 ` George McCollister
@ 2022-01-04 22:20 ` Vladimir Oltean
0 siblings, 0 replies; 20+ messages in thread
From: Vladimir Oltean @ 2022-01-04 22:20 UTC (permalink / raw)
To: George McCollister
Cc: netdev, David S. Miller, Jakub Kicinski, Andrew Lunn,
Vivien Didelot, Florian Fainelli
On Tue, Jan 04, 2022 at 04:03:56PM -0600, George McCollister wrote:
> On Tue, Jan 4, 2022 at 11:14 AM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> >
> > The cross-chip notifiers for HSR are bypass operations, meaning that
> > even though all switches in a tree are notified, only the switch
> > specified in the info structure is targeted.
> >
> > We can eliminate the unnecessary complexity by deleting the cross-chip
> > notifier logic and calling the ds->ops straight from port.c.
> >
> > Cc: George McCollister <george.mccollister@gmail.com>
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > ---
> > net/dsa/dsa_priv.h | 2 --
> > net/dsa/port.c | 20 ++++++--------------
> > net/dsa/switch.c | 24 ------------------------
> > 3 files changed, 6 insertions(+), 40 deletions(-)
> >
> > diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
> > index 54c23479b9ba..b3386d408fc6 100644
> > --- a/net/dsa/dsa_priv.h
> > +++ b/net/dsa/dsa_priv.h
> > @@ -25,8 +25,6 @@ enum {
> > DSA_NOTIFIER_FDB_DEL,
> > DSA_NOTIFIER_HOST_FDB_ADD,
> > DSA_NOTIFIER_HOST_FDB_DEL,
> > - DSA_NOTIFIER_HSR_JOIN,
> > - DSA_NOTIFIER_HSR_LEAVE,
> > DSA_NOTIFIER_LAG_CHANGE,
> > DSA_NOTIFIER_LAG_JOIN,
> > DSA_NOTIFIER_LAG_LEAVE,
> > diff --git a/net/dsa/port.c b/net/dsa/port.c
> > index 5c72f890c6a2..9e7c421c47b9 100644
> > --- a/net/dsa/port.c
> > +++ b/net/dsa/port.c
> > @@ -1317,16 +1317,12 @@ EXPORT_SYMBOL_GPL(dsa_port_get_phy_sset_count);
> >
> > int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr)
> > {
> > - struct dsa_notifier_hsr_info info = {
> > - .sw_index = dp->ds->index,
> > - .port = dp->index,
> > - .hsr = hsr,
> > - };
> > + struct dsa_switch *ds = dp->ds;
> > int err;
> >
> > dp->hsr_dev = hsr;
> >
> > - err = dsa_port_notify(dp, DSA_NOTIFIER_HSR_JOIN, &info);
> > + err = ds->ops->port_hsr_join(ds, dp->index, hsr);
> > if (err)
> > dp->hsr_dev = NULL;
> >
> > @@ -1335,20 +1331,16 @@ int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr)
> >
> > void dsa_port_hsr_leave(struct dsa_port *dp, struct net_device *hsr)
> > {
> > - struct dsa_notifier_hsr_info info = {
> > - .sw_index = dp->ds->index,
> > - .port = dp->index,
> > - .hsr = hsr,
> > - };
> > + struct dsa_switch *ds = dp->ds;
> > int err;
> >
> > dp->hsr_dev = NULL;
> >
> > - err = dsa_port_notify(dp, DSA_NOTIFIER_HSR_LEAVE, &info);
> > + err = ds->ops->port_hsr_leave(ds, dp->index, hsr);
> > if (err)
> > dev_err(dp->ds->dev,
> > - "port %d failed to notify DSA_NOTIFIER_HSR_LEAVE: %pe\n",
> > - dp->index, ERR_PTR(err));
> > + "port %d failed to leave HSR %s: %pe\n",
> > + dp->index, hsr->name, ERR_PTR(err));
> > }
> >
> > int dsa_port_tag_8021q_vlan_add(struct dsa_port *dp, u16 vid, bool broadcast)
> > diff --git a/net/dsa/switch.c b/net/dsa/switch.c
> > index a164ec02b4e9..e3c7d2627a61 100644
> > --- a/net/dsa/switch.c
> > +++ b/net/dsa/switch.c
> > @@ -437,24 +437,6 @@ static int dsa_switch_fdb_del(struct dsa_switch *ds,
> > return dsa_port_do_fdb_del(dp, info->addr, info->vid);
> > }
> >
> > -static int dsa_switch_hsr_join(struct dsa_switch *ds,
> > - struct dsa_notifier_hsr_info *info)
> > -{
> > - if (ds->index == info->sw_index && ds->ops->port_hsr_join)
> > - return ds->ops->port_hsr_join(ds, info->port, info->hsr);
> > -
> > - return -EOPNOTSUPP;
> > -}
> > -
> > -static int dsa_switch_hsr_leave(struct dsa_switch *ds,
> > - struct dsa_notifier_hsr_info *info)
> > -{
> > - if (ds->index == info->sw_index && ds->ops->port_hsr_leave)
> > - return ds->ops->port_hsr_leave(ds, info->port, info->hsr);
> > -
> > - return -EOPNOTSUPP;
> > -}
> > -
> > static int dsa_switch_lag_change(struct dsa_switch *ds,
> > struct dsa_notifier_lag_info *info)
> > {
> > @@ -729,12 +711,6 @@ static int dsa_switch_event(struct notifier_block *nb,
> > case DSA_NOTIFIER_HOST_FDB_DEL:
> > err = dsa_switch_host_fdb_del(ds, info);
> > break;
> > - case DSA_NOTIFIER_HSR_JOIN:
> > - err = dsa_switch_hsr_join(ds, info);
> > - break;
> > - case DSA_NOTIFIER_HSR_LEAVE:
> > - err = dsa_switch_hsr_leave(ds, info);
> > - break;
> > case DSA_NOTIFIER_LAG_CHANGE:
> > err = dsa_switch_lag_change(ds, info);
> > break;
> > --
> > 2.25.1
> >
>
> Looks good.
>
> Reviewed-by: George McCollister <george.mccollister@gmail.com>
The irony is that the check for the presence of ds->ops->port_hsr_join
and ds->ops->port_hsr_leave vanished. So this is broken, we'd get a NULL
pointer dereference now instead of -EOPNOTSUPP. Sorry for the noise.
I'll add this to the list of things to change when I resend.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH net-next 09/15] net: dsa: move dsa_port :: stp_state near dsa_port :: mac
2022-01-04 17:13 [PATCH net-next 00/15] DSA miscellaneous cleanups Vladimir Oltean
` (7 preceding siblings ...)
2022-01-04 17:14 ` [PATCH net-next 08/15] net: dsa: remove cross-chip support for HSR Vladimir Oltean
@ 2022-01-04 17:14 ` Vladimir Oltean
2022-01-04 17:14 ` [PATCH net-next 10/15] net: dsa: merge all bools of struct dsa_port into a single u8 Vladimir Oltean
` (5 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Vladimir Oltean @ 2022-01-04 17:14 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
Florian Fainelli
The MAC address of a port is 6 octets in size, and this creates a 2
octet hole after it. There are some other u8 members of struct dsa_port
that we can put in that hole. One such member is the stp_state.
Before:
pahole -C dsa_port net/dsa/slave.o
struct dsa_port {
union {
struct net_device * master; /* 0 8 */
struct net_device * slave; /* 0 8 */
}; /* 0 8 */
const struct dsa_device_ops * tag_ops; /* 8 8 */
struct dsa_switch_tree * dst; /* 16 8 */
struct sk_buff * (*rcv)(struct sk_buff *, struct net_device *); /* 24 8 */
enum {
DSA_PORT_TYPE_UNUSED = 0,
DSA_PORT_TYPE_CPU = 1,
DSA_PORT_TYPE_DSA = 2,
DSA_PORT_TYPE_USER = 3,
} type; /* 32 4 */
/* XXX 4 bytes hole, try to pack */
struct dsa_switch * ds; /* 40 8 */
unsigned int index; /* 48 4 */
/* XXX 4 bytes hole, try to pack */
const char * name; /* 56 8 */
/* --- cacheline 1 boundary (64 bytes) --- */
struct dsa_port * cpu_dp; /* 64 8 */
u8 mac[6]; /* 72 6 */
/* XXX 2 bytes hole, try to pack */
struct device_node * dn; /* 80 8 */
unsigned int ageing_time; /* 88 4 */
bool vlan_filtering; /* 92 1 */
bool learning; /* 93 1 */
u8 stp_state; /* 94 1 */
/* XXX 1 byte hole, try to pack */
struct dsa_bridge * bridge; /* 96 8 */
struct devlink_port devlink_port; /* 104 288 */
/* --- cacheline 6 boundary (384 bytes) was 8 bytes ago --- */
bool devlink_port_setup; /* 392 1 */
/* XXX 7 bytes hole, try to pack */
struct phylink * pl; /* 400 8 */
struct phylink_config pl_config; /* 408 40 */
/* --- cacheline 7 boundary (448 bytes) --- */
struct net_device * lag_dev; /* 448 8 */
bool lag_tx_enabled; /* 456 1 */
/* XXX 7 bytes hole, try to pack */
struct net_device * hsr_dev; /* 464 8 */
struct list_head list; /* 472 16 */
const struct ethtool_ops * orig_ethtool_ops; /* 488 8 */
const struct dsa_netdevice_ops * netdev_ops; /* 496 8 */
struct mutex addr_lists_lock; /* 504 32 */
/* --- cacheline 8 boundary (512 bytes) was 24 bytes ago --- */
struct list_head fdbs; /* 536 16 */
struct list_head mdbs; /* 552 16 */
bool setup; /* 568 1 */
/* size: 576, cachelines: 9, members: 30 */
/* sum members: 544, holes: 6, sum holes: 25 */
/* padding: 7 */
};
After:
pahole -C dsa_port net/dsa/slave.o
struct dsa_port {
union {
struct net_device * master; /* 0 8 */
struct net_device * slave; /* 0 8 */
}; /* 0 8 */
const struct dsa_device_ops * tag_ops; /* 8 8 */
struct dsa_switch_tree * dst; /* 16 8 */
struct sk_buff * (*rcv)(struct sk_buff *, struct net_device *); /* 24 8 */
enum {
DSA_PORT_TYPE_UNUSED = 0,
DSA_PORT_TYPE_CPU = 1,
DSA_PORT_TYPE_DSA = 2,
DSA_PORT_TYPE_USER = 3,
} type; /* 32 4 */
/* XXX 4 bytes hole, try to pack */
struct dsa_switch * ds; /* 40 8 */
unsigned int index; /* 48 4 */
/* XXX 4 bytes hole, try to pack */
const char * name; /* 56 8 */
/* --- cacheline 1 boundary (64 bytes) --- */
struct dsa_port * cpu_dp; /* 64 8 */
u8 mac[6]; /* 72 6 */
u8 stp_state; /* 78 1 */
/* XXX 1 byte hole, try to pack */
struct device_node * dn; /* 80 8 */
unsigned int ageing_time; /* 88 4 */
bool vlan_filtering; /* 92 1 */
bool learning; /* 93 1 */
/* XXX 2 bytes hole, try to pack */
struct dsa_bridge * bridge; /* 96 8 */
struct devlink_port devlink_port; /* 104 288 */
/* --- cacheline 6 boundary (384 bytes) was 8 bytes ago --- */
bool devlink_port_setup; /* 392 1 */
/* XXX 7 bytes hole, try to pack */
struct phylink * pl; /* 400 8 */
struct phylink_config pl_config; /* 408 40 */
/* --- cacheline 7 boundary (448 bytes) --- */
struct net_device * lag_dev; /* 448 8 */
bool lag_tx_enabled; /* 456 1 */
/* XXX 7 bytes hole, try to pack */
struct net_device * hsr_dev; /* 464 8 */
struct list_head list; /* 472 16 */
const struct ethtool_ops * orig_ethtool_ops; /* 488 8 */
const struct dsa_netdevice_ops * netdev_ops; /* 496 8 */
struct mutex addr_lists_lock; /* 504 32 */
/* --- cacheline 8 boundary (512 bytes) was 24 bytes ago --- */
struct list_head fdbs; /* 536 16 */
struct list_head mdbs; /* 552 16 */
bool setup; /* 568 1 */
/* size: 576, cachelines: 9, members: 30 */
/* sum members: 544, holes: 6, sum holes: 25 */
/* padding: 7 */
};
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
include/net/dsa.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index f16959444ae1..8878f9ce251b 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -258,12 +258,14 @@ struct dsa_port {
const char *name;
struct dsa_port *cpu_dp;
u8 mac[ETH_ALEN];
+
+ u8 stp_state;
+
struct device_node *dn;
unsigned int ageing_time;
bool vlan_filtering;
/* Managed by DSA on user ports and by drivers on CPU and DSA ports */
bool learning;
- u8 stp_state;
struct dsa_bridge *bridge;
struct devlink_port devlink_port;
bool devlink_port_setup;
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH net-next 10/15] net: dsa: merge all bools of struct dsa_port into a single u8
2022-01-04 17:13 [PATCH net-next 00/15] DSA miscellaneous cleanups Vladimir Oltean
` (8 preceding siblings ...)
2022-01-04 17:14 ` [PATCH net-next 09/15] net: dsa: move dsa_port :: stp_state near dsa_port :: mac Vladimir Oltean
@ 2022-01-04 17:14 ` Vladimir Oltean
2022-01-04 17:14 ` [PATCH net-next 11/15] net: dsa: move dsa_port :: type near dsa_port :: index Vladimir Oltean
` (4 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Vladimir Oltean @ 2022-01-04 17:14 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
Florian Fainelli
struct dsa_port has 5 bool members which create quite a number of 7 byte
holes in the structure layout. By merging them all into bitfields of an
u8, and placing that u8 in the 1-byte hole after dp->mac and dp->stp_state,
we can reduce the structure size from 576 bytes to 552 bytes on arm64.
Before:
pahole -C dsa_port net/dsa/slave.o
struct dsa_port {
union {
struct net_device * master; /* 0 8 */
struct net_device * slave; /* 0 8 */
}; /* 0 8 */
const struct dsa_device_ops * tag_ops; /* 8 8 */
struct dsa_switch_tree * dst; /* 16 8 */
struct sk_buff * (*rcv)(struct sk_buff *, struct net_device *); /* 24 8 */
enum {
DSA_PORT_TYPE_UNUSED = 0,
DSA_PORT_TYPE_CPU = 1,
DSA_PORT_TYPE_DSA = 2,
DSA_PORT_TYPE_USER = 3,
} type; /* 32 4 */
/* XXX 4 bytes hole, try to pack */
struct dsa_switch * ds; /* 40 8 */
unsigned int index; /* 48 4 */
/* XXX 4 bytes hole, try to pack */
const char * name; /* 56 8 */
/* --- cacheline 1 boundary (64 bytes) --- */
struct dsa_port * cpu_dp; /* 64 8 */
u8 mac[6]; /* 72 6 */
u8 stp_state; /* 78 1 */
/* XXX 1 byte hole, try to pack */
struct device_node * dn; /* 80 8 */
unsigned int ageing_time; /* 88 4 */
bool vlan_filtering; /* 92 1 */
bool learning; /* 93 1 */
/* XXX 2 bytes hole, try to pack */
struct dsa_bridge * bridge; /* 96 8 */
struct devlink_port devlink_port; /* 104 288 */
/* --- cacheline 6 boundary (384 bytes) was 8 bytes ago --- */
bool devlink_port_setup; /* 392 1 */
/* XXX 7 bytes hole, try to pack */
struct phylink * pl; /* 400 8 */
struct phylink_config pl_config; /* 408 40 */
/* --- cacheline 7 boundary (448 bytes) --- */
struct net_device * lag_dev; /* 448 8 */
bool lag_tx_enabled; /* 456 1 */
/* XXX 7 bytes hole, try to pack */
struct net_device * hsr_dev; /* 464 8 */
struct list_head list; /* 472 16 */
const struct ethtool_ops * orig_ethtool_ops; /* 488 8 */
const struct dsa_netdevice_ops * netdev_ops; /* 496 8 */
struct mutex addr_lists_lock; /* 504 32 */
/* --- cacheline 8 boundary (512 bytes) was 24 bytes ago --- */
struct list_head fdbs; /* 536 16 */
struct list_head mdbs; /* 552 16 */
bool setup; /* 568 1 */
/* size: 576, cachelines: 9, members: 30 */
/* sum members: 544, holes: 6, sum holes: 25 */
/* padding: 7 */
};
After:
pahole -C dsa_port net/dsa/slave.o
struct dsa_port {
union {
struct net_device * master; /* 0 8 */
struct net_device * slave; /* 0 8 */
}; /* 0 8 */
const struct dsa_device_ops * tag_ops; /* 8 8 */
struct dsa_switch_tree * dst; /* 16 8 */
struct sk_buff * (*rcv)(struct sk_buff *, struct net_device *); /* 24 8 */
enum {
DSA_PORT_TYPE_UNUSED = 0,
DSA_PORT_TYPE_CPU = 1,
DSA_PORT_TYPE_DSA = 2,
DSA_PORT_TYPE_USER = 3,
} type; /* 32 4 */
/* XXX 4 bytes hole, try to pack */
struct dsa_switch * ds; /* 40 8 */
unsigned int index; /* 48 4 */
/* XXX 4 bytes hole, try to pack */
const char * name; /* 56 8 */
/* --- cacheline 1 boundary (64 bytes) --- */
struct dsa_port * cpu_dp; /* 64 8 */
u8 mac[6]; /* 72 6 */
u8 stp_state; /* 78 1 */
u8 vlan_filtering:1; /* 79: 0 1 */
u8 learning:1; /* 79: 1 1 */
u8 lag_tx_enabled:1; /* 79: 2 1 */
u8 devlink_port_setup:1; /* 79: 3 1 */
u8 setup:1; /* 79: 4 1 */
/* XXX 3 bits hole, try to pack */
struct device_node * dn; /* 80 8 */
unsigned int ageing_time; /* 88 4 */
/* XXX 4 bytes hole, try to pack */
struct dsa_bridge * bridge; /* 96 8 */
struct devlink_port devlink_port; /* 104 288 */
/* --- cacheline 6 boundary (384 bytes) was 8 bytes ago --- */
struct phylink * pl; /* 392 8 */
struct phylink_config pl_config; /* 400 40 */
struct net_device * lag_dev; /* 440 8 */
/* --- cacheline 7 boundary (448 bytes) --- */
struct net_device * hsr_dev; /* 448 8 */
struct list_head list; /* 456 16 */
const struct ethtool_ops * orig_ethtool_ops; /* 472 8 */
const struct dsa_netdevice_ops * netdev_ops; /* 480 8 */
struct mutex addr_lists_lock; /* 488 32 */
/* --- cacheline 8 boundary (512 bytes) was 8 bytes ago --- */
struct list_head fdbs; /* 520 16 */
struct list_head mdbs; /* 536 16 */
/* size: 552, cachelines: 9, members: 30 */
/* sum members: 539, holes: 3, sum holes: 12 */
/* sum bitfield members: 5 bits, bit holes: 1, sum bit holes: 3 bits */
/* last cacheline: 40 bytes */
};
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
include/net/dsa.h | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 8878f9ce251b..a8f0037b58e2 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -261,18 +261,23 @@ struct dsa_port {
u8 stp_state;
+ u8 vlan_filtering:1,
+ /* Managed by DSA on user ports and by
+ * drivers on CPU and DSA ports
+ */
+ learning:1,
+ lag_tx_enabled:1,
+ devlink_port_setup:1,
+ setup:1;
+
struct device_node *dn;
unsigned int ageing_time;
- bool vlan_filtering;
- /* Managed by DSA on user ports and by drivers on CPU and DSA ports */
- bool learning;
+
struct dsa_bridge *bridge;
struct devlink_port devlink_port;
- bool devlink_port_setup;
struct phylink *pl;
struct phylink_config pl_config;
struct net_device *lag_dev;
- bool lag_tx_enabled;
struct net_device *hsr_dev;
struct list_head list;
@@ -293,8 +298,6 @@ struct dsa_port {
struct mutex addr_lists_lock;
struct list_head fdbs;
struct list_head mdbs;
-
- bool setup;
};
/* TODO: ideally DSA ports would have a single dp->link_dp member,
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH net-next 11/15] net: dsa: move dsa_port :: type near dsa_port :: index
2022-01-04 17:13 [PATCH net-next 00/15] DSA miscellaneous cleanups Vladimir Oltean
` (9 preceding siblings ...)
2022-01-04 17:14 ` [PATCH net-next 10/15] net: dsa: merge all bools of struct dsa_port into a single u8 Vladimir Oltean
@ 2022-01-04 17:14 ` Vladimir Oltean
2022-01-04 17:14 ` [PATCH net-next 12/15] net: dsa: merge all bools of struct dsa_switch into a single u32 Vladimir Oltean
` (3 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Vladimir Oltean @ 2022-01-04 17:14 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
Florian Fainelli
Both dsa_port :: type and dsa_port :: index introduce a 4 octet hole
after them, so we can group them together and the holes would be
eliminated, turning 16 octets of storage into just 8. This makes the
cpu_dp pointer fit in the first cache line, which is good, because
dsa_slave_to_master(), called by dsa_enqueue_skb(), uses it.
Before:
pahole -C dsa_port net/dsa/slave.o
struct dsa_port {
union {
struct net_device * master; /* 0 8 */
struct net_device * slave; /* 0 8 */
}; /* 0 8 */
const struct dsa_device_ops * tag_ops; /* 8 8 */
struct dsa_switch_tree * dst; /* 16 8 */
struct sk_buff * (*rcv)(struct sk_buff *, struct net_device *); /* 24 8 */
enum {
DSA_PORT_TYPE_UNUSED = 0,
DSA_PORT_TYPE_CPU = 1,
DSA_PORT_TYPE_DSA = 2,
DSA_PORT_TYPE_USER = 3,
} type; /* 32 4 */
/* XXX 4 bytes hole, try to pack */
struct dsa_switch * ds; /* 40 8 */
unsigned int index; /* 48 4 */
/* XXX 4 bytes hole, try to pack */
const char * name; /* 56 8 */
/* --- cacheline 1 boundary (64 bytes) --- */
struct dsa_port * cpu_dp; /* 64 8 */
u8 mac[6]; /* 72 6 */
u8 stp_state; /* 78 1 */
u8 vlan_filtering:1; /* 79: 0 1 */
u8 learning:1; /* 79: 1 1 */
u8 lag_tx_enabled:1; /* 79: 2 1 */
u8 devlink_port_setup:1; /* 79: 3 1 */
u8 setup:1; /* 79: 4 1 */
/* XXX 3 bits hole, try to pack */
struct device_node * dn; /* 80 8 */
unsigned int ageing_time; /* 88 4 */
/* XXX 4 bytes hole, try to pack */
struct dsa_bridge * bridge; /* 96 8 */
struct devlink_port devlink_port; /* 104 288 */
/* --- cacheline 6 boundary (384 bytes) was 8 bytes ago --- */
struct phylink * pl; /* 392 8 */
struct phylink_config pl_config; /* 400 40 */
struct net_device * lag_dev; /* 440 8 */
/* --- cacheline 7 boundary (448 bytes) --- */
struct net_device * hsr_dev; /* 448 8 */
struct list_head list; /* 456 16 */
const struct ethtool_ops * orig_ethtool_ops; /* 472 8 */
const struct dsa_netdevice_ops * netdev_ops; /* 480 8 */
struct mutex addr_lists_lock; /* 488 32 */
/* --- cacheline 8 boundary (512 bytes) was 8 bytes ago --- */
struct list_head fdbs; /* 520 16 */
struct list_head mdbs; /* 536 16 */
/* size: 552, cachelines: 9, members: 30 */
/* sum members: 539, holes: 3, sum holes: 12 */
/* sum bitfield members: 5 bits, bit holes: 1, sum bit holes: 3 bits */
/* last cacheline: 40 bytes */
};
After:
pahole -C dsa_port net/dsa/slave.o
struct dsa_port {
union {
struct net_device * master; /* 0 8 */
struct net_device * slave; /* 0 8 */
}; /* 0 8 */
const struct dsa_device_ops * tag_ops; /* 8 8 */
struct dsa_switch_tree * dst; /* 16 8 */
struct sk_buff * (*rcv)(struct sk_buff *, struct net_device *); /* 24 8 */
struct dsa_switch * ds; /* 32 8 */
unsigned int index; /* 40 4 */
enum {
DSA_PORT_TYPE_UNUSED = 0,
DSA_PORT_TYPE_CPU = 1,
DSA_PORT_TYPE_DSA = 2,
DSA_PORT_TYPE_USER = 3,
} type; /* 44 4 */
const char * name; /* 48 8 */
struct dsa_port * cpu_dp; /* 56 8 */
/* --- cacheline 1 boundary (64 bytes) --- */
u8 mac[6]; /* 64 6 */
u8 stp_state; /* 70 1 */
u8 vlan_filtering:1; /* 71: 0 1 */
u8 learning:1; /* 71: 1 1 */
u8 lag_tx_enabled:1; /* 71: 2 1 */
u8 devlink_port_setup:1; /* 71: 3 1 */
u8 setup:1; /* 71: 4 1 */
/* XXX 3 bits hole, try to pack */
struct device_node * dn; /* 72 8 */
unsigned int ageing_time; /* 80 4 */
/* XXX 4 bytes hole, try to pack */
struct dsa_bridge * bridge; /* 88 8 */
struct devlink_port devlink_port; /* 96 288 */
/* --- cacheline 6 boundary (384 bytes) --- */
struct phylink * pl; /* 384 8 */
struct phylink_config pl_config; /* 392 40 */
struct net_device * lag_dev; /* 432 8 */
struct net_device * hsr_dev; /* 440 8 */
/* --- cacheline 7 boundary (448 bytes) --- */
struct list_head list; /* 448 16 */
const struct ethtool_ops * orig_ethtool_ops; /* 464 8 */
const struct dsa_netdevice_ops * netdev_ops; /* 472 8 */
struct mutex addr_lists_lock; /* 480 32 */
/* --- cacheline 8 boundary (512 bytes) --- */
struct list_head fdbs; /* 512 16 */
struct list_head mdbs; /* 528 16 */
/* size: 544, cachelines: 9, members: 30 */
/* sum members: 539, holes: 1, sum holes: 4 */
/* sum bitfield members: 5 bits, bit holes: 1, sum bit holes: 3 bits */
/* last cacheline: 32 bytes */
};
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
include/net/dsa.h | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index a8f0037b58e2..5e42fa7ea377 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -246,6 +246,10 @@ struct dsa_port {
struct dsa_switch_tree *dst;
struct sk_buff *(*rcv)(struct sk_buff *skb, struct net_device *dev);
+ struct dsa_switch *ds;
+
+ unsigned int index;
+
enum {
DSA_PORT_TYPE_UNUSED = 0,
DSA_PORT_TYPE_CPU,
@@ -253,8 +257,6 @@ struct dsa_port {
DSA_PORT_TYPE_USER,
} type;
- struct dsa_switch *ds;
- unsigned int index;
const char *name;
struct dsa_port *cpu_dp;
u8 mac[ETH_ALEN];
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH net-next 12/15] net: dsa: merge all bools of struct dsa_switch into a single u32
2022-01-04 17:13 [PATCH net-next 00/15] DSA miscellaneous cleanups Vladimir Oltean
` (10 preceding siblings ...)
2022-01-04 17:14 ` [PATCH net-next 11/15] net: dsa: move dsa_port :: type near dsa_port :: index Vladimir Oltean
@ 2022-01-04 17:14 ` Vladimir Oltean
2022-01-04 17:14 ` [PATCH net-next 13/15] net: dsa: make dsa_switch :: num_ports an unsigned int Vladimir Oltean
` (2 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Vladimir Oltean @ 2022-01-04 17:14 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
Florian Fainelli
struct dsa_switch has 9 boolean properties, many of which are in fact
set by drivers for custom behavior (vlan_filtering_is_global,
needs_standalone_vlan_filtering, etc etc). The binary layout of the
structure could be improved. For example, the "bool setup" at the
beginning introduces a gratuitous 7 byte hole in the first cache line.
The change merges all boolean properties into bitfields of an u32, and
places that u32 in the first cache line of the structure, since many
bools are accessed from the data path (untag_bridge_pvid, vlan_filtering,
vlan_filtering_is_global).
We place this u32 after the existing ds->index, which is also 4 bytes in
size. As a positive side effect, ds->tagger_data now fits into the first
cache line too, because 4 bytes are saved.
Before:
pahole -C dsa_switch net/dsa/slave.o
struct dsa_switch {
bool setup; /* 0 1 */
/* XXX 7 bytes hole, try to pack */
struct device * dev; /* 8 8 */
struct dsa_switch_tree * dst; /* 16 8 */
unsigned int index; /* 24 4 */
/* XXX 4 bytes hole, try to pack */
struct notifier_block nb; /* 32 24 */
/* XXX last struct has 4 bytes of padding */
void * priv; /* 56 8 */
/* --- cacheline 1 boundary (64 bytes) --- */
void * tagger_data; /* 64 8 */
struct dsa_chip_data * cd; /* 72 8 */
const struct dsa_switch_ops * ops; /* 80 8 */
u32 phys_mii_mask; /* 88 4 */
/* XXX 4 bytes hole, try to pack */
struct mii_bus * slave_mii_bus; /* 96 8 */
unsigned int ageing_time_min; /* 104 4 */
unsigned int ageing_time_max; /* 108 4 */
struct dsa_8021q_context * tag_8021q_ctx; /* 112 8 */
struct devlink * devlink; /* 120 8 */
/* --- cacheline 2 boundary (128 bytes) --- */
unsigned int num_tx_queues; /* 128 4 */
bool vlan_filtering_is_global; /* 132 1 */
bool needs_standalone_vlan_filtering; /* 133 1 */
bool configure_vlan_while_not_filtering; /* 134 1 */
bool untag_bridge_pvid; /* 135 1 */
bool assisted_learning_on_cpu_port; /* 136 1 */
bool vlan_filtering; /* 137 1 */
bool pcs_poll; /* 138 1 */
bool mtu_enforcement_ingress; /* 139 1 */
unsigned int num_lag_ids; /* 140 4 */
unsigned int max_num_bridges; /* 144 4 */
/* XXX 4 bytes hole, try to pack */
size_t num_ports; /* 152 8 */
/* size: 160, cachelines: 3, members: 27 */
/* sum members: 141, holes: 4, sum holes: 19 */
/* paddings: 1, sum paddings: 4 */
/* last cacheline: 32 bytes */
};
After:
pahole -C dsa_switch net/dsa/slave.o
struct dsa_switch {
struct device * dev; /* 0 8 */
struct dsa_switch_tree * dst; /* 8 8 */
unsigned int index; /* 16 4 */
u32 setup:1; /* 20: 0 4 */
u32 vlan_filtering_is_global:1; /* 20: 1 4 */
u32 needs_standalone_vlan_filtering:1; /* 20: 2 4 */
u32 configure_vlan_while_not_filtering:1; /* 20: 3 4 */
u32 untag_bridge_pvid:1; /* 20: 4 4 */
u32 assisted_learning_on_cpu_port:1; /* 20: 5 4 */
u32 vlan_filtering:1; /* 20: 6 4 */
u32 pcs_poll:1; /* 20: 7 4 */
u32 mtu_enforcement_ingress:1; /* 20: 8 4 */
/* XXX 23 bits hole, try to pack */
struct notifier_block nb; /* 24 24 */
/* XXX last struct has 4 bytes of padding */
void * priv; /* 48 8 */
void * tagger_data; /* 56 8 */
/* --- cacheline 1 boundary (64 bytes) --- */
struct dsa_chip_data * cd; /* 64 8 */
const struct dsa_switch_ops * ops; /* 72 8 */
u32 phys_mii_mask; /* 80 4 */
/* XXX 4 bytes hole, try to pack */
struct mii_bus * slave_mii_bus; /* 88 8 */
unsigned int ageing_time_min; /* 96 4 */
unsigned int ageing_time_max; /* 100 4 */
struct dsa_8021q_context * tag_8021q_ctx; /* 104 8 */
struct devlink * devlink; /* 112 8 */
unsigned int num_tx_queues; /* 120 4 */
unsigned int num_lag_ids; /* 124 4 */
/* --- cacheline 2 boundary (128 bytes) --- */
unsigned int max_num_bridges; /* 128 4 */
/* XXX 4 bytes hole, try to pack */
size_t num_ports; /* 136 8 */
/* size: 144, cachelines: 3, members: 27 */
/* sum members: 132, holes: 2, sum holes: 8 */
/* sum bitfield members: 9 bits, bit holes: 1, sum bit holes: 23 bits */
/* paddings: 1, sum paddings: 4 */
/* last cacheline: 16 bytes */
};
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
include/net/dsa.h | 97 +++++++++++++++++++++++++----------------------
1 file changed, 51 insertions(+), 46 deletions(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 5e42fa7ea377..a8a586039033 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -321,8 +321,6 @@ struct dsa_mac_addr {
};
struct dsa_switch {
- bool setup;
-
struct device *dev;
/*
@@ -331,6 +329,57 @@ struct dsa_switch {
struct dsa_switch_tree *dst;
unsigned int index;
+ u32 setup:1,
+ /* Disallow bridge core from requesting
+ * different VLAN awareness settings on ports
+ * if not hardware-supported
+ */
+ vlan_filtering_is_global:1,
+ /* Keep VLAN filtering enabled on ports not
+ * offloading any upper
+ */
+ needs_standalone_vlan_filtering:1,
+ /* Pass .port_vlan_add and .port_vlan_del to
+ * drivers even for bridges that have
+ * vlan_filtering=0. All drivers should ideally
+ * set this (and then the option would get
+ * removed), but it is unknown whether this
+ * would break things or not.
+ */
+ configure_vlan_while_not_filtering:1,
+ /* If the switch driver always programs the CPU
+ * port as egress tagged despite the VLAN
+ * configuration indicating otherwise, then
+ * setting @untag_bridge_pvid will force the
+ * DSA receive path to pop the bridge's
+ * default_pvid VLAN tagged frames to offer a
+ * consistent behavior between a
+ * vlan_filtering=0 and vlan_filtering=1 bridge
+ * device.
+ */
+ untag_bridge_pvid:1,
+ /* Let DSA manage the FDB entries towards the
+ * CPU, based on the software bridge database.
+ */
+ assisted_learning_on_cpu_port:1,
+ /* In case vlan_filtering_is_global is set, the
+ * VLAN awareness state should be retrieved
+ * from here and not from the per-port
+ * settings.
+ */
+ vlan_filtering:1,
+ /* MAC PCS does not provide link state change
+ * interrupt, and requires polling. Flag passed
+ * on to PHYLINK.
+ */
+ pcs_poll:1,
+ /* For switches that only have the MRU
+ * configurable. To ensure the configured MTU
+ * is not exceeded, normalization of MRU on all
+ * bridged interfaces is needed.
+ */
+ mtu_enforcement_ingress:1;
+
/* Listener for switch fabric events */
struct notifier_block nb;
@@ -371,50 +420,6 @@ struct dsa_switch {
/* Number of switch port queues */
unsigned int num_tx_queues;
- /* Disallow bridge core from requesting different VLAN awareness
- * settings on ports if not hardware-supported
- */
- bool vlan_filtering_is_global;
-
- /* Keep VLAN filtering enabled on ports not offloading any upper. */
- bool needs_standalone_vlan_filtering;
-
- /* Pass .port_vlan_add and .port_vlan_del to drivers even for bridges
- * that have vlan_filtering=0. All drivers should ideally set this (and
- * then the option would get removed), but it is unknown whether this
- * would break things or not.
- */
- bool configure_vlan_while_not_filtering;
-
- /* If the switch driver always programs the CPU port as egress tagged
- * despite the VLAN configuration indicating otherwise, then setting
- * @untag_bridge_pvid will force the DSA receive path to pop the bridge's
- * default_pvid VLAN tagged frames to offer a consistent behavior
- * between a vlan_filtering=0 and vlan_filtering=1 bridge device.
- */
- bool untag_bridge_pvid;
-
- /* Let DSA manage the FDB entries towards the CPU, based on the
- * software bridge database.
- */
- bool assisted_learning_on_cpu_port;
-
- /* In case vlan_filtering_is_global is set, the VLAN awareness state
- * should be retrieved from here and not from the per-port settings.
- */
- bool vlan_filtering;
-
- /* MAC PCS does not provide link state change interrupt, and requires
- * polling. Flag passed on to PHYLINK.
- */
- bool pcs_poll;
-
- /* For switches that only have the MRU configurable. To ensure the
- * configured MTU is not exceeded, normalization of MRU on all bridged
- * interfaces is needed.
- */
- bool mtu_enforcement_ingress;
-
/* Drivers that benefit from having an ID associated with each
* offloaded LAG should set this to the maximum number of
* supported IDs. DSA will then maintain a mapping of _at
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH net-next 13/15] net: dsa: make dsa_switch :: num_ports an unsigned int
2022-01-04 17:13 [PATCH net-next 00/15] DSA miscellaneous cleanups Vladimir Oltean
` (11 preceding siblings ...)
2022-01-04 17:14 ` [PATCH net-next 12/15] net: dsa: merge all bools of struct dsa_switch into a single u32 Vladimir Oltean
@ 2022-01-04 17:14 ` Vladimir Oltean
2022-01-04 17:14 ` [PATCH net-next 14/15] net: dsa: move dsa_switch_tree :: ports and lags to first cache line Vladimir Oltean
2022-01-04 17:14 ` [PATCH net-next 15/15] net: dsa: combine two holes in struct dsa_switch_tree Vladimir Oltean
14 siblings, 0 replies; 20+ messages in thread
From: Vladimir Oltean @ 2022-01-04 17:14 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
Florian Fainelli
Currently, num_ports is declared as size_t, which is defined as
__kernel_ulong_t, therefore it occupies 8 bytes of memory.
Even switches with port numbers in the range of tens are exotic, so
there is no need for this amount of storage.
Additionally, because the max_num_bridges member right above it is also
4 bytes, it means the compiler needs to add padding between the last 2
fields. By reducing the size, we don't need that padding and can reduce
the struct size.
Before:
pahole -C dsa_switch net/dsa/slave.o
struct dsa_switch {
struct device * dev; /* 0 8 */
struct dsa_switch_tree * dst; /* 8 8 */
unsigned int index; /* 16 4 */
u32 setup:1; /* 20: 0 4 */
u32 vlan_filtering_is_global:1; /* 20: 1 4 */
u32 needs_standalone_vlan_filtering:1; /* 20: 2 4 */
u32 configure_vlan_while_not_filtering:1; /* 20: 3 4 */
u32 untag_bridge_pvid:1; /* 20: 4 4 */
u32 assisted_learning_on_cpu_port:1; /* 20: 5 4 */
u32 vlan_filtering:1; /* 20: 6 4 */
u32 pcs_poll:1; /* 20: 7 4 */
u32 mtu_enforcement_ingress:1; /* 20: 8 4 */
/* XXX 23 bits hole, try to pack */
struct notifier_block nb; /* 24 24 */
/* XXX last struct has 4 bytes of padding */
void * priv; /* 48 8 */
void * tagger_data; /* 56 8 */
/* --- cacheline 1 boundary (64 bytes) --- */
struct dsa_chip_data * cd; /* 64 8 */
const struct dsa_switch_ops * ops; /* 72 8 */
u32 phys_mii_mask; /* 80 4 */
/* XXX 4 bytes hole, try to pack */
struct mii_bus * slave_mii_bus; /* 88 8 */
unsigned int ageing_time_min; /* 96 4 */
unsigned int ageing_time_max; /* 100 4 */
struct dsa_8021q_context * tag_8021q_ctx; /* 104 8 */
struct devlink * devlink; /* 112 8 */
unsigned int num_tx_queues; /* 120 4 */
unsigned int num_lag_ids; /* 124 4 */
/* --- cacheline 2 boundary (128 bytes) --- */
unsigned int max_num_bridges; /* 128 4 */
/* XXX 4 bytes hole, try to pack */
size_t num_ports; /* 136 8 */
/* size: 144, cachelines: 3, members: 27 */
/* sum members: 132, holes: 2, sum holes: 8 */
/* sum bitfield members: 9 bits, bit holes: 1, sum bit holes: 23 bits */
/* paddings: 1, sum paddings: 4 */
/* last cacheline: 16 bytes */
};
After:
pahole -C dsa_switch net/dsa/slave.o
struct dsa_switch {
struct device * dev; /* 0 8 */
struct dsa_switch_tree * dst; /* 8 8 */
unsigned int index; /* 16 4 */
u32 setup:1; /* 20: 0 4 */
u32 vlan_filtering_is_global:1; /* 20: 1 4 */
u32 needs_standalone_vlan_filtering:1; /* 20: 2 4 */
u32 configure_vlan_while_not_filtering:1; /* 20: 3 4 */
u32 untag_bridge_pvid:1; /* 20: 4 4 */
u32 assisted_learning_on_cpu_port:1; /* 20: 5 4 */
u32 vlan_filtering:1; /* 20: 6 4 */
u32 pcs_poll:1; /* 20: 7 4 */
u32 mtu_enforcement_ingress:1; /* 20: 8 4 */
/* XXX 23 bits hole, try to pack */
struct notifier_block nb; /* 24 24 */
/* XXX last struct has 4 bytes of padding */
void * priv; /* 48 8 */
void * tagger_data; /* 56 8 */
/* --- cacheline 1 boundary (64 bytes) --- */
struct dsa_chip_data * cd; /* 64 8 */
const struct dsa_switch_ops * ops; /* 72 8 */
u32 phys_mii_mask; /* 80 4 */
/* XXX 4 bytes hole, try to pack */
struct mii_bus * slave_mii_bus; /* 88 8 */
unsigned int ageing_time_min; /* 96 4 */
unsigned int ageing_time_max; /* 100 4 */
struct dsa_8021q_context * tag_8021q_ctx; /* 104 8 */
struct devlink * devlink; /* 112 8 */
unsigned int num_tx_queues; /* 120 4 */
unsigned int num_lag_ids; /* 124 4 */
/* --- cacheline 2 boundary (128 bytes) --- */
unsigned int max_num_bridges; /* 128 4 */
unsigned int num_ports; /* 132 4 */
/* size: 136, cachelines: 3, members: 27 */
/* sum members: 128, holes: 1, sum holes: 4 */
/* sum bitfield members: 9 bits, bit holes: 1, sum bit holes: 23 bits */
/* paddings: 1, sum paddings: 4 */
/* last cacheline: 8 bytes */
};
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
include/net/dsa.h | 2 +-
net/dsa/dsa2.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index a8a586039033..fef9d8bb5190 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -435,7 +435,7 @@ struct dsa_switch {
*/
unsigned int max_num_bridges;
- size_t num_ports;
+ unsigned int num_ports;
};
static inline struct dsa_port *dsa_to_port(struct dsa_switch *ds, int p)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index c1da813786a4..3d21521453fe 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -1475,7 +1475,7 @@ static int dsa_switch_parse_ports_of(struct dsa_switch *ds,
}
if (reg >= ds->num_ports) {
- dev_err(ds->dev, "port %pOF index %u exceeds num_ports (%zu)\n",
+ dev_err(ds->dev, "port %pOF index %u exceeds num_ports (%u)\n",
port, reg, ds->num_ports);
of_node_put(port);
err = -EINVAL;
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH net-next 14/15] net: dsa: move dsa_switch_tree :: ports and lags to first cache line
2022-01-04 17:13 [PATCH net-next 00/15] DSA miscellaneous cleanups Vladimir Oltean
` (12 preceding siblings ...)
2022-01-04 17:14 ` [PATCH net-next 13/15] net: dsa: make dsa_switch :: num_ports an unsigned int Vladimir Oltean
@ 2022-01-04 17:14 ` Vladimir Oltean
2022-01-04 17:14 ` [PATCH net-next 15/15] net: dsa: combine two holes in struct dsa_switch_tree Vladimir Oltean
14 siblings, 0 replies; 20+ messages in thread
From: Vladimir Oltean @ 2022-01-04 17:14 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
Florian Fainelli
dst->ports is accessed most notably by dsa_master_find_slave(), which is
invoked in the RX path.
dst->lags is accessed by dsa_lag_dev(), which is invoked in the RX path
of tag_dsa.c.
dst->tag_ops, dst->default_proto and dst->pd don't need to be in the
first cache line, so they are moved out by this change.
Before:
pahole -C dsa_switch_tree net/dsa/slave.o
struct dsa_switch_tree {
struct list_head list; /* 0 16 */
struct raw_notifier_head nh; /* 16 8 */
unsigned int index; /* 24 4 */
struct kref refcount; /* 28 4 */
bool setup; /* 32 1 */
/* XXX 7 bytes hole, try to pack */
const struct dsa_device_ops * tag_ops; /* 40 8 */
enum dsa_tag_protocol default_proto; /* 48 4 */
/* XXX 4 bytes hole, try to pack */
struct dsa_platform_data * pd; /* 56 8 */
/* --- cacheline 1 boundary (64 bytes) --- */
struct list_head ports; /* 64 16 */
struct list_head rtable; /* 80 16 */
struct net_device * * lags; /* 96 8 */
unsigned int lags_len; /* 104 4 */
unsigned int last_switch; /* 108 4 */
/* size: 112, cachelines: 2, members: 13 */
/* sum members: 101, holes: 2, sum holes: 11 */
/* last cacheline: 48 bytes */
};
After:
pahole -C dsa_switch_tree net/dsa/slave.o
struct dsa_switch_tree {
struct list_head list; /* 0 16 */
struct list_head ports; /* 16 16 */
struct raw_notifier_head nh; /* 32 8 */
unsigned int index; /* 40 4 */
struct kref refcount; /* 44 4 */
struct net_device * * lags; /* 48 8 */
bool setup; /* 56 1 */
/* XXX 7 bytes hole, try to pack */
/* --- cacheline 1 boundary (64 bytes) --- */
const struct dsa_device_ops * tag_ops; /* 64 8 */
enum dsa_tag_protocol default_proto; /* 72 4 */
/* XXX 4 bytes hole, try to pack */
struct dsa_platform_data * pd; /* 80 8 */
struct list_head rtable; /* 88 16 */
unsigned int lags_len; /* 104 4 */
unsigned int last_switch; /* 108 4 */
/* size: 112, cachelines: 2, members: 13 */
/* sum members: 101, holes: 2, sum holes: 11 */
/* last cacheline: 48 bytes */
};
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
include/net/dsa.h | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index fef9d8bb5190..cbbac75138d9 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -119,6 +119,9 @@ struct dsa_netdevice_ops {
struct dsa_switch_tree {
struct list_head list;
+ /* List of switch ports */
+ struct list_head ports;
+
/* Notifier chain for switch-wide events */
struct raw_notifier_head nh;
@@ -128,6 +131,11 @@ struct dsa_switch_tree {
/* Number of switches attached to this tree */
struct kref refcount;
+ /* Maps offloaded LAG netdevs to a zero-based linear ID for
+ * drivers that need it.
+ */
+ struct net_device **lags;
+
/* Has this tree been applied to the hardware? */
bool setup;
@@ -145,16 +153,10 @@ struct dsa_switch_tree {
*/
struct dsa_platform_data *pd;
- /* List of switch ports */
- struct list_head ports;
-
/* List of DSA links composing the routing table */
struct list_head rtable;
- /* Maps offloaded LAG netdevs to a zero-based linear ID for
- * drivers that need it.
- */
- struct net_device **lags;
+ /* Length of "lags" array */
unsigned int lags_len;
/* Track the largest switch index within a tree */
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH net-next 15/15] net: dsa: combine two holes in struct dsa_switch_tree
2022-01-04 17:13 [PATCH net-next 00/15] DSA miscellaneous cleanups Vladimir Oltean
` (13 preceding siblings ...)
2022-01-04 17:14 ` [PATCH net-next 14/15] net: dsa: move dsa_switch_tree :: ports and lags to first cache line Vladimir Oltean
@ 2022-01-04 17:14 ` Vladimir Oltean
14 siblings, 0 replies; 20+ messages in thread
From: Vladimir Oltean @ 2022-01-04 17:14 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
Florian Fainelli
There is a 7 byte hole after dst->setup and a 4 byte hole after
dst->default_proto. Combining them, we have a single hole of just 3
bytes on 64 bit machines.
Before:
pahole -C dsa_switch_tree net/dsa/slave.o
struct dsa_switch_tree {
struct list_head list; /* 0 16 */
struct list_head ports; /* 16 16 */
struct raw_notifier_head nh; /* 32 8 */
unsigned int index; /* 40 4 */
struct kref refcount; /* 44 4 */
struct net_device * * lags; /* 48 8 */
bool setup; /* 56 1 */
/* XXX 7 bytes hole, try to pack */
/* --- cacheline 1 boundary (64 bytes) --- */
const struct dsa_device_ops * tag_ops; /* 64 8 */
enum dsa_tag_protocol default_proto; /* 72 4 */
/* XXX 4 bytes hole, try to pack */
struct dsa_platform_data * pd; /* 80 8 */
struct list_head rtable; /* 88 16 */
unsigned int lags_len; /* 104 4 */
unsigned int last_switch; /* 108 4 */
/* size: 112, cachelines: 2, members: 13 */
/* sum members: 101, holes: 2, sum holes: 11 */
/* last cacheline: 48 bytes */
};
After:
pahole -C dsa_switch_tree net/dsa/slave.o
struct dsa_switch_tree {
struct list_head list; /* 0 16 */
struct list_head ports; /* 16 16 */
struct raw_notifier_head nh; /* 32 8 */
unsigned int index; /* 40 4 */
struct kref refcount; /* 44 4 */
struct net_device * * lags; /* 48 8 */
const struct dsa_device_ops * tag_ops; /* 56 8 */
/* --- cacheline 1 boundary (64 bytes) --- */
enum dsa_tag_protocol default_proto; /* 64 4 */
bool setup; /* 68 1 */
/* XXX 3 bytes hole, try to pack */
struct dsa_platform_data * pd; /* 72 8 */
struct list_head rtable; /* 80 16 */
unsigned int lags_len; /* 96 4 */
unsigned int last_switch; /* 100 4 */
/* size: 104, cachelines: 2, members: 13 */
/* sum members: 101, holes: 1, sum holes: 3 */
/* last cacheline: 40 bytes */
};
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
include/net/dsa.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index cbbac75138d9..5d0fec6db3ae 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -136,9 +136,6 @@ struct dsa_switch_tree {
*/
struct net_device **lags;
- /* Has this tree been applied to the hardware? */
- bool setup;
-
/* Tagging protocol operations */
const struct dsa_device_ops *tag_ops;
@@ -147,6 +144,9 @@ struct dsa_switch_tree {
*/
enum dsa_tag_protocol default_proto;
+ /* Has this tree been applied to the hardware? */
+ bool setup;
+
/*
* Configuration data for the platform device that owns
* this dsa switch tree instance.
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread