* Re: [PATCH net-next v3 5/6] devlink: hold a reference to the netdevice around ethtool compat
From: Jiri Pirko @ 2019-02-24 11:00 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, mkubecek, andrew, f.fainelli, netdev, oss-drivers
In-Reply-To: <20190222220758.7117-6-jakub.kicinski@netronome.com>
Fri, Feb 22, 2019 at 11:07:56PM CET, jakub.kicinski@netronome.com wrote:
>When ethtool is calling into devlink compat code make sure we have
>a reference on the netdevice on which the operation was invoked.
>
>v3: move the hold/lock logic into devlink_compat_* functions (Florian)
>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>---
> net/core/devlink.c | 34 +++++++++++++++++++++++-----------
> net/core/ethtool.c | 13 ++-----------
> 2 files changed, 25 insertions(+), 22 deletions(-)
>
>diff --git a/net/core/devlink.c b/net/core/devlink.c
>index a13055160be0..78c6ea1870ca 100644
>--- a/net/core/devlink.c
>+++ b/net/core/devlink.c
>@@ -6430,27 +6430,39 @@ void devlink_compat_running_version(struct net_device *dev,
> {
> struct devlink *devlink;
>
>+ dev_hold(dev);
>+ rtnl_unlock();
Ha, I got it now. You rely on dev_hold to make sure the
devlink instance does not dissappear. But until this patch, that is not
guaranteed (or I'm missing it).
>+
> devlink = netdev_to_devlink(dev);
>- if (!devlink || !devlink->ops || !devlink->ops->info_get)
>- return;
>+ if (devlink && devlink->ops && devlink->ops->info_get) {
>+ mutex_lock(&devlink->lock);
>+ __devlink_compat_running_version(devlink, buf, len);
>+ mutex_unlock(&devlink->lock);
>+ }
>
>- mutex_lock(&devlink->lock);
>- __devlink_compat_running_version(devlink, buf, len);
>- mutex_unlock(&devlink->lock);
>+ rtnl_lock();
>+ dev_put(dev);
> }
>
> int devlink_compat_flash_update(struct net_device *dev, const char *file_name)
> {
> struct devlink *devlink;
>- int ret;
>+ int ret = -EOPNOTSUPP;
>+
>+ dev_hold(dev);
>+ rtnl_unlock();
>
> devlink = netdev_to_devlink(dev);
>- if (!devlink || !devlink->ops || !devlink->ops->flash_update)
>- return -EOPNOTSUPP;
>+ if (devlink && devlink->ops && devlink->ops->flash_update) {
>+ mutex_lock(&devlink->lock);
>+ ret = devlink->ops->flash_update(devlink, file_name,
>+ NULL, NULL);
>+ mutex_unlock(&devlink->lock);
>+ }
>+
>+ rtnl_lock();
>+ dev_put(dev);
>
>- mutex_lock(&devlink->lock);
>- ret = devlink->ops->flash_update(devlink, file_name, NULL, NULL);
>- mutex_unlock(&devlink->lock);
> return ret;
> }
>
>diff --git a/net/core/ethtool.c b/net/core/ethtool.c
>index 1320e8dce559..47558ffae423 100644
>--- a/net/core/ethtool.c
>+++ b/net/core/ethtool.c
>@@ -805,11 +805,9 @@ static noinline_for_stack int ethtool_get_drvinfo(struct net_device *dev,
> if (ops->get_eeprom_len)
> info.eedump_len = ops->get_eeprom_len(dev);
>
>- rtnl_unlock();
> if (!info.fw_version[0])
> devlink_compat_running_version(dev, info.fw_version,
> sizeof(info.fw_version));
>- rtnl_lock();
>
> if (copy_to_user(useraddr, &info, sizeof(info)))
> return -EFAULT;
>@@ -2040,15 +2038,8 @@ static noinline_for_stack int ethtool_flash_device(struct net_device *dev,
> return -EFAULT;
> efl.data[ETHTOOL_FLASH_MAX_FILENAME - 1] = 0;
>
>- if (!dev->ethtool_ops->flash_device) {
>- int ret;
>-
>- rtnl_unlock();
>- ret = devlink_compat_flash_update(dev, efl.data);
>- rtnl_lock();
>-
>- return ret;
>- }
>+ if (!dev->ethtool_ops->flash_device)
>+ return devlink_compat_flash_update(dev, efl.data);
>
> return dev->ethtool_ops->flash_device(dev, &efl);
> }
>--
>2.19.2
>
^ permalink raw reply
* Re: [PATCH net-next v3 6/6] devlink: add missing NULL checks for devlink ops
From: Jiri Pirko @ 2019-02-24 11:03 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, mkubecek, andrew, f.fainelli, netdev, oss-drivers
In-Reply-To: <20190222220758.7117-7-jakub.kicinski@netronome.com>
Fri, Feb 22, 2019 at 11:07:57PM CET, jakub.kicinski@netronome.com wrote:
>Commit 76726ccb7f46 ("devlink: add flash update command") and
>commit 2d8dc5bbf4e7 ("devlink: Add support for reload")
>access devlink ops without NULL-checking. Add the missing checks.
>Note that all drivers currently implementing devlink pass non-NULL
>ops, so this is not a problem.
Wouldn't it be better to rather put WARN_ON&fail when driver calls
devlink_alloc() with NULL ops and avoid these checks in the whole code?
>
>Reported-by: Florian Fainelli <f.fainelli@gmail.com>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>---
> net/core/devlink.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/net/core/devlink.c b/net/core/devlink.c
>index 78c6ea1870ca..38cb0239dede 100644
>--- a/net/core/devlink.c
>+++ b/net/core/devlink.c
>@@ -2651,7 +2651,7 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
> struct devlink *devlink = info->user_ptr[0];
> int err;
>
>- if (!devlink->ops->reload)
>+ if (!devlink->ops || !devlink->ops->reload)
> return -EOPNOTSUPP;
>
> err = devlink_resources_validate(devlink, NULL, info);
>@@ -2669,7 +2669,7 @@ static int devlink_nl_cmd_flash_update(struct sk_buff *skb,
> const char *file_name, *component;
> struct nlattr *nla_component;
>
>- if (!devlink->ops->flash_update)
>+ if (!devlink->ops || !devlink->ops->flash_update)
> return -EOPNOTSUPP;
>
> if (!info->attrs[DEVLINK_ATTR_FLASH_UPDATE_FILE_NAME])
>--
>2.19.2
>
^ permalink raw reply
* Re: [RFC v1 10/19] RDMA/irdma: Add connection manager
From: Gal Pressman @ 2019-02-24 11:21 UTC (permalink / raw)
To: Shiraz Saleem, dledford, jgg, davem
Cc: linux-rdma, netdev, mustafa.ismail, jeffrey.t.kirsher
In-Reply-To: <20190215171107.6464-11-shiraz.saleem@intel.com>
On 15-Feb-19 19:10, Shiraz Saleem wrote:
> +/**
> + * irdma_cm_teardown_connections - teardown QPs
> + * @iwdev: device pointer
> + * @ipaddr: Pointer to IPv4 or IPv6 address
> + * @ipv4: flag indicating IPv4 when true
There is no ipv4 parameter.
> + * @disconnect_all: flag indicating disconnect all QPs
> + * teardown QPs where source or destination addr matches ip addr
> + */
> +void irdma_cm_teardown_connections(struct irdma_device *iwdev, u32 *ipaddr,
> + struct irdma_cm_info *nfo,
> + bool disconnect_all)
> +{
> + struct irdma_cm_core *cm_core = &iwdev->cm_core;
> + struct list_head *list_core_temp;
> + struct list_head *list_node;
> + struct irdma_cm_node *cm_node;
> + struct list_head teardown_list;
> + struct ib_qp_attr attr;
> + struct irdma_sc_vsi *vsi = &iwdev->vsi;
> + struct irdma_sc_qp *sc_qp;
> + struct irdma_qp *qp;
> + int i;
> + unsigned long flags;
> +
> + INIT_LIST_HEAD(&teardown_list);
> +
> + spin_lock_irqsave(&cm_core->ht_lock, flags);
> + list_for_each_safe(list_node, list_core_temp,
> + &cm_core->accelerated_list) {
> + cm_node = container_of(list_node, struct irdma_cm_node, list);
> + if (disconnect_all ||
> + (nfo->vlan_id == cm_node->vlan_id &&
> + !memcmp(cm_node->loc_addr, ipaddr, nfo->ipv4 ? 4 : 16))) {
> + atomic_inc(&cm_node->ref_count);
> + list_add(&cm_node->teardown_entry, &teardown_list);
> + }
> + }
> + list_for_each_safe(list_node, list_core_temp,
> + &cm_core->non_accelerated_list) {
> + cm_node = container_of(list_node, struct irdma_cm_node, list);
> + if (disconnect_all ||
> + (nfo->vlan_id == cm_node->vlan_id &&
> + !memcmp(cm_node->loc_addr, ipaddr, nfo->ipv4 ? 4 : 16))) {
> + atomic_inc(&cm_node->ref_count);
> + list_add(&cm_node->teardown_entry, &teardown_list);
> + }
> + }
> + spin_unlock_irqrestore(&cm_core->ht_lock, flags);
> +
> + list_for_each_safe(list_node, list_core_temp, &teardown_list) {
> + cm_node = container_of(list_node, struct irdma_cm_node,
> + teardown_entry);
> + attr.qp_state = IB_QPS_ERR;
> + irdma_modify_qp(&cm_node->iwqp->ibqp, &attr, IB_QP_STATE, NULL);
> + if (iwdev->reset)
> + irdma_cm_disconn(cm_node->iwqp);
> + irdma_rem_ref_cm_node(cm_node);
> + }
> + if (!iwdev->roce_mode)
> + return;
> +
> + INIT_LIST_HEAD(&teardown_list);
> + for (i = 0; i < IRDMA_MAX_USER_PRIORITY; i++) {
> + spin_lock_irqsave(&vsi->qos[i].lock, flags);
> + list_for_each_safe(list_node, list_core_temp, &vsi->qos[i].qplist) {
> + u32 qp_ip[4];
> +
> + sc_qp = container_of(list_node, struct irdma_sc_qp, list);
> + if (sc_qp->qp_type != IRDMA_QP_TYPE_ROCE_RC)
> + continue;
> +
> + qp = sc_qp->back_qp;
> + if (!disconnect_all) {
> + if (nfo->ipv4)
> + qp_ip[0] = qp->udp_info.local_ipaddr3;
> + else
> + memcpy(qp_ip,
> + &qp->udp_info.local_ipaddr0,
> + sizeof(qp_ip));
> + }
> +
> + if (disconnect_all ||
> + (nfo->vlan_id == qp->udp_info.vlan_tag &&
> + !memcmp(qp_ip, ipaddr, nfo->ipv4 ? 4 : 16))) {
> + spin_lock_irqsave(&iwdev->rf->qptable_lock, flags);
You should use different 'flags' here.
> + if (iwdev->rf->qp_table[sc_qp->qp_uk.qp_id]) {
> + irdma_add_ref(&qp->ibqp);
> + list_add(&qp->teardown_entry, &teardown_list);
> + }
> + spin_unlock_irqrestore(&iwdev->rf->qptable_lock, flags);
> + }
> + }
> + spin_unlock_irqrestore(&vsi->qos[i].lock, flags);
> + }
> +
> + list_for_each_safe(list_node, list_core_temp, &teardown_list) {
> + qp = container_of(list_node, struct irdma_qp, teardown_entry);
> + attr.qp_state = IB_QPS_ERR;
> + irdma_modify_qp_roce(&qp->ibqp, &attr, IB_QP_STATE, NULL);
> + irdma_rem_ref(&qp->ibqp);
> + }
> +}
^ permalink raw reply
* Re: [PATCH 1/3] bpf: add helper to check for a valid SYN cookie
From: kbuild test robot @ 2019-02-24 11:21 UTC (permalink / raw)
To: Lorenz Bauer; +Cc: kbuild-all, ast, daniel, netdev, linux-api, Lorenz Bauer
In-Reply-To: <20190222095057.9442-2-lmb@cloudflare.com>
[-- Attachment #1: Type: text/plain, Size: 1582 bytes --]
Hi Lorenz,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on bpf-next/master]
[also build test ERROR on next-20190222]
[cannot apply to v5.0-rc4]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Lorenz-Bauer/Allow-checking-SYN-cookies-from-XDP-and-tc-cls-act/20190224-180755
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: x86_64-kexec (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All error/warnings (new ones prefixed by >>):
net/core/filter.c: In function '____bpf_sk_check_syncookie':
>> net/core/filter.c:5477:10: error: 'ENOTSUP' undeclared (first use in this function); did you mean 'ENOTSUPP'?
return -ENOTSUP;
^~~~~~~
ENOTSUPP
net/core/filter.c:5477:10: note: each undeclared identifier is reported only once for each function it appears in
>> net/core/filter.c:5479:1: warning: control reaches end of non-void function [-Wreturn-type]
}
^
vim +5477 net/core/filter.c
5467
5468 default:
5469 return -EPROTONOSUPPORT;
5470 }
5471
5472 if (ret > 0)
5473 return 0;
5474
5475 return -ENOENT;
5476 #else
> 5477 return -ENOTSUP;
5478 #endif
> 5479 }
5480
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26394 bytes --]
^ permalink raw reply
* Re: [PATCH v5 0/5] M_CAN Framework re-write
From: Wolfgang Grandegger @ 2019-02-24 11:27 UTC (permalink / raw)
To: Dan Murphy, mkl, davem; +Cc: linux-can, netdev, linux-kernel
In-Reply-To: <d1a71f1f-55b8-0ff1-1507-75d7c6237d90@ti.com>
Hello Dan,
Am 22.02.19 um 18:05 schrieb Dan Murphy:
> Wolfgang
>
> On 2/22/19 6:50 AM, Dan Murphy wrote:
>> Wolfgang
>>
>> On 2/22/19 3:38 AM, Wolfgang Grandegger wrote:
>>> Hello Dan,
>>>
>>> what kernel version is that patch series for. I have problems to apply it!
>>>
>>
>> It is based off of Master
>>
>> commit 2137397c92aec3713fa10be3c9b830f9a1674e60 (linux_master/master)
>>
>> And I successfully rebased on top of
>>
>> commit 8a61716ff2ab23eddd1f7a05a075a374e4d0c3d4 (linux_master/master)
>> Merge tag 'ceph-for-5.0-rc8' of git://github.com/ceph/ceph-client
>>
>
> I just pulled these patches and they applied fine to the top commit of linux master
>
> Do I need to rebase on top of a for-next branch for you?
No, I can apply the series now as well.
Wolfgang,
^ permalink raw reply
* Re: [PATCH 1/3] bpf: add helper to check for a valid SYN cookie
From: kbuild test robot @ 2019-02-24 11:37 UTC (permalink / raw)
To: Lorenz Bauer; +Cc: kbuild-all, ast, daniel, netdev, linux-api, Lorenz Bauer
In-Reply-To: <20190222095057.9442-2-lmb@cloudflare.com>
[-- Attachment #1: Type: text/plain, Size: 1329 bytes --]
Hi Lorenz,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on bpf-next/master]
[also build test ERROR on next-20190222]
[cannot apply to v5.0-rc4]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Lorenz-Bauer/Allow-checking-SYN-cookies-from-XDP-and-tc-cls-act/20190224-180755
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: m68k-sun3_defconfig (attached as .config)
compiler: m68k-linux-gnu-gcc (Debian 8.2.0-11) 8.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.2.0 make.cross ARCH=m68k
All errors (new ones prefixed by >>):
m68k-linux-gnu-ld: drivers/rtc/proc.o: in function `is_rtc_hctosys.isra.0':
proc.c:(.text+0x178): undefined reference to `strcmp'
m68k-linux-gnu-ld: net/core/filter.o: in function `bpf_sk_check_syncookie':
>> filter.c:(.text+0x5a58): undefined reference to `__cookie_v6_check'
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 12145 bytes --]
^ permalink raw reply
* Re: [RFC v1 08/19] RDMA/irdma: Add privileged UDA queue implementation
From: Gal Pressman @ 2019-02-24 11:42 UTC (permalink / raw)
To: Shiraz Saleem, dledford, jgg, davem
Cc: linux-rdma, netdev, mustafa.ismail, jeffrey.t.kirsher
In-Reply-To: <20190215171107.6464-9-shiraz.saleem@intel.com>
On 15-Feb-19 19:10, Shiraz Saleem wrote:
> +/**
> + * irdma_puda_poll_info - poll cq for completion
> + * @cq: cq for poll
> + * @info: info return for successful completion
> + */
> +static enum irdma_status_code
> +irdma_puda_poll_info(struct irdma_sc_cq *cq, struct irdma_puda_cmpl_info *info)
> +{
> + struct irdma_cq_uk *cq_uk = &cq->cq_uk;
> + u64 qword0, qword2, qword3, qword6;
> + __le64 *cqe;
> + __le64 *ext_cqe = NULL;
> + u64 qword7 = 0;
> + u64 comp_ctx;
> + bool valid_bit;
> + bool ext_valid = 0;
> + u32 major_err, minor_err;
> + u32 peek_head;
> + bool error;
> + u8 polarity;
> +
> + cqe = IRDMA_GET_CURRENT_CQ_ELEM(&cq->cq_uk);
> + get_64bit_val(cqe, 24, &qword3);
> + valid_bit = (bool)RS_64(qword3, IRDMA_CQ_VALID);
> + if (valid_bit != cq_uk->polarity)
> + return IRDMA_ERR_Q_EMPTY;
> +
> + if (cq->dev->hw_attrs.hw_rev > IRDMA_GEN_1)
> + ext_valid = (bool)RS_64(qword3, IRDMA_CQ_EXTCQE);
> +
> + if (ext_valid) {
> + enum irdma_status_code ret = 0;
> +
> + peek_head = (cq_uk->cq_ring.head + 1) % cq_uk->cq_ring.size;
> + ext_cqe = cq_uk->cq_base[peek_head].buf;
> + get_64bit_val(ext_cqe, 24, &qword7);
> + polarity = (u8)RS_64(qword7, IRDMA_CQ_VALID);
> + if (!peek_head)
> + polarity ^= 1;
> + if (polarity != cq_uk->polarity)
> + return IRDMA_ERR_Q_EMPTY;
> +
> + IRDMA_RING_MOVE_HEAD(cq_uk->cq_ring, ret);
Should you check for 'ret' value here? It was initialized to 0 and overriden
here but I can't see any actual use for it.
> + if (IRDMA_RING_CURRENT_HEAD(cq_uk->cq_ring) == 0)
> + cq_uk->polarity = !cq_uk->polarity;
> + /* update cq tail in cq shadow memory also */
> + IRDMA_RING_MOVE_TAIL(cq_uk->cq_ring);
> + }
> +
> + irdma_debug_buf(cq->dev, IRDMA_DEBUG_PUDA, "PUDA CQE", cqe, 32);
> + if (ext_valid)
> + irdma_debug_buf(cq->dev, IRDMA_DEBUG_PUDA, "PUDA EXT-CQE",
> + ext_cqe, 32);
> +
> + error = (bool)RS_64(qword3, IRDMA_CQ_ERROR);
> + if (error) {
> + irdma_debug(cq->dev, IRDMA_DEBUG_PUDA, "receive error\n");
> + major_err = (u32)(RS_64(qword3, IRDMA_CQ_MAJERR));
> + minor_err = (u32)(RS_64(qword3, IRDMA_CQ_MINERR));
> + info->compl_error = major_err << 16 | minor_err;
> + return IRDMA_ERR_CQ_COMPL_ERROR;
> + }
> +
> + get_64bit_val(cqe, 0, &qword0);
> + get_64bit_val(cqe, 16, &qword2);
> +
> + info->q_type = (u8)RS_64(qword3, IRDMA_CQ_SQ);
> + info->qp_id = (u32)RS_64(qword2, IRDMACQ_QPID);
> + if (cq->dev->hw_attrs.hw_rev > IRDMA_GEN_1)
> + info->ipv4 = (bool)RS_64(qword3, IRDMACQ_IPV4);
> +
> + get_64bit_val(cqe, 8, &comp_ctx);
> + info->qp = (struct irdma_qp_uk *)(unsigned long)comp_ctx;
> + info->wqe_idx = (u32)RS_64(qword3, IRDMA_CQ_WQEIDX);
> +
> + if (info->q_type == IRDMA_CQE_QTYPE_RQ) {
> + if (ext_valid) {
> + info->vlan_valid = (bool)RS_64(qword7, IRDMA_CQ_UDVLANVALID);
> + if (info->vlan_valid) {
> + get_64bit_val(ext_cqe, 16, &qword6);
> + info->vlan = (u16)RS_64(qword6, IRDMA_CQ_UDVLAN);
> + }
> + info->smac_valid = (bool)RS_64(qword7, IRDMA_CQ_UDSMACVALID);
> + if (info->smac_valid) {
> + get_64bit_val(ext_cqe, 16, &qword6);
> + info->smac[0] = (u8)((qword6 >> 40) & 0xFF);
> + info->smac[1] = (u8)((qword6 >> 32) & 0xFF);
> + info->smac[2] = (u8)((qword6 >> 24) & 0xFF);
> + info->smac[3] = (u8)((qword6 >> 16) & 0xFF);
> + info->smac[4] = (u8)((qword6 >> 8) & 0xFF);
> + info->smac[5] = (u8)(qword6 & 0xFF);
> + }
> + }
> +
> + if (cq->dev->hw_attrs.hw_rev == IRDMA_GEN_1) {
> + info->vlan_valid = (bool)RS_64(qword3, IRDMA_VLAN_TAG_VALID);
> + info->l4proto = (u8)RS_64(qword2, IRDMA_UDA_L4PROTO);
> + info->l3proto = (u8)RS_64(qword2, IRDMA_UDA_L3PROTO);
> + }
> +
> + info->payload_len = (u32)RS_64(qword0, IRDMACQ_PAYLDLEN);
> + }
> +
> + return 0;
> +}
^ permalink raw reply
* [REGRESSION] v5.0-rc5 rtl8192cu breakage
From: Vito Caputo @ 2019-02-24 11:34 UTC (permalink / raw)
To: linux-kernel; +Cc: Jes.Sorensen, kvalo, davem, linux-wireless, netdev
Never seen this before v5.0-rc5, rc5 is the only v5.0 build I've booted
so far, if this is already fixed in a newer rc please let me know.
Just wanted to give a heads up since we're so close to v5.0:
WiFi stopped functioning abruptly, this appeared in dmesg:
[968853.940102] usb 2-1: USB disconnect, device number 2
[968853.949169] wlan3: deauthenticating from 0c:72:d9:27:17:57 by local choice (Reason: 3=DEAUTH_LEAVING)
[968853.949537] rtl_usb: reg 0x102, usbctrl_vendorreq TimeOut! status:0xffffffed value=0x6954341e
[968853.949552] rtl_usb: reg 0x422, usbctrl_vendorreq TimeOut! status:0xffffffed value=0x8531114
[968853.949564] rtl_usb: reg 0x542, usbctrl_vendorreq TimeOut! status:0xffffffed value=0x34085e
[968853.949583] rtl_usb: reg 0x1cc, usbctrl_vendorreq TimeOut! status:0xffffffed value=0x2e
[968854.204153] usb 2-1: new high-speed USB device number 4 using ehci-pci
[968854.335901] usb 2-1: New USB device found, idVendor=7392, idProduct=7811, bcdDevice= 2.00
[968854.335910] usb 2-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[968854.335916] usb 2-1: Product: 802.11n WLAN Adapter
[968854.335921] usb 2-1: Manufacturer: Realtek
[968854.335926] usb 2-1: SerialNumber: 00e04c000001
[968854.340632] rtl8192cu: Chip version 0x10
[968854.431999] rtl8192cu: Board Type 0
[968854.432614] rtl_usb: rx_max_size 15360, rx_urb_num 8, in_ep 1
[968854.432723] rtl8192cu: Loading firmware rtlwifi/rtl8192cufw_TMSC.bin
[968854.433043] ieee80211 phy1: Selected rate control algorithm 'rtl_rc'
[968854.488906] rtl8192cu 2-1:1.0 wlan3: renamed from wlan0
[968854.507120] rtl8192cu: MAC auto ON okay!
[968854.548252] rtl8192cu: Tx queue select: 0x05
[968855.298268] rtl8192c_common: Polling FW ready fail! REG_MCUFWDL:0x00030006.
[968855.298274] rtl8192c_common: Firmware is not ready to run!
Unplugging the USB WiFi dongle and reinserting got everything back up and
working. Only experienced this once so far and it took ~10 days to
happen, only just started using v5.0 so not much more information at
this time.
No reproducer at this time, I wasn't doing anything in particular.
Regards,
Vito Caputo
^ permalink raw reply
* [PATCH v3 iproute2-next 02/11] devlink: Fix print of uint64_t
From: Aya Levin @ 2019-02-24 12:46 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1551012378-29167-1-git-send-email-ayal@mellanox.com>
This patch prints uint64_t with its corresponding format and avoid implicit
cast to uint32_t.
Signed-off-by: Aya Levin <ayal@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
devlink/devlink.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index 71bc6239ea0b..b51380298f8a 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -1626,7 +1626,14 @@ static void pr_out_u64(struct dl *dl, const char *name, uint64_t val)
if (val == (uint64_t) -1)
return pr_out_str(dl, name, "unlimited");
- return pr_out_uint(dl, name, val);
+ if (dl->json_output) {
+ jsonw_u64_field(dl->jw, name, val);
+ } else {
+ if (g_indent_newline)
+ pr_out("%s %lu", name, val);
+ else
+ pr_out(" %s %lu", name, val);
+ }
}
static void pr_out_region_chunk_start(struct dl *dl, uint64_t addr)
--
2.14.1
^ permalink raw reply related
* [PATCH v3 iproute2-next 01/11] devlink: Refactor validation of finding required arguments
From: Aya Levin @ 2019-02-24 12:46 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1551012378-29167-1-git-send-email-ayal@mellanox.com>
Introducing argument's metadata structure matching a bitmap flag per
required argument and an error message if missing. Using this static
array to refactor validation of finding required arguments in devlink
command line and to ease further maintenance.
Signed-off-by: Aya Levin <ayal@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
devlink/devlink.c | 153 ++++++++++++++++--------------------------------------
1 file changed, 45 insertions(+), 108 deletions(-)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index 960cdda99b5b..71bc6239ea0b 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -39,6 +39,7 @@
#define PARAM_CMODE_RUNTIME_STR "runtime"
#define PARAM_CMODE_DRIVERINIT_STR "driverinit"
#define PARAM_CMODE_PERMANENT_STR "permanent"
+#define DL_ARGS_REQUIRED_MAX_ERR_LEN 80
static int g_new_line_count;
@@ -950,6 +951,49 @@ static int param_cmode_get(const char *cmodestr,
return 0;
}
+struct dl_args_metadata {
+ uint32_t o_flag;
+ char err_msg[DL_ARGS_REQUIRED_MAX_ERR_LEN];
+};
+
+static const struct dl_args_metadata dl_args_required[] = {
+ {DL_OPT_PORT_TYPE, "Port type not set."},
+ {DL_OPT_PORT_COUNT, "Port split count option expected."},
+ {DL_OPT_SB_POOL, "Pool index option expected."},
+ {DL_OPT_SB_SIZE, "Pool size option expected."},
+ {DL_OPT_SB_TYPE, "Pool type option expected."},
+ {DL_OPT_SB_THTYPE, "Pool threshold type option expected."},
+ {DL_OPT_SB_TH, "Threshold option expected."},
+ {DL_OPT_SB_TC, "TC index option expected."},
+ {DL_OPT_ESWITCH_MODE, "E-Switch mode option expected."},
+ {DL_OPT_ESWITCH_INLINE_MODE, "E-Switch inline-mode option expected."},
+ {DL_OPT_DPIPE_TABLE_NAME, "Dpipe table name expected."},
+ {DL_OPT_DPIPE_TABLE_COUNTERS, "Dpipe table counter state expected."},
+ {DL_OPT_ESWITCH_ENCAP_MODE, "E-Switch encapsulation option expected."},
+ {DL_OPT_PARAM_NAME, "Parameter name expected."},
+ {DL_OPT_PARAM_VALUE, "Value to set expected."},
+ {DL_OPT_PARAM_CMODE, "Configuration mode expected."},
+ {DL_OPT_REGION_SNAPSHOT_ID, "Region snapshot id expected."},
+ {DL_OPT_REGION_ADDRESS, "Region address value expected."},
+ {DL_OPT_REGION_LENGTH, "Region length value expected."},
+};
+
+static int dl_args_finding_required_validate(uint32_t o_required,
+ uint32_t o_found)
+{
+ uint32_t o_flag;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(dl_args_required); i++) {
+ o_flag = dl_args_required[i].o_flag;
+ if ((o_required & o_flag) && !(o_found & o_flag)) {
+ pr_err("%s\n", dl_args_required[i].err_msg);
+ return -EINVAL;
+ }
+ }
+ return 0;
+}
+
static int dl_argv_parse(struct dl *dl, uint32_t o_required,
uint32_t o_optional)
{
@@ -1198,114 +1242,7 @@ static int dl_argv_parse(struct dl *dl, uint32_t o_required,
opts->present |= DL_OPT_SB;
}
- if ((o_required & DL_OPT_PORT_TYPE) && !(o_found & DL_OPT_PORT_TYPE)) {
- pr_err("Port type option expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_PORT_COUNT) &&
- !(o_found & DL_OPT_PORT_COUNT)) {
- pr_err("Port split count option expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_SB_POOL) && !(o_found & DL_OPT_SB_POOL)) {
- pr_err("Pool index option expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_SB_SIZE) && !(o_found & DL_OPT_SB_SIZE)) {
- pr_err("Pool size option expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_SB_TYPE) && !(o_found & DL_OPT_SB_TYPE)) {
- pr_err("Pool type option expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_SB_THTYPE) && !(o_found & DL_OPT_SB_THTYPE)) {
- pr_err("Pool threshold type option expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_SB_TH) && !(o_found & DL_OPT_SB_TH)) {
- pr_err("Threshold option expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_SB_TC) && !(o_found & DL_OPT_SB_TC)) {
- pr_err("TC index option expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_ESWITCH_MODE) &&
- !(o_found & DL_OPT_ESWITCH_MODE)) {
- pr_err("E-Switch mode option expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_ESWITCH_INLINE_MODE) &&
- !(o_found & DL_OPT_ESWITCH_INLINE_MODE)) {
- pr_err("E-Switch inline-mode option expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_DPIPE_TABLE_NAME) &&
- !(o_found & DL_OPT_DPIPE_TABLE_NAME)) {
- pr_err("Dpipe table name expected\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_DPIPE_TABLE_COUNTERS) &&
- !(o_found & DL_OPT_DPIPE_TABLE_COUNTERS)) {
- pr_err("Dpipe table counter state expected\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_ESWITCH_ENCAP_MODE) &&
- !(o_found & DL_OPT_ESWITCH_ENCAP_MODE)) {
- pr_err("E-Switch encapsulation option expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_PARAM_NAME) &&
- !(o_found & DL_OPT_PARAM_NAME)) {
- pr_err("Parameter name expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_PARAM_VALUE) &&
- !(o_found & DL_OPT_PARAM_VALUE)) {
- pr_err("Value to set expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_PARAM_CMODE) &&
- !(o_found & DL_OPT_PARAM_CMODE)) {
- pr_err("Configuration mode expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_REGION_SNAPSHOT_ID) &&
- !(o_found & DL_OPT_REGION_SNAPSHOT_ID)) {
- pr_err("Region snapshot id expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_REGION_ADDRESS) &&
- !(o_found & DL_OPT_REGION_ADDRESS)) {
- pr_err("Region address value expected.\n");
- return -EINVAL;
- }
-
- if ((o_required & DL_OPT_REGION_LENGTH) &&
- !(o_found & DL_OPT_REGION_LENGTH)) {
- pr_err("Region length value expected.\n");
- return -EINVAL;
- }
-
- return 0;
+ return dl_args_finding_required_validate(o_required, o_found);
}
static void dl_opts_put(struct nlmsghdr *nlh, struct dl *dl)
--
2.14.1
^ permalink raw reply related
* [PATCH v3 iproute2-next 10/11] devlink: Add devlink health set command
From: Aya Levin @ 2019-02-24 12:46 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1551012378-29167-1-git-send-email-ayal@mellanox.com>
Add devlink set command which enables the user to configure parameters
related to the devlink health mechanism per reporter.
1) grace_period [msec] time interval between auto recoveries.
2) auto_recover [true/false] whether the devlink should execute automatic
recover on error.
Add a helper function to retrieve a boolean value as an input parameter.
Example:
$ devlink health set pci/0000:00:09.0 reporter tx grace_period 3500
$ devlink health set pci/0000:00:09.0 reporter tx auto_recover false
Signed-off-by: Aya Levin <ayal@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
devlink/devlink.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 66 insertions(+)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index a10c5957f1cc..d1da218c8367 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -207,6 +207,8 @@ static void ifname_map_free(struct ifname_map *ifname_map)
#define DL_OPT_REGION_ADDRESS BIT(23)
#define DL_OPT_REGION_LENGTH BIT(24)
#define DL_OPT_HEALTH_REPORTER_NAME BIT(25)
+#define DL_OPT_HEALTH_REPORTER_GRACEFUL_PERIOD BIT(26)
+#define DL_OPT_HEALTH_REPORTER_AUTO_RECOVER BIT(27)
struct dl_opts {
uint32_t present; /* flags of present items */
@@ -239,6 +241,8 @@ struct dl_opts {
uint64_t region_address;
uint64_t region_length;
const char *reporter_name;
+ uint64_t reporter_graceful_period;
+ bool reporter_auto_recover;
};
struct dl {
@@ -837,6 +841,24 @@ static int dl_argv_uint16_t(struct dl *dl, uint16_t *p_val)
return 0;
}
+static int dl_argv_bool(struct dl *dl, bool *p_val)
+{
+ char *str = dl_argv_next(dl);
+ int err;
+
+ if (!str) {
+ pr_err("Boolean argument expected\n");
+ return -EINVAL;
+ }
+
+ err = strtobool(str, p_val);
+ if (err) {
+ pr_err("\"%s\" is not a valid boolean value\n", str);
+ return err;
+ }
+ return 0;
+}
+
static int dl_argv_str(struct dl *dl, const char **p_str)
{
const char *str = dl_argv_next(dl);
@@ -1252,6 +1274,21 @@ static int dl_argv_parse(struct dl *dl, uint32_t o_required,
if (err)
return err;
o_found |= DL_OPT_HEALTH_REPORTER_NAME;
+ } else if (dl_argv_match(dl, "grace_period") &&
+ (o_all & DL_OPT_HEALTH_REPORTER_GRACEFUL_PERIOD)) {
+ dl_arg_inc(dl);
+ err = dl_argv_uint64_t(dl,
+ &opts->reporter_graceful_period);
+ if (err)
+ return err;
+ o_found |= DL_OPT_HEALTH_REPORTER_GRACEFUL_PERIOD;
+ } else if (dl_argv_match(dl, "auto_recover") &&
+ (o_all & DL_OPT_HEALTH_REPORTER_AUTO_RECOVER)) {
+ dl_arg_inc(dl);
+ err = dl_argv_bool(dl, &opts->reporter_auto_recover);
+ if (err)
+ return err;
+ o_found |= DL_OPT_HEALTH_REPORTER_AUTO_RECOVER;
} else {
pr_err("Unknown option \"%s\"\n", dl_argv(dl));
return -EINVAL;
@@ -1352,6 +1389,14 @@ static void dl_opts_put(struct nlmsghdr *nlh, struct dl *dl)
if (opts->present & DL_OPT_HEALTH_REPORTER_NAME)
mnl_attr_put_strz(nlh, DEVLINK_ATTR_HEALTH_REPORTER_NAME,
opts->reporter_name);
+ if (opts->present & DL_OPT_HEALTH_REPORTER_GRACEFUL_PERIOD)
+ mnl_attr_put_u64(nlh,
+ DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD,
+ opts->reporter_graceful_period);
+ if (opts->present & DL_OPT_HEALTH_REPORTER_AUTO_RECOVER)
+ mnl_attr_put_u8(nlh, DEVLINK_ATTR_HEALTH_REPORTER_AUTO_RECOVER,
+ opts->reporter_auto_recover);
+
}
static int dl_argv_parse_put(struct nlmsghdr *nlh, struct dl *dl,
@@ -5762,6 +5807,23 @@ static int cmd_region(struct dl *dl)
return -ENOENT;
}
+static int cmd_health_set_params(struct dl *dl)
+{
+ struct nlmsghdr *nlh;
+ int err;
+
+ nlh = mnlg_msg_prepare(dl->nlg, DEVLINK_CMD_HEALTH_REPORTER_SET,
+ NLM_F_REQUEST | NLM_F_ACK);
+ err = dl_argv_parse(dl, DL_OPT_HANDLE | DL_OPT_HEALTH_REPORTER_NAME,
+ DL_OPT_HEALTH_REPORTER_GRACEFUL_PERIOD |
+ DL_OPT_HEALTH_REPORTER_AUTO_RECOVER);
+ if (err)
+ return err;
+
+ dl_opts_put(nlh, dl);
+ return _mnlg_socket_sndrcv(dl->nlg, nlh, NULL, NULL);
+}
+
static int cmd_health_dump_clear(struct dl *dl)
{
struct nlmsghdr *nlh;
@@ -6126,6 +6188,7 @@ static void cmd_health_help(void)
pr_err(" devlink health diagnose DEV reporter REPORTER_NAME\n");
pr_err(" devlink health dump show DEV reporter REPORTER_NAME\n");
pr_err(" devlink health dump clear DEV reporter REPORTER_NAME\n");
+ pr_err(" devlink health set DEV reporter REPORTER_NAME { grace_period | auto_recover } { msec | boolean }\n");
}
static int cmd_health(struct dl *dl)
@@ -6152,6 +6215,9 @@ static int cmd_health(struct dl *dl)
dl_arg_inc(dl);
return cmd_health_dump_clear(dl);
}
+ } else if (dl_argv_match(dl, "set")) {
+ dl_arg_inc(dl);
+ return cmd_health_set_params(dl);
}
pr_err("Command \"%s\" not found\n", dl_argv(dl));
return -ENOENT;
--
2.14.1
^ permalink raw reply related
* [PATCH v3 iproute2-next 06/11] devlink: Add devlink health recover command
From: Aya Levin @ 2019-02-24 12:46 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1551012378-29167-1-git-send-email-ayal@mellanox.com>
Add devlink health recover command which enables the user to initiate a
recovery on a reporter (if a recovery cb was supplied by the reporter).
This operation will increment the recoveries counter displayed in the
show command.
Example:
$ devlink health recover pci/0000:00:09.0 reporter tx
Signed-off-by: Aya Levin <ayal@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
devlink/devlink.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index a53db32952e8..ff1d2fcf97bb 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -5761,6 +5761,23 @@ static int cmd_region(struct dl *dl)
return -ENOENT;
}
+static int cmd_health_recover(struct dl *dl)
+{
+ struct nlmsghdr *nlh;
+ int err;
+
+ nlh = mnlg_msg_prepare(dl->nlg, DEVLINK_CMD_HEALTH_REPORTER_RECOVER,
+ NLM_F_REQUEST | NLM_F_ACK);
+
+ err = dl_argv_parse_put(nlh, dl,
+ DL_OPT_HANDLE | DL_OPT_HEALTH_REPORTER_NAME, 0);
+ if (err)
+ return err;
+
+ dl_opts_put(nlh, dl);
+ return _mnlg_socket_sndrcv(dl->nlg, nlh, NULL, NULL);
+}
+
enum devlink_health_reporter_state {
DEVLINK_HEALTH_REPORTER_STATE_HEALTHY,
DEVLINK_HEALTH_REPORTER_STATE_ERROR,
@@ -5900,6 +5917,7 @@ static int cmd_health_show(struct dl *dl)
static void cmd_health_help(void)
{
pr_err("Usage: devlink health show [ dev DEV reporter REPORTER_NAME ]\n");
+ pr_err(" devlink health recover DEV reporter REPORTER_NAME\n");
}
static int cmd_health(struct dl *dl)
@@ -5911,6 +5929,9 @@ static int cmd_health(struct dl *dl)
dl_argv_match(dl, "list") || dl_no_arg(dl)) {
dl_arg_inc(dl);
return cmd_health_show(dl);
+ } else if (dl_argv_match(dl, "recover")) {
+ dl_arg_inc(dl);
+ return cmd_health_recover(dl);
}
pr_err("Command \"%s\" not found\n", dl_argv(dl));
return -ENOENT;
--
2.14.1
^ permalink raw reply related
* [PATCH v3 iproute2-next 11/11] devlink: Add devlink-health man page
From: Aya Levin @ 2019-02-24 12:46 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1551012378-29167-1-git-send-email-ayal@mellanox.com>
Add a man page describing devlink health's command set. Also add a
reference link from devlink main man page.
Signed-off-by: Aya Levin <ayal@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
man/man8/devlink-health.8 | 197 ++++++++++++++++++++++++++++++++++++++++++++++
man/man8/devlink.8 | 7 +-
2 files changed, 203 insertions(+), 1 deletion(-)
create mode 100644 man/man8/devlink-health.8
diff --git a/man/man8/devlink-health.8 b/man/man8/devlink-health.8
new file mode 100644
index 000000000000..7ed0ae4534dc
--- /dev/null
+++ b/man/man8/devlink-health.8
@@ -0,0 +1,197 @@
+.TH DEVLINK\-HEALTH 8 "20 Feb 2019" "iproute2" "Linux"
+.SH NAME
+devlink-health \- devlink health reporting and recovery
+.SH SYNOPSIS
+.sp
+.ad l
+.in +8
+.ti -8
+.B devlink
+.RI "[ " OPTIONS " ]"
+.B health
+.RI " { " COMMAND " | "
+.BR help " }"
+.sp
+
+.ti -8
+.IR OPTIONS " := { "
+\fB\-V\fR[\fIersion\fR] }
+
+.ti -8
+.BR "devlink health show"
+.RI "[ " DEV ""
+.B reporter
+.RI ""REPORTER " ] "
+
+.ti -8
+.BR "devlink health recover"
+.RI "" DEV ""
+.B reporter
+.RI "" REPORTER ""
+
+.ti -8
+.BR "devlink health diagnose"
+.RI "" DEV ""
+.B reporter
+.RI "" REPORTER ""
+
+.ti -8
+.BR "devlink health dump show"
+.RI "" DEV ""
+.B reporter
+.RI "" REPORTER ""
+
+.ti -8
+.BR "devlink health dump clear"
+.RI "" DEV ""
+.B reporter
+.RI "" REPORTER ""
+
+.ti -8
+.BR "devlink health set"
+.RI "" DEV ""
+.B reporter
+.RI "" REPORTER ""
+.RI " { "
+.B grace_period | auto_recover
+.RI " } { "
+.RI "" msec ""
+.RI "|"
+.RI "" boolean ""
+.RI " } "
+.ti -8
+.B devlink health help
+
+.SH "DESCRIPTION"
+.SS devlink health show - Show status and configuration on all supported reporters on all devlink devices.
+
+.PP
+.I "DEV"
+- specifies the devlink device.
+
+.PP
+.I "REPORTER"
+- specifies the reporter's name registered on the devlink device.
+
+.SS devlink health recover - Initiate a recovery operation on a reporter.
+This action performs a recovery and increases the recoveries counter on success.
+
+.PP
+.I "DEV"
+- specifies the devlink device.
+
+.PP
+.I "REPORTER"
+- specifies the reporter's name registered on the devlink device.
+
+.SS devlink health diagnose - Retrieve diagnostics data on a reporter.
+
+.PP
+.I "DEV"
+- specifies the devlink device.
+
+.PP
+.I "REPORTER"
+- specifies the reporter's name registered on the devlink device.
+
+.SS devlink health dump show - Display the last saved dump.
+
+.PD 0
+.P
+devlink health saves a single dump per reporter. If an dump is
+.P
+not already stored by the Devlink, this command will generate a new
+.P
+dump. The dump can be generated either automatically when a
+.P
+reporter reports on an error or manually at the user's request.
+.PD
+
+.PP
+.I "DEV"
+- specifies the devlink device.
+
+.PP
+.I "REPORTER"
+- specifies the reporter's name registered on the devlink device.
+
+.SS devlink health dump clear - Delete the saved dump.
+Deleting the saved dump enables a generation of a new dump on
+.PD 0
+.P
+the next "devlink health dump show" command.
+.PD
+
+.PP
+.I "DEV"
+- specifies the devlink device.
+
+.PP
+.I "REPORTER"
+- specifies the reporter's name registered on the devlink device.
+
+.SS devlink health set - Enable the user to configure:
+.PD 0
+1) grace_period [msec] - Time interval between consecutive auto recoveries.
+.P
+2) auto_recover [true/false] - Indicates whether the devlink should execute automatic recover on error.
+.P
+Please note that this command is not supported on a reporter which
+doesn't support a recovery method.
+.PD
+
+.PP
+.I "DEV"
+- specifies the devlink device.
+
+.PP
+.I "REPORTER"
+- specifies the reporter's name registered on the devlink device.
+
+.SH "EXAMPLES"
+.PP
+devlink health show
+.RS 4
+List status and configuration of available reporters on devices.
+.RE
+.PP
+devlink health recover pci/0000:00:09.0 reporter tx
+.RS 4
+Initiate recovery on tx reporter registered on pci/0000:00:09.0.
+.RE
+.PP
+devlink health diagnose pci/0000:00:09.0 reporter tx
+.RS 4
+List diagnostics data on the specified device and reporter.
+.RE
+.PP
+devlink health dump show pci/0000:00:09.0 reporter tx
+.RS 4
+Display the last saved dump on the specified device and reporter.
+.RE
+.PP
+devlink health dump clear pci/0000:00:09.0 reporter tx
+.RS 4
+Delete saved dump on the specified device and reporter.
+.RE
+.PP
+devlink health set pci/0000:00:09.0 reporter tx grace_period 3500
+.RS 4
+Set time interval between auto recoveries to minimum of 3500 msec on
+the specified device and reporter.
+.RE
+.PP
+devlink health set pci/0000:00:09.0 reporter tx auto_recover false
+.RS 4
+Turn off auto recovery on the specified device and reporter.
+.RE
+.SH SEE ALSO
+.BR devlink (8),
+.BR devlink-dev (8),
+.BR devlink-port (8),
+.BR devlink-param (8),
+.BR devlink-region (8),
+.br
+
+.SH AUTHOR
+Aya Levin <ayal@mellanox.com>
diff --git a/man/man8/devlink.8 b/man/man8/devlink.8
index 8d527e7e1d60..13d4dcd908b3 100644
--- a/man/man8/devlink.8
+++ b/man/man8/devlink.8
@@ -7,7 +7,7 @@ devlink \- Devlink tool
.in +8
.ti -8
.B devlink
-.RI "[ " OPTIONS " ] { " dev | port | monitor | sb | resource | region " } { " COMMAND " | "
+.RI "[ " OPTIONS " ] { " dev | port | monitor | sb | resource | region | health " } { " COMMAND " | "
.BR help " }"
.sp
@@ -78,6 +78,10 @@ Turn on verbose output.
.B region
- devlink address region access
+.TP
+.B health
+- devlink reporting and recovery
+
.SS
.I COMMAND
@@ -109,6 +113,7 @@ Exit status is 0 if command was successful or a positive integer upon failure.
.BR devlink-sb (8),
.BR devlink-resource (8),
.BR devlink-region (8),
+.BR devlink-health (8),
.br
.SH REPORTING BUGS
--
2.14.1
^ permalink raw reply related
* [PATCH v3 iproute2-next 03/11] devlink: Fix boolean JSON print
From: Aya Levin @ 2019-02-24 12:46 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1551012378-29167-1-git-send-email-ayal@mellanox.com>
This patch removes the inverted commas from boolean values in JSON
format: true/false instead of "true"/"false".
Signed-off-by: Aya Levin <ayal@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
devlink/devlink.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index b51380298f8a..e5aca7b955cc 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -1603,10 +1603,10 @@ static void pr_out_str(struct dl *dl, const char *name, const char *val)
static void pr_out_bool(struct dl *dl, const char *name, bool val)
{
- if (val)
- pr_out_str(dl, name, "true");
+ if (dl->json_output)
+ jsonw_bool_field(dl->jw, name, val);
else
- pr_out_str(dl, name, "false");
+ pr_out_str(dl, name, val ? "true" : "false");
}
static void pr_out_uint(struct dl *dl, const char *name, unsigned int val)
--
2.14.1
^ permalink raw reply related
* [PATCH v3 iproute2-next 07/11] devlink: Add devlink health diagnose command
From: Aya Levin @ 2019-02-24 12:46 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1551012378-29167-1-git-send-email-ayal@mellanox.com>
Add devlink health diagnose command: enabling retrieval of diagnostics data
by the user on a reporter on a device. The command's output is a
free text defined by the reporter.
This patch also introduces an infra structure for flexible format
output. This allow the command to display different data fields
according to the reporter.
Example:
$ devlink health diagnose pci/0000:00:0a.0 reporter tx
SQs:
sqn: 4403 HW state: 1 stopped: false
sqn: 4408 HW state: 1 stopped: false
sqn: 4413 HW state: 1 stopped: false
sqn: 4418 HW state: 1 stopped: false
sqn: 4423 HW state: 1 stopped: false
$ devlink health diagnose pci/0000:00:0a.0 reporter tx -jp
{
"SQs":[
{
"sqn":4403,
"HW state":1,
"stopped":false
},
{
"sqn":4408,
"HW state":1,
"stopped":false
},
{
"sqn":4413,
"HW state":1,
"stopped":false
},
{
"sqn":4418,
"HW state":1,
"stopped":false
},
{
"sqn":4423,
"HW state":1,
"stopped":false
}
]
}
Signed-off-by: Aya Levin <ayal@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
devlink/devlink.c | 187 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 187 insertions(+)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index ff1d2fcf97bb..ce3b427a3670 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -23,6 +23,7 @@
#include <libmnl/libmnl.h>
#include <netinet/ether.h>
#include <sys/sysinfo.h>
+#include <sys/queue.h>
#include "SNAPSHOT.h"
#include "list.h"
@@ -5761,6 +5762,188 @@ static int cmd_region(struct dl *dl)
return -ENOENT;
}
+static int fmsg_value_show(struct dl *dl, int type, struct nlattr *nl_data)
+{
+ uint8_t *data;
+ uint32_t len;
+
+ switch (type) {
+ case MNL_TYPE_FLAG:
+ pr_out_bool_value(dl, mnl_attr_get_u8(nl_data));
+ break;
+ case MNL_TYPE_U8:
+ pr_out_uint_value(dl, mnl_attr_get_u8(nl_data));
+ break;
+ case MNL_TYPE_U16:
+ pr_out_uint_value(dl, mnl_attr_get_u16(nl_data));
+ break;
+ case MNL_TYPE_U32:
+ pr_out_uint_value(dl, mnl_attr_get_u32(nl_data));
+ break;
+ case MNL_TYPE_U64:
+ pr_out_uint64_value(dl, mnl_attr_get_u64(nl_data));
+ break;
+ case MNL_TYPE_NUL_STRING:
+ pr_out_str_value(dl, mnl_attr_get_str(nl_data));
+ break;
+ case MNL_TYPE_BINARY:
+ len = mnl_attr_get_payload_len(nl_data);
+ data = mnl_attr_get_payload(nl_data);
+ pr_out_binary_value(dl, data, len);
+ break;
+ default:
+ return -EINVAL;
+ }
+ return MNL_CB_OK;
+}
+
+struct nest_qentry {
+ int attr_type;
+ TAILQ_ENTRY(nest_qentry) nest_entries;
+};
+
+struct fmsg_cb_data {
+ struct dl *dl;
+ uint8_t value_type;
+ TAILQ_HEAD(, nest_qentry) qhead;
+};
+
+static int cmd_fmsg_nest_queue(struct fmsg_cb_data *fmsg_data,
+ uint8_t *attr_value, bool insert)
+{
+ struct nest_qentry *entry = NULL;
+
+ if (insert) {
+ entry = malloc(sizeof(struct nest_qentry));
+ if (!entry)
+ return -ENOMEM;
+
+ entry->attr_type = *attr_value;
+ TAILQ_INSERT_HEAD(&fmsg_data->qhead, entry, nest_entries);
+ } else {
+ if (TAILQ_EMPTY(&fmsg_data->qhead))
+ return MNL_CB_ERROR;
+ entry = TAILQ_FIRST(&fmsg_data->qhead);
+ *attr_value = entry->attr_type;
+ TAILQ_REMOVE(&fmsg_data->qhead, entry, nest_entries);
+ free(entry);
+ }
+ return MNL_CB_OK;
+}
+
+static int cmd_fmsg_nest(struct fmsg_cb_data *fmsg_data, uint8_t nest_value,
+ bool start)
+{
+ struct dl *dl = fmsg_data->dl;
+ uint8_t value = nest_value;
+ int err;
+
+ err = cmd_fmsg_nest_queue(fmsg_data, &value, start);
+ if (err != MNL_CB_OK)
+ return err;
+
+ switch (value) {
+ case DEVLINK_ATTR_FMSG_OBJ_NEST_START:
+ if (start)
+ pr_out_entry_start(dl);
+ else
+ pr_out_entry_end(dl);
+ break;
+ case DEVLINK_ATTR_FMSG_PAIR_NEST_START:
+ break;
+ case DEVLINK_ATTR_FMSG_ARR_NEST_START:
+ if (dl->json_output) {
+ if (start)
+ jsonw_start_array(dl->jw);
+ else
+ jsonw_end_array(dl->jw);
+ } else {
+ if (start) {
+ __pr_out_newline();
+ __pr_out_indent_inc();
+ } else {
+ __pr_out_indent_dec();
+ }
+ }
+ break;
+ default:
+ return -EINVAL;
+ }
+ return MNL_CB_OK;
+}
+
+static int cmd_fmsg_object_cb(const struct nlmsghdr *nlh, void *data)
+{
+ struct genlmsghdr *genl = mnl_nlmsg_get_payload(nlh);
+ struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {};
+ struct fmsg_cb_data *fmsg_data = data;
+ struct dl *dl = fmsg_data->dl;
+ struct nlattr *nla_object;
+ int attr_type;
+ int err;
+
+ mnl_attr_parse(nlh, sizeof(*genl), attr_cb, tb);
+ if (!tb[DEVLINK_ATTR_FMSG])
+ return MNL_CB_ERROR;
+
+ mnl_attr_for_each_nested(nla_object, tb[DEVLINK_ATTR_FMSG]) {
+ attr_type = mnl_attr_get_type(nla_object);
+ switch (attr_type) {
+ case DEVLINK_ATTR_FMSG_OBJ_NEST_START:
+ case DEVLINK_ATTR_FMSG_PAIR_NEST_START:
+ case DEVLINK_ATTR_FMSG_ARR_NEST_START:
+ err = cmd_fmsg_nest(fmsg_data, attr_type, true);
+ if (err != MNL_CB_OK)
+ return err;
+ break;
+ case DEVLINK_ATTR_FMSG_NEST_END:
+ err = cmd_fmsg_nest(fmsg_data, attr_type, false);
+ if (err != MNL_CB_OK)
+ return err;
+ break;
+ case DEVLINK_ATTR_FMSG_OBJ_NAME:
+ pr_out_name(dl, mnl_attr_get_str(nla_object));
+ break;
+ case DEVLINK_ATTR_FMSG_OBJ_VALUE_TYPE:
+ fmsg_data->value_type = mnl_attr_get_u8(nla_object);
+ break;
+ case DEVLINK_ATTR_FMSG_OBJ_VALUE_DATA:
+ err = fmsg_value_show(dl, fmsg_data->value_type,
+ nla_object);
+ if (err != MNL_CB_OK)
+ return err;
+ break;
+ default:
+ return -EINVAL;
+ }
+ }
+ return MNL_CB_OK;
+}
+
+static int cmd_health_object_common(struct dl *dl, uint8_t cmd)
+{
+ struct fmsg_cb_data data;
+ struct nlmsghdr *nlh;
+ int err;
+
+ nlh = mnlg_msg_prepare(dl->nlg, cmd, NLM_F_REQUEST | NLM_F_ACK);
+
+ err = dl_argv_parse_put(nlh, dl,
+ DL_OPT_HANDLE | DL_OPT_HEALTH_REPORTER_NAME, 0);
+ if (err)
+ return err;
+
+ data.dl = dl;
+ TAILQ_INIT(&data.qhead);
+ err = _mnlg_socket_sndrcv(dl->nlg, nlh, cmd_fmsg_object_cb, &data);
+ return err;
+}
+
+static int cmd_health_diagnose(struct dl *dl)
+{
+ return cmd_health_object_common(dl, DEVLINK_CMD_HEALTH_REPORTER_DIAGNOSE);
+}
+
static int cmd_health_recover(struct dl *dl)
{
struct nlmsghdr *nlh;
@@ -5918,6 +6101,7 @@ static void cmd_health_help(void)
{
pr_err("Usage: devlink health show [ dev DEV reporter REPORTER_NAME ]\n");
pr_err(" devlink health recover DEV reporter REPORTER_NAME\n");
+ pr_err(" devlink health diagnose DEV reporter REPORTER_NAME\n");
}
static int cmd_health(struct dl *dl)
@@ -5932,6 +6116,9 @@ static int cmd_health(struct dl *dl)
} else if (dl_argv_match(dl, "recover")) {
dl_arg_inc(dl);
return cmd_health_recover(dl);
+ } else if (dl_argv_match(dl, "diagnose")) {
+ dl_arg_inc(dl);
+ return cmd_health_diagnose(dl);
}
pr_err("Command \"%s\" not found\n", dl_argv(dl));
return -ENOENT;
--
2.14.1
^ permalink raw reply related
* [PATCH v3 iproute2-next 05/11] devlink: Add devlink health show command
From: Aya Levin @ 2019-02-24 12:46 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1551012378-29167-1-git-send-email-ayal@mellanox.com>
Add devlink health show command which displays status and configuration
info on a specific reporter on a device or dump the info on all
reporters on all devices. Add helper functions to display status and
dump's time stamp.
Example:
$ devlink health show pci/0000:00:09.0 reporter tx
pci/0000:00:09.0:
name tx
state healthy error 0 recover 1 last_dump_date 2019-02-14 last_dump_time 10:10:10 grace_period 600 auto_recover true
$ devlink health show pci/0000:00:09.0 reporter tx -jp
{
"health":{
"pci/0000:00:0a.0":[
{
"name":"tx",
"state":"healthy",
"error":0,
"recover":1,
"last_dump_date":"2019-Feb-14",
"last_dump_time":"10:10:10",
"grace_period":600,
"auto_recover":true
}
]
}
Signed-off-by: Aya Levin <ayal@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
devlink/devlink.c | 185 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 184 insertions(+), 1 deletion(-)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index 60f20b636500..a53db32952e8 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -22,6 +22,7 @@
#include <linux/devlink.h>
#include <libmnl/libmnl.h>
#include <netinet/ether.h>
+#include <sys/sysinfo.h>
#include "SNAPSHOT.h"
#include "list.h"
@@ -41,6 +42,10 @@
#define PARAM_CMODE_PERMANENT_STR "permanent"
#define DL_ARGS_REQUIRED_MAX_ERR_LEN 80
+#define HEALTH_REPORTER_STATE_HEALTHY_STR "healthy"
+#define HEALTH_REPORTER_STATE_ERROR_STR "error"
+#define HEALTH_REPORTER_TIMESTAMP_FMT_LEN 80
+
static int g_new_line_count;
#define pr_err(args...) fprintf(stderr, ##args)
@@ -200,6 +205,7 @@ static void ifname_map_free(struct ifname_map *ifname_map)
#define DL_OPT_REGION_SNAPSHOT_ID BIT(22)
#define DL_OPT_REGION_ADDRESS BIT(23)
#define DL_OPT_REGION_LENGTH BIT(24)
+#define DL_OPT_HEALTH_REPORTER_NAME BIT(25)
struct dl_opts {
uint32_t present; /* flags of present items */
@@ -231,6 +237,7 @@ struct dl_opts {
uint32_t region_snapshot_id;
uint64_t region_address;
uint64_t region_length;
+ const char *reporter_name;
};
struct dl {
@@ -391,6 +398,13 @@ static const enum mnl_attr_data_type devlink_policy[DEVLINK_ATTR_MAX + 1] = {
[DEVLINK_ATTR_INFO_VERSION_STORED] = MNL_TYPE_NESTED,
[DEVLINK_ATTR_INFO_VERSION_NAME] = MNL_TYPE_STRING,
[DEVLINK_ATTR_INFO_VERSION_VALUE] = MNL_TYPE_STRING,
+ [DEVLINK_ATTR_HEALTH_REPORTER] = MNL_TYPE_NESTED,
+ [DEVLINK_ATTR_HEALTH_REPORTER_NAME] = MNL_TYPE_STRING,
+ [DEVLINK_ATTR_HEALTH_REPORTER_STATE] = MNL_TYPE_U8,
+ [DEVLINK_ATTR_HEALTH_REPORTER_ERR_COUNT] = MNL_TYPE_U64,
+ [DEVLINK_ATTR_HEALTH_REPORTER_RECOVER_COUNT] = MNL_TYPE_U64,
+ [DEVLINK_ATTR_HEALTH_REPORTER_DUMP_TS] = MNL_TYPE_U64,
+ [DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD] = MNL_TYPE_U64,
};
static int attr_cb(const struct nlattr *attr, void *data)
@@ -976,6 +990,7 @@ static const struct dl_args_metadata dl_args_required[] = {
{DL_OPT_REGION_SNAPSHOT_ID, "Region snapshot id expected."},
{DL_OPT_REGION_ADDRESS, "Region address value expected."},
{DL_OPT_REGION_LENGTH, "Region length value expected."},
+ {DL_OPT_HEALTH_REPORTER_NAME, "Reporter's name is expected."},
};
static int dl_args_finding_required_validate(uint32_t o_required,
@@ -1229,6 +1244,13 @@ static int dl_argv_parse(struct dl *dl, uint32_t o_required,
if (err)
return err;
o_found |= DL_OPT_REGION_LENGTH;
+ } else if (dl_argv_match(dl, "reporter") &&
+ (o_all & DL_OPT_HEALTH_REPORTER_NAME)) {
+ dl_arg_inc(dl);
+ err = dl_argv_str(dl, &opts->reporter_name);
+ if (err)
+ return err;
+ o_found |= DL_OPT_HEALTH_REPORTER_NAME;
} else {
pr_err("Unknown option \"%s\"\n", dl_argv(dl));
return -EINVAL;
@@ -1326,6 +1348,9 @@ static void dl_opts_put(struct nlmsghdr *nlh, struct dl *dl)
if (opts->present & DL_OPT_REGION_LENGTH)
mnl_attr_put_u64(nlh, DEVLINK_ATTR_REGION_CHUNK_LEN,
opts->region_length);
+ if (opts->present & DL_OPT_HEALTH_REPORTER_NAME)
+ mnl_attr_put_strz(nlh, DEVLINK_ATTR_HEALTH_REPORTER_NAME,
+ opts->reporter_name);
}
static int dl_argv_parse_put(struct nlmsghdr *nlh, struct dl *dl,
@@ -5736,11 +5761,166 @@ static int cmd_region(struct dl *dl)
return -ENOENT;
}
+enum devlink_health_reporter_state {
+ DEVLINK_HEALTH_REPORTER_STATE_HEALTHY,
+ DEVLINK_HEALTH_REPORTER_STATE_ERROR,
+};
+
+static const char *health_state_name(uint8_t state)
+{
+ switch (state) {
+ case DEVLINK_HEALTH_REPORTER_STATE_HEALTHY:
+ return HEALTH_REPORTER_STATE_HEALTHY_STR;
+ case DEVLINK_HEALTH_REPORTER_STATE_ERROR:
+ return HEALTH_REPORTER_STATE_ERROR_STR;
+ default:
+ return "<unknown state>";
+ }
+}
+
+static void format_logtime(uint64_t time_ms, char *ts_date, char *ts_time)
+{
+ struct sysinfo s_info;
+ struct tm *info;
+ time_t now, sec;
+ int err;
+
+ time(&now);
+ info = localtime(&now);
+ err = sysinfo(&s_info);
+ if (err)
+ goto out;
+ /* Subtract uptime in sec from now yields the time of system
+ * uptime. To this, add time_ms which is the amount of
+ * milliseconds elapsed between uptime and the dump taken.
+ */
+ sec = now - s_info.uptime + time_ms / 1000;
+ info = localtime(&sec);
+out:
+ strftime(ts_date, HEALTH_REPORTER_TIMESTAMP_FMT_LEN, "%Y-%m-%d", info);
+ strftime(ts_time, HEALTH_REPORTER_TIMESTAMP_FMT_LEN, "%H:%M:%S", info);
+}
+
+static void pr_out_health(struct dl *dl, struct nlattr **tb_health)
+{
+ struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {};
+ enum devlink_health_reporter_state state;
+ const struct nlattr *attr;
+ uint64_t time_ms;
+ int err;
+
+ err = mnl_attr_parse_nested(tb_health[DEVLINK_ATTR_HEALTH_REPORTER],
+ attr_cb, tb);
+ if (err != MNL_CB_OK)
+ return;
+
+ if (!tb[DEVLINK_ATTR_HEALTH_REPORTER_NAME] ||
+ !tb[DEVLINK_ATTR_HEALTH_REPORTER_ERR_COUNT] ||
+ !tb[DEVLINK_ATTR_HEALTH_REPORTER_RECOVER_COUNT] ||
+ !tb[DEVLINK_ATTR_HEALTH_REPORTER_STATE])
+ return;
+
+ pr_out_handle_start_arr(dl, tb_health);
+
+ pr_out_str(dl, "name",
+ mnl_attr_get_str(tb[DEVLINK_ATTR_HEALTH_REPORTER_NAME]));
+ if (!dl->json_output) {
+ __pr_out_newline();
+ __pr_out_indent_inc();
+ }
+ state = mnl_attr_get_u8(tb[DEVLINK_ATTR_HEALTH_REPORTER_STATE]);
+ pr_out_str(dl, "state", health_state_name(state));
+ pr_out_u64(dl, "error",
+ mnl_attr_get_u64(tb[DEVLINK_ATTR_HEALTH_REPORTER_ERR_COUNT]));
+ pr_out_u64(dl, "recover",
+ mnl_attr_get_u64(tb[DEVLINK_ATTR_HEALTH_REPORTER_RECOVER_COUNT]));
+ if (tb[DEVLINK_ATTR_HEALTH_REPORTER_DUMP_TS]) {
+ char dump_date[HEALTH_REPORTER_TIMESTAMP_FMT_LEN];
+ char dump_time[HEALTH_REPORTER_TIMESTAMP_FMT_LEN];
+
+ attr = tb[DEVLINK_ATTR_HEALTH_REPORTER_DUMP_TS];
+ time_ms = mnl_attr_get_u64(attr);
+ format_logtime(time_ms, dump_date, dump_time);
+
+ pr_out_str(dl, "last_dump_date", dump_date);
+ pr_out_str(dl, "last_dump_time", dump_time);
+ }
+ if (tb[DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD])
+ pr_out_u64(dl, "grace_period",
+ mnl_attr_get_u64(tb[DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD]));
+ if (tb[DEVLINK_ATTR_HEALTH_REPORTER_AUTO_RECOVER])
+ pr_out_bool(dl, "auto_recover",
+ mnl_attr_get_u8(tb[DEVLINK_ATTR_HEALTH_REPORTER_AUTO_RECOVER]));
+
+ __pr_out_indent_dec();
+ pr_out_handle_end(dl);
+}
+
+static int cmd_health_show_cb(const struct nlmsghdr *nlh, void *data)
+{
+ struct genlmsghdr *genl = mnl_nlmsg_get_payload(nlh);
+ struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {};
+ struct dl *dl = data;
+
+ mnl_attr_parse(nlh, sizeof(*genl), attr_cb, tb);
+ if (!tb[DEVLINK_ATTR_BUS_NAME] || !tb[DEVLINK_ATTR_DEV_NAME] ||
+ !tb[DEVLINK_ATTR_HEALTH_REPORTER])
+ return MNL_CB_ERROR;
+
+ pr_out_health(dl, tb);
+
+ return MNL_CB_OK;
+}
+
+static int cmd_health_show(struct dl *dl)
+{
+ struct nlmsghdr *nlh;
+ uint16_t flags = NLM_F_REQUEST | NLM_F_ACK;
+ int err;
+
+ if (dl_argc(dl) == 0)
+ flags |= NLM_F_DUMP;
+ nlh = mnlg_msg_prepare(dl->nlg, DEVLINK_CMD_HEALTH_REPORTER_GET,
+ flags);
+
+ if (dl_argc(dl) > 0) {
+ err = dl_argv_parse_put(nlh, dl,
+ DL_OPT_HANDLE |
+ DL_OPT_HEALTH_REPORTER_NAME, 0);
+ if (err)
+ return err;
+ }
+ pr_out_section_start(dl, "health");
+
+ err = _mnlg_socket_sndrcv(dl->nlg, nlh, cmd_health_show_cb, dl);
+ pr_out_section_end(dl);
+ return err;
+}
+
+static void cmd_health_help(void)
+{
+ pr_err("Usage: devlink health show [ dev DEV reporter REPORTER_NAME ]\n");
+}
+
+static int cmd_health(struct dl *dl)
+{
+ if (dl_argv_match(dl, "help")) {
+ cmd_health_help();
+ return 0;
+ } else if (dl_argv_match(dl, "show") ||
+ dl_argv_match(dl, "list") || dl_no_arg(dl)) {
+ dl_arg_inc(dl);
+ return cmd_health_show(dl);
+ }
+ pr_err("Command \"%s\" not found\n", dl_argv(dl));
+ return -ENOENT;
+}
+
static void help(void)
{
pr_err("Usage: devlink [ OPTIONS ] OBJECT { COMMAND | help }\n"
" devlink [ -f[orce] ] -b[atch] filename\n"
- "where OBJECT := { dev | port | sb | monitor | dpipe | resource | region }\n"
+ "where OBJECT := { dev | port | sb | monitor | dpipe | resource | region | health }\n"
" OPTIONS := { -V[ersion] | -n[o-nice-names] | -j[son] | -p[retty] | -v[erbose] }\n");
}
@@ -5773,6 +5953,9 @@ static int dl_cmd(struct dl *dl, int argc, char **argv)
} else if (dl_argv_match(dl, "region")) {
dl_arg_inc(dl);
return cmd_region(dl);
+ } else if (dl_argv_match(dl, "health")) {
+ dl_arg_inc(dl);
+ return cmd_health(dl);
}
pr_err("Object \"%s\" not found\n", dl_argv(dl));
return -ENOENT;
--
2.14.1
^ permalink raw reply related
* [PATCH v3 iproute2-next 00/11] Add support for devlink health
From: Aya Levin @ 2019-02-24 12:46 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
This series adds support for devlink health commands:
devlink health show [ DEV reporter REPORTER_NAME ]
devlink health recover DEV reporter REPORTER_NAME
devlink health diagnose DEV reporter REPORTER_NAME
devlink health dump show DEV reporter REPORTER_NAME
devlink health dump clear DEV reporter REPORTER_NAME
devlink health set DEV reporter REPORTER_NAME { grace_period | auto_recover } { msec | boolean }
The first patch refactors the validation of input parameters, which
grow way too long. Second and third patches fix bugs that were
discovered during the devlink health development. The forth patch adds
helper functions which enable output of value and labels separately.
Patches 5-10 add the devlink health functionality by command, the last
is the man page.
Changelog:
v2:
-Add patch #4.
-Separate patch "Add support for devlink health" into patches (5-10)
by command.
-Patch #1 Changed function's name dl_args_finding_required_validate
and a small refactor.
-Modify show command's output.
v3:
-Add blank row that was wrongly added patch #4
-Rephrase commit message patch #5
-Some refactoring in patch #5
Note: this series (patch 0005 and on) can be applied after aligning
include/uapi/linux/devlink.h with its corresponding kernel's version.
Aya Levin (11):
devlink: Refactor validation of finding required arguments
devlink: Fix print of uint64_t
devlink: Fix boolean JSON print
devlink: Add helper functions for name and value separately
devlink: Add devlink health show command
devlink: Add devlink health recover command
devlink: Add devlink health diagnose command
devlink: Add devlink health dump show command
devlink: Add devlink health dump clear command
devlink: Add devlink health set command
devlink: Add devlink-health man page
devlink/devlink.c | 725 ++++++++++++++++++++++++++++++++++++++--------
man/man8/devlink-health.8 | 197 +++++++++++++
man/man8/devlink.8 | 7 +-
3 files changed, 815 insertions(+), 114 deletions(-)
create mode 100644 man/man8/devlink-health.8
--
2.14.1
^ permalink raw reply
* [PATCH v3 iproute2-next 04/11] devlink: Add helper functions for name and value separately
From: Aya Levin @ 2019-02-24 12:46 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1551012378-29167-1-git-send-email-ayal@mellanox.com>
Add a new helper functions which outputs only values (without name
label) for different types: boolean, uint, uint64, string and binary.
In addition add a helper function which prints only the name label.
Signed-off-by: Aya Levin <ayal@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
devlink/devlink.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 65 insertions(+)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index e5aca7b955cc..60f20b636500 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -1636,6 +1636,71 @@ static void pr_out_u64(struct dl *dl, const char *name, uint64_t val)
}
}
+static void pr_out_bool_value(struct dl *dl, bool value)
+{
+ if (dl->json_output)
+ jsonw_bool(dl->jw, value);
+ else
+ pr_out(" %s", value ? "true" : "false");
+}
+
+static void pr_out_uint_value(struct dl *dl, unsigned int value)
+{
+ if (dl->json_output)
+ jsonw_uint(dl->jw, value);
+ else
+ pr_out(" %u", value);
+}
+
+static void pr_out_uint64_value(struct dl *dl, uint64_t value)
+{
+ if (dl->json_output)
+ jsonw_u64(dl->jw, value);
+ else
+ pr_out(" %lu", value);
+}
+
+static void pr_out_binary_value(struct dl *dl, uint8_t *data, uint32_t len)
+{
+ int i = 1;
+
+ if (dl->json_output)
+ jsonw_start_array(dl->jw);
+ else
+ pr_out("\n");
+
+ while (i < len) {
+ if (dl->json_output) {
+ jsonw_printf(dl->jw, "%d", data[i]);
+ } else {
+ pr_out(" %02x", data[i]);
+ if (!(i % 16))
+ pr_out("\n");
+ }
+ i++;
+ }
+ if (dl->json_output)
+ jsonw_end_array(dl->jw);
+ else if ((i - 1) % 16)
+ pr_out("\n");
+}
+
+static void pr_out_str_value(struct dl *dl, const char *value)
+{
+ if (dl->json_output)
+ jsonw_string(dl->jw, value);
+ else
+ pr_out(" %s", value);
+}
+
+static void pr_out_name(struct dl *dl, const char *name)
+{
+ if (dl->json_output)
+ jsonw_name(dl->jw, name);
+ else
+ pr_out(" %s:", name);
+}
+
static void pr_out_region_chunk_start(struct dl *dl, uint64_t addr)
{
if (dl->json_output) {
--
2.14.1
^ permalink raw reply related
* [PATCH v3 iproute2-next 09/11] devlink: Add devlink health dump clear command
From: Aya Levin @ 2019-02-24 12:46 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1551012378-29167-1-git-send-email-ayal@mellanox.com>
Add devlink dump clear command which deletes the last saved dump file.
Clearing the last saved dump enables a new dump file to be saved.
Example:
$ devlink health dump clear pci/0000:00:09.0 reporter tx
Signed-off-by: Aya Levin <ayal@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
devlink/devlink.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index 328d4e092688..a10c5957f1cc 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -5762,6 +5762,23 @@ static int cmd_region(struct dl *dl)
return -ENOENT;
}
+static int cmd_health_dump_clear(struct dl *dl)
+{
+ struct nlmsghdr *nlh;
+ int err;
+
+ nlh = mnlg_msg_prepare(dl->nlg, DEVLINK_CMD_HEALTH_REPORTER_DUMP_CLEAR,
+ NLM_F_REQUEST | NLM_F_ACK);
+
+ err = dl_argv_parse_put(nlh, dl,
+ DL_OPT_HANDLE | DL_OPT_HEALTH_REPORTER_NAME, 0);
+ if (err)
+ return err;
+
+ dl_opts_put(nlh, dl);
+ return _mnlg_socket_sndrcv(dl->nlg, nlh, NULL, NULL);
+}
+
static int fmsg_value_show(struct dl *dl, int type, struct nlattr *nl_data)
{
uint8_t *data;
@@ -6108,6 +6125,7 @@ static void cmd_health_help(void)
pr_err(" devlink health recover DEV reporter REPORTER_NAME\n");
pr_err(" devlink health diagnose DEV reporter REPORTER_NAME\n");
pr_err(" devlink health dump show DEV reporter REPORTER_NAME\n");
+ pr_err(" devlink health dump clear DEV reporter REPORTER_NAME\n");
}
static int cmd_health(struct dl *dl)
@@ -6130,6 +6148,9 @@ static int cmd_health(struct dl *dl)
if (dl_argv_match(dl, "show")) {
dl_arg_inc(dl);
return cmd_health_dump_show(dl);
+ } else if (dl_argv_match(dl, "clear")) {
+ dl_arg_inc(dl);
+ return cmd_health_dump_clear(dl);
}
}
pr_err("Command \"%s\" not found\n", dl_argv(dl));
--
2.14.1
^ permalink raw reply related
* [PATCH v3 iproute2-next 08/11] devlink: Add devlink health dump show command
From: Aya Levin @ 2019-02-24 12:46 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Jiri Pirko, Moshe Shemesh, Eran Ben Elisha, Aya Levin
In-Reply-To: <1551012378-29167-1-git-send-email-ayal@mellanox.com>
Add devlink dump show command which displays the last saved dump.
Devlink health saves a single dump. If a dump is not already stored
by the devlink for this reporter, devlink generates a new dump. The dump
can be generated automatically when a reporter reports on an
error or manually by user's request.
The dump's output is defined by the reporter. The command uses the
infra structure for flexible format output introduced in previous patch.
Example:
$ devlink health dump show pci/0000:00:09.0 reporter tx
Signed-off-by: Aya Levin <ayal@mellanox.com>
Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
devlink/devlink.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index ce3b427a3670..328d4e092688 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -5939,6 +5939,11 @@ static int cmd_health_object_common(struct dl *dl, uint8_t cmd)
return err;
}
+static int cmd_health_dump_show(struct dl *dl)
+{
+ return cmd_health_object_common(dl, DEVLINK_CMD_HEALTH_REPORTER_DUMP_GET);
+}
+
static int cmd_health_diagnose(struct dl *dl)
{
return cmd_health_object_common(dl, DEVLINK_CMD_HEALTH_REPORTER_DIAGNOSE);
@@ -6102,6 +6107,7 @@ static void cmd_health_help(void)
pr_err("Usage: devlink health show [ dev DEV reporter REPORTER_NAME ]\n");
pr_err(" devlink health recover DEV reporter REPORTER_NAME\n");
pr_err(" devlink health diagnose DEV reporter REPORTER_NAME\n");
+ pr_err(" devlink health dump show DEV reporter REPORTER_NAME\n");
}
static int cmd_health(struct dl *dl)
@@ -6119,6 +6125,12 @@ static int cmd_health(struct dl *dl)
} else if (dl_argv_match(dl, "diagnose")) {
dl_arg_inc(dl);
return cmd_health_diagnose(dl);
+ } else if (dl_argv_match(dl, "dump")) {
+ dl_arg_inc(dl);
+ if (dl_argv_match(dl, "show")) {
+ dl_arg_inc(dl);
+ return cmd_health_dump_show(dl);
+ }
}
pr_err("Command \"%s\" not found\n", dl_argv(dl));
return -ENOENT;
--
2.14.1
^ permalink raw reply related
* [PATCH 0/4] mwifiex PCI/wake-up interrupt fixes
From: Marc Zyngier @ 2019-02-24 14:04 UTC (permalink / raw)
To: Amitkumar Karwar, Enric Balletbo i Serra, Ganapathi Bhat,
Heiko Stuebner, Kalle Valo, Nishant Sarmukadam, Rob Herring,
Xinming Hu
Cc: David S. Miller, devicetree, linux-arm-kernel, linux-kernel,
linux-rockchip, linux-wireless, netdev
For quite some time, I wondered why the PCI mwifiex device built in my
Chromebook was unable to use the good old legacy interrupts. But as MSIs
were working fine, I never really bothered investigating. I finally had a
look, and the result isn't very pretty.
On this machine (rk3399-based kevin), the wake-up interrupt is described as
such:
&pci_rootport {
mvl_wifi: wifi@0,0 {
compatible = "pci1b4b,2b42";
reg = <0x83010000 0x0 0x00000000 0x0 0x00100000
0x83010000 0x0 0x00100000 0x0 0x00100000>;
interrupt-parent = <&gpio0>;
interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
pinctrl-names = "default";
pinctrl-0 = <&wlan_host_wake_l>;
wakeup-source;
};
};
Note how the interrupt is part of the properties directly attached to the
PCI node. And yet, this interrupt has nothing to do with a PCI legacy
interrupt, as it is attached to the wake-up widget that bypasses the PCIe RC
altogether (Yay for the broken design!). This is in total violation of the
IEEE Std 1275-1994 spec[1], which clearly documents that such interrupt
specifiers describe the PCI device interrupts, and must obey the
INT-{A,B,C,D} mapping. Oops!
The net effect of the above is that Linux tries to do something vaguely
sensible, and uses the same interrupt for both the wake-up widget and the
PCI device. This doesn't work for two reasons: (1) the wake-up widget grabs
the interrupt in exclusive mode, and (2) the PCI interrupt is still routed
to the RC, leading to a screaming interrupt. This simply cannot work.
To sort out this mess, we need to lift the confusion between the two
interrupts. This is done by extending the DT binding to allow the wake-up
interrupt to be described in a 'wake-up' subnode, sidestepping the issue
completely. On my Chromebook, it now looks like this:
&pci_rootport {
mvl_wifi: wifi@0,0 {
compatible = "pci1b4b,2b42";
reg = <0x83010000 0x0 0x00000000 0x0 0x00100000
0x83010000 0x0 0x00100000 0x0 0x00100000>;
pinctrl-names = "default";
pinctrl-0 = <&wlan_host_wake_l>;
wake-up {
interrupt-parent = <&gpio0>;
interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
wakeup-source;
};
};
};
The driver is then updated to look for this subnode first, and fallback to
the original, broken behaviour (spitting out a warning in the offending
configuration).
For good measure, there are two additional patches:
- The wake-up interrupt requesting is horribly racy, and could lead to
unpredictable behaviours. Let's fix that properly.
- A final patch implementing the above transformation for the whole
RK3399-based Chromebook range, which all use the same broken
configuration.
With all that, I finally have PCI legacy interrupts working with the mwifiex
driver on my Chromebook.
[1] http://www.devicetree.org/open-firmware/bindings/pci/pci2_1.pdf
Marc Zyngier (4):
dt-bindings/marvell-8xxx: Allow wake-up interrupt to be placed in a
separate node
mwifiex: Fetch wake-up interrupt from 'wake-up' subnode when it exists
mwifiex: Flag wake-up interrupt as IRQ_NOAUTOEN rather than disabling
it too late
arm64: dts: rockchip: gru: Move wifi wake-up interrupt into its own
subnode
.../bindings/net/wireless/marvell-8xxx.txt | 23 ++++++++++++++++++-
.../dts/rockchip/rk3399-gru-chromebook.dtsi | 8 ++++---
drivers/net/wireless/marvell/mwifiex/main.c | 13 +++++++++--
3 files changed, 38 insertions(+), 6 deletions(-)
--
2.20.1
^ permalink raw reply
* [PATCH 4/4] arm64: dts: rockchip: gru: Move wifi wake-up interrupt into its own subnode
From: Marc Zyngier @ 2019-02-24 14:04 UTC (permalink / raw)
To: Amitkumar Karwar, Enric Balletbo i Serra, Ganapathi Bhat,
Heiko Stuebner, Kalle Valo, Nishant Sarmukadam, Rob Herring,
Xinming Hu
Cc: David S. Miller, devicetree, linux-arm-kernel, linux-kernel,
linux-rockchip, linux-wireless, netdev
In-Reply-To: <20190224140426.3267-1-marc.zyngier@arm.com>
In order to get PCIe legacy interrupts working on gru-based Chromebooks,
let's move the wake-up interrupt out of the way and into its own
subnode. This ensures that this interrupt specifier will not be
mistaken as a PCI interrupt.
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi
index c400be64170e..61fff688770c 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi
@@ -310,11 +310,13 @@ ap_i2c_tp: &i2c5 {
compatible = "pci1b4b,2b42";
reg = <0x83010000 0x0 0x00000000 0x0 0x00100000
0x83010000 0x0 0x00100000 0x0 0x00100000>;
- interrupt-parent = <&gpio0>;
- interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
pinctrl-names = "default";
pinctrl-0 = <&wlan_host_wake_l>;
- wakeup-source;
+ wake-up {
+ interrupt-parent = <&gpio0>;
+ interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
+ wakeup-source;
+ };
};
};
--
2.20.1
^ permalink raw reply related
* [PATCH 3/4] mwifiex: Flag wake-up interrupt as IRQ_NOAUTOEN rather than disabling it too late
From: Marc Zyngier @ 2019-02-24 14:04 UTC (permalink / raw)
To: Amitkumar Karwar, Enric Balletbo i Serra, Ganapathi Bhat,
Heiko Stuebner, Kalle Valo, Nishant Sarmukadam, Rob Herring,
Xinming Hu
Cc: David S. Miller, devicetree, linux-arm-kernel, linux-kernel,
linux-rockchip, linux-wireless, netdev
In-Reply-To: <20190224140426.3267-1-marc.zyngier@arm.com>
The mwifiex driver makes unsafe assumptions about the state of the
wake-up interrupt. It requests it and only then disable it. Of
course, the interrupt may be screaming for whatever reason at that
time, and the handler will then be called without the interrupt
having been registered with the PM/wakeup subsystem. Oops.
The right way to handle this kind of situation is to flag the
interrupt with IRQ_NOAUTOEN before requesting it. It will then
stay disabled until someone (the wake-up subsystem) enables it.
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
drivers/net/wireless/marvell/mwifiex/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index 2105c2b7c627..82cf35e50579 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -1610,6 +1610,7 @@ static void mwifiex_probe_of(struct mwifiex_adapter *adapter)
"wake-up interrupt outside 'wake-up' subnode of %pOF\n",
adapter->dt_node);
+ irq_set_status_flags(adapter->irq_wakeup, IRQ_NOAUTOEN);
ret = devm_request_irq(dev, adapter->irq_wakeup,
mwifiex_irq_wakeup_handler, IRQF_TRIGGER_LOW,
"wifi_wake", adapter);
@@ -1619,7 +1620,6 @@ static void mwifiex_probe_of(struct mwifiex_adapter *adapter)
goto err_exit;
}
- disable_irq(adapter->irq_wakeup);
if (device_init_wakeup(dev, true)) {
dev_err(dev, "fail to init wakeup for mwifiex\n");
goto err_exit;
--
2.20.1
^ permalink raw reply related
* [PATCH 2/4] mwifiex: Fetch wake-up interrupt from 'wake-up' subnode when it exists
From: Marc Zyngier @ 2019-02-24 14:04 UTC (permalink / raw)
To: Amitkumar Karwar, Enric Balletbo i Serra, Ganapathi Bhat,
Heiko Stuebner, Kalle Valo, Nishant Sarmukadam, Rob Herring,
Xinming Hu
Cc: David S. Miller, devicetree, linux-arm-kernel, linux-kernel,
linux-rockchip, linux-wireless, netdev
In-Reply-To: <20190224140426.3267-1-marc.zyngier@arm.com>
Encoding the wake-up interrupt as part of the PCI DT node is completely
broken, as it violates the most basic rules of PCI description in OF:
the interrupts described in such node are supposed to apply to the
PCI device, and not to some non-PCI stuff on the side.
In such a configuration, both the PCI device and the wake-up widget
end-up trying to share an interrupt. Of course, this doesn't work:
The PCI device can only generate interrupts through the root port,
while the wake-up widget uses sideband signaling that bypasses PCI
altogether. Clearly, this was never tested.
So let's first try and obtain the wake-up interrupt from a 'wake-up'
subnode, and fallback to the main DT node otherwise. This ensures
that old DTs will carry on working as bad as before (with the added
warning to let the user know about the situation), and new DT will
enjoy legacy interrupts in case MSIs are unavailable.
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
drivers/net/wireless/marvell/mwifiex/main.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index 20cee5c397fb..2105c2b7c627 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -1590,17 +1590,26 @@ static void mwifiex_probe_of(struct mwifiex_adapter *adapter)
{
int ret;
struct device *dev = adapter->dev;
+ struct device_node *wup_node;
if (!dev->of_node)
goto err_exit;
adapter->dt_node = dev->of_node;
- adapter->irq_wakeup = irq_of_parse_and_map(adapter->dt_node, 0);
+ wup_node = of_get_child_by_name(adapter->dt_node, "wake-up");
+ if (!wup_node)
+ wup_node = adapter->dt_node;
+ adapter->irq_wakeup = irq_of_parse_and_map(wup_node, 0);
if (!adapter->irq_wakeup) {
dev_dbg(dev, "fail to parse irq_wakeup from device tree\n");
goto err_exit;
}
+ if (dev_is_pci(dev) && adapter->dt_node == wup_node)
+ dev_warn(dev,
+ "wake-up interrupt outside 'wake-up' subnode of %pOF\n",
+ adapter->dt_node);
+
ret = devm_request_irq(dev, adapter->irq_wakeup,
mwifiex_irq_wakeup_handler, IRQF_TRIGGER_LOW,
"wifi_wake", adapter);
--
2.20.1
^ permalink raw reply related
* [PATCH 1/4] dt-bindings/marvell-8xxx: Allow wake-up interrupt to be placed in a separate node
From: Marc Zyngier @ 2019-02-24 14:04 UTC (permalink / raw)
To: Amitkumar Karwar, Enric Balletbo i Serra, Ganapathi Bhat,
Heiko Stuebner, Kalle Valo, Nishant Sarmukadam, Rob Herring,
Xinming Hu
Cc: David S. Miller, devicetree, linux-arm-kernel, linux-kernel,
linux-rockchip, linux-wireless, netdev
In-Reply-To: <20190224140426.3267-1-marc.zyngier@arm.com>
The DT binding for the PCI version of the Marvell 8xxx wifi devices
is pretty broken (the fact that a PCI device requires a DT binding
is quite telling on its own).
The binding allows the description of a wake-up interrupt as a sideband
signal, allowing the wifi device to wake-up the system. So far, so good.
Until you realise that placing an interrupt in the PCI device DT node
has a specific meaning, and applies to the actual PCI function, and
not some random sideband stuff. This is of course in total violation
of the OF specification (IEEE Std 1275-1994), but hey, who cares.
Let's thus change the binding to be somewhat compatible with the spec,
by placing the wake-up interrupt in a subnode called "wake-up". This
still is an optional property, but a recommended one for PCI devices.
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
.../bindings/net/wireless/marvell-8xxx.txt | 23 ++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt b/Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
index 9bf9bbac16e2..f9340ca37047 100644
--- a/Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
+++ b/Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
@@ -33,10 +33,16 @@ Optional properties:
this interrupt number. during system suspend, the irq will be enabled
so that the wifi chip can wakeup host platform under certain condition.
during system resume, the irq will be disabled to make sure
- unnecessary interrupt is not received.
+ unnecessary interrupt is not received. For PCI devices, it
+ is recommended that this property is placed in a "wake-up"
+ sub-node in order to limit the ambiguity with PCI legacy
+ interrupts.
- vmmc-supply: a phandle of a regulator, supplying VCC to the card
- mmc-pwrseq: phandle to the MMC power sequence node. See "mmc-pwrseq-*"
for documentation of MMC power sequence bindings.
+ - wake-up: a subnode containing the interrupt specifier for a wake-up
+ interrupt. This is the recommended configuration for PCI
+ devices.
Example:
@@ -66,3 +72,18 @@ so that firmware can wakeup host using this device side pin.
marvell,wakeup-pin = <3>;
};
};
+
+&pci_rootport {
+ mvl_wifi: wifi@0,0 {
+ compatible = "pci1b4b,2b42";
+ reg = <0x83010000 0x0 0x00000000 0x0 0x00100000
+ 0x83010000 0x0 0x00100000 0x0 0x00100000>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&wlan_host_wake_l>;
+ wake-up {
+ interrupt-parent = <&gpio0>;
+ interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
+ wakeup-source;
+ };
+ };
+};
--
2.20.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox