| 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: <CANXQDtZbLhdwEtN+kKg1OVLV+uy-gsNfX+rU4MR77QPuO6_y9A@mail.gmail.com>
Date: Mon, 24 Nov 2025 23:34:34 +0530
From: Bhargava Chenna Marreddy <bhargava.marreddy@...adcom.com>
To: ALOK TIWARI <alok.a.tiwari@...cle.com>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
pabeni@...hat.com, andrew+netdev@...n.ch, horms@...nel.org,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
michael.chan@...adcom.com, pavan.chebbi@...adcom.com,
vsrama-krishna.nemani@...adcom.com, vikas.gupta@...adcom.com,
Rajashekar Hudumula <rajashekar.hudumula@...adcom.com>
Subject: Re: [External] : [v2, net-next 01/12] bng_en: Query PHY and report
link status
On Wed, Nov 19, 2025 at 3:16 PM ALOK TIWARI <alok.a.tiwari@...cle.com> wrote:
>
>
> > @@ -9,8 +9,10 @@
> >
> > #include <linux/etherdevice.h>
> > #include <linux/bnxt/hsi.h>
> > +#include <linux/ethtool.h>
> > #include "bnge_rmem.h"
> > #include "bnge_resc.h"
> > +#include "bnge_link.h"
> >
> > #define DRV_VER_MAJ 1
> > #define DRV_VER_MIN 15
> > @@ -141,6 +143,17 @@ struct bnge_dev {
> > struct bnge_ctx_mem_info *ctx;
> >
> > u64 flags;
> > +#define BNGE_PF(bd) (1)
> > +#define BNGE_VF(bd) (0)
> > +#define BNGE_NPAR(bd) (0)
> > +#define BNGE_MH(bd) (0)
> > +#define BNGE_SINGLE_PF(bd) (BNGE_PF(bd) && !BNGE_NPAR(bd) && !BNGE_MH(bn))
>
> bn is wrong.
>
> > +#define BNGE_SH_PORT_CFG_OK(bd) \
> > + (BNGE_PF(bd) && ((bd)->phy_flags & BNGE_PHY_FL_SHARED_PORT_CFG))
> > +#define BNGE_PHY_CFG_ABLE(bd) \
> > + ((BNGE_SINGLE_PF(bd) || \
> > + BNGE_SH_PORT_CFG_OK(bd)) && \
> > + (bd)->link_info.phy_state == BNGE_PHY_STATE_ENABLED)
> >
> > struct bnge_hw_resc hw_resc;
> >
> > @@ -197,6 +210,22 @@ struct bnge_dev {
> >
> > struct bnge_irq *irq_tbl;
> > u16 irqs_acquired;
> > +
> > + /* To protect link related settings during link changes and
> > + * ethtool settings changes.
> > + */
> > + struct mutex link_lock;
> > + struct bnge_link_info link_info;
> > +
> > + /* copied from flags and flags2 in hwrm_port_phy_qcaps_output */
> > + u32 phy_flags;
> > +#define BNGE_PHY_FL_SHARED_PORT_CFG \
> > + PORT_PHY_QCAPS_RESP_FLAGS_SHARED_PHY_CFG_SUPPORTED
> > +#define BNGE_PHY_FL_NO_FCS PORT_PHY_QCAPS_RESP_FLAGS_NO_FCS
> > +#define BNGE_PHY_FL_SPEEDS2 \
> > + (PORT_PHY_QCAPS_RESP_FLAGS2_SPEEDS2_SUPPORTED << 8)
> > +
> > + u32 msg_enable;
> > };
> >
> > static inline bool bnge_is_roce_en(struct bnge_dev *bd)
> > diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_hwrm_lib.c b/drivers/net/ethernet/broadcom/bnge/bnge_hwrm_lib.c
> > index 198f49b40db..b0e941ad18b 100644
> > --- a/drivers/net/ethernet/broadcom/bnge/bnge_hwrm_lib.c
> > +++ b/drivers/net/ethernet/broadcom/bnge/bnge_hwrm_lib.c
> > @@ -14,6 +14,7 @@
> > #include "bnge_hwrm_lib.h"
> > #include "bnge_rmem.h"
> > #include "bnge_resc.h"
> > +#include "bnge_link.h"
> >
> > int bnge_hwrm_ver_get(struct bnge_dev *bd)
> > {
> > @@ -981,6 +982,192 @@ void bnge_hwrm_vnic_ctx_free_one(struct bnge_dev *bd,
> > vnic->fw_rss_cos_lb_ctx[ctx_idx] = INVALID_HW_RING_ID;
> > }
> >
> > +int bnge_hwrm_phy_qcaps(struct bnge_dev *bd)
> > +{
> > + struct bnge_link_info *link_info = &bd->link_info;
> > + struct hwrm_port_phy_qcaps_output *resp;
> > + struct hwrm_port_phy_qcaps_input *req;
> > + int rc = 0;
> > +
> > + rc = bnge_hwrm_req_init(bd, req, HWRM_PORT_PHY_QCAPS);
> > + if (rc)
> > + return rc;
> > +
> > + resp = bnge_hwrm_req_hold(bd, req);
> > + rc = bnge_hwrm_req_send(bd, req);
> > + if (rc)
> > + goto hwrm_phy_qcaps_exit;
> > +
> > + bd->phy_flags = resp->flags | (le16_to_cpu(resp->flags2) << 8);
> > +
> > + if (bnge_phy_qcaps_no_speed(resp)) {
> > + link_info->phy_state = BNGE_PHY_STATE_DISABLED;
> > + netdev_warn(bd->netdev, "Ethernet link disabled\n");
> > + } else if (link_info->phy_state == BNGE_PHY_STATE_DISABLED) {
> > + link_info->phy_state = BNGE_PHY_STATE_ENABLED;
> > + netdev_info(bd->netdev, "Ethernet link enabled\n");
> > + /* Phy re-enabled, reprobe the speeds */
> > + link_info->support_auto_speeds = 0;
> > + link_info->support_pam4_auto_speeds = 0;
> > + link_info->support_auto_speeds2 = 0;
> > + }
> > + if (resp->supported_speeds_auto_mode)
> > + link_info->support_auto_speeds =
> > + le16_to_cpu(resp->supported_speeds_auto_mode);
> > + if (resp->supported_pam4_speeds_auto_mode)
> > + link_info->support_pam4_auto_speeds =
> > + le16_to_cpu(resp->supported_pam4_speeds_auto_mode);
> > + if (resp->supported_speeds2_auto_mode)
> > + link_info->support_auto_speeds2 =
> > + le16_to_cpu(resp->supported_speeds2_auto_mode);
> > +
> > + bd->port_count = resp->port_cnt;
> > +
> > +hwrm_phy_qcaps_exit:
> > + bnge_hwrm_req_drop(bd, req);
> > + return rc;
> > +}
> > +
> > +int bnge_hwrm_set_link_setting(struct bnge_net *bn, bool set_pause)
> > +{
> > + struct hwrm_port_phy_cfg_input *req;
> > + struct bnge_dev *bd = bn->bd;
> > + int rc;
> > +
> > + rc = bnge_hwrm_req_init(bd, req, HWRM_PORT_PHY_CFG);
> > + if (rc)
> > + return rc;
> > +
> > + if (set_pause)
> > + bnge_hwrm_set_pause_common(bn, req);
> > +
> > + bnge_hwrm_set_link_common(bn, req);
> > +
> > + return bnge_hwrm_req_send(bd, req);
> > +}
> > +
> [snip]
> >
> > +
> > +int bnge_update_phy_setting(struct bnge_net *bn)
> > +{
> > + struct bnge_ethtool_link_info *elink_info;
> > + struct bnge_link_info *link_info;
> > + struct bnge_dev *bd = bn->bd;
> > + bool update_pause = false;
> > + bool update_link = false;
> > + int rc;
> > +
> > + link_info = &bd->link_info;
> > + elink_info = &bn->eth_link_info;
> > + rc = bnge_update_link(bn, true);
> > + if (rc) {
> > + netdev_err(bd->netdev, "failed to update link (rc: %d)\n",
> > + rc);
> > + return rc;
> > + }
> > + if (!BNGE_SINGLE_PF(bd))
> > + return 0;
> > +
> > + if ((elink_info->autoneg & BNGE_AUTONEG_FLOW_CTRL) &&
> > + (link_info->auto_pause_setting & BNGE_LINK_PAUSE_BOTH) !=
> > + elink_info->req_flow_ctrl)
> > + update_pause = true;
> > + if (!(elink_info->autoneg & BNGE_AUTONEG_FLOW_CTRL) &&
> > + link_info->force_pause_setting != elink_info->req_flow_ctrl)
> > + update_pause = true;
> > + if (!(elink_info->autoneg & BNGE_AUTONEG_SPEED)) {
> > + if (BNGE_AUTO_MODE(link_info->auto_mode))
> > + update_link = true;
> > + if (bnge_force_speed_updated(bn))
> > + update_link = true;
> > + if (elink_info->req_duplex != link_info->duplex_setting)
> > + update_link = true;
> > + } else {
> > + if (link_info->auto_mode == BNGE_LINK_AUTO_NONE)
> > + update_link = true;
> > + if (bnge_auto_speed_updated(bn))
> > + update_link = true;
> > + }
> > +
> > + /* The last close may have shutdown the link, so need to call
> > + * PHY_CFG to bring it back up.
> > + */
> > + if (!BNGE_LINK_IS_UP(bd))
> > + update_link = true;
> > +
> > + if (update_link)
> > + rc = bnge_hwrm_set_link_setting(bn, update_pause);
> > + else if (update_pause)
> > + rc = bnge_hwrm_set_pause(bn);
>
> new line help here
Thanks, I'll address this in the next patch.
>
> if
> update_link == false
> update_pause == false
>
> it will return rc = 0 assign by bnge_update_link() correct?
That’s right, neither flag set means no call happens, so rc remains 0
>
> > + if (rc) {
> > + netdev_err(bd->netdev,
> > + "failed to update phy setting (rc: %d)\n", rc);
> > + return rc;
> > + }
> > +
> > + return rc;
> > +}
> > +
> > +void bnge_get_port_module_status(struct bnge_net *bn)
> > +{
> > + struct bnge_dev *bd = bn->bd;
> > + struct bnge_link_info *link_info = &bd->link_info;
> > + struct hwrm_port_phy_qcfg_output *resp = &link_info->phy_qcfg_resp;
> > + u8 module_status;
> > +
> > + if (bnge_update_link(bn, true))
> > + return;
> > +
> > + module_status = link_info->module_status;
> > + switch (module_status) {
> > + case PORT_PHY_QCFG_RESP_MODULE_STATUS_DISABLETX:
> > + case PORT_PHY_QCFG_RESP_MODULE_STATUS_PWRDOWN:
> > + case PORT_PHY_QCFG_RESP_MODULE_STATUS_WARNINGMSG:
> > + netdev_warn(bd->netdev,
> > + "Unqualified SFP+ module detected on port %d\n",
> > + bd->pf.port_id);
> > + netdev_warn(bd->netdev, "Module part number %s\n",
> > + resp->phy_vendor_partnumber);
> > + if (module_status == PORT_PHY_QCFG_RESP_MODULE_STATUS_DISABLETX)
> > + netdev_warn(bd->netdev, "TX is disabled\n");
> > + if (module_status == PORT_PHY_QCFG_RESP_MODULE_STATUS_PWRDOWN)
> > + netdev_warn(bd->netdev, "SFP+ module is shutdown\n");
> > + }
> > +}
> > +
> > +static bool bnge_support_dropped(u16 advertising, u16 supported)
> > +{
> > + u16 diff = advertising ^ supported;
> > +
> > + return ((supported | diff) != supported);
> > +}
> > +
> > +bool bnge_support_speed_dropped(struct bnge_net *bn)
> > +{
> > + struct bnge_ethtool_link_info *elink_info = &bn->eth_link_info;
> > + struct bnge_dev *bd = bn->bd;
> > + struct bnge_link_info *link_info = &bd->link_info;
> > +
> > + /* Check if any advertised speeds are no longer supported. The caller
> > + * holds the link_lock mutex, so we can modify link_info settings.
> > + */
> > + if (bd->phy_flags & BNGE_PHY_FL_SPEEDS2) {
> > + if (bnge_support_dropped(elink_info->advertising,
> > + link_info->support_auto_speeds2)) {
> > + elink_info->advertising =
> > + link_info->support_auto_speeds2;
> > + return true;
> > + }
> > + return false;
> > + }
> > + if (bnge_support_dropped(elink_info->advertising,
> > + link_info->support_auto_speeds)) {
> > + elink_info->advertising = link_info->support_auto_speeds;
> > + return true;
> > + }
> > + if (bnge_support_dropped(elink_info->advertising_pam4,
> > + link_info->support_pam4_auto_speeds)) {
> > + elink_info->advertising_pam4 =
> > + link_info->support_pam4_auto_speeds;
> > + return true;
> > + }
> > + return false;
> > +}
> > +
> > +static char *bnge_report_fec(struct bnge_link_info *link_info)
> > +{
> > + u8 active_fec = link_info->active_fec_sig_mode &
> > + PORT_PHY_QCFG_RESP_ACTIVE_FEC_MASK;
> > +
> > + switch (active_fec) {
> > + default:
> > + case PORT_PHY_QCFG_RESP_ACTIVE_FEC_FEC_NONE_ACTIVE:
> > + return "None";
> > + case PORT_PHY_QCFG_RESP_ACTIVE_FEC_FEC_CLAUSE74_ACTIVE:
> > + return "Clause 74 BaseR";
> > + case PORT_PHY_QCFG_RESP_ACTIVE_FEC_FEC_CLAUSE91_ACTIVE:
> > + return "Clause 91 RS(528,514)";
> > + case PORT_PHY_QCFG_RESP_ACTIVE_FEC_FEC_RS544_1XN_ACTIVE:
> > + return "Clause 91 RS544_1XN";
> > + case PORT_PHY_QCFG_RESP_ACTIVE_FEC_FEC_RS544_IEEE_ACTIVE:
> > + return "Clause 91 RS(544,514)";
> > + case PORT_PHY_QCFG_RESP_ACTIVE_FEC_FEC_RS272_1XN_ACTIVE:
> > + return "Clause 91 RS272_1XN";
> > + case PORT_PHY_QCFG_RESP_ACTIVE_FEC_FEC_RS272_IEEE_ACTIVE:
> > + return "Clause 91 RS(272,257)";
> > + }
> > +}
> > +
> > +void bnge_report_link(struct bnge_dev *bd)
> > +{
> > + if (BNGE_LINK_IS_UP(bd)) {
> > + const char *signal = "";
> > + const char *flow_ctrl;
> > + const char *duplex;
> > + u32 speed;
> > + u16 fec;
> > +
> > + netif_carrier_on(bd->netdev);
> > + speed = bnge_fw_to_ethtool_speed(bd->link_info.link_speed);
> > + if (speed == SPEED_UNKNOWN) {
> > + netdev_info(bd->netdev,
> > + "NIC Link is Up, speed unknown\n");
> > + return;
> > + }
> > + if (bd->link_info.duplex == BNGE_LINK_DUPLEX_FULL)
> > + duplex = "full";
> > + else
> > + duplex = "half";
> > + if (bd->link_info.pause == BNGE_LINK_PAUSE_BOTH)
> > + flow_ctrl = "ON - receive & transmit";
> > + else if (bd->link_info.pause == BNGE_LINK_PAUSE_TX)
> > + flow_ctrl = "ON - transmit";
> > + else if (bd->link_info.pause == BNGE_LINK_PAUSE_RX)
> > + flow_ctrl = "ON - receive";
> > + else
> > + flow_ctrl = "none";
> > + if (bd->link_info.phy_qcfg_resp.option_flags &
> > + PORT_PHY_QCFG_RESP_OPTION_FLAGS_SIGNAL_MODE_KNOWN) {
> > + u8 sig_mode = bd->link_info.active_fec_sig_mode &
> > + PORT_PHY_QCFG_RESP_SIGNAL_MODE_MASK;
> > + switch (sig_mode) {
> > + case PORT_PHY_QCFG_RESP_SIGNAL_MODE_NRZ:
> > + signal = "(NRZ) ";
> > + break;
> > + case PORT_PHY_QCFG_RESP_SIGNAL_MODE_PAM4:
> > + signal = "(PAM4 56Gbps) ";
> > + break;
> > + case PORT_PHY_QCFG_RESP_SIGNAL_MODE_PAM4_112:
> > + signal = "(PAM4 112Gbps) ";
> > + break;
> > + default:
> > + break;
> > + }
> > + }
> > + netdev_info(bd->netdev, "NIC Link is Up, %u Mbps %s%s duplex, Flow control: %s\n",
> > + speed, signal, duplex, flow_ctrl);
> > + fec = bd->link_info.fec_cfg;
> > + if (!(fec & PORT_PHY_QCFG_RESP_FEC_CFG_FEC_NONE_SUPPORTED))
> > + netdev_info(bd->netdev, "FEC autoneg %s encoding: %s\n",
> > + (fec & BNGE_FEC_AUTONEG) ? "on" : "off",
> > + bnge_report_fec(&bd->link_info));
> > + } else {
> > + netif_carrier_off(bd->netdev);
> > + netdev_err(bd->netdev, "NIC Link is Down\n");
> > + }
> > +}
> > diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_link.h b/drivers/net/ethernet/broadcom/bnge/bnge_link.h
> > new file mode 100644
> > index 00000000000..65da27c510b
> > --- /dev/null
> > +++ b/drivers/net/ethernet/broadcom/bnge/bnge_link.h
> > @@ -0,0 +1,185 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/* Copyright (c) 2025 Broadcom */
> > +
> > +#ifndef _BNGE_LINK_H_
> > +#define _BNGE_LINK_H_
> > +
> > +#define BNGE_PHY_AUTO_SPEEDS2_MASK \
> > + PORT_PHY_CFG_REQ_ENABLES_AUTO_LINK_SPEEDS2_MASK
> > +#define BNGE_PHY_AUTO_SPEED_MASK \
> > + PORT_PHY_CFG_REQ_ENABLES_AUTO_LINK_SPEED_MASK
> > +#define BNGE_PHY_AUTO_PAM4_SPEED_MASK \
> > + PORT_PHY_CFG_REQ_ENABLES_AUTO_PAM4_LINK_SPEED_MASK
> > +#define BNGE_PHY_FLAGS_RESTART_AUTO \
> > + PORT_PHY_CFG_REQ_FLAGS_RESTART_AUTONEG
> > +#define BNGE_PHY_FLAGS_ENA_FORCE_SPEEDS2 \
> > + PORT_PHY_CFG_REQ_ENABLES_FORCE_LINK_SPEEDS2
> > +#define BNGE_PHY_FLAGS_ENA_FORCE_PM4_SPEED \
>
> typo should be PAM4 ?
It’s intentional, the name is kept short to stay within ~80 chars
where it’s used.
Thanks,
Bhargava Marreddy
>
> > + PORT_PHY_CFG_REQ_ENABLES_FORCE_PAM4_LINK_SPEED
> > +
> > +struct bnge_link_info {
> > + u8 phy_type;
> > + u8 media_type;
> > + u8 transceiver;
> > + u8 phy_addr;
> > + u8 phy_link_status;
> > +#define BNGE_LINK_LINK PORT_PHY_QCFG_RESP_LINK_LINK
> > + u8 phy_state;
> > +#define BNGE_PHY_STATE_ENABLED 0
> > +#define BNGE_PHY_STATE_DISABLED 1
> > +
> > + u8 link_state;
> > +#define BNGE_LINK_STATE_UNKNOWN 0
> > +#define BNGE_LINK_STATE_DOWN 1
> > +#define BNGE_LINK_STATE_UP 2
> > +#define BNGE_LINK_IS_UP(bd) \
> > + ((bd)->link_info.link_state == BNGE_LINK_STATE_UP)
> > + u8 active_lanes;
>
>
> Thanks,
> Alok
Download attachment "smime.p7s" of type "application/pkcs7-signature" (5496 bytes)
Powered by blists - more mailing lists