| Message ID | 20190118010853.47325-1-willemdebruijn.kernel@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net-next] virtio-net: per-queue RPS config | expand |
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
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.
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>
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>
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.
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);