Messages in this thread | ![/](/images/icornerl.gif) | | Date | Thu, 7 Dec 2023 17:56:52 +0800 | Subject | Re: Re: [PATCH bpf-next] netkit: Add some ethtool ops to provide information to user | From | Feng Zhou <> |
| |
在 2023/12/6 18:57, Daniel Borkmann 写道: > On 12/6/23 7:59 AM, Feng Zhou wrote: >> 在 2023/12/4 23:22, Daniel Borkmann 写道: >>> 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 >>> 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.
| ![\](/images/icornerr.gif) |