On Fri, May 14, 2021 at 3:12 AM Jason Wang <jasowang@xxxxxxxxxx> wrote: > > On Fri, May 14, 2021 at 6:50 AM Willem de Bruijn <willemb@xxxxxxxxxx> wrote: > > > > On Mon, Feb 8, 2021 at 1:56 PM Willem de Bruijn > > <willemdebruijn.kernel@xxxxxxxxx> wrote: > > > > > > From: Willem de Bruijn <willemb@xxxxxxxxxx> > > > > > > RFCv2 for four new features to the virtio network device: > > > > > > 1. pass tx flow state to host, for routing + telemetry > > > 2. pass rx tstamp to guest, for better RTT estimation > > > 3. pass tx tstamp to guest, idem > > > 3. pass tx delivery time to host, for accurate pacing > > > > > > All would introduce an extension to the virtio spec. > > > Concurrently with code review I will write ballots to > > > https://www.oasis-open.org/committees/ballots.php?wg_abbrev=virtio > > > > > > These changes are to the driver side. Evaluation additionally requires > > > achanges to qemu and at least one back-end. I implemented preliminary > > > support in Linux vhost-net. Both patches available through github at > > > > > > https://github.com/wdebruij/linux/tree/virtio-net-txhash-2 > > > https://github.com/wdebruij/qemu/tree/virtio-net-txhash-2 > > > > > > Changes RFC -> RFCv2 > > > - add transmit timestamp patch > > > - see individual patches for other changes > > > > > > Willem de Bruijn (4): > > > virtio-net: support transmit hash report > > > virtio-net: support receive timestamp > > > virtio-net: support transmit timestamp > > > virtio-net: support future packet transmit time > > > > Seeing Yuri's patchset adding new features reminded me that I did not > > follow-up on this patch series on the list. > > > > The patches themselves are mostly in good shape. The last tx tstamp > > issue can be resolved. > > > > But the device implementation I target only supports legacy mode. > > Below conversation that we had in one of the patches makes clear that > > supporting this in legacy is not feasible. Nor is upgrading that > > device in the short term. Until there is a device implementation that > > implements these offloads, these features are a dead letter. Not moving > > forward for now. > > > > Somewhat related: is there a plan for when we run out of 64 feature bits? > > A quick thought: we need add (or reserve) a new feature bit to > indicate that we need more bits, and have transport specific > implementation of those extra bits negotiation. E.g for PCI, we can > introduce new fields in the capability. Thanks Jason. Yes, that makes sense to me. The difference from 32 to 64 bit between virtio_pci_legacy.c and virtio_pci_modern.c is a good example: static u64 vp_get_features(struct virtio_device *vdev) { struct virtio_pci_device *vp_dev = to_vp_device(vdev); /* When someone needs more than 32 feature bits, we'll need to * steal a bit to indicate that the rest are somewhere else. */ return ioread32(vp_dev->ioaddr + VIRTIO_PCI_HOST_FEATURES); } u64 vp_modern_get_features(struct virtio_pci_modern_device *mdev) { struct virtio_pci_common_cfg __iomem *cfg = mdev->common; u64 features; vp_iowrite32(0, &cfg->device_feature_select); features = vp_ioread32(&cfg->device_feature); vp_iowrite32(1, &cfg->device_feature_select); features |= ((u64)vp_ioread32(&cfg->device_feature) << 32); return features; } device_feature_select is a 32-bit field, of which only values 0 and 1 are defined so far, per the virtio 1.1 spec: " device_feature_select The driver uses this to select which feature bits device_feature shows. Value 0x0 selects Feature Bits 0 to 31, 0x1 selects Feature Bits 32 to 63, etc. " That leaves plenty of room for expansion, at least for pci devices. > > > > > > > > Actually, would it be possible to make new features available on > > > > > legacy devices? There is nothing in the features bits precluding it. > > > > > > > > I think it won't be possible: you are using feature bit 55, > > > > legacy devices have up to 32 feature bits. And of course the > > > > header looks a bit differently for legacy, you would have to add special > > > > code to handle that when mergeable buffers are off. > > > > > > I think I can make the latter work. I did start without a dependency > > > on the v1 header initially. > > > > > > Feature bit array length I had not considered. Good point. Need to > > > think about that. It would be very appealing if in particular the > > > tx-hash feature could work in legacy mode. > > > _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
