ethtool icon indicating copy to clipboard operation
ethtool copied to clipboard

TestLinuxClientLinkModes fails on s390x

Open dswarbrick opened this issue 3 years ago • 5 comments

Another test that fails on Debian CI infrastructure (s390x):

=== RUN   TestLinuxClientLinkModes
=== RUN   TestLinuxClientLinkModes/OK
    client_linux_test.go:292: unexpected link mode (-want +got):
          []*ethtool.LinkMode{
          	&{
          		Interface:     {Index: 1, Name: "eth0"},
          		SpeedMegabits: 1000,
          		Ours: []ethtool.AdvertisedLinkMode{
        - 			{Index: 4, Name: "1000baseT/Half"},
        + 			{Index: 28, Name: "56000baseCR4/Full"},
        - 			{Index: 5, Name: "1000baseT/Full"},
        + 			{Index: 29, Name: "56000baseSR4/Full"},
          		},
          		Peer:   nil,
          		Duplex: s"Half",
          	},
          	&{
          		Interface:     {Index: 2, Name: "eth1"},
          		SpeedMegabits: 10000,
          		Ours: []ethtool.AdvertisedLinkMode{
        - 			{Index: 10, Name: "FIBRE"},
        + 			{Index: 18, Name: "10000baseKX4/Full"},
        - 			{Index: 12, Name: "10000baseT/Full"},
        + 			{Index: 20, Name: "10000baseR/FEC"},
          		},
          		Peer:   nil,
          		Duplex: s"Full",
          	},
          }
--- FAIL: TestLinuxClientLinkModes (0.00s)
    --- FAIL: TestLinuxClientLinkModes/OK (0.00s)

s390x is big-endian, so possibly this is an endian decode issue.

dswarbrick avatar Dec 05 '22 11:12 dswarbrick

I put in a skip for now, I don't have access to a big endian machine for troubleshooting. Leaving this open to fix that issue.

mdlayher avatar Dec 05 '22 11:12 mdlayher

@mdlayher I have (non-root) access on a Debian s390x porterbox, if I'm able to be of assistance.

dswarbrick avatar Dec 05 '22 21:12 dswarbrick

similar issue building for Fedora:

--- FAIL: TestPrivateFlags (0.00s)
    client_linux_test.go:907: unexpected request header bytes (-want +got):
          []uint8{
        - 	0x10,
          	0x00,
        - 	0x01,
        + 	0x10,
          	0x80,
        + 	0x01,
        + 	0x00,
          	0x09,
          	0x00,
          	0x02,
        - 	0x00,
          	0x65,
          	0x74,
          	... // 6 identical elements
          }
--- FAIL: TestSetPrivateFlags (0.00s)
    client_linux_test.go:907: unexpected request header bytes (-want +got):
          []uint8{
        - 	0x10, 0x00, 0x01, 0x80, 0x09, 0x00, 0x02, 0x00,                                                 // -|........|
        + 	0x00, 0x10, 0x80, 0x01, 0x00, 0x09, 0x00, 0x02,                                                 // +|........|
          	0x65, 0x74, 0x68, 0x30, 0x00, 0x00, 0x00, 0x00,                                                 //  |eth0....|
        - 	0x24, 0x00, 0x02, 0x80, 0x20, 0x00, 0x03, 0x80, 0x1c, 0x00, 0x01, 0x80, 0x14, 0x00, 0x02, 0x00, // -|$... ...........|
        + 	0x00, 0x24, 0x80, 0x02, 0x00, 0x20, 0x80, 0x03, 0x00, 0x1c, 0x80, 0x01, 0x00, 0x14, 0x00, 0x02, // +|.$... ..........|
          	0x64, 0x69, 0x73, 0x61, 0x62, 0x6c, 0x65, 0x2d, 0x66, 0x77, 0x2d, 0x6c, 0x6c, 0x64, 0x70, 0x00, //  |disable-fw-lldp.|
        - 	0x04, 0x00, 0x03, 0x00,                                                                         // -|....|
        + 	0x00, 0x04, 0x00, 0x03,                                                                         // +|....|
          }
FAIL
exit status 1
FAIL	github.com/mdlayher/ethtool	0.008s

mefuller avatar Sep 12 '24 23:09 mefuller

swapped bytes, so an endianness issue ...

I can provide access to our public s390x machine if needed/helpful.

sharkcz avatar Sep 13 '24 08:09 sharkcz

Bitsets are indeed handled differently on 64-bit big endian hosts such as s390x. This is from net/ethtool/bitset.c:

/* 64-bit big endian architectures are the only case when u32 based bitmaps
 * and unsigned long based bitmaps have different memory layout so that we
 * cannot simply cast the latter to the former and need actual wrappers
 * converting the latter to the former.
 *
 * To reduce the number of slab allocations, the wrappers use fixed size local
 * variables for bitmaps up to ETHNL_SMALL_BITMAP_BITS bits which is the
 * majority of bitmaps used by ethtool.
 */

... compared with:

/* On little endian 64-bit and all 32-bit architectures, an unsigned long
 * based bitmap can be interpreted as u32 based one using a simple cast.
 */

So skipping the endian-sensitive tests on s390x is really just masking the problem. It's highly likely that the code does not work as intended.

dswarbrick avatar Sep 20 '24 00:09 dswarbrick