Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1602760 > unrolled thread

[net-next PATCH 0/5] Add busy poll support for epoll under certain circumstances

Back to article view | Back to linux.kernel

Contents

  [net-next PATCH 0/5] Add busy poll support for epoll under certain  circumstances Alexander Duyck <alexander.duyck@gmail.com> - 2017-03-16 19:40 +0100
    [net-next PATCH 1/5] net: Do not record sender_cpu as napi_id in  socket receive paths Alexander Duyck <alexander.duyck@gmail.com> - 2017-03-16 19:40 +0100
      Re: [net-next PATCH 1/5] net: Do not record sender_cpu as napi_id  in socket receive paths Eric Dumazet <eric.dumazet@gmail.com> - 2017-03-16 23:10 +0100
        Re: [net-next PATCH 1/5] net: Do not record sender_cpu as napi_id in  socket receive paths Alexander Duyck <alexander.duyck@gmail.com> - 2017-03-16 23:50 +0100
          Re: [net-next PATCH 1/5] net: Do not record sender_cpu as napi_id  in socket receive paths Eric Dumazet <eric.dumazet@gmail.com> - 2017-03-17 00:00 +0100
            Re: [net-next PATCH 1/5] net: Do not record sender_cpu as napi_id in  socket receive paths Alexander Duyck <alexander.duyck@gmail.com> - 2017-03-17 03:50 +0100
              Re: [net-next PATCH 1/5] net: Do not record sender_cpu as napi_id  in socket receive paths Eric Dumazet <eric.dumazet@gmail.com> - 2017-03-17 04:00 +0100
              Re: [net-next PATCH 1/5] net: Do not record sender_cpu as napi_id in  socket receive paths Alexander Duyck <alexander.duyck@gmail.com> - 2017-03-17 04:00 +0100
              Re: [net-next PATCH 1/5] net: Do not record sender_cpu as napi_id  in socket receive paths Eric Dumazet <eric.dumazet@gmail.com> - 2017-03-17 04:10 +0100
        Re: [net-next PATCH 1/5] net: Do not record sender_cpu as napi_id  in socket receive paths "Samudrala, Sridhar" <sridhar.samudrala@intel.com> - 2017-03-16 23:50 +0100
    [net-next PATCH 2/5] net: Call sk_mark_napi_id() in the ACK receive  path Alexander Duyck <alexander.duyck@gmail.com> - 2017-03-16 19:40 +0100
      Re: [net-next PATCH 2/5] net: Call sk_mark_napi_id() in the ACK  receive path Eric Dumazet <eric.dumazet@gmail.com> - 2017-03-16 23:10 +0100
        Re: [net-next PATCH 2/5] net: Call sk_mark_napi_id() in the ACK  receive path Alexander Duyck <alexander.duyck@gmail.com> - 2017-03-16 23:40 +0100
    [net-next PATCH 3/5] net: Introduce SO_INCOMING_NAPI_ID Alexander Duyck <alexander.duyck@gmail.com> - 2017-03-16 19:40 +0100
      Re: [net-next PATCH 3/5] net: Introduce SO_INCOMING_NAPI_ID Eric Dumazet <eric.dumazet@gmail.com> - 2017-03-17 00:00 +0100
    Re: [net-next PATCH 0/5] Add busy poll support for epoll under  certain circumstances Michael Kerrisk <mtk.manpages@gmail.com> - 2017-03-18 12:50 +0100

#1602760 — [net-next PATCH 0/5] Add busy poll support for epoll under certain circumstances

This patch series is meant to add busy polling support to epoll when all of
the sockets on a given epoll are either local or are being sourced by the
same NAPI ID.

In order to support this the first two patches clean up a few issues we
found with the NAPI ID tracking and infrastructure.

In the third patch we introduce SO_INCOMING_NAPI_ID so that applications
have a means of trying to sort their incoming sockets to identify which
requests should be routed where in order to keep the epoll listener aligned
to a given Rx queue without having to rely on IRQ pinning.

Finally the last two patches refactor the existing busy poll infrastructure
to make it so that we can call it without necessarily needing a socket, and
enable the bits needed to support epoll when all of the sockets on the
epoll either share the same NAPI ID, or simply are reporting no NAPI ID.

---

