From: Ido Schimmel <idosch@nvidia.com>
This patchset contains various improvements to recently introduced
module EEPROM netlink API. Noticed these while adding module EEPROM
write support.
Ido Schimmel (7):
ethtool: Use correct command name in title
ethtool: Document correct attribute type
ethtool: Decrease size of module EEPROM get policy array
ethtool: Document behavior when module EEPROM bank attribute is
omitted
ethtool: Use kernel data types for internal EEPROM struct
ethtool: Validate module EEPROM length as part of policy
ethtool: Validate module EEPROM offset as part of policy
Documentation/networking/ethtool-netlink.rst | 8 +++++---
include/linux/ethtool.h | 12 ++++++------
include/uapi/linux/ethtool_netlink.h | 2 +-
net/ethtool/eeprom.c | 13 ++++---------
net/ethtool/netlink.h | 2 +-
5 files changed, 17 insertions(+), 20 deletions(-)
--
2.31.1
@@ -1363,8 +1363,8 @@ in an implementation specific way.``ETHTOOL_A_FEC_AUTO`` requests the driver to choose FEC mode based on SFP module parameters. This does not mean autonegotiation.-MODULE_EEPROM-=============+MODULE_EEPROM_GET+================= Fetch module EEPROM data dump. This interface is designed to allow dumps of at most 1/2 page at once. This
From: Ido Schimmel <idosch@nvidia.com>
The 'ETHTOOL_A_MODULE_EEPROM_DATA' attribute is not part of the get
request.
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
net/ethtool/netlink.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Ido Schimmel <idosch@nvidia.com>
The kernel assumes bank 0 when 'ETHTOOL_MSG_MODULE_EEPROM_GET' is sent
without 'ETHTOOL_A_MODULE_EEPROM_BANK'.
Document it as part of the interface documentation.
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
Documentation/networking/ethtool-netlink.rst | 2 ++
1 file changed, 2 insertions(+)
From: Ido Schimmel <idosch@nvidia.com>
The struct is not visible to user space and therefore should not use the
user visible data types.
Instead, use internal data types like other structures in the file.
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
include/linux/ethtool.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
From: Ido Schimmel <idosch@nvidia.com>
Validate the number of bytes to read from the module EEPROM as part of
the netlink policy and remove the corresponding check from the code.
This also makes it possible to query the length range from user space:
$ genl ctrl policy name ethtool
...
ID: 0x14 policy[32]:attr[3]: type=U32 range:[1,128]
...
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
net/ethtool/eeprom.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
@@ -159,9 +159,6 @@ static int eeprom_parse_request(struct ethnl_req_info *req_info, struct nlattr *request->offset=nla_get_u32(tb[ETHTOOL_A_MODULE_EEPROM_OFFSET]);request->length=nla_get_u32(tb[ETHTOOL_A_MODULE_EEPROM_LENGTH]);-if(!request->length)-return-EINVAL;-/* The following set of conditions limit the API to only dump 1/2*EEPROMpagewithoutcrossinglowpageboundarylocatedatoffset128.*Thismeansusermayonlyrequestdumpsoflengthlimitedto128from
From: Ido Schimmel <idosch@nvidia.com>
Validate the offset to read from module EEPROM as part of the netlink
policy and remove the corresponding check from the code.
This also makes it possible to query the offset range from user space:
$ genl ctrl policy name ethtool
...
ID: 0x14 policy[32]:attr[2]: type=U32 range:[0,255]
...
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
net/ethtool/eeprom.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
@@ -177,10 +177,6 @@ static int eeprom_parse_request(struct ethnl_req_info *req_info, struct nlattr *NL_SET_ERR_MSG_ATTR(extack,tb[ETHTOOL_A_MODULE_EEPROM_LENGTH],"reading cross half page boundary is illegal");return-EINVAL;-}elseif(request->offset>=ETH_MODULE_EEPROM_PAGE_LEN*2){-NL_SET_ERR_MSG_ATTR(extack,tb[ETHTOOL_A_MODULE_EEPROM_OFFSET],-"offset is out of bounds");-return-EINVAL;}elseif(request->offset+request->length>ETH_MODULE_EEPROM_PAGE_LEN*2){NL_SET_ERR_MSG_ATTR(extack,tb[ETHTOOL_A_MODULE_EEPROM_LENGTH],"reading cross page boundary is illegal");
From: Jakub Kicinski <kuba@kernel.org> Date: 2021-06-22 16:30:13
On Tue, 22 Jun 2021 09:50:45 +0300 Ido Schimmel wrote:
From: Ido Schimmel <idosch@nvidia.com>
This patchset contains various improvements to recently introduced
module EEPROM netlink API. Noticed these while adding module EEPROM
write support.
Scary that 3/7 was not generating a warning.
Reviewed-by: Jakub Kicinski <kuba@kernel.org>
Hello:
This series was applied to netdev/net-next.git (refs/heads/master):
On Tue, 22 Jun 2021 09:50:45 +0300 you wrote:
From: Ido Schimmel <idosch@nvidia.com>
This patchset contains various improvements to recently introduced
module EEPROM netlink API. Noticed these while adding module EEPROM
write support.
Ido Schimmel (7):
ethtool: Use correct command name in title
ethtool: Document correct attribute type
ethtool: Decrease size of module EEPROM get policy array
ethtool: Document behavior when module EEPROM bank attribute is
omitted
ethtool: Use kernel data types for internal EEPROM struct
ethtool: Validate module EEPROM length as part of policy
ethtool: Validate module EEPROM offset as part of policy
[...]
From: Michal Kubecek <hidden> Date: 2021-06-22 23:10:32
On Tue, Jun 22, 2021 at 09:30:05AM -0700, Jakub Kicinski wrote:
On Tue, 22 Jun 2021 09:50:45 +0300 Ido Schimmel wrote:
quoted
From: Ido Schimmel <idosch@nvidia.com>
This patchset contains various improvements to recently introduced
module EEPROM netlink API. Noticed these while adding module EEPROM
write support.
Scary that 3/7 was not generating a warning.
Actually, it's not as scary as it seems. If an array variable is
declared with fixed size and then defined and initialized with just
"[]", the number of elements is still taken from the declaration, not
from the maximum initializer index. So there was nothing to worry about,
we just had a partially initialized array with extra elements zero
initialized.
Michal