Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Ethtool]: Fixing Build Failure on ARM Platform #7226

Merged
merged 6 commits into from
Apr 8, 2021

Conversation

vivekrnv
Copy link
Contributor

@vivekrnv vivekrnv commented Apr 3, 2021

Signed-off-by: Vivek Reddy Karri vkarri@nvidia.com

Why I did it

Compiling ethtool from source is causing ethtool unit tests to fail on ARM Platforms.

These tests are failing: (By default netlink-interface is enabled while compiling ethtool)
Link: (Test File Link)

FAIL: test-cmdline
==================

E: ethtool 16_char_devname! returns 1
E: ethtool 
127_char_devname0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcde returns 1
E: ethtool --change devname xcvr external returns 0
E: ethtool --change devname speed 100 duplex half port tp autoneg on advertise 0x1 phyad 1 xcvr external wol p sopass 01:23:45:67:89:ab msglvl 1 returns 0

FAIL test-cmdline (exit status: 1)

Tested this on Local ARM Emulated Container:

(Docker Container Emulating ARM)
vkarri@3a03c70eed35:/tmp/ethtool$ ./ethtool 16_char_devname!
netlink interface initialization failed, device name longer than 15 not supported
vkarri@3a03c70eed35:/tmp/ethtool$ echo $?
1 (Expected 0)
vkarri@3a03c70eed35:~/ethtool$ ./ethtool 16_char_devnameee
netlink interface initialization failed, device name longer than 15 not supported

Checked for dependencies: (all are present)
vkarri@3a03c70eed35:~/ethtool$ apt-cache policy libmnl0
libmnl0:
  Installed: 1.0.4-2
  Candidate: 1.0.4-2
  Version table:
 *** 1.0.4-2 500
        500 http://deb.debian.org/debian buster/main armhf Packages
        500 http://packages.trafficmanager.net/debian/debian buster/main armhf Packages
        100 /var/lib/dpkg/status

vkarri@3a03c70eed35:~/ethtool$ apt-cache policy libc6  
libc6:
  Installed: 2.28-10
  Candidate: 2.28-10
  Version table:
 *** 2.28-10 500
        500 http://deb.debian.org/debian buster/main armhf Packages
        500 http://packages.trafficmanager.net/debian/debian buster/main armhf Packages
        100 /var/lib/dpkg/status

How I did it

Disabled netlink-interface for ethtool.

Even though Netlink is not available, it doesn't seem to impact what ethtool was supposed to do. In fact the older version which was in use before this PR #5725 did not have netlink support and everything seemed to work well

Article on Netlink-Support for ethtool: https://lwn.net/Articles/783633/

How to verify it

Build Passed on Local Slave ARM Emulated Container and on Mellanox x86 platforms

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012

Build Log

ethtool_5.9-1_armhf.deb.log.txt

A picture of a cute animal (not mandatory but encouraged)

Vivek Reddy Karri added 3 commits April 2, 2021 06:59
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
Signed-off-by: Vivek Reddy Karri <vkarri@r-build-sonic06.mtr.labs.mlnx>
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@vivekrnv
Copy link
Contributor Author

vivekrnv commented Apr 3, 2021

@shlomibitton Can you review this?

@liat-grozovik
Copy link
Collaborator

@shlomibitton please review the fix as it is on top of your PR.
@vivekredddo we need this on top of the reverted PR or this one replaces + fix the original one?

@vivekrnv
Copy link
Contributor Author

vivekrnv commented Apr 4, 2021

@vivekredddo we need this on top of the reverted PR or this one replaces + fix the original one?

@liat-grozovik This change is needed on top of the original PR #5725.

