Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 18 Feb 2022 11:38:14 +0100
From:   Simon Horman <simon.horman@corigine.com>
To:     Roi Dayan <roid@nvidia.com>, Ido Schimmel <idosch@idosch.org>
Cc:     Jianbo Liu <jianbol@nvidia.com>, linux-kernel@vger.kernel.org,
        netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
        olteanv@gmail.com, andrew@lunn.ch, vivien.didelot@gmail.com,
        f.fainelli@gmail.com, davem@davemloft.net, kuba@kernel.org,
        rajur@chelsio.com, claudiu.manoil@nxp.com, sgoutham@marvell.com,
        gakula@marvell.com, sbhatta@marvell.com, hkelam@marvell.com,
        saeedm@nvidia.com, leon@kernel.org, idosch@nvidia.com,
        petrm@nvidia.com, alexandre.belloni@bootlin.com,
        UNGLinuxDriver@microchip.com, jhs@mojatatu.com,
        xiyou.wangcong@gmail.com, jiri@resnulli.us,
        baowen.zheng@corigine.com, louis.peens@netronome.com,
        peng.zhang@corigine.com, oss-drivers@corigine.com
Subject: Re: [PATCH net-next v2 0/2] flow_offload: add tc police parameters
Message-ID: <20220218103813.GA29654@corigine.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Yg44A/JcKmsTU6N4@shredder>
 <5e9a4e05-8501-4d21-c8b9-91c992c109f0@nvidia.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?aZUN1vKQmeYFRe8qGRkfN9t5UVVFLN0gO9LRORCgACSsmtfiwE1mLSmZPF2M?=
 =?us-ascii?Q?pOAByH2O8+vNbeTUqWgGVD1yU8HBaHWY4j1ezMHX1DJEGD8ToWGVkBUJrXcf?=
 =?us-ascii?Q?CyupHK/k/GDMGyWpAm/ltYF8vrJ03Rq9vUwcP0iWNC//8KleUrgUFBb8Vpvb?=
 =?us-ascii?Q?t7zUhQcYUau+oiftmwYAfbdEt5bDLn9+qDJiZlpddf12rNbOCFR3T16VBKN8?=
 =?us-ascii?Q?XkcG4SPfCg6QNyhGu03qiaHNMP2bf0oB/oTNLy5H+Vw+EejJanIh5WPK3s79?=
 =?us-ascii?Q?+XvIwIDlT6NW3eqAGGv1JgU3rfmiuzNhuJz/SEWsLIORSIa4c7NRSLCq/oLw?=
 =?us-ascii?Q?FI2jFeLAS3sw2E9EoZnb2ODJJBIeA4wK+tgMawU/6eQIL42RUj/OMMx57sFY?=
 =?us-ascii?Q?q3EAqCdc1dfL4Y/MQOKkHImfg6qVoFGxHjiEuu6klaqlF0j5sTC3AU+GkGmu?=
 =?us-ascii?Q?o8ueWomOsBTCcNyrcx7oNK3f1MwbbR1ska+r0mUGns8Ljn2Ig2KeyLOOJo/9?=
 =?us-ascii?Q?vKkGWsiwAl9xgFz9AGWt8D3l0yMG+xfmhrmwbeKK80uN0vB+FoMR5elQycB1?=
 =?us-ascii?Q?/okfNj4sUAW5EDwozlo5Fbu+2xJUW3C0hAeH6v6tlUmHRGtRqk47RZIFDiU8?=
 =?us-ascii?Q?uWVF5fXj63qftPELzoiDHOOOC2eGQrVISvTPDdbWmM+0JrTzBldbS1bUwTb6?=
 =?us-ascii?Q?zvnkArFFas7s0FOEg0fknkFR/RmGErkWI7wOYqwZ7PSmASBwXQcN0Jb5Hp9a?=
 =?us-ascii?Q?4kLIpu+Al82e5uLeWGgRja4QZ9lVswHIS6Hwb93gps6FqXwKTrcRTl+zUXkz?=
 =?us-ascii?Q?zeqousfdjIH+ZbKPCDwg9SuXo6ZiralvziU4z4ZBMcNdiGnFwhXY4zH5/nW6?=
 =?us-ascii?Q?5oAgJlFehdlZTKC+C4r8t9/XcPTVKQ5h2om1nT6NjVCtaVlFbUgN05TC+UmT?=
 =?us-ascii?Q?lK4RJyHVTjhI6evjsGOS+1R4M+l9sS5Cxzx10ytCvOI2g/BS2LOhvoEyBXHT?=
 =?us-ascii?Q?HMKxPSghlLmqjcxCFqAMShJqIiWEWXBwdgKH5mcfabJz6jT0YZABhW48RaD6?=
 =?us-ascii?Q?+FYWom1UVOQwChiGxG9vfbO8grQ199GTW9M9PwIyf9QXvlSuwtAqzVZXFxAe?=
 =?us-ascii?Q?c3f44UxkG/mJ5UHVbFmwy2m6h1025PZMH6MiRGct9+hmDlFuraB+5SXS/pOO?=
 =?us-ascii?Q?/paZHbATT0RhAhNbU1bTGgFKHbVbTPsF0r2EcNAjhygVn4xi5lR2QhwyilTO?=
 =?us-ascii?Q?UnEi4DvR7AMCFN7We8hnofKcPRfX+2oCisTnFtbPqoJbH6UAl14ZjfNZi3fy?=
 =?us-ascii?Q?RFjyoTXBALYk6Stsl97Bmf2kl2sfi/PHyEI0KBSTAtWxS8gQFVPKfCWXqdV4?=
 =?us-ascii?Q?IK3xoib7+H1kFUm38Of4GjFMHUKRofCsHknadi1lxR3MUyYGK6TS5s9vK3Vs?=
 =?us-ascii?Q?a4QPq+j3wr1UUz0i96cB8qpCIIX5AMieJtT+sncUwZSfd6uJTQLyT2L6y4Zc?=
 =?us-ascii?Q?QVKEmMwpRHUK3kAZwA9XwfY5r7MGbskdk7sSZ50kixlC7BhpP/x0LKXVLkh9?=
 =?us-ascii?Q?7eyPjoZeMiFDAfeBFyE5oLbGaZ73ig5bO7Q6Yc9RONCQE1iMIIaJ3j0RXmIo?=
 =?us-ascii?Q?7Ym53aKjjinwanm+SS1OMas93+/8grDi7W7GEy/YRLGk2e1GS4WqTjhr4joD?=
 =?us-ascii?Q?EtPIfSgu3cPF2Ot2YgM/MZC3LP/YV5QcDlKFWCZzkLssYetc7cBNvn+n35cT?=
 =?us-ascii?Q?5Qnd/+C+zGaUL7kjCHny8pFNJGGhxRk=3D?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 8d2ebf7a-2d28-4795-1c08-08d9f2caca07