Sridhar Samudrala (5):
      net: Do not record sender_cpu as napi_id in socket receive paths
      net: Call sk_mark_napi_id() in the ACK receive path
      net: Introduce SO_INCOMING_NAPI_ID
      net: Commonize busy polling code to focus on napi_id instead of socket
      epoll: Add busy poll support to epoll with socket fds.


 arch/alpha/include/uapi/asm/socket.h   |    2 +
 arch/avr32/include/uapi/asm/socket.h   |    2 +
 arch/frv/include/uapi/asm/socket.h     |    2 +
 arch/ia64/include/uapi/asm/socket.h    |    2 +
 arch/m32r/include/uapi/asm/socket.h    |    2 +
 arch/mips/include/uapi/asm/socket.h    |    2 +
 arch/mn10300/include/uapi/asm/socket.h |    2 +
 arch/parisc/include/uapi/asm/socket.h  |    2 +
 arch/powerpc/include/uapi/asm/socket.h |    2 +
 arch/s390/include/uapi/asm/socket.h    |    2 +
 arch/sparc/include/uapi/asm/socket.h   |    2 +
 arch/xtensa/include/uapi/asm/socket.h  |    2 +
 fs/eventpoll.c                         |  115 ++++++++++++++++++++++++++++++++
 include/net/busy_poll.h                |   14 +++-
 include/uapi/asm-generic/socket.h      |    2 +
 net/core/dev.c                         |   16 ++--
 net/core/sock.c                        |   22 ++++++
 net/ipv4/tcp_ipv4.c                    |    1 
 18 files changed, 183 insertions(+), 11 deletions(-)

--

[toc] | [next] | [standalone]

#1602765 — [net-next PATCH 1/5] net: Do not record sender_cpu as napi_id in socket receive paths

From: Sridhar Samudrala <sridhar.samudrala@intel.com>

Fix sk_mark_napi_id() and sk_mark_napi_id_once() to set sk_napi_id only if
skb->napi_id is a valid value.

This happens in loopback paths where skb->napi_id is not updated in
rx path and holds sender_cpu that is set in xmit path.

Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
 include/net/busy_poll.h |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/net/busy_poll.h b/include/net/busy_poll.h
index c0452de83086..67991635953e 100644
--- a/include/net/busy_poll.h
+++ b/include/net/busy_poll.h
@@ -116,7 +116,8 @@ static inline bool sk_busy_loop(struct sock *sk, int nonblock)
 static inline void sk_mark_napi_id(struct sock *sk, const struct sk_buff *skb)
 {
 #ifdef CONFIG_NET_RX_BUSY_POLL
-	sk->sk_napi_id = skb->napi_id;
+	if (skb->napi_id > (u32)NR_CPUS)
+		sk->sk_napi_id = skb->napi_id;
 #endif
 }
 
@@ -125,7 +126,7 @@ static inline void sk_mark_napi_id_once(struct sock *sk,
 					const struct sk_buff *skb)
 {
 #ifdef CONFIG_NET_RX_BUSY_POLL
-	if (!sk->sk_napi_id)
+	if (!sk->sk_napi_id && (skb->napi_id > (u32)NR_CPUS))
 		sk->sk_napi_id = skb->napi_id;
 #endif
 }

[toc] | [prev] | [next] | [standalone]

#1602869 — Re: [net-next PATCH 1/5] net: Do not record sender_cpu as napi_id in socket receive paths

On Thu, 2017-03-16 at 11:32 -0700, Alexander Duyck wrote:
> From: Sridhar Samudrala <sridhar.samudrala@intel.com>
> 
> Fix sk_mark_napi_id() and sk_mark_napi_id_once() to set sk_napi_id only if
> skb->napi_id is a valid value.
> 
> This happens in loopback paths where skb->napi_id is not updated in
> rx path and holds sender_cpu that is set in xmit path.
> 
> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> ---
>  include/net/busy_poll.h |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/include/net/busy_poll.h b/include/net/busy_poll.h
> index c0452de83086..67991635953e 100644
> --- a/include/net/busy_poll.h
> +++ b/include/net/busy_poll.h
> @@ -116,7 +116,8 @@ static inline bool sk_busy_loop(struct sock *sk, int nonblock)
>  static inline void sk_mark_napi_id(struct sock *sk, const struct sk_buff *skb)
>  {
>  #ifdef CONFIG_NET_RX_BUSY_POLL
> -	sk->sk_napi_id = skb->napi_id;
> +	if (skb->napi_id > (u32)NR_CPUS)
> +		sk->sk_napi_id = skb->napi_id;
>  #endif
>  }
>  
> @@ -125,7 +126,7 @@ static inline void sk_mark_napi_id_once(struct sock *sk,
>  					const struct sk_buff *skb)
>  {
>  #ifdef CONFIG_NET_RX_BUSY_POLL
> -	if (!sk->sk_napi_id)
> +	if (!sk->sk_napi_id && (skb->napi_id > (u32)NR_CPUS))
>  		sk->sk_napi_id = skb->napi_id;
>  #endif
>  }
> 

It is not clear why this patch is needed .

What you describe here is the case we might receive packets for a socket
coming from different interfaces ?

