* [PATCH v3 net-next 1/8] net: phy: bcm7xx: add jumbo frame configuration to PHY
2020-03-26 22:40 [PATCH v3 net-next 0/8] Configure the MTU on DSA switches Vladimir Oltean
@ 2020-03-26 22:40 ` Vladimir Oltean
2020-03-26 23:17 ` Florian Fainelli
2020-03-26 22:40 ` [PATCH v3 net-next 2/8] bgmac: configure MTU and add support for frames beyond 8192 byte size Vladimir Oltean
` (6 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Vladimir Oltean @ 2020-03-26 22:40 UTC (permalink / raw)
To: andrew, f.fainelli, vivien.didelot, davem, jakub.kicinski
Cc: murali.policharla, stephen, jiri, idosch, kuba, nikolay, netdev
From: Murali Krishna Policharla <murali.policharla@broadcom.com>
The BCM7XX PHY family requires special configuration to pass jumbo
frames. Do that during initial PHY setup.
Signed-off-by: Murali Krishna Policharla <murali.policharla@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v3:
Using the bcm54xx_auxctl_read and phy_set_bits helpers.
Corrected the value of the shadow register from 7 to 0.
Changes in v2:
Patch is new.
drivers/net/phy/bcm-phy-lib.c | 22 ++++++++++++++++++++++
drivers/net/phy/bcm-phy-lib.h | 1 +
drivers/net/phy/bcm7xxx.c | 4 ++++
include/linux/brcmphy.h | 2 ++
4 files changed, 29 insertions(+)
diff --git a/drivers/net/phy/bcm-phy-lib.c b/drivers/net/phy/bcm-phy-lib.c
index e0d3310957ff..e77b274a09fd 100644
--- a/drivers/net/phy/bcm-phy-lib.c
+++ b/drivers/net/phy/bcm-phy-lib.c
@@ -423,6 +423,28 @@ int bcm_phy_28nm_a0b0_afe_config_init(struct phy_device *phydev)
}
EXPORT_SYMBOL_GPL(bcm_phy_28nm_a0b0_afe_config_init);
+int bcm_phy_enable_jumbo(struct phy_device *phydev)
+{
+ int ret;
+
+ ret = bcm54xx_auxctl_read(phydev, MII_BCM54XX_AUXCTL_SHDWSEL_AUXCTL);
+ if (ret < 0)
+ return ret;
+
+ /* Enable extended length packet reception */
+ ret = bcm54xx_auxctl_write(phydev, MII_BCM54XX_AUXCTL_SHDWSEL_AUXCTL,
+ ret | MII_BCM54XX_AUXCTL_ACTL_EXT_PKT_LEN);
+ if (ret < 0)
+ return ret;
+
+ /* Enable the elastic FIFO for raising the transmission limit from
+ * 4.5KB to 10KB, at the expense of an additional 16 ns in propagation
+ * latency.
+ */
+ return phy_set_bits(phydev, MII_BCM54XX_ECR, MII_BCM54XX_ECR_FIFOE);
+}
+EXPORT_SYMBOL_GPL(bcm_phy_enable_jumbo);
+
MODULE_DESCRIPTION("Broadcom PHY Library");
MODULE_LICENSE("GPL v2");
MODULE_AUTHOR("Broadcom Corporation");
diff --git a/drivers/net/phy/bcm-phy-lib.h b/drivers/net/phy/bcm-phy-lib.h
index c86fb9d1240c..129df819be8c 100644
--- a/drivers/net/phy/bcm-phy-lib.h
+++ b/drivers/net/phy/bcm-phy-lib.h
@@ -65,5 +65,6 @@ void bcm_phy_get_stats(struct phy_device *phydev, u64 *shadow,
struct ethtool_stats *stats, u64 *data);
void bcm_phy_r_rc_cal_reset(struct phy_device *phydev);
int bcm_phy_28nm_a0b0_afe_config_init(struct phy_device *phydev);
+int bcm_phy_enable_jumbo(struct phy_device *phydev);
#endif /* _LINUX_BCM_PHY_LIB_H */
diff --git a/drivers/net/phy/bcm7xxx.c b/drivers/net/phy/bcm7xxx.c
index af8eabe7a6d4..692048d86ab1 100644
--- a/drivers/net/phy/bcm7xxx.c
+++ b/drivers/net/phy/bcm7xxx.c
@@ -178,6 +178,10 @@ static int bcm7xxx_28nm_config_init(struct phy_device *phydev)
break;
}
+ if (ret)
+ return ret;
+
+ ret = bcm_phy_enable_jumbo(phydev);
if (ret)
return ret;
diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h
index b475e7f20d28..6462c5447872 100644
--- a/include/linux/brcmphy.h
+++ b/include/linux/brcmphy.h
@@ -79,6 +79,7 @@
#define MII_BCM54XX_ECR 0x10 /* BCM54xx extended control register */
#define MII_BCM54XX_ECR_IM 0x1000 /* Interrupt mask */
#define MII_BCM54XX_ECR_IF 0x0800 /* Interrupt force */
+#define MII_BCM54XX_ECR_FIFOE 0x0001 /* FIFO elasticity */
#define MII_BCM54XX_ESR 0x11 /* BCM54xx extended status register */
#define MII_BCM54XX_ESR_IS 0x1000 /* Interrupt status */
@@ -119,6 +120,7 @@
#define MII_BCM54XX_AUXCTL_SHDWSEL_AUXCTL 0x00
#define MII_BCM54XX_AUXCTL_ACTL_TX_6DB 0x0400
#define MII_BCM54XX_AUXCTL_ACTL_SMDSP_ENA 0x0800
+#define MII_BCM54XX_AUXCTL_ACTL_EXT_PKT_LEN 0x4000
#define MII_BCM54XX_AUXCTL_SHDWSEL_MISC 0x07
#define MII_BCM54XX_AUXCTL_SHDWSEL_MISC_WIRESPEED_EN 0x0010
--
2.17.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v3 net-next 1/8] net: phy: bcm7xx: add jumbo frame configuration to PHY
2020-03-26 22:40 ` [PATCH v3 net-next 1/8] net: phy: bcm7xx: add jumbo frame configuration to PHY Vladimir Oltean
@ 2020-03-26 23:17 ` Florian Fainelli
0 siblings, 0 replies; 16+ messages in thread
From: Florian Fainelli @ 2020-03-26 23:17 UTC (permalink / raw)
To: Vladimir Oltean, andrew, vivien.didelot, davem, jakub.kicinski
Cc: murali.policharla, stephen, jiri, idosch, kuba, nikolay, netdev
On 3/26/2020 3:40 PM, Vladimir Oltean wrote:
> From: Murali Krishna Policharla <murali.policharla@broadcom.com>
>
> The BCM7XX PHY family requires special configuration to pass jumbo
> frames. Do that during initial PHY setup.
>
> Signed-off-by: Murali Krishna Policharla <murali.policharla@broadcom.com>
> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 net-next 2/8] bgmac: configure MTU and add support for frames beyond 8192 byte size
2020-03-26 22:40 [PATCH v3 net-next 0/8] Configure the MTU on DSA switches Vladimir Oltean
2020-03-26 22:40 ` [PATCH v3 net-next 1/8] net: phy: bcm7xx: add jumbo frame configuration to PHY Vladimir Oltean
@ 2020-03-26 22:40 ` Vladimir Oltean
2020-03-26 22:40 ` [PATCH v3 net-next 3/8] net: dsa: configure the MTU for switch ports Vladimir Oltean
` (5 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Vladimir Oltean @ 2020-03-26 22:40 UTC (permalink / raw)
To: andrew, f.fainelli, vivien.didelot, davem, jakub.kicinski
Cc: murali.policharla, stephen, jiri, idosch, kuba, nikolay, netdev
From: Murali Krishna Policharla <murali.policharla@broadcom.com>
Change DMA descriptor length to handle jumbo frames beyond 8192 bytes.
Also update jumbo frame max size to include FCS, the DMA packet length
received includes FCS.
Signed-off-by: Murali Krishna Policharla <murali.policharla@broadcom.com>
Reviewed-by: Arun Parameswaran <arun.parameswaran@broadcom.com>
Reviewed-by: Ray Jui <ray.jui@broadcom.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v3:
Squashed the entire bgmac implementation in one patch.
Changes in v2:
Patch is new.
drivers/net/ethernet/broadcom/bgmac.c | 12 ++++++++++++
drivers/net/ethernet/broadcom/bgmac.h | 5 +++--
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 1bb07a5d82c9..98ec1b8a7d8e 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -1248,6 +1248,14 @@ static int bgmac_set_mac_address(struct net_device *net_dev, void *addr)
return 0;
}
+static int bgmac_change_mtu(struct net_device *net_dev, int mtu)
+{
+ struct bgmac *bgmac = netdev_priv(net_dev);
+
+ bgmac_write(bgmac, BGMAC_RXMAX_LENGTH, 32 + mtu);
+ return 0;
+}
+
static const struct net_device_ops bgmac_netdev_ops = {
.ndo_open = bgmac_open,
.ndo_stop = bgmac_stop,
@@ -1256,6 +1264,7 @@ static const struct net_device_ops bgmac_netdev_ops = {
.ndo_set_mac_address = bgmac_set_mac_address,
.ndo_validate_addr = eth_validate_addr,
.ndo_do_ioctl = phy_do_ioctl_running,
+ .ndo_change_mtu = bgmac_change_mtu,
};
/**************************************************
@@ -1530,6 +1539,9 @@ int bgmac_enet_probe(struct bgmac *bgmac)
net_dev->hw_features = net_dev->features;
net_dev->vlan_features = net_dev->features;
+ /* Omit FCS from max MTU size */
+ net_dev->max_mtu = BGMAC_RX_MAX_FRAME_SIZE - ETH_FCS_LEN;
+
err = register_netdev(bgmac->net_dev);
if (err) {
dev_err(bgmac->dev, "Cannot register net device\n");
diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index 40d02fec2747..351c598a3ec6 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -351,7 +351,7 @@
#define BGMAC_DESC_CTL0_IOC 0x20000000 /* IRQ on complete */
#define BGMAC_DESC_CTL0_EOF 0x40000000 /* End of frame */
#define BGMAC_DESC_CTL0_SOF 0x80000000 /* Start of frame */
-#define BGMAC_DESC_CTL1_LEN 0x00001FFF
+#define BGMAC_DESC_CTL1_LEN 0x00003FFF
#define BGMAC_PHY_NOREGS BRCM_PSEUDO_PHY_ADDR
#define BGMAC_PHY_MASK 0x1F
@@ -366,7 +366,8 @@
#define BGMAC_RX_FRAME_OFFSET 30 /* There are 2 unused bytes between header and real data */
#define BGMAC_RX_BUF_OFFSET (NET_SKB_PAD + NET_IP_ALIGN - \
BGMAC_RX_FRAME_OFFSET)
-#define BGMAC_RX_MAX_FRAME_SIZE 1536 /* Copied from b44/tg3 */
+/* Jumbo frame size with FCS */
+#define BGMAC_RX_MAX_FRAME_SIZE 9724
#define BGMAC_RX_BUF_SIZE (BGMAC_RX_FRAME_OFFSET + BGMAC_RX_MAX_FRAME_SIZE)
#define BGMAC_RX_ALLOC_SIZE (SKB_DATA_ALIGN(BGMAC_RX_BUF_SIZE + BGMAC_RX_BUF_OFFSET) + \
SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))
--
2.17.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 net-next 3/8] net: dsa: configure the MTU for switch ports
2020-03-26 22:40 [PATCH v3 net-next 0/8] Configure the MTU on DSA switches Vladimir Oltean
2020-03-26 22:40 ` [PATCH v3 net-next 1/8] net: phy: bcm7xx: add jumbo frame configuration to PHY Vladimir Oltean
2020-03-26 22:40 ` [PATCH v3 net-next 2/8] bgmac: configure MTU and add support for frames beyond 8192 byte size Vladimir Oltean
@ 2020-03-26 22:40 ` Vladimir Oltean
2020-03-27 0:06 ` Andrew Lunn
2020-03-26 22:40 ` [PATCH v3 net-next 4/8] net: dsa: implement auto-normalization of MTU for bridge hardware datapath Vladimir Oltean
` (4 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Vladimir Oltean @ 2020-03-26 22:40 UTC (permalink / raw)
To: andrew, f.fainelli, vivien.didelot, davem, jakub.kicinski
Cc: murali.policharla, stephen, jiri, idosch, kuba, nikolay, netdev
From: Vladimir Oltean <vladimir.oltean@nxp.com>
It is useful be able to configure port policers on a switch to accept
frames of various sizes:
- Increase the MTU for better throughput from the default of 1500 if it
is known that there is no 10/100 Mbps device in the network.
- Decrease the MTU to limit the latency of high-priority frames under
congestion, or work around various network segments that add extra
headers to packets which can't be fragmented.
For DSA slave ports, this is mostly a pass-through callback, called
through the regular ndo ops and at probe time (to ensure consistency
across all supported switches).
The CPU port is called with an MTU equal to the largest configured MTU
of the slave ports. The assumption is that the user might want to
sustain a bidirectional conversation with a partner over any switch
port.
The DSA master is configured the same as the CPU port, plus the tagger
overhead. Since the MTU is by definition L2 payload (sans Ethernet
header), it is up to each individual driver to figure out if it needs to
do anything special for its frame tags on the CPU port (it shouldn't
except in special cases). So the MTU does not contain the tagger
overhead on the CPU port.
However the MTU of the DSA master, minus the tagger overhead, is used as
a proxy for the MTU of the CPU port, which does not have a net device.
This is to avoid uselessly calling the .change_mtu function on the CPU
port when nothing should change.
So it is safe to assume that the DSA master and the CPU port MTUs are
apart by exactly the tagger's overhead in bytes.
Some changes were made around dsa_master_set_mtu(), function which was
now removed, for 2 reasons:
- dev_set_mtu() already calls dev_validate_mtu(), so it's redundant to
do the same thing in DSA
- __dev_set_mtu() returns 0 if ops->ndo_change_mtu is an absent method
That is to say, there's no need for this function in DSA, we can safely
call dev_set_mtu() directly, take the rtnl lock when necessary, and just
propagate whatever errors get reported (since the user probably wants to
be informed).
Some inspiration (mainly in the MTU DSA notifier) was taken from a
vaguely similar patch from Murali and Florian, who are credited as
co-developers down below.
Co-developed-by: Murali Krishna Policharla <murali.policharla@broadcom.com>
Signed-off-by: Murali Krishna Policharla <murali.policharla@broadcom.com>
Co-developed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v3:
- Introduced the propagate_upstream variable in the notifier structure
to prevent intermediate MTU configuration to be propagated upstream
when it is going to be overwritten anyway by the MTU setting on the
CPU port.
- Refined the dsa_switch_mtu_match such that the CPU port on the local
switch does not match (it is explicitly programmed).
- Removed the dsa_master_set_mtu function (explanation in commit
message).
Changes in v2:
Introduced a DSA switch notifier for propagating the MTU setting towards
upstream switches.
include/net/dsa.h | 10 +++++
net/dsa/dsa_priv.h | 11 +++++
net/dsa/master.c | 23 ++++------
net/dsa/port.c | 13 ++++++
net/dsa/slave.c | 105 ++++++++++++++++++++++++++++++++++++++++++++-
net/dsa/switch.c | 37 ++++++++++++++++
6 files changed, 183 insertions(+), 16 deletions(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index beeb81a532e3..1bb1e0852e31 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -579,6 +579,16 @@ struct dsa_switch_ops {
struct devlink_param_gset_ctx *ctx);
int (*devlink_param_set)(struct dsa_switch *ds, u32 id,
struct devlink_param_gset_ctx *ctx);
+
+ /*
+ * MTU change functionality. Switches can also adjust their MRU through
+ * this method. By MTU, one understands the SDU, aka L2 payload length.
+ * If the switch needs to account for the DSA tag on the CPU port, this
+ * method needs to to do so privately.
+ */
+ int (*port_change_mtu)(struct dsa_switch *ds, int port,
+ int new_mtu);
+ int (*port_max_mtu)(struct dsa_switch *ds, int port);
};
#define DSA_DEVLINK_PARAM_DRIVER(_id, _name, _type, _cmodes) \
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 760e6ea3178a..da3be60beefe 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -22,6 +22,7 @@ enum {
DSA_NOTIFIER_MDB_DEL,
DSA_NOTIFIER_VLAN_ADD,
DSA_NOTIFIER_VLAN_DEL,
+ DSA_NOTIFIER_MTU,
};
/* DSA_NOTIFIER_AGEING_TIME */
@@ -61,6 +62,14 @@ struct dsa_notifier_vlan_info {
int port;
};
+/* DSA_NOTIFIER_MTU */
+struct dsa_notifier_mtu_info {
+ bool propagate_upstream;
+ int sw_index;
+ int port;
+ int mtu;
+};
+
struct dsa_slave_priv {
/* Copy of CPU port xmit for faster access in slave transmit hot path */
struct sk_buff * (*xmit)(struct sk_buff *skb,
@@ -127,6 +136,8 @@ int dsa_port_vlan_filtering(struct dsa_port *dp, bool vlan_filtering,
struct switchdev_trans *trans);
int dsa_port_ageing_time(struct dsa_port *dp, clock_t ageing_clock,
struct switchdev_trans *trans);
+int dsa_port_mtu_change(struct dsa_port *dp, int new_mtu,
+ bool propagate_upstream);
int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
u16 vid);
int dsa_port_fdb_del(struct dsa_port *dp, const unsigned char *addr,
diff --git a/net/dsa/master.c b/net/dsa/master.c
index bd44bde272f4..ee98d2d70cf5 100644
--- a/net/dsa/master.c
+++ b/net/dsa/master.c
@@ -314,20 +314,6 @@ static const struct attribute_group dsa_group = {
.attrs = dsa_slave_attrs,
};
-static void dsa_master_set_mtu(struct net_device *dev, struct dsa_port *cpu_dp)
-{
- unsigned int mtu = ETH_DATA_LEN + cpu_dp->tag_ops->overhead;
- int err;
-
- rtnl_lock();
- if (mtu <= dev->max_mtu) {
- err = dev_set_mtu(dev, mtu);
- if (err)
- netdev_dbg(dev, "Unable to set MTU to include for DSA overheads\n");
- }
- rtnl_unlock();
-}
-
static void dsa_master_reset_mtu(struct net_device *dev)
{
int err;
@@ -344,7 +330,14 @@ int dsa_master_setup(struct net_device *dev, struct dsa_port *cpu_dp)
{
int ret;
- dsa_master_set_mtu(dev, cpu_dp);
+ rtnl_lock();
+ ret = dev_set_mtu(dev, ETH_DATA_LEN + cpu_dp->tag_ops->overhead);
+ rtnl_unlock();
+ if (ret) {
+ netdev_err(dev, "error %d setting MTU to include DSA overhead\n",
+ ret);
+ return ret;
+ }
/* If we use a tagging format that doesn't have an ethertype
* field, make sure that all packets from this point on get
diff --git a/net/dsa/port.c b/net/dsa/port.c
index a18e65a474a5..231b2d494f1c 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -297,6 +297,19 @@ int dsa_port_mrouter(struct dsa_port *dp, bool mrouter,
return ds->ops->port_egress_floods(ds, port, true, mrouter);
}
+int dsa_port_mtu_change(struct dsa_port *dp, int new_mtu,
+ bool propagate_upstream)
+{
+ struct dsa_notifier_mtu_info info = {
+ .sw_index = dp->ds->index,
+ .propagate_upstream = propagate_upstream,
+ .port = dp->index,
+ .mtu = new_mtu,
+ };
+
+ return dsa_port_notify(dp, DSA_NOTIFIER_MTU, &info);
+}
+
int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
u16 vid)
{
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 5f782fa3029f..1a99bbab0722 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1218,6 +1218,96 @@ static int dsa_slave_vlan_rx_kill_vid(struct net_device *dev, __be16 proto,
return dsa_port_vid_del(dp, vid);
}
+static int dsa_slave_change_mtu(struct net_device *dev, int new_mtu)
+{
+ struct net_device *master = dsa_slave_to_master(dev);
+ struct dsa_port *dp = dsa_slave_to_port(dev);
+ struct dsa_slave_priv *p = netdev_priv(dev);
+ struct dsa_switch *ds = p->dp->ds;
+ struct dsa_port *cpu_dp;
+ int port = p->dp->index;
+ int largest_mtu = 0;
+ int new_master_mtu;
+ int old_master_mtu;
+ int mtu_limit;
+ int cpu_mtu;
+ int err, i;
+
+ if (!ds->ops->port_change_mtu)
+ return -EOPNOTSUPP;
+
+ for (i = 0; i < ds->num_ports; i++) {
+ int slave_mtu;
+
+ if (!dsa_is_user_port(ds, i))
+ continue;
+
+ /* During probe, this function will be called for each slave
+ * device, while not all of them have been allocated. That's
+ * ok, it doesn't change what the maximum is, so ignore it.
+ */
+ if (!dsa_to_port(ds, i)->slave)
+ continue;
+
+ /* Pretend that we already applied the setting, which we
+ * actually haven't (still haven't done all integrity checks)
+ */
+ if (i == port)
+ slave_mtu = new_mtu;
+ else
+ slave_mtu = dsa_to_port(ds, i)->slave->mtu;
+
+ if (largest_mtu < slave_mtu)
+ largest_mtu = slave_mtu;
+ }
+
+ cpu_dp = dsa_to_port(ds, port)->cpu_dp;
+
+ mtu_limit = min_t(int, master->max_mtu, dev->max_mtu);
+ old_master_mtu = master->mtu;
+ new_master_mtu = largest_mtu + cpu_dp->tag_ops->overhead;
+ if (new_master_mtu > mtu_limit)
+ return -ERANGE;
+
+ /* If the master MTU isn't over limit, there's no need to check the CPU
+ * MTU, since that surely isn't either.
+ */
+ cpu_mtu = largest_mtu;
+
+ /* Start applying stuff */
+ if (new_master_mtu != old_master_mtu) {
+ err = dev_set_mtu(master, new_master_mtu);
+ if (err < 0)
+ goto out_master_failed;
+
+ /* We only need to propagate the MTU of the CPU port to
+ * upstream switches.
+ */
+ err = dsa_port_mtu_change(cpu_dp, cpu_mtu, true);
+ if (err)
+ goto out_cpu_failed;
+ }
+
+ err = dsa_port_mtu_change(dp, new_mtu, false);
+ if (err)
+ goto out_port_failed;
+
+ dev->mtu = new_mtu;
+
+ return 0;
+
+out_port_failed:
+ if (new_master_mtu != old_master_mtu)
+ dsa_port_mtu_change(cpu_dp, old_master_mtu -
+ cpu_dp->tag_ops->overhead,
+ true);
+out_cpu_failed:
+ if (new_master_mtu != old_master_mtu)
+ dev_set_mtu(master, old_master_mtu);
+out_master_failed:
+ return err;
+}
+
static const struct ethtool_ops dsa_slave_ethtool_ops = {
.get_drvinfo = dsa_slave_get_drvinfo,
.get_regs_len = dsa_slave_get_regs_len,
@@ -1295,6 +1385,7 @@ static const struct net_device_ops dsa_slave_netdev_ops = {
.ndo_vlan_rx_add_vid = dsa_slave_vlan_rx_add_vid,
.ndo_vlan_rx_kill_vid = dsa_slave_vlan_rx_kill_vid,
.ndo_get_devlink_port = dsa_slave_get_devlink_port,
+ .ndo_change_mtu = dsa_slave_change_mtu,
};
static struct device_type dsa_type = {
@@ -1465,7 +1556,10 @@ int dsa_slave_create(struct dsa_port *port)
slave_dev->priv_flags |= IFF_NO_QUEUE;
slave_dev->netdev_ops = &dsa_slave_netdev_ops;
slave_dev->min_mtu = 0;
- slave_dev->max_mtu = ETH_MAX_MTU;
+ if (ds->ops->port_max_mtu)
+ slave_dev->max_mtu = ds->ops->port_max_mtu(ds, port->index);
+ else
+ slave_dev->max_mtu = ETH_MAX_MTU;
SET_NETDEV_DEVTYPE(slave_dev, &dsa_type);
SET_NETDEV_DEV(slave_dev, port->ds->dev);
@@ -1483,6 +1577,15 @@ int dsa_slave_create(struct dsa_port *port)
p->xmit = cpu_dp->tag_ops->xmit;
port->slave = slave_dev;
+ rtnl_lock();
+ ret = dsa_slave_change_mtu(slave_dev, ETH_DATA_LEN);
+ rtnl_unlock();
+ if (ret && ret != -EOPNOTSUPP) {
+ dev_err(ds->dev, "error %d setting MTU on port %d\n",
+ ret, port->index);
+ goto out_free;
+ }
+
netif_carrier_off(slave_dev);
ret = dsa_slave_phy_setup(slave_dev);
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index df4abe897ed6..f3c32ff552b3 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -52,6 +52,40 @@ static int dsa_switch_ageing_time(struct dsa_switch *ds,
return 0;
}
+static bool dsa_switch_mtu_match(struct dsa_switch *ds, int port,
+ struct dsa_notifier_mtu_info *info)
+{
+ if (ds->index == info->sw_index)
+ return (port == info->port) || dsa_is_dsa_port(ds, port);
+
+ if (!info->propagate_upstream)
+ return false;
+
+ if (dsa_is_dsa_port(ds, port) || dsa_is_cpu_port(ds, port))
+ return true;
+
+ return false;
+}
+
+static int dsa_switch_mtu(struct dsa_switch *ds,
+ struct dsa_notifier_mtu_info *info)
+{
+ int port, ret;
+
+ if (!ds->ops->port_change_mtu)
+ return -EOPNOTSUPP;
+
+ for (port = 0; port < ds->num_ports; port++) {
+ if (dsa_switch_mtu_match(ds, port, info)) {
+ ret = ds->ops->port_change_mtu(ds, port, info->mtu);
+ if (ret)
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
static int dsa_switch_bridge_join(struct dsa_switch *ds,
struct dsa_notifier_bridge_info *info)
{
@@ -328,6 +362,9 @@ static int dsa_switch_event(struct notifier_block *nb,
case DSA_NOTIFIER_VLAN_DEL:
err = dsa_switch_vlan_del(ds, info);
break;
+ case DSA_NOTIFIER_MTU:
+ err = dsa_switch_mtu(ds, info);
+ break;
default:
err = -EOPNOTSUPP;
break;
--
2.17.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v3 net-next 3/8] net: dsa: configure the MTU for switch ports
2020-03-26 22:40 ` [PATCH v3 net-next 3/8] net: dsa: configure the MTU for switch ports Vladimir Oltean
@ 2020-03-27 0:06 ` Andrew Lunn
2020-03-27 10:00 ` Vladimir Oltean
0 siblings, 1 reply; 16+ messages in thread
From: Andrew Lunn @ 2020-03-27 0:06 UTC (permalink / raw)
To: Vladimir Oltean
Cc: f.fainelli, vivien.didelot, davem, jakub.kicinski,
murali.policharla, stephen, jiri, idosch, kuba, nikolay, netdev
> -static void dsa_master_set_mtu(struct net_device *dev, struct dsa_port *cpu_dp)
> -{
> - unsigned int mtu = ETH_DATA_LEN + cpu_dp->tag_ops->overhead;
> - int err;
> -
> - rtnl_lock();
> - if (mtu <= dev->max_mtu) {
> - err = dev_set_mtu(dev, mtu);
> - if (err)
> - netdev_dbg(dev, "Unable to set MTU to include for DSA overheads\n");
> - }
> - rtnl_unlock();
> -}
> -
> static void dsa_master_reset_mtu(struct net_device *dev)
> {
> int err;
> @@ -344,7 +330,14 @@ int dsa_master_setup(struct net_device *dev, struct dsa_port *cpu_dp)
> {
> int ret;
>
> - dsa_master_set_mtu(dev, cpu_dp);
> + rtnl_lock();
> + ret = dev_set_mtu(dev, ETH_DATA_LEN + cpu_dp->tag_ops->overhead);
> + rtnl_unlock();
> + if (ret) {
> + netdev_err(dev, "error %d setting MTU to include DSA overhead\n",
> + ret);
> + return ret;
> + }
I suspect this will break devices. dsa_master_set_mtu() was not fatal
if it failed. I did this deliberately because i suspect there are some
MAC drivers which are unhappy to have the MTU changed, but will still
send and receive frames which are bigger than the MTU.
So please keep setting the MTU of ETH_DATA_LEN +
cpu_dp->tag_ops->overhead or less as non-fatal. Jumbo frame sizes you
should however check the return code.
> @@ -1465,7 +1556,10 @@ int dsa_slave_create(struct dsa_port *port)
> slave_dev->priv_flags |= IFF_NO_QUEUE;
> slave_dev->netdev_ops = &dsa_slave_netdev_ops;
> slave_dev->min_mtu = 0;
> - slave_dev->max_mtu = ETH_MAX_MTU;
> + if (ds->ops->port_max_mtu)
> + slave_dev->max_mtu = ds->ops->port_max_mtu(ds, port->index);
> + else
> + slave_dev->max_mtu = ETH_MAX_MTU;
Does this bring you anything. You have a lot more checks you perform
when performing the actual change. Seems better to keep them all
together.
Andrew
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3 net-next 3/8] net: dsa: configure the MTU for switch ports
2020-03-27 0:06 ` Andrew Lunn
@ 2020-03-27 10:00 ` Vladimir Oltean
0 siblings, 0 replies; 16+ messages in thread
From: Vladimir Oltean @ 2020-03-27 10:00 UTC (permalink / raw)
To: Andrew Lunn
Cc: Florian Fainelli, Vivien Didelot, David S. Miller, Jakub Kicinski,
murali.policharla, Stephen Hemminger, Jiri Pirko, Ido Schimmel,
Jakub Kicinski, Nikolay Aleksandrov, netdev
On Fri, 27 Mar 2020 at 02:06, Andrew Lunn <andrew@lunn.ch> wrote:
>
> > -static void dsa_master_set_mtu(struct net_device *dev, struct dsa_port *cpu_dp)
> > -{
> > - unsigned int mtu = ETH_DATA_LEN + cpu_dp->tag_ops->overhead;
> > - int err;
> > -
> > - rtnl_lock();
> > - if (mtu <= dev->max_mtu) {
> > - err = dev_set_mtu(dev, mtu);
> > - if (err)
> > - netdev_dbg(dev, "Unable to set MTU to include for DSA overheads\n");
> > - }
> > - rtnl_unlock();
> > -}
> > -
> > static void dsa_master_reset_mtu(struct net_device *dev)
> > {
> > int err;
> > @@ -344,7 +330,14 @@ int dsa_master_setup(struct net_device *dev, struct dsa_port *cpu_dp)
> > {
> > int ret;
> >
> > - dsa_master_set_mtu(dev, cpu_dp);
> > + rtnl_lock();
> > + ret = dev_set_mtu(dev, ETH_DATA_LEN + cpu_dp->tag_ops->overhead);
> > + rtnl_unlock();
> > + if (ret) {
> > + netdev_err(dev, "error %d setting MTU to include DSA overhead\n",
> > + ret);
> > + return ret;
> > + }
>
> I suspect this will break devices. dsa_master_set_mtu() was not fatal
> if it failed. I did this deliberately because i suspect there are some
> MAC drivers which are unhappy to have the MTU changed, but will still
> send and receive frames which are bigger than the MTU.
>
> So please keep setting the MTU of ETH_DATA_LEN +
> cpu_dp->tag_ops->overhead or less as non-fatal. Jumbo frame sizes you
> should however check the return code.
>
Ok, I'll leave it non-fatal here.
> > @@ -1465,7 +1556,10 @@ int dsa_slave_create(struct dsa_port *port)
> > slave_dev->priv_flags |= IFF_NO_QUEUE;
> > slave_dev->netdev_ops = &dsa_slave_netdev_ops;
> > slave_dev->min_mtu = 0;
> > - slave_dev->max_mtu = ETH_MAX_MTU;
> > + if (ds->ops->port_max_mtu)
> > + slave_dev->max_mtu = ds->ops->port_max_mtu(ds, port->index);
> > + else
> > + slave_dev->max_mtu = ETH_MAX_MTU;
>
> Does this bring you anything. You have a lot more checks you perform
> when performing the actual change. Seems better to keep them all
> together.
>
This here is for cooperating with dev_validate_mtu() so I can get this
nice message:
ip link set dev sw0p1 mtu 2400
Error: mtu greater than device maximum.
> Andrew
Thanks,
-Vladimir
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 net-next 4/8] net: dsa: implement auto-normalization of MTU for bridge hardware datapath
2020-03-26 22:40 [PATCH v3 net-next 0/8] Configure the MTU on DSA switches Vladimir Oltean
` (2 preceding siblings ...)
2020-03-26 22:40 ` [PATCH v3 net-next 3/8] net: dsa: configure the MTU for switch ports Vladimir Oltean
@ 2020-03-26 22:40 ` Vladimir Oltean
2020-04-02 1:25 ` kbuild test robot
2020-03-26 22:40 ` [PATCH v3 net-next 5/8] net: dsa: b53: add MTU configuration support Vladimir Oltean
` (3 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Vladimir Oltean @ 2020-03-26 22:40 UTC (permalink / raw)
To: andrew, f.fainelli, vivien.didelot, davem, jakub.kicinski
Cc: murali.policharla, stephen, jiri, idosch, kuba, nikolay, netdev
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Many switches don't have an explicit knob for configuring the MTU
(maximum transmission unit per interface). Instead, they do the
length-based packet admission checks on the ingress interface, for
reasons that are easy to understand (why would you accept a packet in
the queuing subsystem if you know you're going to drop it anyway).
So it is actually the MRU that these switches permit configuring.
In Linux there only exists the IFLA_MTU netlink attribute and the
associated dev_set_mtu function. The comments like to play blind and say
that it's changing the "maximum transfer unit", which is to say that
there isn't any directionality in the meaning of the MTU word. So that
is the interpretation that this patch is giving to things: MTU == MRU.
When 2 interfaces having different MTUs are bridged, the bridge driver
MTU auto-adjustment logic kicks in: what br_mtu_auto_adjust() does is it
adjusts the MTU of the bridge net device itself (and not that of the
slave net devices) to the minimum value of all slave interfaces, in
order for forwarded packets to not exceed the MTU regardless of the
interface they are received and send on.
The idea behind this behavior, and why the slave MTUs are not adjusted,
is that normal termination from Linux over the L2 forwarding domain
should happen over the bridge net device, which _is_ properly limited by
the minimum MTU. And termination over individual slave devices is
possible even if those are bridged. But that is not "forwarding", so
there's no reason to do normalization there, since only a single
interface sees that packet.
The problem with those switches that can only control the MRU is with
the offloaded data path, where a packet received on an interface with
MRU 9000 would still be forwarded to an interface with MRU 1500. And the
br_mtu_auto_adjust() function does not really help, since the MTU
configured on the bridge net device is ignored.
In order to enforce the de-facto MTU == MRU rule for these switches, we
need to do MTU normalization, which means: in order for no packet larger
than the MTU configured on this port to be sent, then we need to limit
the MRU on all ports that this packet could possibly come from. AKA
since we are configuring the MRU via MTU, it means that all ports within
a bridge forwarding domain should have the same MTU.
And that is exactly what this patch is trying to do.
From an implementation perspective, we try to follow the intent of the
user, otherwise there is a risk that we might livelock them (they try to
change the MTU on an already-bridged interface, but we just keep
changing it back in an attempt to keep the MTU normalized). So the MTU
that the bridge is normalized to is either:
- The most recently changed one:
ip link set dev swp0 master br0
ip link set dev swp1 master br0
ip link set dev swp0 mtu 1400
This sequence will make swp1 inherit MTU 1400 from swp0.
- The one of the most recently added interface to the bridge:
ip link set dev swp0 master br0
ip link set dev swp1 mtu 1400
ip link set dev swp1 master br0
The above sequence will make swp0 inherit MTU 1400 as well.
Suggested-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v3:
Moved the implementation to the DSA core (it was in the bridge driver
previously).
Added a variable by which drivers should denote if they require this
behavior or not.
Changes in v2:
Patch is new.
include/net/dsa.h | 6 +++
net/dsa/dsa2.c | 2 +-
net/dsa/dsa_priv.h | 4 ++
net/dsa/slave.c | 114 +++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 125 insertions(+), 1 deletion(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 1bb1e0852e31..0f4e55543cf8 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -284,6 +284,12 @@ struct dsa_switch {
*/
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;
+
size_t num_ports;
};
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index e7c30b472034..9a271a58a41d 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -18,8 +18,8 @@
#include "dsa_priv.h"
-static LIST_HEAD(dsa_tree_list);
static DEFINE_MUTEX(dsa2_mutex);
+LIST_HEAD(dsa_tree_list);
static const struct devlink_ops dsa_devlink_ops = {
};
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index da3be60beefe..904cc7c9b882 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -194,4 +194,8 @@ dsa_slave_to_master(const struct net_device *dev)
/* switch.c */
int dsa_switch_register_notifier(struct dsa_switch *ds);
void dsa_switch_unregister_notifier(struct dsa_switch *ds);
+
+/* dsa2.c */
+extern struct list_head dsa_tree_list;
+
#endif
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 1a99bbab0722..8ced165a7908 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1218,6 +1218,116 @@ static int dsa_slave_vlan_rx_kill_vid(struct net_device *dev, __be16 proto,
return dsa_port_vid_del(dp, vid);
}
+struct dsa_hw_port {
+ struct list_head list;
+ struct net_device *dev;
+ int old_mtu;
+};
+
+static int dsa_hw_port_list_set_mtu(struct list_head *hw_port_list, int mtu)
+{
+ const struct dsa_hw_port *p;
+ int err;
+
+ list_for_each_entry(p, hw_port_list, list) {
+ if (p->dev->mtu == mtu)
+ continue;
+
+ err = dev_set_mtu(p->dev, mtu);
+ if (err)
+ goto rollback;
+ }
+
+ return 0;
+
+rollback:
+ list_for_each_entry_continue_reverse(p, hw_port_list, list) {
+ if (p->dev->mtu == p->old_mtu)
+ continue;
+
+ if (dev_set_mtu(p->dev, p->old_mtu))
+ netdev_err(p->dev, "Failed to restore MTU\n");
+ }
+
+ return err;
+}
+
+static void dsa_hw_port_list_free(struct list_head *hw_port_list)
+{
+ struct dsa_hw_port *p, *n;
+
+ list_for_each_entry_safe(p, n, hw_port_list, list)
+ kfree(p);
+}
+
+/* Make the hardware datapath to/from @dev limited to a common MTU */
+void dsa_bridge_mtu_normalization(struct dsa_port *dp)
+{
+ struct list_head hw_port_list;
+ struct dsa_switch_tree *dst;
+ int min_mtu = ETH_MAX_MTU;
+ struct dsa_port *other_dp;
+ int err;
+
+ if (!dp->ds->mtu_enforcement_ingress)
+ return;
+
+ if (!dp->bridge_dev)
+ return;
+
+ INIT_LIST_HEAD(&hw_port_list);
+
+ /* Populate the list of ports that are part of the same bridge
+ * as the newly added/modified port
+ */
+ list_for_each_entry(dst, &dsa_tree_list, list) {
+ list_for_each_entry(other_dp, &dst->ports, list) {
+ struct dsa_hw_port *hw_port;
+ struct net_device *slave;
+
+ if (other_dp->type != DSA_PORT_TYPE_USER)
+ continue;
+
+ if (other_dp->bridge_dev != dp->bridge_dev)
+ continue;
+
+ if (!other_dp->ds->mtu_enforcement_ingress)
+ continue;
+
+ slave = other_dp->slave;
+
+ if (min_mtu > slave->mtu)
+ min_mtu = slave->mtu;
+
+ hw_port = kzalloc(sizeof(*hw_port), GFP_KERNEL);
+ if (!hw_port)
+ goto out;
+
+ hw_port->dev = slave;
+ hw_port->old_mtu = slave->mtu;
+
+ list_add(&hw_port->list, &hw_port_list);
+ }
+ }
+
+ /* Attempt to configure the entire hardware bridge to the newly added
+ * interface's MTU first, regardless of whether the intention of the
+ * user was to raise or lower it.
+ */
+ err = dsa_hw_port_list_set_mtu(&hw_port_list, dp->slave->mtu);
+ if (!err)
+ goto out;
+
+ /* Clearly that didn't work out so well, so just set the minimum MTU on
+ * all hardware bridge ports now. If this fails too, then all ports will
+ * still have their old MTU rolled back anyway.
+ */
+ dsa_hw_port_list_set_mtu(&hw_port_list, min_mtu);
+
+out:
+ dsa_hw_port_list_free(&hw_port_list);
+}
+
static int dsa_slave_change_mtu(struct net_device *dev, int new_mtu)
{
struct net_device *master = dsa_slave_to_master(dev);
@@ -1294,6 +1404,8 @@ static int dsa_slave_change_mtu(struct net_device *dev, int new_mtu)
dev->mtu = new_mtu;
+ dsa_bridge_mtu_normalization(dp);
+
return 0;
out_port_failed:
@@ -1648,6 +1760,8 @@ static int dsa_slave_changeupper(struct net_device *dev,
if (netif_is_bridge_master(info->upper_dev)) {
if (info->linking) {
err = dsa_port_bridge_join(dp, info->upper_dev);
+ if (!err)
+ dsa_bridge_mtu_normalization(dp);
err = notifier_from_errno(err);
} else {
dsa_port_bridge_leave(dp, info->upper_dev);
--
2.17.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v3 net-next 4/8] net: dsa: implement auto-normalization of MTU for bridge hardware datapath
2020-03-26 22:40 ` [PATCH v3 net-next 4/8] net: dsa: implement auto-normalization of MTU for bridge hardware datapath Vladimir Oltean
@ 2020-04-02 1:25 ` kbuild test robot
0 siblings, 0 replies; 16+ messages in thread
From: kbuild test robot @ 2020-04-02 1:25 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 3529 bytes --]
Hi Vladimir,
I love your patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
[also build test WARNING on linus/master v5.6-rc7 next-20200327]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Vladimir-Oltean/Configure-the-MTU-on-DSA-switches/20200327-094801
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 92b7e62e5630a370955a4760bbeb3967457034ec
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-187-gbff9b106-dirty
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
:::::: branch date: 17 hours ago
:::::: commit date: 17 hours ago
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
net/dsa/slave.c:506:13: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected void const [noderef] <asn:3> *__vpp_verify @@ got const [noderef] <asn:3> *__vpp_verify @@
net/dsa/slave.c:506:13: sparse: expected void const [noderef] <asn:3> *__vpp_verify
net/dsa/slave.c:506:13: sparse: got struct pcpu_sw_netstats *
net/dsa/slave.c:640:21: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected void const [noderef] <asn:3> *__vpp_verify @@ got const [noderef] <asn:3> *__vpp_verify @@
net/dsa/slave.c:640:21: sparse: expected void const [noderef] <asn:3> *__vpp_verify
net/dsa/slave.c:640:21: sparse: got struct pcpu_sw_netstats *
net/dsa/slave.c:1106:21: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected void const [noderef] <asn:3> *__vpp_verify @@ got const [noderef] <asn:3> *__vpp_verify @@
net/dsa/slave.c:1106:21: sparse: expected void const [noderef] <asn:3> *__vpp_verify
net/dsa/slave.c:1106:21: sparse: got struct pcpu_sw_netstats *
>> net/dsa/slave.c:1264:6: sparse: sparse: symbol 'dsa_bridge_mtu_normalization' was not declared. Should it be static?
net/dsa/slave.c:1682:20: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct pcpu_sw_netstats *stats64 @@ got struct pcpu_sw_netstruct pcpu_sw_netstats *stats64 @@
net/dsa/slave.c:1682:20: sparse: expected struct pcpu_sw_netstats *stats64
net/dsa/slave.c:1682:20: sparse: got struct pcpu_sw_netstats [noderef] <asn:3> *pcpu_stats
net/dsa/slave.c:1726:22: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void [noderef] <asn:3> *__pdata @@ got svoid [noderef] <asn:3> *__pdata @@
net/dsa/slave.c:1726:22: sparse: expected void [noderef] <asn:3> *__pdata
net/dsa/slave.c:1726:22: sparse: got struct pcpu_sw_netstats *stats64
net/dsa/slave.c:1745:22: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void [noderef] <asn:3> *__pdata @@ got svoid [noderef] <asn:3> *__pdata @@
net/dsa/slave.c:1745:22: sparse: expected void [noderef] <asn:3> *__pdata
net/dsa/slave.c:1745:22: sparse: got struct pcpu_sw_netstats *stats64
Please review and possibly fold the followup patch.
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3 net-next 4/8] net: dsa: implement auto-normalization of MTU for bridge hardware datapath
@ 2020-04-02 1:25 ` kbuild test robot
0 siblings, 0 replies; 16+ messages in thread
From: kbuild test robot @ 2020-04-02 1:25 UTC (permalink / raw)
To: Vladimir Oltean
Cc: kbuild-all, andrew, f.fainelli, vivien.didelot, davem,
jakub.kicinski, murali.policharla, stephen, jiri, idosch, kuba,
nikolay, netdev
Hi Vladimir,
I love your patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
[also build test WARNING on linus/master v5.6-rc7 next-20200327]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Vladimir-Oltean/Configure-the-MTU-on-DSA-switches/20200327-094801
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 92b7e62e5630a370955a4760bbeb3967457034ec
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-187-gbff9b106-dirty
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
:::::: branch date: 17 hours ago
:::::: commit date: 17 hours ago
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
net/dsa/slave.c:506:13: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected void const [noderef] <asn:3> *__vpp_verify @@ got const [noderef] <asn:3> *__vpp_verify @@
net/dsa/slave.c:506:13: sparse: expected void const [noderef] <asn:3> *__vpp_verify
net/dsa/slave.c:506:13: sparse: got struct pcpu_sw_netstats *
net/dsa/slave.c:640:21: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected void const [noderef] <asn:3> *__vpp_verify @@ got const [noderef] <asn:3> *__vpp_verify @@
net/dsa/slave.c:640:21: sparse: expected void const [noderef] <asn:3> *__vpp_verify
net/dsa/slave.c:640:21: sparse: got struct pcpu_sw_netstats *
net/dsa/slave.c:1106:21: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected void const [noderef] <asn:3> *__vpp_verify @@ got const [noderef] <asn:3> *__vpp_verify @@
net/dsa/slave.c:1106:21: sparse: expected void const [noderef] <asn:3> *__vpp_verify
net/dsa/slave.c:1106:21: sparse: got struct pcpu_sw_netstats *
>> net/dsa/slave.c:1264:6: sparse: sparse: symbol 'dsa_bridge_mtu_normalization' was not declared. Should it be static?
net/dsa/slave.c:1682:20: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct pcpu_sw_netstats *stats64 @@ got struct pcpu_sw_netstruct pcpu_sw_netstats *stats64 @@
net/dsa/slave.c:1682:20: sparse: expected struct pcpu_sw_netstats *stats64
net/dsa/slave.c:1682:20: sparse: got struct pcpu_sw_netstats [noderef] <asn:3> *pcpu_stats
net/dsa/slave.c:1726:22: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void [noderef] <asn:3> *__pdata @@ got svoid [noderef] <asn:3> *__pdata @@
net/dsa/slave.c:1726:22: sparse: expected void [noderef] <asn:3> *__pdata
net/dsa/slave.c:1726:22: sparse: got struct pcpu_sw_netstats *stats64
net/dsa/slave.c:1745:22: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void [noderef] <asn:3> *__pdata @@ got svoid [noderef] <asn:3> *__pdata @@
net/dsa/slave.c:1745:22: sparse: expected void [noderef] <asn:3> *__pdata
net/dsa/slave.c:1745:22: sparse: got struct pcpu_sw_netstats *stats64
Please review and possibly fold the followup patch.
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 net-next 5/8] net: dsa: b53: add MTU configuration support
2020-03-26 22:40 [PATCH v3 net-next 0/8] Configure the MTU on DSA switches Vladimir Oltean
` (3 preceding siblings ...)
2020-03-26 22:40 ` [PATCH v3 net-next 4/8] net: dsa: implement auto-normalization of MTU for bridge hardware datapath Vladimir Oltean
@ 2020-03-26 22:40 ` Vladimir Oltean
2020-03-26 23:16 ` Florian Fainelli
2020-03-26 22:40 ` [PATCH v3 net-next 6/8] net: dsa: sja1105: implement the port MTU callbacks Vladimir Oltean
` (2 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Vladimir Oltean @ 2020-03-26 22:40 UTC (permalink / raw)
To: andrew, f.fainelli, vivien.didelot, davem, jakub.kicinski
Cc: murali.policharla, stephen, jiri, idosch, kuba, nikolay, netdev
From: Murali Krishna Policharla <murali.policharla@broadcom.com>
It looks like the Broadcomm switches supported by the b53 driver don't
support precise configuration of the MTU, but just a mumbo-jumbo boolean
flag. Set that.
Also configure BCM583XX devices to send and receive jumbo frames when
ports are configured with 10/100 Mbps speed.
Signed-off-by: Murali Krishna Policharla <murali.policharla@broadcom.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v3:
Using the b53_set_jumbo function that was already there.
Changes in v2:
Patch is new.
drivers/net/dsa/b53/b53_common.c | 27 ++++++++++++++++++++++-----
1 file changed, 22 insertions(+), 5 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index ceafce446317..f432edf618e1 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -699,9 +699,6 @@ int b53_configure_vlan(struct dsa_switch *ds)
b53_write16(dev, B53_VLAN_PAGE,
B53_VLAN_PORT_DEF_TAG(i), def_vid);
- if (!is5325(dev) && !is5365(dev))
- b53_set_jumbo(dev, dev->enable_jumbo, false);
-
return 0;
}
EXPORT_SYMBOL(b53_configure_vlan);
@@ -807,8 +804,6 @@ static int b53_phy_write16(struct dsa_switch *ds, int addr, int reg, u16 val)
static int b53_reset_switch(struct b53_device *priv)
{
/* reset vlans */
- priv->enable_jumbo = false;
-
memset(priv->vlans, 0, sizeof(*priv->vlans) * priv->num_vlans);
memset(priv->ports, 0, sizeof(*priv->ports) * priv->num_ports);
@@ -2065,6 +2060,26 @@ int b53_set_mac_eee(struct dsa_switch *ds, int port, struct ethtool_eee *e)
}
EXPORT_SYMBOL(b53_set_mac_eee);
+static int b53_change_mtu(struct dsa_switch *ds, int port, int mtu)
+{
+ struct b53_device *dev = ds->priv;
+ bool enable_jumbo;
+ bool allow_10_100;
+
+ if (is5325(dev) || is5365(dev))
+ return -EOPNOTSUPP;
+
+ enable_jumbo = (mtu >= JMS_MIN_SIZE);
+ allow_10_100 = (dev->chip_id == BCM58XX_DEVICE_ID);
+
+ return b53_set_jumbo(dev, enable_jumbo, allow_10_100);
+}
+
+static int b53_get_max_mtu(struct dsa_switch *ds, int port)
+{
+ return JMS_MAX_SIZE;
+}
+
static const struct dsa_switch_ops b53_switch_ops = {
.get_tag_protocol = b53_get_tag_protocol,
.setup = b53_setup,
@@ -2102,6 +2117,8 @@ static const struct dsa_switch_ops b53_switch_ops = {
.port_mdb_prepare = b53_mdb_prepare,
.port_mdb_add = b53_mdb_add,
.port_mdb_del = b53_mdb_del,
+ .port_max_mtu = b53_get_max_mtu,
+ .port_change_mtu = b53_change_mtu,
};
struct b53_chip_data {
--
2.17.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v3 net-next 5/8] net: dsa: b53: add MTU configuration support
2020-03-26 22:40 ` [PATCH v3 net-next 5/8] net: dsa: b53: add MTU configuration support Vladimir Oltean
@ 2020-03-26 23:16 ` Florian Fainelli
2020-03-27 13:01 ` Vladimir Oltean
0 siblings, 1 reply; 16+ messages in thread
From: Florian Fainelli @ 2020-03-26 23:16 UTC (permalink / raw)
To: Vladimir Oltean, andrew, vivien.didelot, davem, jakub.kicinski
Cc: murali.policharla, stephen, jiri, idosch, kuba, nikolay, netdev
On 3/26/2020 3:40 PM, Vladimir Oltean wrote:
> From: Murali Krishna Policharla <murali.policharla@broadcom.com>
>
> It looks like the Broadcomm switches supported by the b53 driver don't
^= one too many m's, the attempt to acquire
Qualcomm failed a few years ago :)
> support precise configuration of the MTU, but just a mumbo-jumbo boolean
> flag. Set that.
>
> Also configure BCM583XX devices to send and receive jumbo frames when
> ports are configured with 10/100 Mbps speed.
>
> Signed-off-by: Murali Krishna Policharla <murali.policharla@broadcom.com>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
[snip]
> +static int b53_change_mtu(struct dsa_switch *ds, int port, int mtu)
> +{
> + struct b53_device *dev = ds->priv;
> + bool enable_jumbo;
> + bool allow_10_100;
> +
> + if (is5325(dev) || is5365(dev))
> + return -EOPNOTSUPP;
> +
> + enable_jumbo = (mtu >= JMS_MIN_SIZE);
> + allow_10_100 = (dev->chip_id == BCM58XX_DEVICE_ID);
I believe this was meant to be BCM583XX_DEVICE_ID to be consistent with
the previous patch version. With that:
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3 net-next 5/8] net: dsa: b53: add MTU configuration support
2020-03-26 23:16 ` Florian Fainelli
@ 2020-03-27 13:01 ` Vladimir Oltean
0 siblings, 0 replies; 16+ messages in thread
From: Vladimir Oltean @ 2020-03-27 13:01 UTC (permalink / raw)
To: Florian Fainelli
Cc: Andrew Lunn, Vivien Didelot, David S. Miller, Jakub Kicinski,
murali.policharla, Stephen Hemminger, Jiri Pirko, Ido Schimmel,
Jakub Kicinski, Nikolay Aleksandrov, netdev
On Fri, 27 Mar 2020 at 01:16, Florian Fainelli <f.fainelli@gmail.com> wrote:
>
> On 3/26/2020 3:40 PM, Vladimir Oltean wrote:
> > From: Murali Krishna Policharla <murali.policharla@broadcom.com>
> >
> > It looks like the Broadcomm switches supported by the b53 driver don't
> ^= one too many m's, the attempt to acquire
> Qualcomm failed a few years ago :)
>
I knew you guys were jealous of their extra m!
> > support precise configuration of the MTU, but just a mumbo-jumbo boolean
> > flag. Set that.
I'm more concerned of the mumbo-jumbo, in fact. Right now, DSA sets
dev->mtu = mtu in slave.c. But for hardware like this, it would be
confusing to do that. I would do dev->mtu = JMS_MIN_SIZE or dev->mtu =
JMS_MAX_SIZE depending on configuration, so that the user isn't led
into thinking that their exact requested value went into the hardware.
Any ideas on how I could structure things differently for that?
> >
> > Also configure BCM583XX devices to send and receive jumbo frames when
> > ports are configured with 10/100 Mbps speed.
> >
> > Signed-off-by: Murali Krishna Policharla <murali.policharla@broadcom.com>
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > ---
>
> [snip]
>
> > +static int b53_change_mtu(struct dsa_switch *ds, int port, int mtu)
> > +{
> > + struct b53_device *dev = ds->priv;
> > + bool enable_jumbo;
> > + bool allow_10_100;
> > +
> > + if (is5325(dev) || is5365(dev))
> > + return -EOPNOTSUPP;
> > +
> > + enable_jumbo = (mtu >= JMS_MIN_SIZE);
> > + allow_10_100 = (dev->chip_id == BCM58XX_DEVICE_ID);
>
> I believe this was meant to be BCM583XX_DEVICE_ID to be consistent with
> the previous patch version. With that:
>
Ok.
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> --
> Florian
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 net-next 6/8] net: dsa: sja1105: implement the port MTU callbacks
2020-03-26 22:40 [PATCH v3 net-next 0/8] Configure the MTU on DSA switches Vladimir Oltean
` (4 preceding siblings ...)
2020-03-26 22:40 ` [PATCH v3 net-next 5/8] net: dsa: b53: add MTU configuration support Vladimir Oltean
@ 2020-03-26 22:40 ` Vladimir Oltean
2020-03-26 22:40 ` [PATCH v3 net-next 7/8] net: dsa: vsc73xx: make the MTU configurable Vladimir Oltean
2020-03-26 22:40 ` [PATCH v3 net-next 8/8] net: dsa: felix: support changing the MTU Vladimir Oltean
7 siblings, 0 replies; 16+ messages in thread
From: Vladimir Oltean @ 2020-03-26 22:40 UTC (permalink / raw)
To: andrew, f.fainelli, vivien.didelot, davem, jakub.kicinski
Cc: murali.policharla, stephen, jiri, idosch, kuba, nikolay, netdev
From: Vladimir Oltean <vladimir.oltean@nxp.com>
On this switch, the frame length enforcements are performed by the
ingress policers. There are 2 types of those: regular L2 (also called
best-effort) and Virtual Link policers (an ARINC664/AFDX concept for
defining L2 streams with certain QoS abilities). To avoid future
confusion, I prefer to call the reset reason "Best-effort policers",
even though the VL policers are not yet supported.
We also need to change the setup of the initial static config, such that
DSA calls to .change_mtu (which are expensive) become no-ops and don't
reset the switch 5 times.
A driver-level decision is to unconditionally allow single VLAN-tagged
traffic on all ports. The CPU port must accept an additional VLAN header
for the DSA tag, which is again a driver-level decision.
The policers actually count bytes not only from the SDU, but also from
the Ethernet header and FCS, so those need to be accounted for as well.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v3:
Now setting the ds->mtu_enforcement_ingress variable to true.
Changes in v2:
None.
drivers/net/dsa/sja1105/sja1105.h | 1 +
drivers/net/dsa/sja1105/sja1105_main.c | 50 +++++++++++++++++++++++---
2 files changed, 47 insertions(+), 4 deletions(-)
diff --git a/drivers/net/dsa/sja1105/sja1105.h b/drivers/net/dsa/sja1105/sja1105.h
index a358fc89a6db..0e5b739b2fe8 100644
--- a/drivers/net/dsa/sja1105/sja1105.h
+++ b/drivers/net/dsa/sja1105/sja1105.h
@@ -126,6 +126,7 @@ enum sja1105_reset_reason {
SJA1105_RX_HWTSTAMPING,
SJA1105_AGEING_TIME,
SJA1105_SCHEDULING,
+ SJA1105_BEST_EFFORT_POLICING,
};
int sja1105_static_config_reload(struct sja1105_private *priv,
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index e0c99bb63cdf..763ae1d3bca8 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -519,12 +519,12 @@ static int sja1105_init_avb_params(struct sja1105_private *priv)
#define SJA1105_RATE_MBPS(speed) (((speed) * 64000) / 1000)
static void sja1105_setup_policer(struct sja1105_l2_policing_entry *policing,
- int index)
+ int index, int mtu)
{
policing[index].sharindx = index;
policing[index].smax = 65535; /* Burst size in bytes */
policing[index].rate = SJA1105_RATE_MBPS(1000);
- policing[index].maxlen = ETH_FRAME_LEN + VLAN_HLEN + ETH_FCS_LEN;
+ policing[index].maxlen = mtu;
policing[index].partition = 0;
}
@@ -556,12 +556,16 @@ static int sja1105_init_l2_policing(struct sja1105_private *priv)
*/
for (i = 0, k = 0; i < SJA1105_NUM_PORTS; i++) {
int bcast = (SJA1105_NUM_PORTS * SJA1105_NUM_TC) + i;
+ int mtu = VLAN_ETH_FRAME_LEN + ETH_FCS_LEN;
+
+ if (dsa_is_cpu_port(priv->ds, i))
+ mtu += VLAN_HLEN;
for (j = 0; j < SJA1105_NUM_TC; j++, k++)
- sja1105_setup_policer(policing, k);
+ sja1105_setup_policer(policing, k, mtu);
/* Set up this port's policer for broadcast traffic */
- sja1105_setup_policer(policing, bcast);
+ sja1105_setup_policer(policing, bcast, mtu);
}
return 0;
}
@@ -1544,6 +1548,7 @@ static const char * const sja1105_reset_reasons[] = {
[SJA1105_RX_HWTSTAMPING] = "RX timestamping",
[SJA1105_AGEING_TIME] = "Ageing time",
[SJA1105_SCHEDULING] = "Time-aware scheduling",
+ [SJA1105_BEST_EFFORT_POLICING] = "Best-effort policing",
};
/* For situations where we need to change a setting at runtime that is only
@@ -1952,6 +1957,8 @@ static int sja1105_setup(struct dsa_switch *ds)
/* Advertise the 8 egress queues */
ds->num_tx_queues = SJA1105_NUM_TC;
+ ds->mtu_enforcement_ingress = true;
+
/* The DSA/switchdev model brings up switch ports in standalone mode by
* default, and that means vlan_filtering is 0 since they're not under
* a bridge, so it's safe to set up switch tagging at this time.
@@ -2120,6 +2127,39 @@ static int sja1105_set_ageing_time(struct dsa_switch *ds,
return sja1105_static_config_reload(priv, SJA1105_AGEING_TIME);
}
+static int sja1105_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
+{
+ int bcast = (SJA1105_NUM_PORTS * SJA1105_NUM_TC) + port;
+ struct sja1105_l2_policing_entry *policing;
+ struct sja1105_private *priv = ds->priv;
+ int tc;
+
+ new_mtu += VLAN_ETH_HLEN + ETH_FCS_LEN;
+
+ if (dsa_is_cpu_port(ds, port))
+ new_mtu += VLAN_HLEN;
+
+ policing = priv->static_config.tables[BLK_IDX_L2_POLICING].entries;
+
+ /* We set all 9 port policers to the same value, so just checking the
+ * broadcast one is fine.
+ */
+ if (policing[bcast].maxlen == new_mtu)
+ return 0;
+
+ for (tc = 0; tc < SJA1105_NUM_TC; tc++)
+ policing[port * SJA1105_NUM_TC + tc].maxlen = new_mtu;
+
+ policing[bcast].maxlen = new_mtu;
+
+ return sja1105_static_config_reload(priv, SJA1105_BEST_EFFORT_POLICING);
+}
+
+static int sja1105_get_max_mtu(struct dsa_switch *ds, int port)
+{
+ return 2043 - VLAN_ETH_HLEN - ETH_FCS_LEN;
+}
+
static int sja1105_port_setup_tc(struct dsa_switch *ds, int port,
enum tc_setup_type type,
void *type_data)
@@ -2215,6 +2255,8 @@ static const struct dsa_switch_ops sja1105_switch_ops = {
.setup = sja1105_setup,
.teardown = sja1105_teardown,
.set_ageing_time = sja1105_set_ageing_time,
+ .port_change_mtu = sja1105_change_mtu,
+ .port_max_mtu = sja1105_get_max_mtu,
.phylink_validate = sja1105_phylink_validate,
.phylink_mac_link_state = sja1105_mac_pcs_get_state,
.phylink_mac_config = sja1105_mac_config,
--
2.17.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 net-next 7/8] net: dsa: vsc73xx: make the MTU configurable
2020-03-26 22:40 [PATCH v3 net-next 0/8] Configure the MTU on DSA switches Vladimir Oltean
` (5 preceding siblings ...)
2020-03-26 22:40 ` [PATCH v3 net-next 6/8] net: dsa: sja1105: implement the port MTU callbacks Vladimir Oltean
@ 2020-03-26 22:40 ` Vladimir Oltean
2020-03-26 22:40 ` [PATCH v3 net-next 8/8] net: dsa: felix: support changing the MTU Vladimir Oltean
7 siblings, 0 replies; 16+ messages in thread
From: Vladimir Oltean @ 2020-03-26 22:40 UTC (permalink / raw)
To: andrew, f.fainelli, vivien.didelot, davem, jakub.kicinski
Cc: murali.policharla, stephen, jiri, idosch, kuba, nikolay, netdev
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Instead of hardcoding the MTU to the maximum value allowed by the
hardware, obey the value known by the operating system.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v3:
None.
Changes in v2:
None.
drivers/net/dsa/vitesse-vsc73xx-core.c | 30 +++++++++++++++++---------
1 file changed, 20 insertions(+), 10 deletions(-)
diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c
index 6e21a2a5cf01..19ce4aa0973b 100644
--- a/drivers/net/dsa/vitesse-vsc73xx-core.c
+++ b/drivers/net/dsa/vitesse-vsc73xx-core.c
@@ -664,16 +664,6 @@ static void vsc73xx_init_port(struct vsc73xx *vsc, int port)
VSC73XX_MAC_CFG_TX_EN |
VSC73XX_MAC_CFG_RX_EN);
- /* Max length, we can do up to 9.6 KiB, so allow that.
- * According to application not "VSC7398 Jumbo Frames" setting
- * up the MTU to 9.6 KB does not affect the performance on standard
- * frames, so just enable it. It is clear from the application note
- * that "9.6 kilobytes" == 9600 bytes.
- */
- vsc73xx_write(vsc, VSC73XX_BLOCK_MAC,
- port,
- VSC73XX_MAXLEN, 9600);
-
/* Flow control for the CPU port:
* Use a zero delay pause frame when pause condition is left
* Obey pause control frames
@@ -1030,6 +1020,24 @@ static void vsc73xx_get_ethtool_stats(struct dsa_switch *ds, int port,
}
}
+static int vsc73xx_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
+{
+ struct vsc73xx *vsc = ds->priv;
+
+ return vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port,
+ VSC73XX_MAXLEN, new_mtu);
+}
+
+/* According to application not "VSC7398 Jumbo Frames" setting
+ * up the MTU to 9.6 KB does not affect the performance on standard
+ * frames. It is clear from the application note that
+ * "9.6 kilobytes" == 9600 bytes.
+ */
+static int vsc73xx_get_max_mtu(struct dsa_switch *ds, int port)
+{
+ return 9600;
+}
+
static const struct dsa_switch_ops vsc73xx_ds_ops = {
.get_tag_protocol = vsc73xx_get_tag_protocol,
.setup = vsc73xx_setup,
@@ -1041,6 +1049,8 @@ static const struct dsa_switch_ops vsc73xx_ds_ops = {
.get_sset_count = vsc73xx_get_sset_count,
.port_enable = vsc73xx_port_enable,
.port_disable = vsc73xx_port_disable,
+ .port_change_mtu = vsc73xx_change_mtu,
+ .port_max_mtu = vsc73xx_get_max_mtu,
};
static int vsc73xx_gpio_get(struct gpio_chip *chip, unsigned int offset)
--
2.17.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 net-next 8/8] net: dsa: felix: support changing the MTU
2020-03-26 22:40 [PATCH v3 net-next 0/8] Configure the MTU on DSA switches Vladimir Oltean
` (6 preceding siblings ...)
2020-03-26 22:40 ` [PATCH v3 net-next 7/8] net: dsa: vsc73xx: make the MTU configurable Vladimir Oltean
@ 2020-03-26 22:40 ` Vladimir Oltean
7 siblings, 0 replies; 16+ messages in thread
From: Vladimir Oltean @ 2020-03-26 22:40 UTC (permalink / raw)
To: andrew, f.fainelli, vivien.didelot, davem, jakub.kicinski
Cc: murali.policharla, stephen, jiri, idosch, kuba, nikolay, netdev
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Changing the MTU for this switch means altering the
DEV_GMII:MAC_CFG_STATUS:MAC_MAXLEN_CFG field MAX_LEN, which in turn
limits the size of frames that can be received.
Special accounting needs to be done for the DSA CPU port (NPI port in
hardware terms). The NPI port configuration needs to be held inside the
private ocelot structure, since it is now accessed from multiple places.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v3:
Now setting the ds->mtu_enforcement_ingress variable to true.
Changes in v2:
None.
drivers/net/dsa/ocelot/felix.c | 19 +++++++++++++
drivers/net/ethernet/mscc/ocelot.c | 45 +++++++++++++++++++++++-------
include/soc/mscc/ocelot.h | 7 +++++
3 files changed, 61 insertions(+), 10 deletions(-)
diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 9f9efb974003..eef9fa812a3c 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -533,6 +533,7 @@ static int felix_setup(struct dsa_switch *ds)
ANA_PGID_PGID_PGID(GENMASK(ocelot->num_phys_ports, 0)),
ANA_PGID_PGID, PGID_UC);
+ ds->mtu_enforcement_ingress = true;
/* It looks like the MAC/PCS interrupt register - PM0_IEVENT (0x8040)
* isn't instantiated for the Felix PF.
* In-band AN may take a few ms to complete, so we need to poll.
@@ -610,6 +611,22 @@ static bool felix_txtstamp(struct dsa_switch *ds, int port,
return false;
}
+static int felix_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
+{
+ struct ocelot *ocelot = ds->priv;
+
+ ocelot_port_set_maxlen(ocelot, port, new_mtu);
+
+ return 0;
+}
+
+static int felix_get_max_mtu(struct dsa_switch *ds, int port)
+{
+ struct ocelot *ocelot = ds->priv;
+
+ return ocelot_get_max_mtu(ocelot, port);
+}
+
static int felix_cls_flower_add(struct dsa_switch *ds, int port,
struct flow_cls_offload *cls, bool ingress)
{
@@ -665,6 +682,8 @@ static const struct dsa_switch_ops felix_switch_ops = {
.port_hwtstamp_set = felix_hwtstamp_set,
.port_rxtstamp = felix_rxtstamp,
.port_txtstamp = felix_txtstamp,
+ .port_change_mtu = felix_change_mtu,
+ .port_max_mtu = felix_get_max_mtu,
.cls_flower_add = felix_cls_flower_add,
.cls_flower_del = felix_cls_flower_del,
.cls_flower_stats = felix_cls_flower_stats,
diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 114d6053aa26..b5f925c1b5b2 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -1998,13 +1998,25 @@ EXPORT_SYMBOL(ocelot_switchdev_blocking_nb);
/* Configure the maximum SDU (L2 payload) on RX to the value specified in @sdu.
* The length of VLAN tags is accounted for automatically via DEV_MAC_TAGS_CFG.
+ * In the special case that it's the NPI port that we're configuring, the
+ * length of the tag and optional prefix needs to be accounted for privately,
+ * in order to be able to sustain communication at the requested @sdu.
*/
-static void ocelot_port_set_maxlen(struct ocelot *ocelot, int port, size_t sdu)
+void ocelot_port_set_maxlen(struct ocelot *ocelot, int port, size_t sdu)
{
struct ocelot_port *ocelot_port = ocelot->ports[port];
int maxlen = sdu + ETH_HLEN + ETH_FCS_LEN;
int atop_wm;
+ if (port == ocelot->npi) {
+ maxlen += OCELOT_TAG_LEN;
+
+ if (ocelot->inj_prefix == OCELOT_TAG_PREFIX_SHORT)
+ maxlen += OCELOT_SHORT_PREFIX_LEN;
+ else if (ocelot->inj_prefix == OCELOT_TAG_PREFIX_LONG)
+ maxlen += OCELOT_LONG_PREFIX_LEN;
+ }
+
ocelot_port_writel(ocelot_port, maxlen, DEV_MAC_MAXLEN_CFG);
/* Set Pause WM hysteresis
@@ -2022,6 +2034,24 @@ static void ocelot_port_set_maxlen(struct ocelot *ocelot, int port, size_t sdu)
SYS_ATOP, port);
ocelot_write(ocelot, ocelot_wm_enc(atop_wm), SYS_ATOP_TOT_CFG);
}
+EXPORT_SYMBOL(ocelot_port_set_maxlen);
+
+int ocelot_get_max_mtu(struct ocelot *ocelot, int port)
+{
+ int max_mtu = 65535 - ETH_HLEN - ETH_FCS_LEN;
+
+ if (port == ocelot->npi) {
+ max_mtu -= OCELOT_TAG_LEN;
+
+ if (ocelot->inj_prefix == OCELOT_TAG_PREFIX_SHORT)
+ max_mtu -= OCELOT_SHORT_PREFIX_LEN;
+ else if (ocelot->inj_prefix == OCELOT_TAG_PREFIX_LONG)
+ max_mtu -= OCELOT_LONG_PREFIX_LEN;
+ }
+
+ return max_mtu;
+}
+EXPORT_SYMBOL(ocelot_get_max_mtu);
void ocelot_init_port(struct ocelot *ocelot, int port)
{
@@ -2131,6 +2161,10 @@ void ocelot_configure_cpu(struct ocelot *ocelot, int npi,
{
int cpu = ocelot->num_phys_ports;
+ ocelot->npi = npi;
+ ocelot->inj_prefix = injection;
+ ocelot->xtr_prefix = extraction;
+
/* The unicast destination PGID for the CPU port module is unused */
ocelot_write_rix(ocelot, 0, ANA_PGID_PGID, cpu);
/* Instead set up a multicast destination PGID for traffic copied to
@@ -2143,19 +2177,10 @@ void ocelot_configure_cpu(struct ocelot *ocelot, int npi,
ANA_PORT_PORT_CFG, cpu);
if (npi >= 0 && npi < ocelot->num_phys_ports) {
- int sdu = ETH_DATA_LEN + OCELOT_TAG_LEN;
-
ocelot_write(ocelot, QSYS_EXT_CPU_CFG_EXT_CPUQ_MSK_M |
QSYS_EXT_CPU_CFG_EXT_CPU_PORT(npi),
QSYS_EXT_CPU_CFG);
- if (injection == OCELOT_TAG_PREFIX_SHORT)
- sdu += OCELOT_SHORT_PREFIX_LEN;
- else if (injection == OCELOT_TAG_PREFIX_LONG)
- sdu += OCELOT_LONG_PREFIX_LEN;
-
- ocelot_port_set_maxlen(ocelot, npi, sdu);
-
/* Enable NPI port */
ocelot_write_rix(ocelot,
QSYS_SWITCH_PORT_MODE_INGRESS_DROP_MODE |
diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index db2fb14bd775..23a78d927838 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -522,6 +522,11 @@ struct ocelot {
*/
u8 num_phys_ports;
+ int npi;
+
+ enum ocelot_tag_prefix inj_prefix;
+ enum ocelot_tag_prefix xtr_prefix;
+
u32 *lags;
struct list_head multicast;
@@ -616,6 +621,8 @@ int ocelot_hwstamp_set(struct ocelot *ocelot, int port, struct ifreq *ifr);
int ocelot_port_add_txtstamp_skb(struct ocelot_port *ocelot_port,
struct sk_buff *skb);
void ocelot_get_txtstamp(struct ocelot *ocelot);
+void ocelot_port_set_maxlen(struct ocelot *ocelot, int port, size_t sdu);
+int ocelot_get_max_mtu(struct ocelot *ocelot, int port);
int ocelot_cls_flower_replace(struct ocelot *ocelot, int port,
struct flow_cls_offload *f, bool ingress);
int ocelot_cls_flower_destroy(struct ocelot *ocelot, int port,
--
2.17.1
^ permalink raw reply related [flat|nested] 16+ messages in thread