All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Hubbard <jhubbard@nvidia.com>
To: Leon Romanovsky <leon@kernel.org>
Cc: Jason Gunthorpe <jgg@ziepe.ca>,
	Noa Osherovich <noaos@mellanox.com>, <linux-rdma@vger.kernel.org>,
	John Hubbard <jhubbard@nvidia.com>
Subject: [PATCH 0/1] pyverbs: fix speed_to_str(), to handle disabled links
Date: Fri, 20 Dec 2019 17:32:55 -0800	[thread overview]
Message-ID: <20191221013256.100409-1-jhubbard@nvidia.com> (raw)

Hi,

This came up when I was running rdma-core tests on a two-machine setup,
where each card had two ports, but there was only one cable. So only
one port on each end was connected.

The main thing I expect to be up for debate is, what string to return
for speed, when a port is disabled or down? I initially thought about
returning  '(Disabled/down)', but it seems more accurate to just report
'0.0 Gbps', so that's what I settled on.

Background: here's what I wrote when discussing this over on linux-mm
with Leon [1]:

It looks like this test suite assumes that every link is connected!
(Probably in most test systems, they are.) But in my setup, the ConnectX
cards each have two slots, and I only have (and only need) one cable. So
one link is up, and the other is disabled.

This leads to the other problem, which is that if a link is disabled,
the test suite finds a "0" token for attr.active_speed. That token is
not in the approved list, and so d.speed_to_str() asserts.

With some diagnostics added, I can see it checking each link: one
passes, and the other asserts:

diff --git a/tests/test_device.py b/tests/test_device.py
index 524e0e89..7b33d7db 100644
--- a/tests/test_device.py
+++ b/tests/test_device.py
@@ -110,6 +110,12 @@ class DeviceTest(unittest.TestCase):
         assert 'Invalid' not in d.translate_mtu(attr.max_mtu)
         assert 'Invalid' not in d.translate_mtu(attr.active_mtu)
         assert 'Invalid' not in d.width_to_str(attr.active_width)
+        print("")
+        print('Diagnostics ===========================================')
+        print('phys_state:    ', d.phys_state_to_str(attr.phys_state))
+        print('active_width): ', d.width_to_str(attr.active_width))
+        print('active_speed:  ',   d.speed_to_str(attr.active_speed))
+        print('END of Diagnostics ====================================')
         assert 'Invalid' not in d.speed_to_str(attr.active_speed)
         assert 'Invalid' not in d.translate_link_layer(attr.link_layer)
         assert attr.max_msg_sz > 0x1000

         assert attr.max_msg_sz > 0x1000

...and the test run from that is:

# ./build/bin/run_tests.py --verbose tests.test_device.DeviceTest
test_dev_list (tests.test_device.DeviceTest) ... ok
test_open_dev (tests.test_device.DeviceTest) ... ok
test_query_device (tests.test_device.DeviceTest) ... ok
test_query_device_ex (tests.test_device.DeviceTest) ... ok
test_query_gid (tests.test_device.DeviceTest) ... ok
test_query_port (tests.test_device.DeviceTest) ...
Diagnostics ===========================================
phys_state:     Link up (5)
active_width):  4X (2)
active_speed:   25.0 Gbps (32)
END of Diagnostics ====================================

Diagnostics ===========================================
phys_state:     Disabled (3)
active_width):  4X (2)
active_speed:   Invalid speed
END of Diagnostics ====================================
FAIL
test_query_port_bad_flow (tests.test_device.DeviceTest) ... ok

======================================================================
FAIL: test_query_port (tests.test_device.DeviceTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/kernel_work/rdma-core/tests/test_device.py", line 135, in test_query_port
    self.verify_port_attr(port_attr)
  File "/kernel_work/rdma-core/tests/test_device.py", line 119, in verify_port_attr
    assert 'Invalid' not in d.speed_to_str(attr.active_speed)
AssertionError

----------------------------------------------------------------------
Ran 7 tests in 0.055s

FAILED (failures=1)

[1] https://lore.kernel.org/r/b70ac328-2dc0-efe3-05c2-3e040b662256@nvidia.com


John Hubbard (1):
  pyverbs: fix speed_to_str(), to handle disabled links

 pyverbs/device.pyx | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.24.1


             reply	other threads:[~2019-12-21  1:33 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-21  1:32 John Hubbard [this message]
2019-12-21  1:32 ` [PATCH 1/1] pyverbs: fix speed_to_str(), to handle disabled links John Hubbard
2019-12-23 14:39   ` Noa Osherovich
2019-12-25  1:40     ` John Hubbard
2019-12-26  9:12   ` Leon Romanovsky
2019-12-21 10:03 ` [PATCH 0/1] " Leon Romanovsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191221013256.100409-1-jhubbard@nvidia.com \
    --to=jhubbard@nvidia.com \
    --cc=jgg@ziepe.ca \
    --cc=leon@kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=noaos@mellanox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is 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.