netdev - Re: [virtio-dev] [PATCH RESEND v2 2/2] virtio-net: use mtu size as buffer length for big packets

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]

Date:   Fri, 26 Aug 2022 11:05:09 -0700
From:   Si-Wei Liu <si-wei.liu@...cle.com>
To:     Parav Pandit <parav@...dia.com>, Gavin Li <gavinl@...dia.com>,
        "stephen@...workplumber.org" <stephen@...workplumber.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "jesse.brandeburg@...el.com" <jesse.brandeburg@...el.com>,
        "alexander.h.duyck@...el.com" <alexander.h.duyck@...el.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "sridhar.samudrala@...el.com" <sridhar.samudrala@...el.com>,
        "jasowang@...hat.com" <jasowang@...hat.com>,
        "loseweigh@...il.com" <loseweigh@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "virtualization@...ts.linux-foundation.org" 
        <virtualization@...ts.linux-foundation.org>,
        "virtio-dev@...ts.oasis-open.org" <virtio-dev@...ts.oasis-open.org>,
        "mst@...hat.com" <mst@...hat.com>
Cc:     Gavi Teitz <gavi@...dia.com>
Subject: Re: [virtio-dev] [PATCH RESEND v2 2/2] virtio-net: use mtu size as
 buffer length for big packets



On 8/26/2022 9:41 AM, Parav Pandit wrote:
> 	
> 
> From: Si-Wei Liu <si-wei.liu@...cle.com>
> Sent: Friday, August 26, 2022 4:52 AM
> 
>> Sorry for the delay. Didn't notice as this thread was not addressed to my work email. Please copy to my work email if it needs my immediate attention.
> 
> Can you please setup your mail client to post plain text mail as required by mailing list.
> Conversation without it is close to impossible to track.

Fixed.

> +	/* If we can receive ANY GSO packets, we must allocate large ones. */
> +	if (mtu > ETH_DATA_LEN || guest_gso) {
> +		vi->big_packets = true;
> +		/* if the guest offload is offered by the device, user can modify
> +		 * offload capability. In such posted buffers may short fall of size.
> +		 * Hence allocate for max size.
> +		 */
> +		if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS))
> +			vi->big_packets_sg_num = MAX_SKB_FRAGS;
>> MAX_SKB_FRAGS is needed when any of the guest_gso capability is offered. This is per spec regardless if VIRTIO_NET_F_CTRL_GUEST_OFFLOADS is negotiated or not. Quoting spec:
> 
> 
>> If VIRTIO_NET_F_MRG_RXBUF is not negotiated:
>> If VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6 or VIRTIO_NET_F_GUEST_UFO are negotiated, the driver SHOULD populate the receive queue(s) with buffers of at least 65562 bytes.
> 
> Spec recommendation is good here, but Linux driver knows that such offload settings cannot change if the above feature is not offered.
> So I think we should add the comment and reference to the code to have this optimization.

I don't get what you mean by optimization here. Say if 
VIRTIO_NET_F_GUEST_TSO4 is negotiated while 
VIRTIO_NET_F_CTRL_GUEST_OFFLOADS is not offered, why you consider it an 
optimization to post only MTU sized (with roundup to page) buffers? 
Wouldn't it be an issue if the device is passing up aggregated GSO 
packets of up to 64KB? Noted, GUEST_GSO is implied on when the 
corresponding feature bit is negotiated, regardless the presence of 
VIRTIO_NET_F_CTRL_GUEST_OFFLOADS bit.

-Siwei