Thread (9 messages) 9 messages, 3 authors, 2023-12-07

Re: Re: [PATCH bpf-next] netkit: Add some ethtool ops to provide information to user

From: Feng Zhou <hidden>
Date: 2023-12-07 09:57:09
Also in: bpf, lkml

在 2023/12/6 18:57, Daniel Borkmann 写道:
On 12/6/23 7:59 AM, Feng Zhou wrote:
quoted
在 2023/12/4 23:22, Daniel Borkmann 写道:
quoted
Thanks, so the netkit_get_link_ksettings is optional. 
Yes, netkit_get_link_ksettings really not necessary, I just added it 
in line with veth.

I don't quite
quoted
follow what you
mean with regards to your business logic in veth to obtain peer 
ifindex. What does
the script do exactly with the peer ifindex (aka /why/ is it needed), 
could you
elaborate some more - it's still somewhat too vague? 🙂 E.g. why it 
does not suffice
to look at the device type or other kind of attributes?
The scripting logic of the business colleagues should just be simple 
logging records, using ethtool. Then they saw that netkit has this 
missing, so raised this requirement, so I sent this patch, wanting to 
hear your opinions. If you don't think it's necessary, let the 
business colleagues modify it.
So it is basically only logging the peer_ifindex data from ethtool but 
nothing
more is done with it? Meaning, this was raised as a requirement because 
this was
missing from the logging data, or was there anything more to it?

The peer ifindex is already available via IFLA_LINK (which does 
dev_get_iflink()
internally to fetch the peer's ifindex). This is also what you see in 
iproute2:

# ip l
[...]
163: nk0@nk1: <BROADCAST,MULTICAST,NOARP,M-DOWN> mtu 1500 qdisc noop 
state DOWN group default qlen 1000
     link/ether 00:00:00:00:00:00 brd ff:ff:ff:ff:ff:ff
164: nk1@nk0: <BROADCAST,MULTICAST,NOARP,M-DOWN> mtu 1500 qdisc noop 
state DOWN group default qlen 1000
     link/ether 00:00:00:00:00:00 brd ff:ff:ff:ff:ff:ff
[...]
# ip netns add blue
# ip link set nk1 netns blue
# ip l
[...]
163: nk0@if164: <BROADCAST,MULTICAST,NOARP> mtu 1500 qdisc noop state 
DOWN group default qlen 1000
     link/ether 00:00:00:00:00:00 brd ff:ff:ff:ff:ff:ff link-netns blue
[...]

The `if164` denotes the peer's ifindex and `link-netns blue` provides 
info on the netns of the peer.
This is much better and more generic than the ethtool hack. I would 
suggest changing the logic to
rather look at data populated by rtnl_fill_link_netnsid() instead.

Thanks,
Daniel
Yes, if veth supports ethtool -S to get the function of peer's ifindex, 
it is not necessary for netkit and can be replaced by other ways.


Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help