On 2023/11/01 13:19, Jason Wang wrote:
On Mon, Oct 30, 2023 at 9:15 PM Akihiko Odaki <akihiko.od...@daynix.com> wrote:
On 2023/10/30 21:51, Yuri Benditovich wrote:
On Mon, Oct 30, 2023 at 2:21 PM Akihiko Odaki <akihiko.od...@daynix.com
<mailto:akihiko.od...@daynix.com>> wrote:
On 2023/10/30 21:14, Yuri Benditovich wrote:
>
>
> On Mon, Oct 30, 2023 at 7:14 AM Akihiko Odaki
<akihiko.od...@daynix.com <mailto:akihiko.od...@daynix.com>
> <mailto:akihiko.od...@daynix.com
<mailto:akihiko.od...@daynix.com>>> wrote:
>
> vhost requires eBPF for RSS. When eBPF is not available,
virtio-net
> implicitly disables RSS even if the user explicitly requests
it. Return
> an error instead of implicitly disabling RSS if RSS is
requested but not
> available.
>
>
> I think that suggesting RSS feature when in fact it is not
available is
> not a good idea, this rather desinforms the guest.
> Existing behavior (IMHO) makes more sense.
> We can extend this discussion if needed, of course.
This change is not to advertise RSS when it's not available; it instead
reports an error to the user.
For example, think of the following command line:
qemu-system-x86_64 -device virtio-net,rss=on,netdev=n -netdev user,id=n
Before this change, it gives no error and the user will not know RSS is
not available. With this change it now gives an error as follows:
qemu-system-x86_64: -device virtio-net,rss=on,netdev=n: Can't load
eBPF RSS
Does this mean failure to run QEMU if the RSS required in the command
line and EBPF can't be loaded?
(for example if we run the system with kernel < 5.8)?
I'm not sure this is user-friendly behavior...
This patch is wrong that it assumes software RSS is not enabled at all;
I missed the vhost check before clearing VIRTIO_NET_F_RSS in
virtio_net_get_features().
That said, I indeed intend to make it return a hard error for the case
that RSS is requested for vhost but eBPF can't be loaded.
I believe the current behavior of implicitly disabling a feature
explicitly requested by the user is not good,
Yes, but it has been there for years. It complicates a lot to stick to
the migration compatibility.
but we can still emit a warning instead of an error.
It's better to follow a convention common in QEMU but I see no
documentation regarding this kind of situation. I know virtio-gpu gives
an error in such a case but it's just one example.
The problem is that it's too late to fix old Qemu, so we probably
can't do more than using compatibility flags...
This patch does not affect migration in my understanding; it does not
touch any VM state or runtime behavior.
We had another discussion regarding migration for patch "virtio-net: Do
not clear VIRTIO_NET_F_HASH_REPORT". It does change the runtime behavior
so we need to take migration into account. I still think the patch does
not require a compatibility flag since it only exposes a new feature and
does not prevent migrating from old QEMU that exposes less features. It
instead fixes the case where migrating between hosts with different tap
feature sets.
Regards,
Akihiko Odaki
