| Message ID | 20240816224741.596989-1-rosenp@gmail.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [RFC,net-next] net: ag71xx: disable GRO by default | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/apply | fail | Patch does not apply to net-next-1 |
On Fri, Aug 16, 2024 at 03:47:33PM -0700, Rosen Penev wrote: > ``` > Currently this is handled in userspace with ethtool. Not sure if this > should be done in the kernel or if this is even the proper place for it. > ``` Comments like this should be placed under the ---. If the patch is merged, anything in the commit message under the --- is then discarded. > ag71xx is usually paired with qca8k or ar9331, both DSA drivers. Can it be used without a switch? It looks like this option will disable offloads which are useful when there is no switch. Andrew
On Sat, Aug 17, 2024 at 11:39 AM Andrew Lunn <andrew@lunn.ch> wrote: > > On Fri, Aug 16, 2024 at 03:47:33PM -0700, Rosen Penev wrote: > > ``` > > Currently this is handled in userspace with ethtool. Not sure if this > > should be done in the kernel or if this is even the proper place for it. > > ``` > > Comments like this should be placed under the ---. If the patch is > merged, anything in the commit message under the --- is then > discarded. Well, I don't mean for the patch to be merged. I'm mostly trying to get feedback on it. From what I see in the tree, it's not common to disable NETIF_F_SOFT_FEATURES. > > > ag71xx is usually paired with qca8k or ar9331, both DSA drivers. > > Can it be used without a switch? It looks like this option will > disable offloads which are useful when there is no switch. Most of the time it is used with a switch. Even in cases where only one port is available it still goes through the switch (QCA9531 single port device using AR9331 DSA). Some older devices might not go through the switch. I'm not sure. > > Andrew
diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c index b74856760be3..95da34c71b34 100644 --- a/drivers/net/ethernet/atheros/ag71xx.c +++ b/drivers/net/ethernet/atheros/ag71xx.c @@ -1770,6 +1770,15 @@ static int ag71xx_change_mtu(struct net_device *ndev, int new_mtu) return 0; } +static netdev_features_t ag71xx_fix_features(struct net_device *ndev, + netdev_features_t features) +{ + /* remove GRO. Hardware checksumming is needed to avoid a massive + * reduction in switching speed */ + features &= ~NETIF_F_SOFT_FEATURES; + return features; +} + static const struct net_device_ops ag71xx_netdev_ops = { .ndo_open = ag71xx_open, .ndo_stop = ag71xx_stop, @@ -1777,6 +1786,7 @@ static const struct net_device_ops ag71xx_netdev_ops = { .ndo_eth_ioctl = phy_do_ioctl, .ndo_tx_timeout = ag71xx_tx_timeout, .ndo_change_mtu = ag71xx_change_mtu, + .ndo_fix_features = ag71xx_fix_features, .ndo_set_mac_address = eth_mac_addr, .ndo_validate_addr = eth_validate_addr, };
``` Currently this is handled in userspace with ethtool. Not sure if this should be done in the kernel or if this is even the proper place for it. ``` ag71xx is usually paired with qca8k or ar9331, both DSA drivers. DSA internally uses GRO cells to speed up transactions. But this speed up only works if hardware checksumming is supported. Unfortunately for ag71xx, this is unsupported. On newer QCA devices, there is an external HWNAT module that can be used to provide this, but the necessary code has not been written. Mainly because at the time, the proper netfilter code adding the proper APIs was not present. Signed-off-by: Rosen Penev <rosenp@gmail.com> --- drivers/net/ethernet/atheros/ag71xx.c | 10 ++++++++++ 1 file changed, 10 insertions(+)