Page MenuHomeFreeBSD

bnxt: Workaround for ifconfig showing media as 'Other'
AbandonedPublic

Authored by bhargava.marreddy_broadcom.com on Nov 1 2017, 11:46 AM.
Tags
None
Referenced Files
Unknown Object (File)
Dec 1 2024, 7:54 PM
Unknown Object (File)
Nov 28 2024, 8:14 AM
Unknown Object (File)
Nov 25 2024, 12:39 AM
Unknown Object (File)
Nov 1 2024, 3:02 PM
Unknown Object (File)
Oct 29 2024, 8:31 AM
Unknown Object (File)
Sep 26 2024, 9:33 PM
Unknown Object (File)
Sep 16 2024, 2:05 PM
Unknown Object (File)
Sep 16 2024, 2:04 PM
Subscribers

Details

Summary

Problem:

Ifconfig showing media as 'Other' only for bnxt interfaces where as for ix1 (Intel 10G NIC) it is showing properly.

Output of 'ifconfig bnxt0' with current driver:
        media: Ethernet **`Other`** (100GBase-CR4 <full-duplex>)
     
Expected result:-
         media: Ethernet **`100GBase-CR4`** (100GBase-CR4 <full-duplex>)

Workaround details:-

Ifconfig uses ifmr->ifm_current to display 'media type', setting it in driver for now as quick workaround.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

Problem description:-
Ifconfig showing media as 'Other' only for bnxt interfaces, where as for ix1 (Intel 10G NIC) it is showing properly.

Output of 'ifconfig bnxt0' with current driver:-
media: Ethernet Other (100GBase-CR4 <full-duplex>)

Expected result of 'ifconfig bnxt0':-
media: Ethernet 100GBase-CR4 (100GBase-CR4 <full-duplex>)

Workaround details:-
Ifconfig uses ifmr->ifm_current to display 'media type', setting it in driver for now as quick workaround.

shurd requested changes to this revision.Nov 1 2017, 4:54 PM

This looks like it would break the autoselect indication.

sys/dev/bnxt/if_bnxt.c
1394

What's wrong with:

if (softc->link_info.autoneg & BNXT_AUTONEG_SPEED)
    ifmedia_set(softc->media, IFM_ETHER | IFM_AUTO);
else
    ifmedia_set(softc->media, ifmr->ifm_active);

It looks like bnxt_media_status() is called every time the media type may change, so it should be filled by ifmedia_ioctl().

An interesting question is what happens if you change the SFP module type while the driver is attached (ie: Remove a direct attach module, and insert a fibre one).

Also, are you forcing a media type? The result I would expect is "media: Ethernet autoselect (100GBase-CR4 <full-duplex>)".

This revision now requires changes to proceed.Nov 1 2017, 4:54 PM

Quick update:-

This change caused Crash in ifmedia_set(). in following line,


 void
