Hi Martin,

On Wed, Dec 14, 2022 at 11:52:08AM +0100, Martin Pitt wrote:

Hello Coiby,

thanks for the fix!

Coiby Xu [2022-12-14 14:48 +0800]:

...

diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh index 13e99015..f82349cb 100755 --- a/dracut-module-setup.sh +++ b/dracut-module-setup.sh @@ -387,6 +387,7 @@ kdump_install_nic_driver() { _drivers=()

 for _netif in $1; do

•    ! ip addr show "$_netif" | grep "link/loopback" || continue
  

I confirm that this works, in the sense of "kdump to ssh localhost succeeds again".

Thanks for reviewing the patch!

But it is a bit heavy (calling `ip` and `grep`) and brittle (relying on precise output format of `ip show`, which isn't very machine-readable).

Thanks for raising the above concerns! To find a simpler solution, I checked linux/drivers/net/loopback.c and find there is only one loopback interface and the name always "lo". So the following code would be sufficient,

[ $_netif != lo ] || continue

Btw, ip and grep are used quite often in kexec-tools. I'm curious in what way do you feel they are heavy? In fact, the cost of ip and grep is negligible compared with the cost of calling dracut to build initramfs.

I'm also worried that this might apply to other virtual devices as well, such as veth or wireguard. My laptop has a wireguard device which might also cause trouble:

$ ip link show wgpitti 4: wgpitti: <POINTOPOINT,NOARP,UP,LOWER_UP> mtu 1420 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000 link/none

$ ls -l /sys/class/net/wgpitti/device ls: cannot access '/sys/class/net/wgpitti/device': No such file or directory

Hence I would recommend this instead:

[ -e "/sys/class/net/$_netif/device/driver" ] || continue

and rename the commit to "skip installing driver for virtual devices" or so.

I tested this with ssh/localhost as well. It has the added benefit of not relying on "ip show" format, is more robust, more generic, and not even a new concept: that script already looks at /sys/class/net anyway.

Unfortunately what you propose won't work.

1. kexec-tools wants to only install needed drivers to save memory and it's this dracut module's job to install needed drivers which may include the drivers of virtual devices e.g. bond, team or vlan device as well i.e. no one else will install these drivers.

2. ethtool talks to the driver via the netlink interface [1]. It doesn't use /sys/class/net/$_netif/device/driver to get the driver info.

[1] https://www.kernel.org/doc/html/v5.8/networking/ethtool-netlink.html

...

     _driver=$(_get_nic_driver "$_netif")
     if [[ -z $_driver ]]; then
         derror "Failed to get the driver of $_netif"

@@ -404,6 +405,7 @@ kdump_install_nic_driver() { _drivers+=("$_driver") done

• [[ -n ${_drivers[*]} ]] || return

This is fine of course.

Thanks for the confirmation!

...

 instmods "${_drivers[@]}"

}

Thanks,

Martin

This version of patch looks good to me.

Reviewed-by: Lichen Liu lichliu@redhat.com

On Fri, Dec 16, 2022 at 9:00 AM Coiby Xu coxu@redhat.com wrote:

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2151500

Currently, kdump initrd fails to be built when dumping vmcore to localhost via ssh or nfs,

kdumpctl[3331]: Cannot get driver information: Operation not supported kdumpctl[1991]: dracut: Failed to get the driver of lo dracut[2020]: Failed to get the driver of lo kdumpctl[1775]: kdump: mkdumprd: failed to make kdump initrd kdumpctl[1775]: kdump: Starting kdump: [FAILED] systemd[1]: kdump.service: Main process exited, code=exited, status=1/FAILURE systemd[1]: kdump.service: Failed with result 'exit-code'. systemd[1]: Failed to start Crash recovery kernel arming. systemd[1]: kdump.service: Consumed 1.710s CPU time.

This is because the loopback interface is used for transferring vmcore and ethtool can't get the driver of the loopback interface. In fact, once COFNIG_NET is enabled, the loopback device is enabled and there is no driver for the loopback device. So skip installing driver for the loopback device. The loopback interface is implemented in linux/drivers/net/loopback.c and always has the name "lo". So we can safely tell if a network interface is the loopback interface by its name.

Fixes: a65dde2d ("Reduce kdump memory consumption by only installing needed NIC drivers") Reported-by: Martin Pitt mpitt@redhat.com Reported-by: Rich Megginson rmeggins@redhat.com Signed-off-by: Coiby Xu coxu@redhat.com

---

v2

• distinguish the loopback interface simply by its name

---

dracut-module-setup.sh | 2 ++ 1 file changed, 2 insertions(+)

diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh index 13e99015..aac3d8e5 100755 --- a/dracut-module-setup.sh +++ b/dracut-module-setup.sh @@ -387,6 +387,7 @@ kdump_install_nic_driver() { _drivers=()

 for _netif in $1; do

•    [[ $_netif != lo ]] || continue
     _driver=$(_get_nic_driver "$_netif")
     if [[ -z $_driver ]]; then
         derror "Failed to get the driver of $_netif"
  

@@ -404,6 +405,7 @@ kdump_install_nic_driver() { _drivers+=("$_driver") done

• [[ -n ${_drivers[*]} ]] || return instmods "${_drivers[@]}"

}

-- 2.38.1 _______________________________________________ kexec mailing list -- kexec@lists.fedoraproject.org To unsubscribe send an email to kexec-leave@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/kexec@lists.fedoraproject.org Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue

On Mon, Dec 19, 2022 at 04:00:32PM +0100, Philipp Rudo wrote:

Hi Coiby,

a little nit picking from my side ;-)

On Fri, 16 Dec 2022 08:52:47 +0800 Coiby Xu coxu@redhat.com wrote:

...

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2151500

Currently, kdump initrd fails to be built when dumping vmcore to localhost via ssh or nfs,

kdumpctl[3331]: Cannot get driver information: Operation not supported kdumpctl[1991]: dracut: Failed to get the driver of lo dracut[2020]: Failed to get the driver of lo kdumpctl[1775]: kdump: mkdumprd: failed to make kdump initrd kdumpctl[1775]: kdump: Starting kdump: [FAILED] systemd[1]: kdump.service: Main process exited, code=exited, status=1/FAILURE systemd[1]: kdump.service: Failed with result 'exit-code'. systemd[1]: Failed to start Crash recovery kernel arming. systemd[1]: kdump.service: Consumed 1.710s CPU time.

This is because the loopback interface is used for transferring vmcore and ethtool can't get the driver of the loopback interface. In fact, once COFNIG_NET is enabled, the loopback device is enabled and there is no driver for the loopback device. So skip installing driver for the loopback device. The loopback interface is implemented in linux/drivers/net/loopback.c and always has the name "lo". So we can safely tell if a network interface is the loopback interface by its name.

Fixes: a65dde2d ("Reduce kdump memory consumption by only installing needed NIC drivers") Reported-by: Martin Pitt mpitt@redhat.com Reported-by: Rich Megginson rmeggins@redhat.com Signed-off-by: Coiby Xu coxu@redhat.com

---

v2

• distinguish the loopback interface simply by its name

---

dracut-module-setup.sh | 2 ++ 1 file changed, 2 insertions(+)

diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh index 13e99015..aac3d8e5 100755 --- a/dracut-module-setup.sh +++ b/dracut-module-setup.sh @@ -387,6 +387,7 @@ kdump_install_nic_driver() { _drivers=()

 for _netif in $1; do

•    [[ $_netif != lo ]] || continue
  

personally I find this double negative here hard to understand. So instead of saying "if the _netif is not lo and the test fails" you could just say "if the _netif is lo", aka.

[[ $_netif == "lo" ]] && continue

That would also better match the rest of the function.

Thanks for offering a way to improve the code readability!

Anyway, you choose what you prefer Reviewed-by: Philipp Rudo prudo@redhat.com

And thanks for the code review!

...

     _driver=$(_get_nic_driver "$_netif")
     if [[ -z $_driver ]]; then
         derror "Failed to get the driver of $_netif"

@@ -404,6 +405,7 @@ kdump_install_nic_driver() { _drivers+=("$_driver") done

• [[ -n ${_drivers[*]} ]] || return instmods "${_drivers[@]}"

}