Re: [PATCH netdev 1/2] virtio: add module option to turn off guest offloads

[Jason Wang <jasowang@xxxxxxxxxx>]

On 2020/11/12 下午4:11, Xuan Zhuo wrote:
> * VIRTIO_NET_F_GUEST_CSUM
> * VIRTIO_NET_F_GUEST_TSO4
> * VIRTIO_NET_F_GUEST_TSO6
> * VIRTIO_NET_F_GUEST_ECN
> * VIRTIO_NET_F_GUEST_UFO
> * VIRTIO_NET_F_MTU
>
> If these features are negotiated successfully, it may cause virtio-net to
> receive large packages, which will cause xdp to fail to load. And in
> many cases, it cannot be dynamically turned off, so add a module option
> to turn off these features.


Actually we will disable those through control virtqueue. Or does it 
mean your hardware doesn't support control guest offloads?

Module parameters may introduce a lot of burden for management and 
use-ability.

Disabling guest offloads means you may suffer from low RX throughput.

Thanks


> Signed-off-by: Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx>
> ---
>   drivers/net/virtio_net.c | 36 +++++++++++++++++++++++++++++++++++-
>   1 file changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 21b7114..232a539 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -26,10 +26,11 @@
>   static int napi_weight = NAPI_POLL_WEIGHT;
>   module_param(napi_weight, int, 0444);
>   
> -static bool csum = true, gso = true, napi_tx = true;
> +static bool csum = true, gso = true, napi_tx, guest_offload = true;
>   module_param(csum, bool, 0444);
>   module_param(gso, bool, 0444);
>   module_param(napi_tx, bool, 0644);
> +module_param(guest_offload, bool, 0644);
>   
>   /* FIXME: MTU in config. */
>   #define GOOD_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
> @@ -3245,6 +3246,18 @@ static __maybe_unused int virtnet_restore(struct virtio_device *vdev)
>   	VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
>   	VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY
>   
> +#define VIRTNET_FEATURES_WITHOUT_GUEST_OFFLOADS \
> +	VIRTIO_NET_F_CSUM, \
> +	VIRTIO_NET_F_MAC, \
> +	VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6, \
> +	VIRTIO_NET_F_HOST_ECN, \
> +	VIRTIO_NET_F_MRG_RXBUF, VIRTIO_NET_F_STATUS, VIRTIO_NET_F_CTRL_VQ, \
> +	VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \
> +	VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
> +	VIRTIO_NET_F_CTRL_MAC_ADDR, \
> +	VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
> +	VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY
> +
>   static unsigned int features[] = {
>   	VIRTNET_FEATURES,
>   };
> @@ -3255,6 +3268,16 @@ static __maybe_unused int virtnet_restore(struct virtio_device *vdev)
>   	VIRTIO_F_ANY_LAYOUT,
>   };
>   
> +static unsigned int features_without_offloads[] = {
> +	VIRTNET_FEATURES_WITHOUT_GUEST_OFFLOADS,
> +};
> +
> +static unsigned int features_without_offloads_legacy[] = {
> +	VIRTNET_FEATURES_WITHOUT_GUEST_OFFLOADS,
> +	VIRTIO_NET_F_GSO,
> +	VIRTIO_F_ANY_LAYOUT,
> +};
> +
>   static struct virtio_driver virtio_net_driver = {
>   	.feature_table = features,
>   	.feature_table_size = ARRAY_SIZE(features),
> @@ -3288,6 +3311,17 @@ static __init int virtio_net_driver_init(void)
>   	if (ret)
>   		goto err_dead;
>   
> +	if (!guest_offload) {
> +		virtio_net_driver.feature_table = features_without_offloads;
> +		virtio_net_driver.feature_table_size =
> +			ARRAY_SIZE(features_without_offloads);
> +
> +		virtio_net_driver.feature_table_legacy =
> +			features_without_offloads_legacy;
> +		virtio_net_driver.feature_table_size_legacy =
> +			ARRAY_SIZE(features_without_offloads_legacy);
> +	}
> +
>           ret = register_virtio_driver(&virtio_net_driver);
>   	if (ret)
>   		goto err_virtio;