RE: [PATCH] HV: properly delay KVP packets when negotiation is in progress

[Long Li]

> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuznets@xxxxxxxxxx]
> Sent: Friday, March 17, 2017 9:16 AM
> To: Long Li <longli@xxxxxxxxxxxxx>
> Cc: KY Srinivasan <kys@xxxxxxxxxxxxx>; Haiyang Zhang
> <haiyangz@xxxxxxxxxxxxx>; Stephen Hemminger
> <sthemmin@xxxxxxxxxxxxx>; devel@xxxxxxxxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] HV: properly delay KVP packets when negotiation is in
> progress
> 
> Long Li <longli@xxxxxxxxxxxxx> writes:
> 
> > The host may send multiple KVP packets before the negotiation with
> > daemon is finished. We need to keep those packets in ring buffer until
> > the daemon is negotiated and connected.
> 
> The patch looks OK but previously we always presumed that this can't
> happen for util drivers and host will never send a new request before we
> answer to the previous one. If this is not true we may have more issues
> which need fixing as all three drivers we have are written in a 'transaction'
> fashion.
> 
> So my question would be: can the host send multiple (KVP) packets _after_
> the negotiation with daemon is finished?

Thanks Vitaly. I'm checking with Windows guys and will update soon.

> 
> 
> >
> > This patch is based on the work of Nick Meier
> > <Nick.Meier@xxxxxxxxxxxxx>
> >
> > Signed-off-by: Long Li <longli@xxxxxxxxxxxxx>
> > ---
> >  drivers/hv/hv_kvp.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c index
> > de26371..b9f928d 100644
> > --- a/drivers/hv/hv_kvp.c
> > +++ b/drivers/hv/hv_kvp.c
> > @@ -628,16 +628,17 @@ void hv_kvp_onchannelcallback(void *context)
> >  		     NEGO_IN_PROGRESS,
> >  		     NEGO_FINISHED} host_negotiatied =
> NEGO_NOT_STARTED;
> >
> > -	if (host_negotiatied == NEGO_NOT_STARTED &&
> > -	    kvp_transaction.state < HVUTIL_READY) {
> > +	if (kvp_transaction.state < HVUTIL_READY) {
> >  		/*
> >  		 * If userspace daemon is not connected and host is asking
> >  		 * us to negotiate we need to delay to not lose messages.
> >  		 * This is important for Failover IP setting.
> >  		 */
> > -		host_negotiatied = NEGO_IN_PROGRESS;
> > -		schedule_delayed_work(&kvp_host_handshake_work,
> > +		if (host_negotiatied == NEGO_NOT_STARTED) {
> > +			host_negotiatied = NEGO_IN_PROGRESS;
> > +
> 	schedule_delayed_work(&kvp_host_handshake_work,
> >  				      HV_UTIL_NEGO_TIMEOUT * HZ);
> > +		}
> >  		return;
> >  	}
> >  	if (kvp_transaction.state > HVUTIL_READY)
> 
> --
>   Vitaly