Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 8 Dec 2020 05:50:50 +0100
From:   Andrea Parri <parri.andrea@gmail.com>
To:     Michael Kelley <mikelley@microsoft.com>
Cc:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        KY Srinivasan <kys@microsoft.com>,
        Haiyang Zhang <haiyangz@microsoft.com>,
        Stephen Hemminger <sthemmin@microsoft.com>,
        Wei Liu <wei.liu@kernel.org>,
        "linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>,
        Andres Beltran <lkmlabelt@gmail.com>,
        Saruhan Karademir <skarade@microsoft.com>,
        Juan Vazquez <juvazq@microsoft.com>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        "James E.J. Bottomley" <jejb@linux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@oracle.com>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>
Subject: Re: [PATCH v2] Drivers: hv: vmbus: Copy packets sent by Hyper-V out
 of the ring buffer
Message-ID: <20201208045050.GA9609@andrea>
References: <20201109100727.9207-1-parri.andrea@gmail.com>
 <MW2PR2101MB1052B7CBB14283AA066BF125D7CF1@MW2PR2101MB1052.namprd21.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <MW2PR2101MB1052B7CBB14283AA066BF125D7CF1@MW2PR2101MB1052.namprd21.prod.outlook.com>
Precedence: bulk

> > @@ -419,17 +446,52 @@ static u32 hv_pkt_iter_avail(const struct hv_ring_buffer_info *rbi)
> >  struct vmpacket_descriptor *hv_pkt_iter_first(struct vmbus_channel *channel)
> >  {
> >  	struct hv_ring_buffer_info *rbi = &channel->inbound;
> > -	struct vmpacket_descriptor *desc;
> > +	struct vmpacket_descriptor *desc, *desc_copy;
> > +	u32 bytes_avail, pkt_len, pkt_offset;
> > 
> > -	hv_debug_delay_test(channel, MESSAGE_DELAY);
> > -	if (hv_pkt_iter_avail(rbi) < sizeof(struct vmpacket_descriptor))
> > +	desc = hv_pkt_iter_first_raw(channel);
> > +	if (!desc)
> >  		return NULL;
> > 
> > -	desc = hv_get_ring_buffer(rbi) + rbi->priv_read_index;
> > -	if (desc)
> > -		prefetch((char *)desc + (desc->len8 << 3));
> > +	bytes_avail = hv_pkt_iter_avail(rbi);
> > +
> > +	/*
> > +	 * Ensure the compiler does not use references to incoming Hyper-V values (which
> > +	 * could change at any moment) when reading local variables later in the code
> > +	 */
> > +	pkt_len = READ_ONCE(desc->len8) << 3;
> > +	pkt_offset = READ_ONCE(desc->offset8) << 3;
> > +
> > +	/*
> > +	 * If pkt_len is invalid, set it to the smaller of hv_pkt_iter_avail() and
> > +	 * rbi->pkt_buffer_size
> > +	 */
> > +	if (rbi->pkt_buffer_size < bytes_avail)
> > +		bytes_avail = rbi->pkt_buffer_size;
> 
> I think the above could be combined with the earlier call to hv_pkt_iter_avail(),
> and more logically expressed as:
> 
> 	bytes_avail = min(rbi->pkt_buffer_size, hv_pkt_iter_avail(rbi));
> 
> 
> This is a minor nit.  Everything else in this patch looks good to me.

Thanks for the feedback, Michael; I'll send v3 to address it shortly.

  Andrea