Date: Tue, 22 Aug 2023 18:31:19 -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: <CANCZdfr6fkKPLVA2JAHiSJSsJj9-tQbFGuK%2BPj2JsEZ24qRENw@mail.gmail.com> In-Reply-To: <CANCZdfrO7gJDsnUu465CN96V4eWowtD-eqH2340u1-Yo14F4_w@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> <CANCZdfrO7gJDsnUu465CN96V4eWowtD-eqH2340u1-Yo14F4_w@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--0000000000009562ad06038c3b84 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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=E2=80=AFPM Kevin Bowling <kevin.bowling@kev= 009.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=3D91fcacc355971f74aa26fc7861020d= c3a2a2d717 >> >> >> >> 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, (i= n >> 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=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 >> 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 t= he >> 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? > Duh.. posted to bug above and https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=3D222680 Warner > Warner > >> > >> >> Best regards, >> >> Kristof >> > --0000000000009562ad06038c3b84 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, 6:26 PM Warner Losh <<a href= =3D"mailto:imp@bsdimp.com">imp@bsdimp.com</a>> wrote:<br></div><blockquo= te class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px #ccc so= lid;padding-left:1ex"><div dir=3D"auto"><div><br><br><div class=3D"gmail_qu= ote"><div dir=3D"ltr" class=3D"gmail_attr">On Tue, Aug 22, 2023, 5:59 PM Ke= vin Bowling <<a href=3D"mailto:kevin.bowling@kev009.com" target=3D"_blan= k" rel=3D"noreferrer">kevin.bowling@kev009.com</a>> wrote:<br></div><blo= ckquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px #c= cc solid;padding-left:1ex">On Tue, Aug 22, 2023 at 3:39=E2=80=AFPM Kevin Bo= wling <<a href=3D"mailto:kevin.bowling@kev009.com" rel=3D"noreferrer nor= eferrer" target=3D"_blank">kevin.bowling@kev009.com</a>> wrote:<br> ><br> ><br> ><br> > On Tue, Aug 22, 2023 at 2:07 PM Kristof Provost <<a href=3D"mailto:= kp@freebsd.org" rel=3D"noreferrer noreferrer" target=3D"_blank">kp@freebsd.= org</a>> wrote:<br> >><br> >> On 27 Jun 2023, at 11:17, Kristof Provost wrote:<br> >><br> >> On 4 Nov 2022, at 23:55, Warner Losh wrote:<br> >><br> >> The branch main has been updated by imp:<br> >><br> >> URL: <a href=3D"https://cgit.FreeBSD.org/src/commit/?id=3D91fcacc3= 55971f74aa26fc7861020dc3a2a2d717" rel=3D"noreferrer noreferrer noreferrer" = target=3D"_blank">https://cgit.FreeBSD.org/src/commit/?id=3D91fcacc355971f7= 4aa26fc7861020dc3a2a2d717</a><br> >><br> >> commit 91fcacc355971f74aa26fc7861020dc3a2a2d717<br> >> Author: Sumit Saxena <<a href=3D"mailto:sumit.saxena@broadcom.c= om" rel=3D"noreferrer noreferrer" target=3D"_blank">sumit.saxena@broadcom.c= om</a>><br> >> AuthorDate: 2022-11-04 22:24:32 +0000<br> >> Commit: Warner Losh <imp@FreeBSD.org><br> >> CommitDate: 2022-11-04 22:24:32 +0000<br> >><br> >> if_bnxt: Add support for VLAN on Thor<br> >><br> >> Reviewed by: imp<br> >> Differential Revision: <a href=3D"https://reviews.freebsd.org/D364= 43" rel=3D"noreferrer noreferrer noreferrer" target=3D"_blank">https://revi= ews.freebsd.org/D36443</a><br> >> ---<br> >> sys/dev/bnxt/bnxt.h | 3 +-<br> >> sys/dev/bnxt/bnxt_hwrm.c | 110 ++++++++++++++++++++++++++++-------= ------------<br> >> sys/dev/bnxt/bnxt_hwrm.h | 7 ++-<br> >> sys/dev/bnxt/if_bnxt.c | 7 ++-<br> >> 4 files changed, 74 insertions(+), 53 deletions(-)<br> >><br> >> This commit appears to have broken vlan on these interfaces.<br> >> My hardware is a Broadcom BCM57416 NetXtreme-E 10GBase-T Ethernet,= (in a Dell T640, if that helps).<br> >><br> >> A simple vlan creation on top of one:<br> >> ifconfig vlan create<br> >> ifconfig vlan0 vlan 201 vlandev bnxt0<br> >><br> >> Results in a loss of connectivity. The kernel logs this, which I a= ssume is related:<br> >><br> >> `bnxt0: HWRM_CFA_L2_FILTER_ALLOC command returned INVALID_PARAMS e= rror.`<br> >><br> >> After reverting this (and 72e9dbb58cad5262190cf2eae47f764021072128= ) I can create vlan interfaces on top of bnxt0 without losing connectivity.= <br> >><br> >><br> >> Should we revert this before 14.0?<br> >><br> >> 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 noreferrer" target=3D"_blank">https://bugs.freebs= d.org/bugzilla/show_bug.cgi?id=3D269133</a><br> >><br> >> There=E2=80=99s a proposed patch there, but it did not work for me= .<br> ><br> ><br> > In markj=E2=80=99s analysis he mentions the issue of the reinit.<br> ><br> > 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's what I mean as a patch <a href=3D"https://reviews.freebsd.org/D4= 1558" rel=3D"noreferrer noreferrer noreferrer" target=3D"_blank">https://re= views.freebsd.org/D41558</a><br> <br> > 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></blockqu= ote></div></div><div dir=3D"auto"><br></div><div dir=3D"auto">Duh.. posted = to bug above and</div><div dir=3D"auto"><a href=3D"https://bugs.freebsd.org= /bugzilla/show_bug.cgi?id=3D222680">https://bugs.freebsd.org/bugzilla/show_= bug.cgi?id=3D222680</a></div><div dir=3D"auto"><br></div><div dir=3D"auto">= Warner</div><div dir=3D"auto"><br></div><div dir=3D"auto"><div class=3D"gma= il_quote"><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;bord= er-left:1px #ccc solid;padding-left:1ex"><div dir=3D"auto"><div dir=3D"auto= "><br></div><div dir=3D"auto">Warner</div><div dir=3D"auto"><div class=3D"g= mail_quote"><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;bo= rder-left:1px #ccc solid;padding-left:1ex"> ><br> >> Best regards,<br> >> Kristof<br> </blockquote></div></div></div> </blockquote></div></div></div> --0000000000009562ad06038c3b84--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfr6fkKPLVA2JAHiSJSsJj9-tQbFGuK%2BPj2JsEZ24qRENw>