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
Introduce user specified latency in the packet reception path.
Signed-off-by: Branden Bonaby <[email protected]>
---
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 = &channel->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 = &channel->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 <linux/mod_devicetable.h>
#include <linux/interrupt.h>
#include <linux/reciprocal_div.h>
+#include <linux/delay.h>
#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
Branden Bonaby <[email protected]> writes:
> Introduce user specified latency in the packet reception path.
>
> Signed-off-by: Branden Bonaby <[email protected]>
> ---
> 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 = &channel->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 = &channel->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 */
> 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 <linux/mod_devicetable.h>
> #include <linux/interrupt.h>
> #include <linux/reciprocal_div.h>
> +#include <linux/delay.h>
>
> #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)
--
Vitaly
Branden Bonaby <[email protected]> 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
On Fri, Aug 02, 2019 at 09:30:18AM +0200, Vitaly Kuznetsov wrote:
> Branden Bonaby <[email protected]> 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
On Fri, Aug 02, 2019 at 09:32:59AM +0200, Vitaly Kuznetsov wrote:
> Branden Bonaby <[email protected]> writes:
>
> > Introduce user specified latency in the packet reception path.
> >
> > Signed-off-by: Branden Bonaby <[email protected]>
> > ---
> > 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 = &channel->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 = &channel->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.