From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3BFECC433E2 for ; Tue, 8 Sep 2020 16:55:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id EDF542075A for ; Tue, 8 Sep 2020 16:55:29 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=ti.com header.i=@ti.com header.b="K2KpKLMn" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732039AbgIHQz1 (ORCPT ); Tue, 8 Sep 2020 12:55:27 -0400 Received: from fllv0016.ext.ti.com ([198.47.19.142]:58930 "EHLO fllv0016.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731816AbgIHQzM (ORCPT ); Tue, 8 Sep 2020 12:55:12 -0400 Received: from lelv0266.itg.ti.com ([10.180.67.225]) by fllv0016.ext.ti.com (8.15.2/8.15.2) with ESMTP id 088Gt7MS086519; Tue, 8 Sep 2020 11:55:07 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ti.com; s=ti-com-17Q1; t=1599584107; bh=nlL2npsKlyOY3ft+6UJrgJZRAj6uLlrQ2DMNVsNBne0=; h=Subject:To:CC:References:From:Date:In-Reply-To; b=K2KpKLMn/djBX+ja+slW0iNsT4quD82h6ynB8M9SLsahqXM1ZGHh6Go8RA/eXWUUU q2c1JcVFReazFZWWyMvEu6lE4zAR3JeXayEzExR98sE5X3Hx27c/jKvYvjLIlpHMM4 Hy5KD70FjlcUCJwIV3FjobtV9oUgYDSPzIDlrIZ0= Received: from DFLE113.ent.ti.com (dfle113.ent.ti.com [10.64.6.34]) by lelv0266.itg.ti.com (8.15.2/8.15.2) with ESMTPS id 088Gt7WW022793 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=FAIL); Tue, 8 Sep 2020 11:55:07 -0500 Received: from DFLE103.ent.ti.com (10.64.6.24) by DFLE113.ent.ti.com (10.64.6.34) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1979.3; Tue, 8 Sep 2020 11:55:07 -0500 Received: from lelv0327.itg.ti.com (10.180.67.183) by DFLE103.ent.ti.com (10.64.6.24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1979.3 via Frontend Transport; Tue, 8 Sep 2020 11:55:07 -0500 Received: from [10.250.53.226] (ileax41-snat.itg.ti.com [10.172.224.153]) by lelv0327.itg.ti.com (8.15.2/8.15.2) with ESMTP id 088Gt7qp024121; Tue, 8 Sep 2020 11:55:07 -0500 Subject: Re: [PATCH net-next 0/1] Support for VLAN interface over HSR/PRP To: Willem de Bruijn CC: David Miller , Jakub Kicinski , Network Development , linux-kernel , , Grygorii Strashko References: <20200901195415.4840-1-m-karicheri2@ti.com> <15bbf7d2-627b-1d52-f130-5bae7b7889de@ti.com> From: Murali Karicheri Message-ID: Date: Tue, 8 Sep 2020 12:55:01 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Hi Willem, On 9/4/20 11:52 AM, Willem de Bruijn wrote: > On Thu, Sep 3, 2020 at 12:30 AM Murali Karicheri wrote: >> >> All, >> >> On 9/2/20 12:14 PM, Murali Karicheri wrote: >>> All, >>> >>> On 9/1/20 3:54 PM, Murali Karicheri wrote: >>>> This series add support for creating VLAN interface over HSR or >>>> PRP interface. Typically industrial networks uses VLAN in >>>> deployment and this capability is needed to support these >>>> networks. >>>> >>>> This is tested using two TI AM572x IDK boards connected back >>>> to back over CPSW ports (eth0 and eth1). >>>> >>>> Following is the setup >>>> >>>> Physical Setup >>>> ++++++++++++++ >>>> _______________ (CPSW) _______________ >>>> | |----eth0-----| | >>>> |TI AM572x IDK1| | TI AM572x IDK2| >>>> |______________|----eth1-----|_______________| >>>> >>>> >>>> Network Topolgy >>>> +++++++++++++++ >>>> >>>> TI AM571x IDK TI AM572x IDK >>>> >>>> 192.168.100.10 CPSW ports 192.168.100.20 >>>> IDK-1 IDK-2 >>>> hsr0/prp0.100--| 192.168.2.10 |--eth0--| 192.168.2.20 |--hsr0/prp0.100 >>>> |----hsr0/prp0--| |---hsr0/prp0--| >>>> hsr0/prp0.101--| |--eth1--| |--hsr0/prp0/101 >>>> >>>> 192.168.101.10 192.168.101.20 >>>> >>>> Following tests:- >>>> - create hsr or prp interface and ping the interface IP address >>>> and verify ping is successful. >>>> - Create 2 VLANs over hsr or prp interface on both IDKs (VID 100 and >>>> 101). Ping between the IP address of the VLAN interfaces >>>> - Do iperf UDP traffic test with server on one IDK and client on the >>>> other. Do this using 100 and 101 subnet IP addresses >>>> - Dump /proc/net/vlan/{hsr|prp}0.100 and verify frames are transmitted >>>> and received at these interfaces. >>>> - Delete the vlan and hsr/prp interface and verify interfaces are >>>> removed cleanly. >>>> >>>> Logs for IDK-1 at https://pastebin.ubuntu.com/p/NxF83yZFDX/ >>>> Logs for IDK-2 at https://pastebin.ubuntu.com/p/YBXBcsPgVK/ >>>> >>>> Murali Karicheri (1): >>>> net: hsr/prp: add vlan support >>>> >>>> net/hsr/hsr_device.c | 4 ---- >>>> net/hsr/hsr_forward.c | 16 +++++++++++++--- >>>> 2 files changed, 13 insertions(+), 7 deletions(-) >>>> >>> I am not sure if the packet flow is right for this? >>> >>> VLAN over HSR frame format is like this. >>> >>> >>> >>> My ifconfig stats shows both hsr and hsr0.100 interfaces receiving >>> frames. >>> >>> So I did a WARN_ON() in HSR driver before frame is forwarded to upper >>> layer. >>> >>> a0868495local@uda0868495:~/Projects/upstream-kernel$ git diff >>> diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c >>> index de21df30b0d9..545a3cd8c71b 100644 >>> --- a/net/hsr/hsr_forward.c >>> +++ b/net/hsr/hsr_forward.c >>> @@ -415,9 +415,11 @@ static void hsr_forward_do(struct hsr_frame_info >>> *frame) >>> } >>> >>> skb->dev = port->dev; >>> - if (port->type == HSR_PT_MASTER) >>> + if (port->type == HSR_PT_MASTER) { >>> + if (skb_vlan_tag_present(skb)) >>> + WARN_ON(1); >>> hsr_deliver_master(skb, port->dev, >>> frame->node_src); >>> - else >>> + } else >>> hsr_xmit(skb, port, frame); >>> } >>> } >>> >>> And I get the trace shown below. >>> >>> [ 275.125431] WARNING: CPU: 0 PID: 0 at net/hsr/hsr_forward.c:420 >>> hsr_forward_skb+0x460/0x564 >>> [ 275.133822] Modules linked in: snd_soc_omap_hdmi snd_soc_ti_sdma >>> snd_soc_core snd_pcm_dmaengine snd_pcm snd_time4 >>> [ 275.199705] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W >>> 5.9.0-rc1-00658-g473e463812c2-dirty #8 >>> [ 275.209573] Hardware name: Generic DRA74X (Flattened Device Tree) >>> [ 275.215703] [] (unwind_backtrace) from [] >>> (show_stack+0x10/0x14) >>> [ 275.223487] [] (show_stack) from [] >>> (dump_stack+0xc4/0xe4) >>> [ 275.230747] [] (dump_stack) from [] >>> (__warn+0xc0/0xf4) >>> [ 275.237656] [] (__warn) from [] >>> (warn_slowpath_fmt+0x58/0xb8) >>> [ 275.245177] [] (warn_slowpath_fmt) from [] >>> (hsr_forward_skb+0x460/0x564) >>> [ 275.253657] [] (hsr_forward_skb) from [] >>> (hsr_handle_frame+0x15c/0x190) >>> [ 275.262047] [] (hsr_handle_frame) from [] >>> (__netif_receive_skb_core+0x23c/0xc88) >>> [ 275.271223] [] (__netif_receive_skb_core) from [] >>> (__netif_receive_skb_one_core+0x30/0x74) >>> [ 275.281266] [] (__netif_receive_skb_one_core) from >>> [] (netif_receive_skb+0x50/0x1c4) >>> [ 275.290793] [] (netif_receive_skb) from [] >>> (cpsw_rx_handler+0x230/0x308) >>> [ 275.299272] [] (cpsw_rx_handler) from [] >>> (__cpdma_chan_process+0xf4/0x188) >>> [ 275.307925] [] (__cpdma_chan_process) from [] >>> (cpdma_chan_process+0x3c/0x5c) >>> [ 275.316754] [] (cpdma_chan_process) from [] >>> (cpsw_rx_mq_poll+0x44/0x98) >>> [ 275.325145] [] (cpsw_rx_mq_poll) from [] >>> (net_rx_action+0xf0/0x400) >>> [ 275.333185] [] (net_rx_action) from [] >>> (__do_softirq+0xf0/0x3ac) >>> [ 275.340965] [] (__do_softirq) from [] >>> (irq_exit+0xa8/0xe4) >>> [ 275.348224] [] (irq_exit) from [] >>> (__handle_domain_irq+0x6c/0xe0) >>> [ 275.356093] [] (__handle_domain_irq) from [] >>> (gic_handle_irq+0x4c/0xa8) >>> [ 275.364481] [] (gic_handle_irq) from [] >>> (__irq_svc+0x6c/0x90) >>> [ 275.371996] Exception stack(0xc0e01f18 to 0xc0e01f60) >>> >>> Shouldn't it show vlan_do_receive() ? >>> >>> if (skb_vlan_tag_present(skb)) { >>> if (pt_prev) { >>> ret = deliver_skb(skb, pt_prev, orig_dev); >>> pt_prev = NULL; >>> } >>> if (vlan_do_receive(&skb)) >>> goto another_round; >>> else if (unlikely(!skb)) >>> goto out; >>> } >>> >>> Thanks >>> >> >> I did an ftrace today and I find vlan_do_receive() is called for the >> incoming frames before passing SKB to hsr_handle_frame(). If someone >> can review this, it will help. Thanks. >> >> https://pastebin.ubuntu.com/p/CbRzXjwjR5/ > > hsr_handle_frame is an rx_handler called after > __netif_receive_skb_core called vlan_do_receive and jumped back to > another_round. Yes. hsr_handle_frame() is a rx_handler() after the above code that does vlan_do_receive(). The ftrace shows vlan_do_receive() is called followed by call to hsr_handle_frame(). From ifconfig I can see both hsr and vlan interface stats increments by same count. So I assume, vlan_do_receive() is called initially and it removes the tag, update stats and then return true and go for another round. Do you think that is the case? vlan_do_receive() calls vlan_find_dev(skb->dev, vlan_proto, vlan_id) to retrieve the real netdevice (real device). However VLAN device is attached to hsr device (real device), but SKB will have HSR slave Ethernet netdevice (in our case it is cpsw device) and vlan_find_dev() would have failed since there is no vlan_info in cpsw netdev struct. So below code in vlan_do_receive() should have failed and return false. vlan_dev = vlan_find_dev(skb->dev, vlan_proto, vlan_id); if (!vlan_dev) return false; So how does it goes for another_round ? May be vlan_find_dev is finding the hsr netdevice? I am not an expert and so the question. Probably I can put a traceprintk() to confirm this, but if someone can clarify this it will be great. But for that, I will spin v2 with the above comments addressed as in my reply and post. Thanks > > That's how it should work right? > -- Murali Karicheri Texas Instruments