Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Paul Fertser <fercerpav@gmail.com>
To: Ivan Mikhaylov <i.mikhaylov@yadro.com>
Cc: "David S . Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Samuel Mendoza-Jonas <sam@mendozajonas.com>,
	Joel Stanley <joel@jms.id.au>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	openbmc@lists.ozlabs.org
Subject: Re: [PATCH v2 0/3] net/ncsi: Add NCSI Intel OEM command to keep PHY link up
Date: Tue, 20 Jul 2021 17:16:11 +0300	[thread overview]
Message-ID: <20210720141611.GI875@home.paul.comp> (raw)
In-Reply-To: <10902992a9dfb5b1b4f1d7a9e17ff0e7b121b50b.camel@yadro.com>

On Tue, Jul 20, 2021 at 05:00:40PM +0300, Ivan Mikhaylov wrote:
> > While the host is booted up and fully functional it assumes it has
> > full proper control of network cards, and sometimes it really needs to
> > reset them to e.g. recover from crashed firmware. The PHY resets might
> > also make sense in certain cases, and so in general having this "link
> > up" bit set all the time might be breaking assumptions.
> 
> Paul, what kind of assumption it would break?

The host OS drivers assume they can fully control PCIe network
cards. Doing anything (including inhibiting PHY resets) behind its
back might break assumptions the driver authors had. This bit in
question certainly makes the card behave in an unusual way, so no
wonder Intel didn't enable it by default.

I do not claim I know for a fact it's problematic but it doesn't feel
like "the right thing" so some edge cases might expose issues.

> Joel proposed it as DTS option which may help at runtime.

Sorry, I'm not following. If BMC is fully booted it's able to
configure NC-SI appropriately by a userspace action coordinated with
other BMC tasks. If BMC is not yet ready then we can't communicate
with it via Ethernet anyway. So I can't see when exactly is it going
to be helpful.

> Some of those commands should be applied after channel probe as I
> think including phy reset control.

Do you have any other commands in mind? So far I assumed we're
discussing just the one to mask PHY resets.

> > Ivan, so far I have an impression that the user-space solution would
> > be much easier, flexible and manageable and that there's no need for
> > this command to be in Linux at all.
> 
> You may not have such things on your image with suitable env which you can rely
> on. There is smaf for mellanox which is done in the same way for example.

I can hardly imagine why an OS running on BMC would be using this code
in question and appropriate DT configuration but not having the right
means in userspace to control it. What would be the usecase?

If the network subsystem maintainers think this is a good idea, all
things considered, I'm fine with it. I210 losing link exactly at the
time when you need it (to enter the UEFI interactive menu) is
super-annoying, so probably any fix is better than none :)

Thank you for discussion.

-- 
Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software!
mailto:fercerpav@gmail.com

      reply	other threads:[~2021-07-20 14:22 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-08 12:27 Ivan Mikhaylov
2021-07-08 12:27 ` [PATCH v2 1/3] net/ncsi: fix restricted cast warning of sparse Ivan Mikhaylov
2021-07-12  9:09   ` Joel Stanley
2021-07-13  9:08     ` Ivan Mikhaylov
2021-07-08 12:27 ` [PATCH v2 2/3] net/ncsi: add NCSI Intel OEM command to keep PHY up Ivan Mikhaylov
2021-07-12 10:01   ` Joel Stanley
2021-07-12 19:32     ` Eddie James
2021-07-13 10:16     ` Ivan Mikhaylov
2021-07-08 12:27 ` [PATCH v2 3/3] net/ncsi: add dummy response handler for Intel boards Ivan Mikhaylov
2021-07-12 10:03   ` Joel Stanley
2021-07-13  9:42     ` Ivan Mikhaylov
2021-07-20  9:41   ` Paul Fertser
2021-07-20 13:21     ` Ivan Mikhaylov
2021-07-20 13:29       ` Paul Fertser
2021-07-08 21:20 ` [PATCH v2 0/3] net/ncsi: Add NCSI Intel OEM command to keep PHY link up patchwork-bot+netdevbpf
2021-07-20  9:53 ` Paul Fertser
2021-07-20 14:00   ` Ivan Mikhaylov
2021-07-20 14:16     ` Paul Fertser [this message]

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=20210720141611.GI875@home.paul.comp \
    --to=fercerpav@gmail.com \
    --cc=benh@kernel.crashing.org \
    --cc=davem@davemloft.net \
    --cc=i.mikhaylov@yadro.com \
    --cc=joel@jms.id.au \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=openbmc@lists.ozlabs.org \
    --cc=sam@mendozajonas.com \
    --subject='Re: [PATCH v2 0/3] net/ncsi: Add NCSI Intel OEM command to keep PHY link up' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).