ifmedia_set(ifm, target)
    struct ifmedia *ifm;
    int target;

  {
    struct ifmedia_entry *match;

    match = ifmedia_match(ifm, target, ifm->ifm_mask);

    if (match == NULL) {
            printf("ifmedia_set: no match for 0x%x/0x%x\n",
                target, ~ifm->ifm_mask);
            panic("ifmedia_set");   **<=== Crashed here**
    }//

Snip from Crash dump:-

bnxt0: Link is UP full duplex, FC - receive & transmit - 100000 Mbps
<5>bnxt0: link state changed to UP
ifmedia_set: no match for 0x100e2f/0xfffffff
panic: ifmedia_set
cpuid = 8
time = 1509716949
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe2025018450
vpanic() at vpanic+0x198/frame 0xfffffe20250184d0
panic() at panic+0x43/frame 0xfffffe2025018530
ifmedia_set() at ifmedia_set+0x4d/frame 0xfffffe2025018540
bnxt_media_status() at bnxt_media_status+0x1dc/frame 0xfffffe2025018580
bnxt_init() at bnxt_init+0x6d8/frame 0xfffffe2025018600
iflib_init_locked() at iflib_init_locked+0x2e6/frame 0xfffffe2025018660
iflib_if_init() at iflib_if_init+0x47/frame 0xfffffe2025018690
iflib_if_ioctl() at iflib_if_ioctl+0x839/frame 0xfffffe2025018700
in_control() at in_control+0x95b/frame 0xfffffe2025018790
ifioctl() at ifioctl+0x12ce/frame 0xfffffe2025018830
kern_ioctl() at kern_ioctl+0x237/frame 0xfffffe20250188a0
sys_ioctl() at sys_ioctl+0x171/frame 0xfffffe2025018980
amd64_syscall() at amd64_syscall+0x30e/frame 0xfffffe2025018ab0
Xfast_syscall() at Xfast_syscall+0xfb/frame 0xfffffe2025018ab0

  • syscall (54, FreeBSD ELF64, sys_ioctl), rip = 0x80100a3da, rsp = 0x7fffffffe238, rbp = 0x7fffffffe290 ---

Uptime: 1d7h20m38s
Dumping 6567 out of 130946 MB:..1%..11%..21%..31%..41%..51%..61%..71%..81%..91%

Ah right, you likely need to mask off IFM_GMASK to remove the duplex/flow control bits as well... ifmedia_set(softc->media, ifmr->ifm_active & ~IFM_GMASK);

Thanks Stephen,

That solved Autoselect problem but still seeing as "Other" when in forced (100G) mode.

##ifconfig bnxt0 media autoselect

ifmr.ifm_active = 0x100e2f, ifmr.ifm_current = 0x20

Snip from ifconfig ==> 
   media: Ethernet autoselect (100GBase-CR4 <full-duplex,rxpause,txpause>)
Comment ==> Looks  Good

##ifconfig bnxt0 media 100GBase-CR4

ifmr.ifm_active = 0x100e2f, ifmr.ifm_current = 0x3f

Snip from ifconfig ==> 
   media: Ethernet Other (100GBase-CR4 <full-duplex,rxpause,txpause>) 
Comment ==> "Other" is not expected, right?

Just to add, with following patch, seeing "100GBase-CR4" in place of "Other" ...
(Note:- Values are ifmr.ifm_active = 0x100e2f, ifmr.ifm_current = 0x82f)

if (softc->link_info.autoneg & BNXT_AUTONEG_SPEED) {
         ifmedia_set(softc->media, IFM_ETHER | IFM_AUTO);
 } else {
         ifmedia_set(softc->media, ifmr->ifm_active & ~IFM_GMASK);
         ifmr->ifm_current = (ifmr->ifm_active & ~IFM_GMASK);
 }

Ah, you likely need to clear IFM_ETH_FMASK as well. I think that should be part of the default ifm_mask, but masking that off here should be good for now.

Hi Stephen,

Ah, you likely need to clear IFM_ETH_FMASK as well.

<Chenna>
Can you please confirm if this is what you are suggesting?

if (softc->link_info.autoneg & BNXT_AUTONEG_SPEED) {
         ifmedia_set(softc->media, IFM_ETHER | IFM_AUTO);
} else {
         ifmedia_set(softc->media, ifmr->ifm_active & ~(IFM_GMASK|IFM_ETH_FMASK));
}

But, I still see Crash with that code, Please find attached screenshot.

Hi Stephen,

Ah, you likely need to clear IFM_ETH_FMASK as well.

<Chenna>
Can you please confirm if this is what you are suggesting?

if (softc->link_info.autoneg & BNXT_AUTONEG_SPEED) {
         ifmedia_set(softc->media, IFM_ETHER | IFM_AUTO);
} else {
         ifmedia_set(softc->media, ifmr->ifm_active & ~(IFM_GMASK|IFM_ETH_FMASK));
}

But, I still see Crash with that code, Please find attached screenshot.

Yes, that's what I was suggesting. I was unable to find any attached screenshot.

If you could dump the list of supported media, and the reported media, that would be helpful.

Hi,

Please ignore Crash as that is not related to this particulate issue.

In brief: I still see the problem as ifconfig still displaying "Other"

Please find by observation below.

ifmr->ifm_name: bnxt2
ifmr->ifm_current: 0x3f
ifmr->ifm_mask: 0xf0000000
ifmr->ifm_status:0x3
ifmr->ifm_active:0x100e2f

Value after applying different masks:    
ifmr->ifm_active & ~(IFM_GMASK) = 0xe2f, 
ifmr->ifm_active & ~(IFM_GMASK|IFM_ETH_FMASK))) = 0x82f)

ifmr->ifm_count: 7

And those 7 are ==> 
1. 0x30
2. 0x822
3. 0x835
4. 0x3b
5. 0x838
6. 0x82f
7. 0x20

Snip from ifconfig:-

media: Ethernet Other (100GBase-CR4 <full-duplex,rxpause,txpause>)
status: active

Question:-
I went through the sources if ifconfg.c, it looks like it is considering ifmr->ifm_current to decide whether to print as "Other" / <Given Speed>.
And, that is the reason I added following line in patch which fixed the issue.

ifmr->ifm_current = (ifmr->ifm_active & ~IFM_GMASK);

Looks like ifmedia_set is not touching ifmr->ifm_current, any other ways to update ifmr->ifm_current?

In brief: I still see the problem as ifconfig still displaying "Other"

ifmr->ifm_current: 0x3f

This looks like it's gone through compat_media() in if_media.c, which results in the "Other".

ifmr->ifm_active:0x100e2f

So the subtype here is 0x80f.

Value after applying different masks:    
ifmr->ifm_active & ~(IFM_GMASK) = 0xe2f, 
ifmr->ifm_active & ~(IFM_GMASK|IFM_ETH_FMASK))) = 0x82f)

Subtype is still 0x80f.

And those 7 are ==> 
6. 0x82f

So it's in the supported list (which is good).

Snip from ifconfig:-

media: Ethernet Other (100GBase-CR4 <full-duplex,rxpause,txpause>)
status: active

Question:-
I went through the sources if ifconfg.c, it looks like it is considering ifmr->ifm_current to decide whether to print as "Other" / <Given Speed>.
And, that is the reason I added following line in patch which fixed the issue.

And ifm_current is set from ifm_cur at if_media.c:307, which is set by ifmedia_set() and the IFMEDIA ioctl()... unless its SIOCGIFMEDIA, in which case, it goes through compat_media() and is "Other" any time it's not

ifmr->ifm_current = (ifmr->ifm_active & ~IFM_GMASK);

Looks like ifmedia_set is not touching ifmr->ifm_current, any other ways to update ifmr->ifm_current?

So, it seems iflib.c does not have support for SIOCGIFXMEDIA. I've added that as D13312. Can you try applying that change and see if it fixes the "Other" issue for you?

So with D13312 committed, is there still a need for a change? If not, please abandon this review.

So with D13312 committed, is there still a need for a change? If not, please abandon this review.

Verified the change with D13312 alone, it seems to be working fine, I can abandon this now. Thank you!!