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:   Thu, 20 Apr 2023 16:42:52 +0200
From:   Simon Horman <simon.horman@corigine.com>
To:     Vladimir Oltean <vladimir.oltean@nxp.com>
Cc:     netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
        Michal Kubecek <mkubecek@suse.cz>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Claudiu Manoil <claudiu.manoil@nxp.com>,
        Xiaoliang Yang <xiaoliang.yang_1@nxp.com>,
        Petr Machata <petrm@nvidia.com>,
        Danielle Ratson <danieller@nvidia.com>,
        Pranavi Somisetty <pranavi.somisetty@amd.com>,
        Harini Katakam <harini.katakam@amd.com>,
        Vinicius Costa Gomes <vinicius.gomes@intel.com>,
        Kurt Kanzenbach <kurt@linutronix.de>,
        Gerhard Engleder <gerhard@engleder-embedded.com>,
        Ferenc Fejes <ferenc.fejes@ericsson.com>,
        Aaron Conole <aconole@redhat.com>,
        linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 net-next 3/9] net: enetc: only commit preemptible TCs
 to hardware when MM TX is active
Message-ID: <ZEFPbNCNDWy0c8eK@corigine.com>
References: <20230418111459.811553-1-vladimir.oltean@nxp.com>
 <20230418111459.811553-4-vladimir.oltean@nxp.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230418111459.811553-4-vladimir.oltean@nxp.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?XFiAvAusw2Y4CqdKvJHnggsu4ToHB1YhzY1IsyrTW5AgPdNjjIh1/S72X+rh?=
 =?us-ascii?Q?mKw7X2qlvVF/OULMKOwThgrkjnO5O3n5xS6D/yxY1BsyNp47e62KeHLtJM+d?=
 =?us-ascii?Q?aBsNRefZ+03aJlBmqWjQY1IiJwz8A1uCzCba1NSlK4MA3ak6fPHK/nIKvtqO?=
 =?us-ascii?Q?vspWuaXs6Ufbmyv90JlyR84Ra3eQ+lJYUSnA1Rg6ZlOsOcDqJTR3ZD4KTtNF?=
 =?us-ascii?Q?Sz6ydkpAKOu6ONZLntpp+27PjOvoN0w3/6rI8KW6Ad5YRZuQXdtad9iDgwSc?=
 =?us-ascii?Q?HGiv5J6XVxq99NvCvujjlwQRNfbjn1y6oIVVwSfHV5YTiqIEsMLPYbdTS2zq?=
 =?us-ascii?Q?SuV+KUb7YLrb97e5qjgGhmQtlXPxJEjGf1vxPqywIrRjdV5udXrL5ofarYUq?=
 =?us-ascii?Q?mDS5aVPxw4Ks4U6qBxpqnjSWhcFCW/hreUu1IgfpFMZXNu3/V3KwFQ3E0jzU?=
 =?us-ascii?Q?oLts7r2OyHnEb6Q3i4fN3RYNGRA/7jNrp4B73khOe3chtpwUh7Ndh+bUVRtT?=
 =?us-ascii?Q?VcIej4/O4jGN+XXT46xZJAeLHbrqTC6t0C8KyDpYPYDsnYBfmffxSwvxPovt?=
 =?us-ascii?Q?gRbgDorvIQuz7cp1EMbDwwPGIGvIyovZunWKWMcy1MaInesioneC6Bbv1j1W?=
 =?us-ascii?Q?oVwMZutbvTUkNKEfW8sZwZhgtmwGZEj3ND+EC97FslM7fSpznsKT02CCzi4f?=
 =?us-ascii?Q?zsRoCgsJ1J00NtpBTgEhm0xRH3gYSFiUHuxxhlwOVv8CQLvPQTTPSndGHiI1?=
 =?us-ascii?Q?uS5Z7mPS6IsoNmHw+mVEKAaqFPKJHnn5XMdSTkgy+MIM4BlePlfo8X5GnPMR?=
 =?us-ascii?Q?xhS7UMNynyyLHLZ3HwoppRWlqZ0ParBjNfMNxsmfaQbf97XZatvWXa8z0D4p?=
 =?us-ascii?Q?DwYJWrXtgFAvppLlxVSr3JE8BxzBrUHV4GFY6Xk3Y8Vc9aA18LhKs4+7o5uD?=
 =?us-ascii?Q?4NyBxmNGTxac+jLGNWs3FrDWo80ZFAvWhJfc+/IAWKRbJYkNzRh/RA7xjVQ5?=
 =?us-ascii?Q?ZNnmznFY7hHWII9L2AWgNcXMhS4o4fbe0xgHfex2w06fWPl7HhFGlVMCM+1f?=
 =?us-ascii?Q?4sMd3pF43wUfojVOanmX2xYEDiUc/Eu+1asmcaMNyEx9QSJQsObQvw88dry4?=
 =?us-ascii?Q?NCagDD22AC0xkM5+v+31cxY543piD9Dy3KQHGInevnEbl3MSdZDtXQOGBgoS?=
 =?us-ascii?Q?Xxs6mWObMxMh7fXJliQE9u9Z8VHfX8/aeIJC9prtDWhR9W76rfQkR4U8SKxe?=
 =?us-ascii?Q?9dW2kCvkBZz48jfKnmhBEql3Rx/5CDkRrJwg9hQ5NhAJW6Wxa5fzHIWBs51J?=
 =?us-ascii?Q?/zTLkrXB83ZkMMg1uCFvJg1zVQ91V07Gbj3543SCytJ9pC0v+Av8br2Sb7up?=
 =?us-ascii?Q?a9yyGLdcjaKWtbmARbywpGdcQ/wX1lVxx2nX/VvA4CYfUnZsWuZFgOW6jist?=
 =?us-ascii?Q?jYP40OyLb0u9eLmzs4U0ACdQlFZ5uvo6NtTQp20A0xLGuX7Mf4RkyWiSmGPH?=
 =?us-ascii?Q?NL4DbAtxlANoLTTDXLqGA7JBXwtJqHGD6I6PIeSX+m/Nrqo3TBbBMlx+ZZDE?=
 =?us-ascii?Q?1VX8qKv4vnuqxFZAda+ampwDAAyHrsYpt4dlqmY7EKCw0wN6X3T/26v3p6Xn?=
 =?us-ascii?Q?9qw+XTYfeGaq8jDEV5M/oWqkr5ta8BsgMneHnPMlr8AxSG+4WmUGP1xs6aI+?=
 =?us-ascii?Q?gy22JA=3D=3D?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 83a748a0-7ae7-4991-cce9-08db41ad89da
