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;
Date:   Sun, 25 Jun 2023 14:42:27 +0200
From:   Simon Horman <simon.horman@corigine.com>
To:     Pawel Dembicki <paweldembicki@gmail.com>
Cc:     netdev@vger.kernel.org, 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,
        Dan Carpenter <dan.carpenter@linaro.org>
Subject: Re: [PATCH net-next v2 4/7] net: dsa: vsc73xx: Add dsa tagging based
 on 8021q
Message-ID: <ZJg2M+Qvg3Fv73CH@corigine.com>
References: <20230625115343.1603330-1-paweldembicki@gmail.com>
 <20230625115343.1603330-4-paweldembicki@gmail.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230625115343.1603330-4-paweldembicki@gmail.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?U+jAQ5J318OpA2PfFJzxRvftnuFzz6Xu4GOw4Acw5bmuvnA+qAtyIimmXhqP?=
 =?us-ascii?Q?iF1/vBm02O/sGCmaSuZPbDWqS1I8j+JOYVv/dBxpMkfgBDFBetmrSRgajazG?=
 =?us-ascii?Q?2i6klgVshtugXpDT0cNQxB4drtZ55MlnYn4bgK8kNuxraOrO41X/563mKb2m?=
 =?us-ascii?Q?iSq96fy14r7z7PUmyO/G0whdA8LJCZb9yTOpoEULfjKNaW8WGe6GalilRKem?=
 =?us-ascii?Q?Yuwi1777q0Ew5YRHL+dbw9jDloYqC50/kXu64IQWnFbgJ4vsDNhSyW9PINeS?=
 =?us-ascii?Q?5rpVOVB99+hX5QxX/56Ii6mV3jB9wazjSaTXKYkqgc07FkO70uzdi6Cpd6Nt?=
 =?us-ascii?Q?HovnDLehQ1WJRnpbQNuNmmOK5XCc/4vtr5eooxOn7wrivZ/Fl6HrxvdlJ0jj?=
 =?us-ascii?Q?IOl1De5actxNGuWDVcbHhwaoVNNs+lQcaQqOPIOGuXZmaMiNDkZs2yvPnATT?=
 =?us-ascii?Q?AxL0M3QOhTNF/WgMt3T/HDJUhWK5UfSd4tlMrurt7OCz88V17Bn7Hu3TWhAS?=
 =?us-ascii?Q?CqQPUwMR/Yt/SylYgMt2LQdyuZfIf5iPxeQb1wHapZdErYJ2uW9kvBeh64cS?=
 =?us-ascii?Q?x3XDjjnJh5ADeSR+y2rpINirXD7N0ArEBMoXjhBNXaTVCDyX69b0KbfPOlGe?=
 =?us-ascii?Q?dddX2QooQDk87stcsRWLUQfZ01W+COOJRHon8CgK4ma0F7UUkrvFVNJXDR8c?=
 =?us-ascii?Q?29FR9q/5L4bneqdUhpwLDx/znC2lioZ8wjQTuELAOVLyiMycGcKm1U7VnG0O?=
 =?us-ascii?Q?ttK410rNkKpRvqfdfJD6U8qB8ggve+gNoK1tqaqPNCjDadbr2R28GwB69wCx?=
 =?us-ascii?Q?8sZXXzfs54XKfjfy5Ugh6uGGU/eCNdg7HW+iBOqDxF2n0KTcT61JALpXo6Az?=
 =?us-ascii?Q?8PgzQmsCQHTI/YqAnTNTRsAoKoThEswn+JzLSiQJpGo5Lu5WoCeheTJ3zLU9?=
 =?us-ascii?Q?LhxcrZlUkQ7E25nNdsEpdLIy8IXActnOlzpC8TVH2FqMHpx7SXPABkaKXCuI?=
 =?us-ascii?Q?fLiNHDy5Vgtkgki52rgbqNOcjXDhcqTylLLJ16q/5RLkyGgoZVqiZjwxhog+?=
 =?us-ascii?Q?i3i+sBHOQHUVR+J1UPhRRmkzzUZf4EGGncRb+h9vqXXnFwbfS0ttiK9Iysko?=
 =?us-ascii?Q?1NmV9Rk91vMpnt1haSF2TcKTf/1zmP3QQfaIzKkNIevBcIjKzDAx75zIP4uQ?=
 =?us-ascii?Q?NQbiEOBN84cI+m7B7cO4zq0MEXtV3fGqNlw3lxyd6vh96AMhwBwXEN4vr6Rk?=
 =?us-ascii?Q?vBAFgupbCbU2qVUDHJR+Ct7HPVzDZukFtnh/daO73kSGdkXjnc/XcesAIBhb?=
 =?us-ascii?Q?hnIA+sHaWpy3o1wdIch3TqgYb8MTSm8Or7Us6NJ2H2bEnvb7Y50gwnFrtrRd?=
 =?us-ascii?Q?33eZP4d+msYYYylEGmjNjhey8mv9IriNQQj6G3aTpWZft2nIPv48p8gZbijN?=
 =?us-ascii?Q?mE6jzeN1tyxPTrRdoV2kntbNi5Dp+x+XcjMbUZHSuemAbDe74uIHZjNRVET9?=
 =?us-ascii?Q?dyDdsZmG3EYSm+DW0OR2XKuHyaxpO9SxBMm5131J2vjNCU4eSKFNY7TwS8ce?=
 =?us-ascii?Q?yZGXVlqdZqpxKhdq3/oRL6nahuXc7MJiPm/kun+FwC5jIO8K8I4N3ntAiZXt?=
 =?us-ascii?Q?VO6ItWBrm6OCRJOTOgJiB+Qxjn32M4nCPERGRVPpfUG1GWgk7blzxuvXHiYX?=
 =?us-ascii?Q?a6K3GA=3D=3D?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4d29f0b8-428e-401b-f9c5-08db7579a5f6
