Re: [PATCH 2/4] drivers/net/virtio_net: Added basic RSS support.
Hi all
> Is this correct if both mergeable_rx_bufs and hash_report are set?
Yes, there is a similar code in qemu.
> Can we simply do virtio_cread_feature(vdev, VIRTIO_NET_F_MQ |
> VIRTIO_NET_F_RSS, ...) ?
No, VIRTIO_NET_F_* is bit offset - so in the end "1 <<
(VIRTIO_NET_F_MQ | VIRTIO_NET_F_RSS)" is not valid.
> Is rtnl_lock() really needed here consider we haven't even register netdev?
I'll remove rtnl lock.
> Generally best to avoid __packed.
I'll refactor the structure.
On Tue, Jan 11, 2022 at 2:00 PM Michael S. Tsirkin wrote:
>
> On Sun, Jan 09, 2022 at 11:06:57PM +0200, Andrew Melnychenko wrote:
> > Added features for RSS.
> > Added initialization, RXHASH feature and ethtool ops.
> > By default RSS/RXHASH is disabled.
> > Virtio RSS "IPv6 extensions" hashes disabled.
> > Added ethtools ops to set key and indirection table.
> >
> > Signed-off-by: Andrew Melnychenko
> > ---
> > drivers/net/virtio_net.c | 194 +--
> > 1 file changed, 184 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 66439ca488f4..21794731fc75 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -169,6 +169,28 @@ struct receive_queue {
> > struct xdp_rxq_info xdp_rxq;
> > };
> >
> > +/* This structure can contain rss message with maximum settings for
> > indirection table and keysize
> > + * Note, that default structure that describes RSS configuration
> > virtio_net_rss_config
> > + * contains same info but can't handle table values.
> > + * In any case, structure would be passed to virtio hw through sg_buf
> > split by parts
> > + * because table sizes may be differ according to the device configuration.
> > + */
> > +#define VIRTIO_NET_RSS_MAX_KEY_SIZE 40
> > +#define VIRTIO_NET_RSS_MAX_TABLE_LEN128
> > +struct virtio_net_ctrl_rss {
> > + struct {
> > + __le32 hash_types;
> > + __le16 indirection_table_mask;
> > + __le16 unclassified_queue;
> > + } __packed table_info;
> > + u16 indirection_table[VIRTIO_NET_RSS_MAX_TABLE_LEN];
> > + struct {
> > + u16 max_tx_vq; /* queues */
> > + u8 hash_key_length;
> > + } __packed key_info;
> > + u8 key[VIRTIO_NET_RSS_MAX_KEY_SIZE];
> > +};
> > +
>
> Generally best to avoid __packed.
> I think it's not a bad idea to just follow the spec when
> you lay out the structures. Makes it easier to follow
> that it matches. Spec has just a single struct:
>
> struct virtio_net_rss_config {
> le32 hash_types;
> le16 indirection_table_mask;
> le16 unclassified_queue;
> le16 indirection_table[indirection_table_length];
> le16 max_tx_vq;
> u8 hash_key_length;
> u8 hash_key_data[hash_key_length];
> };
>
> and with this layout you don't need __packed.
>
>
>
> > /* Control VQ buffers: protected by the rtnl lock */
> > struct control_buf {
> > struct virtio_net_ctrl_hdr hdr;
> > @@ -178,6 +200,7 @@ struct control_buf {
> > u8 allmulti;
> > __virtio16 vid;
> > __virtio64 offloads;
> > + struct virtio_net_ctrl_rss rss;
> > };
> >
> > struct virtnet_info {
> > @@ -206,6 +229,12 @@ struct virtnet_info {
> > /* Host will merge rx buffers for big packets (shake it! shake it!) */
> > bool mergeable_rx_bufs;
> >
> > + /* Host supports rss and/or hash report */
> > + bool has_rss;
> > + u8 rss_key_size;
> > + u16 rss_indir_table_size;
> > + u32 rss_hash_types_supported;
> > +
> > /* Has control virtqueue */
> > bool has_cvq;
> >
> > @@ -395,9 +424,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info
> > *vi,
> > hdr_p = p;
> >
> > hdr_len = vi->hdr_len;
> > - if (vi->has_rss_hash_report)
> > - hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
> > - else if (vi->mergeable_rx_bufs)
> > + if (vi->mergeable_rx_bufs)
> > hdr_padded_len = sizeof(*hdr);
> > else
> > hdr_padded_len = sizeof(struct padded_vnet_hdr);
> > @@ -2184,6 +2211,55 @@ static void virtnet_get_ringparam(struct net_device
> > *dev,
> > ring->tx_pending = ring->tx_max_pending;
> > }
> >
> > +static bool virtnet_commit_rss_command(struct virtnet_info *vi)
> > +{
> > + struct net_device *dev = vi->dev;
> > + struct scatterlist sgs[4];
> > + unsigned int sg_buf_size;
> > +
> > + /* prepare sgs */
> > + sg_init_table(sgs, 4);
> > +
> > + sg_buf_size = sizeof(vi->ctrl->rss.table_info);
> > + sg_set_buf([0], >ctrl->rss.table_info, sg_buf_size);
> > +
> > + sg_buf_size = sizeof(uint16_t) * vi->rss_indir_table_size;
> > + sg_set_buf([1], vi->ctrl->rss.indirection_table, sg_buf_size);
> > +
> > + sg_buf_size = sizeof(vi->ctrl->rss.key_info);
> > + sg_set_buf([2], >ctrl->rss.key_info, sg_buf_size);
> > +
> > + sg_buf_size = vi->rss_key_size;
> > + sg_set_buf([3], vi->ctrl->rss.key,
Re: [PATCH 2/4] drivers/net/virtio_net: Added basic RSS support.
On Sun, Jan 09, 2022 at 11:06:57PM +0200, Andrew Melnychenko wrote:
> Added features for RSS.
> Added initialization, RXHASH feature and ethtool ops.
> By default RSS/RXHASH is disabled.
> Virtio RSS "IPv6 extensions" hashes disabled.
> Added ethtools ops to set key and indirection table.
>
> Signed-off-by: Andrew Melnychenko
> ---
> drivers/net/virtio_net.c | 194 +--
> 1 file changed, 184 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 66439ca488f4..21794731fc75 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -169,6 +169,28 @@ struct receive_queue {
> struct xdp_rxq_info xdp_rxq;
> };
>
> +/* This structure can contain rss message with maximum settings for
> indirection table and keysize
> + * Note, that default structure that describes RSS configuration
> virtio_net_rss_config
> + * contains same info but can't handle table values.
> + * In any case, structure would be passed to virtio hw through sg_buf split
> by parts
> + * because table sizes may be differ according to the device configuration.
> + */
> +#define VIRTIO_NET_RSS_MAX_KEY_SIZE 40
> +#define VIRTIO_NET_RSS_MAX_TABLE_LEN128
> +struct virtio_net_ctrl_rss {
> + struct {
> + __le32 hash_types;
> + __le16 indirection_table_mask;
> + __le16 unclassified_queue;
> + } __packed table_info;
> + u16 indirection_table[VIRTIO_NET_RSS_MAX_TABLE_LEN];
> + struct {
> + u16 max_tx_vq; /* queues */
> + u8 hash_key_length;
> + } __packed key_info;
> + u8 key[VIRTIO_NET_RSS_MAX_KEY_SIZE];
> +};
> +
Generally best to avoid __packed.
I think it's not a bad idea to just follow the spec when
you lay out the structures. Makes it easier to follow
that it matches. Spec has just a single struct:
struct virtio_net_rss_config {
le32 hash_types;
le16 indirection_table_mask;
le16 unclassified_queue;
le16 indirection_table[indirection_table_length];
le16 max_tx_vq;
u8 hash_key_length;
u8 hash_key_data[hash_key_length];
};
and with this layout you don't need __packed.
> /* Control VQ buffers: protected by the rtnl lock */
> struct control_buf {
> struct virtio_net_ctrl_hdr hdr;
> @@ -178,6 +200,7 @@ struct control_buf {
> u8 allmulti;
> __virtio16 vid;
> __virtio64 offloads;
> + struct virtio_net_ctrl_rss rss;
> };
>
> struct virtnet_info {
> @@ -206,6 +229,12 @@ struct virtnet_info {
> /* Host will merge rx buffers for big packets (shake it! shake it!) */
> bool mergeable_rx_bufs;
>
> + /* Host supports rss and/or hash report */
> + bool has_rss;
> + u8 rss_key_size;
> + u16 rss_indir_table_size;
> + u32 rss_hash_types_supported;
> +
> /* Has control virtqueue */
> bool has_cvq;
>
> @@ -395,9 +424,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info
> *vi,
> hdr_p = p;
>
> hdr_len = vi->hdr_len;
> - if (vi->has_rss_hash_report)
> - hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
> - else if (vi->mergeable_rx_bufs)
> + if (vi->mergeable_rx_bufs)
> hdr_padded_len = sizeof(*hdr);
> else
> hdr_padded_len = sizeof(struct padded_vnet_hdr);
> @@ -2184,6 +2211,55 @@ static void virtnet_get_ringparam(struct net_device
> *dev,
> ring->tx_pending = ring->tx_max_pending;
> }
>
> +static bool virtnet_commit_rss_command(struct virtnet_info *vi)
> +{
> + struct net_device *dev = vi->dev;
> + struct scatterlist sgs[4];
> + unsigned int sg_buf_size;
> +
> + /* prepare sgs */
> + sg_init_table(sgs, 4);
> +
> + sg_buf_size = sizeof(vi->ctrl->rss.table_info);
> + sg_set_buf([0], >ctrl->rss.table_info, sg_buf_size);
> +
> + sg_buf_size = sizeof(uint16_t) * vi->rss_indir_table_size;
> + sg_set_buf([1], vi->ctrl->rss.indirection_table, sg_buf_size);
> +
> + sg_buf_size = sizeof(vi->ctrl->rss.key_info);
> + sg_set_buf([2], >ctrl->rss.key_info, sg_buf_size);
> +
> + sg_buf_size = vi->rss_key_size;
> + sg_set_buf([3], vi->ctrl->rss.key, sg_buf_size);
> +
> + if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
> + VIRTIO_NET_CTRL_MQ_RSS_CONFIG, sgs)) {
> + dev_warn(>dev, "VIRTIONET issue with committing RSS
> sgs\n");
> + return false;
> + }
> + return true;
> +}
> +
> +static void virtnet_init_default_rss(struct virtnet_info *vi)
> +{
> + u32 indir_val = 0;
> + int i = 0;
> +
> + vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_supported;
> + vi->ctrl->rss.table_info.indirection_table_mask =
> vi->rss_indir_table_size - 1;
> + vi->ctrl->rss.table_info.unclassified_queue = 0;
> +
> + for (; i < vi->rss_indir_table_size; ++i) {
> + indir_val = ethtool_rxfh_indir_default(i, vi->max_queue_pairs);
> +
Re: [PATCH 2/4] drivers/net/virtio_net: Added basic RSS support.
在 2022/1/10 上午5:06, Andrew Melnychenko 写道:
Added features for RSS.
Added initialization, RXHASH feature and ethtool ops.
By default RSS/RXHASH is disabled.
Virtio RSS "IPv6 extensions" hashes disabled.
Added ethtools ops to set key and indirection table.
Signed-off-by: Andrew Melnychenko
---
drivers/net/virtio_net.c | 194 +--
1 file changed, 184 insertions(+), 10 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 66439ca488f4..21794731fc75 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -169,6 +169,28 @@ struct receive_queue {
struct xdp_rxq_info xdp_rxq;
};
+/* This structure can contain rss message with maximum settings for indirection table and keysize
+ * Note, that default structure that describes RSS configuration
virtio_net_rss_config
+ * contains same info but can't handle table values.
+ * In any case, structure would be passed to virtio hw through sg_buf split by
parts
+ * because table sizes may be differ according to the device configuration.
+ */
+#define VIRTIO_NET_RSS_MAX_KEY_SIZE 40
+#define VIRTIO_NET_RSS_MAX_TABLE_LEN128
+struct virtio_net_ctrl_rss {
+ struct {
+ __le32 hash_types;
+ __le16 indirection_table_mask;
+ __le16 unclassified_queue;
+ } __packed table_info;
+ u16 indirection_table[VIRTIO_NET_RSS_MAX_TABLE_LEN];
+ struct {
+ u16 max_tx_vq; /* queues */
+ u8 hash_key_length;
+ } __packed key_info;
+ u8 key[VIRTIO_NET_RSS_MAX_KEY_SIZE];
+};
We need to consider to tweak and use uAPI in the future, e.g split the
above into four parts:
1) first embed structure
2) indirection table
3) second embed structure
4) key array
1) and 3) could be uAPI.
+
/* Control VQ buffers: protected by the rtnl lock */
struct control_buf {
struct virtio_net_ctrl_hdr hdr;
@@ -178,6 +200,7 @@ struct control_buf {
u8 allmulti;
__virtio16 vid;
__virtio64 offloads;
+ struct virtio_net_ctrl_rss rss;
};
struct virtnet_info {
@@ -206,6 +229,12 @@ struct virtnet_info {
/* Host will merge rx buffers for big packets (shake it! shake it!) */
bool mergeable_rx_bufs;
+ /* Host supports rss and/or hash report */
+ bool has_rss;
+ u8 rss_key_size;
+ u16 rss_indir_table_size;
+ u32 rss_hash_types_supported;
+
/* Has control virtqueue */
bool has_cvq;
@@ -395,9 +424,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
hdr_p = p;
hdr_len = vi->hdr_len;
- if (vi->has_rss_hash_report)
- hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
- else if (vi->mergeable_rx_bufs)
+ if (vi->mergeable_rx_bufs)
hdr_padded_len = sizeof(*hdr);
Is this correct if both mergeable_rx_bufs and hash_report are set?
else
hdr_padded_len = sizeof(struct padded_vnet_hdr);
@@ -2184,6 +2211,55 @@ static void virtnet_get_ringparam(struct net_device *dev,
ring->tx_pending = ring->tx_max_pending;
}
+static bool virtnet_commit_rss_command(struct virtnet_info *vi)
+{
+ struct net_device *dev = vi->dev;
+ struct scatterlist sgs[4];
+ unsigned int sg_buf_size;
+
+ /* prepare sgs */
+ sg_init_table(sgs, 4);
+
+ sg_buf_size = sizeof(vi->ctrl->rss.table_info);
+ sg_set_buf([0], >ctrl->rss.table_info, sg_buf_size);
+
+ sg_buf_size = sizeof(uint16_t) * vi->rss_indir_table_size;
+ sg_set_buf([1], vi->ctrl->rss.indirection_table, sg_buf_size);
+
+ sg_buf_size = sizeof(vi->ctrl->rss.key_info);
+ sg_set_buf([2], >ctrl->rss.key_info, sg_buf_size);
+
+ sg_buf_size = vi->rss_key_size;
+ sg_set_buf([3], vi->ctrl->rss.key, sg_buf_size);
+
+ if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
+ VIRTIO_NET_CTRL_MQ_RSS_CONFIG, sgs)) {
+ dev_warn(>dev, "VIRTIONET issue with committing RSS
sgs\n");
+ return false;
+ }
+ return true;
+}
+
+static void virtnet_init_default_rss(struct virtnet_info *vi)
+{
+ u32 indir_val = 0;
+ int i = 0;
+
+ vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_supported;
+ vi->ctrl->rss.table_info.indirection_table_mask =
vi->rss_indir_table_size - 1;
+ vi->ctrl->rss.table_info.unclassified_queue = 0;
+
+ for (; i < vi->rss_indir_table_size; ++i) {
+ indir_val = ethtool_rxfh_indir_default(i, vi->max_queue_pairs);
+ vi->ctrl->rss.indirection_table[i] = indir_val;
+ }
+
+ vi->ctrl->rss.key_info.max_tx_vq = vi->curr_queue_pairs;
+ vi->ctrl->rss.key_info.hash_key_length = vi->rss_key_size;
+
+ netdev_rss_key_fill(vi->ctrl->rss.key, vi->rss_key_size);
+}
+
static void virtnet_get_drvinfo(struct net_device *dev,
[PATCH 2/4] drivers/net/virtio_net: Added basic RSS support.
Added features for RSS.
Added initialization, RXHASH feature and ethtool ops.
By default RSS/RXHASH is disabled.
Virtio RSS "IPv6 extensions" hashes disabled.
Added ethtools ops to set key and indirection table.
Signed-off-by: Andrew Melnychenko
---
drivers/net/virtio_net.c | 194 +--
1 file changed, 184 insertions(+), 10 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 66439ca488f4..21794731fc75 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -169,6 +169,28 @@ struct receive_queue {
struct xdp_rxq_info xdp_rxq;
};
+/* This structure can contain rss message with maximum settings for
indirection table and keysize
+ * Note, that default structure that describes RSS configuration
virtio_net_rss_config
+ * contains same info but can't handle table values.
+ * In any case, structure would be passed to virtio hw through sg_buf split by
parts
+ * because table sizes may be differ according to the device configuration.
+ */
+#define VIRTIO_NET_RSS_MAX_KEY_SIZE 40
+#define VIRTIO_NET_RSS_MAX_TABLE_LEN128
+struct virtio_net_ctrl_rss {
+ struct {
+ __le32 hash_types;
+ __le16 indirection_table_mask;
+ __le16 unclassified_queue;
+ } __packed table_info;
+ u16 indirection_table[VIRTIO_NET_RSS_MAX_TABLE_LEN];
+ struct {
+ u16 max_tx_vq; /* queues */
+ u8 hash_key_length;
+ } __packed key_info;
+ u8 key[VIRTIO_NET_RSS_MAX_KEY_SIZE];
+};
+
/* Control VQ buffers: protected by the rtnl lock */
struct control_buf {
struct virtio_net_ctrl_hdr hdr;
@@ -178,6 +200,7 @@ struct control_buf {
u8 allmulti;
__virtio16 vid;
__virtio64 offloads;
+ struct virtio_net_ctrl_rss rss;
};
struct virtnet_info {
@@ -206,6 +229,12 @@ struct virtnet_info {
/* Host will merge rx buffers for big packets (shake it! shake it!) */
bool mergeable_rx_bufs;
+ /* Host supports rss and/or hash report */
+ bool has_rss;
+ u8 rss_key_size;
+ u16 rss_indir_table_size;
+ u32 rss_hash_types_supported;
+
/* Has control virtqueue */
bool has_cvq;
@@ -395,9 +424,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
hdr_p = p;
hdr_len = vi->hdr_len;
- if (vi->has_rss_hash_report)
- hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
- else if (vi->mergeable_rx_bufs)
+ if (vi->mergeable_rx_bufs)
hdr_padded_len = sizeof(*hdr);
else
hdr_padded_len = sizeof(struct padded_vnet_hdr);
@@ -2184,6 +2211,55 @@ static void virtnet_get_ringparam(struct net_device *dev,
ring->tx_pending = ring->tx_max_pending;
}
+static bool virtnet_commit_rss_command(struct virtnet_info *vi)
+{
+ struct net_device *dev = vi->dev;
+ struct scatterlist sgs[4];
+ unsigned int sg_buf_size;
+
+ /* prepare sgs */
+ sg_init_table(sgs, 4);
+
+ sg_buf_size = sizeof(vi->ctrl->rss.table_info);
+ sg_set_buf([0], >ctrl->rss.table_info, sg_buf_size);
+
+ sg_buf_size = sizeof(uint16_t) * vi->rss_indir_table_size;
+ sg_set_buf([1], vi->ctrl->rss.indirection_table, sg_buf_size);
+
+ sg_buf_size = sizeof(vi->ctrl->rss.key_info);
+ sg_set_buf([2], >ctrl->rss.key_info, sg_buf_size);
+
+ sg_buf_size = vi->rss_key_size;
+ sg_set_buf([3], vi->ctrl->rss.key, sg_buf_size);
+
+ if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
+ VIRTIO_NET_CTRL_MQ_RSS_CONFIG, sgs)) {
+ dev_warn(>dev, "VIRTIONET issue with committing RSS
sgs\n");
+ return false;
+ }
+ return true;
+}
+
+static void virtnet_init_default_rss(struct virtnet_info *vi)
+{
+ u32 indir_val = 0;
+ int i = 0;
+
+ vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_supported;
+ vi->ctrl->rss.table_info.indirection_table_mask =
vi->rss_indir_table_size - 1;
+ vi->ctrl->rss.table_info.unclassified_queue = 0;
+
+ for (; i < vi->rss_indir_table_size; ++i) {
+ indir_val = ethtool_rxfh_indir_default(i, vi->max_queue_pairs);
+ vi->ctrl->rss.indirection_table[i] = indir_val;
+ }
+
+ vi->ctrl->rss.key_info.max_tx_vq = vi->curr_queue_pairs;
+ vi->ctrl->rss.key_info.hash_key_length = vi->rss_key_size;
+
+ netdev_rss_key_fill(vi->ctrl->rss.key, vi->rss_key_size);
+}
+
static void virtnet_get_drvinfo(struct net_device *dev,
struct ethtool_drvinfo *info)
@@ -2412,6 +2488,71 @@ static void virtnet_update_settings(struct virtnet_info
*vi)
vi->duplex = duplex;
}
+static u32 virtnet_get_rxfh_key_size(struct net_device *dev)
+{
+ return ((struct virtnet_info *)netdev_priv(dev))->rss_key_size;
+}
+
+static u32
