Date: Tue, 22 Aug 2023 15:39:35 -0700 From: Kevin Bowling <kevin.bowling@kev009.com> To: Kristof Provost <kp@freebsd.org> Cc: Sumit Saxena <sumit.saxena@broadcom.com>, Warner Losh <imp@freebsd.org>, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org, src-committers@freebsd.org Subject: Re: git: 91fcacc35597 - main - if_bnxt: Add support for VLAN on Thor Message-ID: <CAK7dMtCJhwpV96znB4if48FtWEy091ss-nmvMp5zoLUvtkAs%2BA@mail.gmail.com> In-Reply-To: <05979953-C089-4D56-99E3-BBBF066FE277@FreeBSD.org> References: <202211042255.2A4MtqVv032693@gitrepo.freebsd.org> <419530F4-DB8F-46EA-BF45-C130BCF8A3DE@FreeBSD.org> <05979953-C089-4D56-99E3-BBBF066FE277@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --] 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. 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. Best regards, > Kristof > [-- Attachment #2 --] <div><br></div><div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Aug 22, 2023 at 2:07 PM Kristof Provost <<a href="mailto:kp@freebsd.org">kp@freebsd.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;padding-left:1ex;border-left-color:rgb(204,204,204)"><u></u> <div><div style="font-family:sans-serif"></div></div><div><div style="font-family:sans-serif"><div style="white-space:normal;font-family:sans-serif"> <p dir="auto" style="font-family:sans-serif">On 27 Jun 2023, at 11:17, Kristof Provost wrote:</p> </div><div style="white-space:normal;font-family:sans-serif"><blockquote style="margin:0px 0px 5px;padding-left:5px;border-left-width:2px;border-left-style:solid;font-family:sans-serif;border-left-color:rgb(19,107,206);color:rgb(19,107,206)"><p dir="auto" style="font-family:sans-serif">On 4 Nov 2022, at 23:55, Warner Losh wrote:</p> <blockquote style="margin:0px 0px 5px;padding-left:5px;border-left-width:2px;border-left-style:solid;font-family:sans-serif;border-left-color:rgb(75,137,207);color:rgb(75,137,207)"><p dir="auto" style="font-family:sans-serif">The branch main has been updated by imp:</p> <p dir="auto" style="font-family:sans-serif">URL: <a href="https://cgit.FreeBSD.org/src/commit/?id=91fcacc355971f74aa26fc7861020dc3a2a2d717" target="_blank" style="font-family:sans-serif">https://cgit.FreeBSD.org/src/commit/?id=91fcacc355971f74aa26fc7861020dc3a2a2d717</a></p>; <p dir="auto" style="font-family:sans-serif">commit 91fcacc355971f74aa26fc7861020dc3a2a2d717 <br> Author: Sumit Saxena <<a href="mailto:sumit.saxena@broadcom.com" target="_blank" style="font-family:sans-serif">sumit.saxena@broadcom.com</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</p> <p dir="auto" style="font-family:sans-serif"> if_bnxt: Add support for VLAN on Thor</p> <p dir="auto" style="font-family:sans-serif"> Reviewed by: imp <br> Differential Revision: <a href="https://reviews.freebsd.org/D36443" target="_blank" style="font-family:sans-serif">https://reviews.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(-)</p> </blockquote><p dir="auto" style="font-family:sans-serif">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).</p> <p dir="auto" style="font-family:sans-serif">A simple vlan creation on top of one: <br> ifconfig vlan create <br> ifconfig vlan0 vlan 201 vlandev bnxt0</p> <p dir="auto" style="font-family:sans-serif">Results in a loss of connectivity. The kernel logs this, which I assume is related:</p> <p dir="auto" style="font-family:sans-serif">`bnxt0: HWRM_CFA_L2_FILTER_ALLOC command returned INVALID_PARAMS error.`</p> <p dir="auto" style="font-family:sans-serif">After reverting this (and 72e9dbb58cad5262190cf2eae47f764021072128) I can create vlan interfaces on top of bnxt0 without losing connectivity.</p> <br></blockquote></div> </div></div><div><div style="font-family:sans-serif"><div style="white-space:normal;font-family:sans-serif"> <p dir="auto" style="font-family:sans-serif">Should we revert this before 14.0?</p> <p dir="auto" style="font-family:sans-serif">There’s also a report of what I think is the same issue in <a href="https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=269133" target="_blank" style="font-family:sans-serif">https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=269133</a></p>; <p dir="auto" style="font-family:sans-serif">There’s a proposed patch there, but it did not work for me.</p></div></div></div></blockquote><div dir="auto"><br></div><div dir="auto">In markj’s analysis he mentions the issue of the reinit.</div><div dir="auto"><br></div><div dir="auto">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.</div><div dir="auto"><br></div><div dir="auto">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.</div><div dir="auto"><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;padding-left:1ex;border-left-color:rgb(204,204,204)"><div><div style="font-family:sans-serif"><div style="white-space:normal;font-family:sans-serif"><p dir="auto" style="font-family:sans-serif"></p> <p dir="auto" style="font-family:sans-serif">Best regards,<br> Kristof</p> </div> </div> </div> </blockquote></div></div>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAK7dMtCJhwpV96znB4if48FtWEy091ss-nmvMp5zoLUvtkAs%2BA>