netdev - Re: [PATCH net 3/3] amd-xgbe: fix the SFP compliance codes check for DAC cables

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>] [day] [month] [year] [list]

Message-ID: <03333e34-cd27-35fc-0f4c-c436b7c2857d@amd.com>
Date:   Fri, 7 Oct 2022 09:27:49 -0500
From:   Tom Lendacky <thomas.lendacky@....com>
To:     Raju Rangoju <Raju.Rangoju@....com>, Shyam-sundar.S-k@....com,
        davem@...emloft.net, kuba@...nel.org
Cc:     netdev@...r.kernel.org, rrangoju@....com
Subject: Re: [PATCH net 3/3] amd-xgbe: fix the SFP compliance codes check for
 DAC cables

On 10/7/22 07:25, Raju Rangoju wrote:
> On 10/7/2022 12:34 AM, Tom Lendacky wrote:
>> On 10/6/22 12:37, Raju Rangoju wrote:
>>> On 10/6/2022 8:30 PM, Tom Lendacky wrote:
>>>> On 10/6/22 08:54, Raju Rangoju wrote:
>>>>> The current XGBE code assumes that offset 3 and 6 of EEPROM SFP DAC
>>>>> (passive) cables are NULL. It also assumes the offset 12 is in the
>>>>> range 0x64 to 0x68. However, some of the cables (the 5 meter and 7 meter
>>>>> molex passive cables have non-zero data at offset 3 and 6, also a value
>>>>> 0x78 at offset 12. So, fix the sfp compliance codes check to ignore
>>>>> those offsets. Also extend the macro XGBE_SFP_BASE_BR_10GBE range to 
>>>>> 0x78.
>>>>
>>>> So are these cables going against the specification? Should they be 
>>>> quirks instead of changing the way code is currently operating? How 
>>>> many different cables have you found that do this?
>>>>
>>>> Why would a passive cable be setting any bit other than passive in 
>>>> byte 3? Why would byte 6 also have a non-zero value?
>>>>
>>>> As for the range, 0x78 puts the cable at 12gbps which kind of seems 
>>>> outside the normal range of what a 10gbps cable should be reporting.
>>>>
>>>
>>> For the passive cables, the current SFP checks in driver are not 
>>> expecting any data at offset 3 and 6. Also, the offset 12 is expected 
>>> to be in the range 0x64 - 0x68. This was holding good for Fiber store 
>>> cables so far. However, the 5 and 7 meter Molex passive cables have 
>>> non-zero data at offset 3 and 6, and also a value 0x78 at offset 12.
>>
>> The 0x64 - 0x68 BR range was holding well for the various passive cables 
>> that I tested with. What is the BR value for their other length cables?
> 
> The 1m and 3m cables have the BR value within the range 0x64 - 0x68.

That certainly seems like 5 and 7 meter cables have an inconsistent value 
then, no? A quirk for Molex vendor or the specific part series, e.g. 
74752- or such in xgbe_phy_sfp_bit_rate() might be a better solution here 
than expanding the range for all cable vendors.

Not sure if others with more SFP+ experience could respond and indicate if 
0x78 is really valid for a 10Gbps cable.

> 
>>
>>>
>>> Here is the feedback from cable Vendor when asked about the SFP 
>>> standard for passive cables:
>>>
>>> "For DAC cables –The Ethernet code compliance code standard for passive 
>>> cabling, Offset 3 is “0x0” other offsets  4 and 5 - none of the 
>>> standards are applicable .
>>
>> Ok, so it's not offset 3 that is the issue as none of the bits are set 
>> and won't trigger on the 10G Ethernet Compliance Codes.
> 
> As per the Transceiver compliance codes, offset 3 bits 4,5,6,7 represent 
> 10Gbps speed. However, 5m and 7m transceivers have none of those bits set.

Right, and passive cables shouldn't. Those bits are:

   Bit 7: 10G Base-ER
   Bit 6: 10G Base-LRM
   Bit 5: 10G Base-LR
   Bit 4: 10G Base-SR

which are all Fiber standards.

> On the other hand, offset 6 is set, so the driver incorrectly assumes the 
> transceiver as a 1Gbps cable.

Correct, 1000Base-CX is copper, e.g., passive. So, as stated in my 
previous email, I think moving the passive check up as the first check is 
reasonable.

Thanks,
Tom

> 
> Transceiver details:
> EEPROM sfp_base offset 0-12:
> 0:  0x3   1: 0x4   2: 0x21  3: 0x1   4: 0x0   5: 0x0   6: 0x4   7: 0x41
> 8:  0x84  9: 0x80 10: 0xd5 11: 0x0  12: 0x78
> 
> enp7s0f2:   vendor:         Molex Inc.
> enp7s0f2:   part number:    74752-1701
> enp7s0f2:   revision level:
> enp7s0f2:   serial number:  726341570
> 
>>> Offset 6 – refers to 1000base-cx which is supported .
>>
>> Ok, that makes sense and argues for moving the passive check first, 
>> although the code doesn't support being able to switch to 1000Base-CX.
>>
>>