Date:   Wed, 16 Nov 2022 14:36:21 +0200
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     Xiaoliang Yang <xiaoliang.yang_1@....com>
Cc:     Vinicius Costa Gomes <vinicius.gomes@...el.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Michal Kubecek <mkubecek@...e.cz>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Kurt Kanzenbach <kurt@...utronix.de>,
        Rui Sousa <rui.sousa@....com>,
        Ferenc Fejes <ferenc.fejes@...csson.com>,
        Richie Pearn <richard.pearn@....com>
Subject: Re: 回复: 回复: [RFC PATCH net-next 2/7] net: ethtool: add support for Frame
 Preemption and MAC Merge layer

Hi Xiaoliang,

On Tue, Nov 15, 2022 at 05:01:24AM +0200, Xiaoliang Yang wrote:
> Yes, I know that "Enable" and "activate" are different things. I mean that if we
> want to enable the next lldp exchange verification process, we need to enable
> it but not let the MAC merge layer working until lldp verify successfully.
> There are two verification procedure, one is LLDP verification, based on chapter
> "99.4.2 Determining that the link partner supports preemption" of 802.3 spec.
> This need lldp exchange and ensure preemption is enabled on remote port, and
> this verification doesn’t have parameter to enable/disable it. I just want to add
> this feature.

Why? The mechanism to enable/disable the MAC Merge layer is the ETHTOOL_A_MM_ENABLED
netlink attribute which is settable.

To the kernel, it doesn't matter if ethtool (the program) or openlldp
sets ETHTOOL_A_MM_ENABLED.

> Another is SMD-r/v verification, this is hardware verification and
> already has "verify_disable" to enable/disable it.
> 
> Or we can enable by default through LLDP. That don’t need a new "active"
> parameter. Once the LLDP application has verified that remote port supports
> preemption, it will enable preemption on local port, and tell remote port to
> enable it.

IIUC, it cannot "tell" the link partner to enable preemption. It can
just advertise the following in a subsequent TLV:

Table 79–8—Additional Ethernet capabilities

Bit 0 (preemption capability support) - supported/not supported
Bit 1 (preemption capability status) - enabled/not enabled
Bit 2 (preemption capability active) - active/not active
Bits 4:3 (additional fragment size) - A 2-bit integer value indicating,
in units of 64 octets, the minimum number of octets over 64 octets
required in non-final fragments by the receiver

The only thing we can do, is if the link partner sets Bit 0 to true, to
generate a ETHTOOL_MSG_MM_SET netlink message with ETHTOOL_A_MM_ENABLED
set to true.

We _cannot_ wait for the link partner to set Bit 1 to true, because, if
it's the same implementation as us, nothing will work, we will both wait
for each other...

So either the link partner enables preemption too, as a result of us
advertising a TLV with Bit 0 set, and things eventually work, or the
link partner doesn't enable preemption even though both ends support it.
In that case, preemption doesn't work (and this is also visible in the
TLVs sent by the link partner - Bit 1 and Bit 2 remain zero).

If the SMD-V/SMD-R handshake is enabled locally (ETHTOOL_A_MM_VERIFY_DISABLE
is false), we can also detect that by the fact that the link partner
will not respond with a SMD-R to our SMD-V. So, even if we will set Bit 1
(corresponds to ETHTOOL_A_MM_ENABLED in the ETHTOOL_MSG_MM_GET message)
to true, Bit 2 (corresponding to ETHTOOL_A_MM_ACTIVE in the GET message)
will still remain unset. Otherwise, the condition will remain undetected,
and we will report Bit 2 (active) as true (which it is - verification is
disabled after all).

> If we use ethtool to enable preemption, the preemption will be force
> enabled without LLDP verification.

Yup. I consider that within the expected parameters of the game, no?

> > In turn, LLDP does not control whether verification is used or not. So, from the
> > perspective of the LLDP daemon, you can't "enable" but "not activate"
> > preemption.
> Yes, LLDP does not control whether hardware verification is used or not. Maybe I
> shouldn't use the word "active", I mean preemption can't be working before LLDP
> verification, even though it's enabled by ethtool.

I don't understand this. Why can't preemption work without LLDP
verification? My understanding is that LLDP is just an automated way of
maximizing the enabled feature set depending on what the link partner
advertises as supported. But the same feature set can be force-enabled
manually, if there is prior knowledge that the link partner can do it.
I think of it as a software autoneg, I might be wrong, I don't have too
much XP with LLDP. That's exactly what ethtool (the program, not the
kernel interface) is for - to be able to force settings.

> > And I wouldn't expect the LLDP flow to require use of ethtool.
> If LLDP application can’t require to use ethtool driver, how to enable the preemption
> after LLDP verification?

No, I mean the LLDP flow doesn't require use of ethtool the program, not
ethtool the kernel netlink interface. Of course it requires the latter.

> > And I think that enabling preemption at step 1 is too early. Why enable it, if we
> > don't know if the link partner supports it?
> Same as mentioned above. Enabling at step 1 does not actually enable preemption,
> it means enable the LLDP verification.

Nope. LLDP verification is no more than a user space program that
automates something that can also be done manually, using ethtool the
program. No special netlink messages for LLDP.

> > So those who enable verification should get a response from those who don't
> > enable it, and those who don't enable verification don't cause the link partners
> > who do enable it to fail their own verification.
> The hardware may not obey this rule. I have tested on LS1028a and i.mx8mp boards,
> The verification needs to be enabled at the same time for link partners. So it needs
> LLDP verification, and let LLDP application to enable both partner ports at the same
> time(The hardware verify three times, and each verify time is 10ms in default).

I think the imx8mp has the same EQOS block as the S32G GMAC, right?
If so, I might try experimenting a bit on the S32G linked to LS1028A,
to see exactly what you're talking about.

Keep in mind that hardware quirks don't mean much to what the user space
flow should be. The kernel should deal with quirks. For example, on enetc,
the MAC merge layer must be manually disabled and re-enabled on each
link up/down event by the kernel driver. Otherwise the verification
state machine remains FAILED, as you seem to say. But that changes
exactly nothing in terms of the LLDP flow - because as you say, the
hardware SMD-R/SMD-V verification state machine which you talk about is
independent of LLDP.

> > The only thing to agree on is this: if the link partner supports preemption, and
> > you support preemption, then how does the LLDP daemon decide whether to
> > actually *enable* preemption? My assumption is that it's fine to
> > unconditionally enable it as long as it's supported.
> > No configuration or user override is needed.
> Yes, I also consider this way. Enable the preemption once LLDP exchange ensure
> that both partner support it. But how to enable preemption via LLDP daemon?
> I tried to use the ioctl API of ethtool to enable preemption. Do you have any
> suggestion?

Yeah, see above. The ethtool ioctl is dead and it's not coming back, see ETHTOOL_A_MM_ENABLED.
I wish I could come back to this patch set sooner, but I'm really deadlocked
with some other stuff I need to do this week. Sorry.

Powered by blists - more mailing lists