[PATCH v6 1/2] drivers: hv: vmbus: Introduce latency testing

2019-10-03 Thread Branden Bonaby
Introduce user specified latency in the packet reception path
By exposing the test parameters as part of the debugfs channel
attributes. We will control the testing state via these attributes.

Signed-off-by: Branden Bonaby 
---
changes in v6:
 - changed kernel version in
   Documentation/ABI/testing/debugfs-hyperv to 5.5

 - removed less than 0 if statement when dealing with
   u64 datatype, as suggested by Michael.

changes in v5:
 - As per Stephen's suggestion, Moved CONFIG_HYPERV_TESTING
   to lib/Kconfig.debug.

 - Fixed build issue reported by Kbuild, with Michael's
   suggestion to make hv_debugfs part of the hv_vmbus
   module.

 - updated debugfs-hyperv to show kernel version 5.4

changes in v4:
 - Combined v3 patch 2 into this patch, and changed the
   commit description to reflect this.

 - Moved debugfs code from "vmbus_drv.c" that was in
   previous v3 patch 2, into a new file "debugfs.c" in
   drivers/hv.

 - Updated the Makefile to compile "debugfs.c" if
   CONFIG_HYPERV_TESTING is enabled

 - As per Michael's comments, added empty implementations
   of the new functions, so the compiler will not generate
   code when CONFIG_HYPERV_TESTING is not enabled.

 - Added microseconds into description for files in
   Documentation/ABI/testing/debugfs-hyperv.

Changes in v2:
 - Add #ifdef in Kconfig file so test code will not interfere
   with non-test code.
 - Move test code functions for delay to hyperv_vmbus header
   file.
 - Wrap test code under #ifdef statement.

 Documentation/ABI/testing/debugfs-hyperv |  23 +++
 MAINTAINERS  |   1 +
 drivers/hv/Makefile  |   1 +
 drivers/hv/connection.c  |   1 +
 drivers/hv/hv_debugfs.c  | 178 +++
 drivers/hv/hyperv_vmbus.h|  31 
 drivers/hv/ring_buffer.c |   2 +
 drivers/hv/vmbus_drv.c   |   6 +
 include/linux/hyperv.h   |  19 +++
 lib/Kconfig.debug|   7 +
 10 files changed, 269 insertions(+)
 create mode 100644 Documentation/ABI/testing/debugfs-hyperv
 create mode 100644 drivers/hv/hv_debugfs.c

diff --git a/Documentation/ABI/testing/debugfs-hyperv 
b/Documentation/ABI/testing/debugfs-hyperv
new file mode 100644
index ..9185e1b06bba
--- /dev/null
+++ b/Documentation/ABI/testing/debugfs-hyperv
@@ -0,0 +1,23 @@
+What:   /sys/kernel/debug/hyperv//fuzz_test_state
+Date:   October 2019
+KernelVersion:  5.5
+Contact:Branden Bonaby 
+Description:Fuzz testing status of a vmbus device, whether its in an ON
+state or a OFF state
+Users:  Debugging tools
+
+What:   
/sys/kernel/debug/hyperv//delay/fuzz_test_buffer_interrupt_delay
+Date:   October 2019
+KernelVersion:  5.5
+Contact:Branden Bonaby 
+Description:Fuzz testing buffer interrupt delay value between 0 - 1000
+microseconds (inclusive).
+Users:  Debugging tools
+
+What:   /sys/kernel/debug/hyperv//delay/fuzz_test_message_delay
+Date:   October 2019
+KernelVersion:  5.5
+Contact:Branden Bonaby 
+Description:Fuzz testing message delay value between 0 - 1000 microseconds
+(inclusive).
+Users:  Debugging tools
diff --git a/MAINTAINERS b/MAINTAINERS
index 55199ef7fa74..9801b1924213 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7581,6 +7581,7 @@ F:include/uapi/linux/hyperv.h
 F: include/asm-generic/mshyperv.h
 F: tools/hv/
 F: Documentation/ABI/stable/sysfs-bus-vmbus
+F: Documentation/ABI/testing/debugfs-hyperv
 
 HYPERBUS SUPPORT
 M: Vignesh Raghavendra 
diff --git a/drivers/hv/Makefile b/drivers/hv/Makefile
index a1eec7177c2d..94daf8240c95 100644
--- a/drivers/hv/Makefile
+++ b/drivers/hv/Makefile
@@ -9,4 +9,5 @@ CFLAGS_hv_balloon.o = -I$(src)
 hv_vmbus-y := vmbus_drv.o \
 hv.o connection.o channel.o \
 channel_mgmt.o ring_buffer.o hv_trace.o
+hv_vmbus-$(CONFIG_HYPERV_TESTING)  += hv_debugfs.o
 hv_utils-y := hv_util.o hv_kvp.o hv_snapshot.o hv_fcopy.o hv_utils_transport.o
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 6e4c015783ff..7001b1ab4cdd 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -361,6 +361,7 @@ void vmbus_on_event(unsigned long data)
 
trace_vmbus_on_event(channel);
 
