All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Abdul Rahim, Faizal" <faizal.abdul.rahim@linux.intel.com>
To: Vladimir Oltean <vladimir.oltean@nxp.com>, Furong Xu <0x1207@gmail.com>
Cc: Tony Nguyen <anthony.l.nguyen@intel.com>,
	Przemek Kitszel <przemyslaw.kitszel@intel.com>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Maxime Coquelin <mcoquelin.stm32@gmail.com>,
	Alexandre Torgue <alexandre.torgue@foss.st.com>,
	Simon Horman <horms@kernel.org>,
	Russell King <linux@armlinux.org.uk>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Jesper Dangaard Brouer <hawk@kernel.org>,
	John Fastabend <john.fastabend@gmail.com>,
	Russell King <rmk+kernel@armlinux.org.uk>,
	Serge Semin <fancer.lancer@gmail.com>,
	Xiaolei Wang <xiaolei.wang@windriver.com>,
	Suraj Jaiswal <quic_jsuraj@quicinc.com>,
	Kory Maincent <kory.maincent@bootlin.com>,
	Gal Pressman <gal@nvidia.com>,
	Jesper Nilsson <jesper.nilsson@axis.com>,
	Andrew Halaney <ahalaney@redhat.com>,
	Choong Yong Liang <yong.liang.choong@linux.intel.com>,
	Kunihiko Hayashi <hayashi.kunihiko@socionext.com>,
	Vinicius Costa Gomes <vinicius.gomes@intel.com>,
	intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-stm32@st-md-mailman.stormreply.com,
	linux-arm-kernel@lists.infradead.org, bpf@vger.kernel.org
Subject: Re: [PATCH iwl-next v5 1/9] net: ethtool: mm: extract stmmac verification logic into common library
Date: Fri, 21 Feb 2025 21:30:09 +0800	[thread overview]
Message-ID: <3fbe3955-48b8-449d-93ff-2699a7efcd8d@linux.intel.com> (raw)
In-Reply-To: <20250221104333.6s7nvn2wwco3axr3@skbuf>



On 21/2/2025 6:43 pm, Vladimir Oltean wrote:
> On Fri, Feb 21, 2025 at 06:24:09PM +0800, Furong Xu wrote:
>> Your fix is better when link is up/down, so I vote verify_enabled.
> 
> Hmmm... I thought this was a bug in stmmac that was carried over to
> ethtool_mmsv, but it looks like it isn't.
> 
> In fact, looking at the original refactoring patch I had attached in
> this email:
> https://lore.kernel.org/netdev/20241217002254.lyakuia32jbnva46@skbuf/
> 
> these 2 lines in ethtool_mmsv_link_state_handle() didn't exist at all.
> 
> 	} else {
>>>>> 		mmsv->status = ETHTOOL_MM_VERIFY_STATUS_INITIAL;
>>>>> 		mmsv->verify_retries = ETHTOOL_MM_MAX_VERIFY_RETRIES;
> 
> 		/* No link or pMAC not enabled */
> 		ethtool_mmsv_configure_pmac(mmsv, false);
> 		ethtool_mmsv_configure_tx(mmsv, false);
> 	}
> 
> Faizal, could you remind me why they were added? I don't see this
> explained in change logs.
> 

Hi Vladimir,

Yeah, it wasn’t there originally. I added that change because it failed the 
link down/link up test.
After a successful verification, if the link partner goes down, the status 
still shows ETHTOOL_MM_VERIFY_STATUS_SUCCEEDED, which isn’t correct—so 
that’s why I added it.

Sorry for not mentioning it earlier. I assumed you’d check the delta 
between the original patch and the upstream one, my bad, should have 
mentioned this logic change.

Should I update it to the latest suggestion?




