linux-kernel - RE: [PATCH v2] HV: properly delay KVP packets when negotiation is in progress

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <BN3PR03MB2227B0BCD74A31B362F86259CE3F0@BN3PR03MB2227.namprd03.prod.outlook.com>
Date:   Thu, 23 Mar 2017 17:47:13 +0000
From:   Long Li <longli@...rosoft.com>
To:     Vitaly Kuznetsov <vkuznets@...hat.com>
CC:     KY Srinivasan <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        "devel@...uxdriverproject.org" <devel@...uxdriverproject.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: RE: [PATCH v2] HV: properly delay KVP packets when negotiation is in
 progress



> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuznets@...hat.com]
> Sent: Thursday, March 23, 2017 9:04 AM
> To: Long Li <longli@...rosoft.com>
> Cc: KY Srinivasan <kys@...rosoft.com>; Haiyang Zhang
> <haiyangz@...rosoft.com>; Stephen Hemminger
> <sthemmin@...rosoft.com>; devel@...uxdriverproject.org; linux-
> kernel@...r.kernel.org; stable@...r.kernel.org
> Subject: Re: [PATCH v2] HV: properly delay KVP packets when negotiation is
> in progress
> 
> Long Li <longli@...rosoft.com> writes:
> 
> > The host may send multiple negotiation packets (due to timeout) before
> > the KVP user-mode daemon is connected. We need to defer processing
> > those packets until the daemon is negotiated and connected. It's okay
> > for guest to respond to all negotiation packets.
> >
> > In addition, the host may send multiple staged KVP requests as soon as
> > negotiation is done. We need to properly process those packets using
> > one tasklet for exclusive access to ring buffer.
> >
> > This patch is based on the work of Nick Meier
> > <Nick.Meier@...rosoft.com>
> >
> > The patch v2 has incorporated suggestion from Vitaly Kuznetsov
> > <vkuznets@...hat.com>.
> >
> > Signed-off-by: Long Li <longli@...rosoft.com>
> > ---
> >  drivers/hv/hv_kvp.c | 12 +++++++-----
> >  1 file changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c index
> > de26371..845b70b 100644
> > --- a/drivers/hv/hv_kvp.c
> > +++ b/drivers/hv/hv_kvp.c
> > @@ -113,7 +113,7 @@ static void kvp_poll_wrapper(void *channel)  {
> >  	/* Transaction is finished, reset the state here to avoid races. */
> >  	kvp_transaction.state = HVUTIL_READY;
> > -	hv_kvp_onchannelcallback(channel);
> > +	tasklet_schedule(&((struct vmbus_channel*)channel)-
> >callback_event);
> >  }
> 
> There is one more function in the code which calls
> hv_kvp_onchannelcallback():
> 
> static void kvp_host_handshake_func(struct work_struct *dummy) {
> 	hv_poll_channel(kvp_transaction.recv_channel,
> hv_kvp_onchannelcallback); }
> 
> we can't replace hv_kvp_onchannelcallback with kvp_poll_wrapper here as
> we don't want to reset kvp_transaction.state but it seems this should also
> get updated, e.g. hv_poll_channel() here can be replaced with the direct
> 
>  tasklet_schedule(&((struct vmbus_channel*)channel)->callback_event);
> 
> call. This will ensure hv_kvp_onchannelcallback() calls are always serialized.

Thank you. I will send v3.

> 
> >
> >  static void kvp_register_done(void)
> > @@ -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) @@ -705,6 +706,7 @@
> void
> > hv_kvp_onchannelcallback(void *context)
> >  				       VM_PKT_DATA_INBAND, 0);
> >
> >  		host_negotiatied = NEGO_FINISHED;
> > +		hv_poll_channel(kvp_transaction.recv_channel,
> kvp_poll_wrapper);
> >  	}
> >
> >  }
> 
> --
>   Vitaly