* Re: [PATCH netdev 1/2] virtio: add module option to turn off guest offloads
[not found] ` <5b2e0f71d5feddd9fe23babaad60114208731a59.1605184791.git.xuanzhuo@linux.alibaba.com>
@ 2020-11-13 1:05 ` Jason Wang
2020-11-13 10:55 ` Jesper Dangaard Brouer
0 siblings, 1 reply; 3+ messages in thread
From: Jason Wang @ 2020-11-13 1:05 UTC (permalink / raw)
To: Xuan Zhuo, netdev, bpf
Cc: ast, daniel, hawk, john.fastabend, mst, davem, kuba
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@linux.alibaba.com>
> ---
> 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;
^ permalink raw reply [flat|nested] 3+ messages in thread