Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 6 Dec 2022 04:03:33 +0100
From:   Andrew Lunn <andrew@lunn.ch>
To:     Piergiorgio Beruto <piergiorgio.beruto@gmail.com>
Cc:     "Russell King (Oracle)" <linux@armlinux.org.uk>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>, linux-kernel@vger.kernel.org,
        netdev@vger.kernel.org, Oleksij Rempel <o.rempel@pengutronix.de>
Subject: Re: [PATCH v3 net-next 1/4] net/ethtool: add netlink interface for
 the PLCA RS
Message-ID: <Y46xBVuv9iaUdNLs@lunn.ch>
References: <d53ffecdde8d3950a24155228a3439f2c9b10b9b.1670259123.git.piergiorgio.beruto@gmail.com>
 <Y44y05M+6NGod+4x@shell.armlinux.org.uk>
 <Y4455r631my4LNIU@gvm01>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Y4455r631my4LNIU@gvm01>
Precedence: bulk

> > Also, why do they need to be exported to modules? From what I can see,
> > the only user of these functions is phy_device.c, which is part of
> > the same module as phy.c where the functions live, meaning that they
> > don't need to be exported.
> I did this because similar functions in the same file are all exported
> to modules (e.g. phy_config_aneg, phy_speed_down, phy_start_cable_test).
> Therefore, I assumed the intention was to let modules (maybe out-of-tree
> modules?) make use of these functions. If you think we should not do
> this, would kindly explain why for example the phy_start_cable_test is
> exported?

phy_start_cable_test probably should not be exported. I probably got
this wrong. At one point, it might of been called directly from
net/ethtool/cabletest.c, but not any more. It is now accessed via
phy_ethtool_phy_ops.

Each kernel module is its own name space. You can only reference
something within a kernel module if it is exported. So you will find
all the helper functions in phy_device.c which are used by PHY drivers
are exported, for example.

You need to watch out for circular dependencies between modules, since
they are loaded sequentially. PHYs are generally leaves, they depend
on phylib and nothing else, so that is simple. The phylib module is
loaded first, and then the PHY drivers.

But there are more complex scenarios. A built in driver cannot
reference a module, that would result in undefined symbols. That is
probably what i got wrong with cable tests. I did all my testing with
it built in. But phylib can be built as a module. But that then
implies the core ethtool code cannot be built in, otherwise you get
undefined reference. So it was reworked to reference phylib via
phy_ethtool_phy_ops.

So as Russell says, if a function is only referenced within a module,
there is no need to export it, so keeping the kernel namespace clean.

      Andrew