netdev - Re: [net-next RFC PATCH 0/6] Add support for qca8k mdio rw in Ethernet packet

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 for Android: free password hash cracker in your pocket

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

Date:   Tue, 7 Dec 2021 19:53:43 +0100
From:   Ansuel Smith <ansuelsmth@...il.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org
Subject: Re: [net-next RFC PATCH 0/6] Add support for qca8k mdio rw in
 Ethernet packet

On Tue, Dec 07, 2021 at 07:41:23PM +0100, Andrew Lunn wrote:
> > I still have to find a solution to a slowdown problem and this is where
> > I would love to get some hint.
> > Currently I still didn't find a good way to understand when the tagger
> > starts to accept packets and because of this the initial setup is slow
> > as every completion timeouts. Am I missing something or is there a way
> > to check for this?
> 
> I've not looked at this particular driver, i just know the general
> architecture.
> 
> The MDIO bus driver probes first, maybe as part of the Ethernet
> driver, maybe as a standalone MDIO driver. The switch is found in DT
> and the driver code will at some point later probe the switch driver.
> 
> The switch driver has working MDIO at this point. It should use MDIO
> to talk to the switch, make sure it is there, maybe do some initial
> configuration. Once it is happy, it registers the switch with the DSA
> core using dsa_register_switch().
> 
> If this is a single switch, the DSA core will then start setting
> things up. As part of dsa_switch_setup() it will call the switch
> drivers setup() method. It then figures out what tag driver to use, by
> calling dsa_switch_setup_tag_protocol(). However, the tag driver
> itself is not inserted into the chain yet. That happens later.  Once
> the switch is setup, dsa_tree_setup_master() is called which does
> dsa_master_setup() and in the middle there is:
> 
> 	/* If we use a tagging format that doesn't have an ethertype
> 	 * field, make sure that all packets from this point on get
> 	 * sent to the tag format's receive function.
> 	 */
> 	wmb();
> 
> 	dev->dsa_ptr = cpu_dp;
> 
> This is the magic to actually enable the tagger receiving frames.
> 

Will check if using this is the correct way to prevent use of this
alternative way before it's available.

> I need to look at your patches, but why is the tagger involved?  At
> least for the Marvell switch, you send a pretty normal looking
> Ethernet frame to a specific MAC address, and the switch replies using
> that MAC address. And it has an Ether Type specific to switch
> control. Since this is all normal looking, there are hooks in the
> network stack which can be used to get these frames.
>

The qca tag header provide a TYPE value that refer to a big list of
Frame type. In all of this at value 2 we have the type that tells us
that is a READ_WRITE_REG_ACK (aka a mdio rw Ethernet packet)

The idea of using the tagger is to skip parsing the packet 2 times
considering the qca tag header is present at the same place in both
normal packet and mdio rw Ethernet packet.

Your idea would be hook this before the tagger and parse it?
I assume that is the only way if this has to be generilized. But I
wonder if this would create some overhead by the double parsing.

> 	Andrew
> 

-- 
	Ansuel