Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754540AbdCPWlk (ORCPT ); Thu, 16 Mar 2017 18:41:40 -0400 Received: from mga02.intel.com ([134.134.136.20]:52642 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752051AbdCPWlg (ORCPT ); Thu, 16 Mar 2017 18:41:36 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,174,1486454400"; d="scan'208";a="945237732" Message-ID: <58CB147F.7080401@intel.com> Date: Thu, 16 Mar 2017 15:41:03 -0700 From: "Samudrala, Sridhar" User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Eric Dumazet , Alexander Duyck CC: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, edumazet@google.com, davem@davemloft.net Subject: Re: [net-next PATCH 1/5] net: Do not record sender_cpu as napi_id in socket receive paths References: <20170316183142.15806.38824.stgit@localhost.localdomain> <20170316183238.15806.14293.stgit@localhost.localdomain> <1489701936.28631.249.camel@edumazet-glaptop3.roam.corp.google.com> In-Reply-To: <1489701936.28631.249.camel@edumazet-glaptop3.roam.corp.google.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2375 Lines: 72 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 >> >> 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 >> Signed-off-by: Alexander Duyck >> --- >> 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 > > >