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;
Message-ID: <52b8b6e5-3ef0-6143-1373-e41caef19234@intel.com>
Date:   Wed, 12 Apr 2023 15:38:54 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.1
Subject: Re: [PATCH net] net: enetc: workaround for unresponsive pMAC after
 receiving express traffic
Content-Language: en-US
To:     Vladimir Oltean <vladimir.oltean@nxp.com>, <netdev@vger.kernel.org>
CC:     "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Claudiu Manoil <claudiu.manoil@nxp.com>,
        <linux-kernel@vger.kernel.org>
References: <20230411192645.1896048-1-vladimir.oltean@nxp.com>
From:   Jacob Keller <jacob.e.keller@intel.com>
In-Reply-To: <20230411192645.1896048-1-vladimir.oltean@nxp.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?TGRackMvdVhsN3gvTU1sOFZKQVhHL1luTVYyeGdHYkR2ZHVyeHRhTE9xaEcr?=
 =?utf-8?B?ZUllQkpiUFhManVibFYxYWk4WjdzOFZnOHVCMkFMMFVGZ0JmUU9pcUhkWDBB?=
 =?utf-8?B?a29KR05hQmtQb3hYUHEvaTlYcWw5bVlMSmdrVHRXaVdNUWc4cnF1Y2JkbmNv?=
 =?utf-8?B?aGRHK29ldGowcWYvUXVSUTByazdYZHpJSjNMQ2o5dHFLbGZoS1FEZlozTFds?=
 =?utf-8?B?NFN1K3VLSktrcm5JdjhaYnVPakNib29nUTVDa3ZPR1k3cG9YdWVqMkxyWFlx?=
 =?utf-8?B?bU9mYTMxMzhnVUltNDMybHZXVkZQWkNSMEVVaG1LU3BKWFR0OUhTanhRRk1M?=
 =?utf-8?B?RkNLUjd5WG1QNVBRVFZnbVBHQXJXU3ZRYlpiVXdaZ3kvZWpqdWhqMFlJL0g1?=
 =?utf-8?B?dW9sVVRadEIweld3YkhsSGhXUzF5ekFpWkNNUk5Vd0YyTS9xanNCeXR6YzVp?=
 =?utf-8?B?TVdaQTJzMTVuZ0hEd00rUStzS3p6ZkkycEdXOW9lYVNmN21YQW9DQnJnYkVR?=
 =?utf-8?B?SUxUK2dVaUwrMVpaY3ViSGRLVG56S3IvUS9SUGZuYVBrU0RLcmZMMnFhNzVa?=
 =?utf-8?B?Sjc1ZmxJUFRMbWVia2tHYjE3WFNWZnZtdVVPVjZkM09OT2xtWG1UMEdTS291?=
 =?utf-8?B?NDh0Q05PU2M1bHlMcnp2MXJyVy9XcWJ5THN1a3p4elUrekJTbEh5bVZLeFNE?=
 =?utf-8?B?c2p1ZnNWc0VtUnJ4UkhyblJnL05vcmtEbTlCMVpqay85Y2NqTU5scWVLTGM0?=
 =?utf-8?B?R01PemRMRVRsb3c5NitRdjdiQzlKU01IRnF4Mlp6eEprUmpyNHpIZXg4dVlj?=
 =?utf-8?B?RmdnSTk1MDU4azhzblNyOStDTVRiT0hZM0FmV3ZvRW4wd0Zza3h3M0pEZS8w?=
 =?utf-8?B?SGJzNkNHcDNBVkpvTklMQ1E3L1dLSG03VXl0OEFCMTFEa2dYaTNTRS95bThw?=
 =?utf-8?B?VEIxZlo2VzRQUzNZOXRaOCtKSkZraERkbGFuNThOdkVhdjRkb2JRdDlDUmE2?=
 =?utf-8?B?RHRsU3hLdklEQnE3Vm94YjdsSkFSTUFKa0ExRmRxMEszTTdDWklOVXh3dVIw?=
 =?utf-8?B?VDZIVVpxSXFFYkZOdWRYeWdma0ZJQUlNdWlKOUoxR0xMd2ZCS1lKRStaWlBM?=
 =?utf-8?B?SzVDTjBBUjExZW5vZWdTTlI5a0hzc2c1MGc2SG1BeHpoZUdwMzJ6SW1vNkRL?=
 =?utf-8?B?SEtTZ3hDWjgrcE5EaE1nTmxFSmVZV055dll5OFBramp5Ullxc3hZb1lvQlk2?=
 =?utf-8?B?Zmx1VUhGTmdDcURYd2xtWFZPbk9ScXEzS0NZM3NjVThzZG41S2phVmJTMnNO?=
 =?utf-8?B?SWVWZmFEV1ZOYk5pVmllckJwNzNiNTFLZ3JIV2VmWlg2akZuR2ZJa2t3c2lt?=
 =?utf-8?B?RlRoMmdkUjYzTTFZYzgxeFhmOG5oSHFCL1JHVXU2cUNCY2E0MHMwekNDQS85?=
 =?utf-8?B?aDhqUHZCSFFJWE9VZW5WallNWVlVaHZYRE1qSU03ZGlZLzVyY21keDkyWHE0?=
 =?utf-8?B?YnhoakJPQXBjMUpnR09nR2tFRHUxeUlSWnFndzNOL05hbWp6OTJNallUSEdJ?=
 =?utf-8?B?ZEU0eEhXNFZ2SGIzQjh3cmhDcFB3VjhucmZNUzA4RURqdjFBN1l5S2Fna2dD?=
 =?utf-8?B?djd2bEwwZjFsNFRWTExhU0YzOEt0RFVBbEF5Z0Iwbm5vMkNCRzdSUU13dHdj?=
 =?utf-8?B?OVRhWWFKZnNDYmJqYUtUeEcrOEt3NVhwQk1ueVhqaytFSGxQbEc1YlYzekRU?=
 =?utf-8?B?azhQaTRjb1FTMVNRc1dCVU1vZWZqbmRBVzJoMlhmVzhrMDkvSllSZzBjbjZR?=
 =?utf-8?B?eVovSk94L0VHWTR5dGY5dGlFRlJ5bGJ0UVhkL0FPVFlKVkhKT0tEM0szMUdw?=
 =?utf-8?B?VlZ5Qk5ySTRWalAyVS9ieFQ0QkgzTmhPMmU1RCtGbWhIenlQY21Bc0MxMTc4?=
 =?utf-8?B?RnowTHRSQjh2azRDZ003MGJZSVhkQ1JyQWJGMmI3S2NDQ0VBcFVCMnorQTEr?=
 =?utf-8?B?L2owT0g0cmxxZldwckxRYTl2b2FRQkdnVm5nWVRtV2ZGZXh4VFJxOFNxMnQy?=
 =?utf-8?B?djQya1REZEtVK0d3Z1pTSHpMeURleVBEWUVuVVNjOXo4YlRxRWd2bmRJYXBF?=
 =?utf-8?B?NjlXaFpCckt5Nk0vQzNrcHJSSlYySGxhS0t2R25ITDlyQ3RTcWxuMy9rTFpB?=
 =?utf-8?B?UGc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 897a677b-8daf-42a0-11b7-08db3ba6aee1
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB5089.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Apr 2023 22:38:48.9682
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: XAKYUWtrYwyBQqRwqrhoLdweQ6vnEQ31gmweW4QYjdS6Zr1BfHQUcr/W77C3JsprmpmoikTDN2RGnpAnAhs+EUvBHvVjKjpP2ie0y6o+eZk=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR11MB7251
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-5.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,
        SPF_NONE,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 4/11/2023 12:26 PM, Vladimir Oltean wrote:
