Si-Wei Liu
2023-Mar-15 06:56 UTC
[PATCH 1/2] vdpa/mlx5: Make VIRTIO_NET_F_MRG_RXBUF off by default
On 3/14/2023 7:09 PM, Parav Pandit wrote:>> From: Si-Wei Liu <si-wei.liu at oracle.com> >> Sent: Tuesday, March 14, 2023 3:29 PM >>> If user supplied feature bits and device doesn?t support some of the feature >> bits -> add command fails. >>> If user supplied feature bits, than use only the feature bits. Do not OR user >> supplied feature bits with device supported feature bits. >> I think that is the not the current behavior for mlx5_vdpa? As far as I can see it >> would AND user supplied device_features bits with the device supported >> feature bits (find the common part of both feature bits) then proceed. > Right. > It is not a problem. It is AND operation. My bad.Thanks for confirming it.> > When the user didn?t supply the feature bits, this patch avoids MRG_RXBUF being on as default.This should be fine, the driver should have the freedom to define its own set of default features when user didn't explicitly provision them through the device_features attribute. But better to expose a read-only filed, e.g. "default_features" to the "vdpa mgmtdev show" command output, otherwise users have no way to infer why MRG_RXBUF disappears mysteriously by just telling it from the current "dev_features" field. By definition, parent mgmtdev can have different default value between different devices.> This is because certain devices support it and certain device don?t. > So this patch keeps it backward compatible to be always off by default, which the device couldn?t do.As previously discussed with Jason and I myself also agreed, it's never a requirement for driver to keep backward compatibility for the default feature bits when device_features is not supplied. The only way forward to keep backward compatibility is to perform explicit provisioning through device_features. But I think your point is to keep the performance promise for packet rate for the default device creation, that is another story and it has nothing to do with backward compatibility. Thanks, -Siwei> >> Noted, we don't have this device defaults exposed to users yet other than >> what's shown in the parent mgmtdev (dev_features). And the current and the >> only safe behavior for mlx5_vdpa is to inherit all device supported features >> from parent mgmtdev, otherwise user can't know in advance what default >> features the device might come up with implicitly underneath. > Right. This is fine. > For mlx5_vdpa MRG_RXBUF as default off is optimal as before that maintains the backward compatibility.