[{"id":1798915,"web_url":"http://patchwork.ozlabs.org/comment/1798915/","msgid":"<20171103205610.GA31088@breakpoint.cc>","list_archive_url":null,"date":"2017-11-03T20:56:10","subject":"Re: [PATCH RFC, WIP 5/5] netfilter: nft_flow_offload: add ndo hooks\n\tfor hardware offload","submitter":{"id":1025,"url":"http://patchwork.ozlabs.org/api/people/1025/","name":"Florian Westphal","email":"fw@strlen.de"},"content":"Pablo Neira Ayuso wrote:\n> +static void flow_offload_work(struct work_struct *work)\n> +{\n> +\tstruct flow_hw_offload *offload, *next;\n> +\n> +\tspin_lock_bh(&flow_hw_offload_lock);\n> +\tlist_for_each_entry_safe(offload, next, &flow_hw_offload_pending_list, list) {\n> +\t\tdo_flow_offload(offload->flow);\n\nThis should not offload flows that already have DYING bit set.\n\n> +\t\tnf_conntrack_put(&offload->ct->ct_general);\n> +\t\tlist_del(&offload->list);\n> +\t\tkfree(offload);\n> +\t}\n> +\tspin_unlock_bh(&flow_hw_offload_lock);\n> +\n> +\tqueue_delayed_work(system_power_efficient_wq, &nft_flow_offload_dwork, HZ);\n> +}\n\nMissed this on first round, 1 second is quite large.\n\n[..]\n\n> static int nft_flow_route(const struct nft_pktinfo *pkt,\n> \t\t\t const struct nf_conn *ct,\n> \t\t\t union flow_gateway *orig_gw,\n> @@ -211,6 +290,7 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,\n> \tunion flow_gateway orig_gateway, reply_gateway;\n> \tstruct net_device *outdev = pkt->xt.state->out;\n> \tstruct net_device *indev = pkt->xt.state->in;\n> +\tstruct flow_hw_offload *offload;\n> \tenum ip_conntrack_info ctinfo;\n> \tstruct flow_offload *flow;\n> \tstruct nf_conn *ct;\n> @@ -250,6 +330,21 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,\n> \tif (ret < 0)\n> \t\tgoto err2;\n> \n> +\tif (!indev->netdev_ops->ndo_flow_add)\n> +\t\treturn;\n> +\n> +\toffload = kmalloc(sizeof(struct flow_hw_offload), GFP_ATOMIC);\n> +\tif (!offload)\n> +\t\treturn;\n> +\n> +\tnf_conntrack_get(&ct->ct_general);\n> +\toffload->ct = ct;\n> +\toffload->flow = flow;\n> +\n> +\tspin_lock_bh(&flow_hw_offload_lock);\n> +\tlist_add_tail(&offload->list, &flow_hw_offload_pending_list);\n> +\tspin_unlock_bh(&flow_hw_offload_lock);\n> +\n> \treturn;\n\nSo this aims for lazy offloading (up to 1 second delay).\nIs this intentional, e.g. to avoid offloading short-lived 'RR' flows?\n\nI would have expected this to schedule the workqueue here, and not use\ndelayed wq at all (i.e., also no self-rescheduling from\nflow_offload_work()).","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yTDlg5Wv4z9s7p\n\tfor ;\n\tSat, 4 Nov 2017 07:56:47 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752329AbdKCU4l (ORCPT );\n\tFri, 3 Nov 2017 16:56:41 -0400","from Chamillionaire.breakpoint.cc ([146.0.238.67]:37742 \"EHLO\n\tChamillionaire.breakpoint.cc\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1750772AbdKCU4j (ORCPT\n\t); Fri, 3 Nov 2017 16:56:39 -0400","from fw by Chamillionaire.breakpoint.cc with local (Exim 4.84_2)\n\t(envelope-from )\n\tid 1eAj0c-00087Q-BU; Fri, 03 Nov 2017 21:56:10 +0100"],"Date":"Fri, 3 Nov 2017 21:56:10 +0100","From":"Florian Westphal ","To":"Pablo Neira Ayuso ","Cc":"netfilter-devel@vger.kernel.org, netdev@vger.kernel.org","Subject":"Re: [PATCH RFC, WIP 5/5] netfilter: nft_flow_offload: add ndo hooks\n\tfor hardware offload","Message-ID":"<20171103205610.GA31088@breakpoint.cc>","References":"<20171103152636.9967-1-pablo@netfilter.org>\n\t<20171103152636.9967-6-pablo@netfilter.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20171103152636.9967-6-pablo@netfilter.org>","User-Agent":"Mutt/1.5.23 (2014-03-12)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1803140,"web_url":"http://patchwork.ozlabs.org/comment/1803140/","msgid":"","list_archive_url":null,"date":"2017-11-11T12:49:29","subject":"Re: [PATCH RFC, WIP 5/5] netfilter: nft_flow_offload: add ndo hooks\n\tfor hardware offload","submitter":{"id":8537,"url":"http://patchwork.ozlabs.org/api/people/8537/","name":"Felix Fietkau","email":"nbd@nbd.name"},"content":"On 2017-11-03 16:26, Pablo Neira Ayuso wrote:\n> This patch adds the infrastructure to offload flows to hardware, in case\n> the nic/switch comes with built-in flow tables capabilities.\n> \n> If the hardware comes with not hardware flow tables or they have\n> limitations in terms of features, this falls back to the software\n> generic flow table implementation.\n> \n> The software flow table aging thread skips entries that resides in the\n> hardware, so the hardware will be responsible for releasing this flow\n> table entry too.\n> \n> Signed-off-by: Pablo Neira Ayuso \nHi Pablo,\n\nI'd like to start playing with those patches in OpenWrt/LEDE soon. I'm\nalso considering making a patch that adds iptables support.\nFor that to work, I think it would be a good idea to keep the code that\ntries to offload flows to hardware in nf_flow_offload.c instead, so that\nit can be shared with iptables integration.\n\nBy the way, do you have a git tree where you keep the current version of\nyour patch set?\n\nThanks,\n\n- Felix","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=nbd.name header.i=@nbd.name header.b=\"sUtTSngd\";\n\tdkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yYxYs0zLCz9sNd\n\tfor ;\n\tSat, 11 Nov 2017 23:49:37 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753890AbdKKMtc (ORCPT );\n\tSat, 11 Nov 2017 07:49:32 -0500","from nbd.name ([46.4.11.11]:60146 \"EHLO nbd.name\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1752736AbdKKMtb (ORCPT );\n\tSat, 11 Nov 2017 07:49:31 -0500"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=nbd.name; \n\ts=20160729;\n\th=Content-Transfer-Encoding:Content-Type:In-Reply-To:\n\tMIME-Version:Date:Message-ID:From:References:Cc:To:Subject:Sender:Reply-To:\n\tContent-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender:\n\tResent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:\n\tList-Subscribe:List-Post:List-Owner:List-Archive;\n\tbh=qCULKvbYPCzgqQ0nv+F0OUgrw0BvdNuZE/FbJHEnMZo=;\n\tb=sUtTSngdkk7Lg3gETeONUsixDY\n\tSTJ4OLtIwLC33A/qLqqHNnfrOLyylgipFA+BxfZOBh08WqJb775kcS1QngqBwzI/n3lYRn2iGKv8H\n\tGaP/60a5cuF6CDVXYD6vQhBOHWQ5wpZpwdbWLEF4Agc3WynepUH1mJRliM7grcy+Po38=;","Subject":"Re: [PATCH RFC, WIP 5/5] netfilter: nft_flow_offload: add ndo hooks\n\tfor hardware offload","To":"Pablo Neira Ayuso , netfilter-devel@vger.kernel.org","Cc":"netdev@vger.kernel.org","References":"<20171103152636.9967-1-pablo@netfilter.org>\n\t<20171103152636.9967-6-pablo@netfilter.org>","From":"Felix Fietkau ","Message-ID":"","Date":"Sat, 11 Nov 2017 13:49:29 +0100","User-Agent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:52.0)\n\tGecko/20100101 Thunderbird/52.4.0","MIME-Version":"1.0","In-Reply-To":"<20171103152636.9967-6-pablo@netfilter.org>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}}]