| Message ID | 20191221013256.100409-2-jhubbard@nvidia.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Leon Romanovsky |
| Headers | show |
| Series | [1/1] pyverbs: fix speed_to_str(), to handle disabled links | expand |
Hi John On 12/21/2019 3:32 AM, John Hubbard wrote: > For disabled links, the raw speed token is 0. However, speed_to_str() > doesn't have that in the list. This leads to an assertion when running > tests (test_query_port) when one link is down and other link(s) are up. > > Fix this by returning '0.0 Gbps' for the zero speed case. > > Cc: Noa Osherovich <noaos@mellanox.com> > Signed-off-by: John Hubbard <jhubbard@nvidia.com> > --- > pyverbs/device.pyx | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/pyverbs/device.pyx b/pyverbs/device.pyx > index 33d133fd..cf7b75de 100755 > --- a/pyverbs/device.pyx > +++ b/pyverbs/device.pyx > @@ -923,8 +923,8 @@ def width_to_str(width): > > > def speed_to_str(speed): > - l = {1: '2.5 Gbps', 2: '5.0 Gbps', 4: '5.0 Gbps', 8: '10.0 Gbps', > - 16: '14.0 Gbps', 32: '25.0 Gbps', 64: '50.0 Gbps'} > + l = {0: '0.0 Gbps', 1: '2.5 Gbps', 2: '5.0 Gbps', 4: '5.0 Gbps', > + 8: '10.0 Gbps', 16: '14.0 Gbps', 32: '25.0 Gbps', 64: '50.0 Gbps'} > try: > return '{s} ({n})'.format(s=l[speed], n=speed) > except KeyError: This seems OK to me. BTW, what's the reported active_width for disabled links? Maybe width_to_str could use a similar fix. Thanks, Noa
On 12/23/19 6:39 AM, Noa Osherovich wrote: ... >> diff --git a/pyverbs/device.pyx b/pyverbs/device.pyx >> index 33d133fd..cf7b75de 100755 >> --- a/pyverbs/device.pyx >> +++ b/pyverbs/device.pyx >> @@ -923,8 +923,8 @@ def width_to_str(width): >> >> >> def speed_to_str(speed): >> - l = {1: '2.5 Gbps', 2: '5.0 Gbps', 4: '5.0 Gbps', 8: '10.0 Gbps', >> - 16: '14.0 Gbps', 32: '25.0 Gbps', 64: '50.0 Gbps'} >> + l = {0: '0.0 Gbps', 1: '2.5 Gbps', 2: '5.0 Gbps', 4: '5.0 Gbps', >> + 8: '10.0 Gbps', 16: '14.0 Gbps', 32: '25.0 Gbps', 64: '50.0 Gbps'} >> try: >> return '{s} ({n})'.format(s=l[speed], n=speed) >> except KeyError: > > This seems OK to me. BTW, what's the reported active_width for disabled links? > Maybe width_to_str could use a similar fix. > Thanks for reviewing this! The reported active_width for disabled links on my systems is showing up the same as for active links ("4X" in this case). So I don't think we need a fix for width_to_str(). thanks,
On Fri, Dec 20, 2019 at 05:32:56PM -0800, John Hubbard wrote: > For disabled links, the raw speed token is 0. However, speed_to_str() > doesn't have that in the list. This leads to an assertion when running > tests (test_query_port) when one link is down and other link(s) are up. > > Fix this by returning '0.0 Gbps' for the zero speed case. > > Cc: Noa Osherovich <noaos@mellanox.com> > Signed-off-by: John Hubbard <jhubbard@nvidia.com> > --- > pyverbs/device.pyx | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > Thanks, applied
diff --git a/pyverbs/device.pyx b/pyverbs/device.pyx index 33d133fd..cf7b75de 100755 --- a/pyverbs/device.pyx +++ b/pyverbs/device.pyx @@ -923,8 +923,8 @@ def width_to_str(width): def speed_to_str(speed): - l = {1: '2.5 Gbps', 2: '5.0 Gbps', 4: '5.0 Gbps', 8: '10.0 Gbps', - 16: '14.0 Gbps', 32: '25.0 Gbps', 64: '50.0 Gbps'} + l = {0: '0.0 Gbps', 1: '2.5 Gbps', 2: '5.0 Gbps', 4: '5.0 Gbps', + 8: '10.0 Gbps', 16: '14.0 Gbps', 32: '25.0 Gbps', 64: '50.0 Gbps'} try: return '{s} ({n})'.format(s=l[speed], n=speed) except KeyError:
For disabled links, the raw speed token is 0. However, speed_to_str() doesn't have that in the list. This leads to an assertion when running tests (test_query_port) when one link is down and other link(s) are up. Fix this by returning '0.0 Gbps' for the zero speed case. Cc: Noa Osherovich <noaos@mellanox.com> Signed-off-by: John Hubbard <jhubbard@nvidia.com> --- pyverbs/device.pyx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)