WARNING: multiple messages have this Message-ID (diff)
From: "Abdul Rahim, Faizal" <faizal.abdul.rahim@linux.intel.com>
To: Vladimir Oltean <vladimir.oltean@nxp.com>, Furong Xu <0x1207@gmail.com>
Cc: Tony Nguyen <anthony.l.nguyen@intel.com>,
	Przemek Kitszel <przemyslaw.kitszel@intel.com>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Maxime Coquelin <mcoquelin.stm32@gmail.com>,
	Alexandre Torgue <alexandre.torgue@foss.st.com>,
	Simon Horman <horms@kernel.org>,
	Russell King <linux@armlinux.org.uk>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Jesper Dangaard Brouer <hawk@kernel.org>,
	John Fastabend <john.fastabend@gmail.com>,
	Russell King <rmk+kernel@armlinux.org.uk>,
	Serge Semin <fancer.lancer@gmail.com>,
	Xiaolei Wang <xiaolei.wang@windriver.com>,
	Suraj Jaiswal <quic_jsuraj@quicinc.com>,
	Kory Maincent <kory.maincent@bootlin.com>,
	Gal Pressman <gal@nvidia.com>,
	Jesper Nilsson <jesper.nilsson@axis.com>,
	Andrew Halaney <ahalaney@redhat.com>,
	Choong Yong Liang <yong.liang.choong@linux.intel.com>,
	Kunihiko Hayashi <hayashi.kunihiko@socionext.com>,
	Vinicius Costa Gomes <vinicius.gomes@intel.com>,
	intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-stm32@st-md-mailman.stormreply.com,
	linux-arm-kernel@lists.infradead.org, bpf@vger.kernel.org
Subject: Re: [Intel-wired-lan] [PATCH iwl-next v5 1/9] net: ethtool: mm: extract stmmac verification logic into common library
Date: Fri, 21 Feb 2025 21:30:09 +0800	[thread overview]
Message-ID: <3fbe3955-48b8-449d-93ff-2699a7efcd8d@linux.intel.com> (raw)
In-Reply-To: <20250221104333.6s7nvn2wwco3axr3@skbuf>



On 21/2/2025 6:43 pm, Vladimir Oltean wrote:
> On Fri, Feb 21, 2025 at 06:24:09PM +0800, Furong Xu wrote:
>> Your fix is better when link is up/down, so I vote verify_enabled.
> 
> Hmmm... I thought this was a bug in stmmac that was carried over to
> ethtool_mmsv, but it looks like it isn't.
> 
> In fact, looking at the original refactoring patch I had attached in
> this email:
> https://lore.kernel.org/netdev/20241217002254.lyakuia32jbnva46@skbuf/
> 
> these 2 lines in ethtool_mmsv_link_state_handle() didn't exist at all.
> 
> 	} else {
>>>>> 		mmsv->status = ETHTOOL_MM_VERIFY_STATUS_INITIAL;
>>>>> 		mmsv->verify_retries = ETHTOOL_MM_MAX_VERIFY_RETRIES;
> 
> 		/* No link or pMAC not enabled */
> 		ethtool_mmsv_configure_pmac(mmsv, false);
> 		ethtool_mmsv_configure_tx(mmsv, false);
> 	}
> 
> Faizal, could you remind me why they were added? I don't see this
> explained in change logs.
> 

Hi Vladimir,

Yeah, it wasn’t there originally. I added that change because it failed the 
link down/link up test.
After a successful verification, if the link partner goes down, the status 
still shows ETHTOOL_MM_VERIFY_STATUS_SUCCEEDED, which isn’t correct—so 
that’s why I added it.

Sorry for not mentioning it earlier. I assumed you’d check the delta 
between the original patch and the upstream one, my bad, should have 
mentioned this logic change.

