Re: [EXT] Re: [net-next PATCH 1/2] octeontx2-pf: Add devlink param to init and de-init serdes

Hi,

On Mon, Nov 15, 2021 at 8:41 PM Jakub Kicinski [off-list ref] wrote:

On Sun, 14 Nov 2021 20:19:59 -0800 Roopa Prabhu wrote:

quoted

On 11/14/21 12:38 AM, Ido Schimmel wrote:

quoted

On Thu, Nov 11, 2021 at 08:47:19AM -0800, Jakub Kicinski wrote:

quoted

Hm. How do we come up with the appropriate wording here...

I meant keeping the "PHY level link" up? I think we agree that all the
cases should behave like SFP power behaves today?

The API is to control or query what is forcing the PHY link to stay up
after the netdev was set down. IOW why does the switch still see link
up if the link is down on Linux.

The SFP power policy doesn't affect that. In our systems (and I believe
many others), by default, the transceivers are transitioned to high
power mode upon plug-in, but the link is still down when the netdev is
down because the MAC/PHY are not operational.

Ah, GTK!

quoted

quoted

With SRIOV/Multi-Host, the MAC/PHY are always operational which is why
your link partner has a carrier even when the netdev is down.

I see, I think you're talking about something like IFLA_VF_LINK_STATE_*
but for the PF. That could make sense, although I don't think it was
ever requested.

quoted

quoted

quoted

I don't think we should report carrier up when netdev is down?

This is what happens today, but it's misleading because the carrier is
always up with these systems. When I take the netdev down, I expect my
link partner to lose carrier. If this doesn't happen, then I believe the
netdev should always report IFF_UP. Alternatively, to avoid user space
breakage, this can be reported via a new attribute such as "protoup".

Sounds sensible.

quoted

quoted

quoted

"proto" in "protodown" refers to STP, right?

Not really. I believe the main use case was vrrp / mlag.

VRRP is a proto, mlag maybe a little less clear-cut.

quoted

quoted

The "protdown_reason" is just a bitmap of user enumerated reasons to keep
the interface down. See commit 829eb208e80d ("rtnetlink: add support for
protodown reason") for details.

correct. Its equivalent to errDisable found on most commercial switch OS'es.

Can be used for any control-plane/mgmt-plane/protocol wanting to hold
the link down.

Other use-cases where this can be used (as also quoted by other vendors):

mismatch of link properties

What link properties?

quoted

Link Flapping detection and disable link
Port Security Violation

Port security as established by a .. protocol like 802.1X ?

quoted

Broadcast Storms
etc

Why not take the entire interface down for bcast storm?

quoted

quoted

quoted

Not sure what "proto" in "protoup" would be.

sriov/multi-host/etc ?

agree. Would be nice to re-use protodown ndo and state/reason here

You are the experts so correct me please but the point of protodown
is that the the link is held down for general traffic but you can
still exchange protocol messages on it. STP, VRRP, LAG, 802.1X etc.

For anything that does not require special message exchange the link
can be just brought down completely.

In my head link held up is a completely different beast, the local host
does not participate or otherwise pay attention to any communication on
the link. It's about what other entities do with the link.

But if you prefer "protoup" strongly that's fine, I guess.

As said by Ido, ndo_change_proto_down with proto_down as
on and off is sufficient for our requirement right now. We will use
ndo_change_proto_down
instead of devlink. Thanks everyone for pitching in.

Sundeep