linux-kernel - 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <61afed23.1c69fb81.99660.7328@mx.google.com>
Date:   Wed, 8 Dec 2021 00:24:17 +0100
From:   Ansuel Smith <ansuelsmth@...il.com>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...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 Wed, Dec 08, 2021 at 01:20:20AM +0200, Vladimir Oltean wrote:
> On Wed, Dec 08, 2021 at 12:05:11AM +0100, Ansuel Smith wrote:
> > Hm. Interesting idea. So qca8k would provide the way to parse the packet
> > and made the request. The tagger would just detect the packet and
> > execute the dedicated function.
> > About mib considering the driver autocast counter for every port and
> > every packet have the relevant port to it (set in the qca tag), the
> > idea was to put a big array and directly write the data. The ethtool
> > function will then just read the data and report it. (or even work
> > directly on the ethtool data array).
> 
> Apart from the fact that you'd be running inside the priv->rw_reg_ack_handler()
> which runs in softirq context (so you need spinlocks to serialize with
> the code that runs in process and/or workqueue context), you have access
> to all the data structures from the switch driver that you're used to.
> So you could copy from the void *buf into something owned by struct
> qca8k_priv *priv, sure.
> 
> > >   My current idea is maybe not ideal and a bit fuzzy, because the switch
> > >   driver would need to be aware of the fact that the tagger private data
> > >   is in dp->priv, and some code in one folder needs to be in sync with
> > >   some code in another folder. But at least it should be safer this way,
> > >   because we are in more control over the exact connection that's being
> > >   made.
> > > 
> > > - to avoid leaking memory, we also need to patch dsa_tree_put() to issue
> > >   a disconnect event on unbind.
> > > 
> > > - the tagging protocol driver would always need to NULL-check the
> > >   function pointer before dereferencing it, because it may connect to a
> > >   switch driver that doesn't set them up (dsa_loop):
> > > 
> > > 	struct qca8k_tagger_private *priv = dp->priv;
> > > 
> > > 	if (priv->rw_reg_ack_handler)
> > > 		priv->rw_reg_ack_handler(dp, skb_mac_header(skb));
> > 
> > Ok so your idea is to make the driver the one controlling ""everything""
> > and keep the tagger as dummy as possible. That would also remove all the
> > need to put stuff in the global include dir. Looks complex but handy. We
> > still need to understand the state part. Any hint about that?
> > 
> > In the mean time I will try implement this.
> 
> What do you mean exactly by understanding the state?

I was referring to the "shared state" problem but you already answer
that in the prev email.

-- 
	Ansuel