Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 22 Aug 2023 18:26:52 -0600
From:      Warner Losh <imp@bsdimp.com>
To:        Kevin Bowling <kevin.bowling@kev009.com>
Cc:        Kristof Provost <kp@freebsd.org>, Sumit Saxena <sumit.saxena@broadcom.com>,  Warner Losh <imp@freebsd.org>,  "<dev-commits-src-all@freebsd.org>" <dev-commits-src-all@freebsd.org>,  "<dev-commits-src-main@freebsd.org>" <dev-commits-src-main@freebsd.org>,  src-committers <src-committers@freebsd.org>
Subject:   Re: git: 91fcacc35597 - main - if_bnxt: Add support for VLAN on Thor
Message-ID:  <CANCZdfrO7gJDsnUu465CN96V4eWowtD-eqH2340u1-Yo14F4_w@mail.gmail.com>
In-Reply-To: <CAK7dMtA6FL%2BCzw9Z2TC2-N0cA=WshDBsEVam0__ift9VfFrq7Q@mail.gmail.com>
References:  <202211042255.2A4MtqVv032693@gitrepo.freebsd.org> <419530F4-DB8F-46EA-BF45-C130BCF8A3DE@FreeBSD.org> <05979953-C089-4D56-99E3-BBBF066FE277@FreeBSD.org> <CAK7dMtCJhwpV96znB4if48FtWEy091ss-nmvMp5zoLUvtkAs%2BA@mail.gmail.com> <CAK7dMtA6FL%2BCzw9Z2TC2-N0cA=WshDBsEVam0__ift9VfFrq7Q@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
--000000000000b795f806038c2b3e
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Aug 22, 2023, 5:59 PM Kevin Bowling <kevin.bowling@kev009.com>
wrote:

> On Tue, Aug 22, 2023 at 3:39=E2=80=AFPM Kevin Bowling <kevin.bowling@kev0=
09.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=3D91fcacc355971f74aa26fc7861020dc=
3a2a2d717
> >>
> >> 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 assum=
e
> 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=E2=80=99s also a report of what I think is the same issue in
> https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=3D269133
> >>
> >> There=E2=80=99s a proposed patch there, but it did not work for me.
> >
> >
> > In markj=E2=80=99s analysis he mentions the issue of the reinit.
> >
> > It sounds like there are multiple bugs in this driver but one quick ban=
d
> aid may be to mark the driver as not needing re-init for VLAN
> modifications.. that sounds unlikely and was probably an accident when th=
e
> 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=E2=80=99t have this hw and haven=E2=80=99t audited the code =
so this could be
> noise but it=E2=80=99s an easy thing to check.
>

Is there a good bug we can post that link to for someone to test?

Warner

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

--000000000000b795f806038c2b3e
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"auto"><div><br><br><div class=3D"gmail_quote"><div dir=3D"ltr" =
class=3D"gmail_attr">On Tue, Aug 22, 2023, 5:59 PM Kevin Bowling &lt;<a hre=
f=3D"mailto:kevin.bowling@kev009.com">kevin.bowling@kev009.com</a>&gt; wrot=
e:<br></div><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;bo=
rder-left:1px #ccc solid;padding-left:1ex">On Tue, Aug 22, 2023 at 3:39=E2=
=80=AFPM Kevin Bowling &lt;<a href=3D"mailto:kevin.bowling@kev009.com" targ=
et=3D"_blank" rel=3D"noreferrer">kevin.bowling@kev009.com</a>&gt; wrote:<br=
>
&gt;<br>
&gt;<br>
&gt;<br>
&gt; On Tue, Aug 22, 2023 at 2:07 PM Kristof Provost &lt;<a href=3D"mailto:=
kp@freebsd.org" target=3D"_blank" rel=3D"noreferrer">kp@freebsd.org</a>&gt;=
 wrote:<br>
