| Message ID | 20210208185558.995292-1-willemdebruijn.kernel@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | virtio-net: add tx-hash, rx-tstamp, tx-tstamp and tx-time | expand |
On Mon, Feb 8, 2021 at 1:56 PM Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > > From: Willem de Bruijn <willemb@google.com> > > 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? > > > 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.
On Fri, May 14, 2021 at 6:50 AM Willem de Bruijn <willemb@google.com> wrote: > > On Mon, Feb 8, 2021 at 1:56 PM Willem de Bruijn > <willemdebruijn.kernel@gmail.com> wrote: > > > > From: Willem de Bruijn <willemb@google.com> > > > > 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 > > > > > 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. >
On Fri, May 14, 2021 at 3:12 AM Jason Wang <jasowang@redhat.com> wrote: > > On Fri, May 14, 2021 at 6:50 AM Willem de Bruijn <willemb@google.com> wrote: > > > > On Mon, Feb 8, 2021 at 1:56 PM Willem de Bruijn > > <willemdebruijn.kernel@gmail.com> wrote: > > > > > > From: Willem de Bruijn <willemb@google.com> > > > > > > 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. > > >
From: Willem de Bruijn <willemb@google.com> 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 drivers/net/virtio_net.c | 193 +++++++++++++++++++++++++++++++- drivers/virtio/virtio_ring.c | 3 +- include/linux/virtio.h | 1 + include/uapi/linux/virtio_net.h | 24 +++- 4 files changed, 214 insertions(+), 7 deletions(-)