RE: [PATCH v3 08/12] Drivers: hv: vmbus: Ignore the offers when resuming from hibernation

[Michael Kelley <mikelley@microsoft.com>]

From: Dexuan Cui <decui@microsoft.com>  Sent: Monday, August 19, 2019 6:52 PM
> 
> When the VM resumes, the host re-sends the offers. We should not add the
> offers to the global vmbus_connection.chn_list again.
> 
> This patch assumes the RELIDs of the channels don't change across
> hibernation. Actually this is not always true, especially in the case of
> NIC SR-IOV the VF vmbus device's RELID sometimes can change. A later patch
> will address this issue by mapping the new offers to the old channels and
> fixing up the old channels, if necessary.
> 
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
>  drivers/hv/channel_mgmt.c | 29 ++++++++++++++++++++++++++++-
>  drivers/hv/connection.c   | 27 +++++++++++++++++++++++++++
>  drivers/hv/hyperv_vmbus.h |  3 +++
>  3 files changed, 58 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index addcef5..f7a1184 100644
> --- a/drivers/hv/channel_mgmt.c
> +++ b/drivers/hv/channel_mgmt.c
> @@ -854,12 +854,39 @@ void vmbus_initiate_unload(bool crash)
>  static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
>  {
>  	struct vmbus_channel_offer_channel *offer;
> -	struct vmbus_channel *newchannel;
> +	struct vmbus_channel *oldchannel, *newchannel;
> +	size_t offer_sz;
> 
>  	offer = (struct vmbus_channel_offer_channel *)hdr;
> 
>  	trace_vmbus_onoffer(offer);
> 
> +	mutex_lock(&vmbus_connection.channel_mutex);
> +	oldchannel = find_primary_channel_by_offer(offer);
> +	mutex_unlock(&vmbus_connection.channel_mutex);
> +
> +	if (oldchannel != NULL) {
> +		atomic_dec(&vmbus_connection.offer_in_progress);
> +
> +		/*
> +		 * We're resuming from hibernation: we expect the host to send
> +		 * exactly the same offers that we had before the hibernation.
> +		 */
> +		offer_sz = sizeof(*offer);
> +		if (memcmp(offer, &oldchannel->offermsg, offer_sz) == 0)
> +			return;
> +
> +		pr_debug("Mismatched offer from the host (relid=%d)\n",
> +			 offer->child_relid);
> +
> +		print_hex_dump_debug("Old vmbus offer: ", DUMP_PREFIX_OFFSET,
> +				     16, 4, &oldchannel->offermsg, offer_sz,
> +				     false);
> +		print_hex_dump_debug("New vmbus offer: ", DUMP_PREFIX_OFFSET,
> +				     16, 4, offer, offer_sz, false);
> +		return;
> +	}
> +
>  	/* Allocate the channel object and save this offer. */
>  	newchannel = alloc_channel();
>  	if (!newchannel) {
> diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> index 09829e1..6c7a983 100644
> --- a/drivers/hv/connection.c
> +++ b/drivers/hv/connection.c
> @@ -337,6 +337,33 @@ struct vmbus_channel *relid2channel(u32 relid)
>  }
> 
>  /*
> + * find_primary_channel_by_offer - Get the channel object given the new offer.
> + * This is only used in the resume path of hibernation.
> + */
> +struct vmbus_channel *
> +find_primary_channel_by_offer(const struct vmbus_channel_offer_channel *offer)
> +{
> +	struct vmbus_channel *channel;
> +	const guid_t *inst1, *inst2;
> +
> +	WARN_ON(!mutex_is_locked(&vmbus_connection.channel_mutex));
> +
> +	/* Ignore sub-channel offers. */
> +	if (offer->offer.sub_channel_index != 0)
> +		return NULL;
> +
> +	list_for_each_entry(channel, &vmbus_connection.chn_list, listentry) {
> +		inst1 = &channel->offermsg.offer.if_instance;
> +		inst2 = &offer->offer.if_instance;
> +
> +		if (guid_equal(inst1, inst2))
> +			return channel;
> +	}
> +
> +	return NULL;
> +}

Any particular reason this new function is in connection.c instead of
putting it in channel_mgmt.c where it is called?

> +
> +/*
>   * vmbus_on_event - Process a channel event notification
>   *
>   * For batched channels (default) optimize host to guest signaling
> diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> index 9f7fb6d..c42b46d 100644
> --- a/drivers/hv/hyperv_vmbus.h
> +++ b/drivers/hv/hyperv_vmbus.h
> @@ -310,6 +310,9 @@ int vmbus_add_channel_kobj(struct hv_device *device_obj,
> 
>  struct vmbus_channel *relid2channel(u32 relid);
> 
> +struct vmbus_channel *
> +find_primary_channel_by_offer(const struct vmbus_channel_offer_channel *offer);
> +
>  void vmbus_free_channels(void);
> 
>  /* Connection interface */
> --
> 1.8.3.1