&gt;&gt;<br>
&gt;&gt; On 27 Jun 2023, at 11:17, Kristof Provost wrote:<br>
&gt;&gt;<br>
&gt;&gt; On 4 Nov 2022, at 23:55, Warner Losh wrote:<br>
&gt;&gt;<br>
&gt;&gt; The branch main has been updated by imp:<br>
&gt;&gt;<br>
&gt;&gt; URL: <a href=3D"https://cgit.FreeBSD.org/src/commit/?id=3D91fcacc3=
55971f74aa26fc7861020dc3a2a2d717" rel=3D"noreferrer noreferrer" target=3D"_=
blank">https://cgit.FreeBSD.org/src/commit/?id=3D91fcacc355971f74aa26fc7861=
020dc3a2a2d717</a><br>
&gt;&gt;<br>
&gt;&gt; commit 91fcacc355971f74aa26fc7861020dc3a2a2d717<br>
&gt;&gt; Author: Sumit Saxena &lt;<a href=3D"mailto:sumit.saxena@broadcom.c=
om" target=3D"_blank" rel=3D"noreferrer">sumit.saxena@broadcom.com</a>&gt;<=
br>
&gt;&gt; AuthorDate: 2022-11-04 22:24:32 +0000<br>
&gt;&gt; Commit: Warner Losh &lt;imp@FreeBSD.org&gt;<br>
&gt;&gt; CommitDate: 2022-11-04 22:24:32 +0000<br>
&gt;&gt;<br>
&gt;&gt; if_bnxt: Add support for VLAN on Thor<br>
&gt;&gt;<br>
&gt;&gt; Reviewed by: imp<br>
&gt;&gt; Differential Revision: <a href=3D"https://reviews.freebsd.org/D364=
43" rel=3D"noreferrer noreferrer" target=3D"_blank">https://reviews.freebsd=
.org/D36443</a><br>
&gt;&gt; ---<br>
&gt;&gt; sys/dev/bnxt/bnxt.h | 3 +-<br>
&gt;&gt; sys/dev/bnxt/bnxt_hwrm.c | 110 ++++++++++++++++++++++++++++-------=
------------<br>
&gt;&gt; sys/dev/bnxt/bnxt_hwrm.h | 7 ++-<br>
&gt;&gt; sys/dev/bnxt/if_bnxt.c | 7 ++-<br>
&gt;&gt; 4 files changed, 74 insertions(+), 53 deletions(-)<br>
&gt;&gt;<br>
&gt;&gt; This commit appears to have broken vlan on these interfaces.<br>
&gt;&gt; My hardware is a Broadcom BCM57416 NetXtreme-E 10GBase-T Ethernet,=
 (in a Dell T640, if that helps).<br>
&gt;&gt;<br>
&gt;&gt; A simple vlan creation on top of one:<br>
&gt;&gt; ifconfig vlan create<br>
&gt;&gt; ifconfig vlan0 vlan 201 vlandev bnxt0<br>
&gt;&gt;<br>
&gt;&gt; Results in a loss of connectivity. The kernel logs this, which I a=
ssume is related:<br>
&gt;&gt;<br>
&gt;&gt; `bnxt0: HWRM_CFA_L2_FILTER_ALLOC command returned INVALID_PARAMS e=
rror.`<br>
&gt;&gt;<br>
&gt;&gt; After reverting this (and 72e9dbb58cad5262190cf2eae47f764021072128=
) I can create vlan interfaces on top of bnxt0 without losing connectivity.=
<br>
&gt;&gt;<br>
&gt;&gt;<br>
&gt;&gt; Should we revert this before 14.0?<br>
&gt;&gt;<br>
&gt;&gt; There=E2=80=99s also a report of what I think is the same issue in=
 <a href=3D"https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=3D269133" rel=
=3D"noreferrer noreferrer" target=3D"_blank">https://bugs.freebsd.org/bugzi=
lla/show_bug.cgi?id=3D269133</a><br>
&gt;&gt;<br>
&gt;&gt; There=E2=80=99s a proposed patch there, but it did not work for me=
.<br>
&gt;<br>
&gt;<br>
&gt; In markj=E2=80=99s analysis he mentions the issue of the reinit.<br>
&gt;<br>
&gt; It sounds like there are multiple bugs in this driver but one quick ba=
nd aid may be to mark the driver as not needing re-init for VLAN modificati=
ons.. that sounds unlikely and was probably an accident when the if reset f=
unctionality was added.=C2=A0 You can check e1000 for that.<br>
<br>
Here&#39;s what I mean as a patch <a href=3D"https://reviews.freebsd.org/D4=
1558" rel=3D"noreferrer noreferrer" target=3D"_blank">https://reviews.freeb=
sd.org/D41558</a><br>
<br>
&gt; Note I don=E2=80=99t have this hw and haven=E2=80=99t audited the code=
 so this could be noise but it=E2=80=99s an easy thing to check.<br></block=
quote></div></div><div dir=3D"auto"><br></div><div dir=3D"auto">Is there a =
good bug we can post that link to for someone to test?</div><div dir=3D"aut=
o"><br></div><div dir=3D"auto">Warner</div><div dir=3D"auto"><div class=3D"=
gmail_quote"><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;b=
order-left:1px #ccc solid;padding-left:1ex">
&gt;<br>
&gt;&gt; Best regards,<br>
&gt;&gt; Kristof<br>
</blockquote></div></div></div>

--000000000000b795f806038c2b3e--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfrO7gJDsnUu465CN96V4eWowtD-eqH2340u1-Yo14F4_w>