X-MS-Exchange-CrossTenant-AuthSource: PH0PR13MB4842.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Apr 2023 14:43:00.3678
 (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: hwgXh9IrVupIWUBd6tTblgbS+Hv8r74vqGnWc/FAwQrmesfpZ0KT9gDP+fShy9wrEgsTK7B0m3w6h7FMPxSAuatWyFboHxOIKBrNk6TspOs=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR13MB4410
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,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 Tue, Apr 18, 2023 at 02:14:53PM +0300, Vladimir Oltean wrote:
> This was left as TODO in commit 01e23b2b3bad ("net: enetc: add support
> for preemptible traffic classes") since it's relatively complicated.
> 
> Where this makes a difference is with a configuration as follows:
> 
> ethtool --set-mm eno0 pmac-enabled on tx-enabled on verify-enabled on
> 
> Preemptible packets should only be sent when the MAC Merge TX direction
> becomes active (i.o.w. when the verification process succeeds, aka when
> the link partner confirms it can process preemptible traffic). But the
> tc qdisc with the preemptible traffic classes is offloaded completely
> asynchronously w.r.t. the MM becoming active.
> 
> The ENETC manual does suggest that this should be handled in the driver:
> "On startup, software should wait for the verification process to
> complete (MMCSR[VSTS]=011) before initiating traffic".
> 
> Adding the necessary logic allows future selftests to uphold the claim
> that an inactive or disabled MAC Merge layer should never send data
> packets through the pMAC.
> 
> This change moves enetc_set_ptcfpr() from enetc.c to enetc_ethtool.c,
> where its only caller is now - enetc_mm_commit_preemptible_tcs().
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>

>  int enetc_qos_query_caps(struct net_device *ndev, void *type_data);
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c b/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
> index deb674752851..838a92131963 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
> @@ -991,6 +991,64 @@ static int enetc_get_mm(struct net_device *ndev, struct ethtool_mm_state *state)
>  	return 0;
>  }
>  
> +static int enetc_mm_wait_tx_active(struct enetc_hw *hw, int verify_time)
> +{
> +	int timeout = verify_time * USEC_PER_MSEC * ENETC_MM_VERIFY_RETRIES;
> +	u32 val;
> +
> +	/* This will time out after the standard value of 3 verification
> +	 * attempts. To not sleep forever, it relies on a non-zero verify_time,
> +	 * guarantee which is provided by the ethtool nlattr policy.
> +	 */
> +	return read_poll_timeout(enetc_port_rd, val,
> +				 ENETC_MMCSR_GET_VSTS(val) == 3,

nit: 3 is doing a lot of work here.
     As a follow-up, perhaps it could become part of an enum?

> +				 ENETC_MM_VERIFY_SLEEP_US, timeout,
> +				 true, hw, ENETC_MMCSR);
> +}

...