> I have observed an issue where the RX direction of the LS1028A ENETC pMAC
> seems unresponsive. The minimal procedure to reproduce the issue is:
> 
> 1. Connect ENETC port 0 with a loopback RJ45 cable to one of the Felix
>    switch ports (0).
> 
> 2. Bring the ports up (MAC Merge layer is not enabled on either end).
> 
> 3. Send a large quantity of unidirectional (express) traffic from Felix
>    to ENETC. I tried altering frame size and frame count, and it doesn't
>    appear to be specific to either of them, but rather, to the quantity
>    of octets received. Lowering the frame count, the minimum quantity of
>    packets to reproduce relatively consistently seems to be around 37000
>    frames at 1514 octets (w/o FCS) each.
> 
> 4. Using ethtool --set-mm, enable the pMAC in the Felix and in the ENETC
>    ports, in both RX and TX directions, and with verification on both
>    ends.
> 
> 5. Wait for verification to complete on both sides.
> 
> 6. Configure a traffic class as preemptible on both ends.
> 
> 7. Send some packets again.
> 
> The issue is at step 5, where the verification process of ENETC ends
> (meaning that Felix responds with an SMD-R and ENETC sees the response),
> but the verification process of Felix never ends (it remains VERIFYING).
> 
> If step 3 is skipped or if ENETC receives less traffic than
> approximately that threshold, the test runs all the way through
> (verification succeeds on both ends, preemptible traffic passes fine).
> 
> If, between step 4 and 5, the step below is also introduced:
> 
> 4.1. Disable and re-enable PM0_COMMAND_CONFIG bit RX_EN
> 
> then again, the sequence of steps runs all the way through, and
> verification succeeds, even if there was the previous RX traffic
> injected into ENETC.
> 
> Traffic sent *by* the ENETC port prior to enabling the MAC Merge layer
> does not seem to influence the verification result, only received
> traffic does.
> 
> The LS1028A manual does not mention any relationship between
> PM0_COMMAND_CONFIG and MMCSR, and the hardware people don't seem to
> know for now either.
> 
> The bit that is toggled to work around the issue is also toggled
> by enetc_mac_enable(), called from phylink's mac_link_down() and
> mac_link_up() methods - which is how the workaround was found:
> verification would work after a link down/up.
> 