Should I update it to the latest suggestion?




  reply	other threads:[~2025-02-21 13:30 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-20  2:53 [PATCH iwl-next v5 0/9] igc: Add support for Frame Preemption feature in IGC Faizal Rahim
2025-02-20  2:53 ` [Intel-wired-lan] " Faizal Rahim
2025-02-20  2:53 ` [PATCH iwl-next v5 1/9] net: ethtool: mm: extract stmmac verification logic into common library Faizal Rahim
2025-02-20  2:53   ` [Intel-wired-lan] " Faizal Rahim
2025-02-20 11:38   ` Vladimir Oltean
2025-02-20 11:38     ` [Intel-wired-lan] " Vladimir Oltean
2025-02-21  9:42   ` Furong Xu
2025-02-21  9:42     ` [Intel-wired-lan] " Furong Xu
2025-02-21  9:56     ` Vladimir Oltean
2025-02-21  9:56       ` [Intel-wired-lan] " Vladimir Oltean
2025-02-21 10:24       ` Furong Xu
2025-02-21 10:24         ` [Intel-wired-lan] " Furong Xu
2025-02-21 10:43         ` Vladimir Oltean
2025-02-21 10:43           ` [Intel-wired-lan] " Vladimir Oltean
2025-02-21 13:30           ` Abdul Rahim, Faizal [this message]
2025-02-21 13:30             ` Abdul Rahim, Faizal
2025-02-21 14:44             ` Vladimir Oltean
2025-02-21 14:44               ` [Intel-wired-lan] " Vladimir Oltean
2025-02-22  0:26               ` Abdul Rahim, Faizal
2025-02-22  0:26                 ` [Intel-wired-lan] " Abdul Rahim, Faizal
2025-02-23  5:39   ` Furong Xu
2025-02-23  5:39     ` [Intel-wired-lan] " Furong Xu
2025-02-20  2:53 ` [PATCH iwl-next v5 2/9] igc: Rename xdp_get_tx_ring() for non-xdp usage Faizal Rahim
2025-02-20  2:53   ` [Intel-wired-lan] " Faizal Rahim
2025-02-20  2:53 ` [PATCH iwl-next v5 3/9] igc: Optimize the TX packet buffer utilization Faizal Rahim
2025-02-20  2:53   ` [Intel-wired-lan] " Faizal Rahim
2025-02-20  2:53 ` [PATCH iwl-next v5 4/9] igc: Set the RX packet buffer size for TSN mode Faizal Rahim
2025-02-20  2:53   ` [Intel-wired-lan] " Faizal Rahim
2025-02-20  2:53 ` [PATCH iwl-next v5 5/9] igc: Add support for frame preemption verification Faizal Rahim
2025-02-20  2:53   ` [Intel-wired-lan] " Faizal Rahim
2025-02-20 11:18   ` Vladimir Oltean
2025-02-20 11:18     ` [Intel-wired-lan] " Vladimir Oltean
2025-02-20  2:53 ` [PATCH iwl-next v5 6/9] igc: Add support to set tx-min-frag-size Faizal Rahim
2025-02-20  2:53   ` [Intel-wired-lan] " Faizal Rahim
2025-02-20  2:53 ` [PATCH iwl-next v5 7/9] igc: Add support to get MAC Merge data via ethtool Faizal Rahim
2025-02-20  2:53   ` [Intel-wired-lan] " Faizal Rahim
2025-02-20  2:53 ` [PATCH iwl-next v5 8/9] igc: Add support to get frame preemption statistics " Faizal Rahim
2025-02-20  2:53   ` [Intel-wired-lan] " Faizal Rahim
2025-02-20  2:53 ` [PATCH iwl-next v5 9/9] igc: Block setting preemptible traffic class in taprio Faizal Rahim
2025-02-20  2:53   ` [Intel-wired-lan] " Faizal Rahim
2025-02-20 11:28   ` Vladimir Oltean
2025-02-20 11:28     ` [Intel-wired-lan] " Vladimir Oltean
2025-02-20 11:46 ` [PATCH iwl-next v5 0/9] igc: Add support for Frame Preemption feature in IGC Vladimir Oltean
2025-02-20 11:46   ` [Intel-wired-lan] " Vladimir Oltean

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3fbe3955-48b8-449d-93ff-2699a7efcd8d@linux.intel.com \
    --to=faizal.abdul.rahim@linux.intel.com \
    --cc=0x1207@gmail.com \
    --cc=ahalaney@redhat.com \
    --cc=alexandre.torgue@foss.st.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=anthony.l.nguyen@intel.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=fancer.lancer@gmail.com \
    --cc=gal@nvidia.com \
    --cc=hawk@kernel.org \
    --cc=hayashi.kunihiko@socionext.com \
    --cc=horms@kernel.org \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=jesper.nilsson@axis.com \
    --cc=john.fastabend@gmail.com \
    --cc=kory.maincent@bootlin.com \
    --cc=kuba@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-stm32@st-md-mailman.stormreply.com \
    --cc=linux@armlinux.org.uk \
    --cc=mcoquelin.stm32@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=przemyslaw.kitszel@intel.com \
    --cc=quic_jsuraj@quicinc.com \
    --cc=rmk+kernel@armlinux.org.uk \
    --cc=vinicius.gomes@intel.com \
    --cc=vladimir.oltean@nxp.com \
    --cc=xiaolei.wang@windriver.com \
    --cc=yong.liang.choong@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.