On Fri, Jun 23, 2023 at 11:52 PM Ilya Maximets <i.maxim...@ovn.org> wrote: > >> But we can't really do that from the OVS side, can we? > >> DPDK will clear the bit even if we enable it... > > > > Hum, I did not test it yet, but I was thinking of calling > > rte_vhost_driver_enable_features. > > Now that I look at the code, I think it works, since the clearing > > happens in rte_vhost_driver_register(). > > Yeah, it should work. > I think, we should just always disable every F_HOST_* and F_CSUM right > after rte_vhost_driver_register(), and then enable what we need. Might > be easier to follow the code this way. It's hard to always think in > negatives.
I prefer too, let's see how it goes when implementing. > > > > > > Another thought.. so far, CSUM was serving as a gate for HOST_UFO, > > HOST_TSO etc... > > > > Leaving ECN exposed without TSO is dirty, but the specification states > > that ECN depends on TSO features, so OVS can "hide" behind this > > assumption and expect no application sends ECN offloading requests. > > > > On the other hand, if OVS leaves UFO exposed and CSUM is now the > > default, an application may (rightfully?) expect UFO works... > > And I may have to support a two stage retry for my "recovery patch" > > (not sure I am explaining clearly enough..). > > Yeah, we can't really enable checksum offload if UFO is negotiated. > The checksum is not negotiated in these old VMs, but IIUC, if the > guest driver will be re-initialized, it may pick up new features. > > Currently possible configurations: > > 1. CSUM + TSO + ___ + ___ OVS 3.1 with userspace-tso=true > 2. ____ + TSO + ECN + ___ OVS 3.1 with userspace-tso=false > 3. ____ + ___ + ECN + UFO OVS 2.11 > > The "___" means not negotiated feature. > Only in case 1 we can actually receive packets with offloading > configured, because it's the only case where a base prerequisite > CSUM is negotiated. > > Is that correct or am I missing some case? I don't think TSO is exposed without CSUM when tso is disabled in OVS, neither OVS nor the vhost library seem to reenable this feature. In practice, I caught (with gdb) calls to vhost_user_get_features so that I only look at what we expose to the guest (regardless of what the guest later negotiates): ____ + ____ + ECN + UFO : 2.11 ____ + ____ + ECN + ___ : [2.13 .. 3.1] - tso CSUM + ____ + ECN + ___ : origin/master - tso CSUM + TSO* + ___ + ___ : [2.13 .. origin/master] + tso So I think we have two issues: - when upgrading from 2.11 to 2.13 with running VMs. Which I confirmed with a RHEL8 guest, looking at the qemu endless logs: vhost lacks feature mask 16384 for backend 2023-06-26T08:53:53.542798Z qemu-kvm: failed to init vhost_net for queue 0 $ ./features.sh 16384 VIRTIO_NET_F_HOST_UFO I am surprised nobody complained about the issue, I reproduce it with a default configuration. Maybe people stop/restart their guests if network is not working.. ? And I wonder whether we need a backportable fix for this issue. - if the UFO feature is "restored" in the master branch, OVS can't expose CSUM if the guest negotiated UFO. > > Configurations that we can allow (we can allow any subset of them): > > a. CSUM + TSO + ___ + ___ > b. CSUM + ___ + ECN + ___ Ack. > c. ____ + TSO + ECN + UFO I suppose you meant above that if CSUM is not negociated, we can allow any subset of TSO, ECN, UFO. > > So, we can only safely upgrade/migrate: > > 1 --> a > 2 --> c > 3 --> c > > "Safe" configurations: > > a. CSUM + TSO + ___ + ___ > c. ____ + TSO + ECN + UFO > > Configurations we actually want: > > x. CSUM + TSO + ___ + ___ > y. CSUM + ___ + ___ + ___ > > > Logic can be: > > 1. Try 'a/x' if userspace-tso=true > 2. Try 'y' if userspace-tso=false > 3. Try 'c' if above failed > > As far as I understand we can advertise new features, but we can't remove > already acked ones. So, the configuration 'c' is a catch-all for broken > cases. 'y' will only be possible for new/restarted VMs. > > Does that make sense? I think you covered all cases. -- David Marchand _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
