Message ID | 20230625115343.1603330-6-paweldembicki@gmail.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp6880699vqr; Sun, 25 Jun 2023 05:13:12 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4e+c8emSjvurH1xC8vZ9EeLbeoTYIOLZ19N5ScVicdSNK4H2CkErmhSHUpQjr59tt9sBgT X-Received: by 2002:a17:902:f688:b0:1b2:4042:d227 with SMTP id l8-20020a170902f68800b001b24042d227mr5270311plg.12.1687695192044; Sun, 25 Jun 2023 05:13:12 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1687695192; cv=none; d=google.com; s=arc-20160816; b=QlilykzZWAmFpwOUTPGbknsENufztYRBNHE5PpS5fCq+dg4t9aOreNbtLqRprIL4fT dGTJ7oK6/7H8u4tjSeEeqxUgMonviJuItdtIHvlc6i99SOug8nb5gpCiBwPEWUd7C1rM fsZbPg8NTjgGbZPyYluDyaBoYmRIkD3PH/CJ4CbGcR9HkyvosS4gLNHFAmkcZeZuO5fq WDCqAg9QZqWjIzpEGrUZ1xT/zTT7nIYkZr1qDr0/FFbjhTvebo5OSVGPffpbCQgP1dEy a7Qpuo+WXFeOnJI8Ni5ct1b3LAkJm7WBJxOAtf4/Yj2Ppoevuy8pASOy2ETsC0Eek9or R8iw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=60Zazpocf6dvO7NBzRzsiGBzkHiKo0Zq0HLMlrfAmUs=; fh=TDhZVPIxuB3SapFJ2cfry8UMhra0pONp3eDlzRMzdM4=; b=ygaZjb3ps5VplDCCp4diUOzEJSANEmGO933Vo/gU4GCnCoDfPRVJo2Ws/cUUgBiJBi jVppdNMR0y6wflzLP63xtVHMAXAGThjTCfg9iATi1Yrx/olTVdOFe3j16Nq5n5L5CChe DlQa9LTctNVg8fbHL8uC3Ist1qTm56gBMjKcNU72AAn7RYgCw8RLMMVMIrYAG7OP672n aumAIZVa/epM9Z1ZplDL0KeXgBjYgOs1xWWSBamLsyi5lXSUvrcp3G1jYf5YebvrEans KzHIqTC/fDKoqjeMakFl2r07HWJwAK6dcNW+ATM6TNXjz1LFcIaW0PbVCkpHr30VqObV YB+A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20221208 header.b=T9RvxzdX; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id a4-20020a1709027d8400b001b046acc851si2905841plm.358.2023.06.25.05.12.58; Sun, 25 Jun 2023 05:13:12 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20221208 header.b=T9RvxzdX; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232007AbjFYLza (ORCPT <rfc822;duw91626@gmail.com> + 99 others); Sun, 25 Jun 2023 07:55:30 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43014 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231770AbjFYLzB (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Sun, 25 Jun 2023 07:55:01 -0400 Received: from mail-lj1-x230.google.com (mail-lj1-x230.google.com [IPv6:2a00:1450:4864:20::230]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4A198E4E; Sun, 25 Jun 2023 04:54:56 -0700 (PDT) Received: by mail-lj1-x230.google.com with SMTP id 38308e7fff4ca-2b69ff54321so1270811fa.2; Sun, 25 Jun 2023 04:54:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1687694094; x=1690286094; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=60Zazpocf6dvO7NBzRzsiGBzkHiKo0Zq0HLMlrfAmUs=; b=T9RvxzdXg42/05gDV629CPCIeBkFUTVZE/ckXCF4kVClsItOTXYuj7qB5dw53vtK/+ 80Ul3yLadV+wNgPXxHucIs5zyBTyFMS4EHLS963dOF9NN+9Ni8YWN81jdkBjJmnEEo/H GlSP7obO1LG5IItwhGDpGqVxIyWFBdOw0HZNcGqzBQ88Rdw2VF75NTFJdlq68X4dCBg/ bM1TODTxseOo4WgvuFmQXNF2sh/jEFPDLGQGXhZl5ysBf74ZPLQ+RsH0m0dbdMuQMKDO h2QsemTYGUbVUYA08Gd2bqNuk9GGWp/SOzdgPM0w6/lHbr28NdAYXDvupdqYino1Ua1A ufIQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687694094; x=1690286094; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=60Zazpocf6dvO7NBzRzsiGBzkHiKo0Zq0HLMlrfAmUs=; b=f9ELhFpWLeuI7BnJvYtGMvfO4y1XnvD0hWAn6zHrhIVyRTRyfWuIEuBZevFb2JaY4Q l4mqmTcfd9l9c7RMLLMb61iZ7niP67pjwnOix8b25NkZjr+KkwIZnlpzGvwgRtT1ZIiV 2QJpRqkUV+5LscpSO7CXp1bZl005ls2/FvhSYwO+LmHO38lOjSkDGx39Zyl7paY8iMCW mT6FP+mdk2MK/G9SKwB5FnSiqO5eVIb7Du4xhXtigwVz+t2LYZxGxUXMVKjh9qj3AmEW 7TK/+CYAM56mxpABjLK2Tk37+AzzxTHSWSVcfUsP2pQxFCWxw+GmYO5xsb3tOecQzOfP Fu5g== X-Gm-Message-State: AC+VfDxVXOUseLbFvrH3hSbwWG75noOZ1lE1DlAiClYWWqeuTfI6ULTk S98e2Kv3d2NKIcbQ8nJ/Zg/HunoduIHj+A== X-Received: by 2002:a2e:9f57:0:b0:2b6:99a3:c256 with SMTP id v23-20020a2e9f57000000b002b699a3c256mr848906ljk.38.1687694094148; Sun, 25 Jun 2023 04:54:54 -0700 (PDT) Received: from WBEC325.dom.local ([185.188.71.122]) by smtp.gmail.com with ESMTPSA id w21-20020a2e9595000000b002b6993b9665sm416043ljh.65.2023.06.25.04.54.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 25 Jun 2023 04:54:53 -0700 (PDT) From: Pawel Dembicki <paweldembicki@gmail.com> To: netdev@vger.kernel.org Cc: Pawel Dembicki <paweldembicki@gmail.com>, Linus Walleij <linus.walleij@linaro.org>, Andrew Lunn <andrew@lunn.ch>, Florian Fainelli <f.fainelli@gmail.com>, Vladimir Oltean <olteanv@gmail.com>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, linux-kernel@vger.kernel.org Subject: [PATCH net-next v2 6/7] net: dsa: vsc73xx: Add vlan filtering Date: Sun, 25 Jun 2023 13:53:41 +0200 Message-Id: <20230625115343.1603330-6-paweldembicki@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230625115343.1603330-1-paweldembicki@gmail.com> References: <20230625115343.1603330-1-paweldembicki@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM, RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1769676673316088774?= X-GMAIL-MSGID: =?utf-8?q?1769676673316088774?= |
Series |
[net-next,v2,1/7] net: dsa: vsc73xx: use read_poll_timeout instead delay loop
|
|
Commit Message
Pawel Dembicki
June 25, 2023, 11:53 a.m. UTC
This patch implement vlan filtering for vsc73xx driver. After vlan filtering start, switch is reconfigured from QinQ to simple vlan aware mode. It's required, because VSC73XX chips haven't support for inner vlan tag filter. Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com> --- v2: - no changes done drivers/net/dsa/vitesse-vsc73xx-core.c | 101 +++++++++++++++++++++++++ 1 file changed, 101 insertions(+)
Comments
On Sun, Jun 25, 2023 at 01:53:41PM +0200, Pawel Dembicki wrote: > This patch implement vlan filtering for vsc73xx driver. > > After vlan filtering start, switch is reconfigured from QinQ to simple > vlan aware mode. It's required, because VSC73XX chips haven't support > for inner vlan tag filter. > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com> > --- > v2: > - no changes done > > drivers/net/dsa/vitesse-vsc73xx-core.c | 101 +++++++++++++++++++++++++ > 1 file changed, 101 insertions(+) > > diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c > index 457eb7fddf4c..c946464489ab 100644 > --- a/drivers/net/dsa/vitesse-vsc73xx-core.c > +++ b/drivers/net/dsa/vitesse-vsc73xx-core.c > @@ -1226,6 +1226,30 @@ static int vsc73xx_port_set_double_vlan_aware(struct dsa_switch *ds, int port) > return ret; > } > > +static int > +vsc73xx_port_vlan_filtering(struct dsa_switch *ds, int port, > + bool vlan_filtering, struct netlink_ext_ack *extack) > +{ > + int ret, i; > + > + if (vlan_filtering) { > + vsc73xx_port_set_vlan_conf(ds, port, VSC73XX_VLAN_AWARE); > + } else { > + if (port == CPU_PORT) > + vsc73xx_port_set_vlan_conf(ds, port, VSC73XX_DOUBLE_VLAN_CPU_AWARE); > + else > + vsc73xx_port_set_vlan_conf(ds, port, VSC73XX_DOUBLE_VLAN_AWARE); > + } Why do you need ports to be double VLAN aware when vlan_filtering=0? Isn't VLAN_TCI_IGNORE_ENA sufficient to ignore the 802.1Q header from incoming packets, and set up the PVIDs of user ports as egress-tagged on the CPU port? > + > + for (i = 0; i <= 3072; i++) { > + ret = vsc73xx_port_update_vlan_table(ds, port, i, 0); > + if (ret) > + return ret; > + } What is the purpose of this? > + > + return ret; > +} > + > static int vsc73xx_vlan_set_untagged(struct dsa_switch *ds, int port, u16 vid, > bool port_vlan) > { > @@ -1304,6 +1328,80 @@ static int vsc73xx_vlan_set_pvid(struct dsa_switch *ds, int port, u16 vid, > return 0; > } > > +static int vsc73xx_port_vlan_add(struct dsa_switch *ds, int port, > + const struct switchdev_obj_port_vlan *vlan, > + struct netlink_ext_ack *extack) > +{ > + bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED; > + bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID; > + int ret; > + > + /* Be sure to deny alterations to the configuration done by tag_8021q. > + */ > + if (vid_is_dsa_8021q(vlan->vid)) { > + NL_SET_ERR_MSG_MOD(extack, > + "Range 3072-4095 reserved for dsa_8021q operation"); > + return -EBUSY; > + } > + > + if (untagged && port != CPU_PORT) { > + ret = vsc73xx_vlan_set_untagged(ds, port, vlan->vid, true); > + if (ret) > + return ret; > + } > + if (pvid && port != CPU_PORT) { Missing logic to change hardware PVID only while VLAN-aware, and to restore the tag_8021q PVID when the bridge VLAN awareness gets disabled. DSA does not resolve the conflicts on resources between .port_vlan_add() and .tag_8021q_vlan_add(), the driver must do that. > + ret = vsc73xx_vlan_set_pvid(ds, port, vlan->vid, true); > + if (ret) > + return ret; > + } > + > + ret = vsc73xx_port_update_vlan_table(ds, port, vlan->vid, 1); > + > + return ret; Style: return vsc73xx_port_update_vlan_table(...) > +} > + > +static int vsc73xx_port_vlan_del(struct dsa_switch *ds, int port, > + const struct switchdev_obj_port_vlan *vlan) > +{ > + struct vsc73xx *vsc = ds->priv; > + u16 vlan_no; > + int ret; > + u32 val; > + > + ret = > + vsc73xx_port_update_vlan_table(ds, port, vlan->vid, 0); Style: single line > + if (ret) > + return ret; > + > + vsc73xx_read(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_TXUPDCFG, &val); > + > + if (val & VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA) { > + vsc73xx_read(vsc, VSC73XX_BLOCK_MAC, port, > + VSC73XX_TXUPDCFG, &val); > + vlan_no = (val & VSC73XX_TXUPDCFG_TX_UNTAGGED_VID) >> > + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_SHIFT; > + if (vlan_no == vlan->vid) { > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > + VSC73XX_TXUPDCFG, > + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA, > + 0); > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > + VSC73XX_TXUPDCFG, > + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID, 0); > + } > + } > + > + vsc73xx_read(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_PORT_VLAN, &val); > + vlan_no = val & VSC73XX_CAT_PORT_VLAN_VLAN_VID; > + if (vlan_no && vlan_no == vlan->vid) { > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > + VSC73XX_CAT_PORT_VLAN, > + VSC73XX_CAT_PORT_VLAN_VLAN_VID, 0); As documented in Documentation/networking/switchdev.rst: When the bridge has VLAN filtering enabled and a PVID is not configured on the ingress port, untagged and 802.1p tagged packets must be dropped. When the bridge has VLAN filtering enabled and a PVID exists on the ingress port, untagged and priority-tagged packets must be accepted and forwarded according to the bridge's port membership of the PVID VLAN. When the bridge has VLAN filtering disabled, the presence/lack of a PVID should not influence the packet forwarding decision. Setting the hardware PVID to 0 when the bridge PVID is deleted sounds like it accomplishes none of those. > + } > + > + return 0; > +} > + > static void vsc73xx_update_forwarding_map(struct vsc73xx *vsc) > { > int i; > @@ -1524,6 +1622,9 @@ static const struct dsa_switch_ops vsc73xx_ds_ops = { > .port_change_mtu = vsc73xx_change_mtu, > .port_max_mtu = vsc73xx_get_max_mtu, > .port_stp_state_set = vsc73xx_port_stp_state_set, > + .port_vlan_filtering = vsc73xx_port_vlan_filtering, > + .port_vlan_add = vsc73xx_port_vlan_add, > + .port_vlan_del = vsc73xx_port_vlan_del, > .tag_8021q_vlan_add = vsc73xx_tag_8021q_vlan_add, > .tag_8021q_vlan_del = vsc73xx_tag_8021q_vlan_del, > }; > -- > 2.34.1 >
niedz., 25 cze 2023 o 17:05 Vladimir Oltean <olteanv@gmail.com> napisał(a): > > On Sun, Jun 25, 2023 at 01:53:41PM +0200, Pawel Dembicki wrote: > > This patch implement vlan filtering for vsc73xx driver. > > > > After vlan filtering start, switch is reconfigured from QinQ to simple > > vlan aware mode. It's required, because VSC73XX chips haven't support > > for inner vlan tag filter. > > > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com> > > --- > > v2: > > - no changes done > > > > drivers/net/dsa/vitesse-vsc73xx-core.c | 101 +++++++++++++++++++++++++ > > 1 file changed, 101 insertions(+) > > > > diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c > > index 457eb7fddf4c..c946464489ab 100644 > > --- a/drivers/net/dsa/vitesse-vsc73xx-core.c > > +++ b/drivers/net/dsa/vitesse-vsc73xx-core.c > > @@ -1226,6 +1226,30 @@ static int vsc73xx_port_set_double_vlan_aware(struct dsa_switch *ds, int port) > > return ret; > > } > > > > +static int > > +vsc73xx_port_vlan_filtering(struct dsa_switch *ds, int port, > > + bool vlan_filtering, struct netlink_ext_ack *extack) > > +{ > > + int ret, i; > > + > > + if (vlan_filtering) { > > + vsc73xx_port_set_vlan_conf(ds, port, VSC73XX_VLAN_AWARE); > > + } else { > > + if (port == CPU_PORT) > > + vsc73xx_port_set_vlan_conf(ds, port, VSC73XX_DOUBLE_VLAN_CPU_AWARE); > > + else > > + vsc73xx_port_set_vlan_conf(ds, port, VSC73XX_DOUBLE_VLAN_AWARE); > > + } > > Why do you need ports to be double VLAN aware when vlan_filtering=0? > Isn't VLAN_TCI_IGNORE_ENA sufficient to ignore the 802.1Q header from > incoming packets, and set up the PVIDs of user ports as egress-tagged on > the CPU port? > Because I want to forward tagged and untagged frames when vlan_filtering is off. If I set VSC73XX_DOUBLE_VLAN_AWARE, I can put all (tagged and untagged) traffic into the outer vlan, called by the datasheet as "MAN space". In QinQ mode, it is possible to ignore what goes from a particular port but it is possible to separate traffic from different ports. > > + > > + for (i = 0; i <= 3072; i++) { > > + ret = vsc73xx_port_update_vlan_table(ds, port, i, 0); > > + if (ret) > > + return ret; > > + } > > What is the purpose of this? I want to be sure that the table is cleared when vlan awareness is changed. > > > + > > + return ret; > > +} > > + > > static int vsc73xx_vlan_set_untagged(struct dsa_switch *ds, int port, u16 vid, > > bool port_vlan) > > { > > @@ -1304,6 +1328,80 @@ static int vsc73xx_vlan_set_pvid(struct dsa_switch *ds, int port, u16 vid, > > return 0; > > } > > > > +static int vsc73xx_port_vlan_add(struct dsa_switch *ds, int port, > > + const struct switchdev_obj_port_vlan *vlan, > > + struct netlink_ext_ack *extack) > > +{ > > + bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED; > > + bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID; > > + int ret; > > + > > + /* Be sure to deny alterations to the configuration done by tag_8021q. > > + */ > > + if (vid_is_dsa_8021q(vlan->vid)) { > > + NL_SET_ERR_MSG_MOD(extack, > > + "Range 3072-4095 reserved for dsa_8021q operation"); > > + return -EBUSY; > > + } > > + > > + if (untagged && port != CPU_PORT) { > > + ret = vsc73xx_vlan_set_untagged(ds, port, vlan->vid, true); > > + if (ret) > > + return ret; > > + } > > + if (pvid && port != CPU_PORT) { > > Missing logic to change hardware PVID only while VLAN-aware, and to > restore the tag_8021q PVID when the bridge VLAN awareness gets disabled. > DSA does not resolve the conflicts on resources between .port_vlan_add() > and .tag_8021q_vlan_add(), the driver must do that. > > > + ret = vsc73xx_vlan_set_pvid(ds, port, vlan->vid, true); > > + if (ret) > > + return ret; > > + } > > + > > + ret = vsc73xx_port_update_vlan_table(ds, port, vlan->vid, 1); > > + > > + return ret; > > Style: return vsc73xx_port_update_vlan_table(...) > > > +} > > + > > +static int vsc73xx_port_vlan_del(struct dsa_switch *ds, int port, > > + const struct switchdev_obj_port_vlan *vlan) > > +{ > > + struct vsc73xx *vsc = ds->priv; > > + u16 vlan_no; > > + int ret; > > + u32 val; > > + > > + ret = > > + vsc73xx_port_update_vlan_table(ds, port, vlan->vid, 0); > > Style: single line > > > + if (ret) > > + return ret; > > + > > + vsc73xx_read(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_TXUPDCFG, &val); > > + > > + if (val & VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA) { > > + vsc73xx_read(vsc, VSC73XX_BLOCK_MAC, port, > > + VSC73XX_TXUPDCFG, &val); > > + vlan_no = (val & VSC73XX_TXUPDCFG_TX_UNTAGGED_VID) >> > > + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_SHIFT; > > + if (vlan_no == vlan->vid) { > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > > + VSC73XX_TXUPDCFG, > > + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA, > > + 0); > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > > + VSC73XX_TXUPDCFG, > > + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID, 0); > > + } > > + } > > + > > + vsc73xx_read(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_PORT_VLAN, &val); > > + vlan_no = val & VSC73XX_CAT_PORT_VLAN_VLAN_VID; > > + if (vlan_no && vlan_no == vlan->vid) { > > + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, > > + VSC73XX_CAT_PORT_VLAN, > > + VSC73XX_CAT_PORT_VLAN_VLAN_VID, 0); > > As documented in Documentation/networking/switchdev.rst: > > When the bridge has VLAN filtering enabled and a PVID is not configured on the > ingress port, untagged and 802.1p tagged packets must be dropped. When the bridge > has VLAN filtering enabled and a PVID exists on the ingress port, untagged and > priority-tagged packets must be accepted and forwarded according to the > bridge's port membership of the PVID VLAN. When the bridge has VLAN filtering > disabled, the presence/lack of a PVID should not influence the packet > forwarding decision. > > Setting the hardware PVID to 0 when the bridge PVID is deleted sounds > like it accomplishes none of those. > My bad. I should just set VSC73XX_CAT_DROP_UNTAGGED_ENA here. > > + } > > + > > + return 0; > > +} > > + > > static void vsc73xx_update_forwarding_map(struct vsc73xx *vsc) > > { > > int i; > > @@ -1524,6 +1622,9 @@ static const struct dsa_switch_ops vsc73xx_ds_ops = { > > .port_change_mtu = vsc73xx_change_mtu, > > .port_max_mtu = vsc73xx_get_max_mtu, > > .port_stp_state_set = vsc73xx_port_stp_state_set, > > + .port_vlan_filtering = vsc73xx_port_vlan_filtering, > > + .port_vlan_add = vsc73xx_port_vlan_add, > > + .port_vlan_del = vsc73xx_port_vlan_del, > > .tag_8021q_vlan_add = vsc73xx_tag_8021q_vlan_add, > > .tag_8021q_vlan_del = vsc73xx_tag_8021q_vlan_del, > > }; > > -- > > 2.34.1 > > > Thank you for such detailed responses and clarifying for me many issues. -- Pawel Dembicki
On Thu, Jun 29, 2023 at 10:18:08PM +0200, Paweł Dembicki wrote: > niedz., 25 cze 2023 o 17:05 Vladimir Oltean <olteanv@gmail.com> napisał(a): > > Why do you need ports to be double VLAN aware when vlan_filtering=0? > > Isn't VLAN_TCI_IGNORE_ENA sufficient to ignore the 802.1Q header from > > incoming packets, and set up the PVIDs of user ports as egress-tagged on > > the CPU port? > > Because I want to forward tagged and untagged frames when > vlan_filtering is off. If I set VSC73XX_DOUBLE_VLAN_AWARE, I can put > all (tagged and untagged) traffic into the outer vlan, called by the > datasheet as "MAN space". In QinQ mode, it is possible to ignore what > goes from a particular port but it is possible to separate traffic > from different ports. I think we may have some problem in finding common terminology. Opening the manual and seeing the table "Customer Port Sample Configuration", I think it's indeed what you need. But I wouldn't call it "double VLAN aware". The port is actually configured to be VLAN *unaware* from the perspective of classification, and always encapsulate all packets in one more VLAN (the port PVID). This switch's analyzer is always aware only of the outer VLAN header, and that's not "double VLAN aware" (it can perform no action based on the inner VLAN, if that exists), but it's fine for what is needed of it. You might be mixing these with MAC_CFG::VLAN_AWR and MAC_CFG::VLAN_DBLAWR, which essentially are only there to allow single- and double-VLAN-tagged frames to be longer by 4 and 8 bytes, respectively, than the max frame size. I don't think that these 2 fields have any reason to depend upon the bridge VLAN awareness state of the port. They can be unconditionally enabled. After all, Linux only cares about MTU, and that is the size of the L2 payload, excluding any VLAN headers, if present. I would suggest that if you exclude the MAC_CFG registers from vsc73xx_port_set_vlan_conf(), you end up with not as many VLAN awareness modes as you think. 2, to be precise: on or off. So you don't need the enum. Also, AFAIU, I don't see a reason to modify CAT_VLAN_MISC::VLAN_KEEP_TAG_ENA from the value of 1 at all. You could always keep frames in the queue system with the VID attached, and strip that VID on egress, if necessary, via TXUPDCFG. Not sure if you're noticed this, but drivers/net/ethernet/mscc/ and drivers/net/dsa/ocelot/ contain a driver for a newer generation of hardware than the VSC73xx, but many of the concepts apply. Maybe you can take a look at how some things were done there. > > > + > > > + for (i = 0; i <= 3072; i++) { > > > + ret = vsc73xx_port_update_vlan_table(ds, port, i, 0); > > > + if (ret) > > > + return ret; > > > + } > > > > What is the purpose of this? > > I want to be sure that the table is cleared when vlan awareness is changed. Yes, but why? That should specifically not be done, since there is no code in the kernel to replay the port_vlan_add() and tag_8021q_vlan_add() calls for you when the VLAN awareness state changes. If you delete the VLANs, they're gone, even though in software they're still there.
diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c index 457eb7fddf4c..c946464489ab 100644 --- a/drivers/net/dsa/vitesse-vsc73xx-core.c +++ b/drivers/net/dsa/vitesse-vsc73xx-core.c @@ -1226,6 +1226,30 @@ static int vsc73xx_port_set_double_vlan_aware(struct dsa_switch *ds, int port) return ret; } +static int +vsc73xx_port_vlan_filtering(struct dsa_switch *ds, int port, + bool vlan_filtering, struct netlink_ext_ack *extack) +{ + int ret, i; + + if (vlan_filtering) { + vsc73xx_port_set_vlan_conf(ds, port, VSC73XX_VLAN_AWARE); + } else { + if (port == CPU_PORT) + vsc73xx_port_set_vlan_conf(ds, port, VSC73XX_DOUBLE_VLAN_CPU_AWARE); + else + vsc73xx_port_set_vlan_conf(ds, port, VSC73XX_DOUBLE_VLAN_AWARE); + } + + for (i = 0; i <= 3072; i++) { + ret = vsc73xx_port_update_vlan_table(ds, port, i, 0); + if (ret) + return ret; + } + + return ret; +} + static int vsc73xx_vlan_set_untagged(struct dsa_switch *ds, int port, u16 vid, bool port_vlan) { @@ -1304,6 +1328,80 @@ static int vsc73xx_vlan_set_pvid(struct dsa_switch *ds, int port, u16 vid, return 0; } +static int vsc73xx_port_vlan_add(struct dsa_switch *ds, int port, + const struct switchdev_obj_port_vlan *vlan, + struct netlink_ext_ack *extack) +{ + bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED; + bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID; + int ret; + + /* Be sure to deny alterations to the configuration done by tag_8021q. + */ + if (vid_is_dsa_8021q(vlan->vid)) { + NL_SET_ERR_MSG_MOD(extack, + "Range 3072-4095 reserved for dsa_8021q operation"); + return -EBUSY; + } + + if (untagged && port != CPU_PORT) { + ret = vsc73xx_vlan_set_untagged(ds, port, vlan->vid, true); + if (ret) + return ret; + } + if (pvid && port != CPU_PORT) { + ret = vsc73xx_vlan_set_pvid(ds, port, vlan->vid, true); + if (ret) + return ret; + } + + ret = vsc73xx_port_update_vlan_table(ds, port, vlan->vid, 1); + + return ret; +} + +static int vsc73xx_port_vlan_del(struct dsa_switch *ds, int port, + const struct switchdev_obj_port_vlan *vlan) +{ + struct vsc73xx *vsc = ds->priv; + u16 vlan_no; + int ret; + u32 val; + + ret = + vsc73xx_port_update_vlan_table(ds, port, vlan->vid, 0); + if (ret) + return ret; + + vsc73xx_read(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_TXUPDCFG, &val); + + if (val & VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA) { + vsc73xx_read(vsc, VSC73XX_BLOCK_MAC, port, + VSC73XX_TXUPDCFG, &val); + vlan_no = (val & VSC73XX_TXUPDCFG_TX_UNTAGGED_VID) >> + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_SHIFT; + if (vlan_no == vlan->vid) { + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, + VSC73XX_TXUPDCFG, + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID_ENA, + 0); + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, + VSC73XX_TXUPDCFG, + VSC73XX_TXUPDCFG_TX_UNTAGGED_VID, 0); + } + } + + vsc73xx_read(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_CAT_PORT_VLAN, &val); + vlan_no = val & VSC73XX_CAT_PORT_VLAN_VLAN_VID; + if (vlan_no && vlan_no == vlan->vid) { + vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port, + VSC73XX_CAT_PORT_VLAN, + VSC73XX_CAT_PORT_VLAN_VLAN_VID, 0); + } + + return 0; +} + static void vsc73xx_update_forwarding_map(struct vsc73xx *vsc) { int i; @@ -1524,6 +1622,9 @@ static const struct dsa_switch_ops vsc73xx_ds_ops = { .port_change_mtu = vsc73xx_change_mtu, .port_max_mtu = vsc73xx_get_max_mtu, .port_stp_state_set = vsc73xx_port_stp_state_set, + .port_vlan_filtering = vsc73xx_port_vlan_filtering, + .port_vlan_add = vsc73xx_port_vlan_add, + .port_vlan_del = vsc73xx_port_vlan_del, .tag_8021q_vlan_add = vsc73xx_tag_8021q_vlan_add, .tag_8021q_vlan_del = vsc73xx_tag_8021q_vlan_del, };