From nobody Tue Aug 22 22:39:35 2023 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4RVkn04xPxz4qYJm for ; Tue, 22 Aug 2023 22:39:48 +0000 (UTC) (envelope-from kevin.bowling@kev009.com) Received: from mail-pf1-x433.google.com (mail-pf1-x433.google.com [IPv6:2607:f8b0:4864:20::433]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4RVkn033V3z4TK2 for ; Tue, 22 Aug 2023 22:39:48 +0000 (UTC) (envelope-from kevin.bowling@kev009.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-pf1-x433.google.com with SMTP id d2e1a72fcca58-68a3236a414so2873170b3a.0 for ; Tue, 22 Aug 2023 15:39:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kev009.com; s=google; t=1692743987; x=1693348787; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=vePIr0jqPTLv1V9Aij6wnKeJYX5/h09Oa4V1mYJa31Q=; b=pur6ZGvJDlvsNwxughvQHVxy2uo1gmMo897Fo49s443Q/Atd1FXTHHzPMdepPaGIsc kolQZlOlz1QLHC1vAZDdI3fQeImJ+OKneopPKFjqIvDJHYxP5fT+Z5nB1Q5rI1Bc/u8r BtQ0GFhjgWcC97ILzZCJC76ssZ+4c3F2/iikM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1692743987; x=1693348787; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=vePIr0jqPTLv1V9Aij6wnKeJYX5/h09Oa4V1mYJa31Q=; b=SIgPlC6SkFH6nSiqhq9dV/oaygNjZTc4oUSsyNy2AAlzJ1Xqpr80YUvjAEtHq+X3pB z0JM5DOw1VV/v1SaWE3T1dHkbCv3ekftRwd7p7bmqyZGVlTYLMcssdPVO5OCGGrZdN8W jAyTuqOPhps126bTEdS4nkMg9r5+4lSNXBwyeDxoMUmXVwVpjwidcr9ACRxZB9k7wyVk 7A4QwB97Xs+/aoqgvlfXHw48znggEecDfwE9eFwOK2XZJoaj/WilyA5rPYuemKD0NkzT nCwRRrP7OtCi2AfQZg9aHgt3S1cJfz4w+eNeeTpBHzOHgEQ7XW4Us1jYTKfEnT3jIsaW +Hnw== X-Gm-Message-State: AOJu0YxolcJFsas2IpZKoBBbCNVHTLXxjyrE/E+/eqSJ3GPK04pE9WxL dRmznQ4s/oC039JK1DhQhHyRjXVaLLvqxK7Hru8oLg== X-Google-Smtp-Source: AGHT+IECMoO5FNpARycR5EkNvgJmxAs7EtSQYOg5URollFC1guFp2bXdU7ffO0ilwT6CDrpyuweVIHYIS/q9haTe104= X-Received: by 2002:a05:6a20:7489:b0:13d:b8ed:a5b8 with SMTP id p9-20020a056a20748900b0013db8eda5b8mr14555037pzd.12.1692743987182; Tue, 22 Aug 2023 15:39:47 -0700 (PDT) List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 References: <202211042255.2A4MtqVv032693@gitrepo.freebsd.org> <419530F4-DB8F-46EA-BF45-C130BCF8A3DE@FreeBSD.org> <05979953-C089-4D56-99E3-BBBF066FE277@FreeBSD.org> In-Reply-To: <05979953-C089-4D56-99E3-BBBF066FE277@FreeBSD.org> From: Kevin Bowling Date: Tue, 22 Aug 2023 15:39:35 -0700 Message-ID: Subject: Re: git: 91fcacc35597 - main - if_bnxt: Add support for VLAN on Thor To: Kristof Provost Cc: Sumit Saxena , Warner Losh , dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org, src-committers@freebsd.org Content-Type: multipart/alternative; boundary="0000000000000d585606038aac0f" X-Rspamd-Queue-Id: 4RVkn033V3z4TK2 X-Spamd-Bar: ---- X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US] --0000000000000d585606038aac0f Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, Aug 22, 2023 at 2:07 PM Kristof Provost 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 > AuthorDate: 2022-11-04 22:24:32 +0000 > Commit: Warner Losh > 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 i= s > 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 the if reset functionality was added. You can check e1000 for that. Note I don=E2=80=99t have this hw and haven=E2=80=99t audited the code so t= his could be noise but it=E2=80=99s an easy thing to check. Best regards, > Kristof > --0000000000000d585606038aac0f Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


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=3D91fcacc355971f74aa26fc7861020dc3a2a2d717

commit 91fcacc355971f74aa2= 6fc7861020dc3a2a2d717
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 f= or VLAN on Thor

Reviewed by: imp
Differential Revision: https://reviews.freebsd.o= rg/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 a= ppears to have broken vlan on these interfaces.
My hardware is a Broadcom BCM57416 NetXtreme-E 10GBase-T Ethernet, (in a De= ll 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 conne= ctivity. 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 t= op of bnxt0 without losing connectivity.


Should we revert this befo= re 14.0?

There=E2=80=99s also a rep= ort 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 mo= difications.. that sounds unlikely and was probably an accident when the if= reset functionality was added.=C2=A0 You can check e1000 for that.

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.

Best regards,
Kristof

--0000000000000d585606038aac0f--