Page MenuHomeFreeBSD

Add definitions for the Finisar 40GE LM4 transceiver
ClosedPublic

Authored by mhorne on Sep 1 2020, 3:12 PM.

Details

Summary

As described in the device's datasheet, this is a "multimode adaptation
of IEEE 802.3ba 40GBASE-LR4".

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 33403
Build 30702: arc lint + arc unit

Event Timeline

mhorne requested review of this revision.Sep 1 2020, 3:12 PM

Hi, adding some Intel folks who have appeared on recent ixl(4) reviews/submissions.

I am attempting to upstream this patch from NetApp, who wrote it originally.
It seems that updates to these definitions usually come from upstream as part of
larger ixl driver updates, so forgive me if this is not the usual process.

sys/dev/ixl/i40e_adminq_cmd.h
1943 ↗(On Diff #76490)

This seems to be the important bit that actually identifies the transceiver. Is there any way to verify that this is correct?

Hi, adding some Intel folks who have appeared on recent ixl(4) reviews/submissions.

I am attempting to upstream this patch from NetApp, who wrote it originally.
It seems that updates to these definitions usually come from upstream as part of
larger ixl driver updates, so forgive me if this is not the usual process.

Have they tried this patch to see that it works? The firmware (unless you're using certain ones) will check if the card has a qualified module inserted and will not link up if it doesn't; that might be the real issue here.

sys/dev/ixl/i40e_adminq_cmd.h
1943 ↗(On Diff #76490)

These values come from the firmware. The card's datasheet should have these values in the "Get Link Status" admin queue response data structure, in the PHY Type field. In the latest internal document that I have access to, though, I don't see 0x25 defined. I'm inclined to believe this patch isn't actually correct as a result.

sys/dev/ixl/i40e_adminq_cmd.h
1943 ↗(On Diff #76490)

It is entirely possible that it is incorrect. I will see about consulting the original author on how they arrived at this value and what testing was done.

sys/dev/ixl/i40e_adminq_cmd.h
1943 ↗(On Diff #76490)

Whoops, that person was me. I changed the value while applying the patch to HEAD.

The value that the patch originally used here was 0x0E, which we can see is now defined as I40E_PHY_TYPE_UNRECOGNIZED. So it looks like the original patch relies on the fact that the PHY is unrecognized, and assumes it to be LM4 in particular.

In any case, this ixl(4) part is probably not upstreamable if this PHY doesn't have a defined type yet. Sorry about the noise, I will pare down the diff to just the net/ definitions.

mhorne retitled this revision from ixl: add support for the Finisar 40GE LM4 transceiver to Add definitions for the Finisar 40GE LM4 transceiver.Sep 4 2020, 8:35 PM
mhorne edited the summary of this revision. (Show Details)

@erj any opposition to these definitions?

Yeah, I think the rest of this looks good.

Do you know if there is anything else that can recognize LM4 and use this patch?

This revision is now accepted and ready to land.Sep 11 2020, 8:17 PM
In D26276#587298, @erj wrote:

Yeah, I think the rest of this looks good.

Do you know if there is anything else that can recognize LM4 and use this patch?

I don't know. I would speculate that anything that supports 40GBASE_LR4 (ice, mlx5_en, cxgbe) might also support this, but it could be that the same problem arises where it is unrecognized by the card's firmware.