[PATCH v3 net-next] octeontx2-pf: report port connector type in ethtool link ksettings

[PATCH v3 net-next] octeontx2-pf: report port connector type in ethtool link ksettings

[Ratheesh Kannoth]

From: Hariprasad Kelam <hkelam@marvell.com>

otx2_get_link_ksettings() already reports speed, duplex, autoneg, link
modes and FEC from CGX firmware shared data, but never fills
cmd->base.port.  As a result, ethtool does not show the physical
connector type even though firmware provides it in fwdata.port.

Read the connector type from shared firmware data and expose it via
ethtool.  Extract only the low 8 bits of the firmware field and
validate the value against the standard ethtool PORT_* definitions
before publishing it to userspace.

Signed-off-by: Hariprasad Kelam <hkelam@marvell.com>
Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>

---

v2 -> v3:
- Extract only the connector type bits with FIELD_GET(GENMASK(7, 0), ...)
  instead of casting the full 64-bit firmware field to u8.
- Validate firmware values against the ethtool PORT_* whitelist before
  exposing them to userspace, per Jakub's review.
- Map unrecognized firmware values to PORT_OTHER instead of passing
  them through blindly.

v1 -> v2:
- Addressed Sashiko review comments.
  https://lore.kernel.org/netdev/20260904154833.3b32ea6a@kernel.org/
---
 .../marvell/octeontx2/nic/otx2_ethtool.c       | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
index 9bee1b91eeaa..a05dee0085a3 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
@@ -1209,6 +1209,7 @@ static int otx2_get_link_ksettings(struct net_device *netdev,
 {
 	struct otx2_nic *pfvf = netdev_priv(netdev);
 	struct cgx_fw_data *rsp = NULL;
+	u8 port;
 
 	cmd->base.duplex  = pfvf->linfo.full_duplex;
 	cmd->base.speed   = pfvf->linfo.speed;
@@ -1231,6 +1232,23 @@ static int otx2_get_link_ksettings(struct net_device *netdev,
 				OTX2_MODE_SUPPORTED, cmd);
 	otx2_get_fec_info(rsp->fwdata.supported_fec,
 			  OTX2_MODE_SUPPORTED, cmd);
+
+	port = FIELD_GET(GENMASK(7, 0), rsp->fwdata.port);
+	switch (port) {
+	case PORT_TP:
+	case PORT_AUI:
+	case PORT_MII:
+	case PORT_FIBRE:
+	case PORT_BNC:
+	case PORT_DA:
+	case PORT_NONE:
+		cmd->base.port = port;
+		break;
+	default:
+		cmd->base.port = PORT_OTHER;
+		break;
+	}
+
 	return 0;
 }
 
-- 
2.43.0

Re: [PATCH v3 net-next] octeontx2-pf: report port connector type in ethtool link ksettings

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] otx2_get_link_ksettings()
  (drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c:1236)…

--

octeontx2-pf: report port connector type in ethtool link ksettings

otx2_get_link_ksettings() now reads the connector type out of the CGX
firmware shared data (struct cgx_lmac_fwdata_s.port), masks the low eight
bits, checks the result against a list of ethtool PORT_* values and stores
it into cmd->base.port.  Unrecognized values become PORT_OTHER.

> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
> index 9bee1b91eeaa..a05dee0085a3 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c

[ ... ]

> @@ -1231,6 +1232,23 @@ static int otx2_get_link_ksettings(struct net_device *netdev,
>  				OTX2_MODE_SUPPORTED, cmd);
>  	otx2_get_fec_info(rsp->fwdata.supported_fec,
>  			  OTX2_MODE_SUPPORTED, cmd);
> +
> +	port = FIELD_GET(GENMASK(7, 0), rsp->fwdata.port);
> +	switch (port) {
> +	case PORT_TP:
> +	case PORT_AUI:
> +	case PORT_MII:
> +	case PORT_FIBRE:
> +	case PORT_BNC:
> +	case PORT_DA:
> +	case PORT_NONE:
> +		cmd->base.port = port;
> +		break;
> +	default:
> +		cmd->base.port = PORT_OTHER;
> +		break;
> +	}

[Severity: Medium]
Can this report a wrong connector type when firmware never populated the
field?

