Re: [net-next PATCH v4 5/7] net: Enable Tx queue selection based on Rx queues
On 6/27/2018 3:47 AM, Willem de Bruijn wrote:
+static int get_xps_queue(struct net_device *dev, struct sk_buff *skb)
{
#ifdef CONFIG_XPS
struct xps_dev_maps *dev_maps;
- struct xps_map *map;
+ struct sock *sk = skb->sk;
int queue_index = -1;
if (!static_key_false(_needed))
return -1;
rcu_read_lock();
- dev_maps = rcu_dereference(dev->xps_cpus_map);
+ if (!static_key_false(_rxqs_needed))
+ goto get_cpus_map;
+
+ dev_maps = rcu_dereference(dev->xps_rxqs_map);
if (dev_maps) {
- unsigned int tci = skb->sender_cpu - 1;
+ int tci = sk_rx_queue_get(sk);
>>>
>>> What if the rx device differs from the tx device?
>>>
>> I think I have 3 options here:
>> 1. Cache the ifindex in sock_common which will introduce a new
>> additional field in sock_common.
>> 2. Use dev_get_by_napi_id to get the device id. This could be expensive,
>> if the rxqs_map is set, this will be done on every packet and involves
>> walking through the hashlist for napi_id lookup.
>
> The tx queue mapping is cached in the sk for connected sockets, but
> indeed this would be expensive for many workloads.
>
>> 3. Remove validating device id, similar to how it is in skb_tx_hash
>> where rx_queue recorded is used and if not, fall through to flow hash
>> calculation.
>> What do you think is suitable here?
>
> Alternatively, just accept the misprediction in this rare case. But do
> make the caveat explicit in the documentation.
>
Okay, I will add this in the documentation.
Re: [net-next PATCH v4 5/7] net: Enable Tx queue selection based on Rx queues
> >> +static int get_xps_queue(struct net_device *dev, struct sk_buff *skb)
> >> {
> >> #ifdef CONFIG_XPS
> >> struct xps_dev_maps *dev_maps;
> >> - struct xps_map *map;
> >> + struct sock *sk = skb->sk;
> >> int queue_index = -1;
> >>
> >> if (!static_key_false(_needed))
> >> return -1;
> >>
> >> rcu_read_lock();
> >> - dev_maps = rcu_dereference(dev->xps_cpus_map);
> >> + if (!static_key_false(_rxqs_needed))
> >> + goto get_cpus_map;
> >> +
> >> + dev_maps = rcu_dereference(dev->xps_rxqs_map);
> >> if (dev_maps) {
> >> - unsigned int tci = skb->sender_cpu - 1;
> >> + int tci = sk_rx_queue_get(sk);
> >
> > What if the rx device differs from the tx device?
> >
> I think I have 3 options here:
> 1. Cache the ifindex in sock_common which will introduce a new
> additional field in sock_common.
> 2. Use dev_get_by_napi_id to get the device id. This could be expensive,
> if the rxqs_map is set, this will be done on every packet and involves
> walking through the hashlist for napi_id lookup.
The tx queue mapping is cached in the sk for connected sockets, but
indeed this would be expensive for many workloads.
> 3. Remove validating device id, similar to how it is in skb_tx_hash
> where rx_queue recorded is used and if not, fall through to flow hash
> calculation.
> What do you think is suitable here?
Alternatively, just accept the misprediction in this rare case. But do
make the caveat explicit in the documentation.
Re: [net-next PATCH v4 5/7] net: Enable Tx queue selection based on Rx queues
On 6/26/2018 4:04 AM, Willem de Bruijn wrote:
> On Mon, Jun 25, 2018 at 7:06 PM Amritha Nambiar
> wrote:
>>
>> This patch adds support to pick Tx queue based on the Rx queue(s) map
>> configuration set by the admin through the sysfs attribute
>> for each Tx queue. If the user configuration for receive queue(s) map
>> does not apply, then the Tx queue selection falls back to CPU(s) map
>> based selection and finally to hashing.
>>
>> Signed-off-by: Amritha Nambiar
>> ---
>
>> +static int get_xps_queue(struct net_device *dev, struct sk_buff *skb)
>> {
>> #ifdef CONFIG_XPS
>> struct xps_dev_maps *dev_maps;
>> - struct xps_map *map;
>> + struct sock *sk = skb->sk;
>> int queue_index = -1;
>>
>> if (!static_key_false(_needed))
>> return -1;
>>
>> rcu_read_lock();
>> - dev_maps = rcu_dereference(dev->xps_cpus_map);
>> + if (!static_key_false(_rxqs_needed))
>> + goto get_cpus_map;
>> +
>> + dev_maps = rcu_dereference(dev->xps_rxqs_map);
>> if (dev_maps) {
>> - unsigned int tci = skb->sender_cpu - 1;
>> + int tci = sk_rx_queue_get(sk);
>
> What if the rx device differs from the tx device?
>
I think I have 3 options here:
1. Cache the ifindex in sock_common which will introduce a new
additional field in sock_common.
2. Use dev_get_by_napi_id to get the device id. This could be expensive,
if the rxqs_map is set, this will be done on every packet and involves
walking through the hashlist for napi_id lookup.
3. Remove validating device id, similar to how it is in skb_tx_hash
where rx_queue recorded is used and if not, fall through to flow hash
calculation.
What do you think is suitable here?
Re: [net-next PATCH v4 5/7] net: Enable Tx queue selection based on Rx queues
On Mon, Jun 25, 2018 at 7:06 PM Amritha Nambiar
wrote:
>
> This patch adds support to pick Tx queue based on the Rx queue(s) map
> configuration set by the admin through the sysfs attribute
> for each Tx queue. If the user configuration for receive queue(s) map
> does not apply, then the Tx queue selection falls back to CPU(s) map
> based selection and finally to hashing.
>
> Signed-off-by: Amritha Nambiar
> ---
> +static int get_xps_queue(struct net_device *dev, struct sk_buff *skb)
> {
> #ifdef CONFIG_XPS
> struct xps_dev_maps *dev_maps;
> - struct xps_map *map;
> + struct sock *sk = skb->sk;
> int queue_index = -1;
>
> if (!static_key_false(_needed))
> return -1;
>
> rcu_read_lock();
> - dev_maps = rcu_dereference(dev->xps_cpus_map);
> + if (!static_key_false(_rxqs_needed))
> + goto get_cpus_map;
> +
> + dev_maps = rcu_dereference(dev->xps_rxqs_map);
> if (dev_maps) {
> - unsigned int tci = skb->sender_cpu - 1;
> + int tci = sk_rx_queue_get(sk);
What if the rx device differs from the tx device?
Re: [net-next PATCH v4 5/7] net: Enable Tx queue selection based on Rx queues
Hi Amritha,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net-next/master]
url:
https://github.com/0day-ci/linux/commits/Amritha-Nambiar/Symmetric-queue-selection-using-XPS-for-Rx-queues/20180626-070944
config: i386-randconfig-x078-201825 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
In file included from include/net/cls_cgroup.h:19:0,
from net/socket.c:98:
include/net/sock.h: In function 'sk_rx_queue_get':
>> include/net/sock.h:1715:16: error: 'const struct sock' has no member named
>> 'sk_rx_queue_mapping'
return sk ? sk->sk_rx_queue_mapping - 1 : -1;
^~
vim +1715 include/net/sock.h
1712
1713 static inline int sk_rx_queue_get(const struct sock *sk)
1714 {
> 1715 return sk ? sk->sk_rx_queue_mapping - 1 : -1;
1716 }
1717 static inline void sk_set_socket(struct sock *sk, struct socket *sock)
1718 {
1719 sk_tx_queue_clear(sk);
1720 sk->sk_socket = sock;
1721 }
1722
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
[net-next PATCH v4 5/7] net: Enable Tx queue selection based on Rx queues
This patch adds support to pick Tx queue based on the Rx queue(s) map
configuration set by the admin through the sysfs attribute
for each Tx queue. If the user configuration for receive queue(s) map
does not apply, then the Tx queue selection falls back to CPU(s) map
based selection and finally to hashing.
Signed-off-by: Amritha Nambiar
---
include/net/sock.h |4 +++
net/core/dev.c | 62 ++--
2 files changed, 49 insertions(+), 17 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index 0ff4416..cb18139 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1710,6 +1710,10 @@ static inline void sk_rx_queue_set(struct sock *sk,
const struct sk_buff *skb)
#endif
}
+static inline int sk_rx_queue_get(const struct sock *sk)
+{
+ return sk ? sk->sk_rx_queue_mapping - 1 : -1;
+}
static inline void sk_set_socket(struct sock *sk, struct socket *sock)
{
sk_tx_queue_clear(sk);
diff --git a/net/core/dev.c b/net/core/dev.c
index df2a78d..2450c5e 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3454,35 +3454,63 @@ sch_handle_egress(struct sk_buff *skb, int *ret, struct
net_device *dev)
}
#endif /* CONFIG_NET_EGRESS */
-static inline int get_xps_queue(struct net_device *dev, struct sk_buff *skb)
+#ifdef CONFIG_XPS
+static int __get_xps_queue_idx(struct net_device *dev, struct sk_buff *skb,
+ struct xps_dev_maps *dev_maps, unsigned int tci)
+{
+ struct xps_map *map;
+ int queue_index = -1;
+
+ if (dev->num_tc) {
+ tci *= dev->num_tc;
+ tci += netdev_get_prio_tc_map(dev, skb->priority);
+ }
+
+ map = rcu_dereference(dev_maps->attr_map[tci]);
+ if (map) {
+ if (map->len == 1)
+ queue_index = map->queues[0];
+ else
+ queue_index = map->queues[reciprocal_scale(
+ skb_get_hash(skb), map->len)];
+ if (unlikely(queue_index >= dev->real_num_tx_queues))
+ queue_index = -1;
+ }
+ return queue_index;
+}
+#endif
+
+static int get_xps_queue(struct net_device *dev, struct sk_buff *skb)
{
#ifdef CONFIG_XPS
struct xps_dev_maps *dev_maps;
- struct xps_map *map;
+ struct sock *sk = skb->sk;
int queue_index = -1;
if (!static_key_false(_needed))
return -1;
rcu_read_lock();
- dev_maps = rcu_dereference(dev->xps_cpus_map);
+ if (!static_key_false(_rxqs_needed))
+ goto get_cpus_map;
+
+ dev_maps = rcu_dereference(dev->xps_rxqs_map);
if (dev_maps) {
- unsigned int tci = skb->sender_cpu - 1;
+ int tci = sk_rx_queue_get(sk);
- if (dev->num_tc) {
- tci *= dev->num_tc;
- tci += netdev_get_prio_tc_map(dev, skb->priority);
- }
+ if (tci >= 0 && tci < dev->num_rx_queues)
+ queue_index = __get_xps_queue_idx(dev, skb, dev_maps,
+ tci);
+ }
- map = rcu_dereference(dev_maps->attr_map[tci]);
- if (map) {
- if (map->len == 1)
- queue_index = map->queues[0];
- else
- queue_index =
map->queues[reciprocal_scale(skb_get_hash(skb),
-
map->len)];
- if (unlikely(queue_index >= dev->real_num_tx_queues))
- queue_index = -1;
+get_cpus_map:
+ if (queue_index < 0) {
+ dev_maps = rcu_dereference(dev->xps_cpus_map);
+ if (dev_maps) {
+ unsigned int tci = skb->sender_cpu - 1;
+
+ queue_index = __get_xps_queue_idx(dev, skb, dev_maps,
+ tci);
}
}
rcu_read_unlock();
