Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755102AbdFWURk (ORCPT ); Fri, 23 Jun 2017 16:17:40 -0400 Received: from prod-mx.aristanetworks.com ([162.210.130.12]:12331 "EHLO prod-mx.aristanetworks.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755035AbdFWURj (ORCPT ); Fri, 23 Jun 2017 16:17:39 -0400 From: Julien Gomes Subject: Re: [PATCH net-next 1/2] ipmr: restrict mroute "queue full" warning to related error values To: David Miller Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org References: <20170621175811.16940-1-julien@arista.com> <20170623.133947.433422648753848462.davem@davemloft.net> <20170623.144739.1791220360024944932.davem@davemloft.net> Message-ID: Date: Fri, 23 Jun 2017 13:17:31 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 MIME-Version: 1.0 In-Reply-To: <20170623.144739.1791220360024944932.davem@davemloft.net> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2267 Lines: 58 On 06/23/2017 11:47 AM, David Miller wrote: > From: Julien Gomes > Date: Fri, 23 Jun 2017 10:52:26 -0700 > >> On 06/23/2017 10:39 AM, David Miller wrote: >> >>> From: Julien Gomes >>> Date: Wed, 21 Jun 2017 10:58:10 -0700 >>> >>>> When sending a cache report on mroute_sk, mroute will emit a >>>> "pending queue full" warning for every error value returned by >>>> sock_queue_rcv_skb(). >>>> This warning can be misleading, for example on the EPERM error value >>>> that sk_filter() can return. >>>> >>>> Restricting this warning to only ENOMEM or ENOBUFS seems more >>>> appropriate. >>>> >>>> Signed-off-by: Julien Gomes >>> Incorrect, no other error codes are possible. >>> >>> We never attach a socket filter to these kernel internal sockets, >>> therefore sk_filter() is not even applicable in this analysis. >>> >>> Therefore, -ENOBUFS and -ENOMEM are the only errors we can ever see >>> returned from sock_queue_rcv_skb(). >>> >>> This goes for your second patch as well. >> Up to now I would agree, but now that cache reports are also sent >> through Netlink, wouldn't it make sense to allow the user of mroute_sk >> to attach a filter to it in order to not receive cache reports on it? > There is not visibility of this socket outside of the kernel. Hmm, maybe we are not talking about the same thing. The value of this socket pointer is set by the MRT_INIT sockopt. The socket is definitely visible outside of the kernel as it is first created and used by the user for kernel-user communications via the sockopts and the messages sent by the kernel to the user through it. The typical user setup up to now was to create this socket, MRT_INIT to register it in ipmr, and handle the incoming packets, including the cache reports. Now that the cache reports are also sent through another medium, the user should be able to decide whether it also wants the reports on this socket, which, once again, it is already using. And if the user wants to get the reports only through Netlink, the kernel will currently emit those unrelated warnings. Once again, we are not in the case of a purely internal kernel socket, as this socket is used for internal kernel-user communications. -- Julien Gomes