netdev - Re: [PATCH net-next 0/1] Support for VLAN interface over HSR/PRP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+FuTSde4fAiy6t-Q4o1C8wdU4ZYqNt0Qd2F8PsrnXvA8q03=A@mail.gmail.com>
Date:   Tue, 8 Sep 2020 20:50:26 +0200
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Murali Karicheri <m-karicheri2@...com>
Cc:     Willem de Bruijn <willemdebruijn.kernel@...il.com>,
        David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Network Development <netdev@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>, nsekhar@...com,
        Grygorii Strashko <grygorii.strashko@...com>
Subject: Re: [PATCH net-next 0/1] Support for VLAN interface over HSR/PRP

> > >>>
> > >>> 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?
>
> That was my understanding.
>
> > 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?
>
> It's good to answer this through code inspection and/or
> instrumentation. I do not have the answer immediately either.

Agreed that from reading the code I would vlan_do_receive to not find
a vlan dev associated with the physical nic, then passes the packet
unmodified to hsr_handle_frame.

Perhaps this seems to work because skb_vlan_untag has already
pulled the tag out of the packet?

But then you should not see counters increased on the vlan dev.