ethtool: cmis_cdb: Fix incorrect read / write length extension
authorIdo Schimmel <idosch@nvidia.com>
Wed, 9 Apr 2025 11:24:40 +0000 (14:24 +0300)
committerPaolo Abeni <pabeni@redhat.com>
Thu, 10 Apr 2025 12:32:43 +0000 (14:32 +0200)
The 'read_write_len_ext' field in 'struct ethtool_cmis_cdb_cmd_args'
stores the maximum number of bytes that can be read from or written to
the Local Payload (LPL) page in a single multi-byte access.

Cited commit started overwriting this field with the maximum number of
bytes that can be read from or written to the Extended Payload (LPL)
pages in a single multi-byte access. Transceiver modules that support
auto paging can advertise a number larger than 255 which is problematic
as 'read_write_len_ext' is a 'u8', resulting in the number getting
truncated and firmware flashing failing [1].

Fix by ignoring the maximum EPL access size as the kernel does not
currently support auto paging (even if the transceiver module does) and
will not try to read / write more than 128 bytes at once.

[1]
Transceiver module firmware flashing started for device enp177s0np0
Transceiver module firmware flashing in progress for device enp177s0np0
Progress: 0%
Transceiver module firmware flashing encountered an error for device enp177s0np0
Status message: Write FW block EPL command failed, LPL length is longer
than CDB read write length extension allows.

Fixes: 9a3b0d078bd8 ("net: ethtool: Add support for writing firmware blocks using EPL payload")
Reported-by: Damodharam Ammepalli <damodharam.ammepalli@broadcom.com>
Closes: https://lore.kernel.org/netdev/20250402183123.321036-3-michael.chan@broadcom.com/
Tested-by: Damodharam Ammepalli <damodharam.ammepalli@broadcom.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Damodharam Ammepalli <damodharam.ammepalli@broadcom.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
Link: https://patch.msgid.link/20250409112440.365672-1-idosch@nvidia.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
net/ethtool/cmis.h
net/ethtool/cmis_cdb.c

index 1e790413db0e8dc39332cfa430cf366930a6cfaf..4a9a946cabf05d6262fceb6cd04d2500f65762c7 100644 (file)
@@ -101,7 +101,6 @@ struct ethtool_cmis_cdb_rpl {
 };
 
 u32 ethtool_cmis_get_max_lpl_size(u8 num_of_byte_octs);
-u32 ethtool_cmis_get_max_epl_size(u8 num_of_byte_octs);
 
 void ethtool_cmis_cdb_compose_args(struct ethtool_cmis_cdb_cmd_args *args,
                                   enum ethtool_cmis_cdb_cmd_id cmd, u8 *lpl,
index d159dc121bde583a230475a309595b0eb09b11bd..0e2691ccb0df383e028307e55f789928fdb548b9 100644 (file)
@@ -16,15 +16,6 @@ u32 ethtool_cmis_get_max_lpl_size(u8 num_of_byte_octs)
        return 8 * (1 + min_t(u8, num_of_byte_octs, 15));
 }
 
-/* For accessing the EPL field on page 9Fh, the allowable length extension is
- * min(i, 255) byte octets where i specifies the allowable additional number of
- * byte octets in a READ or a WRITE.
- */
-u32 ethtool_cmis_get_max_epl_size(u8 num_of_byte_octs)
-{
-       return 8 * (1 + min_t(u8, num_of_byte_octs, 255));
-}
-
 void ethtool_cmis_cdb_compose_args(struct ethtool_cmis_cdb_cmd_args *args,
                                   enum ethtool_cmis_cdb_cmd_id cmd, u8 *lpl,
                                   u8 lpl_len, u8 *epl, u16 epl_len,
@@ -33,19 +24,16 @@ void ethtool_cmis_cdb_compose_args(struct ethtool_cmis_cdb_cmd_args *args,
 {
        args->req.id = cpu_to_be16(cmd);
        args->req.lpl_len = lpl_len;
-       if (lpl) {
+       if (lpl)
                memcpy(args->req.payload, lpl, args->req.lpl_len);
-               args->read_write_len_ext =
-                       ethtool_cmis_get_max_lpl_size(read_write_len_ext);
-       }
        if (epl) {
                args->req.epl_len = cpu_to_be16(epl_len);
                args->req.epl = epl;
-               args->read_write_len_ext =
-                       ethtool_cmis_get_max_epl_size(read_write_len_ext);
        }
 
        args->max_duration = max_duration;
+       args->read_write_len_ext =
+               ethtool_cmis_get_max_lpl_size(read_write_len_ext);
        args->msleep_pre_rpl = msleep_pre_rpl;
        args->rpl_exp_len = rpl_exp_len;
        args->flags = flags;