| Message ID | 20181108024909.9897-7-sam@mendozajonas.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | net/ncsi: Allow enabling multiple packages & channels | expand |
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); > }
On Thu, 2018-11-08 at 22:48 +0000, Justin.Lee1@Dell.com wrote: > 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; This actually recreates for me as well; I see now what you mean about channels getting stuck in the invisible state. I believe I've narrowed down the issue. I've pasted an additional patch below if you are able to test on your machine. > > > 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: This *may* be part of the above issue, I don't see this in normal operation. The combination of (with_link && !np->multi_channel) and (with_link && !ndp->multi_package) should prevent additional channels being added without the need for 'goto found'. Please let me know if you still see it with the extra patch. > > 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 Good find, there was a corner case in the LSC AEN handler changes that led to this, I've fixed this in the patch as well. Thanks for testing! From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Samuel Mendoza-Jonas <sam@mendozajonas.com> Date: Fri, 9 Nov 2018 13:11:03 +1100 Subject: [PATCH] net/ncsi: Reset state fixes, single-channel LSC --- net/ncsi/ncsi-aen.c | 8 +++++--- net/ncsi/ncsi-manage.c | 19 +++++++++++++++---- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c index 39c2e9eea2ba..034cb1dc5566 100644 --- a/net/ncsi/ncsi-aen.c +++ b/net/ncsi/ncsi-aen.c @@ -93,14 +93,16 @@ static int ncsi_aen_handler_lsc(struct ncsi_dev_priv *ndp, if ((had_link == has_link) || chained) return 0; - if (!ndp->multi_package && !nc->package->multi_channel) { - if (had_link) - ndp->flags |= NCSI_DEV_RESHUFFLE; + if (!ndp->multi_package && !nc->package->multi_channel && had_link) { + ndp->flags |= NCSI_DEV_RESHUFFLE; ncsi_stop_channel_monitor(nc); spin_lock_irqsave(&ndp->lock, flags); list_add_tail_rcu(&nc->link, &ndp->channel_queue); spin_unlock_irqrestore(&ndp->lock, flags); return ncsi_process_next_channel(ndp); + } else { + /* Configured channel came up */ + return 0; } if (had_link) { diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c index fa3c2144f5ba..92e59f07f9a7 100644 --- a/net/ncsi/ncsi-manage.c +++ b/net/ncsi/ncsi-manage.c @@ -1063,17 +1063,17 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp) case ncsi_dev_state_config_done: netdev_dbg(ndp->ndev.dev, "NCSI: channel %u config done\n", nc->id); + spin_lock_irqsave(&nc->lock, flags); + nc->state = NCSI_CHANNEL_ACTIVE; + if (ndp->flags & NCSI_DEV_RESET) { /* A reset event happened during config, start it now */ - spin_lock_irqsave(&nc->lock, flags); nc->reconfigure_needed = false; spin_unlock_irqrestore(&nc->lock, flags); - nd->state = ncsi_dev_state_functional; ncsi_reset_dev(nd); break; } - spin_lock_irqsave(&nc->lock, flags); if (nc->reconfigure_needed) { /* This channel's configuration has been updated * part-way during the config state - start the @@ -1092,7 +1092,6 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp) break; } - nc->state = NCSI_CHANNEL_ACTIVE; if (nc->modes[NCSI_MODE_LINK].data[2] & 0x1) { hot_nc = nc; } else { @@ -1803,6 +1802,18 @@ int ncsi_reset_dev(struct ncsi_dev *nd) spin_unlock_irqrestore(&ndp->lock, flags); return 0; } + } else { + switch (nd->state) { + case ncsi_dev_state_suspend_done: + case ncsi_dev_state_config_done: + case ncsi_dev_state_functional: + /* Ok */ + break; + default: + /* Current reset operation happening */ + spin_unlock_irqrestore(&ndp->lock, flags); + return 0; + } } if (!list_empty(&ndp->channel_queue)) {
Hi Samuel, The extra patch fixed most issues but I see another corner case. Thanks, Justin > On Thu, 2018-11-08 at 22:48 +0000, Justin.Lee1@Dell.com wrote: > > 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; > > This actually recreates for me as well; I see now what you mean about > channels getting stuck in the invisible state. I believe I've narrowed > down the issue. I've pasted an additional patch below if you are able to > test on your machine. > > > > > > > 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: > > This *may* be part of the above issue, I don't see this in normal > operation. The combination of (with_link && !np->multi_channel) and > (with_link && !ndp->multi_package) should prevent additional channels > being added without the need for 'goto found'. Please let me know if you > still see it with the extra patch. > This one is fixed by your extra patch but I see another corner case. Basically, the issue is due to the link status is not updating during the process of choosing the active channel. During the process, maybe we should find a chance to send Get Link Status command to all whitelist channels. Here is my step. 1. All four channels plug Ethernet cable. 2. BMC starts without any multi-package/multi-channel configuration. 3. The ncsi0 channel is selected by NC-SI driver. 4. Remove cable from ncsi0. 5. The ncsi1 channel is selected by NC-SI driver. 6. Remove cable from ncsi1. 7. The ncsi2 channel is selected by NC-SI driver. 8. Insert cable for ncsi0. Link status will not be updated. 9. Remove cable from ncsi2. 10. The ncsi3 channel is selected by NC-SI driver. 11. Remove cable from ncsi3. 12. The selected channel is ncsi3 without link. 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 0 0 0 0 1 1 0 1 0 0 1 1 0 1 2 eth2 ncsi1 000 001 0 0 0 0 1 1 0 1 0 0 1 1 0 1 2 eth2 ncsi2 001 000 0 0 0 0 1 1 0 1 0 0 1 1 0 1 2 eth2 ncsi3 001 001 1 1 0 0 1 1 0 2 1 0 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 > > > > 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 > > Good find, there was a corner case in the LSC AEN handler changes that > led to this, I've fixed this in the patch as well. Thanks for testing! This one is fixed. > > > From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 > From: Samuel Mendoza-Jonas <sam@mendozajonas.com> > Date: Fri, 9 Nov 2018 13:11:03 +1100 > Subject: [PATCH] net/ncsi: Reset state fixes, single-channel LSC > > --- > net/ncsi/ncsi-aen.c | 8 +++++--- > net/ncsi/ncsi-manage.c | 19 +++++++++++++++---- > 2 files changed, 20 insertions(+), 7 deletions(-) > > diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c > index 39c2e9eea2ba..034cb1dc5566 100644 > --- a/net/ncsi/ncsi-aen.c > +++ b/net/ncsi/ncsi-aen.c > @@ -93,14 +93,16 @@ static int ncsi_aen_handler_lsc(struct ncsi_dev_priv *ndp, > if ((had_link == has_link) || chained) > return 0; > > - if (!ndp->multi_package && !nc->package->multi_channel) { > - if (had_link) > - ndp->flags |= NCSI_DEV_RESHUFFLE; > + if (!ndp->multi_package && !nc->package->multi_channel && had_link) { > + ndp->flags |= NCSI_DEV_RESHUFFLE; > ncsi_stop_channel_monitor(nc); > spin_lock_irqsave(&ndp->lock, flags); > list_add_tail_rcu(&nc->link, &ndp->channel_queue); > spin_unlock_irqrestore(&ndp->lock, flags); > return ncsi_process_next_channel(ndp); > + } else { > + /* Configured channel came up */ > + return 0; > } > > if (had_link) { > diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c > index fa3c2144f5ba..92e59f07f9a7 100644 > --- a/net/ncsi/ncsi-manage.c > +++ b/net/ncsi/ncsi-manage.c > @@ -1063,17 +1063,17 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp) > case ncsi_dev_state_config_done: > netdev_dbg(ndp->ndev.dev, "NCSI: channel %u config done\n", > nc->id); > + spin_lock_irqsave(&nc->lock, flags); > + nc->state = NCSI_CHANNEL_ACTIVE; > + > if (ndp->flags & NCSI_DEV_RESET) { > /* A reset event happened during config, start it now */ > - spin_lock_irqsave(&nc->lock, flags); > nc->reconfigure_needed = false; > spin_unlock_irqrestore(&nc->lock, flags); > - nd->state = ncsi_dev_state_functional; > ncsi_reset_dev(nd); > break; > } > > - spin_lock_irqsave(&nc->lock, flags); > if (nc->reconfigure_needed) { > /* This channel's configuration has been updated > * part-way during the config state - start the > @@ -1092,7 +1092,6 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp) > break; > } > > - nc->state = NCSI_CHANNEL_ACTIVE; > if (nc->modes[NCSI_MODE_LINK].data[2] & 0x1) { > hot_nc = nc; > } else { > @@ -1803,6 +1802,18 @@ int ncsi_reset_dev(struct ncsi_dev *nd) > spin_unlock_irqrestore(&ndp->lock, flags); > return 0; > } > + } else { > + switch (nd->state) { > + case ncsi_dev_state_suspend_done: > + case ncsi_dev_state_config_done: > + case ncsi_dev_state_functional: > + /* Ok */ > + break; > + default: > + /* Current reset operation happening */ > + spin_unlock_irqrestore(&ndp->lock, flags); > + return 0; > + } > } > > if (!list_empty(&ndp->channel_queue)) { > -- > 2.19.1
Hi Samuel, After running more testing, I notice that the extra patch causing failover function to fail. Also, occasionally, I will see two channels having TX enabled if I send netlink command back-to-back. 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 2 1 1 1 1 0 1 2 eth2 ncsi1 000 001 1 1 1 1 1 1 0 2 1 1 1 1 0 1 2 eth2 ncsi2 001 000 0 0 1 1 0 0 0 1 0 1 1 1 0 1 2 eth2 ncsi3 001 001 0 0 1 1 0 0 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 Thanks, Justin > From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 > From: Samuel Mendoza-Jonas <sam@mendozajonas.com> > Date: Fri, 9 Nov 2018 13:11:03 +1100 > Subject: [PATCH] net/ncsi: Reset state fixes, single-channel LSC > > --- > net/ncsi/ncsi-aen.c | 8 +++++--- > net/ncsi/ncsi-manage.c | 19 +++++++++++++++---- > 2 files changed, 20 insertions(+), 7 deletions(-) > > diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c > index 39c2e9eea2ba..034cb1dc5566 100644 > --- a/net/ncsi/ncsi-aen.c > +++ b/net/ncsi/ncsi-aen.c > @@ -93,14 +93,16 @@ static int ncsi_aen_handler_lsc(struct ncsi_dev_priv *ndp, > if ((had_link == has_link) || chained) > return 0; > > - if (!ndp->multi_package && !nc->package->multi_channel) { > - if (had_link) > - ndp->flags |= NCSI_DEV_RESHUFFLE; > + if (!ndp->multi_package && !nc->package->multi_channel && had_link) { > + ndp->flags |= NCSI_DEV_RESHUFFLE; > ncsi_stop_channel_monitor(nc); > spin_lock_irqsave(&ndp->lock, flags); > list_add_tail_rcu(&nc->link, &ndp->channel_queue); > spin_unlock_irqrestore(&ndp->lock, flags); > return ncsi_process_next_channel(ndp); > + } else { > + /* Configured channel came up */ > + return 0; It is always going to else statement if multi_package and/or mutlit_channel is enabled. npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_aen_handler_lsc() - pkg 0 ch 0 state down npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_aen_handler_lsc() - had_link 1, has_link 0, chained 0 These codes have no chance to run. if (had_link) { ncm = &nc->modes[NCSI_MODE_TX_ENABLE]; if (ncsi_channel_is_last(ndp, nc)) { /* No channels left, reconfigure */ return ncsi_reset_dev(&ndp->ndev); } else if (ncm->enable) { /* Need to failover Tx channel */ ncsi_update_tx_channel(ndp, nc->package, nc, NULL); } > } >
On Fri, 2018-11-09 at 21:58 +0000, Justin.Lee1@Dell.com wrote: > Hi Samuel, > > After running more testing, I notice that the extra patch causing failover function > to fail. Also, occasionally, I will see two channels having TX enabled if I > send netlink command back-to-back. > > 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 2 1 1 1 1 0 1 > 2 eth2 ncsi1 000 001 1 1 1 1 1 1 0 2 1 1 1 1 0 1 > 2 eth2 ncsi2 001 000 0 0 1 1 0 0 0 1 0 1 1 1 0 1 > 2 eth2 ncsi3 001 001 0 0 1 1 0 0 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 > > Thanks, > Justin > > > > From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 > > From: Samuel Mendoza-Jonas <sam@mendozajonas.com> > > Date: Fri, 9 Nov 2018 13:11:03 +1100 > > Subject: [PATCH] net/ncsi: Reset state fixes, single-channel LSC > > > > --- > > net/ncsi/ncsi-aen.c | 8 +++++--- > > net/ncsi/ncsi-manage.c | 19 +++++++++++++++---- > > 2 files changed, 20 insertions(+), 7 deletions(-) > > > > diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c > > index 39c2e9eea2ba..034cb1dc5566 100644 > > --- a/net/ncsi/ncsi-aen.c > > +++ b/net/ncsi/ncsi-aen.c > > @@ -93,14 +93,16 @@ static int ncsi_aen_handler_lsc(struct ncsi_dev_priv *ndp, > > if ((had_link == has_link) || chained) > > return 0; > > > > - if (!ndp->multi_package && !nc->package->multi_channel) { > > - if (had_link) > > - ndp->flags |= NCSI_DEV_RESHUFFLE; > > + if (!ndp->multi_package && !nc->package->multi_channel && had_link) { > > + ndp->flags |= NCSI_DEV_RESHUFFLE; > > ncsi_stop_channel_monitor(nc); > > spin_lock_irqsave(&ndp->lock, flags); > > list_add_tail_rcu(&nc->link, &ndp->channel_queue); > > spin_unlock_irqrestore(&ndp->lock, flags); > > return ncsi_process_next_channel(ndp); > > + } else { > > + /* Configured channel came up */ > > + return 0; > > It is always going to else statement if multi_package and/or mutlit_channel is enabled. > > npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_aen_handler_lsc() - pkg 0 ch 0 state down > npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_aen_handler_lsc() - had_link 1, has_link 0, chained 0 > > These codes have no chance to run. > if (had_link) { > ncm = &nc->modes[NCSI_MODE_TX_ENABLE]; > if (ncsi_channel_is_last(ndp, nc)) { > /* No channels left, reconfigure */ > return ncsi_reset_dev(&ndp->ndev); > } else if (ncm->enable) { > /* Need to failover Tx channel */ > ncsi_update_tx_channel(ndp, nc->package, nc, NULL); > } > > > } > > Oops, wrote that patch a little fast it seems. This may also affect the double-Tx above since ncsi_update_tx_channel() won't be reached. I've attached a fixed up version below. For the failover issue you're seeing in your previous email the issue is likely in the ncsi_dev_state_suspend_gls state. This should send a GLS command to all available channels, but it only does it for the current package. Since not all packages are necessarily enabled in single-channel mode I'll need to have a think about the neatest way to handle that, but it's a separate issue from this patch. Cheers, Sam From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Samuel Mendoza-Jonas <sam@mendozajonas.com> Date: Fri, 9 Nov 2018 13:11:03 +1100 Subject: [PATCH] net/ncsi: Reset state fixes, single-channel LSC --- net/ncsi/ncsi-aen.c | 16 ++++++++++------ net/ncsi/ncsi-manage.c | 19 +++++++++++++++---- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c index 39c2e9eea2ba..76559d0eeea8 100644 --- a/net/ncsi/ncsi-aen.c +++ b/net/ncsi/ncsi-aen.c @@ -94,13 +94,17 @@ static int ncsi_aen_handler_lsc(struct ncsi_dev_priv *ndp, return 0; if (!ndp->multi_package && !nc->package->multi_channel) { - if (had_link) + if (had_link) { ndp->flags |= NCSI_DEV_RESHUFFLE; - ncsi_stop_channel_monitor(nc); - spin_lock_irqsave(&ndp->lock, flags); - list_add_tail_rcu(&nc->link, &ndp->channel_queue); - spin_unlock_irqrestore(&ndp->lock, flags); - return ncsi_process_next_channel(ndp); + ncsi_stop_channel_monitor(nc); + spin_lock_irqsave(&ndp->lock, flags); + list_add_tail_rcu(&nc->link, &ndp->channel_queue); + spin_unlock_irqrestore(&ndp->lock, flags); + return ncsi_process_next_channel(ndp); + } else { + /* Configured channel came up */ + return 0; + } } if (had_link) { diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c index fa3c2144f5ba..92e59f07f9a7 100644 --- a/net/ncsi/ncsi-manage.c +++ b/net/ncsi/ncsi-manage.c @@ -1063,17 +1063,17 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp) case ncsi_dev_state_config_done: netdev_dbg(ndp->ndev.dev, "NCSI: channel %u config done\n", nc->id); + spin_lock_irqsave(&nc->lock, flags); + nc->state = NCSI_CHANNEL_ACTIVE; + if (ndp->flags & NCSI_DEV_RESET) { /* A reset event happened during config, start it now */ - spin_lock_irqsave(&nc->lock, flags); nc->reconfigure_needed = false; spin_unlock_irqrestore(&nc->lock, flags); - nd->state = ncsi_dev_state_functional; ncsi_reset_dev(nd); break; } - spin_lock_irqsave(&nc->lock, flags); if (nc->reconfigure_needed) { /* This channel's configuration has been updated * part-way during the config state - start the @@ -1092,7 +1092,6 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp) break; } - nc->state = NCSI_CHANNEL_ACTIVE; if (nc->modes[NCSI_MODE_LINK].data[2] & 0x1) { hot_nc = nc; } else { @@ -1803,6 +1802,18 @@ int ncsi_reset_dev(struct ncsi_dev *nd) spin_unlock_irqrestore(&ndp->lock, flags); return 0; } + } else { + switch (nd->state) { + case ncsi_dev_state_suspend_done: + case ncsi_dev_state_config_done: + case ncsi_dev_state_functional: + /* Ok */ + break; + default: + /* Current reset operation happening */ + spin_unlock_irqrestore(&ndp->lock, flags); + return 0; + } } if (!list_empty(&ndp->channel_queue)) {
Hi Samuel, I have tested your new patch. The failover function works as expected. Thanks, Justin > On Fri, 2018-11-09 at 21:58 +0000, Justin.Lee1@Dell.com wrote: > > Hi Samuel, > > > > After running more testing, I notice that the extra patch causing failover function > > to fail. Also, occasionally, I will see two channels having TX enabled if I > > send netlink command back-to-back. > > > > 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 2 1 1 1 1 0 1 > > 2 eth2 ncsi1 000 001 1 1 1 1 1 1 0 2 1 1 1 1 0 1 > > 2 eth2 ncsi2 001 000 0 0 1 1 0 0 0 1 0 1 1 1 0 1 > > 2 eth2 ncsi3 001 001 0 0 1 1 0 0 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 > > > > Thanks, > > Justin > > > > > > > From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 > > > From: Samuel Mendoza-Jonas <sam@mendozajonas.com> > > > Date: Fri, 9 Nov 2018 13:11:03 +1100 > > > Subject: [PATCH] net/ncsi: Reset state fixes, single-channel LSC > > > > > > --- > > > net/ncsi/ncsi-aen.c | 8 +++++--- > > > net/ncsi/ncsi-manage.c | 19 +++++++++++++++---- > > > 2 files changed, 20 insertions(+), 7 deletions(-) > > > > > > diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c > > > index 39c2e9eea2ba..034cb1dc5566 100644 > > > --- a/net/ncsi/ncsi-aen.c > > > +++ b/net/ncsi/ncsi-aen.c > > > @@ -93,14 +93,16 @@ static int ncsi_aen_handler_lsc(struct ncsi_dev_priv *ndp, > > > if ((had_link == has_link) || chained) > > > return 0; > > > > > > - if (!ndp->multi_package && !nc->package->multi_channel) { > > > - if (had_link) > > > - ndp->flags |= NCSI_DEV_RESHUFFLE; > > > + if (!ndp->multi_package && !nc->package->multi_channel && had_link) { > > > + ndp->flags |= NCSI_DEV_RESHUFFLE; > > > ncsi_stop_channel_monitor(nc); > > > spin_lock_irqsave(&ndp->lock, flags); > > > list_add_tail_rcu(&nc->link, &ndp->channel_queue); > > > spin_unlock_irqrestore(&ndp->lock, flags); > > > return ncsi_process_next_channel(ndp); > > > + } else { > > > + /* Configured channel came up */ > > > + return 0; > > > > It is always going to else statement if multi_package and/or mutlit_channel is enabled. > > > > npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_aen_handler_lsc() - pkg 0 ch 0 state down > > npcm7xx-emc f0825000.eth eth2: NCSI: ncsi_aen_handler_lsc() - had_link 1, has_link 0, chained 0 > > > > These codes have no chance to run. > > if (had_link) { > > ncm = &nc->modes[NCSI_MODE_TX_ENABLE]; > > if (ncsi_channel_is_last(ndp, nc)) { > > /* No channels left, reconfigure */ > > return ncsi_reset_dev(&ndp->ndev); > > } else if (ncm->enable) { > > /* Need to failover Tx channel */ > > ncsi_update_tx_channel(ndp, nc->package, nc, NULL); > > } > > > > > } > > > > > Oops, wrote that patch a little fast it seems. This may also affect the > double-Tx above since ncsi_update_tx_channel() won't be reached. > I've attached a fixed up version below. > > For the failover issue you're seeing in your previous email the issue is > likely in the ncsi_dev_state_suspend_gls state. This should send a GLS > command to all available channels, but it only does it for the current > package. Since not all packages are necessarily enabled in single-channel > mode I'll need to have a think about the neatest way to handle that, but > it's a separate issue from this patch. > > Cheers, > Sam > > > From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 > From: Samuel Mendoza-Jonas <sam@mendozajonas.com> > Date: Fri, 9 Nov 2018 13:11:03 +1100 > Subject: [PATCH] net/ncsi: Reset state fixes, single-channel LSC > > --- > net/ncsi/ncsi-aen.c | 16 ++++++++++------ > net/ncsi/ncsi-manage.c | 19 +++++++++++++++---- > 2 files changed, 25 insertions(+), 10 deletions(-) > > diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c > index 39c2e9eea2ba..76559d0eeea8 100644 > --- a/net/ncsi/ncsi-aen.c > +++ b/net/ncsi/ncsi-aen.c > @@ -94,13 +94,17 @@ static int ncsi_aen_handler_lsc(struct ncsi_dev_priv *ndp, > return 0; > > if (!ndp->multi_package && !nc->package->multi_channel) { > - if (had_link) > + if (had_link) { > ndp->flags |= NCSI_DEV_RESHUFFLE; > - ncsi_stop_channel_monitor(nc); > - spin_lock_irqsave(&ndp->lock, flags); > - list_add_tail_rcu(&nc->link, &ndp->channel_queue); > - spin_unlock_irqrestore(&ndp->lock, flags); > - return ncsi_process_next_channel(ndp); > + ncsi_stop_channel_monitor(nc); > + spin_lock_irqsave(&ndp->lock, flags); > + list_add_tail_rcu(&nc->link, &ndp->channel_queue); > + spin_unlock_irqrestore(&ndp->lock, flags); > + return ncsi_process_next_channel(ndp); > + } else { > + /* Configured channel came up */ > + return 0; > + } > } > > if (had_link) { > diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c > index fa3c2144f5ba..92e59f07f9a7 100644 > --- a/net/ncsi/ncsi-manage.c > +++ b/net/ncsi/ncsi-manage.c > @@ -1063,17 +1063,17 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp) > case ncsi_dev_state_config_done: > netdev_dbg(ndp->ndev.dev, "NCSI: channel %u config done\n", > nc->id); > + spin_lock_irqsave(&nc->lock, flags); > + nc->state = NCSI_CHANNEL_ACTIVE; > + > if (ndp->flags & NCSI_DEV_RESET) { > /* A reset event happened during config, start it now */ > - spin_lock_irqsave(&nc->lock, flags); > nc->reconfigure_needed = false; > spin_unlock_irqrestore(&nc->lock, flags); > - nd->state = ncsi_dev_state_functional; > ncsi_reset_dev(nd); > break; > } > > - spin_lock_irqsave(&nc->lock, flags); > if (nc->reconfigure_needed) { > /* This channel's configuration has been updated > * part-way during the config state - start the > @@ -1092,7 +1092,6 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp) > break; > } > > - nc->state = NCSI_CHANNEL_ACTIVE; > if (nc->modes[NCSI_MODE_LINK].data[2] & 0x1) { > hot_nc = nc; > } else { > @@ -1803,6 +1802,18 @@ int ncsi_reset_dev(struct ncsi_dev *nd) > spin_unlock_irqrestore(&ndp->lock, flags); > return 0; > } > + } else { > + switch (nd->state) { > + case ncsi_dev_state_suspend_done: > + case ncsi_dev_state_config_done: > + case ncsi_dev_state_functional: > + /* Ok */ > + break; > + default: > + /* Current reset operation happening */ > + spin_unlock_irqrestore(&ndp->lock, flags); > + return 0; > + } > } > > if (!list_empty(&ndp->channel_queue)) { > -- > 2.19.1
diff --git a/include/uapi/linux/ncsi.h b/include/uapi/linux/ncsi.h index 0a26a5576645..a3f87c54fdb3 100644 --- a/include/uapi/linux/ncsi.h +++ b/include/uapi/linux/ncsi.h @@ -26,6 +26,12 @@ * @NCSI_CMD_SEND_CMD: send NC-SI command to network card. * Requires NCSI_ATTR_IFINDEX, NCSI_ATTR_PACKAGE_ID * and NCSI_ATTR_CHANNEL_ID. + * @NCSI_CMD_SET_PACKAGE_MASK: set a whitelist of allowed packages. + * Requires NCSI_ATTR_IFINDEX and NCSI_ATTR_PACKAGE_MASK. + * @NCSI_CMD_SET_CHANNEL_MASK: set a whitelist of allowed channels. + * Requires NCSI_ATTR_IFINDEX, NCSI_ATTR_PACKAGE_ID, and + * NCSI_ATTR_CHANNEL_MASK. If NCSI_ATTR_CHANNEL_ID is present it sets + * the primary channel. * @NCSI_CMD_MAX: highest command number */ enum ncsi_nl_commands { @@ -34,6 +40,8 @@ enum ncsi_nl_commands { NCSI_CMD_SET_INTERFACE, NCSI_CMD_CLEAR_INTERFACE, NCSI_CMD_SEND_CMD, + NCSI_CMD_SET_PACKAGE_MASK, + NCSI_CMD_SET_CHANNEL_MASK, __NCSI_CMD_AFTER_LAST, NCSI_CMD_MAX = __NCSI_CMD_AFTER_LAST - 1 @@ -48,6 +56,10 @@ enum ncsi_nl_commands { * @NCSI_ATTR_PACKAGE_ID: package ID * @NCSI_ATTR_CHANNEL_ID: channel ID * @NCSI_ATTR_DATA: command payload + * @NCSI_ATTR_MULTI_FLAG: flag to signal that multi-mode should be enabled with + * NCSI_CMD_SET_PACKAGE_MASK or NCSI_CMD_SET_CHANNEL_MASK. + * @NCSI_ATTR_PACKAGE_MASK: 32-bit mask of allowed packages. + * @NCSI_ATTR_CHANNEL_MASK: 32-bit mask of allowed channels. * @NCSI_ATTR_MAX: highest attribute number */ enum ncsi_nl_attrs { @@ -57,6 +69,9 @@ enum ncsi_nl_attrs { NCSI_ATTR_PACKAGE_ID, NCSI_ATTR_CHANNEL_ID, NCSI_ATTR_DATA, + NCSI_ATTR_MULTI_FLAG, + NCSI_ATTR_PACKAGE_MASK, + NCSI_ATTR_CHANNEL_MASK, __NCSI_ATTR_AFTER_LAST, NCSI_ATTR_MAX = __NCSI_ATTR_AFTER_LAST - 1 diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h index bda51cb179fe..9e3642b802c4 100644 --- a/net/ncsi/internal.h +++ b/net/ncsi/internal.h @@ -222,6 +222,10 @@ struct ncsi_package { unsigned int channel_num; /* Number of channels */ struct list_head channels; /* List of chanels */ struct list_head node; /* Form list of packages */ + + bool multi_channel; /* Enable multiple channels */ + u32 channel_whitelist; /* Channels to configure */ + struct ncsi_channel *preferred_channel; /* Primary channel */ }; struct ncsi_request { @@ -297,8 +301,6 @@ struct ncsi_dev_priv { unsigned int package_num; /* Number of packages */ struct list_head packages; /* List of packages */ struct ncsi_channel *hot_channel; /* Channel was ever active */ - struct ncsi_package *force_package; /* Force a specific package */ - struct ncsi_channel *force_channel; /* Force a specific channel */ struct ncsi_request requests[256]; /* Request table */ unsigned int request_id; /* Last used request ID */ #define NCSI_REQ_START_IDX 1 @@ -311,6 +313,9 @@ struct ncsi_dev_priv { struct list_head node; /* Form NCSI device list */ #define NCSI_MAX_VLAN_VIDS 15 struct list_head vlan_vids; /* List of active VLAN IDs */ + + bool multi_package; /* Enable multiple packages */ + u32 package_whitelist; /* Packages to configure */ }; struct ncsi_cmd_arg { @@ -364,6 +369,13 @@ struct ncsi_request *ncsi_alloc_request(struct ncsi_dev_priv *ndp, void ncsi_free_request(struct ncsi_request *nr); struct ncsi_dev *ncsi_find_dev(struct net_device *dev); int ncsi_process_next_channel(struct ncsi_dev_priv *ndp); +bool ncsi_channel_has_link(struct ncsi_channel *channel); +bool ncsi_channel_is_last(struct ncsi_dev_priv *ndp, + struct ncsi_channel *channel); +int ncsi_update_tx_channel(struct ncsi_dev_priv *ndp, + struct ncsi_package *np, + struct ncsi_channel *disable, + struct ncsi_channel *enable); /* Packet handlers */ u32 ncsi_calculate_checksum(unsigned char *data, int len); diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c index 57f77e5d381a..39c2e9eea2ba 100644 --- a/net/ncsi/ncsi-aen.c +++ b/net/ncsi/ncsi-aen.c @@ -50,14 +50,15 @@ static int ncsi_validate_aen_pkt(struct ncsi_aen_pkt_hdr *h, static int ncsi_aen_handler_lsc(struct ncsi_dev_priv *ndp, struct ncsi_aen_pkt_hdr *h) { - struct ncsi_aen_lsc_pkt *lsc; - struct ncsi_channel *nc; + struct ncsi_channel *nc, *tmp; struct ncsi_channel_mode *ncm; - bool chained; - int state; unsigned long old_data, data; - unsigned long flags; + struct ncsi_aen_lsc_pkt *lsc; + struct ncsi_package *np; bool had_link, has_link; + unsigned long flags; + bool chained; + int state; /* Find the NCSI channel */ ncsi_find_package_and_channel(ndp, h->common.channel, NULL, &nc); @@ -92,14 +93,49 @@ static int ncsi_aen_handler_lsc(struct ncsi_dev_priv *ndp, if ((had_link == has_link) || chained) return 0; - if (had_link) - ndp->flags |= NCSI_DEV_RESHUFFLE; - ncsi_stop_channel_monitor(nc); - spin_lock_irqsave(&ndp->lock, flags); - list_add_tail_rcu(&nc->link, &ndp->channel_queue); - spin_unlock_irqrestore(&ndp->lock, flags); + if (!ndp->multi_package && !nc->package->multi_channel) { + if (had_link) + ndp->flags |= NCSI_DEV_RESHUFFLE; + ncsi_stop_channel_monitor(nc); + spin_lock_irqsave(&ndp->lock, flags); + list_add_tail_rcu(&nc->link, &ndp->channel_queue); + spin_unlock_irqrestore(&ndp->lock, flags); + return ncsi_process_next_channel(ndp); + } - return ncsi_process_next_channel(ndp); + if (had_link) { + ncm = &nc->modes[NCSI_MODE_TX_ENABLE]; + if (ncsi_channel_is_last(ndp, nc)) { + /* No channels left, reconfigure */ + return ncsi_reset_dev(&ndp->ndev); + } else if (ncm->enable) { + /* Need to failover Tx channel */ + ncsi_update_tx_channel(ndp, nc->package, nc, NULL); + } + } else if (has_link && nc->package->preferred_channel == nc) { + /* Return Tx to preferred channel */ + ncsi_update_tx_channel(ndp, nc->package, NULL, nc); + } else if (has_link) { + NCSI_FOR_EACH_PACKAGE(ndp, np) { + NCSI_FOR_EACH_CHANNEL(np, tmp) { + /* Enable Tx on this channel if the current Tx + * channel is down. + */ + ncm = &tmp->modes[NCSI_MODE_TX_ENABLE]; + if (ncm->enable && + !ncsi_channel_has_link(tmp)) { + ncsi_update_tx_channel(ndp, nc->package, + tmp, nc); + break; + } + } + } + } + + /* Leave configured channels active in a multi-channel scenario so + * AEN events are still received. + */ + return 0; } static int ncsi_aen_handler_cr(struct ncsi_dev_priv *ndp, diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c index 4b07f5701186..fa3c2144f5ba 100644 --- a/net/ncsi/ncsi-manage.c +++ b/net/ncsi/ncsi-manage.c @@ -28,6 +28,29 @@ LIST_HEAD(ncsi_dev_list); DEFINE_SPINLOCK(ncsi_dev_lock); +bool ncsi_channel_has_link(struct ncsi_channel *channel) +{ + return !!(channel->modes[NCSI_MODE_LINK].data[2] & 0x1); +} + +bool ncsi_channel_is_last(struct ncsi_dev_priv *ndp, + struct ncsi_channel *channel) +{ + struct ncsi_package *np; + struct ncsi_channel *nc; + + NCSI_FOR_EACH_PACKAGE(ndp, np) + NCSI_FOR_EACH_CHANNEL(np, nc) { + if (nc == channel) + continue; + if (nc->state == NCSI_CHANNEL_ACTIVE && + ncsi_channel_has_link(nc)) + return false; + } + + return true; +} + static void ncsi_report_link(struct ncsi_dev_priv *ndp, bool force_down) { struct ncsi_dev *nd = &ndp->ndev; @@ -52,7 +75,7 @@ static void ncsi_report_link(struct ncsi_dev_priv *ndp, bool force_down) continue; } - if (nc->modes[NCSI_MODE_LINK].data[2] & 0x1) { + if (ncsi_channel_has_link(nc)) { spin_unlock_irqrestore(&nc->lock, flags); nd->link_up = 1; goto report; @@ -267,6 +290,7 @@ struct ncsi_package *ncsi_add_package(struct ncsi_dev_priv *ndp, np->ndp = ndp; spin_lock_init(&np->lock); INIT_LIST_HEAD(&np->channels); + np->channel_whitelist = UINT_MAX; spin_lock_irqsave(&ndp->lock, flags); tmp = ncsi_find_package(ndp, id); @@ -728,13 +752,144 @@ static int ncsi_gma_handler(struct ncsi_cmd_arg *nca, unsigned int mf_id) #endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */ +/* Determine if a given channel from the channel_queue should be used for Tx */ +static bool ncsi_channel_is_tx(struct ncsi_dev_priv *ndp, + struct ncsi_channel *nc) +{ + struct ncsi_channel_mode *ncm; + struct ncsi_channel *channel; + struct ncsi_package *np; + + /* Check if any other channel has Tx enabled; a channel may have already + * been configured and removed from the channel queue. + */ + NCSI_FOR_EACH_PACKAGE(ndp, np) { + if (!ndp->multi_package && np != nc->package) + continue; + NCSI_FOR_EACH_CHANNEL(np, channel) { + ncm = &channel->modes[NCSI_MODE_TX_ENABLE]; + if (ncm->enable) + return false; + } + } + + /* This channel is the preferred channel and has link */ + list_for_each_entry_rcu(channel, &ndp->channel_queue, link) { + np = channel->package; + if (np->preferred_channel && + ncsi_channel_has_link(np->preferred_channel)) { + return np->preferred_channel == nc; + } + } + + /* This channel has link */ + if (ncsi_channel_has_link(nc)) + return true; + + list_for_each_entry_rcu(channel, &ndp->channel_queue, link) + if (ncsi_channel_has_link(channel)) + return false; + + /* No other channel has link; default to this one */ + return true; +} + +/* Change the active Tx channel in a multi-channel setup */ +int ncsi_update_tx_channel(struct ncsi_dev_priv *ndp, + struct ncsi_package *package, + struct ncsi_channel *disable, + struct ncsi_channel *enable) +{ + struct ncsi_cmd_arg nca; + struct ncsi_channel *nc; + struct ncsi_package *np; + int ret = 0; + + if (!package->multi_channel && !ndp->multi_package) + netdev_warn(ndp->ndev.dev, + "NCSI: Trying to update Tx channel in single-channel mode\n"); + nca.ndp = ndp; + nca.req_flags = 0; + + /* Find current channel with Tx enabled */ + NCSI_FOR_EACH_PACKAGE(ndp, np) { + if (disable) + break; + if (!ndp->multi_package && np != package) + continue; + + NCSI_FOR_EACH_CHANNEL(np, nc) + if (nc->modes[NCSI_MODE_TX_ENABLE].enable) { + disable = nc; + break; + } + } + + /* Find a suitable channel for Tx */ + NCSI_FOR_EACH_PACKAGE(ndp, np) { + if (enable) + break; + if (!ndp->multi_package && np != package) + continue; + if (!(ndp->package_whitelist & (0x1 << np->id))) + continue; + + if (np->preferred_channel && + ncsi_channel_has_link(np->preferred_channel)) { + enable = np->preferred_channel; + break; + } + + 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; + } + } + } + + if (disable == enable) + return -1; + + if (!enable) + return -1; + + if (disable) { + nca.channel = disable->id; + nca.package = disable->package->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); + } + + netdev_info(ndp->ndev.dev, "NCSI: channel %u enables Tx\n", enable->id); + + nca.channel = enable->id; + nca.package = enable->package->id; + nca.type = NCSI_PKT_CMD_ECNT; + ret = ncsi_xmit_cmd(&nca); + if (ret) + netdev_err(ndp->ndev.dev, + "Error %d sending ECNT\n", + ret); + + return ret; +} + static void ncsi_configure_channel(struct ncsi_dev_priv *ndp) { - struct ncsi_dev *nd = &ndp->ndev; - struct net_device *dev = nd->dev; struct ncsi_package *np = ndp->active_package; struct ncsi_channel *nc = ndp->active_channel; struct ncsi_channel *hot_nc = NULL; + struct ncsi_dev *nd = &ndp->ndev; + struct net_device *dev = nd->dev; struct ncsi_cmd_arg nca; unsigned char index; unsigned long flags; @@ -856,20 +1011,29 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp) } else if (nd->state == ncsi_dev_state_config_ebf) { nca.type = NCSI_PKT_CMD_EBF; nca.dwords[0] = nc->caps[NCSI_CAP_BC].cap; - nd->state = ncsi_dev_state_config_ecnt; + if (ncsi_channel_is_tx(ndp, nc)) + nd->state = ncsi_dev_state_config_ecnt; + else + nd->state = ncsi_dev_state_config_ec; #if IS_ENABLED(CONFIG_IPV6) if (ndp->inet6_addr_num > 0 && (nc->caps[NCSI_CAP_GENERIC].cap & NCSI_CAP_GENERIC_MC)) nd->state = ncsi_dev_state_config_egmf; - else - nd->state = ncsi_dev_state_config_ecnt; } else if (nd->state == ncsi_dev_state_config_egmf) { nca.type = NCSI_PKT_CMD_EGMF; nca.dwords[0] = nc->caps[NCSI_CAP_MC].cap; - nd->state = ncsi_dev_state_config_ecnt; + if (ncsi_channel_is_tx(ndp, nc)) + nd->state = ncsi_dev_state_config_ecnt; + else + nd->state = ncsi_dev_state_config_ec; #endif /* CONFIG_IPV6 */ } else if (nd->state == ncsi_dev_state_config_ecnt) { + if (np->preferred_channel && + nc != np->preferred_channel) + netdev_info(ndp->ndev.dev, + "NCSI: Tx failed over to channel %u\n", + nc->id); nca.type = NCSI_PKT_CMD_ECNT; nd->state = ncsi_dev_state_config_ec; } else if (nd->state == ncsi_dev_state_config_ec) { @@ -960,43 +1124,35 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp) static int ncsi_choose_active_channel(struct ncsi_dev_priv *ndp) { - struct ncsi_package *np, *force_package; - struct ncsi_channel *nc, *found, *hot_nc, *force_channel; + struct ncsi_channel *nc, *found, *hot_nc; struct ncsi_channel_mode *ncm; - unsigned long flags; + unsigned long flags, cflags; + struct ncsi_package *np; + bool with_link; spin_lock_irqsave(&ndp->lock, flags); hot_nc = ndp->hot_channel; - force_channel = ndp->force_channel; - force_package = ndp->force_package; spin_unlock_irqrestore(&ndp->lock, flags); - /* Force a specific channel whether or not it has link if we have been - * configured to do so - */ - if (force_package && force_channel) { - found = force_channel; - ncm = &found->modes[NCSI_MODE_LINK]; - if (!(ncm->data[2] & 0x1)) - netdev_info(ndp->ndev.dev, - "NCSI: Channel %u forced, but it is link down\n", - found->id); - goto out; - } - - /* The search is done once an inactive channel with up - * link is found. + /* By default the search is done once an inactive channel with up + * link is found, unless a preferred channel is set. + * If multi_package or multi_channel are configured all channels in the + * whitelist are added to the channel queue. */ found = NULL; + with_link = false; NCSI_FOR_EACH_PACKAGE(ndp, np) { - if (ndp->force_package && np != ndp->force_package) + if (!(ndp->package_whitelist & (0x1 << np->id))) continue; NCSI_FOR_EACH_CHANNEL(np, nc) { - spin_lock_irqsave(&nc->lock, flags); + if (!(np->channel_whitelist & (0x1 << nc->id))) + continue; + + spin_lock_irqsave(&nc->lock, cflags); if (!list_empty(&nc->link) || nc->state != NCSI_CHANNEL_INACTIVE) { - spin_unlock_irqrestore(&nc->lock, flags); + spin_unlock_irqrestore(&nc->lock, cflags); continue; } @@ -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; } + if (with_link && !ndp->multi_package) + break; } - 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); } @@ -1518,6 +1691,7 @@ struct ncsi_dev *ncsi_register_dev(struct net_device *dev, INIT_LIST_HEAD(&ndp->channel_queue); INIT_LIST_HEAD(&ndp->vlan_vids); INIT_WORK(&ndp->work, ncsi_dev_work); + ndp->package_whitelist = UINT_MAX; /* Initialize private NCSI device */ spin_lock_init(&ndp->lock); diff --git a/net/ncsi/ncsi-netlink.c b/net/ncsi/ncsi-netlink.c index cde48fe43dba..5d782445d2fc 100644 --- a/net/ncsi/ncsi-netlink.c +++ b/net/ncsi/ncsi-netlink.c @@ -30,6 +30,9 @@ static const struct nla_policy ncsi_genl_policy[NCSI_ATTR_MAX + 1] = { [NCSI_ATTR_PACKAGE_ID] = { .type = NLA_U32 }, [NCSI_ATTR_CHANNEL_ID] = { .type = NLA_U32 }, [NCSI_ATTR_DATA] = { .type = NLA_BINARY, .len = 2048 }, + [NCSI_ATTR_MULTI_FLAG] = { .type = NLA_FLAG }, + [NCSI_ATTR_PACKAGE_MASK] = { .type = NLA_U32 }, + [NCSI_ATTR_CHANNEL_MASK] = { .type = NLA_U32 }, }; static struct ncsi_dev_priv *ndp_from_ifindex(struct net *net, u32 ifindex) @@ -69,7 +72,7 @@ static int ncsi_write_channel_info(struct sk_buff *skb, nla_put_u32(skb, NCSI_CHANNEL_ATTR_LINK_STATE, m->data[2]); if (nc->state == NCSI_CHANNEL_ACTIVE) nla_put_flag(skb, NCSI_CHANNEL_ATTR_ACTIVE); - if (ndp->force_channel == nc) + if (nc == nc->package->preferred_channel) nla_put_flag(skb, NCSI_CHANNEL_ATTR_FORCED); nla_put_u32(skb, NCSI_CHANNEL_ATTR_VERSION_MAJOR, nc->version.version); @@ -114,7 +117,7 @@ static int ncsi_write_package_info(struct sk_buff *skb, if (!pnest) return -ENOMEM; nla_put_u32(skb, NCSI_PKG_ATTR_ID, np->id); - if (ndp->force_package == np) + if ((0x1 << np->id) == ndp->package_whitelist) nla_put_flag(skb, NCSI_PKG_ATTR_FORCED); cnest = nla_nest_start(skb, NCSI_PKG_ATTR_CHANNEL_LIST); if (!cnest) { @@ -290,45 +293,54 @@ static int ncsi_set_interface_nl(struct sk_buff *msg, struct genl_info *info) package_id = nla_get_u32(info->attrs[NCSI_ATTR_PACKAGE_ID]); package = NULL; - spin_lock_irqsave(&ndp->lock, flags); - NCSI_FOR_EACH_PACKAGE(ndp, np) if (np->id == package_id) package = np; if (!package) { /* The user has set a package that does not exist */ - spin_unlock_irqrestore(&ndp->lock, flags); return -ERANGE; } channel = NULL; - if (!info->attrs[NCSI_ATTR_CHANNEL_ID]) { - /* Allow any channel */ - channel_id = NCSI_RESERVED_CHANNEL; - } else { + if (info->attrs[NCSI_ATTR_CHANNEL_ID]) { channel_id = nla_get_u32(info->attrs[NCSI_ATTR_CHANNEL_ID]); NCSI_FOR_EACH_CHANNEL(package, nc) - if (nc->id == channel_id) + if (nc->id == channel_id) { channel = nc; + break; + } + if (!channel) { + netdev_info(ndp->ndev.dev, + "NCSI: Channel %u does not exist!\n", + channel_id); + return -ERANGE; + } } - if (channel_id != NCSI_RESERVED_CHANNEL && !channel) { - /* The user has set a channel that does not exist on this - * package - */ - spin_unlock_irqrestore(&ndp->lock, flags); - netdev_info(ndp->ndev.dev, "NCSI: Channel %u does not exist!\n", - channel_id); - return -ERANGE; - } - - ndp->force_package = package; - ndp->force_channel = channel; + spin_lock_irqsave(&ndp->lock, flags); + ndp->package_whitelist = 0x1 << package->id; + ndp->multi_package = false; spin_unlock_irqrestore(&ndp->lock, flags); - netdev_info(ndp->ndev.dev, "Set package 0x%x, channel 0x%x%s as preferred\n", - package_id, channel_id, - channel_id == NCSI_RESERVED_CHANNEL ? " (any)" : ""); + spin_lock_irqsave(&package->lock, flags); + package->multi_channel = false; + if (channel) { + package->channel_whitelist = 0x1 << channel->id; + package->preferred_channel = channel; + } else { + /* Allow any channel */ + package->channel_whitelist = UINT_MAX; + package->preferred_channel = NULL; + } + spin_unlock_irqrestore(&package->lock, flags); + + if (channel) + netdev_info(ndp->ndev.dev, + "Set package 0x%x, channel 0x%x as preferred\n", + package_id, channel_id); + else + netdev_info(ndp->ndev.dev, "Set package 0x%x as preferred\n", + package_id); /* Update channel configuration */ if (!(ndp->flags & NCSI_DEV_RESET)) @@ -340,6 +352,7 @@ static int ncsi_set_interface_nl(struct sk_buff *msg, struct genl_info *info) static int ncsi_clear_interface_nl(struct sk_buff *msg, struct genl_info *info) { struct ncsi_dev_priv *ndp; + struct ncsi_package *np; unsigned long flags; if (!info || !info->attrs) @@ -353,11 +366,19 @@ static int ncsi_clear_interface_nl(struct sk_buff *msg, struct genl_info *info) if (!ndp) return -ENODEV; - /* Clear any override */ + /* Reset any whitelists and disable multi mode */ spin_lock_irqsave(&ndp->lock, flags); - ndp->force_package = NULL; - ndp->force_channel = NULL; + ndp->package_whitelist = UINT_MAX; + ndp->multi_package = false; spin_unlock_irqrestore(&ndp->lock, flags); + + NCSI_FOR_EACH_PACKAGE(ndp, np) { + spin_lock_irqsave(&np->lock, flags); + np->multi_channel = false; + np->channel_whitelist = UINT_MAX; + np->preferred_channel = NULL; + spin_unlock_irqrestore(&np->lock, flags); + } netdev_info(ndp->ndev.dev, "NCSI: Cleared preferred package/channel\n"); /* Update channel configuration */ @@ -563,6 +584,138 @@ int ncsi_send_netlink_err(struct net_device *dev, return nlmsg_unicast(net->genl_sock, skb, snd_portid); } +static int ncsi_set_package_mask_nl(struct sk_buff *msg, + struct genl_info *info) +{ + struct ncsi_dev_priv *ndp; + unsigned long flags; + int rc; + + if (!info || !info->attrs) + return -EINVAL; + + if (!info->attrs[NCSI_ATTR_IFINDEX]) + return -EINVAL; + + if (!info->attrs[NCSI_ATTR_PACKAGE_MASK]) + return -EINVAL; + + ndp = ndp_from_ifindex(get_net(sock_net(msg->sk)), + nla_get_u32(info->attrs[NCSI_ATTR_IFINDEX])); + if (!ndp) + return -ENODEV; + + spin_lock_irqsave(&ndp->lock, flags); + if (nla_get_flag(info->attrs[NCSI_ATTR_MULTI_FLAG])) { + if (ndp->flags & NCSI_DEV_HWA) { + ndp->multi_package = true; + rc = 0; + } else { + netdev_err(ndp->ndev.dev, + "NCSI: Can't use multiple packages without HWA\n"); + rc = -EPERM; + } + } else { + ndp->multi_package = false; + rc = 0; + } + + if (!rc) + ndp->package_whitelist = + nla_get_u32(info->attrs[NCSI_ATTR_PACKAGE_MASK]); + spin_unlock_irqrestore(&ndp->lock, flags); + + if (!rc) { + /* Update channel configuration */ + if (!(ndp->flags & NCSI_DEV_RESET)) + ncsi_reset_dev(&ndp->ndev); + } + + return rc; +} + +static int ncsi_set_channel_mask_nl(struct sk_buff *msg, + struct genl_info *info) +{ + struct ncsi_package *np, *package; + struct ncsi_channel *nc, *channel; + u32 package_id, channel_id; + struct ncsi_dev_priv *ndp; + unsigned long flags; + + if (!info || !info->attrs) + return -EINVAL; + + if (!info->attrs[NCSI_ATTR_IFINDEX]) + return -EINVAL; + + if (!info->attrs[NCSI_ATTR_PACKAGE_ID]) + return -EINVAL; + + if (!info->attrs[NCSI_ATTR_CHANNEL_MASK]) + return -EINVAL; + + ndp = ndp_from_ifindex(get_net(sock_net(msg->sk)), + nla_get_u32(info->attrs[NCSI_ATTR_IFINDEX])); + if (!ndp) + return -ENODEV; + + package_id = nla_get_u32(info->attrs[NCSI_ATTR_PACKAGE_ID]); + package = NULL; + NCSI_FOR_EACH_PACKAGE(ndp, np) + if (np->id == package_id) { + package = np; + break; + } + if (!package) + return -ERANGE; + + spin_lock_irqsave(&package->lock, flags); + + channel = NULL; + if (info->attrs[NCSI_ATTR_CHANNEL_ID]) { + channel_id = nla_get_u32(info->attrs[NCSI_ATTR_CHANNEL_ID]); + NCSI_FOR_EACH_CHANNEL(np, nc) + if (nc->id == channel_id) { + channel = nc; + break; + } + if (!channel) { + spin_unlock_irqrestore(&package->lock, flags); + return -ERANGE; + } + netdev_dbg(ndp->ndev.dev, + "NCSI: Channel %u set as preferred channel\n", + channel->id); + } + + package->channel_whitelist = + nla_get_u32(info->attrs[NCSI_ATTR_CHANNEL_MASK]); + if (package->channel_whitelist == 0) + netdev_dbg(ndp->ndev.dev, + "NCSI: Package %u set to all channels disabled\n", + package->id); + + package->preferred_channel = channel; + + if (nla_get_flag(info->attrs[NCSI_ATTR_MULTI_FLAG])) { + package->multi_channel = true; + netdev_info(ndp->ndev.dev, + "NCSI: Multi-channel enabled on package %u\n", + package_id); + } else { + package->multi_channel = false; + } + + spin_unlock_irqrestore(&package->lock, flags); + + /* Update channel configuration */ + if (!(ndp->flags & NCSI_DEV_RESET)) + ncsi_reset_dev(&ndp->ndev); + + return 0; +} + static const struct genl_ops ncsi_ops[] = { { .cmd = NCSI_CMD_PKG_INFO, @@ -589,6 +742,18 @@ static const struct genl_ops ncsi_ops[] = { .doit = ncsi_send_cmd_nl, .flags = GENL_ADMIN_PERM, }, + { + .cmd = NCSI_CMD_SET_PACKAGE_MASK, + .policy = ncsi_genl_policy, + .doit = ncsi_set_package_mask_nl, + .flags = GENL_ADMIN_PERM, + }, + { + .cmd = NCSI_CMD_SET_CHANNEL_MASK, + .policy = ncsi_genl_policy, + .doit = ncsi_set_channel_mask_nl, + .flags = GENL_ADMIN_PERM, + }, }; static struct genl_family ncsi_genl_family __ro_after_init = { diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c index 77e07ba3f493..de7737a27889 100644 --- a/net/ncsi/ncsi-rsp.c +++ b/net/ncsi/ncsi-rsp.c @@ -256,7 +256,7 @@ static int ncsi_rsp_handler_dcnt(struct ncsi_request *nr) if (!ncm->enable) return 0; - ncm->enable = 1; + ncm->enable = 0; return 0; }
This patch extends the ncsi-netlink interface with two new commands and three new attributes to configure multiple packages and/or channels at once, and configure specific failover modes. NCSI_CMD_SET_PACKAGE mask and NCSI_CMD_SET_CHANNEL_MASK set a whitelist of packages or channels allowed to be configured with the NCSI_ATTR_PACKAGE_MASK and NCSI_ATTR_CHANNEL_MASK attributes respectively. If one of these whitelists is set only packages or channels matching the whitelist are considered for the channel queue in ncsi_choose_active_channel(). These commands may also use the NCSI_ATTR_MULTI_FLAG to signal that multiple packages or channels may be configured simultaneously. NCSI hardware arbitration (HWA) must be available in order to enable multi-package mode. Multi-channel mode is always available. If the NCSI_ATTR_CHANNEL_ID attribute is present in the NCSI_CMD_SET_CHANNEL_MASK command the it sets the preferred channel as with the NCSI_CMD_SET_INTERFACE command. The combination of preferred channel and channel whitelist defines a primary channel and the allowed failover channels. If the NCSI_ATTR_MULTI_FLAG attribute is also present then the preferred channel is configured for Tx/Rx and the other channels are enabled only for Rx. Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com> --- include/uapi/linux/ncsi.h | 15 +++ net/ncsi/internal.h | 16 ++- net/ncsi/ncsi-aen.c | 60 +++++++-- net/ncsi/ncsi-manage.c | 264 +++++++++++++++++++++++++++++++------- net/ncsi/ncsi-netlink.c | 221 +++++++++++++++++++++++++++---- net/ncsi/ncsi-rsp.c | 2 +- 6 files changed, 490 insertions(+), 88 deletions(-)