| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4aa25d98-4df2-4950-89a4-e749d60116be@linux.dev>
Date: Thu, 30 Jan 2025 12:26:07 +0000
From: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
To: Danielle Ratson <danieller@...dia.com>, netdev@...r.kernel.org
Cc: mkubecek@...e.cz, matt@...verse.com.au, daniel.zahka@...il.com,
amcohen@...dia.com, nbu-mlxsw@...hange.nvidia.com
Subject: Re: [PATCH ethtool-next v2 05/14] qsfp: Refactor sff8636_show_dom()
by moving code into separate functions
On 29/01/2025 05:15, Danielle Ratson wrote:
> The sff8636_show_dom() function is quite lengthy, and with the planned
> addition of JSON support, it will become even longer and more complex.
>
> To improve readability and maintainability, refactor the function by
> moving portions of the code into separate functions, following the
> approach used in the cmis.c module.
>
> Signed-off-by: Danielle Ratson <danieller@...dia.com>
> ---
> qsfp.c | 126 ++++++++++++++++++++++++++++++++++++---------------------
> 1 file changed, 80 insertions(+), 46 deletions(-)
>
> diff --git a/qsfp.c b/qsfp.c
> index d272dbf..994ad5f 100644
> --- a/qsfp.c
> +++ b/qsfp.c
> @@ -649,13 +649,85 @@ out:
> }
> }
>
> -static void sff8636_show_dom(const struct sff8636_memory_map *map)
> +static void sff8636_show_dom_chan_lvl_tx_bias(const struct sff_diags *sd)
> {
> - struct sff_diags sd = {0};
> - char *rx_power_string = NULL;
> char power_string[MAX_DESC_SIZE];
> int i;
>
> + for (i = 0; i < SFF8636_MAX_CHANNEL_NUM; i++) {
> + snprintf(power_string, MAX_DESC_SIZE, "%s (Channel %d)",
> + "Laser tx bias current", i+1);
> + PRINT_BIAS(power_string, sd->scd[i].bias_cur);
> + }
> +}
> +
> +static void sff8636_show_dom_chan_lvl_tx_power(const struct sff_diags *sd)
> +{
> + char power_string[MAX_DESC_SIZE];
> + int i;
> +
> + for (i = 0; i < SFF8636_MAX_CHANNEL_NUM; i++) {
> + snprintf(power_string, MAX_DESC_SIZE, "%s (Channel %d)",
> + "Transmit avg optical power", i+1);
> + PRINT_xX_PWR(power_string, sd->scd[i].tx_power);
> + }
> +}
> +
> +static void sff8636_show_dom_chan_lvl_rx_power(const struct sff_diags *sd)
> +{
> + char power_string[MAX_DESC_SIZE];
> + char *rx_power_string = NULL;
> + int i;
> +
> + if (!sd->rx_power_type)
> + rx_power_string = "Receiver signal OMA";
> + else
> + rx_power_string = "Rcvr signal avg optical power";
> +
> + for (i = 0; i < SFF8636_MAX_CHANNEL_NUM; i++) {
> + snprintf(power_string, MAX_DESC_SIZE, "%s (Channel %d)",
> + rx_power_string, i+1);
> + PRINT_xX_PWR(power_string, sd->scd[i].rx_power);
> + }
> +}
> +
> +static void
> +sff8636_show_dom_chan_lvl_flags(const struct sff8636_memory_map *map)
> +{
> + bool value;
> + int i;
> +
> + for (i = 0; module_aw_chan_flags[i].fmt_str; ++i) {
> + if (module_aw_chan_flags[i].type != MODULE_TYPE_SFF8636)
> + continue;
> +
> + value = map->lower_memory[module_aw_chan_flags[i].offset] &
> + module_aw_chan_flags[i].adver_value;
> + printf("\t%-41s (Chan %d) : %s\n",
> + module_aw_chan_flags[i].fmt_str,
> + (i % SFF8636_MAX_CHANNEL_NUM) + 1,
Looks like this way will only work when MODULE_TYPE_SFF8636 properties
starts at (offset % SFF8636_MAX_CHANNEL_NUM) == 0. Maybe we have to save
the offset of the first SFF8636 item in module_aw_chan_flags[] ?
> + value ? "On" : "Off");
> + }
> +}
> +
> +static void
> +sff8636_show_dom_mod_lvl_flags(const struct sff8636_memory_map *map)
> +{
> + int i;
> +
> + for (i = 0; module_aw_mod_flags[i].str; ++i) {
> + if (module_aw_mod_flags[i].type == MODULE_TYPE_SFF8636)
> + printf("\t%-41s : %s\n",
> + module_aw_mod_flags[i].str,
> + ONOFF(map->lower_memory[module_aw_mod_flags[i].offset]
> + & module_aw_mod_flags[i].value));
> + }
> +}
> +
> +static void sff8636_show_dom(const struct sff8636_memory_map *map)
> +{
> + struct sff_diags sd = {0};
> +
> /*
> * There is no clear identifier to signify the existence of
> * optical diagnostics similar to SFF-8472. So checking existence
> @@ -687,51 +759,13 @@ static void sff8636_show_dom(const struct sff8636_memory_map *map)
> printf("\t%-41s : %s\n", "Alarm/warning flags implemented",
> (sd.supports_alarms ? "Yes" : "No"));
>
> - for (i = 0; i < SFF8636_MAX_CHANNEL_NUM; i++) {
> - snprintf(power_string, MAX_DESC_SIZE, "%s (Channel %d)",
> - "Laser tx bias current", i+1);
> - PRINT_BIAS(power_string, sd.scd[i].bias_cur);
> - }
> -
> - for (i = 0; i < SFF8636_MAX_CHANNEL_NUM; i++) {
> - snprintf(power_string, MAX_DESC_SIZE, "%s (Channel %d)",
> - "Transmit avg optical power", i+1);
> - PRINT_xX_PWR(power_string, sd.scd[i].tx_power);
> - }
> -
> - if (!sd.rx_power_type)
> - rx_power_string = "Receiver signal OMA";
> - else
> - rx_power_string = "Rcvr signal avg optical power";
> -
> - for (i = 0; i < SFF8636_MAX_CHANNEL_NUM; i++) {
> - snprintf(power_string, MAX_DESC_SIZE, "%s(Channel %d)",
> - rx_power_string, i+1);
> - PRINT_xX_PWR(power_string, sd.scd[i].rx_power);
> - }
> + sff8636_show_dom_chan_lvl_tx_bias(&sd);
> + sff8636_show_dom_chan_lvl_tx_power(&sd);
> + sff8636_show_dom_chan_lvl_rx_power(&sd);
>
> if (sd.supports_alarms) {
> - bool value;
> -
> - for (i = 0; module_aw_chan_flags[i].fmt_str; ++i) {
> - if (module_aw_chan_flags[i].type != MODULE_TYPE_SFF8636)
> - continue;
> -
> - value = map->lower_memory[module_aw_chan_flags[i].offset] &
> - module_aw_chan_flags[i].adver_value;
> - printf("\t%-41s (Chan %d) : %s\n",
> - module_aw_chan_flags[i].fmt_str,
> - (i % SFF8636_MAX_CHANNEL_NUM) + 1,
> - value ? "On" : "Off");
> - }
> - for (i = 0; module_aw_mod_flags[i].str; ++i) {
> - if (module_aw_mod_flags[i].type == MODULE_TYPE_SFF8636)
> - printf("\t%-41s : %s\n",
> - module_aw_mod_flags[i].str,
> - (map->lower_memory[module_aw_mod_flags[i].offset]
> - & module_aw_mod_flags[i].value) ?
> - "On" : "Off");
> - }
> + sff8636_show_dom_chan_lvl_flags(map);
> + sff8636_show_dom_mod_lvl_flags(map);
>
> sff_show_thresholds(sd);
> }
Powered by blists - more mailing lists