X-MS-Exchange-CrossTenant-AuthSource: PH0PR13MB4842.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Feb 2022 10:38:23.9851
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: fe128f2c-073b-4c20-818e-7246a585940c
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: GLSMo26Q9sOK8XOqJDGHF6qCmNyMyQ+KtCQWApG0mtuWAcHmYNzPvG9hufDsN5VzAlf8lnu67CRaHSV1rukVJVB79WPZFMbWHsx80POCWIk=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR13MB1248
X-Spam-Status: No, score=-0.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,FAKE_REPLY_C,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,
        SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, Feb 17, 2022 at 01:52:47PM +0200, Roi Dayan wrote:
> On 2022-02-17 1:34 PM, Simon Horman wrote:
> > On Thu, Feb 17, 2022 at 08:28:01AM +0000, Jianbo Liu wrote:
> > > As a preparation for more advanced police offload in mlx5 (e.g.,
> > > jumping to another chain when bandwidth is not exceeded), extend the
> > > flow offload API with more tc-police parameters. Adjust existing
> > > drivers to reject unsupported configurations.
> > 
> > Hi,
> > 
> > I have a concern that
> > a) patch 1 introduces a facility that may break existing drivers; and
> > b) patch 2 then fixes this
> > 
> > I'd slightly prefer if the series was rearranged to avoid this problem.
> > 
> > ...
> 
> Hi Simon,
> 
> It can't be rearranged as patch 2 can't compile without patch 1.
> Patch 1 only adds more information passing to the driver.
> 
> The drivers functionality doesn't change. drivers today ignore
> police information, like actions, and still being ignored after patch 1.
> 
> patch 2 updates the drivers to use that information instead of
> ignoring it. so it fixes the drivers that without patch 1 can't be
> fixed.

I think it would be possible, but...

On Thu, Feb 17, 2022 at 01:56:51PM +0200, Ido Schimmel wrote:

...

> Not sure what you mean by the above. Patch #1 extends the flow offload
> API with tc-police parameters that weren't communicated to drivers until
> now. Drivers still ignore the new parameters after this patch. It is
> only in patch #2 that these drivers reject configurations where the
> parameters are set.
> 
> Therefore, the only breakage I see is the one that can happen after
> patch #2: unaware user space that was installing actions that weren't
> fully reflected to hardware.
> 
> If we want to be on the safe side, it is possible to remove the errors,
> but keep the extack messages so that user space is at least somewhat
> aware.

Yes, I see what you mean.
I'm now comfortable with the way this patchset is arranged.