* [PATCH] net: dsa: qca8k: implement the port MTU callbacks @ 2020-07-18 9:35 Jonathan McDowell 2020-07-18 10:38 ` Vladimir Oltean 2020-07-18 16:32 ` [PATCH v2] " Jonathan McDowell 0 siblings, 2 replies; 6+ messages in thread From: Jonathan McDowell @ 2020-07-18 9:35 UTC (permalink / raw) To: Vladimir Oltean, Andrew Lunn, Vivien Didelot, Florian Fainelli, David Miller, Jakub Kicinski, Matthew Hagan, netdev, linux-kernel This switch has a single max frame size configuration register, so we track the requested MTU for each port and apply the largest. Signed-off-by: Jonathan McDowell <noodles@earth.li> --- drivers/net/dsa/qca8k.c | 38 ++++++++++++++++++++++++++++++++++++++ drivers/net/dsa/qca8k.h | 3 +++ 2 files changed, 41 insertions(+) diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c index 4acad5fa0c84..3690f02aea3a 100644 --- a/drivers/net/dsa/qca8k.c +++ b/drivers/net/dsa/qca8k.c @@ -670,6 +670,12 @@ qca8k_setup(struct dsa_switch *ds) } } + /* Setup our port MTUs to match power on defaults */ + for (i = 0; i < QCA8K_NUM_PORTS; i++) { + priv->port_mtu[i] = ETH_FRAME_LEN + ETH_FCS_LEN; + } + qca8k_write(priv, QCA8K_MAX_FRAME_SIZE, ETH_FRAME_LEN + ETH_FCS_LEN); + /* Flush the FDB table */ qca8k_fdb_flush(priv); @@ -1098,6 +1104,36 @@ qca8k_port_disable(struct dsa_switch *ds, int port) priv->port_sts[port].enabled = 0; } +static int +qca8k_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu) +{ + struct qca8k_priv *priv = ds->priv; + int i, mtu; + + if ((new_mtu < ETH_MIN_MTU) || (new_mtu > QCA8K_MAX_MTU)) { + return -EINVAL; + } + + priv->port_mtu[port] = new_mtu; + + mtu = 0; + for (i = 0; i < QCA8K_NUM_PORTS; i++) { + if (priv->port_mtu[port] > mtu) + mtu = priv->port_mtu[port]; + } + + /* Include L2 header / FCS length */ + qca8k_write(priv, QCA8K_MAX_FRAME_SIZE, mtu + ETH_HLEN + ETH_FCS_LEN); + + return 0; +} + +static int +qca8k_port_max_mtu(struct dsa_switch *ds, int port) +{ + return QCA8K_MAX_MTU; +} + static int qca8k_port_fdb_insert(struct qca8k_priv *priv, const u8 *addr, u16 port_mask, u16 vid) @@ -1174,6 +1210,8 @@ static const struct dsa_switch_ops qca8k_switch_ops = { .set_mac_eee = qca8k_set_mac_eee, .port_enable = qca8k_port_enable, .port_disable = qca8k_port_disable, + .port_change_mtu = qca8k_port_change_mtu, + .port_max_mtu = qca8k_port_max_mtu, .port_stp_state_set = qca8k_port_stp_state_set, .port_bridge_join = qca8k_port_bridge_join, .port_bridge_leave = qca8k_port_bridge_leave, diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h index 10ef2bca2cde..31439396401c 100644 --- a/drivers/net/dsa/qca8k.h +++ b/drivers/net/dsa/qca8k.h @@ -13,6 +13,7 @@ #include <linux/gpio.h> #define QCA8K_NUM_PORTS 7 +#define QCA8K_MAX_MTU 9000 #define PHY_ID_QCA8337 0x004dd036 #define QCA8K_ID_QCA8337 0x13 @@ -58,6 +59,7 @@ #define QCA8K_MDIO_MASTER_MAX_REG 32 #define QCA8K_GOL_MAC_ADDR0 0x60 #define QCA8K_GOL_MAC_ADDR1 0x64 +#define QCA8K_MAX_FRAME_SIZE 0x78 #define QCA8K_REG_PORT_STATUS(_i) (0x07c + (_i) * 4) #define QCA8K_PORT_STATUS_SPEED GENMASK(1, 0) #define QCA8K_PORT_STATUS_SPEED_10 0 @@ -189,6 +191,7 @@ struct qca8k_priv { struct device *dev; struct dsa_switch_ops ops; struct gpio_desc *reset_gpio; + unsigned int port_mtu[QCA8K_NUM_PORTS]; }; struct qca8k_mib_desc { -- 2.27.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] net: dsa: qca8k: implement the port MTU callbacks 2020-07-18 9:35 [PATCH] net: dsa: qca8k: implement the port MTU callbacks Jonathan McDowell @ 2020-07-18 10:38 ` Vladimir Oltean 2020-07-18 11:21 ` Jonathan McDowell 2020-07-18 16:32 ` [PATCH v2] " Jonathan McDowell 1 sibling, 1 reply; 6+ messages in thread From: Vladimir Oltean @ 2020-07-18 10:38 UTC (permalink / raw) To: Jonathan McDowell Cc: Vladimir Oltean, Andrew Lunn, Vivien Didelot, Florian Fainelli, David Miller, Jakub Kicinski, Matthew Hagan, netdev, linux-kernel On Sat, Jul 18, 2020 at 10:35:55AM +0100, Jonathan McDowell wrote: > This switch has a single max frame size configuration register, so we > track the requested MTU for each port and apply the largest. > > Signed-off-by: Jonathan McDowell <noodles@earth.li> > --- > drivers/net/dsa/qca8k.c | 38 ++++++++++++++++++++++++++++++++++++++ > drivers/net/dsa/qca8k.h | 3 +++ > 2 files changed, 41 insertions(+) > > diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c > index 4acad5fa0c84..3690f02aea3a 100644 > --- a/drivers/net/dsa/qca8k.c > +++ b/drivers/net/dsa/qca8k.c > @@ -670,6 +670,12 @@ qca8k_setup(struct dsa_switch *ds) > } > } > > + /* Setup our port MTUs to match power on defaults */ > + for (i = 0; i < QCA8K_NUM_PORTS; i++) { > + priv->port_mtu[i] = ETH_FRAME_LEN + ETH_FCS_LEN; > + } I am not quite sure the curly brackets are needed. And nowhere else in qca8k.c is this convention being used. > + qca8k_write(priv, QCA8K_MAX_FRAME_SIZE, ETH_FRAME_LEN + ETH_FCS_LEN); > + > /* Flush the FDB table */ > qca8k_fdb_flush(priv); > > @@ -1098,6 +1104,36 @@ qca8k_port_disable(struct dsa_switch *ds, int port) > priv->port_sts[port].enabled = 0; > } > > +static int > +qca8k_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu) > +{ > + struct qca8k_priv *priv = ds->priv; > + int i, mtu; > + > + if ((new_mtu < ETH_MIN_MTU) || (new_mtu > QCA8K_MAX_MTU)) { > + return -EINVAL; > + } I'm pretty sure this check should not be needed. The only reason why slave_dev->min_mtu is 0 seems to be: commit 8b1efc0f83f1f75b8f85c70d2211007de8fd7633 Author: Jarod Wilson <jarod@redhat.com> Date: Thu Oct 20 23:25:27 2016 -0400 net: remove MTU limits on a few ether_setup callers These few drivers call ether_setup(), but have no ndo_change_mtu, and thus were overlooked for changes to MTU range checking behavior. They previously had no range checks, so for feature-parity, set their min_mtu to 0 and max_mtu to ETH_MAX_MTU (65535), instead of the 68 and 1500 inherited from the ether_setup() changes. Fine-tuning can come after we get back to full feature-parity here. CC: netdev@vger.kernel.org Reported-by: Asbjoern Sloth Toennesen <asbjorn@asbjorn.st> CC: Asbjoern Sloth Toennesen <asbjorn@asbjorn.st> CC: R Parameswaran <parameswaran.r7@gmail.com> Signed-off-by: Jarod Wilson <jarod@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net> which is an oversight on my part. Since now DSA supports ndo_change_mtu(), the "slave_dev->min_mtu = 0;" line in net/dsa/slave.c can be removed and so can this check. > + > + priv->port_mtu[port] = new_mtu; > + > + mtu = 0; I think it's more typical to initialize mtu to 0 at declaration time. > + for (i = 0; i < QCA8K_NUM_PORTS; i++) { > + if (priv->port_mtu[port] > mtu) > + mtu = priv->port_mtu[port]; > + } Again, curly brackets are not needed here, although some might feel it aids readability. > + > + /* Include L2 header / FCS length */ > + qca8k_write(priv, QCA8K_MAX_FRAME_SIZE, mtu + ETH_HLEN + ETH_FCS_LEN); > + > + return 0; > +} > + > +static int > +qca8k_port_max_mtu(struct dsa_switch *ds, int port) > +{ > + return QCA8K_MAX_MTU; So what is the maximum value that you can write into QCA8K_MAX_FRAME_SIZE? 9000 or 9018? If it's 9000, you should report a max MTU of 8982, and let the network stack do the range check for you, that's why this callback exists in the first place. Do you know how are VLAN tags accounted for (i.e. does iperf3 TCP work over a VLAN sub-interface after your patch)? There are 2 options: - The ports automatically increase the maximum accepted frame size by 4 (or 8, in case of double tag) bytes if they see VLAN tagged traffic. Case in which you don't need to do anything. - You need to manually account for the possibility that VLAN-tagged traffic will be received, since the 802.1Q header is not part of the SDU whose max length is measured by the MTU. So you might want to write a value to QCA8K_MAX_FRAME_SIZE that is either "mtu + VLAN_ETH_HLEN + ETH_FCS_LEN", or "mtu + ETH_HLEN + 2 * VLAN_HLEN + ETH_FCS_LEN", depending on whether you foresee double-tagging being used. > +} > + > static int > qca8k_port_fdb_insert(struct qca8k_priv *priv, const u8 *addr, > u16 port_mask, u16 vid) > @@ -1174,6 +1210,8 @@ static const struct dsa_switch_ops qca8k_switch_ops = { > .set_mac_eee = qca8k_set_mac_eee, > .port_enable = qca8k_port_enable, > .port_disable = qca8k_port_disable, > + .port_change_mtu = qca8k_port_change_mtu, > + .port_max_mtu = qca8k_port_max_mtu, > .port_stp_state_set = qca8k_port_stp_state_set, > .port_bridge_join = qca8k_port_bridge_join, > .port_bridge_leave = qca8k_port_bridge_leave, > diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h > index 10ef2bca2cde..31439396401c 100644 > --- a/drivers/net/dsa/qca8k.h > +++ b/drivers/net/dsa/qca8k.h > @@ -13,6 +13,7 @@ > #include <linux/gpio.h> > > #define QCA8K_NUM_PORTS 7 > +#define QCA8K_MAX_MTU 9000 > > #define PHY_ID_QCA8337 0x004dd036 > #define QCA8K_ID_QCA8337 0x13 > @@ -58,6 +59,7 @@ > #define QCA8K_MDIO_MASTER_MAX_REG 32 > #define QCA8K_GOL_MAC_ADDR0 0x60 > #define QCA8K_GOL_MAC_ADDR1 0x64 > +#define QCA8K_MAX_FRAME_SIZE 0x78 > #define QCA8K_REG_PORT_STATUS(_i) (0x07c + (_i) * 4) > #define QCA8K_PORT_STATUS_SPEED GENMASK(1, 0) > #define QCA8K_PORT_STATUS_SPEED_10 0 > @@ -189,6 +191,7 @@ struct qca8k_priv { > struct device *dev; > struct dsa_switch_ops ops; > struct gpio_desc *reset_gpio; > + unsigned int port_mtu[QCA8K_NUM_PORTS]; > }; > > struct qca8k_mib_desc { > -- > 2.27.0 > Thanks, -Vladimir ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: dsa: qca8k: implement the port MTU callbacks 2020-07-18 10:38 ` Vladimir Oltean @ 2020-07-18 11:21 ` Jonathan McDowell 0 siblings, 0 replies; 6+ messages in thread From: Jonathan McDowell @ 2020-07-18 11:21 UTC (permalink / raw) To: Vladimir Oltean Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, David Miller, Jakub Kicinski, Matthew Hagan, netdev, linux-kernel On Sat, Jul 18, 2020 at 01:38:08PM +0300, Vladimir Oltean wrote: > On Sat, Jul 18, 2020 at 10:35:55AM +0100, Jonathan McDowell wrote: > > This switch has a single max frame size configuration register, so we > > track the requested MTU for each port and apply the largest. > > > > Signed-off-by: Jonathan McDowell <noodles@earth.li> > > --- > > drivers/net/dsa/qca8k.c | 38 ++++++++++++++++++++++++++++++++++++++ > > drivers/net/dsa/qca8k.h | 3 +++ > > 2 files changed, 41 insertions(+) > > > > diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c > > index 4acad5fa0c84..3690f02aea3a 100644 > > --- a/drivers/net/dsa/qca8k.c > > +++ b/drivers/net/dsa/qca8k.c > > @@ -670,6 +670,12 @@ qca8k_setup(struct dsa_switch *ds) > > } > > } > > > > + /* Setup our port MTUs to match power on defaults */ > > + for (i = 0; i < QCA8K_NUM_PORTS; i++) { > > + priv->port_mtu[i] = ETH_FRAME_LEN + ETH_FCS_LEN; > > + } > > I am not quite sure the curly brackets are needed. And nowhere else in > qca8k.c is this convention being used. Good point; force of habit from coding standards elsewhere. Fixed (and also the instance below). > > + qca8k_write(priv, QCA8K_MAX_FRAME_SIZE, ETH_FRAME_LEN + ETH_FCS_LEN); > > + > > /* Flush the FDB table */ > > qca8k_fdb_flush(priv); > > > > @@ -1098,6 +1104,36 @@ qca8k_port_disable(struct dsa_switch *ds, int port) > > priv->port_sts[port].enabled = 0; > > } > > > > +static int > > +qca8k_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu) > > +{ > > + struct qca8k_priv *priv = ds->priv; > > + int i, mtu; > > + > > + if ((new_mtu < ETH_MIN_MTU) || (new_mtu > QCA8K_MAX_MTU)) { > > + return -EINVAL; > > + } > > I'm pretty sure this check should not be needed. > The only reason why slave_dev->min_mtu is 0 seems to be: > > commit 8b1efc0f83f1f75b8f85c70d2211007de8fd7633 > Author: Jarod Wilson <jarod@redhat.com> > Date: Thu Oct 20 23:25:27 2016 -0400 > > net: remove MTU limits on a few ether_setup callers > > These few drivers call ether_setup(), but have no ndo_change_mtu, and thus > were overlooked for changes to MTU range checking behavior. They > previously had no range checks, so for feature-parity, set their min_mtu > to 0 and max_mtu to ETH_MAX_MTU (65535), instead of the 68 and 1500 > inherited from the ether_setup() changes. Fine-tuning can come after we get > back to full feature-parity here. > > CC: netdev@vger.kernel.org > Reported-by: Asbjoern Sloth Toennesen <asbjorn@asbjorn.st> > CC: Asbjoern Sloth Toennesen <asbjorn@asbjorn.st> > CC: R Parameswaran <parameswaran.r7@gmail.com> > Signed-off-by: Jarod Wilson <jarod@redhat.com> > Signed-off-by: David S. Miller <davem@davemloft.net> > > which is an oversight on my part. Since now DSA supports > ndo_change_mtu(), the "slave_dev->min_mtu = 0;" line in net/dsa/slave.c > can be removed and so can this check. Ok. > > + > > + priv->port_mtu[port] = new_mtu; > > + > > + mtu = 0; > > I think it's more typical to initialize mtu to 0 at declaration time. Sure, fixed. > > + for (i = 0; i < QCA8K_NUM_PORTS; i++) { > > + if (priv->port_mtu[port] > mtu) > > + mtu = priv->port_mtu[port]; > > + } > > Again, curly brackets are not needed here, although some might feel it > aids readability. > > > + > > + /* Include L2 header / FCS length */ > > + qca8k_write(priv, QCA8K_MAX_FRAME_SIZE, mtu + ETH_HLEN + ETH_FCS_LEN); > > + > > + return 0; > > +} > > + > > +static int > > +qca8k_port_max_mtu(struct dsa_switch *ds, int port) > > +{ > > + return QCA8K_MAX_MTU; > > So what is the maximum value that you can write into > QCA8K_MAX_FRAME_SIZE? 9000 or 9018? If it's 9000, you should report a > max MTU of 8982, and let the network stack do the range check for you, > that's why this callback exists in the first place. 9018 (including header + fcs), hence returning QCA8K_MAX_MTU here. > Do you know how are VLAN tags accounted for (i.e. does iperf3 TCP work > over a VLAN sub-interface after your patch)? There are 2 options: > - The ports automatically increase the maximum accepted frame size by 4 > (or 8, in case of double tag) bytes if they see VLAN tagged traffic. > Case in which you don't need to do anything. > - You need to manually account for the possibility that VLAN-tagged > traffic will be received, since the 802.1Q header is not part of the > SDU whose max length is measured by the MTU. So you might want to > write a value to QCA8K_MAX_FRAME_SIZE that is either "mtu + > VLAN_ETH_HLEN + ETH_FCS_LEN", or "mtu + ETH_HLEN + 2 * VLAN_HLEN + > ETH_FCS_LEN", depending on whether you foresee double-tagging being > used. The value is for a normal packet; the switch accounts for VLAN / double VLAN / the switch tagging header size itself. > > +} > > + > > static int > > qca8k_port_fdb_insert(struct qca8k_priv *priv, const u8 *addr, > > u16 port_mask, u16 vid) > > @@ -1174,6 +1210,8 @@ static const struct dsa_switch_ops qca8k_switch_ops = { > > .set_mac_eee = qca8k_set_mac_eee, > > .port_enable = qca8k_port_enable, > > .port_disable = qca8k_port_disable, > > + .port_change_mtu = qca8k_port_change_mtu, > > + .port_max_mtu = qca8k_port_max_mtu, > > .port_stp_state_set = qca8k_port_stp_state_set, > > .port_bridge_join = qca8k_port_bridge_join, > > .port_bridge_leave = qca8k_port_bridge_leave, > > diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h > > index 10ef2bca2cde..31439396401c 100644 > > --- a/drivers/net/dsa/qca8k.h > > +++ b/drivers/net/dsa/qca8k.h > > @@ -13,6 +13,7 @@ > > #include <linux/gpio.h> > > > > #define QCA8K_NUM_PORTS 7 > > +#define QCA8K_MAX_MTU 9000 > > > > #define PHY_ID_QCA8337 0x004dd036 > > #define QCA8K_ID_QCA8337 0x13 > > @@ -58,6 +59,7 @@ > > #define QCA8K_MDIO_MASTER_MAX_REG 32 > > #define QCA8K_GOL_MAC_ADDR0 0x60 > > #define QCA8K_GOL_MAC_ADDR1 0x64 > > +#define QCA8K_MAX_FRAME_SIZE 0x78 > > #define QCA8K_REG_PORT_STATUS(_i) (0x07c + (_i) * 4) > > #define QCA8K_PORT_STATUS_SPEED GENMASK(1, 0) > > #define QCA8K_PORT_STATUS_SPEED_10 0 > > @@ -189,6 +191,7 @@ struct qca8k_priv { > > struct device *dev; > > struct dsa_switch_ops ops; > > struct gpio_desc *reset_gpio; > > + unsigned int port_mtu[QCA8K_NUM_PORTS]; > > }; > > > > struct qca8k_mib_desc { > > -- > > 2.27.0 J. -- /-\ | 101 things you can't have too much |@/ Debian GNU/Linux Developer | of : 19 - A Good Thing. \- | ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] net: dsa: qca8k: implement the port MTU callbacks 2020-07-18 9:35 [PATCH] net: dsa: qca8k: implement the port MTU callbacks Jonathan McDowell 2020-07-18 10:38 ` Vladimir Oltean @ 2020-07-18 16:32 ` Jonathan McDowell 2020-07-18 18:11 ` Vladimir Oltean 2020-07-21 1:34 ` David Miller 1 sibling, 2 replies; 6+ messages in thread From: Jonathan McDowell @ 2020-07-18 16:32 UTC (permalink / raw) To: Vladimir Oltean, Andrew Lunn, Vivien Didelot, Florian Fainelli, David Miller, Jakub Kicinski, Matthew Hagan, netdev, linux-kernel This switch has a single max frame size configuration register, so we track the requested MTU for each port and apply the largest. v2: - Address review feedback from Vladimir Oltean Signed-off-by: Jonathan McDowell <noodles@earth.li> --- drivers/net/dsa/qca8k.c | 31 +++++++++++++++++++++++++++++++ drivers/net/dsa/qca8k.h | 3 +++ 2 files changed, 34 insertions(+) diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c index 4acad5fa0c84..a5566de82853 100644 --- a/drivers/net/dsa/qca8k.c +++ b/drivers/net/dsa/qca8k.c @@ -670,6 +670,11 @@ qca8k_setup(struct dsa_switch *ds) } } + /* Setup our port MTUs to match power on defaults */ + for (i = 0; i < QCA8K_NUM_PORTS; i++) + priv->port_mtu[i] = ETH_FRAME_LEN + ETH_FCS_LEN; + qca8k_write(priv, QCA8K_MAX_FRAME_SIZE, ETH_FRAME_LEN + ETH_FCS_LEN); + /* Flush the FDB table */ qca8k_fdb_flush(priv); @@ -1098,6 +1103,30 @@ qca8k_port_disable(struct dsa_switch *ds, int port) priv->port_sts[port].enabled = 0; } +static int +qca8k_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu) +{ + struct qca8k_priv *priv = ds->priv; + int i, mtu = 0; + + priv->port_mtu[port] = new_mtu; + + for (i = 0; i < QCA8K_NUM_PORTS; i++) + if (priv->port_mtu[port] > mtu) + mtu = priv->port_mtu[port]; + + /* Include L2 header / FCS length */ + qca8k_write(priv, QCA8K_MAX_FRAME_SIZE, mtu + ETH_HLEN + ETH_FCS_LEN); + + return 0; +} + +static int +qca8k_port_max_mtu(struct dsa_switch *ds, int port) +{ + return QCA8K_MAX_MTU; +} + static int qca8k_port_fdb_insert(struct qca8k_priv *priv, const u8 *addr, u16 port_mask, u16 vid) @@ -1174,6 +1203,8 @@ static const struct dsa_switch_ops qca8k_switch_ops = { .set_mac_eee = qca8k_set_mac_eee, .port_enable = qca8k_port_enable, .port_disable = qca8k_port_disable, + .port_change_mtu = qca8k_port_change_mtu, + .port_max_mtu = qca8k_port_max_mtu, .port_stp_state_set = qca8k_port_stp_state_set, .port_bridge_join = qca8k_port_bridge_join, .port_bridge_leave = qca8k_port_bridge_leave, diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h index 10ef2bca2cde..31439396401c 100644 --- a/drivers/net/dsa/qca8k.h +++ b/drivers/net/dsa/qca8k.h @@ -13,6 +13,7 @@ #include <linux/gpio.h> #define QCA8K_NUM_PORTS 7 +#define QCA8K_MAX_MTU 9000 #define PHY_ID_QCA8337 0x004dd036 #define QCA8K_ID_QCA8337 0x13 @@ -58,6 +59,7 @@ #define QCA8K_MDIO_MASTER_MAX_REG 32 #define QCA8K_GOL_MAC_ADDR0 0x60 #define QCA8K_GOL_MAC_ADDR1 0x64 +#define QCA8K_MAX_FRAME_SIZE 0x78 #define QCA8K_REG_PORT_STATUS(_i) (0x07c + (_i) * 4) #define QCA8K_PORT_STATUS_SPEED GENMASK(1, 0) #define QCA8K_PORT_STATUS_SPEED_10 0 @@ -189,6 +191,7 @@ struct qca8k_priv { struct device *dev; struct dsa_switch_ops ops; struct gpio_desc *reset_gpio; + unsigned int port_mtu[QCA8K_NUM_PORTS]; }; struct qca8k_mib_desc { -- 2.27.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] net: dsa: qca8k: implement the port MTU callbacks 2020-07-18 16:32 ` [PATCH v2] " Jonathan McDowell @ 2020-07-18 18:11 ` Vladimir Oltean 2020-07-21 1:34 ` David Miller 1 sibling, 0 replies; 6+ messages in thread From: Vladimir Oltean @ 2020-07-18 18:11 UTC (permalink / raw) To: Jonathan McDowell Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, David Miller, Jakub Kicinski, Matthew Hagan, netdev, linux-kernel On Sat, Jul 18, 2020 at 05:32:14PM +0100, Jonathan McDowell wrote: > This switch has a single max frame size configuration register, so we > track the requested MTU for each port and apply the largest. > > v2: > - Address review feedback from Vladimir Oltean > > Signed-off-by: Jonathan McDowell <noodles@earth.li> > --- I think there's still an open question on whether the slave_dev->mtu of all other ports should be updated to reflect the new, potentially larger value. But we could always update drivers after the fact, if that ever becomes required. Acked-by: Vladimir Oltean <olteanv@gmail.com> > drivers/net/dsa/qca8k.c | 31 +++++++++++++++++++++++++++++++ > drivers/net/dsa/qca8k.h | 3 +++ > 2 files changed, 34 insertions(+) > > diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c > index 4acad5fa0c84..a5566de82853 100644 > --- a/drivers/net/dsa/qca8k.c > +++ b/drivers/net/dsa/qca8k.c > @@ -670,6 +670,11 @@ qca8k_setup(struct dsa_switch *ds) > } > } > > + /* Setup our port MTUs to match power on defaults */ > + for (i = 0; i < QCA8K_NUM_PORTS; i++) > + priv->port_mtu[i] = ETH_FRAME_LEN + ETH_FCS_LEN; > + qca8k_write(priv, QCA8K_MAX_FRAME_SIZE, ETH_FRAME_LEN + ETH_FCS_LEN); > + > /* Flush the FDB table */ > qca8k_fdb_flush(priv); > > @@ -1098,6 +1103,30 @@ qca8k_port_disable(struct dsa_switch *ds, int port) > priv->port_sts[port].enabled = 0; > } > > +static int > +qca8k_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu) > +{ > + struct qca8k_priv *priv = ds->priv; > + int i, mtu = 0; > + > + priv->port_mtu[port] = new_mtu; > + > + for (i = 0; i < QCA8K_NUM_PORTS; i++) > + if (priv->port_mtu[port] > mtu) > + mtu = priv->port_mtu[port]; > + > + /* Include L2 header / FCS length */ > + qca8k_write(priv, QCA8K_MAX_FRAME_SIZE, mtu + ETH_HLEN + ETH_FCS_LEN); > + > + return 0; > +} > + > +static int > +qca8k_port_max_mtu(struct dsa_switch *ds, int port) > +{ > + return QCA8K_MAX_MTU; > +} > + > static int > qca8k_port_fdb_insert(struct qca8k_priv *priv, const u8 *addr, > u16 port_mask, u16 vid) > @@ -1174,6 +1203,8 @@ static const struct dsa_switch_ops qca8k_switch_ops = { > .set_mac_eee = qca8k_set_mac_eee, > .port_enable = qca8k_port_enable, > .port_disable = qca8k_port_disable, > + .port_change_mtu = qca8k_port_change_mtu, > + .port_max_mtu = qca8k_port_max_mtu, > .port_stp_state_set = qca8k_port_stp_state_set, > .port_bridge_join = qca8k_port_bridge_join, > .port_bridge_leave = qca8k_port_bridge_leave, > diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h > index 10ef2bca2cde..31439396401c 100644 > --- a/drivers/net/dsa/qca8k.h > +++ b/drivers/net/dsa/qca8k.h > @@ -13,6 +13,7 @@ > #include <linux/gpio.h> > > #define QCA8K_NUM_PORTS 7 > +#define QCA8K_MAX_MTU 9000 > > #define PHY_ID_QCA8337 0x004dd036 > #define QCA8K_ID_QCA8337 0x13 > @@ -58,6 +59,7 @@ > #define QCA8K_MDIO_MASTER_MAX_REG 32 > #define QCA8K_GOL_MAC_ADDR0 0x60 > #define QCA8K_GOL_MAC_ADDR1 0x64 > +#define QCA8K_MAX_FRAME_SIZE 0x78 > #define QCA8K_REG_PORT_STATUS(_i) (0x07c + (_i) * 4) > #define QCA8K_PORT_STATUS_SPEED GENMASK(1, 0) > #define QCA8K_PORT_STATUS_SPEED_10 0 > @@ -189,6 +191,7 @@ struct qca8k_priv { > struct device *dev; > struct dsa_switch_ops ops; > struct gpio_desc *reset_gpio; > + unsigned int port_mtu[QCA8K_NUM_PORTS]; > }; > > struct qca8k_mib_desc { > -- > 2.27.0 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] net: dsa: qca8k: implement the port MTU callbacks 2020-07-18 16:32 ` [PATCH v2] " Jonathan McDowell 2020-07-18 18:11 ` Vladimir Oltean @ 2020-07-21 1:34 ` David Miller 1 sibling, 0 replies; 6+ messages in thread From: David Miller @ 2020-07-21 1:34 UTC (permalink / raw) To: noodles Cc: olteanv, andrew, vivien.didelot, f.fainelli, kuba, mnhagan88, netdev, linux-kernel From: Jonathan McDowell <noodles@earth.li> Date: Sat, 18 Jul 2020 17:32:14 +0100 > This switch has a single max frame size configuration register, so we > track the requested MTU for each port and apply the largest. > > v2: > - Address review feedback from Vladimir Oltean > > Signed-off-by: Jonathan McDowell <noodles@earth.li> Applied to net-next, thanks. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-07-21 1:34 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-18 9:35 [PATCH] net: dsa: qca8k: implement the port MTU callbacks Jonathan McDowell 2020-07-18 10:38 ` Vladimir Oltean 2020-07-18 11:21 ` Jonathan McDowell 2020-07-18 16:32 ` [PATCH v2] " Jonathan McDowell 2020-07-18 18:11 ` Vladimir Oltean 2020-07-21 1:34 ` David Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).