If skb->napi_id is a sender_cpu, why should we prevent overwriting the
sk_napi_id with it, knowing that busy polling will simply ignore the
invalid value ?

Do not get me wrong :

I simply try to understand why the test about napi_id validity is now
done twice :

1) At the time we are writing into sk->sk_napi_id

2) At busy polling time when we read sk->sk_napi_id

[toc] | [prev] | [next] | [standalone]

#1602900 — Re: [net-next PATCH 1/5] net: Do not record sender_cpu as napi_id in socket receive paths

On Thu, Mar 16, 2017 at 3:05 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Thu, 2017-03-16 at 11:32 -0700, Alexander Duyck wrote:
>> From: Sridhar Samudrala <sridhar.samudrala@intel.com>
>>
>> Fix sk_mark_napi_id() and sk_mark_napi_id_once() to set sk_napi_id only if
>> skb->napi_id is a valid value.
>>
>> This happens in loopback paths where skb->napi_id is not updated in
>> rx path and holds sender_cpu that is set in xmit path.
>>
>> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
>> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
>> ---
>>  include/net/busy_poll.h |    5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/net/busy_poll.h b/include/net/busy_poll.h
>> index c0452de83086..67991635953e 100644
>> --- a/include/net/busy_poll.h
>> +++ b/include/net/busy_poll.h
>> @@ -116,7 +116,8 @@ static inline bool sk_busy_loop(struct sock *sk, int nonblock)
>>  static inline void sk_mark_napi_id(struct sock *sk, const struct sk_buff *skb)
>>  {
>>  #ifdef CONFIG_NET_RX_BUSY_POLL
>> -     sk->sk_napi_id = skb->napi_id;
>> +     if (skb->napi_id > (u32)NR_CPUS)
>> +             sk->sk_napi_id = skb->napi_id;
>>  #endif
>>  }
>>
>> @@ -125,7 +126,7 @@ static inline void sk_mark_napi_id_once(struct sock *sk,
>>                                       const struct sk_buff *skb)
>>  {
>>  #ifdef CONFIG_NET_RX_BUSY_POLL
>> -     if (!sk->sk_napi_id)
>> +     if (!sk->sk_napi_id && (skb->napi_id > (u32)NR_CPUS))
>>               sk->sk_napi_id = skb->napi_id;
>>  #endif
>>  }
>>
>
> It is not clear why this patch is needed .
>
> What you describe here is the case we might receive packets for a socket
> coming from different interfaces ?
>
> If skb->napi_id is a sender_cpu, why should we prevent overwriting the
> sk_napi_id with it, knowing that busy polling will simply ignore the
> invalid value ?
>
> Do not get me wrong :
>
> I simply try to understand why the test about napi_id validity is now
> done twice :
>
> 1) At the time we are writing into sk->sk_napi_id

I would argue that this is the one piece we were missing.

> 2) At busy polling time when we read sk->sk_napi_id

Unless there was something recently added I don't think this was ever
checked.  Instead we start digging into the hash looking for the ID
that won't ever be there.  Maybe we should add something to napi_by_id
that just returns NULL in these cases.

On top of that I think there end up being several spots where once we
lock in a non-NAPI ID it is stuck such as the sk_mark_napi_id_once
call.  I figure we are better off locking in an actual NAPI ID rather
than getting a sender_cpu stuck in there.

[toc] | [prev] | [next] | [standalone]

#1602906 — Re: [net-next PATCH 1/5] net: Do not record sender_cpu as napi_id in socket receive paths

On Thu, 2017-03-16 at 15:33 -0700, Alexander Duyck wrote:
> On Thu, Mar 16, 2017 at 3:05 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:

> > It is not clear why this patch is needed .
> >
> > What you describe here is the case we might receive packets for a socket
> > coming from different interfaces ?
> >
> > If skb->napi_id is a sender_cpu, why should we prevent overwriting the
> > sk_napi_id with it, knowing that busy polling will simply ignore the
> > invalid value ?
> >
> > Do not get me wrong :
> >
> > I simply try to understand why the test about napi_id validity is now
> > done twice :
> >
> > 1) At the time we are writing into sk->sk_napi_id
> 
> I would argue that this is the one piece we were missing.
> 
> > 2) At busy polling time when we read sk->sk_napi_id
> 
> Unless there was something recently added I don't think this was ever
> checked.  Instead we start digging into the hash looking for the ID
> that won't ever be there.  Maybe we should add something to napi_by_id
> that just returns NULL in these cases.

But this is exactly what should happen.

For invalid ID, we return NULL from napi_by_id()

No need to add code for that, since the function is meant to deal with
valid cases.



