Messages in this thread Patch in this message | | | From | <> | Subject | RE: [PATCH net-next v3 6/6] net/ncsi: Configure multi-package, multi-channel modes with failover | Date | Thu, 8 Nov 2018 22:48:09 +0000 |
| |
Hi Samuel,
For multi-package and multi-channel case, channel seems to be select correctly. Expect that, I still see the timing issue for back-to-back netlink command. Due to that, channel might be set to invisible state. Please refer to ncsi0 and ncsi2 below. The channel state is set to 3.
cat /sys/kernel/debug/ncsi_protocol/ncsi_device_status 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 0 0 3 0 1 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 1 0 1 1 1 1 0 3 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 IA/A/IV 1/2/3 PS: Poll Status LS: Link Status RU: Running CR: Carrier OK NQ: Queue Stopped HA: Hardware Arbitration
The timing issue is not only happening in application. If I use using the following way to send the request, I can see the issue as well.
ncsi_netlink -l 2 -a 0x01 -m; ncsi_netlink -l 2 -p 0 -b 0x03 -m; ncsi_netlink -l 2 -p 1 -b 0x00 -m; ncsi_netlink -l 2 -a 0x03 -m; ncsi_netlink -l 2 -p 0 -b 0x00 -m; ncsi_netlink -l 2 -p 1 -b 0x03 -m;
Also, there is one issue below for non-multi-package/non-multi-channel case.
Thanks, Justin
> @@ -1008,32 +1164,49 @@ static int ncsi_choose_active_channel(struct ncsi_dev_priv *ndp) > > ncm = &nc->modes[NCSI_MODE_LINK]; > if (ncm->data[2] & 0x1) { > - spin_unlock_irqrestore(&nc->lock, flags); > found = nc; > - goto out; > + with_link = true; > } > > - spin_unlock_irqrestore(&nc->lock, flags); > + /* If multi_channel is enabled configure all valid > + * channels whether or not they currently have link > + * so they will have AENs enabled. > + */ > + if (with_link || np->multi_channel) { > + spin_lock_irqsave(&ndp->lock, flags); > + list_add_tail_rcu(&nc->link, > + &ndp->channel_queue); > + spin_unlock_irqrestore(&ndp->lock, flags); > + > + netdev_dbg(ndp->ndev.dev, > + "NCSI: Channel %u added to queue (link %s)\n", > + nc->id, > + ncm->data[2] & 0x1 ? "up" : "down"); > + } > + > + spin_unlock_irqrestore(&nc->lock, cflags); > + > + if (with_link && !np->multi_channel) > + break;
The line needs to change to "goto found". If not, all channels with link will be added even if the multi-channel is not enabled for that package. The ncsi1 below is enabled. There is no netlink command sent to enable multi-package or multi-channel.
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 0 0 1 1 0 2 1 1 1 1 0 1 2 eth2 ncsi1 000 001 1 0 0 0 1 1 0 2 1 1 1 1 0 1 2 eth2 ncsi2 001 000 0 0 0 0 1 1 0 1 0 1 1 1 0 1 2 eth2 ncsi3 001 001 0 0 0 0 1 1 0 1 0 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 IA/A/IV 1/2/3 PS: Poll Status LS: Link Status RU: Running CR: Carrier OK NQ: Queue Stopped HA: Hardware Arbitration
> } > + if (with_link && !ndp->multi_package) > + break; > }
found:
After applying this change, I notice that if there is no link available to BMC when BMC starts, NC-SI can't properly configure channel once I plug in the Ethernet cable.
npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_aen_handler_lsc() - pkg 0 ch 0 state up npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_aen_handler_lsc() - had_link 0, has_link 1, chained 0 npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_stop_channel_monitor() - pkg 0 ch 0 npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_process_next_channel() npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_process_next_channel() - pkg 0 ch 0 INVISIBLE npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_process_next_channel() - suspending pkg 0 ch 0 npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_suspend_channel() - pkg 0 ch 0 state 0400 select npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_dev_work() npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_suspend_channel() - pkg 0 ch 0 state 0403 dc npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_dev_work() npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_suspend_channel() - pkg 0 ch 0 state 0404 deselect npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_dev_work() npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_suspend_channel() - pkg 0 ch 0 state 0405 done npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_rsp_handler_dp() - pkg 0 ch 0 INACTIVE npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_rsp_handler_dp() - pkg 0 ch 1 INACTIVE npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_dev_work() npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_suspend_channel() - pkg 0 ch 0 state 0406 deselect npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_suspend_channel() - pkg 0 ch 0 INACTIVE npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_process_next_channel() npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_process_next_channel() - No more channels to process npcm7xx-emc f0825000.eth eth2: NCSI interface down
> > - if (!found) { > + if (list_empty(&ndp->channel_queue) && found) { > + netdev_info(ndp->ndev.dev, > + "NCSI: No channel with link found, configuring channel %u\n", > + found->id); > + spin_lock_irqsave(&ndp->lock, flags); > + list_add_tail_rcu(&found->link, &ndp->channel_queue); > + spin_unlock_irqrestore(&ndp->lock, flags); > + } else if (!found) { > netdev_warn(ndp->ndev.dev, > - "NCSI: No channel found with link\n"); > + "NCSI: No channel found to configure!\n"); > ncsi_report_link(ndp, true); > return -ENODEV; > } > > - ncm = &found->modes[NCSI_MODE_LINK]; > - netdev_dbg(ndp->ndev.dev, > - "NCSI: Channel %u added to queue (link %s)\n", > - found->id, ncm->data[2] & 0x1 ? "up" : "down"); > - > -out: > - spin_lock_irqsave(&ndp->lock, flags); > - list_add_tail_rcu(&found->link, &ndp->channel_queue); > - spin_unlock_irqrestore(&ndp->lock, flags); > - > return ncsi_process_next_channel(ndp); > }
| |