* [PATCH] virtio_net: Support RX hash XDP hint
@ 2024-01-22 10:22 Liang Chen
2024-01-22 11:10 ` Heng Qi
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Liang Chen @ 2024-01-22 10:22 UTC (permalink / raw)
To: mst, jasowang; +Cc: virtualization, linux-kernel, liangchen.linux
The RSS hash report is a feature that's part of the virtio specification.
Currently, virtio backends like qemu, vdpa (mlx5), and potentially vhost
(still a work in progress as per [1]) support this feature. While the
capability to obtain the RSS hash has been enabled in the normal path,
it's currently missing in the XDP path. Therefore, we are introducing XDP
hints through kfuncs to allow XDP programs to access the RSS hash.
Signed-off-by: Liang Chen <liangchen.linux@gmail.com>
---
drivers/net/virtio_net.c | 56 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 56 insertions(+)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index d7ce4a1011ea..1463a4709e3c 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -4579,6 +4579,60 @@ static void virtnet_set_big_packets(struct virtnet_info *vi, const int mtu)
}
}
+static int virtnet_xdp_rx_hash(const struct xdp_md *_ctx, u32 *hash,
+ enum xdp_rss_hash_type *rss_type)
+{
+ const struct xdp_buff *xdp = (void *)_ctx;
+ struct virtio_net_hdr_v1_hash *hdr_hash;
+ struct virtnet_info *vi;
+
+ if (!(xdp->rxq->dev->features & NETIF_F_RXHASH))
+ return -ENODATA;
+
+ vi = netdev_priv(xdp->rxq->dev);
+ hdr_hash = (struct virtio_net_hdr_v1_hash *)(xdp->data - vi->hdr_len);
+
+ switch (__le16_to_cpu(hdr_hash->hash_report)) {
+ case VIRTIO_NET_HASH_REPORT_TCPv4:
+ *rss_type = XDP_RSS_TYPE_L4_IPV4_TCP;
+ break;
+ case VIRTIO_NET_HASH_REPORT_UDPv4:
+ *rss_type = XDP_RSS_TYPE_L4_IPV4_UDP;
+ break;
+ case VIRTIO_NET_HASH_REPORT_TCPv6:
+ *rss_type = XDP_RSS_TYPE_L4_IPV6_TCP;
+ break;
+ case VIRTIO_NET_HASH_REPORT_UDPv6:
+ *rss_type = XDP_RSS_TYPE_L4_IPV6_UDP;
+ break;
+ case VIRTIO_NET_HASH_REPORT_TCPv6_EX:
+ *rss_type = XDP_RSS_TYPE_L4_IPV6_TCP_EX;
+ break;
+ case VIRTIO_NET_HASH_REPORT_UDPv6_EX:
+ *rss_type = XDP_RSS_TYPE_L4_IPV6_UDP_EX;
+ break;
+ case VIRTIO_NET_HASH_REPORT_IPv4:
+ *rss_type = XDP_RSS_TYPE_L3_IPV4;
+ break;
+ case VIRTIO_NET_HASH_REPORT_IPv6:
+ *rss_type = XDP_RSS_TYPE_L3_IPV6;
+ break;
+ case VIRTIO_NET_HASH_REPORT_IPv6_EX:
+ *rss_type = XDP_RSS_TYPE_L3_IPV6_EX;
+ break;
+ case VIRTIO_NET_HASH_REPORT_NONE:
+ default:
+ *rss_type = XDP_RSS_TYPE_NONE;
+ }
+
+ *hash = __le32_to_cpu(hdr_hash->hash_value);
+ return 0;
+}
+
+static const struct xdp_metadata_ops virtnet_xdp_metadata_ops = {
+ .xmo_rx_hash = virtnet_xdp_rx_hash,
+};
+
static int virtnet_probe(struct virtio_device *vdev)
{
int i, err = -ENOMEM;
@@ -4613,6 +4667,8 @@ static int virtnet_probe(struct virtio_device *vdev)
dev->ethtool_ops = &virtnet_ethtool_ops;
SET_NETDEV_DEV(dev, &vdev->dev);
+ dev->xdp_metadata_ops = &virtnet_xdp_metadata_ops;
+
/* Do we support "hardware" checksums? */
if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) {
/* This opens up the world of extra features. */
--
2.40.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] virtio_net: Support RX hash XDP hint
2024-01-22 10:22 [PATCH] virtio_net: Support RX hash XDP hint Liang Chen
@ 2024-01-22 11:10 ` Heng Qi
2024-01-24 2:04 ` Liang Chen
2024-01-23 7:02 ` Michael S. Tsirkin
2024-01-24 2:12 ` Jason Wang
2 siblings, 1 reply; 9+ messages in thread
From: Heng Qi @ 2024-01-22 11:10 UTC (permalink / raw)
To: Liang Chen, mst, jasowang; +Cc: virtualization, linux-kernel
Hi Liang Chen,
在 2024/1/22 下午6:22, Liang Chen 写道:
> The RSS hash report is a feature that's part of the virtio specification.
> Currently, virtio backends like qemu, vdpa (mlx5), and potentially vhost
> (still a work in progress as per [1]) support this feature. While the
> capability to obtain the RSS hash has been enabled in the normal path,
> it's currently missing in the XDP path. Therefore, we are introducing XDP
> hints through kfuncs to allow XDP programs to access the RSS hash.
>
> Signed-off-by: Liang Chen <liangchen.linux@gmail.com>
> ---
> drivers/net/virtio_net.c | 56 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 56 insertions(+)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index d7ce4a1011ea..1463a4709e3c 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -4579,6 +4579,60 @@ static void virtnet_set_big_packets(struct virtnet_info *vi, const int mtu)
> }
> }
>
> +static int virtnet_xdp_rx_hash(const struct xdp_md *_ctx, u32 *hash,
> + enum xdp_rss_hash_type *rss_type)
> +{
> + const struct xdp_buff *xdp = (void *)_ctx;
> + struct virtio_net_hdr_v1_hash *hdr_hash;
> + struct virtnet_info *vi;
> +
> + if (!(xdp->rxq->dev->features & NETIF_F_RXHASH))
I think 'vi->has_rss_hash_report' should be used here.
NETIF_F_RXHASH cannot guarantee that the hash report feature is negotiated,
and accessing hash_report and hash_value is unsafe at this time.
> + return -ENODATA;
> +
> + vi = netdev_priv(xdp->rxq->dev);
> + hdr_hash = (struct virtio_net_hdr_v1_hash *)(xdp->data - vi->hdr_len);
If the virtio-net-hdr is overrided by the XDP prog, how can this be done
correctly and as expected?
Thanks,
Heng
> +
> + switch (__le16_to_cpu(hdr_hash->hash_report)) {
> + case VIRTIO_NET_HASH_REPORT_TCPv4:
> + *rss_type = XDP_RSS_TYPE_L4_IPV4_TCP;
> + break;
> + case VIRTIO_NET_HASH_REPORT_UDPv4:
> + *rss_type = XDP_RSS_TYPE_L4_IPV4_UDP;
> + break;
> + case VIRTIO_NET_HASH_REPORT_TCPv6:
> + *rss_type = XDP_RSS_TYPE_L4_IPV6_TCP;
> + break;
> + case VIRTIO_NET_HASH_REPORT_UDPv6:
> + *rss_type = XDP_RSS_TYPE_L4_IPV6_UDP;
> + break;
> + case VIRTIO_NET_HASH_REPORT_TCPv6_EX:
> + *rss_type = XDP_RSS_TYPE_L4_IPV6_TCP_EX;
> + break;
> + case VIRTIO_NET_HASH_REPORT_UDPv6_EX:
> + *rss_type = XDP_RSS_TYPE_L4_IPV6_UDP_EX;
> + break;
> + case VIRTIO_NET_HASH_REPORT_IPv4:
> + *rss_type = XDP_RSS_TYPE_L3_IPV4;
> + break;
> + case VIRTIO_NET_HASH_REPORT_IPv6:
> + *rss_type = XDP_RSS_TYPE_L3_IPV6;
> + break;
> + case VIRTIO_NET_HASH_REPORT_IPv6_EX:
> + *rss_type = XDP_RSS_TYPE_L3_IPV6_EX;
> + break;
> + case VIRTIO_NET_HASH_REPORT_NONE:
> + default:
> + *rss_type = XDP_RSS_TYPE_NONE;
> + }
> +
> + *hash = __le32_to_cpu(hdr_hash->hash_value);
> + return 0;
> +}
> +
> +static const struct xdp_metadata_ops virtnet_xdp_metadata_ops = {
> + .xmo_rx_hash = virtnet_xdp_rx_hash,
> +};
> +
> static int virtnet_probe(struct virtio_device *vdev)
> {
> int i, err = -ENOMEM;
> @@ -4613,6 +4667,8 @@ static int virtnet_probe(struct virtio_device *vdev)
> dev->ethtool_ops = &virtnet_ethtool_ops;
> SET_NETDEV_DEV(dev, &vdev->dev);
>
> + dev->xdp_metadata_ops = &virtnet_xdp_metadata_ops;
> +
> /* Do we support "hardware" checksums? */
> if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) {
> /* This opens up the world of extra features. */
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] virtio_net: Support RX hash XDP hint
2024-01-22 10:22 [PATCH] virtio_net: Support RX hash XDP hint Liang Chen
2024-01-22 11:10 ` Heng Qi
@ 2024-01-23 7:02 ` Michael S. Tsirkin
2024-01-24 2:27 ` Liang Chen
2024-01-24 2:12 ` Jason Wang
2 siblings, 1 reply; 9+ messages in thread
From: Michael S. Tsirkin @ 2024-01-23 7:02 UTC (permalink / raw)
To: Liang Chen; +Cc: jasowang, virtualization, linux-kernel
On Mon, Jan 22, 2024 at 06:22:56PM +0800, Liang Chen wrote:
> The RSS hash report is a feature that's part of the virtio specification.
> Currently, virtio backends like qemu, vdpa (mlx5), and potentially vhost
> (still a work in progress as per [1]) support this feature. While the
> capability to obtain the RSS hash has been enabled in the normal path,
> it's currently missing in the XDP path. Therefore, we are introducing XDP
> hints through kfuncs to allow XDP programs to access the RSS hash.
>
> Signed-off-by: Liang Chen <liangchen.linux@gmail.com>
> ---
> drivers/net/virtio_net.c | 56 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 56 insertions(+)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index d7ce4a1011ea..1463a4709e3c 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -4579,6 +4579,60 @@ static void virtnet_set_big_packets(struct virtnet_info *vi, const int mtu)
> }
> }
>
> +static int virtnet_xdp_rx_hash(const struct xdp_md *_ctx, u32 *hash,
> + enum xdp_rss_hash_type *rss_type)
> +{
> + const struct xdp_buff *xdp = (void *)_ctx;
> + struct virtio_net_hdr_v1_hash *hdr_hash;
> + struct virtnet_info *vi;
> +
> + if (!(xdp->rxq->dev->features & NETIF_F_RXHASH))
> + return -ENODATA;
> +
> + vi = netdev_priv(xdp->rxq->dev);
> + hdr_hash = (struct virtio_net_hdr_v1_hash *)(xdp->data - vi->hdr_len);
> +
> + switch (__le16_to_cpu(hdr_hash->hash_report)) {
> + case VIRTIO_NET_HASH_REPORT_TCPv4:
> + *rss_type = XDP_RSS_TYPE_L4_IPV4_TCP;
> + break;
> + case VIRTIO_NET_HASH_REPORT_UDPv4:
> + *rss_type = XDP_RSS_TYPE_L4_IPV4_UDP;
> + break;
> + case VIRTIO_NET_HASH_REPORT_TCPv6:
> + *rss_type = XDP_RSS_TYPE_L4_IPV6_TCP;
> + break;
> + case VIRTIO_NET_HASH_REPORT_UDPv6:
> + *rss_type = XDP_RSS_TYPE_L4_IPV6_UDP;
> + break;
> + case VIRTIO_NET_HASH_REPORT_TCPv6_EX:
> + *rss_type = XDP_RSS_TYPE_L4_IPV6_TCP_EX;
> + break;
> + case VIRTIO_NET_HASH_REPORT_UDPv6_EX:
> + *rss_type = XDP_RSS_TYPE_L4_IPV6_UDP_EX;
> + break;
> + case VIRTIO_NET_HASH_REPORT_IPv4:
> + *rss_type = XDP_RSS_TYPE_L3_IPV4;
> + break;
> + case VIRTIO_NET_HASH_REPORT_IPv6:
> + *rss_type = XDP_RSS_TYPE_L3_IPV6;
> + break;
> + case VIRTIO_NET_HASH_REPORT_IPv6_EX:
> + *rss_type = XDP_RSS_TYPE_L3_IPV6_EX;
> + break;
> + case VIRTIO_NET_HASH_REPORT_NONE:
> + default:
> + *rss_type = XDP_RSS_TYPE_NONE;
> + }
> +
> + *hash = __le32_to_cpu(hdr_hash->hash_value);
> + return 0;
> +}
> +
> +static const struct xdp_metadata_ops virtnet_xdp_metadata_ops = {
> + .xmo_rx_hash = virtnet_xdp_rx_hash,
> +};
> +
> static int virtnet_probe(struct virtio_device *vdev)
> {
> int i, err = -ENOMEM;
> @@ -4613,6 +4667,8 @@ static int virtnet_probe(struct virtio_device *vdev)
> dev->ethtool_ops = &virtnet_ethtool_ops;
> SET_NETDEV_DEV(dev, &vdev->dev);
>
> + dev->xdp_metadata_ops = &virtnet_xdp_metadata_ops;
> +
How about making this assignment depend on
xdp->rxq->dev->features & NETIF_F_RXHASH
?
> /* Do we support "hardware" checksums? */
> if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) {
> /* This opens up the world of extra features. */
> --
> 2.40.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] virtio_net: Support RX hash XDP hint
2024-01-22 11:10 ` Heng Qi
@ 2024-01-24 2:04 ` Liang Chen
2024-01-24 6:05 ` Xuan Zhuo
0 siblings, 1 reply; 9+ messages in thread
From: Liang Chen @ 2024-01-24 2:04 UTC (permalink / raw)
To: Heng Qi; +Cc: mst, jasowang, virtualization, linux-kernel
On Mon, Jan 22, 2024 at 7:10 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
>
> Hi Liang Chen,
>
> 在 2024/1/22 下午6:22, Liang Chen 写道:
> > The RSS hash report is a feature that's part of the virtio specification.
> > Currently, virtio backends like qemu, vdpa (mlx5), and potentially vhost
> > (still a work in progress as per [1]) support this feature. While the
> > capability to obtain the RSS hash has been enabled in the normal path,
> > it's currently missing in the XDP path. Therefore, we are introducing XDP
> > hints through kfuncs to allow XDP programs to access the RSS hash.
> >
> > Signed-off-by: Liang Chen <liangchen.linux@gmail.com>
> > ---
> > drivers/net/virtio_net.c | 56 ++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 56 insertions(+)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index d7ce4a1011ea..1463a4709e3c 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -4579,6 +4579,60 @@ static void virtnet_set_big_packets(struct virtnet_info *vi, const int mtu)
> > }
> > }
> >
> > +static int virtnet_xdp_rx_hash(const struct xdp_md *_ctx, u32 *hash,
> > + enum xdp_rss_hash_type *rss_type)
> > +{
> > + const struct xdp_buff *xdp = (void *)_ctx;
> > + struct virtio_net_hdr_v1_hash *hdr_hash;
> > + struct virtnet_info *vi;
> > +
> > + if (!(xdp->rxq->dev->features & NETIF_F_RXHASH))
>
> I think 'vi->has_rss_hash_report' should be used here.
> NETIF_F_RXHASH cannot guarantee that the hash report feature is negotiated,
> and accessing hash_report and hash_value is unsafe at this time.
>
My understanding is that rxhash in dev->features is turned on only if
the corresponding hw feature is set. We will double check on this.
> > + return -ENODATA;
> > +
> > + vi = netdev_priv(xdp->rxq->dev);
> > + hdr_hash = (struct virtio_net_hdr_v1_hash *)(xdp->data - vi->hdr_len);
>
> If the virtio-net-hdr is overrided by the XDP prog, how can this be done
> correctly and as expected?
>
Yeah, thanks for bringing up this concern. We are actively working on
a fix of this issue(possibly with a wrapper structure of xdp_buff).
Thanks,
Liang
> Thanks,
> Heng
>
> > +
> > + switch (__le16_to_cpu(hdr_hash->hash_report)) {
> > + case VIRTIO_NET_HASH_REPORT_TCPv4:
> > + *rss_type = XDP_RSS_TYPE_L4_IPV4_TCP;
> > + break;
> > + case VIRTIO_NET_HASH_REPORT_UDPv4:
> > + *rss_type = XDP_RSS_TYPE_L4_IPV4_UDP;
> > + break;
> > + case VIRTIO_NET_HASH_REPORT_TCPv6:
> > + *rss_type = XDP_RSS_TYPE_L4_IPV6_TCP;
> > + break;
> > + case VIRTIO_NET_HASH_REPORT_UDPv6:
> > + *rss_type = XDP_RSS_TYPE_L4_IPV6_UDP;
> > + break;
> > + case VIRTIO_NET_HASH_REPORT_TCPv6_EX:
> > + *rss_type = XDP_RSS_TYPE_L4_IPV6_TCP_EX;
> > + break;
> > + case VIRTIO_NET_HASH_REPORT_UDPv6_EX:
> > + *rss_type = XDP_RSS_TYPE_L4_IPV6_UDP_EX;
> > + break;
> > + case VIRTIO_NET_HASH_REPORT_IPv4:
> > + *rss_type = XDP_RSS_TYPE_L3_IPV4;
> > + break;
> > + case VIRTIO_NET_HASH_REPORT_IPv6:
> > + *rss_type = XDP_RSS_TYPE_L3_IPV6;
> > + break;
> > + case VIRTIO_NET_HASH_REPORT_IPv6_EX:
> > + *rss_type = XDP_RSS_TYPE_L3_IPV6_EX;
> > + break;
> > + case VIRTIO_NET_HASH_REPORT_NONE:
> > + default:
> > + *rss_type = XDP_RSS_TYPE_NONE;
> > + }
> > +
> > + *hash = __le32_to_cpu(hdr_hash->hash_value);
> > + return 0;
> > +}
> > +
> > +static const struct xdp_metadata_ops virtnet_xdp_metadata_ops = {
> > + .xmo_rx_hash = virtnet_xdp_rx_hash,
> > +};
> > +
> > static int virtnet_probe(struct virtio_device *vdev)
> > {
> > int i, err = -ENOMEM;
> > @@ -4613,6 +4667,8 @@ static int virtnet_probe(struct virtio_device *vdev)
> > dev->ethtool_ops = &virtnet_ethtool_ops;
> > SET_NETDEV_DEV(dev, &vdev->dev);
> >
> > + dev->xdp_metadata_ops = &virtnet_xdp_metadata_ops;
> > +
> > /* Do we support "hardware" checksums? */
> > if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) {
> > /* This opens up the world of extra features. */
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] virtio_net: Support RX hash XDP hint
2024-01-22 10:22 [PATCH] virtio_net: Support RX hash XDP hint Liang Chen
2024-01-22 11:10 ` Heng Qi
2024-01-23 7:02 ` Michael S. Tsirkin
@ 2024-01-24 2:12 ` Jason Wang
2024-01-24 8:03 ` Liang Chen
2 siblings, 1 reply; 9+ messages in thread
From: Jason Wang @ 2024-01-24 2:12 UTC (permalink / raw)
To: Liang Chen; +Cc: mst, virtualization, linux-kernel
On Mon, Jan 22, 2024 at 6:23 PM Liang Chen <liangchen.linux@gmail.com> wrote:
>
> The RSS hash report is a feature that's part of the virtio specification.
> Currently, virtio backends like qemu, vdpa (mlx5), and potentially vhost
> (still a work in progress as per [1]) support this feature. While the
> capability to obtain the RSS hash has been enabled in the normal path,
> it's currently missing in the XDP path. Therefore, we are introducing XDP
> hints through kfuncs to allow XDP programs to access the RSS hash.
>
> Signed-off-by: Liang Chen <liangchen.linux@gmail.com>
> ---
> drivers/net/virtio_net.c | 56 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 56 insertions(+)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index d7ce4a1011ea..1463a4709e3c 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -4579,6 +4579,60 @@ static void virtnet_set_big_packets(struct virtnet_info *vi, const int mtu)
> }
> }
>
> +static int virtnet_xdp_rx_hash(const struct xdp_md *_ctx, u32 *hash,
> + enum xdp_rss_hash_type *rss_type)
> +{
> + const struct xdp_buff *xdp = (void *)_ctx;
> + struct virtio_net_hdr_v1_hash *hdr_hash;
> + struct virtnet_info *vi;
> +
> + if (!(xdp->rxq->dev->features & NETIF_F_RXHASH))
> + return -ENODATA;
> +
> + vi = netdev_priv(xdp->rxq->dev);
> + hdr_hash = (struct virtio_net_hdr_v1_hash *)(xdp->data - vi->hdr_len);
Is there a guarantee that the hdr is not modified?
Thanks
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] virtio_net: Support RX hash XDP hint
2024-01-23 7:02 ` Michael S. Tsirkin
@ 2024-01-24 2:27 ` Liang Chen
0 siblings, 0 replies; 9+ messages in thread
From: Liang Chen @ 2024-01-24 2:27 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: jasowang, virtualization, linux-kernel
On Tue, Jan 23, 2024 at 3:02 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Mon, Jan 22, 2024 at 06:22:56PM +0800, Liang Chen wrote:
> > The RSS hash report is a feature that's part of the virtio specification.
> > Currently, virtio backends like qemu, vdpa (mlx5), and potentially vhost
> > (still a work in progress as per [1]) support this feature. While the
> > capability to obtain the RSS hash has been enabled in the normal path,
> > it's currently missing in the XDP path. Therefore, we are introducing XDP
> > hints through kfuncs to allow XDP programs to access the RSS hash.
> >
> > Signed-off-by: Liang Chen <liangchen.linux@gmail.com>
> > ---
> > drivers/net/virtio_net.c | 56 ++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 56 insertions(+)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index d7ce4a1011ea..1463a4709e3c 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -4579,6 +4579,60 @@ static void virtnet_set_big_packets(struct virtnet_info *vi, const int mtu)
> > }
> > }
> >
> > +static int virtnet_xdp_rx_hash(const struct xdp_md *_ctx, u32 *hash,
> > + enum xdp_rss_hash_type *rss_type)
> > +{
> > + const struct xdp_buff *xdp = (void *)_ctx;
> > + struct virtio_net_hdr_v1_hash *hdr_hash;
> > + struct virtnet_info *vi;
> > +
> > + if (!(xdp->rxq->dev->features & NETIF_F_RXHASH))
> > + return -ENODATA;
> > +
> > + vi = netdev_priv(xdp->rxq->dev);
> > + hdr_hash = (struct virtio_net_hdr_v1_hash *)(xdp->data - vi->hdr_len);
> > +
> > + switch (__le16_to_cpu(hdr_hash->hash_report)) {
> > + case VIRTIO_NET_HASH_REPORT_TCPv4:
> > + *rss_type = XDP_RSS_TYPE_L4_IPV4_TCP;
> > + break;
> > + case VIRTIO_NET_HASH_REPORT_UDPv4:
> > + *rss_type = XDP_RSS_TYPE_L4_IPV4_UDP;
> > + break;
> > + case VIRTIO_NET_HASH_REPORT_TCPv6:
> > + *rss_type = XDP_RSS_TYPE_L4_IPV6_TCP;
> > + break;
> > + case VIRTIO_NET_HASH_REPORT_UDPv6:
> > + *rss_type = XDP_RSS_TYPE_L4_IPV6_UDP;
> > + break;
> > + case VIRTIO_NET_HASH_REPORT_TCPv6_EX:
> > + *rss_type = XDP_RSS_TYPE_L4_IPV6_TCP_EX;
> > + break;
> > + case VIRTIO_NET_HASH_REPORT_UDPv6_EX:
> > + *rss_type = XDP_RSS_TYPE_L4_IPV6_UDP_EX;
> > + break;
> > + case VIRTIO_NET_HASH_REPORT_IPv4:
> > + *rss_type = XDP_RSS_TYPE_L3_IPV4;
> > + break;
> > + case VIRTIO_NET_HASH_REPORT_IPv6:
> > + *rss_type = XDP_RSS_TYPE_L3_IPV6;
> > + break;
> > + case VIRTIO_NET_HASH_REPORT_IPv6_EX:
> > + *rss_type = XDP_RSS_TYPE_L3_IPV6_EX;
> > + break;
> > + case VIRTIO_NET_HASH_REPORT_NONE:
> > + default:
> > + *rss_type = XDP_RSS_TYPE_NONE;
> > + }
> > +
> > + *hash = __le32_to_cpu(hdr_hash->hash_value);
> > + return 0;
> > +}
> > +
> > +static const struct xdp_metadata_ops virtnet_xdp_metadata_ops = {
> > + .xmo_rx_hash = virtnet_xdp_rx_hash,
> > +};
> > +
> > static int virtnet_probe(struct virtio_device *vdev)
> > {
> > int i, err = -ENOMEM;
> > @@ -4613,6 +4667,8 @@ static int virtnet_probe(struct virtio_device *vdev)
> > dev->ethtool_ops = &virtnet_ethtool_ops;
> > SET_NETDEV_DEV(dev, &vdev->dev);
> >
> > + dev->xdp_metadata_ops = &virtnet_xdp_metadata_ops;
> > +
>
> How about making this assignment depend on
>
> xdp->rxq->dev->features & NETIF_F_RXHASH
>
Thanks! How about dev->hw_features & NETIF_F_RXHASH? dev->features &
NETIF_F_RXHASH has not been enabled at this point.
> ?
>
> > /* Do we support "hardware" checksums? */
> > if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) {
> > /* This opens up the world of extra features. */
> > --
> > 2.40.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] virtio_net: Support RX hash XDP hint
2024-01-24 2:04 ` Liang Chen
@ 2024-01-24 6:05 ` Xuan Zhuo
2024-01-24 8:04 ` Liang Chen
0 siblings, 1 reply; 9+ messages in thread
From: Xuan Zhuo @ 2024-01-24 6:05 UTC (permalink / raw)
To: Liang Chen; +Cc: mst, jasowang, virtualization, linux-kernel, Heng Qi
On Wed, 24 Jan 2024 10:04:51 +0800, Liang Chen <liangchen.linux@gmail.com> wrote:
> On Mon, Jan 22, 2024 at 7:10 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
> >
> > Hi Liang Chen,
> >
> > 在 2024/1/22 下午6:22, Liang Chen 写道:
> > > The RSS hash report is a feature that's part of the virtio specification.
> > > Currently, virtio backends like qemu, vdpa (mlx5), and potentially vhost
> > > (still a work in progress as per [1]) support this feature. While the
> > > capability to obtain the RSS hash has been enabled in the normal path,
> > > it's currently missing in the XDP path. Therefore, we are introducing XDP
> > > hints through kfuncs to allow XDP programs to access the RSS hash.
> > >
> > > Signed-off-by: Liang Chen <liangchen.linux@gmail.com>
> > > ---
> > > drivers/net/virtio_net.c | 56 ++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 56 insertions(+)
> > >
> > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > index d7ce4a1011ea..1463a4709e3c 100644
> > > --- a/drivers/net/virtio_net.c
> > > +++ b/drivers/net/virtio_net.c
> > > @@ -4579,6 +4579,60 @@ static void virtnet_set_big_packets(struct virtnet_info *vi, const int mtu)
> > > }
> > > }
> > >
> > > +static int virtnet_xdp_rx_hash(const struct xdp_md *_ctx, u32 *hash,
> > > + enum xdp_rss_hash_type *rss_type)
> > > +{
> > > + const struct xdp_buff *xdp = (void *)_ctx;
> > > + struct virtio_net_hdr_v1_hash *hdr_hash;
> > > + struct virtnet_info *vi;
> > > +
> > > + if (!(xdp->rxq->dev->features & NETIF_F_RXHASH))
> >
> > I think 'vi->has_rss_hash_report' should be used here.
> > NETIF_F_RXHASH cannot guarantee that the hash report feature is negotiated,
> > and accessing hash_report and hash_value is unsafe at this time.
> >
>
> My understanding is that rxhash in dev->features is turned on only if
> the corresponding hw feature is set. We will double check on this.
>
> > > + return -ENODATA;
> > > +
> > > + vi = netdev_priv(xdp->rxq->dev);
> > > + hdr_hash = (struct virtio_net_hdr_v1_hash *)(xdp->data - vi->hdr_len);
> >
> > If the virtio-net-hdr is overrided by the XDP prog, how can this be done
> > correctly and as expected?
> >
>
> Yeah, thanks for bringing up this concern. We are actively working on
> a fix of this issue(possibly with a wrapper structure of xdp_buff).
Are there some places to save the hash before run xdp?
Thanks.
>
> Thanks,
> Liang
>
> > Thanks,
> > Heng
> >
> > > +
> > > + switch (__le16_to_cpu(hdr_hash->hash_report)) {
> > > + case VIRTIO_NET_HASH_REPORT_TCPv4:
> > > + *rss_type = XDP_RSS_TYPE_L4_IPV4_TCP;
> > > + break;
> > > + case VIRTIO_NET_HASH_REPORT_UDPv4:
> > > + *rss_type = XDP_RSS_TYPE_L4_IPV4_UDP;
> > > + break;
> > > + case VIRTIO_NET_HASH_REPORT_TCPv6:
> > > + *rss_type = XDP_RSS_TYPE_L4_IPV6_TCP;
> > > + break;
> > > + case VIRTIO_NET_HASH_REPORT_UDPv6:
> > > + *rss_type = XDP_RSS_TYPE_L4_IPV6_UDP;
> > > + break;
> > > + case VIRTIO_NET_HASH_REPORT_TCPv6_EX:
> > > + *rss_type = XDP_RSS_TYPE_L4_IPV6_TCP_EX;
> > > + break;
> > > + case VIRTIO_NET_HASH_REPORT_UDPv6_EX:
> > > + *rss_type = XDP_RSS_TYPE_L4_IPV6_UDP_EX;
> > > + break;
> > > + case VIRTIO_NET_HASH_REPORT_IPv4:
> > > + *rss_type = XDP_RSS_TYPE_L3_IPV4;
> > > + break;
> > > + case VIRTIO_NET_HASH_REPORT_IPv6:
> > > + *rss_type = XDP_RSS_TYPE_L3_IPV6;
> > > + break;
> > > + case VIRTIO_NET_HASH_REPORT_IPv6_EX:
> > > + *rss_type = XDP_RSS_TYPE_L3_IPV6_EX;
> > > + break;
> > > + case VIRTIO_NET_HASH_REPORT_NONE:
> > > + default:
> > > + *rss_type = XDP_RSS_TYPE_NONE;
> > > + }
> > > +
> > > + *hash = __le32_to_cpu(hdr_hash->hash_value);
> > > + return 0;
> > > +}
> > > +
> > > +static const struct xdp_metadata_ops virtnet_xdp_metadata_ops = {
> > > + .xmo_rx_hash = virtnet_xdp_rx_hash,
> > > +};
> > > +
> > > static int virtnet_probe(struct virtio_device *vdev)
> > > {
> > > int i, err = -ENOMEM;
> > > @@ -4613,6 +4667,8 @@ static int virtnet_probe(struct virtio_device *vdev)
> > > dev->ethtool_ops = &virtnet_ethtool_ops;
> > > SET_NETDEV_DEV(dev, &vdev->dev);
> > >
> > > + dev->xdp_metadata_ops = &virtnet_xdp_metadata_ops;
> > > +
> > > /* Do we support "hardware" checksums? */
> > > if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) {
> > > /* This opens up the world of extra features. */
> >
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] virtio_net: Support RX hash XDP hint
2024-01-24 2:12 ` Jason Wang
@ 2024-01-24 8:03 ` Liang Chen
0 siblings, 0 replies; 9+ messages in thread
From: Liang Chen @ 2024-01-24 8:03 UTC (permalink / raw)
To: Jason Wang; +Cc: mst, virtualization, linux-kernel
On Wed, Jan 24, 2024 at 10:12 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Mon, Jan 22, 2024 at 6:23 PM Liang Chen <liangchen.linux@gmail.com> wrote:
> >
> > The RSS hash report is a feature that's part of the virtio specification.
> > Currently, virtio backends like qemu, vdpa (mlx5), and potentially vhost
> > (still a work in progress as per [1]) support this feature. While the
> > capability to obtain the RSS hash has been enabled in the normal path,
> > it's currently missing in the XDP path. Therefore, we are introducing XDP
> > hints through kfuncs to allow XDP programs to access the RSS hash.
> >
> > Signed-off-by: Liang Chen <liangchen.linux@gmail.com>
> > ---
> > drivers/net/virtio_net.c | 56 ++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 56 insertions(+)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index d7ce4a1011ea..1463a4709e3c 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -4579,6 +4579,60 @@ static void virtnet_set_big_packets(struct virtnet_info *vi, const int mtu)
> > }
> > }
> >
> > +static int virtnet_xdp_rx_hash(const struct xdp_md *_ctx, u32 *hash,
> > + enum xdp_rss_hash_type *rss_type)
> > +{
> > + const struct xdp_buff *xdp = (void *)_ctx;
> > + struct virtio_net_hdr_v1_hash *hdr_hash;
> > + struct virtnet_info *vi;
> > +
> > + if (!(xdp->rxq->dev->features & NETIF_F_RXHASH))
> > + return -ENODATA;
> > +
> > + vi = netdev_priv(xdp->rxq->dev);
> > + hdr_hash = (struct virtio_net_hdr_v1_hash *)(xdp->data - vi->hdr_len);
>
> Is there a guarantee that the hdr is not modified?
>
We cannot guarantee the hdr is not modified by the XDP prog, So the
idea is to save it in a wrapper structure before running the xdp prog.
Patch is coming soon.
Thanks,
Liang
> Thanks
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] virtio_net: Support RX hash XDP hint
2024-01-24 6:05 ` Xuan Zhuo
@ 2024-01-24 8:04 ` Liang Chen
0 siblings, 0 replies; 9+ messages in thread
From: Liang Chen @ 2024-01-24 8:04 UTC (permalink / raw)
To: Xuan Zhuo; +Cc: mst, jasowang, virtualization, linux-kernel, Heng Qi
On Wed, Jan 24, 2024 at 2:06 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> On Wed, 24 Jan 2024 10:04:51 +0800, Liang Chen <liangchen.linux@gmail.com> wrote:
> > On Mon, Jan 22, 2024 at 7:10 PM Heng Qi <hengqi@linux.alibaba.com> wrote:
> > >
> > > Hi Liang Chen,
> > >
> > > 在 2024/1/22 下午6:22, Liang Chen 写道:
> > > > The RSS hash report is a feature that's part of the virtio specification.
> > > > Currently, virtio backends like qemu, vdpa (mlx5), and potentially vhost
> > > > (still a work in progress as per [1]) support this feature. While the
> > > > capability to obtain the RSS hash has been enabled in the normal path,
> > > > it's currently missing in the XDP path. Therefore, we are introducing XDP
> > > > hints through kfuncs to allow XDP programs to access the RSS hash.
> > > >
> > > > Signed-off-by: Liang Chen <liangchen.linux@gmail.com>
> > > > ---
> > > > drivers/net/virtio_net.c | 56 ++++++++++++++++++++++++++++++++++++++++
> > > > 1 file changed, 56 insertions(+)
> > > >
> > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > > index d7ce4a1011ea..1463a4709e3c 100644
> > > > --- a/drivers/net/virtio_net.c
> > > > +++ b/drivers/net/virtio_net.c
> > > > @@ -4579,6 +4579,60 @@ static void virtnet_set_big_packets(struct virtnet_info *vi, const int mtu)
> > > > }
> > > > }
> > > >
> > > > +static int virtnet_xdp_rx_hash(const struct xdp_md *_ctx, u32 *hash,
> > > > + enum xdp_rss_hash_type *rss_type)
> > > > +{
> > > > + const struct xdp_buff *xdp = (void *)_ctx;
> > > > + struct virtio_net_hdr_v1_hash *hdr_hash;
> > > > + struct virtnet_info *vi;
> > > > +
> > > > + if (!(xdp->rxq->dev->features & NETIF_F_RXHASH))
> > >
> > > I think 'vi->has_rss_hash_report' should be used here.
> > > NETIF_F_RXHASH cannot guarantee that the hash report feature is negotiated,
> > > and accessing hash_report and hash_value is unsafe at this time.
> > >
> >
> > My understanding is that rxhash in dev->features is turned on only if
> > the corresponding hw feature is set. We will double check on this.
> >
> > > > + return -ENODATA;
> > > > +
> > > > + vi = netdev_priv(xdp->rxq->dev);
> > > > + hdr_hash = (struct virtio_net_hdr_v1_hash *)(xdp->data - vi->hdr_len);
> > >
> > > If the virtio-net-hdr is overrided by the XDP prog, how can this be done
> > > correctly and as expected?
> > >
> >
> > Yeah, thanks for bringing up this concern. We are actively working on
> > a fix of this issue(possibly with a wrapper structure of xdp_buff).
>
> Are there some places to save the hash before run xdp?
>
Yeah, it will be saved in a wrapper structure. Thanks.
> Thanks.
>
>
> >
> > Thanks,
> > Liang
> >
> > > Thanks,
> > > Heng
> > >
> > > > +
> > > > + switch (__le16_to_cpu(hdr_hash->hash_report)) {
> > > > + case VIRTIO_NET_HASH_REPORT_TCPv4:
> > > > + *rss_type = XDP_RSS_TYPE_L4_IPV4_TCP;
> > > > + break;
> > > > + case VIRTIO_NET_HASH_REPORT_UDPv4:
> > > > + *rss_type = XDP_RSS_TYPE_L4_IPV4_UDP;
> > > > + break;
> > > > + case VIRTIO_NET_HASH_REPORT_TCPv6:
> > > > + *rss_type = XDP_RSS_TYPE_L4_IPV6_TCP;
> > > > + break;
> > > > + case VIRTIO_NET_HASH_REPORT_UDPv6:
> > > > + *rss_type = XDP_RSS_TYPE_L4_IPV6_UDP;
> > > > + break;
> > > > + case VIRTIO_NET_HASH_REPORT_TCPv6_EX:
> > > > + *rss_type = XDP_RSS_TYPE_L4_IPV6_TCP_EX;
> > > > + break;
> > > > + case VIRTIO_NET_HASH_REPORT_UDPv6_EX:
> > > > + *rss_type = XDP_RSS_TYPE_L4_IPV6_UDP_EX;
> > > > + break;
> > > > + case VIRTIO_NET_HASH_REPORT_IPv4:
> > > > + *rss_type = XDP_RSS_TYPE_L3_IPV4;
> > > > + break;
> > > > + case VIRTIO_NET_HASH_REPORT_IPv6:
> > > > + *rss_type = XDP_RSS_TYPE_L3_IPV6;
> > > > + break;
> > > > + case VIRTIO_NET_HASH_REPORT_IPv6_EX:
> > > > + *rss_type = XDP_RSS_TYPE_L3_IPV6_EX;
> > > > + break;
> > > > + case VIRTIO_NET_HASH_REPORT_NONE:
> > > > + default:
> > > > + *rss_type = XDP_RSS_TYPE_NONE;
> > > > + }
> > > > +
> > > > + *hash = __le32_to_cpu(hdr_hash->hash_value);
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static const struct xdp_metadata_ops virtnet_xdp_metadata_ops = {
> > > > + .xmo_rx_hash = virtnet_xdp_rx_hash,
> > > > +};
> > > > +
> > > > static int virtnet_probe(struct virtio_device *vdev)
> > > > {
> > > > int i, err = -ENOMEM;
> > > > @@ -4613,6 +4667,8 @@ static int virtnet_probe(struct virtio_device *vdev)
> > > > dev->ethtool_ops = &virtnet_ethtool_ops;
> > > > SET_NETDEV_DEV(dev, &vdev->dev);
> > > >
> > > > + dev->xdp_metadata_ops = &virtnet_xdp_metadata_ops;
> > > > +
> > > > /* Do we support "hardware" checksums? */
> > > > if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) {
> > > > /* This opens up the world of extra features. */
> > >
> >
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-01-24 8:05 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-22 10:22 [PATCH] virtio_net: Support RX hash XDP hint Liang Chen
2024-01-22 11:10 ` Heng Qi
2024-01-24 2:04 ` Liang Chen
2024-01-24 6:05 ` Xuan Zhuo
2024-01-24 8:04 ` Liang Chen
2024-01-23 7:02 ` Michael S. Tsirkin
2024-01-24 2:27 ` Liang Chen
2024-01-24 2:12 ` Jason Wang
2024-01-24 8:03 ` Liang Chen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).