> On top of that I think there end up being several spots where once we
> lock in a non-NAPI ID it is stuck such as the sk_mark_napi_id_once
> call.  I figure we are better off locking in an actual NAPI ID rather
> than getting a sender_cpu stuck in there.

Are you referring to sk_mark_napi_id_once() ?

Since this is only used by UDP, I would be OK to avoid the 'locking' for
'sender_cpu'  ids.

[toc] | [prev] | [next] | [standalone]

#1603002 — Re: [net-next PATCH 1/5] net: Do not record sender_cpu as napi_id in socket receive paths

On Thu, Mar 16, 2017 at 3:50 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Thu, 2017-03-16 at 15:33 -0700, Alexander Duyck wrote:
>> On Thu, Mar 16, 2017 at 3:05 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>> > It is not clear why this patch is needed .
>> >
>> > What you describe here is the case we might receive packets for a socket
>> > coming from different interfaces ?
>> >
>> > If skb->napi_id is a sender_cpu, why should we prevent overwriting the
>> > sk_napi_id with it, knowing that busy polling will simply ignore the
>> > invalid value ?
>> >
>> > Do not get me wrong :
>> >
>> > I simply try to understand why the test about napi_id validity is now
>> > done twice :
>> >
>> > 1) At the time we are writing into sk->sk_napi_id
>>
>> I would argue that this is the one piece we were missing.
>>
>> > 2) At busy polling time when we read sk->sk_napi_id
>>
>> Unless there was something recently added I don't think this was ever
>> checked.  Instead we start digging into the hash looking for the ID
>> that won't ever be there.  Maybe we should add something to napi_by_id
>> that just returns NULL in these cases.
>
> But this is exactly what should happen.
>
> For invalid ID, we return NULL from napi_by_id()
>
> No need to add code for that, since the function is meant to deal with
> valid cases.

I don't know.  My concern here is about the cost of going through all
that code just for something that we know shouldn't be valid.  If
nothing else I might update sk_can_busy_loop so that it doesn't try
busy looping on a sk_napi_id that is NR_CPU or less.

>> On top of that I think there end up being several spots where once we
>> lock in a non-NAPI ID it is stuck such as the sk_mark_napi_id_once
>> call.  I figure we are better off locking in an actual NAPI ID rather
>> than getting a sender_cpu stuck in there.
>
> Are you referring to sk_mark_napi_id_once() ?
>
> Since this is only used by UDP, I would be OK to avoid the 'locking' for
> 'sender_cpu'  ids.

What I probably can do is go through and replace all the spots where
we where checking for sk_napi_id being 0, and instead replace it with
a check against NR_CPUS.

[toc] | [prev] | [next] | [standalone]

#1603003 — Re: [net-next PATCH 1/5] net: Do not record sender_cpu as napi_id in socket receive paths

On Thu, 2017-03-16 at 19:40 -0700, Alexander Duyck wrote:

> I don't know.  My concern here is about the cost of going through all
> that code just for something that we know shouldn't be valid.  If
> nothing else I might update sk_can_busy_loop so that it doesn't try
> busy looping on a sk_napi_id that is NR_CPU or less.

But why would that be a win ?

if napi_by_id() returns NULL, we immediately give up, (goto out;)

So why should we add a code that will add something that will not be
useful for the vast majority of the cases where the ID is valid and we
need to do the hash look up ?

Is libc trying to avoid doing syscalls like close(-1) ?

[toc] | [prev] | [next] | [standalone]

#1603004 — Re: [net-next PATCH 1/5] net: Do not record sender_cpu as napi_id in socket receive paths

On Thu, Mar 16, 2017 at 7:57 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Thu, 2017-03-16 at 19:40 -0700, Alexander Duyck wrote:
>
>> What I probably can do is go through and replace all the spots where
>> we where checking for sk_napi_id being 0, and instead replace it with
>> a check against NR_CPUS.
>
> This seems a good idea.

Right.  This is the path I am planning to go with.  It will require
about the same amount of code change but replaces those checks.  I
just have to make sure I catch all the spots where we were checking it
for 0 but that shouldn't be too difficult of an issue.

[toc] | [prev] | [next] | [standalone]

#1603005 — Re: [net-next PATCH 1/5] net: Do not record sender_cpu as napi_id in socket receive paths

On Thu, 2017-03-16 at 19:40 -0700, Alexander Duyck wrote:

> What I probably can do is go through and replace all the spots where
> we where checking for sk_napi_id being 0, and instead replace it with
> a check against NR_CPUS.

This seems a good idea.

[toc] | [prev] | [next] | [standalone]

#1602902 — Re: [net-next PATCH 1/5] net: Do not record sender_cpu as napi_id in socket receive paths

