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: <96c4f619-857c-4a28-ac86-5a07214842a8@kernel.org>
Date: Mon, 4 Dec 2023 11:30:53 +0200
From: Roger Quadros <rogerq@...nel.org>
To: "Varis, Pekka" <p-varis@...com>, "davem@...emloft.net"
 <davem@...emloft.net>, "edumazet@...gle.com" <edumazet@...gle.com>,
 "kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com"
 <pabeni@...hat.com>, "vladimir.oltean@....com" <vladimir.oltean@....com>
Cc: "Vadapalli, Siddharth" <s-vadapalli@...com>,
 "Gunasekaran, Ravi" <r-gunasekaran@...com>,
 "Raghavendra, Vignesh" <vigneshr@...com>,
 "Govindarajan, Sriramakrishnan" <srk@...com>,
 "horms@...nel.org" <horms@...nel.org>,
 "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [EXTERNAL] [PATCH v7 net-next 6/8] net: ethernet: ti:
 am65-cpsw-qos: Add Frame Preemption MAC Merge support

Hi Pekka,

On 01/12/2023 18:01, Varis, Pekka wrote:
> 
> 
>> -----Original Message-----
>> From: Roger Quadros <rogerq@...nel.org>
>>
>> Add driver support for viewing / changing the MAC Merge sublayer
>> parameters and seeing the verification state machine's current state via
>> ethtool.
>>
>> As hardware does not support interrupt notification for verification events
>> we resort to polling on link up. On link up we try a couple of times for
>> verification success and if unsuccessful then give up.
>>
>> The Frame Preemption feature is described in the Technical Reference
>> Manual [1] in section:
>> 	12.3.1.4.6.7 Intersperced Express Traffic (IET – P802.3br/D2.0)
>>
>> Due to Silicon Errata i2208 [2] we set limit min IET fragment size to 124.
> 
> Should be 128 not 124

User space setting is without FCS.