PORT_TP is 0x00 in include/uapi/linux/ethtool.h, and the ethtool core
zero-fills the settings before calling into the driver:

net/ethtool/ioctl.c:netif_get_link_ksettings() {
	memset(link_ksettings, 0, sizeof(*link_ksettings));
	return dev->ethtool_ops->get_link_ksettings(dev, link_ksettings);
}

So a zero firmware word takes the "case PORT_TP:" arm rather than the
default PORT_OTHER arm, and ethtool prints "Port: Twisted Pair" on a
fibre, DAC or backplane board.

Is a zero word possible here?  The member was carved out of the previously
reserved (zero-filled) tail of struct cgx_lmac_fwdata_s by commit
49f02e6877d1b, which shrank LMAC_FWDATA_RESERVED_MEM from 1021 to 1018:

drivers/net/ethernet/marvell/octeontx2/af/mbox.h:
	u64 mgmt_port:1;
	u64 advertised_an:1;
	u64 port;
#define LMAC_FWDATA_RESERVED_MEM 1018
	u64 reserved[LMAC_FWDATA_RESERVED_MEM];

RVU_FWDATA_VERSION was not bumped at the same time, and
is_rvu_fwdata_valid() in af/rvu.h only compares header_magic and version,
so firmware images that predate the field still pass the ABI gate and hand
back zeros for this word.

Would it make sense to also consult rw_valid from the same struct?  It is
the per-LMAC validity flag, and grep shows it is currently only referenced
in mbox.h and rvu_debugfs.c, never by this new consumer.  Note also that
the debugfs dumper added alongside the field prints rw_valid, lmac_type,
portm_idx and mgmt_port, but not port.

Separately, does the masking defeat part of the validation?  FIELD_GET()
with GENMASK(7, 0) drops bits 63:8 before the switch runs, so firmware
values such as 0x100 or 0x103 are published as PORT_TP and PORT_FIBRE
respectively.  mbox.h documents no bitfield layout, mask or reserved-bit
contract for this bare u64, so it isn't clear the upper bits can be
discarded rather than treated as "not a valid connector code".

Related to that, the commit message says:

  "Extract only the low 8 bits of the firmware field and validate the
   value against the standard ethtool PORT_* definitions before
   publishing it to userspace."

Given that neither the unpopulated (zero) case nor invalid values living in
bits 63:8 can be rejected by this check, could the description be made
more precise about what is actually validated?

One more question on the encoding itself: is there an in-tree statement
that fwdata.port uses ethtool PORT_* numbering?  The only other firmware
description of a "port" value is struct cgx_lnk_sts.port:8 with
RESP_LINKSTAT_PORT in af/cgx_fw_if.h, and that macro is defined but never
decoded by cgx.c, so the mapping to PORT_TP/PORT_FIBRE/... appears
undocumented here.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260909040621.397255-1-rkannoth%40marvell.com

Re: [PATCH v3 net-next] octeontx2-pf: report port connector type in ethtool link ksettings

[Ratheesh Kannoth]

On 2026-09-10 at 10:02:43, netdev-bot+sashiko@kernel.org (netdev-bot+sashiko@kernel.org) wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
>
> Critical: 0 · High: 0 · Medium: 1 · Low: 0
>
> - [Medium] otx2_get_link_ksettings()
>   (drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c:1236)…
FW is setting it properly.