On 3/16/2017 3:05 PM, Eric Dumazet wrote:
> On Thu, 2017-03-16 at 11:32 -0700, Alexander Duyck wrote:
>> From: Sridhar Samudrala <sridhar.samudrala@intel.com>
>>
>> Fix sk_mark_napi_id() and sk_mark_napi_id_once() to set sk_napi_id only if
>> skb->napi_id is a valid value.
>>
>> This happens in loopback paths where skb->napi_id is not updated in
>> rx path and holds sender_cpu that is set in xmit path.
>>
>> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
>> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
>> ---
>>   include/net/busy_poll.h |    5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/net/busy_poll.h b/include/net/busy_poll.h
>> index c0452de83086..67991635953e 100644
>> --- a/include/net/busy_poll.h
>> +++ b/include/net/busy_poll.h
>> @@ -116,7 +116,8 @@ static inline bool sk_busy_loop(struct sock *sk, int nonblock)
>>   static inline void sk_mark_napi_id(struct sock *sk, const struct sk_buff *skb)
>>   {
>>   #ifdef CONFIG_NET_RX_BUSY_POLL
>> -	sk->sk_napi_id = skb->napi_id;
>> +	if (skb->napi_id > (u32)NR_CPUS)
>> +		sk->sk_napi_id = skb->napi_id;
>>   #endif
>>   }
>>   
>> @@ -125,7 +126,7 @@ static inline void sk_mark_napi_id_once(struct sock *sk,
>>   					const struct sk_buff *skb)
>>   {
>>   #ifdef CONFIG_NET_RX_BUSY_POLL
>> -	if (!sk->sk_napi_id)
>> +	if (!sk->sk_napi_id && (skb->napi_id > (u32)NR_CPUS))
>>   		sk->sk_napi_id = skb->napi_id;
>>   #endif
>>   }
>>
> It is not clear why this patch is needed .
>
> What you describe here is the case we might receive packets for a socket
> coming from different interfaces ?

This is seen with AF_UNIX or AF_INET sockets over loopback.
>
> If skb->napi_id is a sender_cpu, why should we prevent overwriting the
> sk_napi_id with it, knowing that busy polling will simply ignore the
> invalid value ?

We are not checking for invalid VALUE(< NR_CPUs) in busy_poll,
Non-zero sk->napi_id is considered valid.

If we don't want to add this check while setting sk->sk_napi_Id, we 
could change the
check in ep_set_busy_poll_napi_id() to check for invalid value rather 
than non-zero value.

>
> Do not get me wrong :
>
> I simply try to understand why the test about napi_id validity is now
> done twice :
>
> 1) At the time we are writing into sk->sk_napi_id
>
> 2) At busy polling time when we read sk->sk_napi_id
>
>
>

[toc] | [prev] | [next] | [standalone]

#1602767 — [net-next PATCH 2/5] net: Call sk_mark_napi_id() in the ACK receive path

From: Sridhar Samudrala <sridhar.samudrala@intel.com>

Call sk_mark_napi_id() in the ACK receive path of a TCP_NEW_SYN_RECV
socket, so that sk->napi_id is set even if the socket hasn't yet received
any data.  With this change we should be able to start busy polling
slightly earlier.

Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
 net/ipv4/tcp_ipv4.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 08d870e45658..b86002a296f1 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1687,6 +1687,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
 			tcp_v4_send_reset(nsk, skb);
 			goto discard_and_relse;
 		} else {
+			sk_mark_napi_id(nsk, skb);
 			sock_put(sk);
 			return 0;
 		}

[toc] | [prev] | [next] | [standalone]

#1602868 — Re: [net-next PATCH 2/5] net: Call sk_mark_napi_id() in the ACK receive path

On Thu, 2017-03-16 at 11:32 -0700, Alexander Duyck wrote:
> From: Sridhar Samudrala <sridhar.samudrala@intel.com>
> 
> Call sk_mark_napi_id() in the ACK receive path of a TCP_NEW_SYN_RECV
> socket, so that sk->napi_id is set even if the socket hasn't yet received
> any data.  With this change we should be able to start busy polling
> slightly earlier.
> 
> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> ---
>  net/ipv4/tcp_ipv4.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index 08d870e45658..b86002a296f1 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -1687,6 +1687,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
>  			tcp_v4_send_reset(nsk, skb);
>  			goto discard_and_relse;
>  		} else {
> +			sk_mark_napi_id(nsk, skb);
>  			sock_put(sk);
>  			return 0;
>  		}
> 

Seems good, but what about IPv6 ?

Frankly this calls for the sk_mark_napi_id() being done in
tcp_child_process() instead of its four callers.

[toc] | [prev] | [next] | [standalone]

#1602893 — Re: [net-next PATCH 2/5] net: Call sk_mark_napi_id() in the ACK receive path

