Fix ixgbe flow control autoneg reporting
ClosedPublic
Actions

Authored by smh on Jan 27 2016, 12:34 PM.

Details

Reviewers

erj
sbruno

Group Reviewers

Intel Networking

Commits

rS295094: Fix ixgbe flow control autoneg reporting

Summary

Fix ixgbe reporting of flow control autoneg when running under DBG 1

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 2286
Build 2295: arc lint + arc unit

Event Timeline

smh updated this revision to Diff 12753.Jan 27 2016, 12:34 PM

smh retitled this revision from to Fix ixgbe flow control autoneg reporting.

smh updated this object.

smh edited the test plan for this revision. (Show Details)

smh added reviewers: sbruno, erj.

Herald added a reviewer: Intel Networking. · View Herald TranscriptJan 27 2016, 12:34 PM

Herald added a subscriber: imp. · View Herald Transcript

We need to be careful here, as this is shared code. Our Linux drivers use this as well.

In D5089#108263, @jeffrey.e.pieper_intel.com wrote:

We need to be careful here, as this is shared code. Our Linux drivers use this as well.

We can trivially wrap this in a #ifdef FreeBSD

How's that sound?

In D5089#108263, @jeffrey.e.pieper_intel.com wrote:

We need to be careful here, as this is shared code. Our Linux drivers use this as well.

The fix should be relevant to all versions, currently its printing out unsupported even when it is actually supported, so shouldn't be a problem?

Well Linux changes FC settings through ethtool, so maybe that makes a difference. I'm pretty sure FC in the Linux driver works as expected. I was saying we need to be careful when changing shared code because as it may not get accepted when it goes though our code review internally. Eric may have a different opinion, although Sean's suggestion seems fine to me.

In D5089#108282, @jeffrey.e.pieper_intel.com wrote:

Well Linux changes FC settings through ethtool, so maybe that makes a difference. I'm pretty sure FC in the Linux driver works as expected. I was saying we need to be careful when changing shared code because as it may not get accepted when it goes though our code review internally. Eric may have a different opinion, although Sean's suggestion seems fine to me.

I think there may be a miss-understanding here, this is purely related to the debug output the results of ixgbe_device_supports_autoneg_fc reporting when it shouldn't.

This function specifically calculates if FC autoneg is supported and stores that in the supported var which is returned, the debug output wasn't checking the result of the calculation instead always saying it was unsupported which is incorrect.

The fix is to only output the unsupported message if supported is false.

Does that make more sense?

It looks like this does legitimately fix a bug; @jeffrey.e.pieper_intel.com we can work on getting this fixed internally.

This revision is now accepted and ready to land.Jan 27 2016, 6:39 PM

Closed by commit rS295094: Fix ixgbe flow control autoneg reporting (authored by smh). · Explain WhyJan 31 2016, 3:18 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Path

Size

sys/

dev/

ixgbe/

ixgbe_common.c

5 lines

Diff 12753

View Options

sys/dev/ixgbe/ixgbe_common.c

Fix ixgbe flow control autoneg reportingClosedPublicActions