| Message ID | 20230314182405.2449898-2-vladimir.oltean@nxp.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 636e8adf7878eab3614250234341bde45537f47a |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Fix MTU reporting for Marvell DSA switches where we can't change it | expand |
| Context | Check | Description |
|---|---|---|
| netdev/series_format | success | Posting correctly formatted |
| netdev/tree_selection | success | Clearly marked for net |
| netdev/fixes_present | success | Fixes tag present in non-next series |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 18 this patch: 18 |
| netdev/cc_maintainers | success | CCed 9 of 9 maintainers |
| netdev/build_clang | success | Errors and warnings before: 18 this patch: 18 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/deprecated_api | success | None detected |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | Fixes tag looks correct |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 18 this patch: 18 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 28 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Tue, Mar 14, 2023 at 08:24:04PM +0200, Vladimir Oltean wrote: > Currently, when dsa_slave_change_mtu() is called on a user port where > dev->max_mtu is 1500 (as returned by ds->ops->port_max_mtu()), the code > will stumble upon this check: > > if (new_master_mtu > mtu_limit) > return -ERANGE; > > because new_master_mtu is adjusted for the tagger overhead but mtu_limit > is not. > > But it would be good if the logic went through, for example if the DSA > master really depends on an MTU adjustment to accept DSA-tagged frames. > > To make the code pass through the check, we need to adjust mtu_limit for > the overhead as well, if the minimum restriction was caused by the DSA > user port's MTU (dev->max_mtu). A DSA user port MTU and a DSA master MTU > are always offset by the protocol overhead. > > Currently no drivers return 1500 .port_max_mtu(), but this is only > temporary and a bug in itself - mv88e6xxx should have done that, but > since commit b9c587fed61c ("dsa: mv88e6xxx: Include tagger overhead when > setting MTU for DSA and CPU ports") it no longer does. This is a > preparation for fixing that. > > Fixes: bfcb813203e6 ("net: dsa: configure the MTU for switch ports") > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Reviewed-by: Simon Horman <simon.horman@corigine.com>
On 3/14/23 11:24, Vladimir Oltean wrote: > Currently, when dsa_slave_change_mtu() is called on a user port where > dev->max_mtu is 1500 (as returned by ds->ops->port_max_mtu()), the code > will stumble upon this check: > > if (new_master_mtu > mtu_limit) > return -ERANGE; > > because new_master_mtu is adjusted for the tagger overhead but mtu_limit > is not. > > But it would be good if the logic went through, for example if the DSA > master really depends on an MTU adjustment to accept DSA-tagged frames. > > To make the code pass through the check, we need to adjust mtu_limit for > the overhead as well, if the minimum restriction was caused by the DSA > user port's MTU (dev->max_mtu). A DSA user port MTU and a DSA master MTU > are always offset by the protocol overhead. > > Currently no drivers return 1500 .port_max_mtu(), but this is only > temporary and a bug in itself - mv88e6xxx should have done that, but > since commit b9c587fed61c ("dsa: mv88e6xxx: Include tagger overhead when > setting MTU for DSA and CPU ports") it no longer does. This is a > preparation for fixing that. > > Fixes: bfcb813203e6 ("net: dsa: configure the MTU for switch ports") > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
diff --git a/net/dsa/slave.c b/net/dsa/slave.c index 6957971c2db2..cac17183589f 100644 --- a/net/dsa/slave.c +++ b/net/dsa/slave.c @@ -1933,6 +1933,7 @@ int dsa_slave_change_mtu(struct net_device *dev, int new_mtu) int new_master_mtu; int old_master_mtu; int mtu_limit; + int overhead; int cpu_mtu; int err; @@ -1961,9 +1962,10 @@ int dsa_slave_change_mtu(struct net_device *dev, int new_mtu) largest_mtu = slave_mtu; } - mtu_limit = min_t(int, master->max_mtu, dev->max_mtu); + overhead = dsa_tag_protocol_overhead(cpu_dp->tag_ops); + mtu_limit = min_t(int, master->max_mtu, dev->max_mtu + overhead); old_master_mtu = master->mtu; - new_master_mtu = largest_mtu + dsa_tag_protocol_overhead(cpu_dp->tag_ops); + new_master_mtu = largest_mtu + overhead; if (new_master_mtu > mtu_limit) return -ERANGE; @@ -1998,8 +2000,7 @@ int dsa_slave_change_mtu(struct net_device *dev, int new_mtu) out_port_failed: if (new_master_mtu != old_master_mtu) - dsa_port_mtu_change(cpu_dp, old_master_mtu - - dsa_tag_protocol_overhead(cpu_dp->tag_ops)); + dsa_port_mtu_change(cpu_dp, old_master_mtu - overhead); out_cpu_failed: if (new_master_mtu != old_master_mtu) dev_set_mtu(master, old_master_mtu);
Currently, when dsa_slave_change_mtu() is called on a user port where dev->max_mtu is 1500 (as returned by ds->ops->port_max_mtu()), the code will stumble upon this check: if (new_master_mtu > mtu_limit) return -ERANGE; because new_master_mtu is adjusted for the tagger overhead but mtu_limit is not. But it would be good if the logic went through, for example if the DSA master really depends on an MTU adjustment to accept DSA-tagged frames. To make the code pass through the check, we need to adjust mtu_limit for the overhead as well, if the minimum restriction was caused by the DSA user port's MTU (dev->max_mtu). A DSA user port MTU and a DSA master MTU are always offset by the protocol overhead. Currently no drivers return 1500 .port_max_mtu(), but this is only temporary and a bug in itself - mv88e6xxx should have done that, but since commit b9c587fed61c ("dsa: mv88e6xxx: Include tagger overhead when setting MTU for DSA and CPU ports") it no longer does. This is a preparation for fixing that. Fixes: bfcb813203e6 ("net: dsa: configure the MTU for switch ports") Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- net/dsa/slave.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)