On Thu, Mar 16, 2017 at 3:04 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Thu, 2017-03-16 at 11:32 -0700, Alexander Duyck wrote:
>> From: Sridhar Samudrala <sridhar.samudrala@intel.com>
>>
>> Call sk_mark_napi_id() in the ACK receive path of a TCP_NEW_SYN_RECV
>> socket, so that sk->napi_id is set even if the socket hasn't yet received
>> any data.  With this change we should be able to start busy polling
>> slightly earlier.
>>
>> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
>> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
>> ---
>>  net/ipv4/tcp_ipv4.c |    1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
>> index 08d870e45658..b86002a296f1 100644
>> --- a/net/ipv4/tcp_ipv4.c
>> +++ b/net/ipv4/tcp_ipv4.c
>> @@ -1687,6 +1687,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
>>                       tcp_v4_send_reset(nsk, skb);
>>                       goto discard_and_relse;
>>               } else {
>> +                     sk_mark_napi_id(nsk, skb);
>>                       sock_put(sk);
>>                       return 0;
>>               }
>>
>
> Seems good, but what about IPv6 ?

Sorry, I spaced out and overlooked that this would also be an issue for IPv6.

> Frankly this calls for the sk_mark_napi_id() being done in
> tcp_child_process() instead of its four callers.

We can look into that for the next version.

[toc] | [prev] | [next] | [standalone]

#1602770 — [net-next PATCH 3/5] net: Introduce SO_INCOMING_NAPI_ID

From: Sridhar Samudrala <sridhar.samudrala@intel.com>

This socket option returns the napi id associated with the queue on which
the last frame is received. This information can be used by the apps to
split the incoming flows among the threads based on the Rx queue on which
they are received.

Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
 arch/alpha/include/uapi/asm/socket.h   |    2 ++
 arch/avr32/include/uapi/asm/socket.h   |    2 ++
 arch/frv/include/uapi/asm/socket.h     |    2 ++
 arch/ia64/include/uapi/asm/socket.h    |    2 ++
 arch/m32r/include/uapi/asm/socket.h    |    2 ++
 arch/mips/include/uapi/asm/socket.h    |    2 ++
 arch/mn10300/include/uapi/asm/socket.h |    2 ++
 arch/parisc/include/uapi/asm/socket.h  |    2 ++
 arch/powerpc/include/uapi/asm/socket.h |    2 ++
 arch/s390/include/uapi/asm/socket.h    |    2 ++
 arch/sparc/include/uapi/asm/socket.h   |    2 ++
 arch/xtensa/include/uapi/asm/socket.h  |    2 ++
 include/uapi/asm-generic/socket.h      |    2 ++
 net/core/sock.c                        |    4 ++++
 14 files changed, 30 insertions(+)

diff --git a/arch/alpha/include/uapi/asm/socket.h b/arch/alpha/include/uapi/asm/socket.h
index afc901b7a6f6..c90a4eb8fe2d 100644
--- a/arch/alpha/include/uapi/asm/socket.h
+++ b/arch/alpha/include/uapi/asm/socket.h
@@ -99,4 +99,6 @@
 
 #define SCM_TIMESTAMPING_OPT_STATS	54
 
+#define SO_INCOMING_NAPI_ID	55
+
 #endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/avr32/include/uapi/asm/socket.h b/arch/avr32/include/uapi/asm/socket.h
index 5a650426f357..bee51e585ff6 100644
--- a/arch/avr32/include/uapi/asm/socket.h
+++ b/arch/avr32/include/uapi/asm/socket.h
@@ -92,4 +92,6 @@
 
 #define SCM_TIMESTAMPING_OPT_STATS	54
 
+#define SO_INCOMING_NAPI_ID	55
+
 #endif /* _UAPI__ASM_AVR32_SOCKET_H */
diff --git a/arch/frv/include/uapi/asm/socket.h b/arch/frv/include/uapi/asm/socket.h
index 81e03530ed39..4ac94eff4a64 100644
--- a/arch/frv/include/uapi/asm/socket.h
+++ b/arch/frv/include/uapi/asm/socket.h
@@ -92,5 +92,7 @@
 
 #define SCM_TIMESTAMPING_OPT_STATS	54
 
+#define SO_INCOMING_NAPI_ID	55
+
 #endif /* _ASM_SOCKET_H */
 
diff --git a/arch/ia64/include/uapi/asm/socket.h b/arch/ia64/include/uapi/asm/socket.h
index 57feb0c1f7d7..1bb0c810495e 100644
--- a/arch/ia64/include/uapi/asm/socket.h
+++ b/arch/ia64/include/uapi/asm/socket.h
@@ -101,4 +101,6 @@
 
 #define SCM_TIMESTAMPING_OPT_STATS	54
 