For backporting to 202012 though, this can be followed. This commit (#5725 Revert Commit) can be reverted back and #5725 Fix can then be backported to 202012

Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@shlomibitton
Copy link
Contributor

@shlomibitton Can you review this?

LGTM, but I did a test to check this issue and used a real system based on ARM CPU x64, the build for the tool succeeded without this fix.
@lguohan @qiluo-msft could you check this please? maybe it is a build environment issue?

@vivekrnv
Copy link
Contributor Author

vivekrnv commented Apr 5, 2021

@shlomibitton Can you review this?

LGTM, but I did a test to check this issue and used a real system based on ARM CPU x64, the build for the tool succeeded without this fix.
@lguohan @qiluo-msft could you check this please? maybe it is a build environment issue?

Hi @shlomibitton,

Not sure if these builds ran on real ARM server or an emulated one, the ARM builds were failing.
Azure Pipelines Run

@lguohan
Copy link
Collaborator

lguohan commented Apr 6, 2021

the build is running on qemu emulation. are you saying the test is working fine on real arm cpu?

in this, should we disable the unit test, instead of disable the configuration option.

Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@vivekrnv
Copy link
Contributor Author

vivekrnv commented Apr 7, 2021

@lguohan
Done, disabled tests instead of the netlink configuration option

@@ -11,6 +11,10 @@ $(addprefix $(DEST)/, $(MAIN_TARGET)): $(DEST)/% :
git clone https://salsa.debian.org/kernel-team/ethtool/
pushd ethtool
git checkout tags/debian/1%$(ETHTOOL_VERSION_BASE)-1
ifneq (,(filter $(CONFIGURED_ARCH),arm64 armhf))
# Disabling Tests
sed -i -e 's/\(override_dh_auto_configure:\)/override_dh_auto_test:\n\techo \"Skipping Tests\"\n\n\1/g' debian/rules
Copy link
Collaborator

Choose a reason for hiding this comment

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

NO NEED TO IT HERE.

you can refer this example.

rules/sairedis.mk:$(LIBSAIREDIS)_DEB_BUILD_OPTIONS = nocheck

it will disable check

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the example, @lguohan

Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@lguohan lguohan merged commit 69b03fc into sonic-net:master Apr 8, 2021
@yxieca
Copy link
Contributor

yxieca commented Apr 8, 2021

@vivekreddynv this change cannot be cherry-picked to 202012 branch cleanly, please create a separate PR for 202012 branch.

@vivekrnv
Copy link
Contributor Author

vivekrnv commented Apr 8, 2021

@vivekreddynv this change cannot be cherry-picked to 202012 branch cleanly, please create a separate PR for 202012 branch.

Yeah that's probably because this change is made on the top of #5725 and that commit is reverted #5725 Revert for 202012 branch

So, to port this change to 202012, you can try reverting this commit #5725 Revert and then try cherry picking this change in 202012.

I will do it and raise a PR against 202012

vivekrnv added a commit to vivekrnv/sonic-buildimage that referenced this pull request Apr 8, 2021
…nic-net#7226)

Compiling ethtool from source is causing ethtool unit tests to fail on ARM Platforms.

These tests are failing: (By default netlink-interface is enabled while compiling ethtool)
Link: ([Test File Link](https://salsa.debian.org/kernel-team/ethtool/-/blob/debian/1%255.9-1/test-cmdline.c#L28))
```
FAIL: test-cmdline
==================

E: ethtool 16_char_devname! returns 1
E: ethtool 
127_char_devname0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcde returns 1
E: ethtool --change devname xcvr external returns 0
E: ethtool --change devname speed 100 duplex half port tp autoneg on advertise 0x1 phyad 1 xcvr external wol p sopass 01:23:45:67:89:ab msglvl 1 returns 0

FAIL test-cmdline (exit status: 1)
```
Tested this on Local ARM Emulated Container:

```
(Docker Container Emulating ARM)
vkarri@3a03c70eed35:/tmp/ethtool$ ./ethtool 16_char_devname!
netlink interface initialization failed, device name longer than 15 not supported
vkarri@3a03c70eed35:/tmp/ethtool$ echo $?
1 (Expected 0)
vkarri@3a03c70eed35:~/ethtool$ ./ethtool 16_char_devnameee
netlink interface initialization failed, device name longer than 15 not supported

Checked for dependencies: (all are present)
vkarri@3a03c70eed35:~/ethtool$ apt-cache policy libmnl0
libmnl0:
  Installed: 1.0.4-2
  Candidate: 1.0.4-2
  Version table:
 *** 1.0.4-2 500
        500 http://deb.debian.org/debian buster/main armhf Packages
        500 http://packages.trafficmanager.net/debian/debian buster/main armhf Packages
        100 /var/lib/dpkg/status

vkarri@3a03c70eed35:~/ethtool$ apt-cache policy libc6  
libc6:
  Installed: 2.28-10
  Candidate: 2.28-10
  Version table:
 *** 2.28-10 500
        500 http://deb.debian.org/debian buster/main armhf Packages
        500 http://packages.trafficmanager.net/debian/debian buster/main armhf Packages
        100 /var/lib/dpkg/status
```

#### How I did it
Disabled netlink-interface for ethtool. 

Even though Netlink is not available, it doesn't seem to impact what ethtool was supposed to do. In fact the older version which was in use before this PR [sonic-net#5725](sonic-net#5725) did not have netlink support and everything seemed to work well

Article on Netlink-Support for ethtool: https://lwn.net/Articles/783633/

Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
lguohan pushed a commit that referenced this pull request Apr 16, 2021
)

Compiling ethtool from source is causing ethtool unit tests to fail on ARM Platforms.

These tests are failing: (By default netlink-interface is enabled while compiling ethtool)
Link: ([Test File Link](https://salsa.debian.org/kernel-team/ethtool/-/blob/debian/1%255.9-1/test-cmdline.c#L28))
```
FAIL: test-cmdline
==================

E: ethtool 16_char_devname! returns 1
E: ethtool 
127_char_devname0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcde returns 1
E: ethtool --change devname xcvr external returns 0
E: ethtool --change devname speed 100 duplex half port tp autoneg on advertise 0x1 phyad 1 xcvr external wol p sopass 01:23:45:67:89:ab msglvl 1 returns 0

FAIL test-cmdline (exit status: 1)
```
Tested this on Local ARM Emulated Container:

```
(Docker Container Emulating ARM)
vkarri@3a03c70eed35:/tmp/ethtool$ ./ethtool 16_char_devname!
netlink interface initialization failed, device name longer than 15 not supported
vkarri@3a03c70eed35:/tmp/ethtool$ echo $?
1 (Expected 0)
vkarri@3a03c70eed35:~/ethtool$ ./ethtool 16_char_devnameee
netlink interface initialization failed, device name longer than 15 not supported

Checked for dependencies: (all are present)
vkarri@3a03c70eed35:~/ethtool$ apt-cache policy libmnl0
libmnl0:
  Installed: 1.0.4-2
  Candidate: 1.0.4-2
  Version table:
 *** 1.0.4-2 500
        500 http://deb.debian.org/debian buster/main armhf Packages
        500 http://packages.trafficmanager.net/debian/debian buster/main armhf Packages
        100 /var/lib/dpkg/status

vkarri@3a03c70eed35:~/ethtool$ apt-cache policy libc6  
libc6:
  Installed: 2.28-10
  Candidate: 2.28-10
  Version table:
 *** 2.28-10 500
        500 http://deb.debian.org/debian buster/main armhf Packages
        500 http://packages.trafficmanager.net/debian/debian buster/main armhf Packages
        100 /var/lib/dpkg/status
```

#### How I did it
Disabled netlink-interface for ethtool. 

Even though Netlink is not available, it doesn't seem to impact what ethtool was supposed to do. In fact the older version which was in use before this PR [#5725](#5725) did not have netlink support and everything seemed to work well

Article on Netlink-Support for ethtool: https://lwn.net/Articles/783633/

Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-buildimage that referenced this pull request May 23, 2021
…nic-net#7226)

Compiling ethtool from source is causing ethtool unit tests to fail on ARM Platforms.

These tests are failing: (By default netlink-interface is enabled while compiling ethtool)
Link: ([Test File Link](https://salsa.debian.org/kernel-team/ethtool/-/blob/debian/1%255.9-1/test-cmdline.c#L28))
```
FAIL: test-cmdline
==================

E: ethtool 16_char_devname! returns 1
E: ethtool 
127_char_devname0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcde returns 1
E: ethtool --change devname xcvr external returns 0
E: ethtool --change devname speed 100 duplex half port tp autoneg on advertise 0x1 phyad 1 xcvr external wol p sopass 01:23:45:67:89:ab msglvl 1 returns 0

FAIL test-cmdline (exit status: 1)
```
Tested this on Local ARM Emulated Container:

```
(Docker Container Emulating ARM)
vkarri@3a03c70eed35:/tmp/ethtool$ ./ethtool 16_char_devname!
netlink interface initialization failed, device name longer than 15 not supported
vkarri@3a03c70eed35:/tmp/ethtool$ echo $?
1 (Expected 0)
vkarri@3a03c70eed35:~/ethtool$ ./ethtool 16_char_devnameee
netlink interface initialization failed, device name longer than 15 not supported

Checked for dependencies: (all are present)
vkarri@3a03c70eed35:~/ethtool$ apt-cache policy libmnl0
libmnl0:
  Installed: 1.0.4-2
  Candidate: 1.0.4-2
  Version table:
 *** 1.0.4-2 500
        500 http://deb.debian.org/debian buster/main armhf Packages
        500 http://packages.trafficmanager.net/debian/debian buster/main armhf Packages
        100 /var/lib/dpkg/status

vkarri@3a03c70eed35:~/ethtool$ apt-cache policy libc6  
libc6:
  Installed: 2.28-10
  Candidate: 2.28-10
  Version table:
 *** 2.28-10 500
        500 http://deb.debian.org/debian buster/main armhf Packages
        500 http://packages.trafficmanager.net/debian/debian buster/main armhf Packages
        100 /var/lib/dpkg/status
```

#### How I did it
Disabled netlink-interface for ethtool. 

Even though Netlink is not available, it doesn't seem to impact what ethtool was supposed to do. In fact the older version which was in use before this PR [sonic-net#5725](sonic-net#5725) did not have netlink support and everything seemed to work well

Article on Netlink-Support for ethtool: https://lwn.net/Articles/783633/

Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
carl-nokia pushed a commit to carl-nokia/sonic-buildimage that referenced this pull request Aug 7, 2021
…nic-net#7226)

Compiling ethtool from source is causing ethtool unit tests to fail on ARM Platforms.

These tests are failing: (By default netlink-interface is enabled while compiling ethtool)
Link: ([Test File Link](https://salsa.debian.org/kernel-team/ethtool/-/blob/debian/1%255.9-1/test-cmdline.c#L28))
```
FAIL: test-cmdline
==================

E: ethtool 16_char_devname! returns 1
E: ethtool 
127_char_devname0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcde returns 1
E: ethtool --change devname xcvr external returns 0
E: ethtool --change devname speed 100 duplex half port tp autoneg on advertise 0x1 phyad 1 xcvr external wol p sopass 01:23:45:67:89:ab msglvl 1 returns 0

FAIL test-cmdline (exit status: 1)
```
Tested this on Local ARM Emulated Container:

```
(Docker Container Emulating ARM)
vkarri@3a03c70eed35:/tmp/ethtool$ ./ethtool 16_char_devname!
netlink interface initialization failed, device name longer than 15 not supported
vkarri@3a03c70eed35:/tmp/ethtool$ echo $?
1 (Expected 0)
vkarri@3a03c70eed35:~/ethtool$ ./ethtool 16_char_devnameee
netlink interface initialization failed, device name longer than 15 not supported

Checked for dependencies: (all are present)
vkarri@3a03c70eed35:~/ethtool$ apt-cache policy libmnl0
libmnl0:
  Installed: 1.0.4-2
  Candidate: 1.0.4-2
  Version table:
 *** 1.0.4-2 500
        500 http://deb.debian.org/debian buster/main armhf Packages
        500 http://packages.trafficmanager.net/debian/debian buster/main armhf Packages
        100 /var/lib/dpkg/status

vkarri@3a03c70eed35:~/ethtool$ apt-cache policy libc6  
libc6:
  Installed: 2.28-10
  Candidate: 2.28-10
  Version table:
 *** 2.28-10 500
        500 http://deb.debian.org/debian buster/main armhf Packages
        500 http://packages.trafficmanager.net/debian/debian buster/main armhf Packages
        100 /var/lib/dpkg/status
```

#### How I did it
Disabled netlink-interface for ethtool. 

Even though Netlink is not available, it doesn't seem to impact what ethtool was supposed to do. In fact the older version which was in use before this PR [sonic-net#5725](sonic-net#5725) did not have netlink support and everything seemed to work well

Article on Netlink-Support for ethtool: https://lwn.net/Articles/783633/

Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
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.

6 participants