Re: git: 91fcacc35597 - main - if_bnxt: Add support for VLAN on Thor

From: Warner Losh <imp_at_bsdimp.com>
Date: Wed, 23 Aug 2023 00:31:19 UTC
On Tue, Aug 22, 2023, 6:26 PM Warner Losh <imp@bsdimp.com> wrote:

>
>
> On Tue, Aug 22, 2023, 5:59 PM Kevin Bowling <kevin.bowling@kev009.com>
> wrote:
>
>> On Tue, Aug 22, 2023 at 3:39 PM Kevin Bowling <kevin.bowling@kev009.com>
>> wrote:
>> >
>> >
>> >
>> > On Tue, Aug 22, 2023 at 2:07 PM Kristof Provost <kp@freebsd.org> wrote:
>> >>
>> >> On 27 Jun 2023, at 11:17, Kristof Provost wrote:
>> >>
>> >> On 4 Nov 2022, at 23:55, Warner Losh wrote:
>> >>
>> >> The branch main has been updated by imp:
>> >>
>> >> URL:
>> https://cgit.FreeBSD.org/src/commit/?id=91fcacc355971f74aa26fc7861020dc3a2a2d717
>> >>
>> >> commit 91fcacc355971f74aa26fc7861020dc3a2a2d717
>> >> Author: Sumit Saxena <sumit.saxena@broadcom.com>
>> >> AuthorDate: 2022-11-04 22:24:32 +0000
>> >> Commit: Warner Losh <imp@FreeBSD.org>
>> >> CommitDate: 2022-11-04 22:24:32 +0000
>> >>
>> >> if_bnxt: Add support for VLAN on Thor
>> >>
>> >> Reviewed by: imp
>> >> Differential Revision: https://reviews.freebsd.org/D36443
>> >> ---
>> >> sys/dev/bnxt/bnxt.h | 3 +-
>> >> sys/dev/bnxt/bnxt_hwrm.c | 110
>> ++++++++++++++++++++++++++++-------------------
>> >> sys/dev/bnxt/bnxt_hwrm.h | 7 ++-
>> >> sys/dev/bnxt/if_bnxt.c | 7 ++-
>> >> 4 files changed, 74 insertions(+), 53 deletions(-)
>> >>
>> >> This commit appears to have broken vlan on these interfaces.
>> >> My hardware is a Broadcom BCM57416 NetXtreme-E 10GBase-T Ethernet, (in
>> a Dell T640, if that helps).
>> >>
>> >> A simple vlan creation on top of one:
>> >> ifconfig vlan create
>> >> ifconfig vlan0 vlan 201 vlandev bnxt0
>> >>
>> >> Results in a loss of connectivity. The kernel logs this, which I
>> assume is related:
>> >>
>> >> `bnxt0: HWRM_CFA_L2_FILTER_ALLOC command returned INVALID_PARAMS
>> error.`
>> >>
>> >> After reverting this (and 72e9dbb58cad5262190cf2eae47f764021072128) I
>> can create vlan interfaces on top of bnxt0 without losing connectivity.
>> >>
>> >>
>> >> Should we revert this before 14.0?
>> >>
>> >> There’s also a report of what I think is the same issue in
>> https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=269133
>> >>
>> >> There’s a proposed patch there, but it did not work for me.
>> >
>> >
>> > In markj’s analysis he mentions the issue of the reinit.
>> >
>> > It sounds like there are multiple bugs in this driver but one quick
>> band aid may be to mark the driver as not needing re-init for VLAN
>> modifications.. that sounds unlikely and was probably an accident when the
>> if reset functionality was added.  You can check e1000 for that.
>>
>> Here's what I mean as a patch https://reviews.freebsd.org/D41558
>>
>> > Note I don’t have this hw and haven’t audited the code so this could be
>> noise but it’s an easy thing to check.
>>
>
> Is there a good bug we can post that link to for someone to test?
>

Duh.. posted to bug above and
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=222680

Warner


> Warner
>
>> >
>> >> Best regards,
>> >> Kristof
>>
>