From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
To: Tom Herbert <tom@herbertland.com>
Cc: Amritha Nambiar <amritha.nambiar@intel.com>,
Linux Kernel Network Developers <netdev@vger.kernel.org>,
"David S. Miller" <davem@davemloft.net>,
Alexander Duyck <alexander.h.duyck@intel.com>,
Sridhar Samudrala <sridhar.samudrala@intel.com>,
Eric Dumazet <edumazet@google.com>,
Hannes Frederic Sowa <hannes@stressinduktion.org>
Subject: Re: [net-next PATCH v2 2/4] net: Enable Tx queue selection based on Rx queues
Date: Sat, 19 May 2018 16:13:44 -0400 [thread overview]
Message-ID: <CAF=yD-JXpiJwxM_mHvAgJ6qhsgq4uOZYbsMBVvcOmZawbueayQ@mail.gmail.com> (raw)
In-Reply-To: <CALx6S3652mgSjpQtF+9N90CYYhvyaMjS94zbgyhcFAa_G0h4tQ@mail.gmail.com>
On Fri, May 18, 2018 at 12:03 AM, Tom Herbert <tom@herbertland.com> wrote:
> On Tue, May 15, 2018 at 6:26 PM, Amritha Nambiar
> <amritha.nambiar@intel.com> wrote:
>> This patch adds support to pick Tx queue based on the Rx queue map
>> configuration set by the admin through the sysfs attribute
>> for each Tx queue. If the user configuration for receive
>> queue map does not apply, then the Tx queue selection falls back
>> to CPU map based selection and finally to hashing.
>>
>> Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
>> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
>> ---
>> include/net/sock.h | 18 ++++++++++++++++++
>> net/core/dev.c | 36 +++++++++++++++++++++++++++++-------
>> net/core/sock.c | 5 +++++
>> net/ipv4/tcp_input.c | 7 +++++++
>> net/ipv4/tcp_ipv4.c | 1 +
>> net/ipv4/tcp_minisocks.c | 1 +
>> 6 files changed, 61 insertions(+), 7 deletions(-)
>>
>> diff --git a/include/net/sock.h b/include/net/sock.h
>> index 4f7c584..0613f63 100644
>> --- a/include/net/sock.h
>> +++ b/include/net/sock.h
>> @@ -139,6 +139,8 @@ typedef __u64 __bitwise __addrpair;
>> * @skc_node: main hash linkage for various protocol lookup tables
>> * @skc_nulls_node: main hash linkage for TCP/UDP/UDP-Lite protocol
>> * @skc_tx_queue_mapping: tx queue number for this connection
>> + * @skc_rx_queue_mapping: rx queue number for this connection
>> + * @skc_rx_ifindex: rx ifindex for this connection
>> * @skc_flags: place holder for sk_flags
>> * %SO_LINGER (l_onoff), %SO_BROADCAST, %SO_KEEPALIVE,
>> * %SO_OOBINLINE settings, %SO_TIMESTAMPING settings
>> @@ -215,6 +217,10 @@ struct sock_common {
>> struct hlist_nulls_node skc_nulls_node;
>> };
>> int skc_tx_queue_mapping;
>> +#ifdef CONFIG_XPS
>> + int skc_rx_queue_mapping;
>> + int skc_rx_ifindex;
>
> Isn't this increasing size of sock_common for a narrow use case functionality?
You can get the device from the already recorded sk_napi_id.
Sadly, not the queue number as far as I can see.
>> +static inline void sk_mark_rx_queue(struct sock *sk, struct sk_buff *skb)
>> +{
>> +#ifdef CONFIG_XPS
>> + sk->sk_rx_ifindex = skb->skb_iif;
>> + sk->sk_rx_queue_mapping = skb_get_rx_queue(skb);
>> +#endif
>> +}
>> +
Instead of adding this function and calls to it in many locations in
the stack, you can expand sk_mark_napi_id.
Also, it is not clear why this should be called in locations where
sk_mark_napi_id is not.
>> +static int get_xps_queue(struct net_device *dev, struct sk_buff *skb)
>> +{
>> +#ifdef CONFIG_XPS
>> + enum xps_map_type i = XPS_MAP_RXQS;
>> + struct xps_dev_maps *dev_maps;
>> + struct sock *sk = skb->sk;
>> + int queue_index = -1;
>> + unsigned int tci = 0;
>> +
>> + if (sk && sk->sk_rx_queue_mapping <= dev->real_num_rx_queues &&
>> + dev->ifindex == sk->sk_rx_ifindex)
>> + tci = sk->sk_rx_queue_mapping;
>> +
>> + rcu_read_lock();
>> + while (queue_index < 0 && i < __XPS_MAP_MAX) {
>> + if (i == XPS_MAP_CPUS)
>
> This while loop typifies exactly why I don't think the XPS maps should
> be an array.
+1
next prev parent reply other threads:[~2018-05-19 20:14 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-05-16 1:26 [net-next PATCH v2 0/4] Symmetric queue selection using XPS for Rx queues Amritha Nambiar
2018-05-16 1:26 ` [net-next PATCH v2 1/4] net: Refactor XPS for CPUs and " Amritha Nambiar
2018-05-17 18:38 ` David Miller
2018-05-17 18:47 ` Nambiar, Amritha
2018-05-18 4:08 ` Tom Herbert
2018-05-23 18:59 ` Nambiar, Amritha
2018-05-21 5:38 ` [lkp-robot] [net] 3416099d53: net/core/dev.c:#suspicious_rcu_dereference_protected()usage kernel test robot
2018-05-16 1:26 ` [net-next PATCH v2 2/4] net: Enable Tx queue selection based on Rx queues Amritha Nambiar
2018-05-18 4:03 ` Tom Herbert
2018-05-19 20:13 ` Willem de Bruijn [this message]
2018-05-19 20:27 ` Willem de Bruijn
2018-05-21 14:51 ` Tom Herbert
2018-05-21 15:12 ` Willem de Bruijn
2018-05-22 14:09 ` Tom Herbert
2018-05-23 19:31 ` Nambiar, Amritha
2018-05-23 19:19 ` Nambiar, Amritha
2018-05-16 1:26 ` [net-next PATCH v2 3/4] net-sysfs: Add interface for Rx queue map per Tx queue Amritha Nambiar
2018-05-17 19:05 ` Florian Fainelli
2018-05-17 22:43 ` Nambiar, Amritha
2018-05-16 1:27 ` [net-next PATCH v2 4/4] Documentation: Add explanation for XPS using Rx-queue map Amritha Nambiar
2018-05-18 4:11 ` [net-next PATCH v2 0/4] Symmetric queue selection using XPS for Rx queues Tom Herbert
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAF=yD-JXpiJwxM_mHvAgJ6qhsgq4uOZYbsMBVvcOmZawbueayQ@mail.gmail.com' \
--to=willemdebruijn.kernel@gmail.com \
--cc=alexander.h.duyck@intel.com \
--cc=amritha.nambiar@intel.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=hannes@stressinduktion.org \
--cc=netdev@vger.kernel.org \
--cc=sridhar.samudrala@intel.com \
--cc=tom@herbertland.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).