+#define SO_INCOMING_NAPI_ID	55
+
 #endif /* _ASM_IA64_SOCKET_H */
diff --git a/arch/m32r/include/uapi/asm/socket.h b/arch/m32r/include/uapi/asm/socket.h
index 5853f8e92c20..234119518dc1 100644
--- a/arch/m32r/include/uapi/asm/socket.h
+++ b/arch/m32r/include/uapi/asm/socket.h
@@ -92,4 +92,6 @@
 
 #define SCM_TIMESTAMPING_OPT_STATS	54
 
+#define SO_INCOMING_NAPI_ID	55
+
 #endif /* _ASM_M32R_SOCKET_H */
diff --git a/arch/mips/include/uapi/asm/socket.h b/arch/mips/include/uapi/asm/socket.h
index 566ecdcb5b4b..0b6d2f0b82b4 100644
--- a/arch/mips/include/uapi/asm/socket.h
+++ b/arch/mips/include/uapi/asm/socket.h
@@ -110,4 +110,6 @@
 
 #define SCM_TIMESTAMPING_OPT_STATS	54
 
+#define SO_INCOMING_NAPI_ID	55
+
 #endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/mn10300/include/uapi/asm/socket.h b/arch/mn10300/include/uapi/asm/socket.h
index 0e12527c4b0e..881fa88cbf60 100644
--- a/arch/mn10300/include/uapi/asm/socket.h
+++ b/arch/mn10300/include/uapi/asm/socket.h
@@ -92,4 +92,6 @@
 
 #define SCM_TIMESTAMPING_OPT_STATS	54
 
+#define SO_INCOMING_NAPI_ID	55
+
 #endif /* _ASM_SOCKET_H */
diff --git a/arch/parisc/include/uapi/asm/socket.h b/arch/parisc/include/uapi/asm/socket.h
index 7a109b73ddf7..70d6f5ffebef 100644
--- a/arch/parisc/include/uapi/asm/socket.h
+++ b/arch/parisc/include/uapi/asm/socket.h
@@ -91,4 +91,6 @@
 
 #define SCM_TIMESTAMPING_OPT_STATS	0x402F
 
+#define SO_INCOMING_NAPI_ID	0x4030
+
 #endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/powerpc/include/uapi/asm/socket.h b/arch/powerpc/include/uapi/asm/socket.h
index 44583a52f882..8188f0ebae5f 100644
--- a/arch/powerpc/include/uapi/asm/socket.h
+++ b/arch/powerpc/include/uapi/asm/socket.h
@@ -99,4 +99,6 @@
 
 #define SCM_TIMESTAMPING_OPT_STATS	54
 
+#define SO_INCOMING_NAPI_ID	55
+
 #endif	/* _ASM_POWERPC_SOCKET_H */
diff --git a/arch/s390/include/uapi/asm/socket.h b/arch/s390/include/uapi/asm/socket.h
index b24a64cbfeb1..22ef97b26377 100644
--- a/arch/s390/include/uapi/asm/socket.h
+++ b/arch/s390/include/uapi/asm/socket.h
@@ -98,4 +98,6 @@
 
 #define SCM_TIMESTAMPING_OPT_STATS	54
 
+#define SO_INCOMING_NAPI_ID	55
+
 #endif /* _ASM_SOCKET_H */
diff --git a/arch/sparc/include/uapi/asm/socket.h b/arch/sparc/include/uapi/asm/socket.h
index a25dc32f5d6a..6b7e10019763 100644
--- a/arch/sparc/include/uapi/asm/socket.h
+++ b/arch/sparc/include/uapi/asm/socket.h
@@ -88,6 +88,8 @@
 
 #define SCM_TIMESTAMPING_OPT_STATS	0x0038
 
+#define SO_INCOMING_NAPI_ID	0x0039
+
 /* Security levels - as per NRL IPv6 - don't actually do anything */
 #define SO_SECURITY_AUTHENTICATION		0x5001
 #define SO_SECURITY_ENCRYPTION_TRANSPORT	0x5002
diff --git a/arch/xtensa/include/uapi/asm/socket.h b/arch/xtensa/include/uapi/asm/socket.h
index 9fdbe1fe0473..77ba8be3738c 100644
--- a/arch/xtensa/include/uapi/asm/socket.h
+++ b/arch/xtensa/include/uapi/asm/socket.h
@@ -103,4 +103,6 @@
 
 #define SCM_TIMESTAMPING_OPT_STATS	54
 
+#define SO_INCOMING_NAPI_ID	55
+
 #endif	/* _XTENSA_SOCKET_H */
diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
index 2c748ddad5f8..e1ffc8d4aca3 100644
--- a/include/uapi/asm-generic/socket.h
+++ b/include/uapi/asm-generic/socket.h
@@ -94,4 +94,6 @@
 
 #define SCM_TIMESTAMPING_OPT_STATS	54
 
+#define SO_INCOMING_NAPI_ID	55
+
 #endif /* __ASM_GENERIC_SOCKET_H */
diff --git a/net/core/sock.c b/net/core/sock.c
index a83731c36761..74288b2d4b3d 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1313,6 +1313,10 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
 		v.val = sk->sk_incoming_cpu;
 		break;
 
+	case SO_INCOMING_NAPI_ID:
+		v.val = sk->sk_napi_id;
+		break;
+
 	default:
 		/* We implement the SO_SNDLOWAT etc to not be settable
 		 * (1003.1g 7).

[toc] | [prev] | [next] | [standalone]

#1602904 — Re: [net-next PATCH 3/5] net: Introduce SO_INCOMING_NAPI_ID

On Thu, 2017-03-16 at 11:32 -0700, Alexander Duyck wrote:

>  
> +	case SO_INCOMING_NAPI_ID:
> +		v.val = sk->sk_napi_id;
> +		break;

I guess that here you should filter invalid values.

(So that you no longer need the first patch in this series)

Also, it looks like eBPF will need to get access to skb->napi_id for
efficient SO_REUSEPORT support ?

Thanks.

[toc] | [prev] | [next] | [standalone]

#1603771 — Re: [net-next PATCH 0/5] Add busy poll support for epoll under certain circumstances

[CC += linux-api@vger.kernel.org]

Hello Alexander

Since this is a kernel-user-space API change, please CC linux-api@
(and on future iterations of the patch). The kernel source file
Documentation/SubmitChecklist notes that all Linux kernel patches that
change userspace interfaces should be CCed to
linux-api@vger.kernel.org, so that the various parties who are
interested in API changes are informed. For further information, see
https://www.kernel.org/doc/man-pages/linux-api-ml.html

Thanks,

Michael



On Thu, Mar 16, 2017 at 7:32 PM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> This patch series is meant to add busy polling support to epoll when all of
> the sockets on a given epoll are either local or are being sourced by the
> same NAPI ID.
>
> In order to support this the first two patches clean up a few issues we
> found with the NAPI ID tracking and infrastructure.
>
> In the third patch we introduce SO_INCOMING_NAPI_ID so that applications
> have a means of trying to sort their incoming sockets to identify which
> requests should be routed where in order to keep the epoll listener aligned
> to a given Rx queue without having to rely on IRQ pinning.
>
> Finally the last two patches refactor the existing busy poll infrastructure
> to make it so that we can call it without necessarily needing a socket, and
> enable the bits needed to support epoll when all of the sockets on the
> epoll either share the same NAPI ID, or simply are reporting no NAPI ID.
>
> ---
>
> Sridhar Samudrala (5):
>       net: Do not record sender_cpu as napi_id in socket receive paths
>       net: Call sk_mark_napi_id() in the ACK receive path
>       net: Introduce SO_INCOMING_NAPI_ID
>       net: Commonize busy polling code to focus on napi_id instead of socket
>       epoll: Add busy poll support to epoll with socket fds.
>
>
>  arch/alpha/include/uapi/asm/socket.h   |    2 +
>  arch/avr32/include/uapi/asm/socket.h   |    2 +
>  arch/frv/include/uapi/asm/socket.h     |    2 +
>  arch/ia64/include/uapi/asm/socket.h    |    2 +
>  arch/m32r/include/uapi/asm/socket.h    |    2 +
>  arch/mips/include/uapi/asm/socket.h    |    2 +
>  arch/mn10300/include/uapi/asm/socket.h |    2 +
>  arch/parisc/include/uapi/asm/socket.h  |    2 +
>  arch/powerpc/include/uapi/asm/socket.h |    2 +
>  arch/s390/include/uapi/asm/socket.h    |    2 +
>  arch/sparc/include/uapi/asm/socket.h   |    2 +
>  arch/xtensa/include/uapi/asm/socket.h  |    2 +
>  fs/eventpoll.c                         |  115 ++++++++++++++++++++++++++++++++
>  include/net/busy_poll.h                |   14 +++-
>  include/uapi/asm-generic/socket.h      |    2 +
>  net/core/dev.c                         |   16 ++--
>  net/core/sock.c                        |   22 ++++++
>  net/ipv4/tcp_ipv4.c                    |    1
>  18 files changed, 183 insertions(+), 11 deletions(-)
>
> --



-- 
Michael Kerrisk Linux man-pages maintainer;
http://www.kernel.org/doc/man-pages/
Author of "The Linux Programming Interface", http://blog.man7.org/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web