Frustrating that we don't know why this is required, but your outline
here is convincing enough. Thanks for a thorough explanation.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

> Fixes: c7b9e8086902 ("net: enetc: add support for MAC Merge layer")
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  .../net/ethernet/freescale/enetc/enetc_ethtool.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c b/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
> index da9d4b310fcd..838750a03cf6 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
> @@ -989,6 +989,20 @@ static int enetc_get_mm(struct net_device *ndev, struct ethtool_mm_state *state)
>  	return 0;
>  }
>  
> +/* FIXME: Workaround for the link partner's verification failing if ENETC
> + * priorly received too much express traffic. The documentation doesn't
> + * suggest this is needed.
> + */
> +static void enetc_restart_emac_rx(struct enetc_si *si)
> +{
> +	u32 val = enetc_port_rd(&si->hw, ENETC_PM0_CMD_CFG);
> +
> +	enetc_port_wr(&si->hw, ENETC_PM0_CMD_CFG, val & ~ENETC_PM0_RX_EN);
> +
> +	if (val & ENETC_PM0_RX_EN)
> +		enetc_port_wr(&si->hw, ENETC_PM0_CMD_CFG, val);
> +}
> +
>  static int enetc_set_mm(struct net_device *ndev, struct ethtool_mm_cfg *cfg,
>  			struct netlink_ext_ack *extack)
>  {
> @@ -1040,6 +1054,8 @@ static int enetc_set_mm(struct net_device *ndev, struct ethtool_mm_cfg *cfg,
>  
>  	enetc_port_wr(hw, ENETC_MMCSR, val);
>  
> +	enetc_restart_emac_rx(priv->si);
> +
>  	mutex_unlock(&priv->mm_lock);
>  
>  	return 0;