From: Samuel Mendoza-Jonas <sam@mendozajonas.com>
To: Justin.Lee1@Dell.com, netdev@vger.kernel.org
Cc: davem@davemloft.net, linux-kernel@vger.kernel.org,
openbmc@lists.ozlabs.org
Subject: Re: [PATCH net-next v2 6/6] net/ncsi: Configure multi-package, multi-channel modes with failover
Date: Tue, 30 Oct 2018 14:05:48 +1100 [thread overview]
Message-ID: <c64d1e440e2469ad0125b6c27c3bc4c59723bdb9.camel@mendozajonas.com> (raw)
In-Reply-To: <93324b3c0a9b4929aa64801b4ed9c25c@AUSX13MPS306.AMER.DELL.COM>
On Fri, 2018-10-26 at 21:48 +0000, Justin.Lee1@Dell.com wrote:
> Hi Samuel,
>
> There is one place that we assume the next available TX channel is under the same package.
> Please see the comment below.
>
> Thanks,
> Justin
>
>
> +/* Change the active Tx channel in a multi-channel setup */
> +int ncsi_update_tx_channel(struct ncsi_dev_priv *ndp,
> > + struct ncsi_package *np,
> > + struct ncsi_channel *disable,
> > + struct ncsi_channel *enable)
> > +{
> > + struct ncsi_cmd_arg nca;
> > + struct ncsi_channel *nc;
> > + int ret = 0;
> > +
> > + if (!np->multi_channel)
> > + netdev_warn(ndp->ndev.dev,
> > + "NCSI: Trying to update Tx channel in single-channel mode\n");
> > + nca.ndp = ndp;
> > + nca.package = np->id;
>
> If the channel may be on different package, the package ID here may not be correct
> in some cases.
>
> > + nca.req_flags = 0;
> > +
> > + /* Find current channel with Tx enabled */
> > + if (!disable) {
> > + NCSI_FOR_EACH_CHANNEL(np, nc)
> > + if (nc->modes[NCSI_MODE_TX_ENABLE].enable)
> > + disable = nc;
> > + }
> > +
> > + /* Find a suitable channel for Tx */
> > + if (!enable) {
> > + if (np->preferred_channel &&
> > + ncsi_channel_has_link(np->preferred_channel)) {
> > + enable = np->preferred_channel;
> > + } else {
> > + NCSI_FOR_EACH_CHANNEL(np, nc) {
> > + if (!(np->channel_whitelist & 0x1 << nc->id))
> > + continue;
> > + if (nc->state != NCSI_CHANNEL_ACTIVE)
> > + continue;
> > + if (ncsi_channel_has_link(nc)) {
> > + enable = nc;
> > + break;
> > + }
> > + }
>
> When we search, we need to consider the other available channel might be on the
> package.
Good point, I've updated this to allow for enabling Tx on a different
package.
Thanks,
Sam
>
> > + }
> > + }
> > +
> > + if (disable == enable)
> > + return -1;
> > +
> > + if (!enable)
> > + return -1;
> > +
> > + if (disable) {
> > + nca.channel = disable->id;
> > + nca.type = NCSI_PKT_CMD_DCNT;
> > + ret = ncsi_xmit_cmd(&nca);
> > + if (ret)
> > + netdev_err(ndp->ndev.dev,
> > + "Error %d sending DCNT\n",
> > + ret);
> > + }
>
> I remove the cable from ncsi0 and it doesn't failover to ncsi3 as ncsi0 and ncsi3 are not under
> the same package.
>
> cat /sys/kernel/debug/ncsi_protocol/ncsi_device_
> IFIDX IFNAME NAME PID CID RX TX MP MC WP WC PC CS PS LS RU CR NQ HA
> ======================================================================
> 2 eth2 ncsi0 000 000 1 1 1 1 1 1 1 3 0 0 1 1 0 1
> 2 eth2 ncsi1 000 001 0 0 1 1 1 0 0 1 0 1 1 1 0 1
> 2 eth2 ncsi2 001 000 0 0 1 1 1 0 0 1 0 1 1 1 0 1
> 2 eth2 ncsi3 001 001 1 0 1 1 1 1 0 2 1 1 1 1 0 1
> ======================================================================
> MP: Multi-mode Package WP: Whitelist Package
> MC: Multi-mode Channel WC: Whitelist Channel
> PC: Primary Channel CS: Channel State
> PS: Poll Status LS: Link Status
> RU: Running CR: Carrier OK
> NQ: Queue Stopped HA: Hardware Arbitration
next prev parent reply other threads:[~2018-10-30 3:05 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-23 21:51 [PATCH net-next v2 0/6] net/ncsi: Allow enabling multiple packages & channels Samuel Mendoza-Jonas
2018-10-23 21:51 ` [PATCH net-next v2 1/6] net/ncsi: Don't enable all channels when HWA available Samuel Mendoza-Jonas
2018-10-24 3:12 ` kbuild test robot
2018-10-23 21:51 ` [PATCH net-next v2 2/6] net/ncsi: Probe single packages to avoid conflict Samuel Mendoza-Jonas
2018-10-23 21:51 ` [PATCH net-next v2 3/6] net/ncsi: Don't deselect package in suspend if active Samuel Mendoza-Jonas
2018-10-23 21:51 ` [PATCH net-next v2 4/6] net/ncsi: Don't mark configured channels inactive Samuel Mendoza-Jonas
2018-10-23 21:52 ` [PATCH net-next v2 5/6] net/ncsi: Reset channel state in ncsi_start_dev() Samuel Mendoza-Jonas
2018-10-26 17:25 ` Justin.Lee1
2018-10-30 0:23 ` Samuel Mendoza-Jonas
2018-10-30 18:23 ` Justin.Lee1
2018-11-01 4:30 ` Samuel Mendoza-Jonas
2018-11-01 15:51 ` Justin.Lee1
2018-10-23 21:52 ` [PATCH net-next v2 6/6] net/ncsi: Configure multi-package, multi-channel modes with failover Samuel Mendoza-Jonas
2018-10-26 21:48 ` Justin.Lee1
2018-10-30 3:05 ` Samuel Mendoza-Jonas [this message]
2018-10-23 23:55 ` [PATCH net-next v2 0/6] net/ncsi: Allow enabling multiple packages & channels David Miller
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=c64d1e440e2469ad0125b6c27c3bc4c59723bdb9.camel@mendozajonas.com \
--to=sam@mendozajonas.com \
--cc=Justin.Lee1@Dell.com \
--cc=davem@davemloft.net \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=openbmc@lists.ozlabs.org \
/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 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).