netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next] virtio-net: per-queue RPS config
@ 2019-01-18  1:08 Willem de Bruijn
  2019-01-18  1:43 ` Michael S. Tsirkin
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Willem de Bruijn @ 2019-01-18  1:08 UTC (permalink / raw)
  To: netdev; +Cc: davem, mst, jasowang, Willem de Bruijn, Mark Hlady

From: Willem de Bruijn <willemb@google.com>

On multiqueue network devices, RPS maps are configured independently
for each receive queue through /sys/class/net/$DEV/queues/rx-*.

On virtio-net currently all packets use the map from rx-0, because the
real rx queue is not known at time of map lookup by get_rps_cpu.

Call skb_record_rx_queue in the driver rx path to make lookup work.

Recording the receive queue has ramifications beyond RPS, such as in
sticky load balancing decisions for sockets (skb_tx_hash) and XPS.

Reported-by: Mark Hlady <mhlady@google.com>
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 drivers/net/virtio_net.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 0237250860467..236ba5d5fb4bb 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1035,6 +1035,7 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
 		goto frame_err;
 	}
 
+	skb_record_rx_queue(skb, vq2rxq(rq->vq));
 	skb->protocol = eth_type_trans(skb, dev);
 	pr_debug("Receiving skb proto 0x%04x len %i type %i\n",
 		 ntohs(skb->protocol), skb->len, skb->pkt_type);
-- 
2.20.1.321.g9e740568ce-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next] virtio-net: per-queue RPS config
  2019-01-18  1:08 [PATCH net-next] virtio-net: per-queue RPS config Willem de Bruijn
@ 2019-01-18  1:43 ` Michael S. Tsirkin
  2019-01-18  3:37   ` Willem de Bruijn
  2019-01-18  3:49 ` Jason Wang
  2019-01-19 18:05 ` David Miller
  2 siblings, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2019-01-18  1:43 UTC (permalink / raw)
  To: Willem de Bruijn; +Cc: netdev, davem, jasowang, Willem de Bruijn, Mark Hlady

On Thu, Jan 17, 2019 at 08:08:53PM -0500, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
> 
> On multiqueue network devices, RPS maps are configured independently
> for each receive queue through /sys/class/net/$DEV/queues/rx-*.
> 
> On virtio-net currently all packets use the map from rx-0, because the
> real rx queue is not known at time of map lookup by get_rps_cpu.
> 
> Call skb_record_rx_queue in the driver rx path to make lookup work.
> 
> Recording the receive queue has ramifications beyond RPS, such as in
> sticky load balancing decisions for sockets (skb_tx_hash) and XPS.
> 
> Reported-by: Mark Hlady <mhlady@google.com>
> Signed-off-by: Willem de Bruijn <willemb@google.com>

And any examples how to see the benefit of this?


> ---
>  drivers/net/virtio_net.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 0237250860467..236ba5d5fb4bb 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1035,6 +1035,7 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
>  		goto frame_err;
>  	}
>  
> +	skb_record_rx_queue(skb, vq2rxq(rq->vq));
>  	skb->protocol = eth_type_trans(skb, dev);
>  	pr_debug("Receiving skb proto 0x%04x len %i type %i\n",
>  		 ntohs(skb->protocol), skb->len, skb->pkt_type);
> -- 
> 2.20.1.321.g9e740568ce-goog

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next] virtio-net: per-queue RPS config
  2019-01-18  1:43 ` Michael S. Tsirkin
@ 2019-01-18  3:37   ` Willem de Bruijn
  2019-01-18  3:59     ` Michael S. Tsirkin
  0 siblings, 1 reply; 6+ messages in thread
From: Willem de Bruijn @ 2019-01-18  3:37 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Network Development, David Miller, Jason Wang, Willem de Bruijn,
	Mark Hlady

On Thu, Jan 17, 2019 at 8:43 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Thu, Jan 17, 2019 at 08:08:53PM -0500, Willem de Bruijn wrote:
> > From: Willem de Bruijn <willemb@google.com>
> >
> > On multiqueue network devices, RPS maps are configured independently
> > for each receive queue through /sys/class/net/$DEV/queues/rx-*.
> >
> > On virtio-net currently all packets use the map from rx-0, because the
> > real rx queue is not known at time of map lookup by get_rps_cpu.
> >
> > Call skb_record_rx_queue in the driver rx path to make lookup work.
> >
> > Recording the receive queue has ramifications beyond RPS, such as in
> > sticky load balancing decisions for sockets (skb_tx_hash) and XPS.
> >
> > Reported-by: Mark Hlady <mhlady@google.com>
> > Signed-off-by: Willem de Bruijn <willemb@google.com>
>
> And any examples how to see the benefit of this?

When there are fewer queues than cpus and rps is used to spread load
across all cpus, it can be preferable to setup disjoint sets, such
that each cpu handling an rxq interrupt spreads to an exclusive set of
neighbors instead of having all interrupt handling cores contend on
all other cores' softnet_data.

More subtly, even if the policy is to spread uniformly, it can be
preferable to set the RPS map to all cores except the core that
handled the interrupt, as it already had to do some work in the
initial receive path.

