The `devlink -j dev show` command output may not contain the "flavour"
key, for example:
$ devlink -j dev show
{"dev":{"pci/0001:00:00.0":{},"pci/0002:00:00.0":{}}}
This will cause a KeyError exception. Fix this by checking the key
existence first.
Also, if max lanes is 0 the port splitting won't be tested at all.
but the script will end normally and thus causing a false-negative
test result.
Use a test_ran flag to determine if these tests were skipped and
return KSFT_SKIP accordingly.
Link: https://bugs.launchpad.net/bugs/1937133
Fixes: f3348a82e727 ("selftests: net: Add port split test")
Signed-off-by: Po-Hsu Lin <po-hsu.lin(a)canonical.com>
---
tools/testing/selftests/net/devlink_port_split.py | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/devlink_port_split.py b/tools/testing/selftests/net/devlink_port_split.py
index 2b5d6ff..462f3df 100755
--- a/tools/testing/selftests/net/devlink_port_split.py
+++ b/tools/testing/selftests/net/devlink_port_split.py
@@ -61,7 +61,7 @@ class devlink_ports(object):
for port in ports:
if dev in port:
- if ports[port]['flavour'] == 'physical':
+ if 'flavour' in ports[port] and ports[port]['flavour'] == 'physical':
arr.append(Port(bus_info=port, name=ports[port]['netdev']))
return arr
@@ -231,6 +231,7 @@ def make_parser():
def main(cmdline=None):
+ test_ran = False
parser = make_parser()
args = parser.parse_args(cmdline)
@@ -277,6 +278,11 @@ def main(cmdline=None):
split_splittable_port(port, lane, max_lanes, dev)
lane //= 2
+ test_ran = True
+
+ if not test_ran:
+ print("No suitable device for the test, test skipped")
+ sys.exit(KSFT_SKIP)
if __name__ == "__main__":
--
2.7.4
As discussed here is a small series to address the confusing lifetime
scheme for the hwpt. This was some leftover from prior rework that was
never fully cleaned up.
Make it clear that the ioas and ioas->hwpt_list are associated with the
hwpt during creation and never changed until it is destroyed. A hwpt with
a positive reference count is always valid for device attachment.
This also improves the selftest handling of the mock domains so that we
can implement more testing for the hwpt model.
This is a step toward the nesting and replace series.
I have this on github:
https://github.com/jgunthorpe/linux/commits/iommufd_hwpt
v3:
- Split patch to just make the hwpt_item handling consistent
- Remove two confusing comments
- Four new patches to clean up the confusing 'domain_id' in the selftest
v2: https://lore.kernel.org/r/0-v2-406f7ac07936+6a-iommufd_hwpt_jgg@nvidia.com
- Basically completely different
- Add a test for HWPT cases by enhancing the mock iommu driver to be more
complete
- Move the device attachment as well into iommufd_hw_pagetable_alloc()
so destroy is more symmetric
v1: https://lore.kernel.org/r/0-v1-4336b5cb2fe4+1d7-iommufd_hwpt_jgg@nvidia.com
Jason Gunthorpe (12):
iommufd: Assert devices_lock for iommufd_hw_pagetable_has_group()
iommufd: Add iommufd_lock_obj() around the auto-domains hwpts
iommufd: Consistently manage hwpt_item
iommufd: Move ioas related HWPT destruction into
iommufd_hw_pagetable_destroy()
iommufd: Move iommufd_device to iommufd_private.h
iommufd: Make iommufd_hw_pagetable_alloc() do iopt_table_add_domain()
iommufd/selftest: Rename the sefltest 'device_id' to 'stdev_id'
iommufd/selftest: Rename domain_id to stdev_id for FIXTURE
iommufd_ioas
iommufd/selftest: Rename domain_id to hwpt_id for FIXTURE
iommufd_mock_domain
iommufd/selftest: Rename the remaining mock device_id's to stdev_id
iommufd/selftest: Make selftest create a more complete mock device
iommufd/selftest: Add a selftest for iommufd_device_attach() with a
hwpt argument
drivers/iommu/iommufd/device.c | 149 +++++--------
drivers/iommu/iommufd/hw_pagetable.c | 70 ++++++-
drivers/iommu/iommufd/iommufd_private.h | 35 +++-
drivers/iommu/iommufd/iommufd_test.h | 2 +-
drivers/iommu/iommufd/selftest.c | 198 +++++++++++++++---
tools/testing/selftests/iommu/iommufd.c | 68 +++---
.../selftests/iommu/iommufd_fail_nth.c | 38 ++--
tools/testing/selftests/iommu/iommufd_utils.h | 16 +-
8 files changed, 368 insertions(+), 208 deletions(-)
base-commit: 8473685c99683f9e2c7e2a42ef3d3f14a79be274
--
2.39.2
On 3/2/23 17:57, Stephen Boyd wrote:
> Quoting Rob Herring (2023-03-02 12:18:34)
>> On Thu, Mar 2, 2023 at 1:44 PM Stephen Boyd <sboyd(a)kernel.org> wrote:
>>>
>>> Quoting Rob Herring (2023-03-02 09:13:59)
>>>>
>>>> Good to see bindings for this. I've been meaning to do something about
>>>> the DT unittest ones being undocumented, but I hadn't really decided
>>>> whether it was worth writing schemas for them. The compatibles at
>>>> least show up with 'make dt_compatible_check'. Perhaps we want to just
>>>> define some vendor (not 'linux') that's an exception rather than
>>>> requiring schemas (actually, that already works for 'foo').
>>>
>>> Sure. Maybe "kunit" should be the vendor prefix? Or "dtbunit"?
>>
>> We'd want to use the same thing on the DT unittests or anything else
>> potentially. How about just 'test'?
>
> Sounds good.
>
>>
>>>> It's
>>>> likely that we want test DTs that fail normal checks and schemas get
>>>> in the way of that as we don't have a way to turn off checks.
>>>
>>> Having the schemas is nice to make sure tests that are expecting some
>>> binding are actually getting that. But supporting broken bindings is
>>> also important to test any error paths in functions that parse
>>> properties. Maybe we keep the schema and have it enforce that incorrect
>>> properties are being set?
>>
>> I wasn't suggesting throwing them out. More why I hadn't written any I guess.
>>
>>> Do we really need to test incorrect bindings? Doesn't the
>>> dt_bindings_check catch these problems so we don't have to write DTB
>>> verifiers in the kernel?
>>
>> Fair enough. Using my frequently stated position against me. :)
>>
>> I do have a secret plan to implement (debug) type checks into the
>> of_property_* APIs by extracting the type information from schemas
>> into C.
>>
>
> Ok. I suspect we may want to test error paths though so I don't know
Yes, exactly.
> what to do here. For now I'll just leave the bindings in place and
> change the prefix to "test".
>
>>
>>>> We already have GPIO tests in the DT unittests, so why is clocks
>>>> different? Or should the GPIO tests be moved out (yes, please!)?
>>>
>>> Ah I didn't notice the GPIO tests in there. There are i2c tests too,
>>> right? All I can say is clks are using kunit, that's the difference ;-)
>>
>> Yeah, they should perhaps all move to the subsystems.
>
> Got it.
>
>>
>>>> What happens when/if the DT unittest is converted to kunit? I think
>>>> that would look confusing from the naming. My initial thought is
>>>> 'kunit' should be dropped from the naming of a lot of this. Note that
>>>> the original kunit submission converted the DT unittests. I would
>>>> still like to see that happen. Frank disagreed over what's a unit test
>>>> or not, then agreed, then didn't... I don't really care. If there's a
>>>> framework to use, then we should use it IMO.
>>>
>>> Honestly I don't want to get involved in migrating the existing DT
>>> unittest code to kunit. I'm aware that it was attempted years ago when
>>> kunit was introduced. Maybe if the overlay route works well enough I can
>>> completely sidestep introducing any code in drivers/of/ besides some
>>> kunit wrappers for this. I'll cross my fingers!
>>
>> Yeah, I wasn't expecting you to. I just want to make sure this meshes
>> with any future conversion to kunit.
>
> Phew!
>
>>
>> There's also some plans to always populate the DT root node if not
>> present. That may help here. Or not. There's been a few versions
>> posted with Frank's in the last week or 2.
>>
>
> Ok. I think I have some time to try this overlay approach so let me see
> what is needed.
Please avoid overlays. See my other replies in this thread for why.