netdev - Re: [PATCH v2 net-next 3/9] net: enetc: only commit preemptible TCs to hardware when MM TX is active

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZEFPbNCNDWy0c8eK@corigine.com>
Date:   Thu, 20 Apr 2023 16:42:52 +0200
From:   Simon Horman <simon.horman@...igine.com>
To:     Vladimir Oltean <vladimir.oltean@....com>
Cc:     netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
        Michal Kubecek <mkubecek@...e.cz>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Xiaoliang Yang <xiaoliang.yang_1@....com>,
        Petr Machata <petrm@...dia.com>,
        Danielle Ratson <danieller@...dia.com>,
        Pranavi Somisetty <pranavi.somisetty@....com>,
        Harini Katakam <harini.katakam@....com>,
        Vinicius Costa Gomes <vinicius.gomes@...el.com>,
        Kurt Kanzenbach <kurt@...utronix.de>,
        Gerhard Engleder <gerhard@...leder-embedded.com>,
        Ferenc Fejes <ferenc.fejes@...csson.com>,
        Aaron Conole <aconole@...hat.com>,
        linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 net-next 3/9] net: enetc: only commit preemptible TCs
 to hardware when MM TX is active

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@....com>

Reviewed-by: Simon Horman <simon.horman@...igine.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);
> +}

...