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=-8.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,NICE_REPLY_A,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,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 00C41C2D0A3 for ; Mon, 16 Nov 2020 18:40:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id AD5A720855 for ; Mon, 16 Nov 2020 18:40:38 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=ti.com header.i=@ti.com header.b="mAIum+d/" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727425AbgKPSkS (ORCPT ); Mon, 16 Nov 2020 13:40:18 -0500 Received: from fllv0015.ext.ti.com ([198.47.19.141]:53652 "EHLO fllv0015.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726473AbgKPSkR (ORCPT ); Mon, 16 Nov 2020 13:40:17 -0500 Received: from lelv0266.itg.ti.com ([10.180.67.225]) by fllv0015.ext.ti.com (8.15.2/8.15.2) with ESMTP id 0AGIe4VZ065486; Mon, 16 Nov 2020 12:40:04 -0600 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ti.com; s=ti-com-17Q1; t=1605552004; bh=vZvZmpRXLdOHlrWsWFSQrRNMSkunry0WyLmDdpA6E9Q=; h=Subject:To:CC:References:From:Date:In-Reply-To; b=mAIum+d/03aEKtL3z96ILs1FYIi438i/qXPanl4WxXaUN8l0B3Mr6DE2ybAToxjUu GavW2u9m0ewd8iL+5XSVEzB381bOZgzZ/TGbYdPmgGHLpzmEiG2oa4bVWOSYOHTB3e KrBfgNzSL3Ps8nhgMASvyuglV6NhhAG0wPHwcBtg= Received: from DFLE113.ent.ti.com (dfle113.ent.ti.com [10.64.6.34]) by lelv0266.itg.ti.com (8.15.2/8.15.2) with ESMTPS id 0AGIe4pi049994 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=FAIL); Mon, 16 Nov 2020 12:40:04 -0600 Received: from DFLE109.ent.ti.com (10.64.6.30) by DFLE113.ent.ti.com (10.64.6.34) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1979.3; Mon, 16 Nov 2020 12:40:03 -0600 Received: from lelv0326.itg.ti.com (10.180.67.84) by DFLE109.ent.ti.com (10.64.6.30) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1979.3 via Frontend Transport; Mon, 16 Nov 2020 12:40:03 -0600 Received: from [10.250.100.73] (ileax41-snat.itg.ti.com [10.172.224.153]) by lelv0326.itg.ti.com (8.15.2/8.15.2) with ESMTP id 0AGIdxRj054660; Mon, 16 Nov 2020 12:40:00 -0600 Subject: Re: [PATCH net-next 3/3] net: ethernet: ti: am65-cpsw: enable broadcast/multicast rate limit support To: Vladimir Oltean CC: "David S. Miller" , , Jakub Kicinski , Vignesh Raghavendra , Sekhar Nori , , , Tony Lindgren , Jamal Hadi Salim , Andrew Lunn , Cong Wang , Jiri Pirko References: <20201114035654.32658-1-grygorii.strashko@ti.com> <20201114035654.32658-4-grygorii.strashko@ti.com> <20201114191723.rvmhyrqinkhdjtpr@skbuf> From: Grygorii Strashko Message-ID: Date: Mon, 16 Nov 2020 20:39:54 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <20201114191723.rvmhyrqinkhdjtpr@skbuf> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 Precedence: bulk List-ID: X-Mailing-List: linux-omap@vger.kernel.org On 14/11/2020 21:17, Vladimir Oltean wrote: > On Sat, Nov 14, 2020 at 05:56:54AM +0200, Grygorii Strashko wrote: >> This patch enables support for ingress broadcast(BC)/multicast(MC) rate limiting >> in TI AM65x CPSW driver (the corresponding ALE support was added in previous >> patch) by implementing HW offload for simple tc-flower policer with matches >> on dst_mac: >> - ff:ff:ff:ff:ff:ff has to be used for BC rate limiting >> - 01:00:00:00:00:00 fixed value has to be used for MC rate limiting >> >> Hence tc policer defines rate limit in terms of bits per second, but the >> ALE supports limiting in terms of packets per second - the rate limit >> bits/sec is converted to number of packets per second assuming minimum >> Ethernet packet size ETH_ZLEN=60 bytes. >> >> Examples: >> - BC rate limit to 1000pps: >> tc qdisc add dev eth0 clsact >> tc filter add dev eth0 ingress flower skip_sw dst_mac ff:ff:ff:ff:ff:ff \ >> action police rate 480kbit burst 64k >> >> rate 480kbit - 1000pps * 60 bytes * 8, burst - not used. >> >> - MC rate limit to 20000pps: >> tc qdisc add dev eth0 clsact >> tc filter add dev eth0 ingress flower skip_sw dst_mac 01:00:00:00:00:00 \ >> action police rate 9600kbit burst 64k >> >> rate 9600kbit - 20000pps * 60 bytes * 8, burst - not used. >> >> Signed-off-by: Grygorii Strashko >> --- > > I understand this is unpleasant feedback, but don't you want to extend > tc-police to have an option to rate-limit based on packet count and not > based on byte count? Huh. I'd be appreciated if you could provide more detailed opinion of how it can look like? Sry, it's my first experience with tc. > The assumption you make in the driver that the > packets are all going to be minimum-sized is not a great one. > I can > imagine that the user's policer budget is vastly exceeded if they enable > jumbo frames and they put a policer at 9.6 Mbps, and this is not at all > according to their expectation. 20Kpps assuming 60 bytes per packet > might be 9.6 Mbps, and the user will assume this bandwidth profile is > not exceeded, that's the whole point. But 20Kpps assuming 9KB per packet > is 1.44Gbps. Weird. Sry, not sure I completely understood above. This is specific HW feature, which can imit packet rate only. And it is expected to be applied by admin who know what he is doing. The main purpose is to preserve CPU resource, which first of all affected by packet rate. So, I see it as not "assumption", but requirement/agreement which will be reflected in docs and works for a specific use case which is determined by presence of: - skip_sw flag - specific dst_mac/dst_mac_mask pair in which case rate determines pps and not K/Mbps. Also some ref on previous discussion [1] [2] [1] https://www.spinics.net/lists/netdev/msg494630.html [2] https://lore.kernel.org/patchwork/patch/481285/ -- Best regards, grygorii