All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next] ibmvnic: Enable TSO support
@ 2017-05-25  2:29 Thomas Falcon
  2017-05-25  3:57 ` kbuild test robot
  2017-05-25 18:46 ` David Miller
  0 siblings, 2 replies; 5+ messages in thread
From: Thomas Falcon @ 2017-05-25  2:29 UTC (permalink / raw)
  To: netdev; +Cc: Thomas Falcon, Nathan Fontenot, John Allen

Enable TSO in the ibmvnic driver. Scatter-gather is also enabled,
though there currently is no support for scatter-gather in
vNIC-compatible hardware, resulting in forced linearization
of all fragmented SKB's. Though not ideal, given the throughput
improvement gained by enabling TSO, it has been decided
that this is an acceptable tradeoff.

The feature is also enabled by a module parameter.
This parameter is necessary because TSO can not easily be
enabled or disabled in firmware without reinitializing the driver.

CC: Nathan Fontenot <nfont@linux.vnet.ibm.com>
CC: John Allen <jallen@linux.vnet.ibm.com>
Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 39 +++++++++++++++++++++++++++++++++++---
 1 file changed, 36 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index abe2b6e..64e8d50 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -81,6 +81,11 @@
 static const char ibmvnic_driver_name[] = "ibmvnic";
 static const char ibmvnic_driver_string[] = "IBM System i/p Virtual NIC Driver";
 
+static int large_send_offload;
+module_param(large_send_offload, bool, 0644);
+MODULE_PARM_DESC(large_send_offload,
+		 "Determines whether large send offload is enabled");
+
 MODULE_AUTHOR("Santiago Leon <santi_leon@yahoo.com>");
 MODULE_DESCRIPTION("IBM System i/p Virtual NIC Driver");
 MODULE_LICENSE("GPL");
@@ -1025,6 +1030,17 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct net_device *netdev)
 		goto out;
 	}
 
+	/* All scatter-gather SKB's will be linearized for the time being, but
+	 * scatter-gather is only enabled if the user wishes to use TSO.
+	 */
+	if (skb_shinfo(skb)->nr_frags && __skb_linearize(skb)) {
+		dev_kfree_skb_any(skb);
+		tx_send_failed++;
+		tx_dropped++;
+		ret = NETDEV_TX_OK;
+		goto out;
+	}
+
 	tx_pool = &adapter->tx_pool[queue_num];
 	tx_scrq = adapter->tx_scrq[queue_num];
 	txq = netdev_get_tx_queue(netdev, skb_get_queue_mapping(skb));
@@ -1082,6 +1098,11 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct net_device *netdev)
 		tx_crq.v1.flags1 |= IBMVNIC_TX_CHKSUM_OFFLOAD;
 		hdrs += 2;
 	}
+	if (skb_is_gso(skb)) {
+		tx_crq.v1.flags1 |= IBMVNIC_TX_LSO;
+		tx_crq.v1.mss = cpu_to_be16(skb_shinfo(skb)->gso_size);
+		hdrs += 2;
+	}
 	/* determine if l2/3/4 headers are sent to firmware */
 	if ((*hdrs >> 7) & 1 &&
 	    (skb->protocol == htons(ETH_P_IP) ||
@@ -2629,10 +2650,10 @@ static void handle_query_ip_offload_rsp(struct ibmvnic_adapter *adapter)
 	adapter->ip_offload_ctrl.udp_ipv4_chksum = buf->udp_ipv4_chksum;
 	adapter->ip_offload_ctrl.tcp_ipv6_chksum = buf->tcp_ipv6_chksum;
 	adapter->ip_offload_ctrl.udp_ipv6_chksum = buf->udp_ipv6_chksum;
+	adapter->ip_offload_ctrl.large_tx_ipv4 = buf->large_tx_ipv4;
+	adapter->ip_offload_ctrl.large_tx_ipv6 = buf->large_tx_ipv6;
 
-	/* large_tx/rx disabled for now, additional features needed */
-	adapter->ip_offload_ctrl.large_tx_ipv4 = 0;
-	adapter->ip_offload_ctrl.large_tx_ipv6 = 0;
+	/* large_rx disabled for now, additional features needed */
 	adapter->ip_offload_ctrl.large_rx_ipv4 = 0;
 	adapter->ip_offload_ctrl.large_rx_ipv6 = 0;
 
@@ -2648,6 +2669,18 @@ static void handle_query_ip_offload_rsp(struct ibmvnic_adapter *adapter)
 	    (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM)))
 		adapter->netdev->features |= NETIF_F_RXCSUM;
 