+   hv_debug_delay_test(channel, INTERRUPT_DELAY);
do {
void (*callback_fn)(void *);
 
diff --git a/drivers/hv/hv_debugfs.c b/drivers/hv/hv_debugfs.c
new file mode 100644
index ..8a2878573582
--- /dev/null
+++ b/drivers/hv/hv_debugfs.c
@@ -0,0 +1,178 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Authors:
+ *   Branden Bonaby 
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include "hyperv_vmbus.h"
+
+struct dentry *hv_debug_root;
+
+static int hv_debugfs_delay_get(void 

[PATCH v6 2/2] tools: hv: add vmbus testing tool

2019-10-03 Thread Branden Bonaby
This is a userspace tool to drive the testing. Currently it supports
introducing user specified delay in the host to guest communication
path on a per-channel basis.

Signed-off-by: Branden Bonaby 
---
Changes in v4:
- Based on Harrys comments, made the tool more
  user friendly and added more error checking.

Changes in v3:
- Align python tool to match Linux coding style.

Changes in v2:
 - Move testing location to new location in debugfs.

 tools/hv/vmbus_testing | 376 +
 1 file changed, 376 insertions(+)
 create mode 100755 tools/hv/vmbus_testing

diff --git a/tools/hv/vmbus_testing b/tools/hv/vmbus_testing
new file mode 100755
index ..e7212903dd1d
--- /dev/null
+++ b/tools/hv/vmbus_testing
@@ -0,0 +1,376 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+#
+# Program to allow users to fuzz test Hyper-V drivers
+# by interfacing with Hyper-V debugfs attributes.
+# Current test methods available:
+#   1. delay testing
+#
+# Current file/directory structure of hyper-V debugfs:
+#   /sys/kernel/debug/hyperv/UUID
+#   /sys/kernel/debug/hyperv/UUID/
+#   /sys/kernel/debug/hyperv/UUID/
+#
+# author: Branden Bonaby 
+
+import os
+import cmd
+import argparse
+import glob
+from argparse import RawDescriptionHelpFormatter
+from argparse import RawTextHelpFormatter
+from enum import Enum
+
+# Do not change unless, you change the debugfs attributes
+# in /drivers/hv/debugfs.c. All fuzz testing
+# attributes will start with "fuzz_test".
+
+# debugfs path for hyperv must exist before proceeding
+debugfs_hyperv_path = "/sys/kernel/debug/hyperv"
+if not os.path.isdir(debugfs_hyperv_path):
+print("{} doesn't exist/check permissions".format(debugfs_hyperv_path))
+exit(-1)
+
+class dev_state(Enum):
+off = 0
+on = 1
+
+# File names, that correspond to the files created in
+# /drivers/hv/debugfs.c
+class f_names(Enum):
+state_f = "fuzz_test_state"
+buff_f =  "fuzz_test_buffer_interrupt_delay"
+mess_f =  "fuzz_test_message_delay"
+
+# Both single_actions and all_actions are used
+# for error checking and to allow for some subparser
+# names to be abbreviated. Do not abbreviate the
+# test method names, as it will become less intuitive
+# as to what the user can do. If you do decide to
+# abbreviate the test method name, make sure the main
+# function reflects this change.
+
+all_actions = [
+"disable_all",
+"D",
+"enable_all",
+"view_all",
+"V"
+]
+
+single_actions = [
+"disable_single",
+"d",
+"enable_single",
+"view_single",
+"v"
+]
+
+def main():
+
+file_map = recursive_file_lookup(debugfs_hyperv_path, dict())
+args = parse_args()
+if (not args.action):
+print ("Error, no options selected...exiting")
+exit(-1)
+arg_set = { k for (k,v) in vars(args).items() if v and k != "action" }
+arg_set.add(args.action)
+path = args.path if "path" in arg_set else None
+if (path and path[-1] == "/"):
+path = path[:-1]
+validate_args_path(path, arg_set, file_map)
+if (path and "enable_single" in arg_set):
+state_path = locate_state(path, file_map)
+set_test_state(state_path, dev_state.on.value, args.quiet)
+
+# Use subparsers as the key for different actions
+if ("delay" in arg_set):
+validate_delay_values(args.delay_time)
+if (args.enable_all):
+set_delay_all_devices(file_map, args.delay_time,
+  args.quiet)
+else:
+set_delay_values(path, file_map, args.delay_time,
+ args.quiet)
+elif ("disable_all" in arg_set or "D" in arg_set):
+disable_all_testing(file_map)
+elif ("disable_single" in arg_set or "d" in arg_set):
+disable_testing_single_device(path, file_map)
+elif ("view_all" in arg_set or "V" in arg_set):
+get_all_devices_test_status(file_map)
+elif ("view_single" in arg_set or  "v" in arg_set):
+get_device_test_values(path, file_map)
+
+# Get the state location
+def locate_state(device, file_map):
+return file_map[device][f_names.state_f.value]
+
+# Validate delay values to make sure they are acceptable to
+# enable delays on a device
+def validate_delay_values(delay):
+
+if (delay[0]  == -1 and delay[1] == -1):
+print("\nError, At least 1 value must be gre

[PATCH v6 0/2] hv: vmbus: add fuzz testing to hv device

2019-10-03 Thread Branden Bonaby
This patchset introduces a testing framework for Hyper-V drivers.
This framework allows us to introduce delays in the packet receive
path on a per-device basis. While the current code only supports
introducing arbitrary delays in the host/guest communication path,
we intend to expand this to support error injection in the future.

changes in v6:
  patch 1:
changed kernel version in 
Documentation/ABI/testing/debugfs-hyperv to 5.5

removed less than 0 if statement when dealing with
u64 datatype, as suggested by michael.

changes in v5:
  patch 1:
As per Stephen's suggestion, Moved CONFIG_HYPERV_TESTING
to lib/Kconfig.debug.

Fixed build issue reported by Kbuild, with Michael's
suggestion to make hv_debugfs part of the hv_vmbus
module.

changes in v4:
  patch 1:
Combined previous v3 patches 1 and 2, into a single patch
which is now patch 1. This was done so that calls to
the new debugfs functions are in the same patch as
the definitions for these functions.

Moved debugfs code from "vmbus_drv.c" that was in
previous v3 patch 2, into a new file "debugfs.c" in
drivers/hv.

Updated the Makefile to compile "debugfs.c" if
CONFIG_HYPERV_TESTING is enabled

As per Michael's comments, added empty implementations
of the new functions, so the compiler will not generate
code when CONFIG_HYPERV_TESTING is not enabled.

  patch 2 (was previously v3 patch 3):
Based on Harrys comments, made the tool more
user friendly and added more error checking.

changes in v3:
  patch 2: change call to IS_ERR_OR_NULL, to IS_ERR.

  patch 3: Align python tool to match Linux coding style.

Changes in v2:
  Patch 1: As per Vitaly's suggestion, wrapped the test code under an
   #ifdef and updated the Kconfig file, so that the test code
   will only be used when the config option is set to true.
   (default is false).

   Updated hyperv_vmbus header to contain new #ifdef with new
   new functions for the test code.

  Patch 2: Moved code from under sysfs to debugfs and wrapped it under
   the new ifdef.

   Updated MAINTAINERS file with new debugfs-hyperv file under
   the section for hyperv.

  Patch 3: Updated testing tool with new debugfs location.

Branden Bonaby (2):
  drivers: hv: vmbus: Introduce latency testing
  tools: hv: add vmbus testing tool

 Documentation/ABI/testing/debugfs-hyperv |  23 ++
 MAINTAINERS  |   1 +
 drivers/hv/Makefile  |   1 +
 drivers/hv/connection.c  |   1 +
 drivers/hv/hv_debugfs.c  | 178 +++
 drivers/hv/hyperv_vmbus.h|  31 ++
 drivers/hv/ring_buffer.c |   2 +
 drivers/hv/vmbus_drv.c   |   6 +
 include/linux/hyperv.h   |  19 ++
 lib/Kconfig.debug|   7 +
 tools/hv/vmbus_testing   | 376 +++
 11 files changed, 645 insertions(+)
 create mode 100644 Documentation/ABI/testing/debugfs-hyperv
 create mode 100644 drivers/hv/hv_debugfs.c
 create mode 100755 tools/hv/vmbus_testing

-- 
2.17.1



Re: [PATCH v5 1/2] drivers: hv: vmbus: Introduce latency testing

2019-10-03 Thread Branden Bonaby
On Thu, Sep 19, 2019 at 10:52:41PM +, Michael Kelley wrote:
> From: Branden Bonaby  Sent: Thursday, September 
> 12, 2019 7:32 PM
> > 
> > +
> > +static int hv_debugfs_delay_set(void *data, u64 val)
> > +{
> > +   int ret = 0;
> > +
> > +   if (val >= 0 && val <= 1000)
> > +   *(u32 *)data = val;
> > +   else
> > +   ret = -EINVAL;
> > +
> > +   return ret;
> > +}
> 
> I should probably quit picking at your code, but I'm going to
> do it one more time. :-)
> 
> The above test for val >=0 is redundant as 'val' is declared
> as 'u64'.  As an unsigned value, it will always be >= 0.  More
> broadly, the above function could be written as follows
> with no loss of clarity.  This accomplishes the same thing in
> only 4 lines of code instead of 6, and the main execution path
> is in the sequential execution flow, not in an 'if' statement.
> 
> {
>   if (val > 1000)
>   return -EINVAL;
>   *(u32 *)data = val;
>   return 0;
> }
> 
> Your code is correct as written, so this is arguably more a
> matter of style, but Linux generally likes to do things clearly
> and compactly with no extra motion.
> 

Yea the less than 0 comparison isnt needed, so I'll update that

> +/* Delay buffer/message reads on a vmbus channel */
> > +void hv_debug_delay_test(struct vmbus_channel *channel, enum delay 
> > delay_type)
> > +{
> > +   struct vmbus_channel *test_channel =channel->primary_channel ?
> > +   channel->primary_channel :
> > +   channel;
> > +   bool state = test_channel->fuzz_testing_state;
> > +
> > +   if (state) {
> > +   if (delay_type == 0)
> > +   udelay(test_channel->fuzz_testing_interrupt_delay);
> > +   else
> > +   udelay(test_channel->fuzz_testing_message_delay);
> 
> This 'if/else' statement got me thinking.  You have an enum declared below
> that lists the two options -- INTERRUPT_DELAY or MESSAGE_DELAY.  The
> implication is that we might add more options in the future.  But the
> above 'if/else' statement isn't really set up to easily add more options, and
> the individual fields for fuzz_testing_interrupt_delay and
> fuzz_testing_message_delay mean adding more branches to the 'if/else'
> statement whenever a new DELAY type is added to the enum.   And the
> same is true when adding the entries into debugfs.  A more general
> solution might use arrays and loops, and treat the enum value as an
> index into an array of delay values.  Extending to add another delay type
> could be as easy as adding another entry to the enum declaration.
> 
> The current code is for the case where n=2 (i.e., two different delay
> types), and as such probably doesn't warrant the full index/looping
> treatment.  But in the future, if we add additional delay types, we'll
> probably revise the code to do the index/looping approach.
> 
> So to be clear, at this point I'm not asking you to change the existing
> code.  My comments are more of an observation and something to
> think about in the future.
> 

I do see your point, thanks for the input. I think since its just two
it might be better to leave it but it definitely makes sense.

> > 
> > +enum delay {
> > +   INTERRUPT_DELAY = 0,
> > +   MESSAGE_DELAY   = 1,
> > +};
> > +
> 
> Michael


[PATCH v5 1/2] drivers: hv: vmbus: Introduce latency testing

2019-09-12 Thread Branden Bonaby
Introduce user specified latency in the packet reception path
By exposing the test parameters as part of the debugfs channel
attributes. We will control the testing state via these attributes.

Signed-off-by: Branden Bonaby 
---
changes in v5:
 - As per Stephen's suggestion, Moved CONFIG_HYPERV_TESTING
   to lib/Kconfig.debug.

 - Fixed build issue reported by Kbuild, with Michael's
   suggestion to make hv_debugfs part of the hv_vmbus
   module.

 - updated debugfs-hyperv to show kernel version 5.4

changes in v4:
 - Combined v3 patch 2 into this patch, and changed the
   commit description to reflect this.

 - Moved debugfs code from "vmbus_drv.c" that was in
   previous v3 patch 2, into a new file "debugfs.c" in
   drivers/hv.

 - Updated the Makefile to compile "debugfs.c" if
   CONFIG_HYPERV_TESTING is enabled

 - As per Michael's comments, added empty implementations
   of the new functions, so the compiler will not generate
   code when CONFIG_HYPERV_TESTING is not enabled.

 - Added microseconds into description for files in
   Documentation/ABI/testing/debugfs-hyperv.

Changes in v2:
 - Add #ifdef in Kconfig file so test code will not interfere
   with non-test code.
 - Move test code functions for delay to hyperv_vmbus header
   file.
 - Wrap test code under #ifdef statement.
 
Documentation/ABI/testing/debugfs-hyperv |  23 +++
 MAINTAINERS  |   1 +
 drivers/hv/Makefile  |   1 +
 drivers/hv/connection.c  |   1 +
 drivers/hv/hv_debugfs.c  | 185 +++
 drivers/hv/hyperv_vmbus.h|  31 
 drivers/hv/ring_buffer.c |   2 +
 drivers/hv/vmbus_drv.c   |   6 +
 include/linux/hyperv.h   |  19 +++
 lib/Kconfig.debug|   7 +
 10 files changed, 276 insertions(+)
 create mode 100644 Documentation/ABI/testing/debugfs-hyperv
 create mode 100644 drivers/hv/hv_debugfs.c

diff --git a/Documentation/ABI/testing/debugfs-hyperv 
b/Documentation/ABI/testing/debugfs-hyperv
new file mode 100644
index ..4427503ec762
--- /dev/null
+++ b/Documentation/ABI/testing/debugfs-hyperv
@@ -0,0 +1,23 @@
+What:   /sys/kernel/debug/hyperv//fuzz_test_state
+Date:   August 2019
+KernelVersion:  5.4
+Contact:Branden Bonaby 
+Description:Fuzz testing status of a vmbus device, whether its in an ON
+state or a OFF state
+Users:  Debugging tools
+
+What:   
/sys/kernel/debug/hyperv//delay/fuzz_test_buffer_interrupt_delay
+Date:   August 2019
+KernelVersion:  5.4
+Contact:Branden Bonaby 
+Description:Fuzz testing buffer interrupt delay value between 0 - 1000
+microseconds (inclusive).
+Users:  Debugging tools
+
+What:   /sys/kernel/debug/hyperv//delay/fuzz_test_message_delay
+Date:   August 2019
+KernelVersion:  5.4
+Contact:Branden Bonaby 
+Description:Fuzz testing message delay value between 0 - 1000 microseconds
+(inclusive).
+Users:  Debugging tools
diff --git a/MAINTAINERS b/MAINTAINERS
index e7a47b5210fd..00831931eb22 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7468,6 +7468,7 @@ F:include/uapi/linux/hyperv.h
 F: include/asm-generic/mshyperv.h
 F: tools/hv/
 F: Documentation/ABI/stable/sysfs-bus-vmbus
+F: Documentation/ABI/testing/debugfs-hyperv
 
 HYPERBUS SUPPORT
 M: Vignesh Raghavendra 
diff --git a/drivers/hv/Makefile b/drivers/hv/Makefile
index a1eec7177c2d..94daf8240c95 100644
--- a/drivers/hv/Makefile
+++ b/drivers/hv/Makefile
@@ -9,4 +9,5 @@ CFLAGS_hv_balloon.o = -I$(src)
 hv_vmbus-y := vmbus_drv.o \
 hv.o connection.o channel.o \
 channel_mgmt.o ring_buffer.o hv_trace.o
+hv_vmbus-$(CONFIG_HYPERV_TESTING)  += hv_debugfs.o
 hv_utils-y := hv_util.o hv_kvp.o hv_snapshot.o hv_fcopy.o hv_utils_transport.o
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 09829e15d4a0..4d4d40832846 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -357,6 +357,7 @@ void vmbus_on_event(unsigned long data)
 
trace_vmbus_on_event(channel);
 
+   hv_debug_delay_test(channel, INTERRUPT_DELAY);
do {
void (*callback_fn)(void *);
 
diff --git a/drivers/hv/hv_debugfs.c b/drivers/hv/hv_debugfs.c
new file mode 100644
index ..933080b51410
--- /dev/null
+++ b/drivers/hv/hv_debugfs.c
@@ -0,0 +1,185 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Authors:
+ *   Branden Bonaby 
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include "hyperv_vmbus.h"
+
+struct dentry *hv_debug_root;
+
+static int hv_debugfs_delay_get(void *data, u64 *val)
+{
+   *val = *(u32 *)data;
+   return 0;
+}
+
+static int hv_debugfs_delay_set(void *data, u64 val)
+{
+   int ret = 0;
+
+   if (val >= 0 && val <= 

[PATCH v5 2/2] tools: hv: add vmbus testing tool

2019-09-12 Thread Branden Bonaby
This is a userspace tool to drive the testing. Currently it supports
introducing user specified delay in the host to guest communication
path on a per-channel basis.

Signed-off-by: Branden Bonaby 
---
Changes in v4:
- Based on Harrys comments, made the tool more
  user friendly and added more error checking.

Changes in v3:
- Align python tool to match Linux coding style.

Changes in v2:
 - Move testing location to new location in debugfs.

 tools/hv/vmbus_testing | 376 +
 1 file changed, 376 insertions(+)
 create mode 100644 tools/hv/vmbus_testing

diff --git a/tools/hv/vmbus_testing b/tools/hv/vmbus_testing
new file mode 100644
index ..e7212903dd1d
--- /dev/null
+++ b/tools/hv/vmbus_testing
@@ -0,0 +1,376 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+#
+# Program to allow users to fuzz test Hyper-V drivers
+# by interfacing with Hyper-V debugfs attributes.
+# Current test methods available:
+#   1. delay testing
+#
+# Current file/directory structure of hyper-V debugfs:
+#   /sys/kernel/debug/hyperv/UUID
+#   /sys/kernel/debug/hyperv/UUID/
+#   /sys/kernel/debug/hyperv/UUID/
+#
+# author: Branden Bonaby 
+
+import os
+import cmd
+import argparse
+import glob
+from argparse import RawDescriptionHelpFormatter
+from argparse import RawTextHelpFormatter
+from enum import Enum
+
+# Do not change unless, you change the debugfs attributes
+# in /drivers/hv/debugfs.c. All fuzz testing
+# attributes will start with "fuzz_test".
+
+# debugfs path for hyperv must exist before proceeding
+debugfs_hyperv_path = "/sys/kernel/debug/hyperv"
+if not os.path.isdir(debugfs_hyperv_path):
+print("{} doesn't exist/check permissions".format(debugfs_hyperv_path))
+exit(-1)
+
+class dev_state(Enum):
+off = 0
+on = 1
+
+# File names, that correspond to the files created in
+# /drivers/hv/debugfs.c
+class f_names(Enum):
+state_f = "fuzz_test_state"
+buff_f =  "fuzz_test_buffer_interrupt_delay"
+mess_f =  "fuzz_test_message_delay"
+
+# Both single_actions and all_actions are used
+# for error checking and to allow for some subparser
+# names to be abbreviated. Do not abbreviate the
+# test method names, as it will become less intuitive
+# as to what the user can do. If you do decide to
+# abbreviate the test method name, make sure the main
+# function reflects this change.
+
+all_actions = [
+"disable_all",
+"D",
+"enable_all",
+"view_all",
+"V"
+]
+
+single_actions = [
+"disable_single",
+"d",
+"enable_single",
+"view_single",
+"v"
+]
+
+def main():
+
+file_map = recursive_file_lookup(debugfs_hyperv_path, dict())
+args = parse_args()
+if (not args.action):
+print ("Error, no options selected...exiting")
+exit(-1)
+arg_set = { k for (k,v) in vars(args).items() if v and k != "action" }
+arg_set.add(args.action)
+path = args.path if "path" in arg_set else None
+if (path and path[-1] == "/"):
+path = path[:-1]
+validate_args_path(path, arg_set, file_map)
+if (path and "enable_single" in arg_set):
+state_path = locate_state(path, file_map)
+set_test_state(state_path, dev_state.on.value, args.quiet)
+
+# Use subparsers as the key for different actions
+if ("delay" in arg_set):
+validate_delay_values(args.delay_time)
+if (args.enable_all):
+set_delay_all_devices(file_map, args.delay_time,
+  args.quiet)
+else:
+set_delay_values(path, file_map, args.delay_time,
+ args.quiet)
+elif ("disable_all" in arg_set or "D" in arg_set):
+disable_all_testing(file_map)
+elif ("disable_single" in arg_set or "d" in arg_set):
+disable_testing_single_device(path, file_map)
+elif ("view_all" in arg_set or "V" in arg_set):
+get_all_devices_test_status(file_map)
+elif ("view_single" in arg_set or  "v" in arg_set):
+get_device_test_values(path, file_map)
+
+# Get the state location
+def locate_state(device, file_map):
+return file_map[device][f_names.state_f.value]
+
+# Validate delay values to make sure they are acceptable to
+# enable delays on a device
+def validate_delay_values(delay):
+
+if (delay[0]  == -1 and delay[1] == -1):
+print("\nError, At least 1 value must be gre

[PATCH v5 0/2] hv: vmbus: add fuzz testing to hv device

2019-09-12 Thread Branden Bonaby
This patchset introduces a testing framework for Hyper-V drivers.
This framework allows us to introduce delays in the packet receive
path on a per-device basis. While the current code only supports
introducing arbitrary delays in the host/guest communication path,
we intend to expand this to support error injection in the future.

changes in v5:
  patch 1:
As per Stephen's suggestion, Moved CONFIG_HYPERV_TESTING
to lib/Kconfig.debug.

Fixed build issue reported by Kbuild, with Michael's
suggestion to make hv_debugfs part of the hv_vmbus
module.

changes in v4:
  patch 1:
Combined previous v3 patches 1 and 2, into a single patch
which is now patch 1. This was done so that calls to
the new debugfs functions are in the same patch as
the definitions for these functions.

Moved debugfs code from "vmbus_drv.c" that was in
previous v3 patch 2, into a new file "debugfs.c" in
drivers/hv.

Updated the Makefile to compile "debugfs.c" if
CONFIG_HYPERV_TESTING is enabled

As per Michael's comments, added empty implementations
of the new functions, so the compiler will not generate
code when CONFIG_HYPERV_TESTING is not enabled.

  patch 2 (was previously v3 patch 3):
Based on Harrys comments, made the tool more
user friendly and added more error checking.

changes in v3:
  patch 2: change call to IS_ERR_OR_NULL, to IS_ERR.

  patch 3: Align python tool to match Linux coding style.

Changes in v2:
  Patch 1: As per Vitaly's suggestion, wrapped the test code under an
   #ifdef and updated the Kconfig file, so that the test code
   will only be used when the config option is set to true.
   (default is false).

   Updated hyperv_vmbus header to contain new #ifdef with new
   new functions for the test code.

  Patch 2: Moved code from under sysfs to debugfs and wrapped it under
   the new ifdef.

   Updated MAINTAINERS file with new debugfs-hyperv file under
   the section for hyperv.

  Patch 3: Updated testing tool with new debugfs location.

Branden Bonaby (2):
  drivers: hv: vmbus: Introduce latency testing
  tools: hv: add vmbus testing tool

 Documentation/ABI/testing/debugfs-hyperv |  23 ++
 MAINTAINERS  |   1 +
 drivers/hv/Makefile  |   1 +
 drivers/hv/connection.c  |   1 +
 drivers/hv/hv_debugfs.c  | 185 +++
 drivers/hv/hyperv_vmbus.h|  31 ++
 drivers/hv/ring_buffer.c |   2 +
 drivers/hv/vmbus_drv.c   |   6 +
 include/linux/hyperv.h   |  19 ++
 lib/Kconfig.debug|   7 +
 tools/hv/vmbus_testing   | 376 +++
 11 files changed, 652 insertions(+)
 create mode 100644 Documentation/ABI/testing/debugfs-hyperv
 create mode 100644 drivers/hv/hv_debugfs.c
 create mode 100644 tools/hv/vmbus_testing

-- 
2.17.1



Re: [PATCH v4 1/2] drivers: hv: vmbus: Introduce latency testing

2019-09-05 Thread Branden Bonaby
On Mon, Sep 02, 2019 at 06:31:04PM +, Michael Kelley wrote:
> From: Branden Bonaby  Sent: Wednesday, August 28, 
> 2019 9:24 PM
> > 
> > Introduce user specified latency in the packet reception path
> > By exposing the test parameters as part of the debugfs channel
> > attributes. We will control the testing state via these attributes.
> > 
> > Signed-off-by: Branden Bonaby 
> > ---
> > diff --git a/Documentation/ABI/testing/debugfs-hyperv
> > b/Documentation/ABI/testing/debugfs-hyperv
> > new file mode 100644
> > index ..0903e6427a2d
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/debugfs-hyperv
> > @@ -0,0 +1,23 @@
> > +What:   /sys/kernel/debug/hyperv//fuzz_test_state
> > +Date:   August 2019
> > +KernelVersion:  5.3
> 
> Given the way the timing works for adding code to the Linux kernel,
> the earliest your new code will be included is 5.4.  The merge window
> for 5.4 will open in two weeks, so your code would need to be accepted
> by then to be included in 5.4.  Otherwise, it won't make it until the next
> merge window, which would be for 5.5.  I would suggest changing this
> (and the others below) to 5.4 for now, but you might have to change to
> 5.5 if additional revisions are needed.
> 

I see, I'll keep this in mind when submitting the further revisions
thanks

> > diff --git a/drivers/hv/Makefile b/drivers/hv/Makefile
> > index a1eec7177c2d..d754bd86ca47 100644
> > --- a/drivers/hv/Makefile
> > +++ b/drivers/hv/Makefile
> > @@ -2,6 +2,7 @@
> >  obj-$(CONFIG_HYPERV)   += hv_vmbus.o
> >  obj-$(CONFIG_HYPERV_UTILS) += hv_utils.o
> >  obj-$(CONFIG_HYPERV_BALLOON)   += hv_balloon.o
> > +obj-$(CONFIG_HYPERV_TESTING)   += hv_debugfs.o
> 
> There's an additional complexity here that I failed to describe to
> you in my previous comments.  :-(The above line makes the
> hv_debugfs code part of the main Linux OS image -- i.e., the
> vmlinuz file that gets installed into /boot, such that if hv_debugfs
> is built, it is always loaded into the memory of the running system.
> That's OK, but we should consider the alternative, which is to
> make the hv_debugfs code part of the hv_vmbus module that
> is specified a bit further down in the same Makefile.   A module
> is installed into /lib/modules//kernel, and
> is only loaded into memory on the running system if something
> actually references code in the module.  This approach helps avoid
> loading code into memory that isn't actually used.
> 
> Whether code is built as a separate module or is just built into
> the main vmlinuz file is also controlled by the .config file setting.
> For example, CONFIG_HYPERV=m says to treat hv_vmbus as a
> separate module, while CONFIG_HYPERV=y says to build the
> hv_vmbus module directly into the vmlinuz file even though the
> Makefile constructs it as a module.
> 
> Making hv_debugfs part of the hv_vmbus module is generally better
> than just building it into the main vmlinuz because it's best to include
> only things that must always be present in the main vmlinuz file.
> hv_debugfs doesn't have any reason it needs to always be present.
> By comparison, hv_balloon is included in the main vmlinuz, which
> might be due to it dealing with memory mgmt and needing to be
> in the vmlinuz image, but I'm not sure.
> 
> You can make hv_debugfs part of the hv_vmbus module with this line
> placed after the line specifying hv_vmbus_y, instead of the line you
> currently have:
> 
> hv_vmbus-$(CONFIG_HYPERV_TESTING)   += hv_debugfs.o
> 

Ah, I see now. That makes sense I'll go ahead and make the adjustments

thanks

> > +
> > +static int hv_debugfs_delay_set(void *data, u64 val)
> > +{
> > +   int ret = 0;
> > +
> > +   if (val >= 1 && val <= 1000)
> > +   *(u32 *)data = val;
> > +   else if (val == 0)
> > +   *(u32 *)data = 0;
> 
> I think this "else if" clause can be folded into the first
> "if" statement by just checking "val >= 0".
> 

good call, I'll fold it into one statement 

> 
> > +/* Remove dentry associated with released hv device */
> > +void hv_debug_rm_dev_dir(struct hv_device *dev)
> > +{
> > +   if (!IS_ERR(hv_debug_root))
> > +   debugfs_remove_recursive(dev->debug_dir);
> > +}
> 
> This function is the first of five functions that are called by
> code outside of hv_debugfs.c.   You probably saw the separate
> email from the kbuild test robot showing a build error on each
> of these five functions.  Here's why.
> 
> When CONFIG_HYPERV=m is set, and hv_vmbus is built a

[PATCH v4 1/2] drivers: hv: vmbus: Introduce latency testing

2019-08-28 Thread Branden Bonaby
Introduce user specified latency in the packet reception path
By exposing the test parameters as part of the debugfs channel
attributes. We will control the testing state via these attributes.

Signed-off-by: Branden Bonaby 
---
changes in v4:
 - Combined v3 patch 2 into this patch, and changed the
   commit description to reflect this.

 - Moved debugfs code from "vmbus_drv.c" that was in
   previous v3 patch 2, into a new file "debugfs.c" in
   drivers/hv.

 - Updated the Makefile to compile "debugfs.c" if
   CONFIG_HYPERV_TESTING is enabled

 - As per Michael's comments, added empty implementations
   of the new functions, so the compiler will not generate
   code when CONFIG_HYPERV_TESTING is not enabled.

 - Added microseconds into description for files in
   Documentation/ABI/testing/debugfs-hyperv.

Changes in v2:
 - Add #ifdef in Kconfig file so test code will not interfere
   with non-test code.
 - Move test code functions for delay to hyperv_vmbus header
   file.
 - Wrap test code under #ifdef statement.

 Documentation/ABI/testing/debugfs-hyperv |  23 +++
 MAINTAINERS  |   1 +
 drivers/hv/Kconfig   |   7 +
 drivers/hv/Makefile  |   1 +
 drivers/hv/connection.c  |   1 +
 drivers/hv/hv_debugfs.c  | 187 +++
 drivers/hv/hyperv_vmbus.h|  31 
 drivers/hv/ring_buffer.c |   2 +
 drivers/hv/vmbus_drv.c   |   6 +
 include/linux/hyperv.h   |  19 +++
 10 files changed, 278 insertions(+)
 create mode 100644 Documentation/ABI/testing/debugfs-hyperv
 create mode 100644 drivers/hv/hv_debugfs.c

diff --git a/Documentation/ABI/testing/debugfs-hyperv 
b/Documentation/ABI/testing/debugfs-hyperv
new file mode 100644
index ..0903e6427a2d
--- /dev/null
+++ b/Documentation/ABI/testing/debugfs-hyperv
@@ -0,0 +1,23 @@
+What:   /sys/kernel/debug/hyperv//fuzz_test_state
+Date:   August 2019
+KernelVersion:  5.3
+Contact:Branden Bonaby 
+Description:Fuzz testing status of a vmbus device, whether its in an ON
+state or a OFF state
+Users:  Debugging tools
+
+What:   
/sys/kernel/debug/hyperv//delay/fuzz_test_buffer_interrupt_delay
+Date:   August 2019
+KernelVersion:  5.3
+Contact:Branden Bonaby 
+Description:Fuzz testing buffer interrupt delay value between 0 - 1000
+microseconds (inclusive).
+Users:  Debugging tools
+
+What:   /sys/kernel/debug/hyperv//delay/fuzz_test_message_delay
+Date:   August 2019
+KernelVersion:  5.3
+Contact:Branden Bonaby 
+Description:Fuzz testing message delay value between 0 - 1000 microseconds
+(inclusive).
+Users:  Debugging tools
diff --git a/MAINTAINERS b/MAINTAINERS
index b1bc9c87838b..6931a4eeaac0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7460,6 +7460,7 @@ F:include/uapi/linux/hyperv.h
 F: include/asm-generic/mshyperv.h
 F: tools/hv/
 F: Documentation/ABI/stable/sysfs-bus-vmbus
+F: Documentation/ABI/testing/debugfs-hyperv
 
 HYPERBUS SUPPORT
 M: Vignesh Raghavendra 
diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
index 9a59957922d4..d97437ba0626 100644
--- a/drivers/hv/Kconfig
+++ b/drivers/hv/Kconfig
@@ -29,4 +29,11 @@ config HYPERV_BALLOON
help
  Select this option to enable Hyper-V Balloon driver.
 
+config HYPERV_TESTING
+bool "Hyper-V testing"
+default n
+depends on HYPERV && DEBUG_FS
+help
+  Select this option to enable Hyper-V vmbus testing.
+
 endmenu
diff --git a/drivers/hv/Makefile b/drivers/hv/Makefile
index a1eec7177c2d..d754bd86ca47 100644
--- a/drivers/hv/Makefile
+++ b/drivers/hv/Makefile
@@ -2,6 +2,7 @@
 obj-$(CONFIG_HYPERV)   += hv_vmbus.o
 obj-$(CONFIG_HYPERV_UTILS) += hv_utils.o
 obj-$(CONFIG_HYPERV_BALLOON)   += hv_balloon.o
+obj-$(CONFIG_HYPERV_TESTING)   += hv_debugfs.o
 
 CFLAGS_hv_trace.o = -I$(src)
 CFLAGS_hv_balloon.o = -I$(src)
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 09829e15d4a0..4d4d40832846 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -357,6 +357,7 @@ void vmbus_on_event(unsigned long data)
 
trace_vmbus_on_event(channel);
 
+   hv_debug_delay_test(channel, INTERRUPT_DELAY);
do {
void (*callback_fn)(void *);
 
diff --git a/drivers/hv/hv_debugfs.c b/drivers/hv/hv_debugfs.c
new file mode 100644
index ..7a3f1ce71ffa
--- /dev/null
+++ b/drivers/hv/hv_debugfs.c
@@ -0,0 +1,187 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Authors:
+ *   Branden Bonaby 
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include "hyperv_vmbus.h"
+
+struct dentry *hv_debug_root;
+
+static int hv_debugfs_delay_get(void *data, u64 *val)
+{
+

[PATCH v4 0/2] hv: vmbus: add fuzz testing to hv device

2019-08-28 Thread Branden Bonaby
This patchset introduces a testing framework for Hyper-V drivers.
This framework allows us to introduce delays in the packet receive
path on a per-device basis. While the current code only supports
introducing arbitrary delays in the host/guest communication path,
we intend to expand this to support error injection in the future.

changes in v4:
  patch 1:
Combined previous v3 patches 1 and 2, into a single patch
which is now patch 1. This was done so that calls to
the new debugfs functions are in the same patch as
the definitions for these functions.

Moved debugfs code from "vmbus_drv.c" that was in
previous v3 patch 2, into a new file "debugfs.c" in
drivers/hv.

Updated the Makefile to compile "debugfs.c" if
CONFIG_HYPERV_TESTING is enabled

As per Michael's comments, added empty implementations
of the new functions, so the compiler will not generate
code when CONFIG_HYPERV_TESTING is not enabled.

  patch 2 (was previously v3 patch 3):
Based on Harrys comments, made the tool more
user friendly and added more error checking.

changes in v3:
  patch 2: change call to IS_ERR_OR_NULL, to IS_ERR.

  patch 3: Align python tool to match Linux coding style.

Changes in v2:
  Patch 1: As per Vitaly's suggestion, wrapped the test code under an
   #ifdef and updated the Kconfig file, so that the test code
   will only be used when the config option is set to true.
   (default is false).

   Updated hyperv_vmbus header to contain new #ifdef with new
   new functions for the test code.

  Patch 2: Moved code from under sysfs to debugfs and wrapped it under
   the new ifdef.

   Updated MAINTAINERS file with new debugfs-hyperv file under
   the section for hyperv.

  Patch 3: Updated testing tool with new debugfs location.

Branden Bonaby (2):
  drivers: hv: vmbus: Introduce latency testing
  tools: hv: add vmbus testing tool

 Documentation/ABI/testing/debugfs-hyperv |  23 ++
 MAINTAINERS  |   1 +
 drivers/hv/Kconfig   |   7 +
 drivers/hv/Makefile  |   1 +
 drivers/hv/connection.c  |   1 +
 drivers/hv/hv_debugfs.c  | 187 +++
 drivers/hv/hyperv_vmbus.h|  31 ++
 drivers/hv/ring_buffer.c |   2 +
 drivers/hv/vmbus_drv.c   |   6 +
 include/linux/hyperv.h   |  19 ++
 tools/hv/vmbus_testing   | 376 +++
 11 files changed, 654 insertions(+)
 create mode 100644 Documentation/ABI/testing/debugfs-hyperv
 create mode 100644 drivers/hv/hv_debugfs.c
 create mode 100644 tools/hv/vmbus_testing

-- 
2.17.1



[PATCH v4 2/2] tools: hv: add vmbus testing tool

2019-08-28 Thread Branden Bonaby
This is a userspace tool to drive the testing. Currently it supports
introducing user specified delay in the host to guest communication
path on a per-channel basis.

Signed-off-by: Branden Bonaby 
---
Changes in v4:
- Based on Harrys comments, made the tool more
  user friendly and added more error checking.

Changes in v3:
- Align python tool to match Linux coding style.

Changes in v2:
 - Move testing location to new location in debugfs.
 
 tools/hv/vmbus_testing | 376 +
 1 file changed, 376 insertions(+)
 create mode 100644 tools/hv/vmbus_testing

diff --git a/tools/hv/vmbus_testing b/tools/hv/vmbus_testing
new file mode 100644
index ..e7212903dd1d
--- /dev/null
+++ b/tools/hv/vmbus_testing
@@ -0,0 +1,376 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+#
+# Program to allow users to fuzz test Hyper-V drivers
+# by interfacing with Hyper-V debugfs attributes.
+# Current test methods available:
+#   1. delay testing
+#
+# Current file/directory structure of hyper-V debugfs:
+#   /sys/kernel/debug/hyperv/UUID
+#   /sys/kernel/debug/hyperv/UUID/
+#   /sys/kernel/debug/hyperv/UUID/
+#
+# author: Branden Bonaby 
+
+import os
+import cmd
+import argparse
+import glob
+from argparse import RawDescriptionHelpFormatter
+from argparse import RawTextHelpFormatter
+from enum import Enum
+
+# Do not change unless, you change the debugfs attributes
+# in /drivers/hv/debugfs.c. All fuzz testing
+# attributes will start with "fuzz_test".
+
+# debugfs path for hyperv must exist before proceeding
+debugfs_hyperv_path = "/sys/kernel/debug/hyperv"
+if not os.path.isdir(debugfs_hyperv_path):
+print("{} doesn't exist/check permissions".format(debugfs_hyperv_path))
+exit(-1)
+
+class dev_state(Enum):
+off = 0
+on = 1
+
+# File names, that correspond to the files created in
+# /drivers/hv/debugfs.c
+class f_names(Enum):
+state_f = "fuzz_test_state"
+buff_f =  "fuzz_test_buffer_interrupt_delay"
+mess_f =  "fuzz_test_message_delay"
+
+# Both single_actions and all_actions are used
+# for error checking and to allow for some subparser
+# names to be abbreviated. Do not abbreviate the
+# test method names, as it will become less intuitive
+# as to what the user can do. If you do decide to
+# abbreviate the test method name, make sure the main
+# function reflects this change.
+
+all_actions = [
+"disable_all",
+"D",
+"enable_all",
+"view_all",
+"V"
+]
+
+single_actions = [
+"disable_single",
+"d",
+"enable_single",
+"view_single",
+"v"
+]
+
+def main():
+
+file_map = recursive_file_lookup(debugfs_hyperv_path, dict())
+args = parse_args()
+if (not args.action):
+print ("Error, no options selected...exiting")
+exit(-1)
+arg_set = { k for (k,v) in vars(args).items() if v and k != "action" }
+arg_set.add(args.action)
+path = args.path if "path" in arg_set else None
+if (path and path[-1] == "/"):
+path = path[:-1]
+validate_args_path(path, arg_set, file_map)
+if (path and "enable_single" in arg_set):
+state_path = locate_state(path, file_map)
+set_test_state(state_path, dev_state.on.value, args.quiet)
+
+# Use subparsers as the key for different actions
+if ("delay" in arg_set):
+validate_delay_values(args.delay_time)
+if (args.enable_all):
+set_delay_all_devices(file_map, args.delay_time,
+  args.quiet)
+else:
+set_delay_values(path, file_map, args.delay_time,
+ args.quiet)
+elif ("disable_all" in arg_set or "D" in arg_set):
+disable_all_testing(file_map)
+elif ("disable_single" in arg_set or "d" in arg_set):
+disable_testing_single_device(path, file_map)
+elif ("view_all" in arg_set or "V" in arg_set):
+get_all_devices_test_status(file_map)
+elif ("view_single" in arg_set or  "v" in arg_set):
+get_device_test_values(path, file_map)
+
+# Get the state location
+def locate_state(device, file_map):
+return file_map[device][f_names.state_f.value]
+
+# Validate delay values to make sure they are acceptable to
+# enable delays on a device
+def validate_delay_values(delay):
+
+if (delay[0]  == -1 and delay[1] == -1):
+print("\nError, At least 1 value must be gre

Re: [PATCH v2 1/3] drivers: hv: vmbus: Introduce latency testing

2019-08-23 Thread Branden Bonaby
On Fri, Aug 23, 2019 at 04:44:09PM +, Michael Kelley wrote:
> From: Branden Bonaby  Sent: Thursday, August 22, 
> 2019 8:39 PM
> > > > diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> > > > index 09829e15d4a0..c9c63a4033cd 100644
> > > > --- a/drivers/hv/connection.c
> > > > +++ b/drivers/hv/connection.c
> > > > @@ -357,6 +357,9 @@ void vmbus_on_event(unsigned long data)
> > > >
> > > > trace_vmbus_on_event(channel);
> > > >
> > > > +#ifdef CONFIG_HYPERV_TESTING
> > > > +   hv_debug_delay_test(channel, INTERRUPT_DELAY);
> > > > +#endif /* CONFIG_HYPERV_TESTING */
> > >
> > > You are following Vitaly's suggestion to use #ifdef's so no code is
> > > generated when HYPERV_TESTING is not enabled.  However, this
> > > direct approach to using #ifdef's really clutters the code and makes
> > > it harder to read and follow.  The better approach is to use the
> > > #ifdef in the include file where the functions are defined.  If
> > > HYPERV_TESTING is not enabled, provide a #else that defines
> > > the function with an empty implementation for which the compiler
> > > will generate no code.   An as example, see the function definition
> > > for hyperv_init() in arch/x86/include/asm/mshyperv.h.  There are
> > > several functions treated similarly in that include file.
> > >
> > 
> > I checked out the code in arch/x86/include/asm/mshyperv.h, after
> > thinking about it, I'm wondering if it would be better just to have
> > two files one called hv_debugfs.c and the other hyperv_debugfs.h.
> > I could put the code definitions in hv_debugfs.c and at the top
> > include the hyperv_debugfs.h file which would house the declarations
> > of these functions under the ifdef. Then like you alluded too use
> > an #else statement that would have the null implementations of the
> > above functions. Then put an #include "hyperv_debugfs.h" in the
> > hyperv_vmbus.h file. I figured instead of putting the code directly
> > into the vmbus_drv.c file it might be best to put them in a seperate
> > file like hv_debugfs.c. This way when we start adding more tests we
> > don't bloat the vmbus_drv.c file unnecessarily. The hv_debugfs.c
> > file would have the #ifdef CONFIG_HYPERV_TESTING at the top so if
> > its not enabled  those null implementations in "hyperv_debugfs.h"
> > woud kick in anywhere that included the hyperv_vmbus.h file which
> > is what we want.
> > 
> > what do you think?
> > 
> 
> I'll preface my comments by saying that how code gets structured
> into files is always a bit of a judgment call.  The goal is to group code
> into sensible chunks to make it easy to understand and to make it
> easy to modify and extend later.  The latter is a prediction about the
> future, which may or may not be accurate.   For the former, what's
> "easy to understand," is often in the eye of the beholder.  So you may
> get different opinions from different reviewers.
> 
> That said, I like the idea of a separate hv_debugfs.c file to contain
> the implementation of the various functions you have added to
> provide the fuzzing capability.   I'm less convinced about the value
> of a separate hyperv_debugfs.h file.   I think you have one big
> #ifdef CONFIG_HYPERV_TESTING followed by the declarations of
> the functions in hv_debugfs.c, followed by #else and null
> implementations of those functions.  This is 20 lines of code or so,
> and could easily go in hyperv_vmbus.h.
> 
> For the new hv_debugfs.c, you can avoid the need for
> #ifdef CONFIG_HYPERV_TESTING by modifying the Makefile in
> drivers/hv so that hv_debugfs.o is built only if CONFIG_HYPERV_TESTING
> is defined.  Look at the current Makefile to see how this is done
> with CONFIG_HYPERV_UTILS and CONFIG_HYPERV_BALLOON.
> 
> Michael
>

I see, that does make sense, I'll go ahead and add these changes.

thanks

branden bonaby


Re: [PATCH v2 1/3] drivers: hv: vmbus: Introduce latency testing

2019-08-22 Thread Branden Bonaby
> >  endmenu
> > diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> > index 09829e15d4a0..c9c63a4033cd 100644
> > --- a/drivers/hv/connection.c
> > +++ b/drivers/hv/connection.c
> > @@ -357,6 +357,9 @@ void vmbus_on_event(unsigned long data)
> > 
> > trace_vmbus_on_event(channel);
> > 
> > +#ifdef CONFIG_HYPERV_TESTING
> > +   hv_debug_delay_test(channel, INTERRUPT_DELAY);
> > +#endif /* CONFIG_HYPERV_TESTING */
> 
> You are following Vitaly's suggestion to use #ifdef's so no code is
> generated when HYPERV_TESTING is not enabled.  However, this
> direct approach to using #ifdef's really clutters the code and makes
> it harder to read and follow.  The better approach is to use the
> #ifdef in the include file where the functions are defined.  If
> HYPERV_TESTING is not enabled, provide a #else that defines
> the function with an empty implementation for which the compiler
> will generate no code.   An as example, see the function definition
> for hyperv_init() in arch/x86/include/asm/mshyperv.h.  There are
> several functions treated similarly in that include file.
> 

I checked out the code in arch/x86/include/asm/mshyperv.h, after
thinking about it, I'm wondering if it would be better just to have
two files one called hv_debugfs.c and the other hyperv_debugfs.h.
I could put the code definitions in hv_debugfs.c and at the top
include the hyperv_debugfs.h file which would house the declarations
of these functions under the ifdef. Then like you alluded too use
an #else statement that would have the null implementations of the
above functions. Then put an #include "hyperv_debugfs.h" in the
hyperv_vmbus.h file. I figured instead of putting the code directly
into the vmbus_drv.c file it might be best to put them in a seperate
file like hv_debugfs.c. This way when we start adding more tests we
don't bloat the vmbus_drv.c file unnecessarily. The hv_debugfs.c
file would have the #ifdef CONFIG_HYPERV_TESTING at the top so if
its not enabled  those null implementations in "hyperv_debugfs.h"
woud kick in anywhere that included the hyperv_vmbus.h file which
is what we want.

what do you think?

> 
> > do {
> > void (*callback_fn)(void *);
> > 
> > diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> > index 362e70e9d145..edf14f596d8c 100644
> > --- a/drivers/hv/hyperv_vmbus.h
> > +++ b/drivers/hv/hyperv_vmbus.h
> > @@ -357,4 +357,24 @@ enum hvutil_device_state {
> > HVUTIL_DEVICE_DYING, /* driver unload is in progress */
> >  };
> > 
> > +#ifdef CONFIG_HYPERV_TESTING
> > +#include 
> > +#include 
> > +#include 
> 
> Generally #include files should go at the top of the file, even if they
> are only needed conditionally.
>

I see , will change

> > +#define TESTING "hyperv"
> 
> I'm not seeing what this line is for, or how it is used.

I used it as the top level name for the dentry that
would appear in debugfs but now I realize its actually
not needed, so i'll remove this.

> > --- a/include/linux/hyperv.h
> > +++ b/include/linux/hyperv.h
> > @@ -926,6 +926,21 @@ struct vmbus_channel {
> >  * full outbound ring buffer.
> >  */
> > u64 out_full_first;
> > +
> > +#ifdef CONFIG_HYPERV_TESTING
> > +   /* enabling/disabling fuzz testing on the channel (default is false)*/
> > +   bool fuzz_testing_state;
> > +
> > +   /* Interrupt delay will delay the guest from emptying the ring buffer
> > +* for a specific amount of time. The delay is in microseconds and will
> > +* be between 1 to a maximum of 1000, its default is 0 (no delay).
> > +* The  Message delay will delay guest reading on a per message basis
> > +* in microseconds between 1 to 1000 with the default being 0
> > +* (no delay).
> > +*/
> > +   u32 fuzz_testing_interrupt_delay;
> > +   u32 fuzz_testing_message_delay;
> > +#endif /* CONFIG_HYPERV_TESTING */
> 
> For fields in a data structure like this, you don't have much choice
> but to put the #ifdef directly inline.  However, for small fields like this
> and where the data structure isn't size sensitive, you could consider
> omitting the #ifdef and just always including the fields even when
> HYPERV_TESTING is not enabled.  I don't have a strong preference
> either way.
>

I'll take the ifdefs out since the fields aren't too big



Re: [PATCH v3 2/3] drivers: hv: vmbus: add test attributes to debugfs

2019-08-21 Thread Branden Bonaby
> > +What:   /sys/kernel/debug/hyperv//fuzz_test_state
> > +Date:   August 2019
> > +KernelVersion:  5.3
> > +Contact:Branden Bonaby 
> > +Description:Fuzz testing status of a vmbus device, whether its in an ON
> > +state or a OFF state
> 
> Document what values are actually returned?  
> 
> > +Users:  Debugging tools
> > +
> > +What:   
> > /sys/kernel/debug/hyperv//delay/fuzz_test_buffer_interrupt_delay
> > +Date:       August 2019
> > +KernelVersion:  5.3
> > +Contact:Branden Bonaby 
> > +Description:Fuzz testing buffer delay value between 0 - 1000
> 
> It would be helpful to document the units -- I think this is 0 to 1000
> microseconds.

you're right, that makes sense I'll add that information in. Also 
to confirm, it is microseconds like you said.

> > +static int hv_debugfs_delay_set(void *data, u64 val)
> > +{
> > +   if (val >= 1 && val <= 1000)
> > +   *(u32 *)data = val;
> > +   /*Best to not use else statement here since we want
> > +* the delay to remain the same if val > 1000
> > +*/
> 
> The standard multi-line comment style would be:
> 
>   /*
>* Best to not use else statement here since we want
>* the delay to remain the same if val > 1000
>*/
>

will change

> > +   else if (val <= 0)
> > +   *(u32 *)data = 0;
> 
> You could consider returning an error for an invalid
> value (< 0, or > 1000).
> 

its subtle but it does make sense and shows anyone
reading that the only acceptable values in the 
function are 0 <= 1000 at a glance. I'll add
that in.



Re: [PATCH v3 3/3] tools: hv: add vmbus testing tool

2019-08-21 Thread Branden Bonaby
On Thu, Aug 22, 2019 at 01:36:09AM +, Harry Zhang wrote:
> Tool function issues:  Please validate args errors for  '-p' and '--path',  
> in or following validate_args_path().  
> 
> Comments of functionality:
> - it's confusing when fuzz_testing are all OFF, then user run ' python3 
> /home/lisa/vmbus_testing -p 
> /sys/kernel/debug/hyperv/000d3a6e-4548-000d-3a6e-4548000d3a6e delay -d 0 0 -D 
> ' which will enable all delay testing state ('Y' in state files).  even I 
> used "-D", "--dis_all" param. 
> - if we have subparsers of "disable-all" for the testing tool, then 
> probably we don't need the mutually_exclusive_group under subparsers of 
> "delay"
> - the path argument (-p) could be an argument for subparsers of "delay" 
> and "view" only.
> 
> Regards,
> Harry
>

So I made the choice to keep disabling the state and disabling delay
testing seperate, because once we start adding other testing options
you wouldn't want to inadvertently disable all testing especially
if you were doing more than one type of test at a time.
So with your configuration

'python3 /home/lisa/vmbus_testing -p 
/sys/kernel/debug/hyperv/000d3a6e-4548-000d-3a6e-4548000d3a6e delay -d 0 0 -D '

this would stop all delay testing on all the devices but wouldn't change
their test state to OFF 'N'.So thats why I have the option -s --state to
change the state to Off with a -s 0. Then to disable all types of testing
and change the state to OFF thats where the 'disable-all' subparser  comes in.
with:

'python3 /home/lisa/vmbus_testing disable-all

For that last point I don't understand what you mean, are you saying it would be
better to have something like this using  delay as an example?

'python3 /home/lisa/vmbus_testing delay -p 
/sys/kernel/debug/hyperv/000d3a6e-4548-000d-3a6e-4548000d3a6e'

If thats what you mean I figured it was better to make the -p accessible
to all test type so I made it apart of the main parser. This would allow
us to just have it there once instead of having to make a -p for every
subparser.

Also maybe I need to change the examples and the help text
because with the -D option for delay you wouldnt actually need to put in 
the path. As

'python3 /home/lisa/vmbus_testing delay -d 0 0 -D '

would suffice to stop delay testing on all devices; -E would enable
it for all devices and change the state to On 'Y' if it wasn't already.

let me know your thoughts

branden bonaby


[PATCH v3 3/3] tools: hv: add vmbus testing tool

2019-08-20 Thread Branden Bonaby
This is a userspace tool to drive the testing. Currently it supports
introducing user specified delay in the host to guest communication
path on a per-channel basis.

Signed-off-by: Branden Bonaby 
---
Changes in v3:
 - Align python tool to match Linux coding style.

Changes in v2:
 - Move testing location to new location in debugfs.

 tools/hv/vmbus_testing | 342 +
 1 file changed, 342 insertions(+)
 create mode 100644 tools/hv/vmbus_testing

diff --git a/tools/hv/vmbus_testing b/tools/hv/vmbus_testing
new file mode 100644
index ..0f249f6ee698
--- /dev/null
+++ b/tools/hv/vmbus_testing
@@ -0,0 +1,342 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+#
+# Program to allow users to fuzz test Hyper-V drivers
+# by interfacing with Hyper-V debugfs directories
+# author: Branden Bonaby
+
+import os
+import cmd
+import argparse
+from collections import defaultdict
+from argparse import RawDescriptionHelpFormatter
+
+# debugfs paths for vmbus must exist (same as in lsvmbus)
+debugfs_sys_path = "/sys/kernel/debug/hyperv"
+if not os.path.isdir(debugfs_sys_path):
+print("{} doesn't exist/check permissions".format(debugfs_sys_path))
+exit(-1)
+# Do not change unless, you change the debugfs attributes
+# in "/sys/kernel/debug/hyperv//". All fuzz testing
+# attributes will start with "fuzz_test".
+pathlen = len(debugfs_sys_path)
+fuzz_state_location = "fuzz_test_state"
+fuzz_states = {
+0 : "Disable",
+1 : "Enable"
+}
+
+fuzz_methods = {
+1 : "Delay_testing"
+}
+
+fuzz_delay_types = {
+1 : "fuzz_test_buffer_interrupt_delay",
+2 : "fuzz_test_message_delay"
+}
+
+def parse_args():
+parser = argparse.ArgumentParser(description = "vmbus_testing "
+"[-s] [0|1] [-q] [-p] \n""vmbus_testing [-s]"
+" [0|1] [-q][-p]  delay [-d] [val][val] 
[-E|-D]\n"
+"vmbus_testing [-q] disable-all\n"
+"vmbus_testing [-q] view [-v|-V]\n"
+"vmbus_testing --version",
+epilog = "Current testing options {}".format(fuzz_methods),
+prog = 'vmbus_testing',
+formatter_class = RawDescriptionHelpFormatter)
+subparsers = parser.add_subparsers(dest = "action")
+parser.add_argument("--version", action = "version",
+version = '%(prog)s 1.0')
+parser.add_argument("-q","--quiet", action = "store_true",
+help = "silence none important test messages")
+parser.add_argument("-s","--state", metavar = "", type = int,
+choices = range(0, 2),
+help = "Turn testing ON or OFF for a single device."
+" The value (1) will turn testing ON. The value"
+" of (0) will turn testing OFF with the default set"
+" to (0).")
+parser.add_argument("-p","--path", metavar = "",
+help = "Refers to the debugfs path to a vmbus device."
+" If the path is not a valid path to a vmbus device,"
+" the program will exit. The path must be the"
+" absolute path; use the lsvmbus command to find"
+" the path.")
+parser_delay = subparsers.add_parser("delay",
+help = "Delay buffer/message reads in microseconds.",
+description = "vmbus_testing -s [0|1] [-q] -p "
+" delay -d "
+"[buffer-delay-value] [message-delay-value]\n"
+"vmbus_testing [-q] delay [buffer-delay-value] "
+"[message-delay-value] -E\n"
+"vmbus_testing [-q] delay [buffer-delay-value] "
+"[message-delay-value] -D",
+formatter_class = RawDescriptionHelpFormatter)
+delay_group = parser_delay.add_mutually_exclusive_group()
+delay_group.add_argument("-E", "--en_all", action = "store_true",
+help = "Enable Buffer/Message Delay testing on ALL"
+" devices. Use -d option with this to set the values"
+" for both the buffer delay and the message delay. No"
+"

[PATCH v3 2/3] drivers: hv: vmbus: add test attributes to debugfs

2019-08-20 Thread Branden Bonaby
Expose the test parameters as part of the debugfs channel attributes.
We will control the testing state via these attributes.

Signed-off-by: Branden Bonaby 
---
Changes in v3:
 - Change call to IS_ERR_OR_NULL, to IS_ERR.

Changes in v2:
 - Move test attributes to debugfs.
 - Wrap test code under #ifdef statements.
 - Add new documentation file under Documentation/ABI/testing.
 - Make commit message reflect the change from from sysfs to debugfs.

 Documentation/ABI/testing/debugfs-hyperv |  21 +++
 MAINTAINERS  |   1 +
 drivers/hv/vmbus_drv.c   | 167 +++
 3 files changed, 189 insertions(+)
 create mode 100644 Documentation/ABI/testing/debugfs-hyperv

diff --git a/Documentation/ABI/testing/debugfs-hyperv 
b/Documentation/ABI/testing/debugfs-hyperv
new file mode 100644
index ..b25f751fafa8
--- /dev/null
+++ b/Documentation/ABI/testing/debugfs-hyperv
@@ -0,0 +1,21 @@
+What:   /sys/kernel/debug/hyperv//fuzz_test_state
+Date:   August 2019
+KernelVersion:  5.3
+Contact:Branden Bonaby 
+Description:Fuzz testing status of a vmbus device, whether its in an ON
+state or a OFF state
+Users:  Debugging tools
+
+What:   
/sys/kernel/debug/hyperv//delay/fuzz_test_buffer_interrupt_delay
+Date:   August 2019
+KernelVersion:  5.3
+Contact:Branden Bonaby 
+Description:Fuzz testing buffer delay value between 0 - 1000
+Users:  Debugging tools
+
+What:   /sys/kernel/debug/hyperv//delay/fuzz_test_message_delay
+Date:   August 2019
+KernelVersion:  5.3
+Contact:Branden Bonaby 
+Description:Fuzz testing message delay value between 0 - 1000
+Users:  Debugging tools
diff --git a/MAINTAINERS b/MAINTAINERS
index e81e60bd7c26..120284a8185f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7460,6 +7460,7 @@ F:include/uapi/linux/hyperv.h
 F: include/asm-generic/mshyperv.h
 F: tools/hv/
 F: Documentation/ABI/stable/sysfs-bus-vmbus
+F: Documentation/ABI/testing/debugfs-hyperv
 
 HYPERBUS SUPPORT
 M: Vignesh Raghavendra 
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index ebd35fc35290..d2e47f04d172 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -919,6 +919,10 @@ static void vmbus_device_release(struct device *device)
struct hv_device *hv_dev = device_to_hv_device(device);
struct vmbus_channel *channel = hv_dev->channel;
 
+#ifdef CONFIG_HYPERV_TESTING
+   hv_debug_rm_dev_dir(hv_dev);
+#endif /* CONFIG_HYPERV_TESTING */
+
mutex_lock(_connection.channel_mutex);
hv_process_channel_removal(channel);
mutex_unlock(_connection.channel_mutex);
@@ -1727,6 +1731,9 @@ int vmbus_device_register(struct hv_device 
*child_device_obj)
pr_err("Unable to register primary channeln");
goto err_kset_unregister;
}
+#ifdef CONFIG_HYPERV_TESTING
+   hv_debug_add_dev_dir(child_device_obj);
+#endif /* CONFIG_HYPERV_TESTING */
 
return 0;
 
@@ -2086,6 +2093,159 @@ static void hv_crash_handler(struct pt_regs *regs)
hyperv_cleanup();
 };
 
+#ifdef CONFIG_HYPERV_TESTING
+
+struct dentry *hv_root;
+
+static int hv_debugfs_delay_get(void *data, u64 *val)
+{
+   *val = *(u32 *)data;
+   return 0;
+}
+
+static int hv_debugfs_delay_set(void *data, u64 val)
+{
+   if (val >= 1 && val <= 1000)
+   *(u32 *)data = val;
+   /*Best to not use else statement here since we want
+* the delay to remain the same if val > 1000
+*/
+   else if (val <= 0)
+   *(u32 *)data = 0;
+   return 0;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(hv_debugfs_delay_fops, hv_debugfs_delay_get,
+hv_debugfs_delay_set, "%llu\n");
+
+/* Setup delay files to store test values */
+int hv_debug_delay_files(struct hv_device *dev, struct dentry *root)
+{
+   struct vmbus_channel *channel = dev->channel;
+   char *buffer = "fuzz_test_buffer_interrupt_delay";
+   char *message = "fuzz_test_message_delay";
+   int *buffer_val = >fuzz_testing_interrupt_delay;
+   int *message_val = >fuzz_testing_message_delay;
+   struct dentry *buffer_file, *message_file;
+
+   buffer_file = debugfs_create_file(buffer, 0644, root,
+ buffer_val,
+ _debugfs_delay_fops);
+   if (IS_ERR(buffer_file)) {
+   pr_debug("debugfs_hyperv: file %s not created\n", buffer);
+   return PTR_ERR(buffer_file);
+   }
+
+   message_file = debugfs_create_file(message, 0644, root,
+  message_val,
+  _debugfs_delay_fops);
+   if (IS_ERR(message_file)) {
+   pr_debug("debugf

[PATCH v3 1/3] drivers: hv: vmbus: Introduce latency testing

2019-08-20 Thread Branden Bonaby
Introduce user specified latency in the packet reception path.

Signed-off-by: Branden Bonaby 
---
Changes in v2:
 - Add #ifdef in Kconfig file so test code will not interfere
   with non-test code.
 - Move test code functions for delay to hyperv_vmbus header
   file.
 - Wrap test code under #ifdef statement. 

 drivers/hv/Kconfig|  7 +++
 drivers/hv/connection.c   |  3 +++
 drivers/hv/hyperv_vmbus.h | 20 
 drivers/hv/ring_buffer.c  |  7 +++
 include/linux/hyperv.h| 21 +
 5 files changed, 58 insertions(+)

diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
index 9a59957922d4..d97437ba0626 100644
--- a/drivers/hv/Kconfig
+++ b/drivers/hv/Kconfig
@@ -29,4 +29,11 @@ config HYPERV_BALLOON
help
  Select this option to enable Hyper-V Balloon driver.
 
+config HYPERV_TESTING
+bool "Hyper-V testing"
+default n
+depends on HYPERV && DEBUG_FS
+help
+  Select this option to enable Hyper-V vmbus testing.
+
 endmenu
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 09829e15d4a0..c9c63a4033cd 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -357,6 +357,9 @@ void vmbus_on_event(unsigned long data)
 
trace_vmbus_on_event(channel);
 
+#ifdef CONFIG_HYPERV_TESTING
+   hv_debug_delay_test(channel, INTERRUPT_DELAY);
+#endif /* CONFIG_HYPERV_TESTING */
do {
void (*callback_fn)(void *);
 
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 362e70e9d145..edf14f596d8c 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -357,4 +357,24 @@ enum hvutil_device_state {
HVUTIL_DEVICE_DYING, /* driver unload is in progress */
 };
 
+#ifdef CONFIG_HYPERV_TESTING
+#include 
+#include 
+#include 
+#define TESTING "hyperv"
+
+enum delay {
+   INTERRUPT_DELAY = 0,
+   MESSAGE_DELAY   = 1,
+};
+
+int hv_debug_delay_files(struct hv_device *dev, struct dentry *root);
+int hv_debug_add_dev_dir(struct hv_device *dev);
+void hv_debug_rm_dev_dir(struct hv_device *dev);
+void hv_debug_rm_all_dir(void);
+void hv_debug_set_dir_dentry(struct hv_device *dev, struct dentry *root);
+void hv_debug_delay_test(struct vmbus_channel *channel, enum delay delay_type);
+
+#endif /* CONFIG_HYPERV_TESTING */
+
 #endif /* _HYPERV_VMBUS_H */
diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index 9a03b163cbbd..51adda23b398 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -396,6 +396,10 @@ struct vmpacket_descriptor *hv_pkt_iter_first(struct 
vmbus_channel *channel)
struct hv_ring_buffer_info *rbi = >inbound;
struct vmpacket_descriptor *desc;
 
+#ifdef CONFIG_HYPERV_TESTING
+   hv_debug_delay_test(channel, MESSAGE_DELAY);
+#endif /* CONFIG_HYPERV_TESTING */
+
if (hv_pkt_iter_avail(rbi) < sizeof(struct vmpacket_descriptor))
return NULL;
 
@@ -421,6 +425,9 @@ __hv_pkt_iter_next(struct vmbus_channel *channel,
u32 packetlen = desc->len8 << 3;
u32 dsize = rbi->ring_datasize;
 
+#ifdef CONFIG_HYPERV_TESTING
+   hv_debug_delay_test(channel, MESSAGE_DELAY);
+#endif /* CONFIG_HYPERV_TESTING */
/* bump offset to next potential packet */
rbi->priv_read_index += packetlen + VMBUS_PKT_TRAILER;
if (rbi->priv_read_index >= dsize)
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 6256cc34c4a6..6bf8ef5c780c 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -926,6 +926,21 @@ struct vmbus_channel {
 * full outbound ring buffer.
 */
u64 out_full_first;
+
+#ifdef CONFIG_HYPERV_TESTING
+   /* enabling/disabling fuzz testing on the channel (default is false)*/
+   bool fuzz_testing_state;
+
+   /* Interrupt delay will delay the guest from emptying the ring buffer
+* for a specific amount of time. The delay is in microseconds and will
+* be between 1 to a maximum of 1000, its default is 0 (no delay).
+* The  Message delay will delay guest reading on a per message basis
+* in microseconds between 1 to 1000 with the default being 0
+* (no delay).
+*/
+   u32 fuzz_testing_interrupt_delay;
+   u32 fuzz_testing_message_delay;
+#endif /* CONFIG_HYPERV_TESTING */
 };
 
 static inline bool is_hvsock_channel(const struct vmbus_channel *c)
@@ -1166,6 +1181,12 @@ struct hv_device {
 
struct vmbus_channel *channel;
struct kset  *channels_kset;
+
+#ifdef CONFIG_HYPERV_TESTING
+   /* place holder to keep track of the dir for hv device in debugfs */
+   struct dentry *debug_dir;
+#endif /* CONFIG_HYPERV_TESTING */
+
 };
 
 
-- 
2.17.1



[PATCH v3 0/3] hv: vmbus: add fuzz testing to hv device

2019-08-20 Thread Branden Bonaby
This patchset introduces a testing framework for Hyper-V drivers.
This framework allows us to introduce delays in the packet receive
path on a per-device basis. While the current code only supports
introducing arbitrary delays in the host/guest communication path,
we intend to expand this to support error injection in the future.

changes in v3:
  patch 2: change call to IS_ERR_OR_NULL, to IS_ERR.

  patch 3: Align python tool to match Linux coding style.

Changes in v2:
  Patch 1: As per Vitaly's suggestion, wrapped the test code under an
   #ifdef and updated the Kconfig file, so that the test code
   will only be used when the config option is set to true.
   (default is false).

   Updated hyperv_vmbus header to contain new #ifdef with new
   new functions for the test code.

  Patch 2: Moved code from under sysfs to debugfs and wrapped it under
   the new ifdef.

   Updated MAINTAINERS file with new debugfs-hyperv file under
   the section for hyperv.

  Patch 3: Updated testing tool with new debugfs location.

Branden Bonaby (3):
  drivers: hv: vmbus: Introduce latency testing
  drivers: hv: vmbus: add fuzz test attributes to debugfs
  tools: hv: add vmbus testing tool

 Documentation/ABI/testing/debugfs-hyperv |  21 ++
 MAINTAINERS  |   1 +
 drivers/hv/Kconfig   |   7 +
 drivers/hv/connection.c  |   3 +
 drivers/hv/hyperv_vmbus.h|  20 ++
 drivers/hv/ring_buffer.c |   7 +
 drivers/hv/vmbus_drv.c   | 167 +++
 include/linux/hyperv.h   |  21 ++
 tools/hv/vmbus_testing   | 342 +++
 9 files changed, 589 insertions(+)
 create mode 100644 Documentation/ABI/testing/debugfs-hyperv
 create mode 100644 tools/hv/vmbus_testing

-- 
2.17.1



Re: [PATCH v2 2/3] drivers: hv: vmbus: add test attributes to debugfs

2019-08-20 Thread Branden Bonaby
On Mon, Aug 19, 2019 at 10:44:48PM -0400, Branden Bonaby wrote:
> Expose the test parameters as part of the debugfs channel attributes.
> We will control the testing state via these attributes.
> 
> Signed-off-by: Branden Bonaby 
> ---
> Changes in v2:
>  - Move test attributes to debugfs.
>  - Wrap test code under #ifdef statements.
>  - Add new documentation file under Documentation/ABI/testing.
>  - Make commit message reflect the change from from sysfs to debugfs.
>  
> 
> + if (IS_ERR_OR_NULL(dev_root)) {
> + pr_debug("debugfs_hyperv: %s/%s/ not created\n",
> +  TESTING, device);
> + return PTR_ERR(dev_root);
> + }

Whoops, that single IS_ERR_OR_NULL shouldn't be there, it should just
be IS_ERR I'll change and resend the patch.


[PATCH v2 3/3] tools: hv: add vmbus testing tool

2019-08-19 Thread Branden Bonaby
This is a userspace tool to drive the testing. Currently it supports
introducing user specified delay in the host to guest communication
path on a per-channel basis.

Signed-off-by: Branden Bonaby 
---
Changes in v2:
 - Move testing location to new location in debugfs.
 
 tools/hv/vmbus_testing | 334 +
 1 file changed, 334 insertions(+)
 create mode 100644 tools/hv/vmbus_testing

diff --git a/tools/hv/vmbus_testing b/tools/hv/vmbus_testing
new file mode 100644
index ..f615009b7393
--- /dev/null
+++ b/tools/hv/vmbus_testing
@@ -0,0 +1,334 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+#
+#Program to allow users to fuzz test Hyper-V drivers
+#by interfacing with Hyper-V debugfs directories
+#author: Branden Bonaby
+
+import os
+import cmd
+import argparse
+from collections import defaultdict
+from argparse import RawDescriptionHelpFormatter
+
+#debugfs paths for vmbus must exist (same as in lsvmbus)
+debugfs_sys_path = '/sys/kernel/debug/hyperv'
+if not os.path.isdir(debugfs_sys_path):
+print("{} doesn't exist/check 
permissions".format(debugfs_sys_path))
+exit(-1)
+#Do not change unless, you change the debugfs attributes
+#in "/sys/kernel/debug/hyperv//". All fuzz testing
+#attributes will start with "fuzz_test".
+pathlen = len(debugfs_sys_path)
+fuzz_state_location = "fuzz_test_state"
+fuzz_states = {0 : "Disable", 1 : "Enable"}
+fuzz_methods ={1 : "Delay_testing"}
+fuzz_delay_types = {1 : "fuzz_test_buffer_interrupt_delay", 2 
:"fuzz_test_message_delay"}
+
+def parse_args():
+parser = argparse.ArgumentParser(description = "vmbus_testing "\
+"[-s] [0|1] [-q] [-p] \n""vmbus_testing [-s]"\
+" [0|1] [-q][-p]  delay [-d] [val][val] 
[-E|-D]\n"
+"vmbus_testing [-q] disable-all\n"
+"vmbus_testing [-q] view [-v|-V]\n"\
+"vmbus_testing --version",
+epilog = "Current testing options {}".format(fuzz_methods),
+prog = 'vmbus_testing',
+formatter_class = RawDescriptionHelpFormatter)
+subparsers = parser.add_subparsers(dest="action")
+parser.add_argument('--version', action='version',\
+version = '%(prog)s 1.0')
+parser.add_argument("-q","--quiet",action = "store_true",\
+help = "silence none important test messages")
+parser.add_argument("-s","--state",metavar = "", type = int,\
+choices = range(0,2),\
+help = "Turn testing ON or OFF for a single device."\
+" The value (1) will turn testing ON. The value"\
+" of (0) will turn testing OFF with the default set"\
+" to (0).")
+parser.add_argument("-p","--path", metavar = "",\
+help = "Refers to the debugfs path to a vmbus device."
+" If the path is not a valid path to a vmbus device,"\
+" the program will exit. The path must be the"\
+" absolute path; use the lsvmbus command to find"\
+" the path.")
+parser_delay = subparsers.add_parser("delay",\
+help = "Delay buffer/message reads in microseconds.",
+description = "vmbus_testing -s [0|1] [-q] -p "\
+" delay -d "\
+"[buffer-delay-value] [message-delay-value]\n"
+"vmbus_testing [-q] delay [buffer-delay-value] "\
+"[message-delay-value] -E\n"
+"vmbus_testing [-q] delay [buffer-delay-value] "\
+"[message-delay-value] -D",
+formatter_class = RawDescriptionHelpFormatter)
+delay_group = parser_delay.add_mutually_exclusive_group()
+delay_group.add_argument("-E","--en-all-delay", action = "store_true",\
+help = "Enable Buffer/Message Delay testing on ALL"\
+" devices. Use -d option with this to set the values"\
+" for both the buffer delay and the message delay. No"\
+" value can be (0) or less than (-1). If testing is"\
+" disabled on a device prior to runni

[PATCH v2 2/3] drivers: hv: vmbus: add test attributes to debugfs

2019-08-19 Thread Branden Bonaby
Expose the test parameters as part of the debugfs channel attributes.
We will control the testing state via these attributes.

Signed-off-by: Branden Bonaby 
---
Changes in v2:
 - Move test attributes to debugfs.
 - Wrap test code under #ifdef statements.
 - Add new documentation file under Documentation/ABI/testing.
 - Make commit message reflect the change from from sysfs to debugfs.
 
 Documentation/ABI/testing/debugfs-hyperv |  21 +++
 MAINTAINERS  |   1 +
 drivers/hv/vmbus_drv.c   | 167 +++
 3 files changed, 189 insertions(+)
 create mode 100644 Documentation/ABI/testing/debugfs-hyperv

diff --git a/Documentation/ABI/testing/debugfs-hyperv 
b/Documentation/ABI/testing/debugfs-hyperv
new file mode 100644
index ..b25f751fafa8
--- /dev/null
+++ b/Documentation/ABI/testing/debugfs-hyperv
@@ -0,0 +1,21 @@
+What:   /sys/kernel/debug/hyperv//fuzz_test_state
+Date:   August 2019
+KernelVersion:  5.3
+Contact:Branden Bonaby 
+Description:Fuzz testing status of a vmbus device, whether its in an ON
+state or a OFF state
+Users:  Debugging tools
+
+What:   
/sys/kernel/debug/hyperv//delay/fuzz_test_buffer_interrupt_delay
+Date:   August 2019
+KernelVersion:  5.3
+Contact:Branden Bonaby 
+Description:Fuzz testing buffer delay value between 0 - 1000
+Users:  Debugging tools
+
+What:   /sys/kernel/debug/hyperv//delay/fuzz_test_message_delay
+Date:   August 2019
+KernelVersion:  5.3
+Contact:Branden Bonaby 
+Description:Fuzz testing message delay value between 0 - 1000
+Users:  Debugging tools
diff --git a/MAINTAINERS b/MAINTAINERS
index e81e60bd7c26..120284a8185f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7460,6 +7460,7 @@ F:include/uapi/linux/hyperv.h
 F: include/asm-generic/mshyperv.h
 F: tools/hv/
 F: Documentation/ABI/stable/sysfs-bus-vmbus
+F: Documentation/ABI/testing/debugfs-hyperv
 
 HYPERBUS SUPPORT
 M: Vignesh Raghavendra 
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index ebd35fc35290..b6d023ae9664 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -919,6 +919,10 @@ static void vmbus_device_release(struct device *device)
struct hv_device *hv_dev = device_to_hv_device(device);
struct vmbus_channel *channel = hv_dev->channel;
 
+#ifdef CONFIG_HYPERV_TESTING
+   hv_debug_rm_dev_dir(hv_dev);
+#endif /* CONFIG_HYPERV_TESTING */
+
mutex_lock(_connection.channel_mutex);
hv_process_channel_removal(channel);
mutex_unlock(_connection.channel_mutex);
@@ -1727,6 +1731,9 @@ int vmbus_device_register(struct hv_device 
*child_device_obj)
pr_err("Unable to register primary channeln");
goto err_kset_unregister;
}
+#ifdef CONFIG_HYPERV_TESTING
+   hv_debug_add_dev_dir(child_device_obj);
+#endif /* CONFIG_HYPERV_TESTING */
 
return 0;
 
@@ -2086,6 +2093,159 @@ static void hv_crash_handler(struct pt_regs *regs)
hyperv_cleanup();
 };
 
+#ifdef CONFIG_HYPERV_TESTING
+
+struct dentry *hv_root;
+
+static int hv_debugfs_delay_get(void *data, u64 *val)
+{
+   *val = *(u32 *)data;
+   return 0;
+}
+
+static int hv_debugfs_delay_set(void *data, u64 val)
+{
+   if (val >= 1 && val <= 1000)
+   *(u32 *)data = val;
+   /*Best to not use else statement here since we want
+* the delay to remain the same if val > 1000
+*/
+   else if (val <= 0)
+   *(u32 *)data = 0;
+   return 0;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(hv_debugfs_delay_fops, hv_debugfs_delay_get,
+hv_debugfs_delay_set, "%llu\n");
+
+/* Setup delay files to store test values */
+int hv_debug_delay_files(struct hv_device *dev, struct dentry *root)
+{
+   struct vmbus_channel *channel = dev->channel;
+   char *buffer = "fuzz_test_buffer_interrupt_delay";
+   char *message = "fuzz_test_message_delay";
+   int *buffer_val = >fuzz_testing_interrupt_delay;
+   int *message_val = >fuzz_testing_message_delay;
+   struct dentry *buffer_file, *message_file;
+
+   buffer_file = debugfs_create_file(buffer, 0644, root,
+ buffer_val,
+ _debugfs_delay_fops);
+   if (IS_ERR(buffer_file)) {
+   pr_debug("debugfs_hyperv: file %s not created\n", buffer);
+   return PTR_ERR(buffer_file);
+   }
+
+   message_file = debugfs_create_file(message, 0644, root,
+  message_val,
+  _debugfs_delay_fops);
+   if (IS_ERR(message_file)) {
+   pr_debug("debugfs_hyperv: file %s not created\n", message);
+   return PTR_ERR(m

[PATCH v2 0/3] hv: vmbus: add fuzz testing to hv device

2019-08-19 Thread Branden Bonaby
This patchset introduces a testing framework for Hyper-V drivers.
This framework allows us to introduce delays in the packet receive
path on a per-device basis. While the current code only supports
introducing arbitrary delays in the host/guest communication path,
we intend to expand this to support error injection in the future.

Changes in v2:
  Patch 1: As per Vitaly's suggestion, wrapped the test code under an
   #ifdef and updated the Kconfig file, so that the test code
   will only be used when the config option is set to true.
   (default is false).

   Updated hyperv_vmbus header to contain new #ifdef with new
   new functions for the test code.

  Patch 2: Moved code from under sysfs to debugfs and wrapped it under
   the new ifdef.

   Updated MAINTAINERS file with new debugfs-hyperv file under
   the section for hyperv.

  Patch 3: Updated testing tool with new debugfs location.

Branden Bonaby (3):
  drivers: hv: vmbus: Introduce latency testing
  drivers: hv: vmbus: add fuzz test attributes to debugfs
  tools: hv: add vmbus testing tool

 Documentation/ABI/testing/debugfs-hyperv |  21 ++
 MAINTAINERS  |   1 +
 drivers/hv/Kconfig   |   7 +
 drivers/hv/connection.c  |   3 +
 drivers/hv/hyperv_vmbus.h|  20 ++
 drivers/hv/ring_buffer.c |   7 +
 drivers/hv/vmbus_drv.c   | 167 
 include/linux/hyperv.h   |  21 ++
 tools/hv/vmbus_testing   | 334 +++
 9 files changed, 581 insertions(+)
 create mode 100644 Documentation/ABI/testing/debugfs-hyperv
 create mode 100644 tools/hv/vmbus_testing

-- 
2.17.1



[PATCH v2 1/3] drivers: hv: vmbus: Introduce latency testing

2019-08-19 Thread Branden Bonaby
Introduce user specified latency in the packet reception path.

Signed-off-by: Branden Bonaby 
---
Changes in v2:
 - Add #ifdef in Kconfig file so test code will not interfere
   with non-test code.
 - Move test code functions for delay to hyperv_vmbus header
   file.
 - Wrap test code under #ifdef statement. 

 drivers/hv/Kconfig|  7 +++
 drivers/hv/connection.c   |  3 +++
 drivers/hv/hyperv_vmbus.h | 20 
 drivers/hv/ring_buffer.c  |  7 +++
 include/linux/hyperv.h| 21 +
 5 files changed, 58 insertions(+)

diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
index 9a59957922d4..d97437ba0626 100644
--- a/drivers/hv/Kconfig
+++ b/drivers/hv/Kconfig
@@ -29,4 +29,11 @@ config HYPERV_BALLOON
help
  Select this option to enable Hyper-V Balloon driver.
 
+config HYPERV_TESTING
+bool "Hyper-V testing"
+default n
+depends on HYPERV && DEBUG_FS
+help
+  Select this option to enable Hyper-V vmbus testing.
+
 endmenu
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 09829e15d4a0..c9c63a4033cd 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -357,6 +357,9 @@ void vmbus_on_event(unsigned long data)
 
trace_vmbus_on_event(channel);
 
+#ifdef CONFIG_HYPERV_TESTING
+   hv_debug_delay_test(channel, INTERRUPT_DELAY);
+#endif /* CONFIG_HYPERV_TESTING */
do {
void (*callback_fn)(void *);
 
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 362e70e9d145..edf14f596d8c 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -357,4 +357,24 @@ enum hvutil_device_state {
HVUTIL_DEVICE_DYING, /* driver unload is in progress */
 };
 
+#ifdef CONFIG_HYPERV_TESTING
+#include 
+#include 
+#include 
+#define TESTING "hyperv"
+
+enum delay {
+   INTERRUPT_DELAY = 0,
+   MESSAGE_DELAY   = 1,
+};
+
+int hv_debug_delay_files(struct hv_device *dev, struct dentry *root);
+int hv_debug_add_dev_dir(struct hv_device *dev);
+void hv_debug_rm_dev_dir(struct hv_device *dev);
+void hv_debug_rm_all_dir(void);
+void hv_debug_set_dir_dentry(struct hv_device *dev, struct dentry *root);
+void hv_debug_delay_test(struct vmbus_channel *channel, enum delay delay_type);
+
+#endif /* CONFIG_HYPERV_TESTING */
+
 #endif /* _HYPERV_VMBUS_H */
diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index 9a03b163cbbd..51adda23b398 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -396,6 +396,10 @@ struct vmpacket_descriptor *hv_pkt_iter_first(struct 
vmbus_channel *channel)
struct hv_ring_buffer_info *rbi = >inbound;
struct vmpacket_descriptor *desc;
 
+#ifdef CONFIG_HYPERV_TESTING
+   hv_debug_delay_test(channel, MESSAGE_DELAY);
+#endif /* CONFIG_HYPERV_TESTING */
+
if (hv_pkt_iter_avail(rbi) < sizeof(struct vmpacket_descriptor))
return NULL;
 
@@ -421,6 +425,9 @@ __hv_pkt_iter_next(struct vmbus_channel *channel,
u32 packetlen = desc->len8 << 3;
u32 dsize = rbi->ring_datasize;
 
+#ifdef CONFIG_HYPERV_TESTING
+   hv_debug_delay_test(channel, MESSAGE_DELAY);
+#endif /* CONFIG_HYPERV_TESTING */
/* bump offset to next potential packet */
rbi->priv_read_index += packetlen + VMBUS_PKT_TRAILER;
if (rbi->priv_read_index >= dsize)
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 6256cc34c4a6..6bf8ef5c780c 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -926,6 +926,21 @@ struct vmbus_channel {
 * full outbound ring buffer.
 */
u64 out_full_first;
+
+#ifdef CONFIG_HYPERV_TESTING
+   /* enabling/disabling fuzz testing on the channel (default is false)*/
+   bool fuzz_testing_state;
+
+   /* Interrupt delay will delay the guest from emptying the ring buffer
+* for a specific amount of time. The delay is in microseconds and will
+* be between 1 to a maximum of 1000, its default is 0 (no delay).
+* The  Message delay will delay guest reading on a per message basis
+* in microseconds between 1 to 1000 with the default being 0
+* (no delay).
+*/
+   u32 fuzz_testing_interrupt_delay;
+   u32 fuzz_testing_message_delay;
+#endif /* CONFIG_HYPERV_TESTING */
 };
 
 static inline bool is_hvsock_channel(const struct vmbus_channel *c)
@@ -1166,6 +1181,12 @@ struct hv_device {
 
struct vmbus_channel *channel;
struct kset  *channels_kset;
+
+#ifdef CONFIG_HYPERV_TESTING
+   /* place holder to keep track of the dir for hv device in debugfs */
+   struct dentry *debug_dir;
+#endif /* CONFIG_HYPERV_TESTING */
+
 };
 
 
-- 
2.17.1



Re: [PATCH 1/3] drivers: hv: vmbus: Introduce latency testing

2019-08-02 Thread Branden Bonaby
On Fri, Aug 02, 2019 at 09:32:59AM +0200, Vitaly Kuznetsov wrote:
> Branden Bonaby  writes:
> 
> > Introduce user specified latency in the packet reception path.
> >
> > Signed-off-by: Branden Bonaby 
> > ---
> >  drivers/hv/connection.c  |  5 +
> >  drivers/hv/ring_buffer.c | 10 ++
> >  include/linux/hyperv.h   | 14 ++
> >  3 files changed, 29 insertions(+)
> >
> > diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> > index 09829e15d4a0..2a2c22f5570e 100644
> > --- a/drivers/hv/connection.c
> > +++ b/drivers/hv/connection.c
> > @@ -354,9 +354,14 @@ void vmbus_on_event(unsigned long data)
> >  {
> > struct vmbus_channel *channel = (void *) data;
> > unsigned long time_limit = jiffies + 2;
> > +   struct vmbus_channel *test_channel = !channel->primary_channel ?
> > +   channel :
> > +   channel->primary_channel;
> >  
> > trace_vmbus_on_event(channel);
> >  
> > +   if (unlikely(test_channel->fuzz_testing_buffer_delay > 0))
> > +   udelay(test_channel->fuzz_testing_buffer_delay);
> > do {
> > void (*callback_fn)(void *);
> >  
> > diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
> > index 9a03b163cbbd..d7627c9023d6 100644
> > --- a/drivers/hv/ring_buffer.c
> > +++ b/drivers/hv/ring_buffer.c
> > @@ -395,7 +395,12 @@ struct vmpacket_descriptor *hv_pkt_iter_first(struct 
> > vmbus_channel *channel)
> >  {
> > struct hv_ring_buffer_info *rbi = >inbound;
> > struct vmpacket_descriptor *desc;
> > +   struct vmbus_channel *test_channel = !channel->primary_channel ?
> > +   channel :
> > +   channel->primary_channel;
> >  
> > +   if (unlikely(test_channel->fuzz_testing_message_delay > 0))
> > +   udelay(test_channel->fuzz_testing_message_delay);
> > if (hv_pkt_iter_avail(rbi) < sizeof(struct vmpacket_descriptor))
> > return NULL;
> >  
> > @@ -420,7 +425,12 @@ __hv_pkt_iter_next(struct vmbus_channel *channel,
> > struct hv_ring_buffer_info *rbi = >inbound;
> > u32 packetlen = desc->len8 << 3;
> > u32 dsize = rbi->ring_datasize;
> > +   struct vmbus_channel *test_channel = !channel->primary_channel ?
> > +   channel :
> > +   channel->primary_channel;
> 
> This pattern is repeated 3 times so a define is justified. I would also
> reversed the logic:
> 
>test_channel = channel->primary_channel ? channel->primary_channel : 
> channel;
> 
> >  
> > +   if (unlikely(test_channel->fuzz_testing_message_delay > 0))
> > +   udelay(test_channel->fuzz_testing_message_delay);
> 
> unlikely() is good but if it was under #ifdef it would've been even better.
> 
> > /* bump offset to next potential packet */
> -- 
> Vitaly

Makes sense, I'll address the repeated code and will change the way I
handled that if statement. Using an ifdef CONFIG_HYPERV_TESTING
seems like a good thing to add in here like you suggested.


Re: [PATCH 2/3] drivers: hv: vmbus: add fuzz test attributes to sysfs

2019-08-02 Thread Branden Bonaby
On Fri, Aug 02, 2019 at 09:34:40AM +0200, Vitaly Kuznetsov wrote:
> Branden Bonaby  writes:
> 
> > Expose the test parameters as part of the sysfs channel attributes.
> > We will control the testing state via these attributes.
> >
> > Signed-off-by: Branden Bonaby 
> > ---
> >  Documentation/ABI/stable/sysfs-bus-vmbus | 22 ++
> >  drivers/hv/vmbus_drv.c   | 97 +++-
> >  2 files changed, 118 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/ABI/stable/sysfs-bus-vmbus 
> > b/Documentation/ABI/stable/sysfs-bus-vmbus
> > index 8e8d167eca31..239fcb6fdc75 100644
> > --- a/Documentation/ABI/stable/sysfs-bus-vmbus
> > +++ b/Documentation/ABI/stable/sysfs-bus-vmbus
> > @@ -185,3 +185,25 @@ Contact:Michael Kelley 
> >  Description:Total number of write operations that encountered an 
> > outbound
> > ring buffer full condition
> >  Users:  Debugging tools
> > +
> > +What:   /sys/bus/vmbus/devices//fuzz_test_state
> 
> I would prefer this to go under /sys/kernel/debug/ as this is clearly a
> debug/test feature.
> -- 
> Vitaly

Alright, it is testing so I see what you mean and why the code in 
this patch should go in debugfs. Will fix that and resend.

Branden Bonaby 


Re: [PATCH 0/3] hv: vmbus: add fuzz testing to hv devices

2019-08-02 Thread Branden Bonaby
On Fri, Aug 02, 2019 at 09:30:18AM +0200, Vitaly Kuznetsov wrote:
> Branden Bonaby  writes:
> 
> > This patchset introduces a testing framework for Hyper-V drivers.
> > This framework allows us to introduce delays in the packet receive
> > path on a per-device basis. While the current code only supports 
> > introducing arbitrary delays in the host/guest communication path,
> > we intend to expand this to support error injection in the future.
> >
> > Branden Bonaby (3):
> >   drivers: hv: vmbus: Introduce latency testing
> >   drivers: hv: vmbus: add fuzz test attributes to sysfs
> >   tools: hv: add vmbus testing tool
> >
> >  Documentation/ABI/stable/sysfs-bus-vmbus |  22 ++
> >  drivers/hv/connection.c  |   5 +
> >  drivers/hv/ring_buffer.c |  10 +
> >  drivers/hv/vmbus_drv.c   |  97 ++-
> 
> Can we have something like CONFIG_HYPERV_TESTING and put this new 
> code under #ifdef?
> 
> >  include/linux/hyperv.h   |  14 +
> >  tools/hv/vmbus_testing   | 326 +++
> >  6 files changed, 473 insertions(+), 1 deletion(-)
> >  create mode 100644 tools/hv/vmbus_testing
> 
> -- 
> Vitaly

You're right, it would be better to do it that way with ifdef's.
Will edit my patches and resend.

Branden Bonaby


[PATCH 3/3] tools: hv: add vmbus testing tool

2019-08-01 Thread Branden Bonaby
This is a userspace tool to drive the testing. Currently it supports
introducing user specified delay in the host to guest communication
path on a per-channel basis.

Signed-off-by: Branden Bonaby 
---
 tools/hv/vmbus_testing | 326 +
 1 file changed, 326 insertions(+)
 create mode 100644 tools/hv/vmbus_testing

diff --git a/tools/hv/vmbus_testing b/tools/hv/vmbus_testing
new file mode 100644
index ..8f5cd997952b
--- /dev/null
+++ b/tools/hv/vmbus_testing
@@ -0,0 +1,326 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+#
+#Program to allow users to fuzz test Hyper-V drivers
+#by interfacing with Hyper-V sysfs directories
+#author: Branden Bonaby 
+
+import os
+import cmd
+import argparse
+from collections import defaultdict
+from argparse import RawDescriptionHelpFormatter
+
+#sysfs paths for vmbus must exist (same as in lsvmbus)
+vmbus_sys_path = '/sys/bus/vmbus/devices'
+if not os.path.isdir(vmbus_sys_path):
+print("{} doesn't exist: exiting...".format(vmbus_sys_path))
+exit(-1)
+
+#Do not change unless, you change the sysfs attributes
+#in "/sys/bus/vmbus/devices//". All fuzz testing
+#attributes will start with "fuzz_test".
+pathlen = len(vmbus_sys_path)
+fuzz_state_location = "fuzz_test_state"
+fuzz_states = {0 : "Disable", 1 : "Enable"}
+fuzz_methods ={1 : "Delay_testing"}
+fuzz_delay_types = {1 : "fuzz_test_buffer_delay", 2 :"fuzz_test_message_delay"}
+
+def parse_args():
+parser = argparse.ArgumentParser(description = "vmbus_testing "\
+"[-s] [0|1] [-q] [-p] \n""vmbus_testing [-s]"\
+" [0|1] [-q] [-p]  delay [-d] [val] [val] 
[-E|-D]\n"
+"vmbus_testing [-q] disable-all\n"
+"vmbus_testing [-q] view [-v|-V]\n"\
+"vmbus_testing --version",
+epilog = "Current testing options {}".format(fuzz_methods),
+prog = 'vmbus_testing',
+formatter_class = RawDescriptionHelpFormatter)
+subparsers = parser.add_subparsers(dest="action")
+parser.add_argument('--version', action='version',\
+version = '%(prog)s 1.0')
+parser.add_argument("-q","--quiet",action = "store_true",\
+help = "silence none important test messages")
+parser.add_argument("-s","--state",metavar = "", type = int,\
+choices = range(0,2),\
+help = "Turn testing ON or OFF for a single device."\
+" The value (1) will turn testing ON. The value"\
+" of (0) will turn testing OFF with the default set"\
+" to (0).")
+parser.add_argument("-p","--path", metavar = "",\
+help = "Refers to the sysfs path to a vmbus device."
+" If the path is not a valid path to a sysfs device,"\
+" the program will exit. The path must be the"\
+" absolute path; use the lsvmbus command to find"\
+" the path.")
+parser_delay = subparsers.add_parser("delay",\
+help = "Delay buffer/message reads in microseconds.",
+description = "vmbus_testing -s [0|1] [-q] -p "\
+" delay -d "\
+"[buffer-delay-value] [message-delay-value]\n"
+"vmbus_testing [-q] delay [buffer-delay-value] "\
+"[message-delay-value] -E\n"
+"vmbus_testing [-q] delay [buffer-delay-value] "\
+"[message-delay-value] -D",
+formatter_class = RawDescriptionHelpFormatter)
+delay_group = parser_delay.add_mutually_exclusive_group()
+delay_group.add_argument("-E","--en-all-delay", action = "store_true",\
+help = "Enable Buffer/Message Delay testing on ALL"\
+" devices. Use -d option with this to set the values"\
+" for both the buffer delay and the message delay. No"\
+" value can be (0) or less than (-1). If testing is"\
+" disabled on a device prior to running this command,"\
+" testing will be enabled on the device as a result"\
+   

[PATCH 2/3] drivers: hv: vmbus: add fuzz test attributes to sysfs

2019-08-01 Thread Branden Bonaby
Expose the test parameters as part of the sysfs channel attributes.
We will control the testing state via these attributes.

Signed-off-by: Branden Bonaby 
---
 Documentation/ABI/stable/sysfs-bus-vmbus | 22 ++
 drivers/hv/vmbus_drv.c   | 97 +++-
 2 files changed, 118 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/stable/sysfs-bus-vmbus 
b/Documentation/ABI/stable/sysfs-bus-vmbus
index 8e8d167eca31..239fcb6fdc75 100644
--- a/Documentation/ABI/stable/sysfs-bus-vmbus
+++ b/Documentation/ABI/stable/sysfs-bus-vmbus
@@ -185,3 +185,25 @@ Contact:Michael Kelley 
 Description:Total number of write operations that encountered an outbound
ring buffer full condition
 Users:  Debugging tools
+
+What:   /sys/bus/vmbus/devices//fuzz_test_state
+Date:   July 2019
+KernelVersion:  5.2
+Contact:Branden Bonaby 
+Description:Fuzz testing status of a vmbus device, whether its in an ON
+state or a OFF state
+Users:  Debugging tools
+
+What:   /sys/bus/vmbus/devices//fuzz_test_buffer_delay
+Date:   July 2019
+KernelVersion:  5.2
+Contact:Branden Bonaby 
+Description:Fuzz testing buffer delay value between 0 - 1000
+Users:  Debugging tools
+
+What:   /sys/bus/vmbus/devices//fuzz_test_message_delay
+Date:   July 2019
+KernelVersion:  5.2
+Contact:Branden Bonaby 
+Description:Fuzz testing message delay value between 0 - 1000
+Users:  Debugging tools
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 92b1874b3eb3..0c71fd66ef81 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -22,7 +22,7 @@
 #include 
 #include 
 #include 
-
+#include 
 #include 
 #include 
 #include 
@@ -584,6 +584,98 @@ static ssize_t driver_override_show(struct device *dev,
 }
 static DEVICE_ATTR_RW(driver_override);
 
+static ssize_t fuzz_test_state_store(struct device *dev,
+struct device_attribute *attr,
+const char *buf, size_t count)
+{
+   struct hv_device *hv_dev = device_to_hv_device(dev);
+   struct vmbus_channel *channel = hv_dev->channel;
+   int state;
+   int delay = kstrtoint(buf, 0, );
+
+   if (delay)
+   return count;
+   if (state)
+   channel->fuzz_testing_state = 1;
+   else
+   channel->fuzz_testing_state = 0;
+   return count;
+}
+
+static ssize_t fuzz_test_state_show(struct device *dev,
+   struct device_attribute *dev_attr,
+   char *buf)
+{
+   struct hv_device *hv_dev = device_to_hv_device(dev);
+   struct vmbus_channel *channel = hv_dev->channel;
+
+   return sprintf(buf, "%u\n", channel->fuzz_testing_state);
+}
+static DEVICE_ATTR_RW(fuzz_test_state);
+
+static ssize_t fuzz_test_buffer_delay_store(struct device *dev,
+   struct device_attribute *attr,
+   const char *buf, size_t count)
+{
+   struct hv_device *hv_dev = device_to_hv_device(dev);
+   struct vmbus_channel *channel = hv_dev->channel;
+   int val;
+   int delay = kstrtoint(buf, 0, );
+
+   if (delay)
+   return count;
+   if (val >= 1 && val <= 1000)
+   channel->fuzz_testing_buffer_delay = val;
+   /*Best to not use else statement here since we want
+*the buffer delay to remain the same if val > 1000
+*/
+   else if (val <= 0)
+   channel->fuzz_testing_buffer_delay = 0;
+   return count;
+}
+
+static ssize_t fuzz_test_buffer_delay_show(struct device *dev,
+  struct device_attribute *dev_attr,
+  char *buf)
+{
+   struct hv_device *hv_dev = device_to_hv_device(dev);
+   struct vmbus_channel *channel = hv_dev->channel;
+
+   return sprintf(buf, "%u\n", channel->fuzz_testing_buffer_delay);
+}
+static DEVICE_ATTR_RW(fuzz_test_buffer_delay);
+
+static ssize_t fuzz_test_message_delay_store(struct device *dev,
+struct device_attribute *attr,
+const char *buf, size_t count)
+{
+   struct hv_device *hv_dev = device_to_hv_device(dev);
+   struct vmbus_channel *channel = hv_dev->channel;
+   int val;
+   int delay = kstrtoint(buf, 0, );
+
+   if (delay)
+   return count;
+   if (val >= 1 && val <= 1000)
+   channel->fuzz_testing_message_delay = val;
+   /*Best to not use else statement here since we want
+*the message delay to remain the same if val > 1000
+*/
+   else if (val <= 0)
+   channel->fuzz_testin

[PATCH 1/3] drivers: hv: vmbus: Introduce latency testing

2019-08-01 Thread Branden Bonaby
Introduce user specified latency in the packet reception path.

Signed-off-by: Branden Bonaby 
---
 drivers/hv/connection.c  |  5 +
 drivers/hv/ring_buffer.c | 10 ++
 include/linux/hyperv.h   | 14 ++
 3 files changed, 29 insertions(+)

diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 09829e15d4a0..2a2c22f5570e 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -354,9 +354,14 @@ void vmbus_on_event(unsigned long data)
 {
struct vmbus_channel *channel = (void *) data;
unsigned long time_limit = jiffies + 2;
+   struct vmbus_channel *test_channel = !channel->primary_channel ?
+   channel :
+   channel->primary_channel;
 
trace_vmbus_on_event(channel);
 
+   if (unlikely(test_channel->fuzz_testing_buffer_delay > 0))
+   udelay(test_channel->fuzz_testing_buffer_delay);
do {
void (*callback_fn)(void *);
 
diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index 9a03b163cbbd..d7627c9023d6 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -395,7 +395,12 @@ struct vmpacket_descriptor *hv_pkt_iter_first(struct 
vmbus_channel *channel)
 {
struct hv_ring_buffer_info *rbi = >inbound;
struct vmpacket_descriptor *desc;
+   struct vmbus_channel *test_channel = !channel->primary_channel ?
+   channel :
+   channel->primary_channel;
 
+   if (unlikely(test_channel->fuzz_testing_message_delay > 0))
+   udelay(test_channel->fuzz_testing_message_delay);
if (hv_pkt_iter_avail(rbi) < sizeof(struct vmpacket_descriptor))
return NULL;
 
@@ -420,7 +425,12 @@ __hv_pkt_iter_next(struct vmbus_channel *channel,
struct hv_ring_buffer_info *rbi = >inbound;
u32 packetlen = desc->len8 << 3;
u32 dsize = rbi->ring_datasize;
+   struct vmbus_channel *test_channel = !channel->primary_channel ?
+   channel :
+   channel->primary_channel;
 
+   if (unlikely(test_channel->fuzz_testing_message_delay > 0))
+   udelay(test_channel->fuzz_testing_message_delay);
/* bump offset to next potential packet */
rbi->priv_read_index += packetlen + VMBUS_PKT_TRAILER;
if (rbi->priv_read_index >= dsize)
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 6256cc34c4a6..8d068956dd67 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define MAX_PAGE_BUFFER_COUNT  32
 #define MAX_MULTIPAGE_BUFFER_COUNT 32 /* 128K */
@@ -926,6 +927,19 @@ struct vmbus_channel {
 * full outbound ring buffer.
 */
u64 out_full_first;
+
+   /* enabling/disabling fuzz testing on the channel (default is false)*/
+   bool fuzz_testing_state;
+
+   /* Buffer delay will delay the guest from emptying the ring buffer
+* for a specific amount of time. The delay is in microseconds and will
+* be between 1 to a maximum of 1000, its default is 0 (no delay).
+* The  Message delay will delay guest reading on a per message basis
+* in microseconds between 1 to 1000 with the default being 0
+* (no delay).
+*/
+   u32 fuzz_testing_buffer_delay;
+   u32 fuzz_testing_message_delay;
 };
 
 static inline bool is_hvsock_channel(const struct vmbus_channel *c)
-- 
2.17.1



[PATCH 0/3] hv: vmbus: add fuzz testing to hv devices

2019-08-01 Thread Branden Bonaby
This patchset introduces a testing framework for Hyper-V drivers.
This framework allows us to introduce delays in the packet receive
path on a per-device basis. While the current code only supports 
introducing arbitrary delays in the host/guest communication path,
we intend to expand this to support error injection in the future.

Branden Bonaby (3):
  drivers: hv: vmbus: Introduce latency testing
  drivers: hv: vmbus: add fuzz test attributes to sysfs
  tools: hv: add vmbus testing tool

 Documentation/ABI/stable/sysfs-bus-vmbus |  22 ++
 drivers/hv/connection.c  |   5 +
 drivers/hv/ring_buffer.c |  10 +
 drivers/hv/vmbus_drv.c   |  97 ++-
 include/linux/hyperv.h   |  14 +
 tools/hv/vmbus_testing   | 326 +++
 6 files changed, 473 insertions(+), 1 deletion(-)
 create mode 100644 tools/hv/vmbus_testing

-- 
2.17.1



Re: [PATCH] scsi: storvsc: Add ability to change scsi queue depth

2019-06-18 Thread Branden Bonaby
On Tue, Jun 18, 2019 at 09:08:59PM -0400, Martin K. Petersen wrote:
> 
> Branden,
> 
> > Adding functionality to allow the SCSI queue depth to be changed, by
> > utilizing the "scsi_change_queue_depth" function.
> 
> Applied to 5.3/scsi-queue. Please run checkpatch before submission. I
> fixed it up this time.
> 
> Thanks!

Oh I see what you mean about the brackets, thanks will do next time.


[PATCH] scsi: storvsc: Add ability to change scsi queue depth

2019-06-14 Thread Branden Bonaby
Adding functionality to allow the SCSI queue depth to be changed,
by utilizing the "scsi_change_queue_depth" function.

Signed-off-by: Branden Bonaby 
---
 drivers/scsi/storvsc_drv.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 8472de1007ff..719ca9906fc2 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -387,6 +387,7 @@ enum storvsc_request_type {
 
 static int storvsc_ringbuffer_size = (128 * 1024);
 static u32 max_outstanding_req_per_channel;
+static int storvsc_change_queue_depth(struct scsi_device *sdev, int 
queue_depth);
 
 static int storvsc_vcpus_per_sub_channel = 4;
 
@@ -1711,6 +1712,7 @@ static struct scsi_host_template scsi_driver = {
.dma_boundary = PAGE_SIZE-1,
.no_write_same =1,
.track_queue_depth =1,
+   .change_queue_depth =   storvsc_change_queue_depth,
 };
 
 enum {
@@ -1917,6 +1919,15 @@ static int storvsc_probe(struct hv_device *device,
return ret;
 }
 
+/* Change a scsi target's queue depth */
+static int storvsc_change_queue_depth(struct scsi_device *sdev, int 
queue_depth)
+{
+   if (queue_depth > scsi_driver.can_queue){
+   queue_depth = scsi_driver.can_queue;
+   }
+   return scsi_change_queue_depth(sdev, queue_depth);
+}
+
 static int storvsc_remove(struct hv_device *dev)
 {
struct storvsc_device *stor_device = hv_get_drvdata(dev);
-- 
2.17.1