netdev - Re: [PATCH net-next v3 5/7] net: dsa: Add helpers to convert netdev to ds or port index

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]

Date: Tue, 2 Apr 2024 14:31:34 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Vladimir Oltean <olteanv@...il.com>
Cc: Andrew Lunn <andrew@...n.ch>, Florian Fainelli <f.fainelli@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Gregory Clement <gregory.clement@...tlin.com>,
	netdev@...r.kernel.org
Subject: Re: [PATCH net-next v3 5/7] net: dsa: Add helpers to convert netdev
 to ds or port index

On Tue, Apr 02, 2024 at 01:56:52PM +0300, Vladimir Oltean wrote:
> On Mon, Apr 01, 2024 at 08:35:50AM -0500, Andrew Lunn wrote:
> > The LED helpers make use of a struct netdev. Add helpers a DSA driver
> > can use to convert a netdev to a struct dsa_switch and the port index.
> > 
> > To do this, dsa_user_to_port() has to be made available out side of
> > net/dev, to convert the inline function in net/dsa/user.h into a
> > normal function, and export it.
> > 
> > Signed-off-by: Andrew Lunn <andrew@...n.ch>
> > ---
> 
> I think the API we have today is sufficient: we have dsa_port_to_netdev(),
> introduced at Vivien's request rather than exporting dsa_user_to_port().
> 
> Also, I believe that having a single API function which returns a single
> struct dsa_port *, from which we force the caller to get the dp->ds and
> dp->index, is better (cheaper) than requiring 2 API functions, one for
> getting the ds and the other for the index.

Yes, I would tend to agree having done this for my experimental phylink
changes. struct dsa_port seems to make the most sense:

static inline struct dsa_port *
dsa_phylink_to_port(struct phylink_config *config)
{
        return container_of(config, struct dsa_port, pl_config);
}

which then means e.g.:

static void mv88e6xxx_mac_config(struct phylink_config *config,
                                 unsigned int mode,
                                 const struct phylink_link_state *state)
{
        struct dsa_port *dp = dsa_phylink_to_port(config);
        struct mv88e6xxx_chip *chip = dp->ds->priv;
        int port = dp->index;
        int err = 0;

        mv88e6xxx_reg_lock(chip);

        if (mode != MLO_AN_PHY || !mv88e6xxx_phy_is_internal(chip, port)) {
                err = mv88e6xxx_port_config_interface(chip, port,
                                                      state->interface);
                if (err && err != -EOPNOTSUPP)
                        goto err_unlock;
        }

err_unlock:
        mv88e6xxx_reg_unlock(chip);

        if (err && err != -EOPNOTSUPP)
                dev_err(dp->ds->dev, "p%d: failed to configure MAC/PCS\n",
                        port);
}

vs the current code:

static void mv88e6xxx_mac_config(struct dsa_switch *ds, int port,
                                 unsigned int mode,
                                 const struct phylink_link_state *state)
{
        struct mv88e6xxx_chip *chip = ds->priv;
        int err = 0;

        mv88e6xxx_reg_lock(chip);

        if (mode != MLO_AN_PHY || !mv88e6xxx_phy_is_internal(chip, port)) {
                err = mv88e6xxx_port_config_interface(chip, port,
                                                      state->interface);
                if (err && err != -EOPNOTSUPP)
                        goto err_unlock;
        }

err_unlock:
        mv88e6xxx_reg_unlock(chip);

        if (err && err != -EOPNOTSUPP)
                dev_err(ds->dev, "p%d: failed to configure MAC/PCS\n", port);
}

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!