X-MS-Exchange-CrossTenant-AuthSource: PH0PR13MB4842.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Jun 2023 12:42:34.0703
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: fe128f2c-073b-4c20-818e-7246a585940c
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: H4MLqiqPAUUWd5ZQ/nNm3VW4CsiKZlQ0e9YcciqfMhKWFMtyujyYIothzfIGhPJ2zmDWWLCF5HbEoKYEKKIz5YM8Td72ShlLeXUR8QsOJNs=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN0PR13MB5200
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,SPF_HELO_PASS,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

+ Dan Carpenter

On Sun, Jun 25, 2023 at 01:53:39PM +0200, Pawel Dembicki wrote:
> This patch is simple implementation of 8021q tagging in vsc73xx driver.
> At this moment devices with DSA_TAG_PROTO_NONE are useless. VSC73XX
> family doesn't provide any tag support for external ethernet ports.
> 
> The only way is vlan-based tagging. It require constant hardware vlan
> filtering. VSC73XX family support provider bridging but QinQ only without
> fully implemented 802.1AD. It allow only doubled 0x8100 TPID.
> 
> In simple port mode QinQ is enabled to preserve forwarding vlan tagged
> frames.
> 
> Tag driver introduce most simple funcionality required for proper taging
> support.
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>

...

> +static void vsc73xx_vlan_rcv(struct sk_buff *skb, int *source_port,
> +			     int *switch_id, int *vbid, u16 *vid)
> +{
> +	if (vid_is_dsa_8021q(skb_vlan_tag_get(skb) & VLAN_VID_MASK))
> +		return dsa_8021q_rcv(skb, source_port, switch_id, vbid);
> +
> +	/* Try our best with imprecise RX */
> +	*vid = skb_vlan_tag_get(skb) & VLAN_VID_MASK;
> +}
> +
> +static struct sk_buff *vsc73xx_rcv(struct sk_buff *skb,
> +				   struct net_device *netdev)
> +{
> +	int src_port = -1, switch_id = -1, vbid = -1;
> +	u16 vid;
> +
> +	if (skb_vlan_tag_present(skb))
> +		/* Normal traffic path. */
> +		vsc73xx_vlan_rcv(skb, &src_port, &switch_id, &vbid, &vid);
> +
> +	if (vbid >= 1)
> +		skb->dev = dsa_tag_8021q_find_port_by_vbid(netdev, vbid);
> +	else if (src_port == -1 || switch_id == -1)
> +		skb->dev = dsa_find_designated_bridge_port_by_vid(netdev, vid);

Hi Pawel,

Smatch warns that vid may be used uninitialised here.
And it's not clear to me why that cannot be the case.

> +	else
> +		skb->dev = dsa_master_find_slave(netdev, switch_id, src_port);
> +	if (!skb->dev) {
> +		netdev_warn(netdev, "Couldn't decode source port\n");
> +		return NULL;
> +	}
> +
> +	dsa_default_offload_fwd_mark(skb);
> +
> +	if (dsa_port_is_vlan_filtering(dsa_slave_to_port(skb->dev)) &&
> +	    eth_hdr(skb)->h_proto == htons(ETH_P_8021Q))
> +		__vlan_hwaccel_clear_tag(skb);
> +
> +	return skb;
> +}

...