Skip to content

Conversation

@akerouanton
Copy link
Contributor

These attributes are supported since kernel v5.14 (see 1). Here's what iproute2 shows:

$ ip -d link show eth0
4: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 65535 qdisc pfifo_fast state UP mode DEFAULT group default qlen 1000
    ... parentbus virtio parentdev virtio0

@akerouanton
Copy link
Contributor Author

akerouanton commented Jan 20, 2025

It seems the failing test is a flake. I was able to reproduce it once out of a few runs on main.

--- FAIL: TestRuleListFiltered (1.01s)
    --- FAIL: TestRuleListFiltered/IPv6 (1.01s)
        --- FAIL: TestRuleListFiltered/IPv6/returns_one_rule_filtered_by_Dst (1.00s)

These attributes are supported since kernel v5.14 (see [1]). Here's
what iproute2 shows:

```
$ ip -d link show eth0
4: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 65535 qdisc pfifo_fast state UP mode DEFAULT group default qlen 1000
    ... parentbus virtio parentdev virtio0
```

[1]: torvalds/linux@00e77ed

Signed-off-by: Albin Kerouanton <[email protected]>
Copy link
Contributor

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@aboch
Copy link
Collaborator

aboch commented Jan 21, 2025

Thanks @akerouanton
Would it be possible to add a new or extend and existing UT function to cover these changes?

@akerouanton
Copy link
Contributor Author

@aboch Testing this one is quite challenging because you need a physical interface to have a 'parent' reported, and there's no way to guarantee that the same parent will be available across CI, developers' machine, etc… or even if there'll be at least one interface with a 'parent'.

I'd need to mock the netlink response to test that, but it seems no other tests are using mocks.

@aboch
Copy link
Collaborator

aboch commented Jan 28, 2025

LGTM

@aboch aboch merged commit 7c2350b into vishvananda:main Jan 28, 2025
2 checks passed
@akerouanton akerouanton deleted the parent-dev-bus-name branch January 28, 2025 01:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants