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 EFBE0C433F5 for ; Wed, 6 Apr 2022 19:52:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233193AbiDFTyj (ORCPT ); Wed, 6 Apr 2022 15:54:39 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45226 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236335AbiDFTxv (ORCPT ); Wed, 6 Apr 2022 15:53:51 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 41DAB2652 for ; Wed, 6 Apr 2022 10:31:13 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id D03E061934 for ; Wed, 6 Apr 2022 17:31:12 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id D66E3C385A3; Wed, 6 Apr 2022 17:31:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1649266272; bh=TW8+JplZUdW0SOaVDpxePPZ7klA8MZbflXlS1vvA5IA=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=KdFC/UIN9DVI4V8MfKehOCgWaxdhVxI0AZKwmzxCx9mJRDKaDE0PBNU4NUw+EuZZG wLyNy8TaU0iEBtxRTMYcHmQH5gFiDYroJnD6LihUv0q0l7lbJpH6blqMpRYJspumcK iFaCCbKhXQYFLknFSMWUpV228hhqGfoPYTS+3eQH50lvOc0YWkD3zI6nD9NSHn3q4L NPEvM2lYMK+ErBcFjveH+ZT7oPolFBlV8hL4EWjhiprwI6DFmnNuoEL3iZokE8ca7v Bj658OSRAM1iG9/zRHZHsseTu2a/OxbdbRrN3cbx/3t+73dhvbZX1YI8SFar+8/aPM VcWCUHqY6/YRw== Date: Wed, 6 Apr 2022 10:31:10 -0700 From: Jakub Kicinski To: Mattias Forsblad Cc: netdev@vger.kernel.org, Andrew Lunn , Vivien Didelot , Florian Fainelli , Vladimir Oltean , "David S . Miller" , Paolo Abeni , Tobias Waldekranz Subject: Re: [PATCH v3 net-next 0/2] net: tc: dsa: Implement offload of matchall for bridged DSA ports Message-ID: <20220406103110.05481cb5@kernel.org> In-Reply-To: <84412805-f095-3e39-9747-e800c862095d@gmail.com> References: <20220404104826.1902292-1-mattias.forsblad@gmail.com> <20220405180949.3dd204a1@kernel.org> <84412805-f095-3e39-9747-e800c862095d@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Wed, 6 Apr 2022 11:24:46 +0200 Mattias Forsblad wrote: > On 2022-04-06 03:09, Jakub Kicinski wrote: > > On Mon, 4 Apr 2022 12:48:24 +0200 Mattias Forsblad wrote: > >> Limitations > >> If there is tc rules on a bridge and all the ports leave the bridge > >> and then joins the bridge again, the indirect framwork doesn't seem > >> to reoffload them at join. The tc rules need to be torn down and > >> re-added. > > > > You should unregister your callback when last DSA port leaves and > > re-register when first joins. That way you'll get replay. > > > > So I've tried that and it partially works. I get the FLOW_BLOCK_BIND > callback but tcf_action_reoffload_cb() bails out here (tc_act_bind() == 1): > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/sched/act_api.c?h=v5.18-rc1#n1819 > > B.c. that flag is set here: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/sched/cls_api.c?h=v5.18-rc1#n3088 > > I cannot say I fully understand this logic. Can you perhaps advise? tcf_action_reoffload_cb() is for action-as-first-class-citizen offload. I think you should get the reply thru tcf_block_playback_offloads(). But I haven't really kept up with the TC offloads, non-zero chance they got broken :/ > > Also the code needs to check the matchall is highest prio. > > Isn't sufficient with this check? > > else if (flow_offload_has_one_action(&cls->rule->action) && > cls->rule->action.entries[0].id == FLOW_ACTION_DROP) > err = dsa_slave_add_cls_matchall_drop(dev, cls, ingress); > > If it only has one action is must be the highest priority or am I > missing something? That just checks there is a single action on the rule. There could be multiple rules, adding something like: if (flow->common.prio != 1) goto bail; is what I had in mind.