netdev - Re: [PATCH net-next 3/5] net: txgbe: Support to setup link

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20221114154824.704046-1-alexandr.lobakin@intel.com>
Date:   Mon, 14 Nov 2022 16:48:24 +0100
From:   Alexander Lobakin <alexandr.lobakin@...el.com>
To:     Mengyuan Lou <mengyuanlou@...-swift.com>
Cc:     Alexander Lobakin <alexandr.lobakin@...el.com>,
        netdev@...r.kernel.org, jiawenwu@...stnetic.com
Subject: Re: [PATCH net-next 3/5] net: txgbe: Support to setup link

From: Mengyuan Lou <mengyuanlou@...-swift.com>
Date: Tue,  8 Nov 2022 19:19:05 +0800

> From: Jiawen Wu <jiawenwu@...stnetic.com>
> 
> Get link capabilities, setup MAC and PHY link, and support to enable
> or disable Tx laser.
> 
> Signed-off-by: Jiawen Wu <jiawenwu@...stnetic.com>
> ---
> 
> [...]
> 
> @@ -46,6 +55,13 @@ struct txgbe_adapter {
>  	struct timer_list service_timer;
>  	struct work_struct service_task;
>  
> +	u32 flags;
> +
> +	bool link_up;

Use can reuse the previous field flags to store link status.

Also pls try to avoid placing bools in structures, there are lots of
discussions around it and how compilers place/declare them, but
better to know for sure it would work the same way on each setup,
right?

> +	u32 link_speed;
> +	unsigned long sfp_poll_time;
> +	unsigned long link_check_timeout;
> +
>  	/* structs defined in txgbe_type.h */
>  	struct txgbe_hw hw;
>  	u16 msg_enable;

[...]

> @@ -188,11 +280,22 @@ struct txgbe_phy_info {
>  	u32 orig_vr_xs_or_pcs_mmd_digi_ctl1;
>  	bool orig_link_settings_stored;
>  	bool multispeed_fiber;
> +	bool autotry_restart;

Could be also one u32, e.g. via bitfields:

	u32 orig_link_settings_stored:1;
	u32 multispeed_fiber:1;
	u32 autotry_restart:1;

There will be 29 free slots more here then for new flags.

> +	u32 autoneg_advertised;
> +	u32 link_mode;
> +};
> +
> +/* link status for KX/KX4 */
> +enum txgbe_link_status {
> +	TXGBE_LINK_STATUS_NONE = 0,
> +	TXGBE_LINK_STATUS_KX,
> +	TXGBE_LINK_STATUS_KX4
>  };
>  
>  struct txgbe_hw {
>  	struct wx_hw wxhw;
>  	struct txgbe_phy_info phy;
> +	enum txgbe_link_status link_status;
>  };
>  
>  #endif /* _TXGBE_TYPE_H_ */
> -- 
> 2.38.1

Thanks,
Olek