> 
>>
>> [1] AM62x TRM - https://www.ti.com/lit/ug/spruiv7a/spruiv7a.pdf
>> [2] AM62x Silicon Errata - https://www.ti.com/lit/er/sprz487c/sprz487c.pdf
>>
>> Signed-off-by: Roger Quadros <rogerq@...nel.org>
>> ---
>>  drivers/net/ethernet/ti/am65-cpsw-ethtool.c | 157 ++++++++++++++++++
>>  drivers/net/ethernet/ti/am65-cpsw-nuss.c    |   2 +
>>  drivers/net/ethernet/ti/am65-cpsw-nuss.h    |   5 +
>>  drivers/net/ethernet/ti/am65-cpsw-qos.c     | 175 ++++++++++++++++++++
>>  drivers/net/ethernet/ti/am65-cpsw-qos.h     | 102 ++++++++++++
>>  5 files changed, 441 insertions(+)
>>
>> Changelog:
>>
>> v7:
>> - use else if
>> - drop FIXME comment
>> - fix lldp kselftest failure by limiting max_verify_time to spec limit of 128ms.
>> - now passes all ethtool_mm.sh kselftests (patch 8 required)
>>
>> v6:
>> - get mutex around am65_cpsw_iet_commit_preemptible_tcs() in
>>   am65_cpsw_iet_change_preemptible_tcs()
>> - use "preemption" instead of "pre-emption"
>> - call am65_cpsw_setup_mqprio() from within am65_cpsw_setup_taprio()
>> - Now works with kselftest except the last test which fails
>>
>> v5:
>> - No change
>>
>> v4:
>> - Rebase and include in the same series as mqprio support.
>>
>> v3:
>> - Rebase on top of v6.6-rc1 and mqprio support [1]
>> - Support ethtool_ops :: get_mm_stats()
>> - drop unused variables cmn_ctrl and verify_cnt
>> - make am65_cpsw_iet_link_state_update() and
>>   am65_cpsw_iet_change_preemptible_tcs() static
>>
>> [1] https://lore.kernel.org/all/20230918075358.5878-1-rogerq@kernel.org/
>>
>> v2:
>> - Use proper control bits for PMAC enable
>> (AM65_CPSW_PN_CTL_IET_PORT_EN)
>>   and TX enable (AM65_CPSW_PN_IET_MAC_PENABLE)
>> - Common IET Enable (AM65_CPSW_CTL_IET_EN) is set if any port has
>>   AM65_CPSW_PN_CTL_IET_PORT_EN set.
>> - Fix workaround for erratum i2208. i.e. Limit rx_min_frag_size to 124
> 
> Should be 128 not 124
> 
>> - Fix am65_cpsw_iet_get_verify_timeout_ms() to default to timeout for
>>   1G link if link is inactive.
>> - resize the RX FIFO based on pmac_enabled, not tx_enabled.
>>
>>
>> diff --git a/drivers/net/ethernet/ti/am65-cpsw-ethtool.c
>> b/drivers/net/ethernet/ti/am65-cpsw-ethtool.c
>> index b9e1d568604b..5571385b4baf 100644
>> --- a/drivers/net/ethernet/ti/am65-cpsw-ethtool.c
>> +++ b/drivers/net/ethernet/ti/am65-cpsw-ethtool.c
>> @@ -11,6 +11,7 @@
>>  #include <linux/pm_runtime.h>
>>
>>  #include "am65-cpsw-nuss.h"
>> +#include "am65-cpsw-qos.h"
>>  #include "cpsw_ale.h"
>>  #include "am65-cpts.h"
>>
>> @@ -740,6 +741,159 @@ static int am65_cpsw_set_ethtool_priv_flags(struct
>> net_device *ndev, u32 flags)
>>  	return 0;
>>  }
>>
>> +static void am65_cpsw_port_iet_rx_enable(struct am65_cpsw_port *port,
>> +bool enable) {
>> +	u32 val;
>> +
>> +	val = readl(port->port_base + AM65_CPSW_PN_REG_CTL);
>> +	if (enable)
>> +		val |= AM65_CPSW_PN_CTL_IET_PORT_EN;
>> +	else
>> +		val &= ~AM65_CPSW_PN_CTL_IET_PORT_EN;
>> +
>> +	writel(val, port->port_base + AM65_CPSW_PN_REG_CTL);
>> +	am65_cpsw_iet_common_enable(port->common);
>> +}
>> +
>> +static void am65_cpsw_port_iet_tx_enable(struct am65_cpsw_port *port,
>> +bool enable) {
>> +	u32 val;
>> +
>> +	val = readl(port->port_base + AM65_CPSW_PN_REG_IET_CTRL);
>> +	if (enable)
>> +		val |= AM65_CPSW_PN_IET_MAC_PENABLE;
>> +	else
>> +		val &= ~AM65_CPSW_PN_IET_MAC_PENABLE;
>> +
>> +	writel(val, port->port_base + AM65_CPSW_PN_REG_IET_CTRL); }
>> +
>> +static int am65_cpsw_get_mm(struct net_device *ndev, struct
>> +ethtool_mm_state *state) {
>> +	struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
>> +	struct am65_cpsw_ndev_priv *priv = netdev_priv(ndev);
>> +	u32 port_ctrl, iet_ctrl, iet_status;
>> +	u32 add_frag_size;
>> +
>> +	mutex_lock(&priv->mm_lock);
>> +
>> +	iet_ctrl = readl(port->port_base + AM65_CPSW_PN_REG_IET_CTRL);
>> +	port_ctrl = readl(port->port_base + AM65_CPSW_PN_REG_CTL);
>> +
>> +	state->tx_enabled = !!(iet_ctrl &
>> AM65_CPSW_PN_IET_MAC_PENABLE);
>> +	state->pmac_enabled = !!(port_ctrl &
>> AM65_CPSW_PN_CTL_IET_PORT_EN);
>> +
>> +	iet_status = readl(port->port_base +
>> AM65_CPSW_PN_REG_IET_STATUS);
>> +
>> +	if (iet_ctrl & AM65_CPSW_PN_IET_MAC_DISABLEVERIFY)
>> +		state->verify_status =
>> ETHTOOL_MM_VERIFY_STATUS_DISABLED;
>> +	else if (iet_status & AM65_CPSW_PN_MAC_VERIFIED)
>> +		state->verify_status =
>> ETHTOOL_MM_VERIFY_STATUS_SUCCEEDED;
>> +	else if (iet_status & AM65_CPSW_PN_MAC_VERIFY_FAIL)
>> +		state->verify_status =
>> ETHTOOL_MM_VERIFY_STATUS_FAILED;
>> +	else
>> +		state->verify_status =
>> ETHTOOL_MM_VERIFY_STATUS_UNKNOWN;
>> +
>> +	add_frag_size =
>> AM65_CPSW_PN_IET_MAC_GET_ADDFRAGSIZE(iet_ctrl);
>> +	state->tx_min_frag_size =
>> +ethtool_mm_frag_size_add_to_min(add_frag_size);
>> +
>> +	/* Errata i2208: RX min fragment size cannot be less than 124 */
>> +	state->rx_min_frag_size = 124;
> 
> /* Errata i2208: RX min fragment size cannot be less than 128 */
> state->rx_min_frag_size = 128;

ethtool man page says
"           tx-min-frag-size
                  Shows the minimum size (in octets) of transmitted non-
                  final fragments which can be received by the link
                  partner. Corresponds to the standard addFragSize
                  variable using the formula:

                  tx-min-frag-size = 64 * (1 + addFragSize) - 4"

Which means user needs to put a -4 offset i.e. drop FCS size.

Drivers show rx-min-frag-size also without the FCS.

e.g.
https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/mscc/ocelot_mm.c#L260

-- 
cheers,
-roger

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