Re: [dpdk-dev] [PATCH v9 5/9] net/virtio: add vectorized packed ring Rx path

["Liu, Yong" <yong.liu@intel.com>]

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Friday, April 24, 2020 9:34 PM
> To: Liu, Yong <yong.liu@intel.com>; Ye, Xiaolong <xiaolong.ye@intel.com>;
> Wang, Zhihong <zhihong.wang@intel.com>
> Cc: dev@dpdk.org; Van Haaren, Harry <harry.van.haaren@intel.com>
> Subject: Re: [PATCH v9 5/9] net/virtio: add vectorized packed ring Rx path
> 
> 
> 
> On 4/24/20 3:12 PM, Liu, Yong wrote:
> >> IIUC, the only difference with the non-vectorized version is the GSO
> >> support removed here.
> >> gso_type being in the same cacheline as flags in virtio_net_hdr, I don't
> >> think checking the performance gain is worth the added maintainance
> >> effort due to code duplication.
> >>
> >> Please prove I'm wrong, otherwise please move virtio_rx_offload() in a
> >> header and use it here. Alternative if it really imapcts performance is
> >> to put all the shared code in a dedicated function that can be re-used
> >> by both implementations.
> >>
> > Maxime,
> > It won't be much performance difference between non-vectorized and
> vectorized.
> > The reason to add special vectorized version is for skipping the handling of
> garbage GSO packets.
> > As all descs have been handled in batch, it is needed to revert when found
> garbage packets.
> > That will introduce complicated logic in vectorized path.
> 
		
Dequeue function will call virtio_discard_rxbuf when found gso info in hdr is invalid.
IMHO, there's no need to check gso info when GSO not negotiated.
There's an alternative way is that use single function handle GSO packets but its performance will be worse than normal function.

if ((hdr->gso_type & VIRTIO_NET_HDR_GSO_ECN) ||
	 (hdr->gso_size == 0)) {
	 return -EINVAL;
}

> 
> What do you mean by garbage packet?
> Is it really good to just ignore such issues?
> 
> Thanks,
> Maxime
> 
> > Regards,
> > Marvin
> >