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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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. */
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 <[email protected]>
> ---
> 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
On Mon, Jan 22, 2024 at 7:10 PM Heng Qi <[email protected]> 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 <[email protected]>
> > ---
> > 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. */
>
On Mon, Jan 22, 2024 at 6:23 PM Liang Chen <[email protected]> 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 <[email protected]>
> ---
> 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
On Tue, Jan 23, 2024 at 3:02 PM Michael S. Tsirkin <[email protected]> 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 <[email protected]>
> > ---
> > 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
>
On Wed, 24 Jan 2024 10:04:51 +0800, Liang Chen <[email protected]> wrote:
> On Mon, Jan 22, 2024 at 7:10 PM Heng Qi <[email protected]> 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 <[email protected]>
> > > ---
> > > 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. */
> >
>
On Wed, Jan 24, 2024 at 10:12 AM Jason Wang <[email protected]> wrote:
>
> On Mon, Jan 22, 2024 at 6:23 PM Liang Chen <liangchen.linux@gmailcom> 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 <[email protected]>
> > ---
> > 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
>
On Wed, Jan 24, 2024 at 2:06 PM Xuan Zhuo <[email protected]> wrote:
>
> On Wed, 24 Jan 2024 10:04:51 +0800, Liang Chen <[email protected]> wrote:
> > On Mon, Jan 22, 2024 at 7:10 PM Heng Qi <[email protected]> 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 <[email protected]>
> > > > ---
> > > > 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. */
> > >
> >