Re: [PATCH 1/3] drivers: hv: vmbus: Introduce latency testing
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 1/3] drivers: hv: vmbus: Introduce latency testing
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 */
> 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_COUNT32
> #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
[PATCH 1/3] drivers: hv: vmbus: Introduce latency testing
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
