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 X-Spam-Level: X-Spam-Status: No, score=-10.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 328C3C636C8 for ; Thu, 15 Jul 2021 21:36:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 12835613AB for ; Thu, 15 Jul 2021 21:36:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231254AbhGOVjZ (ORCPT ); Thu, 15 Jul 2021 17:39:25 -0400 Received: from mail.netfilter.org ([217.70.188.207]:43466 "EHLO mail.netfilter.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229794AbhGOVjX (ORCPT ); Thu, 15 Jul 2021 17:39:23 -0400 Received: from netfilter.org (unknown [90.77.255.23]) by mail.netfilter.org (Postfix) with ESMTPSA id 2584664228; Thu, 15 Jul 2021 23:36:09 +0200 (CEST) Date: Thu, 15 Jul 2021 23:36:26 +0200 From: Pablo Neira Ayuso To: Felix Fietkau Cc: linux-wireless@vger.kernel.org, netdev@vger.kernel.org, ryder.lee@mediatek.com Subject: Re: [RFC 3/7] net: ethernet: mtk_eth_soc: implement flow offloading to WED devices Message-ID: <20210715213626.GA19271@salvia> References: <20210713160745.59707-1-nbd@nbd.name> <20210713160745.59707-4-nbd@nbd.name> <20210713185641.GB26070@salvia> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Wed, Jul 14, 2021 at 10:26:08AM +0200, Felix Fietkau wrote: > On 2021-07-13 20:56, Pablo Neira Ayuso wrote: [...] > >> --- a/drivers/net/ethernet/mediatek/mtk_ppe_offload.c > >> +++ b/drivers/net/ethernet/mediatek/mtk_ppe_offload.c > >> @@ -10,6 +10,7 @@ > >> #include > >> #include > >> #include "mtk_eth_soc.h" > >> +#include "mtk_wed.h" > >> > >> struct mtk_flow_data { > >> struct ethhdr eth; > >> @@ -39,6 +40,7 @@ struct mtk_flow_entry { > >> struct rhash_head node; > >> unsigned long cookie; > >> u16 hash; > >> + s8 wed_index; > >> }; > >> > >> static const struct rhashtable_params mtk_flow_ht_params = { > >> @@ -127,35 +129,38 @@ mtk_flow_mangle_ipv4(const struct flow_action_entry *act, > >> } > >> > >> static int > >> -mtk_flow_get_dsa_port(struct net_device **dev) > >> +mtk_flow_set_output_device(struct mtk_eth *eth, struct mtk_foe_entry *foe, > >> + struct net_device *dev, const u8 *dest_mac, > >> + int *wed_index) > >> { > >> -#if IS_ENABLED(CONFIG_NET_DSA) > >> - struct dsa_port *dp; > >> - > >> - dp = dsa_port_from_netdev(*dev); > >> - if (IS_ERR(dp)) > >> - return -ENODEV; > >> - > >> - if (dp->cpu_dp->tag_ops->proto != DSA_TAG_PROTO_MTK) > >> - return -ENODEV; > >> + struct net_device_path_ctx ctx = { > >> + .dev = dev, > >> + .daddr = dest_mac, > >> + }; > >> + struct net_device_path path = {}; > >> + int pse_port; > >> > >> - *dev = dp->cpu_dp->master; > >> + if (!dev->netdev_ops->ndo_fill_forward_path || > >> + dev->netdev_ops->ndo_fill_forward_path(&ctx, &path) < 0) > >> + path.type = DEV_PATH_ETHERNET; > > > > Maybe expose this through flow offload API so there is no need to call > > ndo_fill_forward_path again from the driver? > > Can you give me a pseudo-code example? I'm not sure how you want it to > be exposed through the flow offload API. in a few steps: 1) Extend nft_dev_path_info() to deal with DEV_PATH_WDMA, it will just actually fetch a pointer to structure that is allocated by the driver. - Update the net_device_path structure with this layout: struct flow_action_wdma { enum wdma_type type; // MTK_WDMA goes here union { struct { ...; } mtk; }; } wdma; Add: struct flow_action_wdma *wdma; to net_device_path. 2) Pass on this pointer to structure to the nf_flow_route wheelbarrow. 3) Store this information in the struct flow_offload_tuple, in a new struct flow_offload_hw *field to store all hardware offload specific information (not needed by software path). There is already hw_outdev that can be placed there. 4) Add a FLOW_ACTION_WDMA action to the flow offload API to pass on the flow_action_wdma structure. It's a bit of work the first time to accomodate the requirements of new API, but then all drivers will benefit from this. It's also a bit of layering, but with more drivers in the tree, this API can be simplified incrementally. I can take a stab at it and send you a patch. > To me it seems easier and cleaner to just have a single > ndo_fill_forward_path call for the final output device to check the > device types that don't have any corresponding sw offload. It's simpler yes, but this results in two calls for ndo_fill_forward_path, one from the core and another from the driver. I think it's better there's a single point to call to ndo_fill_forward_path for consolidation. My proposal requires a bit more plumbing, but all drivers will get the information that represents the offload in the same way.