It is also simply expected behavior for network devices to be able to
configure rxq rps maps individually, so the current silent fallback to
rx0 is confusing, especially since rx-1/rps_cpus, .. rx-n/rps_cpus
files do exist and can be configured.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next] virtio-net: per-queue RPS config
  2019-01-18  1:08 [PATCH net-next] virtio-net: per-queue RPS config Willem de Bruijn
  2019-01-18  1:43 ` Michael S. Tsirkin
@ 2019-01-18  3:49 ` Jason Wang
  2019-01-19 18:05 ` David Miller
  2 siblings, 0 replies; 6+ messages in thread
From: Jason Wang @ 2019-01-18  3:49 UTC (permalink / raw)
  To: Willem de Bruijn, netdev; +Cc: davem, mst, Willem de Bruijn, Mark Hlady


On 2019/1/18 上午9:08, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
>
> On multiqueue network devices, RPS maps are configured independently
> for each receive queue through /sys/class/net/$DEV/queues/rx-*.
>
> On virtio-net currently all packets use the map from rx-0, because the
> real rx queue is not known at time of map lookup by get_rps_cpu.
>
> Call skb_record_rx_queue in the driver rx path to make lookup work.
>
> Recording the receive queue has ramifications beyond RPS, such as in
> sticky load balancing decisions for sockets (skb_tx_hash) and XPS.
>
> Reported-by: Mark Hlady <mhlady@google.com>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> ---
>   drivers/net/virtio_net.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 0237250860467..236ba5d5fb4bb 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1035,6 +1035,7 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
>   		goto frame_err;
>   	}
>   
> +	skb_record_rx_queue(skb, vq2rxq(rq->vq));
>   	skb->protocol = eth_type_trans(skb, dev);
>   	pr_debug("Receiving skb proto 0x%04x len %i type %i\n",
>   		 ntohs(skb->protocol), skb->len, skb->pkt_type);


Acked-by: Jason Wang <jasowang@redhat.com>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next] virtio-net: per-queue RPS config
  2019-01-18  3:37   ` Willem de Bruijn
@ 2019-01-18  3:59     ` Michael S. Tsirkin
  0 siblings, 0 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2019-01-18  3:59 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: Network Development, David Miller, Jason Wang, Willem de Bruijn,
	Mark Hlady

On Thu, Jan 17, 2019 at 10:37:26PM -0500, Willem de Bruijn wrote:
> On Thu, Jan 17, 2019 at 8:43 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Thu, Jan 17, 2019 at 08:08:53PM -0500, Willem de Bruijn wrote:
> > > From: Willem de Bruijn <willemb@google.com>
> > >
> > > On multiqueue network devices, RPS maps are configured independently
> > > for each receive queue through /sys/class/net/$DEV/queues/rx-*.
> > >
> > > On virtio-net currently all packets use the map from rx-0, because the
> > > real rx queue is not known at time of map lookup by get_rps_cpu.
> > >
> > > Call skb_record_rx_queue in the driver rx path to make lookup work.
> > >
> > > Recording the receive queue has ramifications beyond RPS, such as in
> > > sticky load balancing decisions for sockets (skb_tx_hash) and XPS.
> > >
> > > Reported-by: Mark Hlady <mhlady@google.com>
> > > Signed-off-by: Willem de Bruijn <willemb@google.com>
> >
> > And any examples how to see the benefit of this?
> 
> When there are fewer queues than cpus and rps is used to spread load
> across all cpus, it can be preferable to setup disjoint sets, such
> that each cpu handling an rxq interrupt spreads to an exclusive set of
> neighbors instead of having all interrupt handling cores contend on
> all other cores' softnet_data.
> 
> More subtly, even if the policy is to spread uniformly, it can be
> preferable to set the RPS map to all cores except the core that
> handled the interrupt, as it already had to do some work in the
> initial receive path.
> 
> It is also simply expected behavior for network devices to be able to
> configure rxq rps maps individually, so the current silent fallback to
> rx0 is confusing, especially since rx-1/rps_cpus, .. rx-n/rps_cpus
> files do exist and can be configured.

OK I think I got it.

Acked-by: Michael S. Tsirkin <mst@redhat.com>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next] virtio-net: per-queue RPS config
  2019-01-18  1:08 [PATCH net-next] virtio-net: per-queue RPS config Willem de Bruijn
  2019-01-18  1:43 ` Michael S. Tsirkin
  2019-01-18  3:49 ` Jason Wang
@ 2019-01-19 18:05 ` David Miller
  2 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2019-01-19 18:05 UTC (permalink / raw)
  To: willemdebruijn.kernel; +Cc: netdev, mst, jasowang, willemb, mhlady

From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Thu, 17 Jan 2019 20:08:53 -0500

> From: Willem de Bruijn <willemb@google.com>
> 
> On multiqueue network devices, RPS maps are configured independently
> for each receive queue through /sys/class/net/$DEV/queues/rx-*.
> 
> On virtio-net currently all packets use the map from rx-0, because the
> real rx queue is not known at time of map lookup by get_rps_cpu.
> 
> Call skb_record_rx_queue in the driver rx path to make lookup work.
> 
> Recording the receive queue has ramifications beyond RPS, such as in
> sticky load balancing decisions for sockets (skb_tx_hash) and XPS.
> 
> Reported-by: Mark Hlady <mhlady@google.com>
> Signed-off-by: Willem de Bruijn <willemb@google.com>

Applied, thanks Willem.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-01-19 18:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-18  1:08 [PATCH net-next] virtio-net: per-queue RPS config Willem de Bruijn
2019-01-18  1:43 ` Michael S. Tsirkin
2019-01-18  3:37   ` Willem de Bruijn
2019-01-18  3:59     ` Michael S. Tsirkin
2019-01-18  3:49 ` Jason Wang
2019-01-19 18:05 ` David Miller

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).