netdev - Re: [Intel-wired-lan] [PATCH iwl-next v1 3/5] ixgbe: Add link management support for E610 device

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: <87r0fkbr7i.fsf@intel.com>
Date: Thu, 04 Apr 2024 17:14:57 -0700
From: Vinicius Costa Gomes <vinicius.gomes@...el.com>
To: Piotr Kwapulinski <piotr.kwapulinski@...el.com>,
 intel-wired-lan@...ts.osuosl.org
Cc: Piotr Kwapulinski <piotr.kwapulinski@...el.com>, netdev@...r.kernel.org,
 Jedrzej Jagielski <jedrzej.jagielski@...el.com>, Jan Glaza
 <jan.glaza@...el.com>, Stefan Wegrzyn <stefan.wegrzyn@...el.com>
Subject: Re: [Intel-wired-lan] [PATCH iwl-next v1 3/5] ixgbe: Add link
 management support for E610 device

Piotr Kwapulinski <piotr.kwapulinski@...el.com> writes:

> Add low level link management support for E610 device. Link management
> operations are handled via the Admin Command Interface. Add the following
> link management operations:
> - get link capabilities
> - set up link
> - get media type
> - get link status, link status events
> - link power management
>
> Co-developed-by: Stefan Wegrzyn <stefan.wegrzyn@...el.com>
> Signed-off-by: Stefan Wegrzyn <stefan.wegrzyn@...el.com>
> Co-developed-by: Jedrzej Jagielski <jedrzej.jagielski@...el.com>
> Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@...el.com>
> Reviewed-by: Jan Glaza <jan.glaza@...el.com>
> Signed-off-by: Piotr Kwapulinski <piotr.kwapulinski@...el.com>
> ---

[...]

> +/**
> + * ixgbe_update_link_info - update status of the HW network link
> + * @hw: pointer to the HW struct
> + *
> + * Update the status of the HW network link.
> + *
> + * Return: the exit code of the operation.
> + */
> +int ixgbe_update_link_info(struct ixgbe_hw *hw)
> +{
> +	struct ixgbe_link_status *li;
> +	int err;
> +
> +	if (!hw)
> +		return -EINVAL;
> +
> +	li = &hw->link.link_info;
> +
> +	err = ixgbe_aci_get_link_info(hw, true, NULL);
> +	if (err)
> +		return err;
> +
> +	if (li->link_info & IXGBE_ACI_MEDIA_AVAILABLE) {
> +		struct ixgbe_aci_cmd_get_phy_caps_data __free(kfree) *pcaps;
> +
> +		pcaps =	kzalloc(sizeof(*pcaps), GFP_KERNEL);
> +		if (!pcaps)
> +			return -ENOMEM;
> +

Seems that 'pcaps' is leaking here.

> +		err = ixgbe_aci_get_phy_caps(hw, false,
> +					     IXGBE_ACI_REPORT_TOPO_CAP_MEDIA,
> +					     pcaps);
> +
> +		if (!err)
> +			memcpy(li->module_type, &pcaps->module_type,
> +			       sizeof(li->module_type));
> +	}
> +
> +	return err;
> +}
> +
[...]

> +/**
> + * ixgbe_get_media_type_e610 - Gets media type
> + * @hw: pointer to the HW struct
> + *
> + * In order to get the media type, the function gets PHY
> + * capabilities and later on use them to identify the PHY type
> + * checking phy_type_high and phy_type_low.
> + *
> + * Return: the type of media in form of ixgbe_media_type enum
> + * or ixgbe_media_type_unknown in case of an error.
> + */
> +enum ixgbe_media_type ixgbe_get_media_type_e610(struct ixgbe_hw *hw)
> +{
> +	struct ixgbe_aci_cmd_get_phy_caps_data pcaps;
> +	int rc;
> +
> +	rc = ixgbe_update_link_info(hw);
> +	if (rc)
> +		return ixgbe_media_type_unknown;
> +
> +	/* If there is no link but PHY (dongle) is available SW should use
> +	 * Get PHY Caps admin command instead of Get Link Status, find most
> +	 * significant bit that is set in PHY types reported by the command
> +	 * and use it to discover media type.
> +	 */
> +	if (!(hw->link.link_info.link_info & IXGBE_ACI_LINK_UP) &&
> +	    (hw->link.link_info.link_info & IXGBE_ACI_MEDIA_AVAILABLE)) {
> +		u64 phy_mask;
> +		u8 i;
> +
> +		/* Get PHY Capabilities */
> +		rc = ixgbe_aci_get_phy_caps(hw, false,
> +					    IXGBE_ACI_REPORT_TOPO_CAP_MEDIA,
> +					    &pcaps);
> +		if (rc)
> +			return ixgbe_media_type_unknown;
> +
> +		/* Check if there is some bit set in phy_type_high */
> +		for (i = 64; i > 0; i--) {
> +			phy_mask = (u64)((u64)1 << (i - 1));
> +			if ((pcaps.phy_type_high & phy_mask) != 0) {
> +				/* If any bit is set treat it as PHY type */
> +				hw->link.link_info.phy_type_high = phy_mask;
> +				hw->link.link_info.phy_type_low = 0;
> +				break;
> +			}
> +			phy_mask = 0;
> +		}
> +
> +		/* If nothing found in phy_type_high search in phy_type_low */
> +		if (phy_mask == 0) {
> +			for (i = 64; i > 0; i--) {
> +				phy_mask = (u64)((u64)1 << (i - 1));
> +				if ((pcaps.phy_type_low & phy_mask) != 0) {
> +					/* Treat as PHY type is any bit set */
> +					hw->link.link_info.phy_type_high = 0;
> +					hw->link.link_info.phy_type_low = phy_mask;
> +					break;
> +				}
> +			}
> +		}

These two look like they are doing something very similar to fls64().
Could that work?

> +
> +		/* Based on search above try to discover media type */
> +		hw->phy.media_type = ixgbe_get_media_type_from_phy_type(hw);
> +	}
> +
> +	return hw->phy.media_type;
> +}
> +


-- 
Vinicius