From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from aserp2120.oracle.com ([141.146.126.78]:47564 "EHLO aserp2120.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751137AbeCHXeM (ORCPT ); Thu, 8 Mar 2018 18:34:12 -0500 Subject: Re: [PATCH net] macvlan: filter out xfrm feature flags To: David Miller Cc: netdev@vger.kernel.org, steffen.klassert@secunet.com References: <1520377028-14818-1-git-send-email-shannon.nelson@oracle.com> <20180308.123310.406604126920910381.davem@davemloft.net> From: Shannon Nelson Message-ID: <1a5020a8-015e-474c-0ec0-8f594e38a7b5@oracle.com> Date: Thu, 8 Mar 2018 15:34:02 -0800 MIME-Version: 1.0 In-Reply-To: <20180308.123310.406604126920910381.davem@davemloft.net> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: netdev-owner@vger.kernel.org List-ID: On 3/8/2018 9:33 AM, David Miller wrote: > From: Shannon Nelson > Date: Tue, 6 Mar 2018 14:57:08 -0800 > >> This isn't broken for vlans because they use a separate features >> connection (vlan_features) for inheriting features. This is >> fine, but I don't think trying to add something like this to >> every driver for every new upperdev is a good idea - I think >> the upperdev should try to protect itself. > > I think this fix is correct. > > But for how many upperdevs are we going to duplicate code like this, > and how many subtle differences and in fact bugs will result from all > of that duplication? > > I think we really need something common for these upperdev drivers > to use. Maybe just a macro defining feature bits to trim in this > situation. > > Thanks. Thanks, Dave. Yes, this could use something a little more generic, something that would catch any future "dangerous" bits. I'm not sure we can come up with a generic mask to be used by everyone, as each upper and lower dev has their own feature support levels. But we might come up with a better example for others to follow. Rather than calling out specific non-supported bits, we can probably just build a mask from bits that we already know are supported, something like this: features &= (ALWAYS_ON_FEATURES | MACVLAN_FEATURES); which would take care of NETIF_F_NETNS_LOCAL, the ESP flags, and anything else that wasn't already specifically called for. I'll repost with this and see what folks think. sln