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:   Tue, 1 Aug 2023 20:43:47 +0300
From:   Vladimir Oltean <vladimir.oltean@nxp.com>
To:     Vinicius Costa Gomes <vinicius.gomes@intel.com>
Cc:     netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Jamal Hadi Salim <jhs@mojatatu.com>,
        Cong Wang <xiyou.wangcong@gmail.com>,
        Jiri Pirko <jiri@resnulli.us>, linux-kernel@vger.kernel.org,
        intel-wired-lan@lists.osuosl.org,
        Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>,
        Peilin Ye <yepeilin.cs@gmail.com>,
        Pedro Tammela <pctammela@mojatatu.com>,
        Richard Cochran <richardcochran@gmail.com>,
        Zhengchao Shao <shaozhengchao@huawei.com>,
        Maxim Georgiev <glipus@gmail.com>
Subject: Re: [PATCH v2 net-next 7/9] net: netdevsim: mimic tc-taprio offload
Message-ID: <20230801174347.gtmxoqybhvbbl2rg@skbuf>
References: <20230613215440.2465708-1-vladimir.oltean@nxp.com>
 <20230613215440.2465708-8-vladimir.oltean@nxp.com>
 <877cs5twqn.fsf@intel.com>
 <20230801164534.2nklcql2nh6x6p7y@skbuf>
 <87o7jq64s4.fsf@intel.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87o7jq64s4.fsf@intel.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?L6deT84znxjLaSE6CfN+SClEo3JpCrTxPEmVXCveyHcyVRDeOykv7LQwVZ6y?=
 =?us-ascii?Q?/QbzWPgd9KtW7O/S/KHm0xPkETzOrrgXuShNZKsreOlxMTzElewP7KgvD1Y9?=
 =?us-ascii?Q?J1A+YbZBBL1zVb+IldD6W/xVXg7On6s7HHlctOI5KwlaBFsCxnBaI6WrM9TX?=
 =?us-ascii?Q?eJLwWKY70yrPSNJTrX4HAAejbFox/1pyoPrmLspXQSciyY1ytKRUEZSRwbYS?=
 =?us-ascii?Q?ZZuZClggXHCDEenBgVB3+e+TczEijjc3Y7eczEQaMYzsRhIahBXWBdHyhT8I?=
 =?us-ascii?Q?9S6TDZHbMh8iOYQFbf2H0xF7mtn9r47ZqSrW09dw5V5kVRr59L8krIOftMCv?=
 =?us-ascii?Q?vL6VJZ1Uc65/lNmW65H9tei07qXbjd038lp4V3GKq9WCXa0ZzEw8YgjuQytZ?=
 =?us-ascii?Q?+02Kj5K0I4R0ZP/6/yADMcq32rlVA11kC3M1lNShGCT/uHTBOyY1fWVw+lWV?=
 =?us-ascii?Q?/+5OybBlVg7HpfuX58yF4nyihi5u7d+4KsjqhYqke5wNIiLoHJ6FLaQGA8of?=
 =?us-ascii?Q?MnAR6qTmOT+QRo1VwyYMM62+EyiGPhHOb/hDqcwr2Mc/l3b/oiKFuJowFUV7?=
 =?us-ascii?Q?QsSF+lDD3Vj4+TIY0ANHrr3CIL91Dv1xH7fXb5vxwwvB9fRnEXNDcqT3Kwny?=
 =?us-ascii?Q?0ajIG77r04nGGRauEzrPJReBv2nM15ffeBBcZuAMWen0zz9hO6K/B28khPGH?=
 =?us-ascii?Q?chbG0SQi90Tt8T66gIvA6ovXPUcmVk3zR8EaOftK5r2Qgl8aLnUsTNGYjcSX?=
 =?us-ascii?Q?kYrW0v0k+AOYjNoG8U4Q8wt+I883KRMi+62MRiRVWCyNWUn0YqK/7eDXEvOz?=
 =?us-ascii?Q?Ubjovl/IKjXXggdTlr78W4x3e57YBvggunIcj4YK24d68k88TPXKzt+we60G?=
 =?us-ascii?Q?uczeduCCQfI1RdAj/oh6X+dRvOWLmajb9yai3WoZ42KEj93rjGFjW80I9MZp?=
 =?us-ascii?Q?+fdX3bRb8r2Mlt/A7FyR2RI7RAukvl2EoA+SsdWI8/rZVufMa/SbnNK8P5A6?=
 =?us-ascii?Q?aqv/ekYB+UgaNOpT2uu10eQFcgWFOCFwtg4Ki/yFmViCWfwUmZJoTf1YhDxO?=
 =?us-ascii?Q?fdOzHCs9ZmMy8F6NJQhiZ75FnyNcb/9nbHdDhQwPvHFI09t0kE+4/fpTG2QW?=
 =?us-ascii?Q?u++omynhViGwncjeOWuaE31VxpkU5bEfY2FAMY2oyURoRakxsvbERFFK+hRN?=
 =?us-ascii?Q?FAar06cKKsxBtRlgNRKXlEF+tb8MhdriKNOyCS6myWwz0e4t7FyXyIbNWfq/?=
 =?us-ascii?Q?FDbnivi0cgypmARcCFjU4in+xkFmGo5sv40RnjQ4PW9v9Yq/d1BBNZkGPOuO?=
 =?us-ascii?Q?xKkAr8jkUTYAnchjWH5Jr0ZE5rEsMQg8xtIF9Whbimbl1irqOXxmB/anMsgP?=
 =?us-ascii?Q?HUSrxBjX+rfMoRdkqsB42BRzzs2w8SrlgLmkqJ5ml8GUUnuN80kZmtctOEyJ?=
 =?us-ascii?Q?cLNBQb3KT5BGf/LgekCqIzjGLH8hwhbs2ymHCisR7Mfay249+pF7cJbh8xqN?=
 =?us-ascii?Q?wkyLgPNxonhjp/ECr3jkAV6d0Z/eCDjeQqEWvDJoLv26+YfLCvYqZSGisoab?=
 =?us-ascii?Q?ArAAS6rGE5VVcGX5ZDiOytzVnK9Rd6REg2+Pqrn0Z+hLRUfXmNpAhmURrkQ3?=
 =?us-ascii?Q?+w=3D=3D?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 24fbe469-1f0a-4819-acbe-08db92b6de26
