[V4,net-next,4/5] net:hns: Add support of ethtool TSO set option for Hip06 in HNS

Commit Message

Salil Mehta Nov. 20, 2015, 7:49 p.m. UTC

From: Salil <[e-mail]>

This patch adds the support of ethtool TSO option to support
Hip06 SoC to HNS

Signed-off-by: Salil Mehta <[e-mail]>
Signed-off-by: lisheng <[e-mail]>
---

PATCH V4:
This fixes the comments given by Sergei Shtylyov over the PATCH V3:
 Link: https://lkml.org/lkml/2015/11/20/358

PATCH V3/V2:
- No change over the initial patch

PATCH V1:
- Initial version of Ethtool support of TSO by Lisheng
---
 drivers/net/ethernet/hisilicon/hns/hns_enet.c |   47 +++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

Comments

Yuval Mintz Nov. 22, 2015, 11:17 a.m. UTC | #1

> +static netdev_features_t hns_nic_fix_features(
> +		struct net_device *netdev, netdev_features_t features) {
> +	struct hns_nic_priv *priv = netdev_priv(netdev);
> +
> +	switch (priv->enet_ver) {
> +	case AE_VERSION_1:
> +		features &= ~(NETIF_F_TSO | NETIF_F_TSO6 |
> +				NETIF_F_HW_VLAN_CTAG_FILTER);
> +		break;
> +	default:
> +		break;
> +	}
> +	return features;
> +}
> +

Isn't AE_VERSION_1 something fixed once you publish your features?
If it can't be changed, why not simply remove the features from
`hw_features' instead of having to implement this ndo?

Salil Mehta Dec. 1, 2015, 1:56 p.m. UTC | #2

On 22/11/15 11:17, Yuval Mintz wrote:
>> +static netdev_features_t hns_nic_fix_features(
>> +		struct net_device *netdev, netdev_features_t features) {
>> +	struct hns_nic_priv *priv = netdev_priv(netdev);
>> +
>> +	switch (priv->enet_ver) {
>> +	case AE_VERSION_1:
>> +		features &= ~(NETIF_F_TSO | NETIF_F_TSO6 |
>> +				NETIF_F_HW_VLAN_CTAG_FILTER);
>> +		break;
>> +	default:
>> +		break;
>> +	}
>> +	return features;
>> +}
>> +
> Isn't AE_VERSION_1 something fixed once you publish your features?
> If it can't be changed, why not simply remove the features from
> `hw_features' instead of having to implement this ndo?
Hi Yuval,
I some how missed to reply this, though I worked on your suggestions
in the already floated V5 patch earlier. Sorry for this!
There could be a case where the feature is supported by the SoC
and therefore it is already part of the 'hw_features' but it has been
say DISABLED or ENABLED by ethtool. In such a case, we need to
make sure we strike off that feature from the 'features' flag.

Therefore, we need this leg I suppose. Let me know If I am missing
something here or there is a gap in my understanding.

Thanks
Salil

Yuval Mintz Dec. 6, 2015, 6:43 a.m. UTC | #3

> > Isn't AE_VERSION_1 something fixed once you publish your features?
> > If it can't be changed, why not simply remove the features from
> > `hw_features' instead of having to implement this ndo?
> There could be a case where the feature is supported by the SoC
> and therefore it is already part of the 'hw_features' but it has been
> say DISABLED or ENABLED by ethtool. In such a case, we need to
> make sure we strike off that feature from the 'features' flag.
> 
> Therefore, we need this leg I suppose. Let me know If I am missing
> something here or there is a gap in my understanding.

Look at ethtool_set_features() - if something isn't listed in hw_features,
you're not support to be able to change it.

If you can make a one-time decision when registering the netdevice
whether this feature is supported by HW or not [assuming AE_VERSION_1
is available at that point and can't later change] than it's the better alternative.

Cheers,
Yuval

Patch

diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.c b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
index 055e14c..09995d2 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
@@ -1386,6 +1386,51 @@  static int hns_nic_change_mtu(struct net_device *ndev, int new_mtu)
 	return ret;
 }
 
+static int hns_nic_set_features(struct net_device *netdev,
+				netdev_features_t features)
+{
+	struct hns_nic_priv *priv = netdev_priv(netdev);
+	struct hnae_handle *h = priv->ae_handle;
+
+	switch (priv->enet_ver) {
+	case AE_VERSION_1:
+		if (features & (NETIF_F_TSO | NETIF_F_TSO6))
+			netdev_info(netdev, "enet v1 do not support tso!\n");
+		break;
+	default:
+		if (features & (NETIF_F_TSO | NETIF_F_TSO6)) {
+			priv->ops.fill_desc = fill_tso_desc;
+			priv->ops.maybe_stop_tx = hns_nic_maybe_stop_tso;
+			/* The chip only support 7*4096 */
+			netif_set_gso_max_size(netdev, 7 * 4096);
+			h->dev->ops->set_tso_stats(h, 1);
+		} else {
+			priv->ops.fill_desc = fill_v2_desc;
+			priv->ops.maybe_stop_tx = hns_nic_maybe_stop_tx;
+			h->dev->ops->set_tso_stats(h, 0);
+		}
+		break;
+	}
+	netdev->features = features;
+	return 0;
+}
+
+static netdev_features_t hns_nic_fix_features(
+		struct net_device *netdev, netdev_features_t features)
+{
+	struct hns_nic_priv *priv = netdev_priv(netdev);
+
+	switch (priv->enet_ver) {
+	case AE_VERSION_1:
+		features &= ~(NETIF_F_TSO | NETIF_F_TSO6 |
+				NETIF_F_HW_VLAN_CTAG_FILTER);
+		break;
+	default:
+		break;
+	}
+	return features;
+}
+
 /**
  * nic_set_multicast_list - set mutl mac address
  * @netdev: net device
@@ -1481,6 +1526,8 @@  static const struct net_device_ops hns_nic_netdev_ops = {
 	.ndo_set_mac_address = hns_nic_net_set_mac_address,
 	.ndo_change_mtu = hns_nic_change_mtu,
 	.ndo_do_ioctl = hns_nic_do_ioctl,
+	.ndo_set_features = hns_nic_set_features,
+	.ndo_fix_features = hns_nic_fix_features,
 	.ndo_get_stats64 = hns_nic_get_stats64,
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	.ndo_poll_controller = hns_nic_poll_controller,