Fix ethtoolSsetInfo data type according to kernel's uapi ethtool_sset…#69
Fix ethtoolSsetInfo data type according to kernel's uapi ethtool_sset…#69safchain merged 1 commit intosafchain:masterfrom
Conversation
|
@safchain could you please review it? |
ethtool.go
Outdated
| sset_mask uint32 | ||
| data uintptr | ||
| sset_mask uint64 | ||
| data [1]uint32 |
There was a problem hiding this comment.
@madeelibm this doesn't look right compared to the kernel UAPI:
/**
* struct ethtool_sset_info - string set information
* @cmd: Command number = %ETHTOOL_GSSET_INFO
* @reserved: Reserved for future use; see the note on reserved space.
* @sset_mask: On entry, a bitmask of string sets to query, with bits
* numbered according to &enum ethtool_stringset. On return, a
* bitmask of those string sets queried that are supported.
* @data: Buffer for string set sizes. On return, this contains the
* size of each string set that was queried and supported, in
* order of ID.
*
* Example: The user passes in @sset_mask = 0x7 (sets 0, 1, 2) and on
* return @sset_mask == 0x6 (sets 1, 2). Then @data[0] contains the
* size of set 1 and @data[1] contains the size of set 2.
*
* Users must allocate a buffer of the appropriate size (4 * number of
* sets queried) immediately following this structure.
*/
struct ethtool_sset_info {
__u32 cmd;
__u32 reserved;
__u64 sset_mask;
__u32 data[];
};
your change only allows a single data item (which to be fair, the current code does too), while the kernel API allows arbitrary lengths passed back. The current code only works because it only ever sets a single bit in the SSET_INFO sset_mask field; if there are ever more bits set then we'll overflow data. Seems pretty fragile.
I think you could (should?) define a SSET_INFO_MAX as a const and use that as the array size for data and initialize the array like is done for the ethtoolGStrings just below, then leave a comment in getNames() that it only handles a single string set or something like that.
|
@dcbw thanks for the good catch. As suggested i have modified the PR. The array can now store results for multiple sset_mask bit s, if required in future. |
…4-83e5e0097c91 Update the version to include big-endianness fix to safchain/ethtools (safchain/ethtool#69). Steps done for this commit: * replace: `github.com/safchain/ethtool v0.3.0` with `github.com/safchain/ethtool v0.3.1-0.20231027162144-83e5e0097c91` in go.mod, vendor/modules.txt * `go mod tidy` * `go mod vendor` Signed-off-by: Dominik Werle <dwerle@redhat.com>
Update the version to include big-endianness fix to safchain/ethtools (safchain/ethtool#69). Steps done for this commit: * replace: `github.com/safchain/ethtool v0.3.0` with `github.com/safchain/ethtool v0.3.1-0.20231027162144-83e5e0097c91` in go.mod, vendor/modules.txt * `go mod tidy` * `go mod vendor` Signed-off-by: Dominik Werle <dwerle@redhat.com>
…_info
In the latest kernel versions, the type definition for SsetInfo struct has been updated. Therefore, updating it here so the ioctl doesn't break on latest kernel versions. It is also currently breaking under big endian systems. This patch will fix the issue on big endian systems.