From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id F39ABC64ED6 for ; Fri, 17 Feb 2023 17:44:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229650AbjBQRoj (ORCPT ); Fri, 17 Feb 2023 12:44:39 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46018 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229510AbjBQRoh (ORCPT ); Fri, 17 Feb 2023 12:44:37 -0500 Received: from mail-wm1-x330.google.com (mail-wm1-x330.google.com [IPv6:2a00:1450:4864:20::330]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 311DD70940; Fri, 17 Feb 2023 09:44:36 -0800 (PST) Received: by mail-wm1-x330.google.com with SMTP id 4-20020a05600c22c400b003dc4fd6e61dso1500087wmg.5; Fri, 17 Feb 2023 09:44:36 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=auVrVbOSTA9pQeJfPXz91TlriegzpUKwC2EMUID2gPE=; b=kPts36a0DOfrWbE5EDMVM8XUTfPhcct1+Wh6iYrEUNULUaJQQ940e2QgPodOvnNQ3O PYVdDfrIzrIPE0Vc2g/vfe89g2tIAWRInxGmIeY2ABwKLrTGnwzSZXS4/ClMgmErrlyZ 2VJyFiXyf6FXfFu8h4TAk5sbo9ObWj2hpaMsr5iTHYK0yLpQeXx/71BXJmUTcn6Wvk9c PmQQIK6oRsnk/d8C/SuDJEsTzFKhOVo3Uq5HnXasc+0Bv5iNajhahka3DZoCuSzV0nXu U1ZgVBV32o0fJltdnu8cEis7bX2g96yWawZ5DbL8oya33c0Sw88227zU/BpoJxCJOwQD DEAg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=auVrVbOSTA9pQeJfPXz91TlriegzpUKwC2EMUID2gPE=; b=zhR1inJqGY8yG2qKKs9jUFCHLGNNcYodvO+Gttq9qnflp4+yhjX3jBLcGTZ/uJEsKB JbpQ2SJysQId/lrk101SKxenFsery1IpFEwNRHSpeZUYxfAUpWb/KnLBDlODQAL115aa kKADHUqy3arQJnObYxtuDOV0ks7u9Lfue53bMEr7KZDIRNR0H3dKn9yt9Ki+I5hVbLS/ WPIiCElH8IwRDOOMRfg0PMRRjHIWeXeoWTcnn27VpUCumQipcBiOfyZoMuxkYMNzzI+O li5PGj1qQO+89N4zmm3OqwroPlqiJP/THgtJJmZF8JmyYb7Ml8B309ZXY7R+8Tff9RBx oNHw== X-Gm-Message-State: AO0yUKXVqSjHKUby8f4xewJC/5inYIYvNtRVLFLErsegTv6jqLq8BRFN jxBIngbcl8P9ZVoFrdX4cB4= X-Google-Smtp-Source: AK7set/fy/Eb7Sue92rcdrDm3lCCSH8M5LFbef1BLFLsabLm+s8mqQPhJsjPKF5bbxALYzTFXwfvuA== X-Received: by 2002:a05:600c:30ca:b0:3d5:365b:773e with SMTP id h10-20020a05600c30ca00b003d5365b773emr1835463wmn.39.1676655874493; Fri, 17 Feb 2023 09:44:34 -0800 (PST) Received: from skbuf ([188.25.231.176]) by smtp.gmail.com with ESMTPSA id x14-20020a1c7c0e000000b003e20970175dsm6651711wmc.32.2023.02.17.09.44.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 17 Feb 2023 09:44:34 -0800 (PST) Date: Fri, 17 Feb 2023 19:44:31 +0200 From: Vladimir Oltean To: Hans Schultz Cc: Simon Horman , davem@davemloft.net, kuba@kernel.org, netdev@vger.kernel.org, Florian Fainelli , Andrew Lunn , Eric Dumazet , Paolo Abeni , Kurt Kanzenbach , Hauke Mehrtens , Woojung Huh , "maintainer:MICROCHIP KSZ SERIES ETHERNET SWITCH DRIVER" , Sean Wang , Landen Chao , DENG Qingfang , Matthias Brugger , Claudiu Manoil , Alexandre Belloni , =?utf-8?B?Q2zDqW1lbnQgTMOpZ2Vy?= , Jiri Pirko , Ivan Vecera , Roopa Prabhu , Nikolay Aleksandrov , Russell King , Christian Marangi , open list , "moderated list:ARM/Mediatek SoC support" , "moderated list:ARM/Mediatek SoC support" , "open list:RENESAS RZ/N1 A5PSW SWITCH DRIVER" , "moderated list:ETHERNET BRIDGE" Subject: Re: [PATCH net-next 5/5] net: dsa: mv88e6xxx: implementation of dynamic ATU entries Message-ID: <20230217174431.bkkvfmtno56mfh5a@skbuf> References: <20230130173429.3577450-1-netdev@kapio-technology.com> <20230130173429.3577450-6-netdev@kapio-technology.com> <9b12275969a204739ccfab972d90f20f@kapio-technology.com> <20230203204422.4wrhyathxfhj6hdt@skbuf> <4abbe32d007240b9c3aea9c8ca936fa3@kapio-technology.com> <87fsb83q5s.fsf@kapio-technology.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87fsb83q5s.fsf@kapio-technology.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Feb 14, 2023 at 10:14:55PM +0100, Hans Schultz wrote: > On Mon, Feb 06, 2023 at 17:02, Simon Horman wrote: > > > > Just to clarify my suggestion one last time, it would be along the lines > > of the following (completely untested!). I feel that it robustly covers > > all cases for fdb_flags. And as a bonus doesn't need to be modified > > if other (unsupported) flags are added in future. > > > > if (fdb_flags & ~(DSA_FDB_FLAG_DYNAMIC)) > > return -EOPNOTSUPP; > > > > is_dynamic = !!(fdb_flags & DSA_FDB_FLAG_DYNAMIC) > > if (is_dynamic) > > state = MV88E6XXX_G1_ATU_DATA_STATE_UC_AGE_7_NEWEST; > > > > > > And perhaps for other drivers: > > > > if (fdb_flags & ~(DSA_FDB_FLAG_DYNAMIC)) > > return -EOPNOTSUPP; > > if (fdb_flags) > > return 0; > > > > Perhaps a helper would be warranted for the above. > > How would such a helper look? Inline function is not clean. > > > > > But in writing this I think that, perhaps drivers could return -EOPNOTSUPP > > for the DSA_FDB_FLAG_DYNAMIC case and the caller can handle, rather tha > > propagate, -EOPNOTSUPP. > > I looked at that, but changing the caller is also a bit ugly. Answering on behalf of Simon, and hoping he will agree. You are missing a big opportunity to make the kernel avoid doing useless work. The dsa_slave_fdb_event() function runs in atomic switchdev notifier context, and schedules a deferred workqueue item - dsa_schedule_work() - to get sleepable context to program hardware. Only that scheduling a deferred work item is not exactly cheap, so we try to avoid doing that unless we know that we'll end up doing something with that FDB entry once the deferred work does get scheduled: /* Check early that we're not doing work in vain. * Host addresses on LAG ports still require regular FDB ops, * since the CPU port isn't in a LAG. */ if (dp->lag && !host_addr) { if (!ds->ops->lag_fdb_add || !ds->ops->lag_fdb_del) return -EOPNOTSUPP; } else { if (!ds->ops->port_fdb_add || !ds->ops->port_fdb_del) return -EOPNOTSUPP; } What you should be doing is you should be using the pahole tool to find a good place for a new unsigned long field in struct dsa_switch, and add a new field ds->supported_fdb_flags. You should extend the early checking from dsa_slave_fdb_event() and exit without doing anything if the (fdb->flags & ~ds->supported_fdb_flags) expression is non-zero. This way you would kill 2 birds with 1 stone, since individual drivers would no longer need to check the flags; DSA would guarantee not calling them with unsupported flags. It would be also very good to reach an agreement with switchdev maintainers regarding the naming of the is_static/is_dyn field. It would also be excellent if you could rename "fdb_flags" to just "flags" within DSA. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 043DAC636D6 for ; Fri, 17 Feb 2023 17:45:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=wgRjvQY5zDVgzJrjrtq3wCQnktA5vp0RBc8g4n+VYIs=; b=Cxgx4WxIOq/w8K pCxlFXHBr5jQPOWSxRuPPJAOJQQL0nHiBxXvWTHu4trCDTuGh9b/4Qw80VwAPXq38ExeJe3YTN7xv QipzfuNbnznnQYjPxGZo03d3oKhX50nxfBHxvumOu6iDJG9Yk/Bw0b90xOlwaKG7HPpTC/ua9yFpT ylGJuFzQcNWdFyH2/ySU6nm9WLeWCbomog3AbDM31LUepNEZ15sVW9FmQ2hxGmPM3cIWWe3WDxshL XfUXE9hTTnLHyXAXFeUV8xM0VDAGSmRagzydDGnCtsoumHgDXZv+7wLiAcN6eemMf0b/Sk+YFEDOt +fbNPbIzfYh0c5f0iM4g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pT4mn-00FJTm-9u; Fri, 17 Feb 2023 17:44:41 +0000 Received: from mail-wm1-x333.google.com ([2a00:1450:4864:20::333]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1pT4mj-00FJSn-Gh; Fri, 17 Feb 2023 17:44:38 +0000 Received: by mail-wm1-x333.google.com with SMTP id p22so1531460wms.0; Fri, 17 Feb 2023 09:44:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=auVrVbOSTA9pQeJfPXz91TlriegzpUKwC2EMUID2gPE=; b=kPts36a0DOfrWbE5EDMVM8XUTfPhcct1+Wh6iYrEUNULUaJQQ940e2QgPodOvnNQ3O PYVdDfrIzrIPE0Vc2g/vfe89g2tIAWRInxGmIeY2ABwKLrTGnwzSZXS4/ClMgmErrlyZ 2VJyFiXyf6FXfFu8h4TAk5sbo9ObWj2hpaMsr5iTHYK0yLpQeXx/71BXJmUTcn6Wvk9c PmQQIK6oRsnk/d8C/SuDJEsTzFKhOVo3Uq5HnXasc+0Bv5iNajhahka3DZoCuSzV0nXu U1ZgVBV32o0fJltdnu8cEis7bX2g96yWawZ5DbL8oya33c0Sw88227zU/BpoJxCJOwQD DEAg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=auVrVbOSTA9pQeJfPXz91TlriegzpUKwC2EMUID2gPE=; b=4p8vbw5X5eTFdjymOYyIbaIUJDowTupKVh/L9M5Dlbv9Vr7qbAHHHELA/+hOoWM4HQ HKMEUVTJr3cqYD0ePD7mfE0kzJ8Jr9DvfOFv2eZL7QRcRfeJwumysTrbGAYPdlzxxbh3 8fJdoI2te1kIiWZIqTCxvDsFoK4W6V+Mcm8i0QoTXaLGdHfOOsBa0gvl1fGBIaz2Gf+t RwBE/c5BCyKwkuXRjzdbfyF9J/tqIwsPe9Iuv7wHiXHe8RseFJKR+sf+t8xhDQxPe8Qm YiyqnYpxR3MQPFHqm8wNjW4Ygv3IGazmRfqzf/Jdj1UpNaDwAUXm2zgAM++sz30gLVex J+uQ== X-Gm-Message-State: AO0yUKWSM05A9/SZJfYJ0HaMK16QkErK8OuNo1VRavs1Z0E61ASJkbue LHfE8saBjeGij+jmDOgPNm8= X-Google-Smtp-Source: AK7set/fy/Eb7Sue92rcdrDm3lCCSH8M5LFbef1BLFLsabLm+s8mqQPhJsjPKF5bbxALYzTFXwfvuA== X-Received: by 2002:a05:600c:30ca:b0:3d5:365b:773e with SMTP id h10-20020a05600c30ca00b003d5365b773emr1835463wmn.39.1676655874493; Fri, 17 Feb 2023 09:44:34 -0800 (PST) Received: from skbuf ([188.25.231.176]) by smtp.gmail.com with ESMTPSA id x14-20020a1c7c0e000000b003e20970175dsm6651711wmc.32.2023.02.17.09.44.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 17 Feb 2023 09:44:34 -0800 (PST) Date: Fri, 17 Feb 2023 19:44:31 +0200 From: Vladimir Oltean To: Hans Schultz Cc: Simon Horman , davem@davemloft.net, kuba@kernel.org, netdev@vger.kernel.org, Florian Fainelli , Andrew Lunn , Eric Dumazet , Paolo Abeni , Kurt Kanzenbach , Hauke Mehrtens , Woojung Huh , "maintainer:MICROCHIP KSZ SERIES ETHERNET SWITCH DRIVER" , Sean Wang , Landen Chao , DENG Qingfang , Matthias Brugger , Claudiu Manoil , Alexandre Belloni , =?utf-8?B?Q2zDqW1lbnQgTMOpZ2Vy?= , Jiri Pirko , Ivan Vecera , Roopa Prabhu , Nikolay Aleksandrov , Russell King , Christian Marangi , open list , "moderated list:ARM/Mediatek SoC support" , "moderated list:ARM/Mediatek SoC support" , "open list:RENESAS RZ/N1 A5PSW SWITCH DRIVER" , "moderated list:ETHERNET BRIDGE" Subject: Re: [PATCH net-next 5/5] net: dsa: mv88e6xxx: implementation of dynamic ATU entries Message-ID: <20230217174431.bkkvfmtno56mfh5a@skbuf> References: <20230130173429.3577450-1-netdev@kapio-technology.com> <20230130173429.3577450-6-netdev@kapio-technology.com> <9b12275969a204739ccfab972d90f20f@kapio-technology.com> <20230203204422.4wrhyathxfhj6hdt@skbuf> <4abbe32d007240b9c3aea9c8ca936fa3@kapio-technology.com> <87fsb83q5s.fsf@kapio-technology.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <87fsb83q5s.fsf@kapio-technology.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230217_094437_615914_317A7AEE X-CRM114-Status: GOOD ( 26.92 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Feb 14, 2023 at 10:14:55PM +0100, Hans Schultz wrote: > On Mon, Feb 06, 2023 at 17:02, Simon Horman wrote: > > > > Just to clarify my suggestion one last time, it would be along the lines > > of the following (completely untested!). I feel that it robustly covers > > all cases for fdb_flags. And as a bonus doesn't need to be modified > > if other (unsupported) flags are added in future. > > > > if (fdb_flags & ~(DSA_FDB_FLAG_DYNAMIC)) > > return -EOPNOTSUPP; > > > > is_dynamic = !!(fdb_flags & DSA_FDB_FLAG_DYNAMIC) > > if (is_dynamic) > > state = MV88E6XXX_G1_ATU_DATA_STATE_UC_AGE_7_NEWEST; > > > > > > And perhaps for other drivers: > > > > if (fdb_flags & ~(DSA_FDB_FLAG_DYNAMIC)) > > return -EOPNOTSUPP; > > if (fdb_flags) > > return 0; > > > > Perhaps a helper would be warranted for the above. > > How would such a helper look? Inline function is not clean. > > > > > But in writing this I think that, perhaps drivers could return -EOPNOTSUPP > > for the DSA_FDB_FLAG_DYNAMIC case and the caller can handle, rather tha > > propagate, -EOPNOTSUPP. > > I looked at that, but changing the caller is also a bit ugly. Answering on behalf of Simon, and hoping he will agree. You are missing a big opportunity to make the kernel avoid doing useless work. The dsa_slave_fdb_event() function runs in atomic switchdev notifier context, and schedules a deferred workqueue item - dsa_schedule_work() - to get sleepable context to program hardware. Only that scheduling a deferred work item is not exactly cheap, so we try to avoid doing that unless we know that we'll end up doing something with that FDB entry once the deferred work does get scheduled: /* Check early that we're not doing work in vain. * Host addresses on LAG ports still require regular FDB ops, * since the CPU port isn't in a LAG. */ if (dp->lag && !host_addr) { if (!ds->ops->lag_fdb_add || !ds->ops->lag_fdb_del) return -EOPNOTSUPP; } else { if (!ds->ops->port_fdb_add || !ds->ops->port_fdb_del) return -EOPNOTSUPP; } What you should be doing is you should be using the pahole tool to find a good place for a new unsigned long field in struct dsa_switch, and add a new field ds->supported_fdb_flags. You should extend the early checking from dsa_slave_fdb_event() and exit without doing anything if the (fdb->flags & ~ds->supported_fdb_flags) expression is non-zero. This way you would kill 2 birds with 1 stone, since individual drivers would no longer need to check the flags; DSA would guarantee not calling them with unsupported flags. It would be also very good to reach an agreement with switchdev maintainers regarding the naming of the is_static/is_dyn field. It would also be excellent if you could rename "fdb_flags" to just "flags" within DSA. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 9595F61AD5 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org AF96360E71 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=auVrVbOSTA9pQeJfPXz91TlriegzpUKwC2EMUID2gPE=; b=kPts36a0DOfrWbE5EDMVM8XUTfPhcct1+Wh6iYrEUNULUaJQQ940e2QgPodOvnNQ3O PYVdDfrIzrIPE0Vc2g/vfe89g2tIAWRInxGmIeY2ABwKLrTGnwzSZXS4/ClMgmErrlyZ 2VJyFiXyf6FXfFu8h4TAk5sbo9ObWj2hpaMsr5iTHYK0yLpQeXx/71BXJmUTcn6Wvk9c PmQQIK6oRsnk/d8C/SuDJEsTzFKhOVo3Uq5HnXasc+0Bv5iNajhahka3DZoCuSzV0nXu U1ZgVBV32o0fJltdnu8cEis7bX2g96yWawZ5DbL8oya33c0Sw88227zU/BpoJxCJOwQD DEAg== Date: Fri, 17 Feb 2023 19:44:31 +0200 From: Vladimir Oltean Message-ID: <20230217174431.bkkvfmtno56mfh5a@skbuf> References: <20230130173429.3577450-1-netdev@kapio-technology.com> <20230130173429.3577450-6-netdev@kapio-technology.com> <9b12275969a204739ccfab972d90f20f@kapio-technology.com> <20230203204422.4wrhyathxfhj6hdt@skbuf> <4abbe32d007240b9c3aea9c8ca936fa3@kapio-technology.com> <87fsb83q5s.fsf@kapio-technology.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87fsb83q5s.fsf@kapio-technology.com> Subject: Re: [Bridge] [PATCH net-next 5/5] net: dsa: mv88e6xxx: implementation of dynamic ATU entries List-Id: Linux Ethernet Bridging List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Hans Schultz Cc: Andrew Lunn , Alexandre Belloni , Simon Horman , Nikolay Aleksandrov , Kurt Kanzenbach , Eric Dumazet , Ivan Vecera , Florian Fainelli , "moderated list:ETHERNET BRIDGE" , Russell King , Roopa Prabhu , kuba@kernel.org, Paolo Abeni , =?utf-8?B?Q2zDqW1lbnQgTMOpZ2Vy?= , Christian Marangi , Woojung Huh , Landen Chao , Jiri Pirko , Hauke Mehrtens , Sean Wang , DENG Qingfang , Claudiu Manoil , "moderated list:ARM/Mediatek SoC support" , Matthias Brugger , "moderated list:ARM/Mediatek SoC support" , netdev@vger.kernel.org, open list , "maintainer:MICROCHIP KSZ SERIES ETHERNET SWITCH DRIVER" , "open list:RENESAS RZ/N1 A5PSW SWITCH DRIVER" , davem@davemloft.net On Tue, Feb 14, 2023 at 10:14:55PM +0100, Hans Schultz wrote: > On Mon, Feb 06, 2023 at 17:02, Simon Horman wrote: > > > > Just to clarify my suggestion one last time, it would be along the lines > > of the following (completely untested!). I feel that it robustly covers > > all cases for fdb_flags. And as a bonus doesn't need to be modified > > if other (unsupported) flags are added in future. > > > > if (fdb_flags & ~(DSA_FDB_FLAG_DYNAMIC)) > > return -EOPNOTSUPP; > > > > is_dynamic = !!(fdb_flags & DSA_FDB_FLAG_DYNAMIC) > > if (is_dynamic) > > state = MV88E6XXX_G1_ATU_DATA_STATE_UC_AGE_7_NEWEST; > > > > > > And perhaps for other drivers: > > > > if (fdb_flags & ~(DSA_FDB_FLAG_DYNAMIC)) > > return -EOPNOTSUPP; > > if (fdb_flags) > > return 0; > > > > Perhaps a helper would be warranted for the above. > > How would such a helper look? Inline function is not clean. > > > > > But in writing this I think that, perhaps drivers could return -EOPNOTSUPP > > for the DSA_FDB_FLAG_DYNAMIC case and the caller can handle, rather tha > > propagate, -EOPNOTSUPP. > > I looked at that, but changing the caller is also a bit ugly. Answering on behalf of Simon, and hoping he will agree. You are missing a big opportunity to make the kernel avoid doing useless work. The dsa_slave_fdb_event() function runs in atomic switchdev notifier context, and schedules a deferred workqueue item - dsa_schedule_work() - to get sleepable context to program hardware. Only that scheduling a deferred work item is not exactly cheap, so we try to avoid doing that unless we know that we'll end up doing something with that FDB entry once the deferred work does get scheduled: /* Check early that we're not doing work in vain. * Host addresses on LAG ports still require regular FDB ops, * since the CPU port isn't in a LAG. */ if (dp->lag && !host_addr) { if (!ds->ops->lag_fdb_add || !ds->ops->lag_fdb_del) return -EOPNOTSUPP; } else { if (!ds->ops->port_fdb_add || !ds->ops->port_fdb_del) return -EOPNOTSUPP; } What you should be doing is you should be using the pahole tool to find a good place for a new unsigned long field in struct dsa_switch, and add a new field ds->supported_fdb_flags. You should extend the early checking from dsa_slave_fdb_event() and exit without doing anything if the (fdb->flags & ~ds->supported_fdb_flags) expression is non-zero. This way you would kill 2 birds with 1 stone, since individual drivers would no longer need to check the flags; DSA would guarantee not calling them with unsupported flags. It would be also very good to reach an agreement with switchdev maintainers regarding the naming of the is_static/is_dyn field. It would also be excellent if you could rename "fdb_flags" to just "flags" within DSA.