X-MS-Exchange-CrossTenant-AuthSource: AM0PR04MB6452.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Aug 2023 17:43:51.7032
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 2rITiI4JU1QEoqbE8LAqv5/NonCH4G6YA8X4GUjJbgi91xAOEnmKlpRV93X3rpXJArb+r6NYsmy8hUM9QfGBSA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS5PR04MB9941
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE,
        URIBL_BLOCKED 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 Tue, Aug 01, 2023 at 10:39:23AM -0700, Vinicius Costa Gomes wrote:
> Hi Vladimir,
> 
> Vladimir Oltean <vladimir.oltean@nxp.com> writes:
> 
> > On Wed, Jun 14, 2023 at 05:06:24PM -0700, Vinicius Costa Gomes wrote:
> >> > +static int nsim_setup_tc_taprio(struct net_device *dev,
> >> > +				struct tc_taprio_qopt_offload *offload)
> >> > +{
> >> > +	int err = 0;
> >> > +
> >> > +	switch (offload->cmd) {
> >> > +	case TAPRIO_CMD_REPLACE:
> >> > +	case TAPRIO_CMD_DESTROY:
> >> > +		break;
> >> 
> >> I was thinking about how useful would proper validation of the
> >> parameters be? Thinking that we could detect "driver API" breakages
> >> earlier, and we want it documented that the drivers should check for the
> >> things that it supports.
> >> 
> >> Makes sense?
> >
> > Sorry, I lack imagination as to what the netdevsim driver may check for.
> > The taprio offload parameters should always be valid, properly speaking,
> > otherwise the Qdisc wouldn't be passing them on to the driver. At least
> > that would be the intention. The rest are hardware specific checks for
> > hardware specific limitations. Here there is no hardware.
> >
> 
> Trying to remember what was going through my mind when I said that.
> 
> What I seem to recall is something that would help us "keep honest":
> I was worrying about someone (perhaps myself ;-) sneaking a new feature
> in taprio and forgetting to update other drivers.
> 
> I thought that adding a check for the existing parameters would help
> detect those kind of things. If anything unknown was there in the
> offload struct, netdevsim would complain loudly.
> 
> Perhaps I was worrying too much. And the way to solve that is to keep
> active attention against that during review.

Ok, so I'm not making any change to the patch set as a result of this
comment, would you agree?