>
> --
>
> octeontx2-pf: report port connector type in ethtool link ksettings
>
> otx2_get_link_ksettings() now reads the connector type out of the CGX
> firmware shared data (struct cgx_lmac_fwdata_s.port), masks the low eight
> bits, checks the result against a list of ethtool PORT_* values and stores
> it into cmd->base.port.  Unrecognized values become PORT_OTHER.
>
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
> > index 9bee1b91eeaa..a05dee0085a3 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
>
> [ ... ]
>
> > @@ -1231,6 +1232,23 @@ static int otx2_get_link_ksettings(struct net_device *netdev,
> >  				OTX2_MODE_SUPPORTED, cmd);
> >  	otx2_get_fec_info(rsp->fwdata.supported_fec,
> >  			  OTX2_MODE_SUPPORTED, cmd);
> > +
> > +	port = FIELD_GET(GENMASK(7, 0), rsp->fwdata.port);
> > +	switch (port) {
> > +	case PORT_TP:
> > +	case PORT_AUI:
> > +	case PORT_MII:
> > +	case PORT_FIBRE:
> > +	case PORT_BNC:
> > +	case PORT_DA:
> > +	case PORT_NONE:
> > +		cmd->base.port = port;
> > +		break;
> > +	default:
> > +		cmd->base.port = PORT_OTHER;
> > +		break;
> > +	}
>
> [Severity: Medium]
> Can this report a wrong connector type when firmware never populated the
> field?
>
> PORT_TP is 0x00 in include/uapi/linux/ethtool.h, and the ethtool core
> zero-fills the settings before calling into the driver:
>
> net/ethtool/ioctl.c:netif_get_link_ksettings() {
> 	memset(link_ksettings, 0, sizeof(*link_ksettings));
> 	return dev->ethtool_ops->get_link_ksettings(dev, link_ksettings);
> }
>
> So a zero firmware word takes the "case PORT_TP:" arm rather than the
> default PORT_OTHER arm, and ethtool prints "Port: Twisted Pair" on a
> fibre, DAC or backplane board.
>
> Is a zero word possible here?  The member was carved out of the previously
> reserved (zero-filled) tail of struct cgx_lmac_fwdata_s by commit
> 49f02e6877d1b, which shrank LMAC_FWDATA_RESERVED_MEM from 1021 to 1018:
>
> drivers/net/ethernet/marvell/octeontx2/af/mbox.h:
> 	u64 mgmt_port:1;
> 	u64 advertised_an:1;
> 	u64 port;
> #define LMAC_FWDATA_RESERVED_MEM 1018
> 	u64 reserved[LMAC_FWDATA_RESERVED_MEM];
>
> RVU_FWDATA_VERSION was not bumped at the same time, and
> is_rvu_fwdata_valid() in af/rvu.h only compares header_magic and version,
> so firmware images that predate the field still pass the ABI gate and hand
> back zeros for this word.
>
> Would it make sense to also consult rw_valid from the same struct?  It is
> the per-LMAC validity flag, and grep shows it is currently only referenced
> in mbox.h and rvu_debugfs.c, never by this new consumer.  Note also that
> the debugfs dumper added alongside the field prints rw_valid, lmac_type,
> portm_idx and mgmt_port, but not port.
>
> Separately, does the masking defeat part of the validation?  FIELD_GET()
> with GENMASK(7, 0) drops bits 63:8 before the switch runs, so firmware
> values such as 0x100 or 0x103 are published as PORT_TP and PORT_FIBRE
> respectively.  mbox.h documents no bitfield layout, mask or reserved-bit
> contract for this bare u64, so it isn't clear the upper bits can be
> discarded rather than treated as "not a valid connector code".
>
> Related to that, the commit message says:
>
>   "Extract only the low 8 bits of the firmware field and validate the
>    value against the standard ethtool PORT_* definitions before
>    publishing it to userspace."
>
> Given that neither the unpopulated (zero) case nor invalid values living in
> bits 63:8 can be rejected by this check, could the description be made
> more precise about what is actually validated?
>
> One more question on the encoding itself: is there an in-tree statement
> that fwdata.port uses ethtool PORT_* numbering?  The only other firmware
> description of a "port" value is struct cgx_lnk_sts.port:8 with
> RESP_LINKSTAT_PORT in af/cgx_fw_if.h, and that macro is defined but never
> decoded by cgx.c, so the mapping to PORT_TP/PORT_FIBRE/... appears
> undocumented here.
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260909040621.397255-1-rkannoth%40marvell.com

Re: [PATCH v3 net-next] octeontx2-pf: report port connector type in ethtool link ksettings

[Simon Horman]

On Thu, Sep 10, 2026 at 02:33:44PM +0530, Ratheesh Kannoth wrote:
> On 2026-09-10 at 10:02:43, netdev-bot+sashiko@kernel.org (netdev-bot+sashiko@kernel.org) wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential
> > issue(s) to consider:
> >
> > Critical: 0 · High: 0 · Medium: 1 · Low: 0
> >
> > - [Medium] otx2_get_link_ksettings()
> >   (drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c:1236)…
> FW is setting it properly.

Fair enough.

Reviewed-by: Simon Horman <horms@kernel.org>