+	if (large_send_offload) {
+		/* Scatter-gather is not currently supported by firmware.
+		 * It will only be enabled to support TSO operations.
+		 */
+		adapter->netdev->features = NETIF_F_SG;
+
+		if (buf->large_tx_ipv4)
+			adapter->netdev->features |= NETIF_F_TSO;
+		if (buf->large_tx_ipv6)
+			adapter->netdev->features |= NETIF_F_TSO6;
+	}
+
 	memset(&crq, 0, sizeof(crq));
 	crq.control_ip_offload.first = IBMVNIC_CRQ_CMD;
 	crq.control_ip_offload.cmd = CONTROL_IP_OFFLOAD;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next] ibmvnic: Enable TSO support
  2017-05-25  2:29 [PATCH net-next] ibmvnic: Enable TSO support Thomas Falcon
@ 2017-05-25  3:57 ` kbuild test robot
  2017-05-25 18:46 ` David Miller
  1 sibling, 0 replies; 5+ messages in thread
From: kbuild test robot @ 2017-05-25  3:57 UTC (permalink / raw)
  To: Thomas Falcon
  Cc: kbuild-all, netdev, Thomas Falcon, Nathan Fontenot, John Allen

[-- Attachment #1: Type: text/plain, Size: 2844 bytes --]

Hi Thomas,

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Thomas-Falcon/ibmvnic-Enable-TSO-support/20170525-111039
config: powerpc-allmodconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=powerpc 

All warnings (new ones prefixed by >>):

   In file included from include/linux/module.h:18:0,
                    from drivers/net/ethernet/ibm/ibmvnic.c:46:
   drivers/net/ethernet/ibm/ibmvnic.c: In function '__check_large_send_offload':
   include/linux/moduleparam.h:148:27: error: return from incompatible pointer type [-Werror=incompatible-pointer-types]
     param_check_##type(name, &(value));       \
                              ^
   include/linux/moduleparam.h:346:68: note: in definition of macro '__param_check'
     static inline type __always_unused *__check_##name(void) { return(p); }
                                                                       ^
   include/linux/moduleparam.h:148:2: note: in expansion of macro 'param_check_bool'
     param_check_##type(name, &(value));       \
     ^~~~~~~~~~~~
   include/linux/moduleparam.h:128:2: note: in expansion of macro 'module_param_named'
     module_param_named(name, name, type, perm)
     ^~~~~~~~~~~~~~~~~~
>> drivers/net/ethernet/ibm/ibmvnic.c:85:1: note: in expansion of macro 'module_param'
    module_param(large_send_offload, bool, 0644);
    ^~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/module_param +85 drivers/net/ethernet/ibm/ibmvnic.c

    69	#include <net/net_namespace.h>
    70	#include <asm/hvcall.h>
    71	#include <linux/atomic.h>
    72	#include <asm/vio.h>
    73	#include <asm/iommu.h>
    74	#include <linux/uaccess.h>
    75	#include <asm/firmware.h>
    76	#include <linux/workqueue.h>
    77	#include <linux/if_vlan.h>
    78	
    79	#include "ibmvnic.h"
    80	
    81	static const char ibmvnic_driver_name[] = "ibmvnic";
    82	static const char ibmvnic_driver_string[] = "IBM System i/p Virtual NIC Driver";
    83	
    84	static int large_send_offload;
  > 85	module_param(large_send_offload, bool, 0644);
    86	MODULE_PARM_DESC(large_send_offload,
    87			 "Determines whether large send offload is enabled");
    88	
    89	MODULE_AUTHOR("Santiago Leon <santi_leon@yahoo.com>");
    90	MODULE_DESCRIPTION("IBM System i/p Virtual NIC Driver");
    91	MODULE_LICENSE("GPL");
    92	MODULE_VERSION(IBMVNIC_DRIVER_VERSION);
    93	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 53754 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next] ibmvnic: Enable TSO support
  2017-05-25  2:29 [PATCH net-next] ibmvnic: Enable TSO support Thomas Falcon
  2017-05-25  3:57 ` kbuild test robot
@ 2017-05-25 18:46 ` David Miller
  2017-05-25 18:49   ` David Miller
  1 sibling, 1 reply; 5+ messages in thread
From: David Miller @ 2017-05-25 18:46 UTC (permalink / raw)
  To: tlfalcon; +Cc: netdev, nfont, jallen

From: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
Date: Wed, 24 May 2017 21:29:26 -0500

> The feature is also enabled by a module parameter.
> This parameter is necessary because TSO can not easily be
> enabled or disabled in firmware without reinitializing the driver.

Sorry, this is unacceptable.  When I say no module parameters,
I really really mean it.

Users should not be burdoned with having to know a special knob for
every driver in order to adjust what is a generic feature.

You'll have to find another way to accomodate this.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next] ibmvnic: Enable TSO support
  2017-05-25 18:46 ` David Miller
