netdev - Re: [PATCH V2 2/3 net-next] Drivers: hv: vmbus: Fix a bug in the signalling logic with kick

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] [thread-next>] [day] [month] [year] [list]

Message-Id: <1426129662.12154.2@smtp.corp.redhat.com>
Date:	Thu, 12 Mar 2015 11:07:42 +0800
From:	Jason Wang <jasowang@...hat.com>
To:	"K. Y. Srinivasan" <kys@...rosoft.com>
Cc:	davem@...emloft.net, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, devel@...uxdriverproject.org,
	olaf@...fle.de, apw@...onical.com, gregkh@...uxfoundation.org,
	"K. Y. Srinivasan" <kys@...rosoft.com>
Subject: Re: [PATCH V2 2/3 net-next] Drivers: hv: vmbus: Fix a bug in the
 signalling logic with kick_q



On Thu, Mar 12, 2015 at 3:04 AM, K. Y. Srinivasan <kys@...rosoft.com> 
wrote:
> When the caller specifies that signalling should be deferred, we need 
> to
> address the case where we are not able to place the current packet 
> because
> the buffer is full. In this case, we will signal the host as some 
> packets
> may have been placed on the ring buffer.
> I would like to thank Jason Wang <jasowang@...hat.com> for pointing
> out this issue.
> 
> Signed-off-by: K. Y. Srinivasan <kys@...rosoft.com>
> ---
>  drivers/hv/channel.c |   32 ++++++++++++++++++++++++++++++++
>  1 files changed, 32 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> index e58cdb7..ae06ba9 100644
> --- a/drivers/hv/channel.c
> +++ b/drivers/hv/channel.c
> @@ -614,8 +614,24 @@ int vmbus_sendpacket_ctl(struct vmbus_channel 
> *channel, void *buffer,
>  
>  	ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, 
> &signal);
>  
> +	/*
> +	 * Here is the logic for signalling the host:
> +	 * 1. If the host is already draining the ringbuffer,
> +	 *    don't signal. This is indicated by the parameter
> +	 *    "signal".
> +	 *
> +	 * 2. If we are not able to write, signal if kick_q is false.
> +	 *    kick_q being false indicates that we may have placed zero or
> +	 *    more packets with more packets to come. We will signal in
> +	 *    this case even if potentially we may have not placed any
> +	 *    packet. This is a rare enough condition that it should not
> +	 *    matter.
> +	 */
> +
>  	if ((ret == 0) && kick_q && signal)
>  		vmbus_setevent(channel);
> +	else if ((ret != 0) && !kick_q)
> +		vmbus_setevent(channel);
>  
>  	return ret;
>  }
> @@ -705,8 +721,24 @@ int vmbus_sendpacket_pagebuffer_ctl(struct 
> vmbus_channel *channel,
>  
>  	ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, 
> &signal);
>  
> +	/*
> +	 * Here is the logic for signalling the host:
> +	 * 1. If the host is already draining the ringbuffer,
> +	 *    don't signal. This is indicated by the parameter
> +	 *    "signal".
> +	 *
> +	 * 2. If we are not able to write, signal if kick_q is false.
> +	 *    kick_q being false indicates that we may have placed zero or
> +	 *    more packets with more packets to come. We will signal in
> +	 *    this case even if potentially we may have not placed any
> +	 *    packet. This is a rare enough condition that it should not
> +	 *    matter.
> +	 */
> +
>  	if ((ret == 0) && kick_q && signal)
>  		vmbus_setevent(channel);
> +	else if ((ret != 0) && !kick_q)
> +		vmbus_setevent(channel);
>  
>  	return ret;
>  }
> -- 

Looks like we need to kick unconditionally here. Consider we may get 
-EAGAIN when we want to send the last skb (kick_q is true) from the 
list. We need kick host in this case.

Btw, another method is let the driver to decide e.g exporting the 
vmbus_setevent() and call it in netvsc_start_xmit().




--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html