[net-next] ibmvnic: Enable TSO support

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(-)

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

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.

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.

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.