@ 2017-05-25 18:49   ` David Miller
  2017-05-26 16:11     ` Thomas Falcon
  0 siblings, 1 reply; 5+ messages in thread
From: David Miller @ 2017-05-25 18:49 UTC (permalink / raw)
  To: tlfalcon; +Cc: netdev, nfont, jallen

From: David Miller <davem@davemloft.net>
Date: Thu, 25 May 2017 14:46:26 -0400 (EDT)

> From: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
> Date: Wed, 24 May 2017 21:29:26 -0500
> 
>> The feature is also enabled by a module parameter.
>> This parameter is necessary because TSO can not easily be
>> enabled or disabled in firmware without reinitializing the driver.
> 
> Sorry, this is unacceptable.  When I say no module parameters,
> I really really mean it.
> 
> Users should not be burdoned with having to know a special knob for
> every driver in order to adjust what is a generic feature.
> 
> You'll have to find another way to accomodate this.

Also, TSO helps without SG only because you haven't implemented
support for xmit_more in this driver to decrease the number of
doorball updates and VM enters.

I bet if you added xmit_more support, TSO wouldn't give you much
if any performance boost if you have to linearize.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next] ibmvnic: Enable TSO support
  2017-05-25 18:49   ` David Miller
@ 2017-05-26 16:11     ` Thomas Falcon
  0 siblings, 0 replies; 5+ messages in thread
From: Thomas Falcon @ 2017-05-26 16:11 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, nfont, jallen

On 05/25/2017 01:49 PM, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Thu, 25 May 2017 14:46:26 -0400 (EDT)
>
>> From: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
>> Date: Wed, 24 May 2017 21:29:26 -0500
>>
>>> The feature is also enabled by a module parameter.
>>> This parameter is necessary because TSO can not easily be
>>> enabled or disabled in firmware without reinitializing the driver.
>> Sorry, this is unacceptable.  When I say no module parameters,
>> I really really mean it.
>>
>> Users should not be burdoned with having to know a special knob for
>> every driver in order to adjust what is a generic feature.
>>
>> You'll have to find another way to accomodate this.
> Also, TSO helps without SG only because you haven't implemented
> support for xmit_more in this driver to decrease the number of
> doorball updates and VM enters.
>
> I bet if you added xmit_more support, TSO wouldn't give you much
> if any performance boost if you have to linearize.
>
Thanks for the feedback. I'm looking into providing that support, but for the time being we would like to continue with supporting TSO, without using a module parameter this time.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-05-26 16:11 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-25  2:29 [PATCH net-next] ibmvnic: Enable TSO support Thomas Falcon
2017-05-25  3:57 ` kbuild test robot
2017-05-25 18:46 ` David Miller
2017-05-25 18:49   ` David Miller
2017-05-26 16:11     ` Thomas Falcon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.