| Message ID | 1291653582-14177-1-git-send-email-leitao@linux.vnet.ibm.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
On Mon, 6 Dec 2010 14:39:42 -0200 leitao@linux.vnet.ibm.com wrote: > Currently ehea is not setting NETIF_F_LRO, and it is not providing > a callback for get_flags on ethtool. This patch fixes it. > > Signed-off-by: Breno Leitao <leitao@linux.vnet.ibm.com> More importantly, ehea does not support set_flags to disable LRO. I will be more blunt. Any device that supports LRO and does not have the necessary interface to disable it is broken and should not be used.
On 12/06/2010 02:48 PM, Stephen Hemminger wrote: > On Mon, 6 Dec 2010 14:39:42 -0200 > leitao@linux.vnet.ibm.com wrote: > >> Currently ehea is not setting NETIF_F_LRO, and it is not providing >> a callback for get_flags on ethtool. This patch fixes it. >> >> Signed-off-by: Breno Leitao<leitao@linux.vnet.ibm.com> > > More importantly, ehea does not support set_flags to disable LRO. Correct, currently LRO is a module parameter. I have an item in my TODO list to implement set_flags, and thus, the LRO scheme. So, if you prefer I can send this patch with the future set_flags ones. But, for now, this patch allows the user to check when LRO is enabled. As it is today, it shows that LRO is disabled all the time. Anyway, you choose what is the best option. Thanks, Breno -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Breno Leitao <leitao@linux.vnet.ibm.com> Date: Mon, 06 Dec 2010 14:57:26 -0200 > On 12/06/2010 02:48 PM, Stephen Hemminger wrote: >> On Mon, 6 Dec 2010 14:39:42 -0200 >> leitao@linux.vnet.ibm.com wrote: >> >>> Currently ehea is not setting NETIF_F_LRO, and it is not providing >>> a callback for get_flags on ethtool. This patch fixes it. >>> >>> Signed-off-by: Breno Leitao<leitao@linux.vnet.ibm.com> >> >> More importantly, ehea does not support set_flags to disable LRO. > Correct, currently LRO is a module parameter. I have an item in my > TODO list to implement set_flags, and thus, the LRO scheme. > > So, if you prefer I can send this patch with the future set_flags > ones. But, for now, this patch allows the user to check when LRO is > enabled. As it is today, it shows that LRO is disabled all the time. > > Anyway, you choose what is the best option. Your options are: 1) send a set_flags patch now 2) rip LRO support completely out of the ehea driver. The feature is implemented improperly, and as such we have every right to forcefully disable it or remove it until it is fixed to function correctly. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ehea/ehea_ethtool.c b/drivers/net/ehea/ehea_ethtool.c index 75b099c..b0892b0 100644 --- a/drivers/net/ehea/ehea_ethtool.c +++ b/drivers/net/ehea/ehea_ethtool.c @@ -273,6 +273,7 @@ const struct ethtool_ops ehea_ethtool_ops = { .get_ethtool_stats = ehea_get_ethtool_stats, .get_rx_csum = ehea_get_rx_csum, .set_settings = ehea_set_settings, + .get_flags = ethtool_op_get_flags, .nway_reset = ehea_nway_reset, /* Restart autonegotiation */ }; diff --git a/drivers/net/ehea/ehea_main.c b/drivers/net/ehea/ehea_main.c index a84c389..da75846 100644 --- a/drivers/net/ehea/ehea_main.c +++ b/drivers/net/ehea/ehea_main.c @@ -3266,6 +3266,9 @@ struct ehea_port *ehea_setup_single_port(struct ehea_adapter *adapter, | NETIF_F_LLTX; dev->watchdog_timeo = EHEA_WATCH_DOG_TIMEOUT; + if (use_lro) + dev->features |= NETIF_F_LRO; + INIT_WORK(&port->reset_task, ehea_reset_port); ret = register_netdev(dev);
Currently ehea is not setting NETIF_F_LRO, and it is not providing a callback for get_flags on ethtool. This patch fixes it. Signed-off-by: Breno Leitao <leitao@linux.vnet.ibm.com> --- drivers/net/ehea/ehea_ethtool.c | 1 + drivers/net/ehea/ehea_main.c | 3 +++ 2 files changed, 4 insertions(+), 0 deletions(-)