* [PATCH net-next 1/2] net/ncsi: Improve general state logging
@ 2017-11-08 5:30 Samuel Mendoza-Jonas
2017-11-08 5:30 ` [PATCH net-next 2/2] net/ncsi: Don't return error on normal response Samuel Mendoza-Jonas
2017-11-11 6:10 ` [PATCH net-next 1/2] net/ncsi: Improve general state logging David Miller
0 siblings, 2 replies; 4+ messages in thread
From: Samuel Mendoza-Jonas @ 2017-11-08 5:30 UTC (permalink / raw)
To: netdev; +Cc: Samuel Mendoza-Jonas, David S . Miller, linux-kernel
The NCSI driver is mostly silent which becomes a headache when trying to
determine what has occurred on the NCSI connection. This adds additional
logging in a few key areas such as state transitions and calling out
certain errors more visibly.
Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
---
net/ncsi/ncsi-aen.c | 15 +++++++++-
net/ncsi/ncsi-manage.c | 76 +++++++++++++++++++++++++++++++++++++-------------
net/ncsi/ncsi-rsp.c | 10 ++++++-
3 files changed, 80 insertions(+), 21 deletions(-)
diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c
index f135938bf781..67e708e98ccf 100644
--- a/net/ncsi/ncsi-aen.c
+++ b/net/ncsi/ncsi-aen.c
@@ -73,6 +73,9 @@ static int ncsi_aen_handler_lsc(struct ncsi_dev_priv *ndp,
ncm->data[2] = data;
ncm->data[4] = ntohl(lsc->oem_status);
+ netdev_info(ndp->ndev.dev, "NCSI: LSC AEN - channel %u state %s\n",
+ nc->id, data & 0x1 ? "up" : "down");
+
chained = !list_empty(&nc->link);
state = nc->state;
spin_unlock_irqrestore(&nc->lock, flags);
@@ -145,6 +148,8 @@ static int ncsi_aen_handler_hncdsc(struct ncsi_dev_priv *ndp,
ncm = &nc->modes[NCSI_MODE_LINK];
hncdsc = (struct ncsi_aen_hncdsc_pkt *)h;
ncm->data[3] = ntohl(hncdsc->status);
+ netdev_info(ndp->ndev.dev, "NCSI: HNCDSC AEN - channel %u state %s\n",
+ nc->id, ncm->data[3] & 0x3 ? "up" : "down");
if (!list_empty(&nc->link) ||
nc->state != NCSI_CHANNEL_ACTIVE) {
spin_unlock_irqrestore(&nc->lock, flags);
@@ -212,10 +217,18 @@ int ncsi_aen_handler(struct ncsi_dev_priv *ndp, struct sk_buff *skb)
}
ret = ncsi_validate_aen_pkt(h, nah->payload);
- if (ret)
+ if (ret) {
+ netdev_warn(ndp->ndev.dev,
+ "NCSI: 'bad' packet ignored for AEN type 0x%x\n",
+ h->type);
goto out;
+ }
ret = nah->handler(ndp, h);
+ if (ret)
+ netdev_err(ndp->ndev.dev,
+ "NCSI: Handler for AEN type 0x%x returned %d\n",
+ h->type, ret);
out:
consume_skb(skb);
return ret;
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 47baf914eec2..a2b904a718c6 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -229,6 +229,8 @@ static void ncsi_channel_monitor(unsigned long data)
case NCSI_CHANNEL_MONITOR_WAIT ... NCSI_CHANNEL_MONITOR_WAIT_MAX:
break;
default:
+ netdev_err(ndp->ndev.dev, "NCSI Channel %d timed out!\n",
+ nc->id);
if (!(ndp->flags & NCSI_DEV_HWA)) {
ncsi_report_link(ndp, true);
ndp->flags |= NCSI_DEV_RESHUFFLE;
@@ -682,7 +684,7 @@ static int clear_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
data = ncsi_get_filter(nc, NCSI_FILTER_VLAN, index);
if (!data) {
netdev_err(ndp->ndev.dev,
- "ncsi: failed to retrieve filter %d\n", index);
+ "NCSI: failed to retrieve filter %d\n", index);
/* Set the VLAN id to 0 - this will still disable the entry in
* the filter table, but we won't know what it was.
*/
@@ -692,7 +694,7 @@ static int clear_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
}
netdev_printk(KERN_DEBUG, ndp->ndev.dev,
- "ncsi: removed vlan tag %u at index %d\n",
+ "NCSI: removed vlan tag %u at index %d\n",
vid, index + 1);
ncsi_remove_filter(nc, NCSI_FILTER_VLAN, index);
@@ -718,7 +720,7 @@ static int set_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
if (index < 0) {
/* New tag to add */
netdev_printk(KERN_DEBUG, ndp->ndev.dev,
- "ncsi: new vlan id to set: %u\n",
+ "NCSI: new vlan id to set: %u\n",
vlan->vid);
break;
}
@@ -745,7 +747,7 @@ static int set_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
}
netdev_printk(KERN_DEBUG, ndp->ndev.dev,
- "ncsi: set vid %u in packet, index %u\n",
+ "NCSI: set vid %u in packet, index %u\n",
vlan->vid, index + 1);
nca->type = NCSI_PKT_CMD_SVF;
nca->words[1] = vlan->vid;
@@ -784,8 +786,11 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
nca.package = np->id;
nca.channel = NCSI_RESERVED_CHANNEL;
ret = ncsi_xmit_cmd(&nca);
- if (ret)
+ if (ret) {
+ netdev_err(ndp->ndev.dev,
+ "NCSI: Failed to transmit CMD_SP\n");
goto error;
+ }
nd->state = ncsi_dev_state_config_cis;
break;
@@ -797,8 +802,11 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
nca.package = np->id;
nca.channel = nc->id;
ret = ncsi_xmit_cmd(&nca);
- if (ret)
+ if (ret) {
+ netdev_err(ndp->ndev.dev,
+ "NCSI: Failed to transmit CMD_CIS\n");
goto error;
+ }
nd->state = ncsi_dev_state_config_clear_vids;
break;
@@ -895,10 +903,16 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
}
ret = ncsi_xmit_cmd(&nca);
- if (ret)
+ if (ret) {
+ netdev_err(ndp->ndev.dev,
+ "NCSI: Failed to transmit CMD %x\n",
+ nca.type);
goto error;
+ }
break;
case ncsi_dev_state_config_done:
+ netdev_printk(KERN_DEBUG, ndp->ndev.dev,
+ "NCSI: channel %u config done\n", nc->id);
spin_lock_irqsave(&nc->lock, flags);
if (nc->reconfigure_needed) {
/* This channel's configuration has been updated
@@ -925,6 +939,9 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
} else {
hot_nc = NULL;
nc->state = NCSI_CHANNEL_INACTIVE;
+ netdev_warn(ndp->ndev.dev,
+ "NCSI: channel %u link down after config\n",
+ nc->id);
}
spin_unlock_irqrestore(&nc->lock, flags);
@@ -937,8 +954,8 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
ncsi_process_next_channel(ndp);
break;
default:
- netdev_warn(dev, "Wrong NCSI state 0x%x in config\n",
- nd->state);
+ netdev_alert(dev, "Wrong NCSI state 0x%x in config\n",
+ nd->state);
}
return;
@@ -990,10 +1007,17 @@ static int ncsi_choose_active_channel(struct ncsi_dev_priv *ndp)
}
if (!found) {
+ netdev_warn(ndp->ndev.dev,
+ "NCSI: No channel found with link\n");
ncsi_report_link(ndp, true);
return -ENODEV;
}
+ ncm = &found->modes[NCSI_MODE_LINK];
+ netdev_printk(KERN_DEBUG, 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);
@@ -1055,6 +1079,8 @@ static int ncsi_enable_hwa(struct ncsi_dev_priv *ndp)
/* We can have no channels in extremely case */
if (list_empty(&ndp->channel_queue)) {
+ netdev_err(ndp->ndev.dev,
+ "NCSI: No available channels for HWA\n");
ncsi_report_link(ndp, false);
return -ENOENT;
}
@@ -1223,6 +1249,9 @@ static void ncsi_probe_channel(struct ncsi_dev_priv *ndp)
return;
error:
+ netdev_err(ndp->ndev.dev,
+ "NCSI: Failed to transmit cmd 0x%x during probe\n",
+ nca.type);
ncsi_report_link(ndp, true);
}
@@ -1276,10 +1305,14 @@ int ncsi_process_next_channel(struct ncsi_dev_priv *ndp)
switch (old_state) {
case NCSI_CHANNEL_INACTIVE:
ndp->ndev.state = ncsi_dev_state_config;
+ netdev_info(ndp->ndev.dev, "NCSI: configuring channel %u\n",
+ nc->id);
ncsi_configure_channel(ndp);
break;
case NCSI_CHANNEL_ACTIVE:
ndp->ndev.state = ncsi_dev_state_suspend;
+ netdev_info(ndp->ndev.dev, "NCSI: suspending channel %u\n",
+ nc->id);
ncsi_suspend_channel(ndp);
break;
default:
@@ -1299,6 +1332,8 @@ int ncsi_process_next_channel(struct ncsi_dev_priv *ndp)
return ncsi_choose_active_channel(ndp);
}
+ netdev_printk(KERN_DEBUG, ndp->ndev.dev,
+ "NCSI: No more channels to process\n");
ncsi_report_link(ndp, false);
return -ENODEV;
}
@@ -1390,7 +1425,7 @@ static int ncsi_kick_channels(struct ncsi_dev_priv *ndp)
ncsi_dev_state_config ||
!list_empty(&nc->link)) {
netdev_printk(KERN_DEBUG, nd->dev,
- "ncsi: channel %p marked dirty\n",
+ "NCSI: channel %p marked dirty\n",
nc);
nc->reconfigure_needed = true;
}
@@ -1410,7 +1445,7 @@ static int ncsi_kick_channels(struct ncsi_dev_priv *ndp)
spin_unlock_irqrestore(&ndp->lock, flags);
netdev_printk(KERN_DEBUG, nd->dev,
- "ncsi: kicked channel %p\n", nc);
+ "NCSI: kicked channel %p\n", nc);
n++;
}
}
@@ -1431,7 +1466,7 @@ int ncsi_vlan_rx_add_vid(struct net_device *dev, __be16 proto, u16 vid)
nd = ncsi_find_dev(dev);
if (!nd) {
- netdev_warn(dev, "ncsi: No net_device?\n");
+ netdev_warn(dev, "NCSI: No net_device?\n");
return 0;
}
@@ -1442,7 +1477,7 @@ int ncsi_vlan_rx_add_vid(struct net_device *dev, __be16 proto, u16 vid)
n_vids++;
if (vlan->vid == vid) {
netdev_printk(KERN_DEBUG, dev,
- "vid %u already registered\n", vid);
+ "NCSI: vid %u already registered\n", vid);
return 0;
}
}
@@ -1461,7 +1496,7 @@ int ncsi_vlan_rx_add_vid(struct net_device *dev, __be16 proto, u16 vid)
vlan->vid = vid;
list_add_rcu(&vlan->list, &ndp->vlan_vids);
- netdev_printk(KERN_DEBUG, dev, "Added new vid %u\n", vid);
+ netdev_printk(KERN_DEBUG, dev, "NCSI: Added new vid %u\n", vid);
found = ncsi_kick_channels(ndp) != 0;
@@ -1481,7 +1516,7 @@ int ncsi_vlan_rx_kill_vid(struct net_device *dev, __be16 proto, u16 vid)
nd = ncsi_find_dev(dev);
if (!nd) {
- netdev_warn(dev, "ncsi: no net_device?\n");
+ netdev_warn(dev, "NCSI: no net_device?\n");
return 0;
}
@@ -1491,14 +1526,14 @@ int ncsi_vlan_rx_kill_vid(struct net_device *dev, __be16 proto, u16 vid)
list_for_each_entry_safe(vlan, tmp, &ndp->vlan_vids, list)
if (vlan->vid == vid) {
netdev_printk(KERN_DEBUG, dev,
- "vid %u found, removing\n", vid);
+ "NCSI: vid %u found, removing\n", vid);
list_del_rcu(&vlan->list);
found = true;
kfree(vlan);
}
if (!found) {
- netdev_err(dev, "ncsi: vid %u wasn't registered!\n", vid);
+ netdev_err(dev, "NCSI: vid %u wasn't registered!\n", vid);
return -EINVAL;
}
@@ -1581,10 +1616,12 @@ int ncsi_start_dev(struct ncsi_dev *nd)
return 0;
}
- if (ndp->flags & NCSI_DEV_HWA)
+ if (ndp->flags & NCSI_DEV_HWA) {
+ netdev_info(ndp->ndev.dev, "NCSI: Enabling HWA mode\n");
ret = ncsi_enable_hwa(ndp);
- else
+ } else {
ret = ncsi_choose_active_channel(ndp);
+ }
return ret;
}
@@ -1615,6 +1652,7 @@ void ncsi_stop_dev(struct ncsi_dev *nd)
}
}
+ netdev_printk(KERN_DEBUG, ndp->ndev.dev, "NCSI: Stopping device\n");
ncsi_report_link(ndp, true);
}
EXPORT_SYMBOL_GPL(ncsi_stop_dev);
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 927dad4759d1..58186c4102f0 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -1032,11 +1032,19 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
if (payload < 0)
payload = ntohs(hdr->length);
ret = ncsi_validate_rsp_pkt(nr, payload);
- if (ret)
+ if (ret) {
+ netdev_warn(ndp->ndev.dev,
+ "NCSI: 'bad' packet ignored for type 0x%x\n",
+ hdr->type);
goto out;
+ }
/* Process the packet */
ret = nrh->handler(nr);
+ if (ret)
+ netdev_err(ndp->ndev.dev,
+ "NCSI: Handler for packet type 0x%x returned %d\n",
+ hdr->type, ret);
out:
ncsi_free_request(nr);
return ret;
--
2.14.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH net-next 2/2] net/ncsi: Don't return error on normal response
2017-11-08 5:30 [PATCH net-next 1/2] net/ncsi: Improve general state logging Samuel Mendoza-Jonas
@ 2017-11-08 5:30 ` Samuel Mendoza-Jonas
2017-11-11 6:10 ` David Miller
2017-11-11 6:10 ` [PATCH net-next 1/2] net/ncsi: Improve general state logging David Miller
1 sibling, 1 reply; 4+ messages in thread
From: Samuel Mendoza-Jonas @ 2017-11-08 5:30 UTC (permalink / raw)
To: netdev; +Cc: Samuel Mendoza-Jonas, David S . Miller, linux-kernel
Several response handlers return EBUSY if the data corresponding to the
command/response pair is already set. There is no reason to return an
error here; the channel is advertising something as enabled because we
told it to enable it, and it's possible that the feature has been
enabled previously.
Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
---
net/ncsi/ncsi-rsp.c | 31 ++++++++++++++-----------------
1 file changed, 14 insertions(+), 17 deletions(-)
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 58186c4102f0..efd933ff5570 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -146,7 +146,7 @@ static int ncsi_rsp_handler_ec(struct ncsi_request *nr)
ncm = &nc->modes[NCSI_MODE_ENABLE];
if (ncm->enable)
- return -EBUSY;
+ return 0;
ncm->enable = 1;
return 0;
@@ -173,7 +173,7 @@ static int ncsi_rsp_handler_dc(struct ncsi_request *nr)
ncm = &nc->modes[NCSI_MODE_ENABLE];
if (!ncm->enable)
- return -EBUSY;
+ return 0;
ncm->enable = 0;
return 0;
@@ -217,7 +217,7 @@ static int ncsi_rsp_handler_ecnt(struct ncsi_request *nr)
ncm = &nc->modes[NCSI_MODE_TX_ENABLE];
if (ncm->enable)
- return -EBUSY;
+ return 0;
ncm->enable = 1;
return 0;
@@ -239,7 +239,7 @@ static int ncsi_rsp_handler_dcnt(struct ncsi_request *nr)
ncm = &nc->modes[NCSI_MODE_TX_ENABLE];
if (!ncm->enable)
- return -EBUSY;
+ return 0;
ncm->enable = 1;
return 0;
@@ -263,7 +263,7 @@ static int ncsi_rsp_handler_ae(struct ncsi_request *nr)
/* Check if the AEN has been enabled */
ncm = &nc->modes[NCSI_MODE_AEN];
if (ncm->enable)
- return -EBUSY;
+ return 0;
/* Update to AEN configuration */
cmd = (struct ncsi_cmd_ae_pkt *)skb_network_header(nr->cmd);
@@ -382,7 +382,7 @@ static int ncsi_rsp_handler_ev(struct ncsi_request *nr)
/* Check if VLAN mode has been enabled */
ncm = &nc->modes[NCSI_MODE_VLAN];
if (ncm->enable)
- return -EBUSY;
+ return 0;
/* Update to VLAN mode */
cmd = (struct ncsi_cmd_ev_pkt *)skb_network_header(nr->cmd);
@@ -409,7 +409,7 @@ static int ncsi_rsp_handler_dv(struct ncsi_request *nr)
/* Check if VLAN mode has been enabled */
ncm = &nc->modes[NCSI_MODE_VLAN];
if (!ncm->enable)
- return -EBUSY;
+ return 0;
/* Update to VLAN mode */
ncm->enable = 0;
@@ -455,13 +455,10 @@ static int ncsi_rsp_handler_sma(struct ncsi_request *nr)
bitmap = &ncf->bitmap;
if (cmd->at_e & 0x1) {
- if (test_and_set_bit(cmd->index, bitmap))
- return -EBUSY;
+ set_bit(cmd->index, bitmap);
memcpy(ncf->data + 6 * cmd->index, cmd->mac, 6);
} else {
- if (!test_and_clear_bit(cmd->index, bitmap))
- return -EBUSY;
-
+ clear_bit(cmd->index, bitmap);
memset(ncf->data + 6 * cmd->index, 0, 6);
}
@@ -485,7 +482,7 @@ static int ncsi_rsp_handler_ebf(struct ncsi_request *nr)
/* Check if broadcast filter has been enabled */
ncm = &nc->modes[NCSI_MODE_BC];
if (ncm->enable)
- return -EBUSY;
+ return 0;
/* Update to broadcast filter mode */
cmd = (struct ncsi_cmd_ebf_pkt *)skb_network_header(nr->cmd);
@@ -511,7 +508,7 @@ static int ncsi_rsp_handler_dbf(struct ncsi_request *nr)
/* Check if broadcast filter isn't enabled */
ncm = &nc->modes[NCSI_MODE_BC];
if (!ncm->enable)
- return -EBUSY;
+ return 0;
/* Update to broadcast filter mode */
ncm->enable = 0;
@@ -538,7 +535,7 @@ static int ncsi_rsp_handler_egmf(struct ncsi_request *nr)
/* Check if multicast filter has been enabled */
ncm = &nc->modes[NCSI_MODE_MC];
if (ncm->enable)
- return -EBUSY;
+ return 0;
/* Update to multicast filter mode */
cmd = (struct ncsi_cmd_egmf_pkt *)skb_network_header(nr->cmd);
@@ -564,7 +561,7 @@ static int ncsi_rsp_handler_dgmf(struct ncsi_request *nr)
/* Check if multicast filter has been enabled */
ncm = &nc->modes[NCSI_MODE_MC];
if (!ncm->enable)
- return -EBUSY;
+ return 0;
/* Update to multicast filter mode */
ncm->enable = 0;
@@ -591,7 +588,7 @@ static int ncsi_rsp_handler_snfc(struct ncsi_request *nr)
/* Check if flow control has been enabled */
ncm = &nc->modes[NCSI_MODE_FC];
if (ncm->enable)
- return -EBUSY;
+ return 0;
/* Update to flow control mode */
cmd = (struct ncsi_cmd_snfc_pkt *)skb_network_header(nr->cmd);
--
2.14.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net-next 1/2] net/ncsi: Improve general state logging
2017-11-08 5:30 [PATCH net-next 1/2] net/ncsi: Improve general state logging Samuel Mendoza-Jonas
2017-11-08 5:30 ` [PATCH net-next 2/2] net/ncsi: Don't return error on normal response Samuel Mendoza-Jonas
@ 2017-11-11 6:10 ` David Miller
1 sibling, 0 replies; 4+ messages in thread
From: David Miller @ 2017-11-11 6:10 UTC (permalink / raw)
To: sam; +Cc: netdev, linux-kernel
From: Samuel Mendoza-Jonas <sam@mendozajonas.com>
Date: Wed, 8 Nov 2017 16:30:44 +1100
> The NCSI driver is mostly silent which becomes a headache when trying to
> determine what has occurred on the NCSI connection. This adds additional
> logging in a few key areas such as state transitions and calling out
> certain errors more visibly.
>
> Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
Applied.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next 2/2] net/ncsi: Don't return error on normal response
2017-11-08 5:30 ` [PATCH net-next 2/2] net/ncsi: Don't return error on normal response Samuel Mendoza-Jonas
@ 2017-11-11 6:10 ` David Miller
0 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2017-11-11 6:10 UTC (permalink / raw)
To: sam; +Cc: netdev, linux-kernel
From: Samuel Mendoza-Jonas <sam@mendozajonas.com>
Date: Wed, 8 Nov 2017 16:30:45 +1100
> Several response handlers return EBUSY if the data corresponding to the
> command/response pair is already set. There is no reason to return an
> error here; the channel is advertising something as enabled because we
> told it to enable it, and it's possible that the feature has been
> enabled previously.
>
> Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
Applied.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-11-11 6:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-08 5:30 [PATCH net-next 1/2] net/ncsi: Improve general state logging Samuel Mendoza-Jonas
2017-11-08 5:30 ` [PATCH net-next 2/2] net/ncsi: Don't return error on normal response Samuel Mendoza-Jonas
2017-11-11 6:10 ` David Miller
2017-11-11 6:10 ` [PATCH net-next 1/2] net/ncsi: Improve general state logging David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.