[PATCH] net: dsa: Fix conditional handling of Wake-on-Lan configuration in dsa_user_set_wol

[PATCH] net: dsa: Fix conditional handling of Wake-on-Lan configuration in dsa_user_set_wol

[Mohammed Anees]

The WOL configuration now checks if the DSA switch supports setting WOL
before attempting to apply settings via phylink. This prevents
unnecessary calls to phylink_ethtool_set_wol when WOL is not supported.

Signed-off-by: Mohammed Anees <pvmohammedanees2003@gmail.com>
---
 net/dsa/user.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/dsa/user.c b/net/dsa/user.c
index 74eda9b30608..c685ccea9ddf 100644
--- a/net/dsa/user.c
+++ b/net/dsa/user.c
@@ -1217,10 +1217,12 @@ static int dsa_user_set_wol(struct net_device *dev, struct ethtool_wolinfo *w)
 	struct dsa_switch *ds = dp->ds;
 	int ret = -EOPNOTSUPP;
 
-	phylink_ethtool_set_wol(dp->pl, w);
-
-	if (ds->ops->set_wol)
+	if (ds->ops->set_wol) {
 		ret = ds->ops->set_wol(ds, dp->index, w);
+		if (ret)
+			return ret;
+		phylink_ethtool_set_wol(dp->pl, w);
+	}
 
 	return ret;
 }
-- 
2.46.0

Re: [PATCH] net: dsa: Fix conditional handling of Wake-on-Lan configuration in dsa_user_set_wol

[Andrew Lunn]

On Sat, Oct 05, 2024 at 03:32:06AM +0530, Mohammed Anees wrote:
> The WOL configuration now checks if the DSA switch supports setting WOL
> before attempting to apply settings via phylink. This prevents
> unnecessary calls to phylink_ethtool_set_wol when WOL is not supported.

The commit message should say why a change is being made. Why should
phylink_ethtool_set_wol() not be called? Why is it unnecassary? What
if the PHY supports WoL, and does not need any help from DSA?


    Andrew

---
pw-bot: cr

Re: [PATCH] net: dsa: Fix conditional handling of Wake-on-Lan configuration in dsa_user_set_wol

[Mohammed Anees]

In the original code, we initialize ret = -EOPNOTSUPP and then call 
phylink_ethtool_set_wol(). If DSA supports WOL, we call set_wol(). 
However, we aren’t checking if phylink_ethtool_set_wol() succeeds, 
so I assumed both functions should be called, and if either fails,
we return -EOPNOTSUPP.


static int dsa_user_set_wol(struct net_device *dev, struct ethtool_wolinfo *w)
{
	struct dsa_port *dp = dsa_user_to_port(dev);
	struct dsa_switch *ds = dp->ds;
	int ret = -EOPNOTSUPP;

	phylink_ethtool_set_wol(dp->pl, w);

	if (ds->ops->set_wol)
		ret = ds->ops->set_wol(ds, dp->index, w);

	return ret;
}

From your response, it seems either of the two function can handle setting 
WOL, if so shouldn't we check the return value of phylink_ethtool_set_wol() 
to ensure it succeeds?

Thanks!

Re: [PATCH] net: dsa: Fix conditional handling of Wake-on-Lan configuration in dsa_user_set_wol

[Andrew Lunn]

On Sun, Oct 06, 2024 at 12:12:33AM +0530, Mohammed Anees wrote:
> In the original code, we initialize ret = -EOPNOTSUPP and then call 
> phylink_ethtool_set_wol(). If DSA supports WOL, we call set_wol(). 
> However, we aren’t checking if phylink_ethtool_set_wol() succeeds, 
> so I assumed both functions should be called, and if either fails,
> we return -EOPNOTSUPP.
> 
> 
> static int dsa_user_set_wol(struct net_device *dev, struct ethtool_wolinfo *w)
> {
> 	struct dsa_port *dp = dsa_user_to_port(dev);
> 	struct dsa_switch *ds = dp->ds;
> 	int ret = -EOPNOTSUPP;
> 
> 	phylink_ethtool_set_wol(dp->pl, w);
> 
> 	if (ds->ops->set_wol)
> 		ret = ds->ops->set_wol(ds, dp->index, w);
> 
> 	return ret;
> }
> 
> >From your response, it seems either of the two function can handle setting 
> WOL, if so shouldn't we check the return value of phylink_ethtool_set_wol() 
> to ensure it succeeds?

It is actually a bit more subtle than that, and i think everything
gets it wrong. Yes, we should check the return code from
phylink_ethtool_set_wol. If it does not return an error, we are
done. If it returns an error other than EOPNOTSUPP, it should return
it. And in the case of EOPNOTSUPP we should try to see if DSA supports
the WoL mode. And this is probably an over simplification. ethtool man
page says:

          wol p|u|m|b|a|g|s|f|d...
                  Sets Wake-on-LAN options.  Not all devices support this.  The
                  argument to this option is a string of characters  specifying
                  which options to enable.
                  p   Wake on PHY activity
                  u   Wake on unicast messages
                  m   Wake on multicast messages
                  b   Wake on broadcast messages
                  a   Wake on ARP
                  g   Wake on MagicPacket™
                  s   Enable SecureOn™ password for MagicPacket™
                  f   Wake on filter(s)
                  d   Disable  (wake  on  nothing).  This option
                      clears all previous options.

So userspace could say pumbagsf, with the PHY supporting pmub and the
MAC supporting agsf, and the two need to cooperate.

get_wol() needs to call both phylink_ethtool_get_wol() and dsa
get_wol, and combine the results.

	Andrew

Re: [PATCH] net: dsa: Fix conditional handling of Wake-on-Lan configuration in dsa_user_set_wol

[Mohammed Anees]

Considering the insight you've provided, I've written the code below

static int dsa_user_set_wol(struct net_device *dev, struct ethtool_wolinfo *w)
{
	struct dsa_port *dp = dsa_user_to_port(dev);
	struct dsa_switch *ds = dp->ds;
	int ret;

	ret = phylink_ethtool_set_wol(dp->pl, w);

	if (ret != -EOPNOTSUPP)
		return ret;

	if (ds->ops->set_wol)
		ret = ds->ops->set_wol(ds, dp->index, w);
		if (ret != -EOPNOTSUPP)
			return ret;

	return -EOPNOTSUPP;
}

Does this approach address the issue, or do you think it would
be better to pass a struct that explicitly contains only the
options supported by each function (phylink_ethtool_set_wol()
and ds->ops->set_wol())? That way we don't have to check for
-EOPNOTSUPP as we are giving valid options to each function.

Re: [PATCH] net: dsa: Fix conditional handling of Wake-on-Lan configuration in dsa_user_set_wol

[Andrew Lunn]

On Sun, Oct 06, 2024 at 09:40:32PM +0530, Mohammed Anees wrote:
> Considering the insight you've provided, I've written the code below
> 
> static int dsa_user_set_wol(struct net_device *dev, struct ethtool_wolinfo *w)
> {
> 	struct dsa_port *dp = dsa_user_to_port(dev);
> 	struct dsa_switch *ds = dp->ds;
> 	int ret;
> 
> 	ret = phylink_ethtool_set_wol(dp->pl, w);
> 
> 	if (ret != -EOPNOTSUPP)
> 		return ret;
> 
> 	if (ds->ops->set_wol)
> 		ret = ds->ops->set_wol(ds, dp->index, w);
> 		if (ret != -EOPNOTSUPP)
> 			return ret;

This can be simplified to just:

> 	if (ds->ops->set_wol)
> 		return ds->ops->set_wol(ds, dp->index, w);
> 
> 	return -EOPNOTSUPP;
> }

	Andrew

Re: [PATCH] net: dsa: Fix conditional handling of Wake-on-Lan configuration in dsa_user_set_wol

[Mohammed Anees]

I shall apply these changes and send a new v2 patch,
thanks!

[PATCH v2] net: dsa: Fix conditional handling of Wake-on-Lan configuration in dsa_user_set_wol

[Mohammed Anees]

In the original implementation of dsa_user_set_wol(), the return
value of phylink_ethtool_set_wol() was not checked, which could
lead to errors being ignored. This wouldn't matter if it returned
-EOPNOTSUPP, since that indicates the PHY layer doesn't support
the option, but if any other value is returned, it is problematic
and must be checked. The solution is to check the return value of
phylink_ethtool_set_wol(), and if it returns anything other than
-EOPNOTSUPP, immediately return the error. Only if it returns
-EOPNOTSUPP should the function proceed to check whether WoL can
be set by ds->ops->set_wol().

Fixes: 57719771a244 ("Merge tag 'sound-6.11' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound")
Signed-off-by: Mohammed Anees <pvmohammedanees2003@gmail.com>
---
v2:
- Added error checking for phylink_ethtool_set_wol(), ensuring correct
handling compared to v1.
___
 net/dsa/user.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/net/dsa/user.c b/net/dsa/user.c
index 74eda9b30608..bae5ed22db91 100644
--- a/net/dsa/user.c
+++ b/net/dsa/user.c
@@ -1215,14 +1215,17 @@ static int dsa_user_set_wol(struct net_device *dev, struct ethtool_wolinfo *w)
 {
 	struct dsa_port *dp = dsa_user_to_port(dev);
 	struct dsa_switch *ds = dp->ds;
-	int ret = -EOPNOTSUPP;
-
-	phylink_ethtool_set_wol(dp->pl, w);
-
+	int ret;
+
+	ret = phylink_ethtool_get_wol(dp->pl, w);
+
+	if (ret != -EOPNOTSUPP)
+		return ret;
+
 	if (ds->ops->set_wol)
-		ret = ds->ops->set_wol(ds, dp->index, w);
+		return ds->ops->set_wol(ds, dp->index, w);
 
-	return ret;
+	return -EOPNOTSUPP;
 }
 
 static int dsa_user_set_eee(struct net_device *dev, struct ethtool_keee *e)
-- 
2.46.0

Re: [PATCH] net: dsa: Fix conditional handling of Wake-on-Lan configuration in dsa_user_set_wol

[Russell King (Oracle)]

On Sun, Oct 06, 2024 at 09:57:26PM +0200, Andrew Lunn wrote:
> On Sun, Oct 06, 2024 at 09:40:32PM +0530, Mohammed Anees wrote:
> > Considering the insight you've provided, I've written the code below
> > 
> > static int dsa_user_set_wol(struct net_device *dev, struct ethtool_wolinfo *w)
> > {
> > 	struct dsa_port *dp = dsa_user_to_port(dev);
> > 	struct dsa_switch *ds = dp->ds;
> > 	int ret;
> > 
> > 	ret = phylink_ethtool_set_wol(dp->pl, w);
> > 
> > 	if (ret != -EOPNOTSUPP)
> > 		return ret;
> > 
> > 	if (ds->ops->set_wol)
> > 		ret = ds->ops->set_wol(ds, dp->index, w);
> > 		if (ret != -EOPNOTSUPP)
> > 			return ret;
> 
> This can be simplified to just:
> 
> > 	if (ds->ops->set_wol)
> > 		return ds->ops->set_wol(ds, dp->index, w);
> > 
> > 	return -EOPNOTSUPP;
> > }

I don't think the above is correct. While the simplification is, the
overall logic is not.

Let's go back to what Andrew said in his previous reply:

"So userspace could say pumbagsf, with the PHY supporting pmub and the
MAC supporting agsf, and the two need to cooperate."

The above does not do this. Let's go back further:

        phylink_ethtool_set_wol(dp->pl, w);

        if (ds->ops->set_wol)
                ret = ds->ops->set_wol(ds, dp->index, w);

The original code _does_ do this, allowing the PHY and MAC to set
their modes, although the return code is not correct.

I notice V2 of the patch has been posted - in my opinion prematurely
because there's clearly the discussion on the first version has not
reached a conclusion yet.

What I would propose is the following:

	int phy_ret, mac_ret;

	phy_ret = phylink_ethtool_set_wol(dp->pl, w);
	if (phy_ret != 0 && phy_ret != -EOPNOTSUPP)
		return phy_ret;

	if (ds->ops->set_wol)
		mac_ret = ds->ops->set_wol(ds, dp->index, w);
	else
		mac_ret = -EOPNOTSUPP;

	if (mac_ret != 0 && mac_ret != -EOPNOTSUPP)
		return mac_ret;

	/* Combine the two return codes. If either returned zero,
	 * then we have been successful.
	 */
	if (phy_ret == 0 || mac_ret == 0)
		return 0;
	
	return -EOPNOTSUPP;

Which I think is the closest one can get to - there is the possibility
for phylink_ethtool_set_wol() to have modified the WoL state, but
ds->ops->set_wol() to fail with an error code, causing this to return
failure, but I don't see that as being avoidable without yet more
complexity.

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

Re: [PATCH v2] net: dsa: Fix conditional handling of Wake-on-Lan configuration in dsa_user_set_wol

[Russell King (Oracle)]

On Mon, Oct 07, 2024 at 04:49:38AM +0530, Mohammed Anees wrote:
> In the original implementation of dsa_user_set_wol(), the return
> value of phylink_ethtool_set_wol() was not checked, which could
> lead to errors being ignored. This wouldn't matter if it returned
> -EOPNOTSUPP, since that indicates the PHY layer doesn't support
> the option, but if any other value is returned, it is problematic
> and must be checked. The solution is to check the return value of
> phylink_ethtool_set_wol(), and if it returns anything other than
> -EOPNOTSUPP, immediately return the error. Only if it returns
> -EOPNOTSUPP should the function proceed to check whether WoL can
> be set by ds->ops->set_wol().
> 
> Fixes: 57719771a244 ("Merge tag 'sound-6.11' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound")
> Signed-off-by: Mohammed Anees <pvmohammedanees2003@gmail.com>
> ---
> v2:
> - Added error checking for phylink_ethtool_set_wol(), ensuring correct
> handling compared to v1.

I don't think this adds "correct handing". See my reply to the first
version.

pw-bot: cr

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

Re: [PATCH v2] net: dsa: Fix conditional handling of Wake-on-Lan configuration in dsa_user_set_wol

[kernel test robot]

Hi Mohammed,

kernel test robot noticed the following build errors:

[auto build test ERROR on net/main]
[also build test ERROR on net-next/main linus/master v6.12-rc2 next-20241004]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Mohammed-Anees/net-dsa-Fix-conditional-handling-of-Wake-on-Lan-configuration-in-dsa_user_set_wol/20241007-072229
base:   net/main
patch link:    https://lore.kernel.org/r/20241006231938.4382-1-pvmohammedanees2003%40gmail.com
patch subject: [PATCH v2] net: dsa: Fix conditional handling of Wake-on-Lan configuration in dsa_user_set_wol
config: arc-allmodconfig (https://download.01.org/0day-ci/archive/20241008/202410080459.sbnWWj91-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241008/202410080459.sbnWWj91-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410080459.sbnWWj91-lkp@intel.com/

All errors (new ones prefixed by >>):

   net/dsa/user.c: In function 'dsa_user_set_wol':
>> net/dsa/user.c:1220:13: error: void value not ignored as it ought to be
    1220 |         ret = phylink_ethtool_get_wol(dp->pl, w);
         |             ^

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for GET_FREE_REGION
   Depends on [n]: SPARSEMEM [=n]
   Selected by [m]:
   - RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]


vim +1220 net/dsa/user.c

  1213	
  1214	static int dsa_user_set_wol(struct net_device *dev, struct ethtool_wolinfo *w)
  1215	{
  1216		struct dsa_port *dp = dsa_user_to_port(dev);
  1217		struct dsa_switch *ds = dp->ds;
  1218		int ret;
  1219	
> 1220		ret = phylink_ethtool_get_wol(dp->pl, w);
  1221	
  1222		if (ret != -EOPNOTSUPP)
  1223			return ret;
  1224	
  1225		if (ds->ops->set_wol)
  1226			return ds->ops->set_wol(ds, dp->index, w);
  1227	
  1228		return -EOPNOTSUPP;
  1229	}
  1230	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2] net: dsa: Fix conditional handling of Wake-on-Lan configuration in dsa_user_set_wol

[Vladimir Oltean]

Hi Mohammed,

On Mon, Oct 07, 2024 at 04:49:38AM +0530, Mohammed Anees wrote:
> In the original implementation of dsa_user_set_wol(), the return
> value of phylink_ethtool_set_wol() was not checked, which could
> lead to errors being ignored. This wouldn't matter if it returned
> -EOPNOTSUPP, since that indicates the PHY layer doesn't support
> the option, but if any other value is returned, it is problematic
> and must be checked. The solution is to check the return value of
> phylink_ethtool_set_wol(), and if it returns anything other than
> -EOPNOTSUPP, immediately return the error. Only if it returns
> -EOPNOTSUPP should the function proceed to check whether WoL can
> be set by ds->ops->set_wol().
> 
> Fixes: 57719771a244 ("Merge tag 'sound-6.11' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound")

The Fixes: tag is completely bogus. It's supposed to point to the commit
which introduced the issue (aka what 'git bisect' would land on).

> Signed-off-by: Mohammed Anees <pvmohammedanees2003@gmail.com>
> ---
> v2:
> - Added error checking for phylink_ethtool_set_wol(), ensuring correct
> handling compared to v1.
> ___

this should have been "---" not "___".

>  net/dsa/user.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/net/dsa/user.c b/net/dsa/user.c
> index 74eda9b30608..bae5ed22db91 100644
> --- a/net/dsa/user.c
> +++ b/net/dsa/user.c
> @@ -1215,14 +1215,17 @@ static int dsa_user_set_wol(struct net_device *dev, struct ethtool_wolinfo *w)
> -	phylink_ethtool_set_wol(dp->pl, w);
> +	ret = phylink_ethtool_get_wol(dp->pl, w);

Could you tell us a bit about your motivation for making a change?
How have you noticed the lack of error checking in phylink_ethtool_set_wol()?
What user-visible problem has it caused?

Since patches with Fixes: tags to older than net-next commits
get backported to stable kernels, the triage rules from
Documentation/process/stable-kernel-rules.rst apply.

If this is purely theoretical and you are not already testing this,
then please do so. (it seems you aren't, because you replaced
phylink_ethtool_get_wol() with phylink_ethtool_set_wol()).

Luckily, you could somewhat exercise the code paths using the dsa_loop
mock-up driver even if you don't have a supported hardware switch. It
isn't the same as the real thing, but with some instrumentation and
carefully chosen test cases and simulated return codes, I could see it
being used to prove a point.

If you don't have the interest of testing this patch, then I would
request you to abandon it. The topic of combining MAC WoL with PHY WoL
is not trivial and a theoretical "fix" can make things that used to work
stop working. It's unlikely that basing patches off of a chat on the
mailing list is going to make things better if it all stays theoretical
and no one tests anything, even if that chat is with Andrew and Russell,
the opinions of both of whom are extremely respectable in this area.

In principle there's also the option of requesting somebody else to
test if you cannot, like the submitter of the blamed patch, if there's
reasonable suspicion that something is not right. In this case,
interesting thing, the phylink_ethtool_set_wol() call got introduced in
commit aab9c4067d23 ("net: dsa: Plug in PHYLINK support") by Florian,
but there was no phy_ethtool_set_wol() prior to that. So it's not clear
at all what use cases came to depend upon the phylink_ethtool_set_wol()
call in the meantime since 2018. I'm not convinced that said commit
voluntarily introduced the call.

If this is not an exclusively theoretical issue and this was just an
honest mistake, then please do continue the patch submission process.
But for my curiosity, what platform are you experimenting on?

Re: [PATCH] net: dsa: Fix conditional handling of Wake-on-Lan configuration in dsa_user_set_wol

[Mohammed Anees]

Apologies for overlooking the previous code. Based on your 
suggestion, I’ve refined the implementation. Below is the 
final version, please let me know if this works, and I’ll 
send a new patch.

int phy_ret, mac_ret = -EOPNOTSUPP;

phy_ret = phylink_ethtool_set_wol(dp->pl, w);
if (phy_ret != 0 && phy_ret != -EOPNOTSUPP)
    return phy_ret;

if (ds->ops->set_wol) {
    mac_ret = ds->ops->set_wol(ds, dp->index, w);
    if (mac_ret != 0 && mac_ret != -EOPNOTSUPP)
        return mac_ret;
}

// Return success if either PHY or MAC succeeded
if (phy_ret == 0 || mac_ret == 0)
	return 0;

return -EOPNOTSUPP;

Thanks!

Re: [PATCH] net: dsa: Fix conditional handling of Wake-on-Lan configuration in dsa_user_set_wol

[Mohammed Anees]

Hi Vladimir,

Thank you for your thoughtful feedback; I really appreciate it. I want 
to be honest, I’m quite new to this process and have been exploring the 
the code to learn as much as I can, which was when i noticed this. I was 
excited to contribute and took a more theoretical approach without fully
grasping the importance of testing to see if this is ever possible.

I completely understand your points now and apologize for not testing 
this. I haven’t had the chance to experiment with it on any platform yet, 
but I plan to do so to see if I encounter any issues. If I do run into 
problems, I’ll definitely continue with this patch and work on resolving them.

Thanks once more for your guidance and for clarifying the intricacies 
of MAC WoL and PHY WoL interaction.

Re: [PATCH v2] net: dsa: Fix conditional handling of Wake-on-Lan configuration in dsa_user_set_wol

[kernel test robot]

Hi Mohammed,

kernel test robot noticed the following build errors:

[auto build test ERROR on net/main]
[also build test ERROR on net-next/main linus/master v6.12-rc2 next-20241004]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Mohammed-Anees/net-dsa-Fix-conditional-handling-of-Wake-on-Lan-configuration-in-dsa_user_set_wol/20241007-072229
base:   net/main
patch link:    https://lore.kernel.org/r/20241006231938.4382-1-pvmohammedanees2003%40gmail.com
patch subject: [PATCH v2] net: dsa: Fix conditional handling of Wake-on-Lan configuration in dsa_user_set_wol
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20241008/202410080616.wpZV4fAa-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project fef3566a25ff0e34fb87339ba5e13eca17cec00f)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241008/202410080616.wpZV4fAa-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410080616.wpZV4fAa-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from net/dsa/user.c:8:
   In file included from include/linux/etherdevice.h:20:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:10:
   In file included from include/linux/mm.h:2213:
   include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     504 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     505 |                            item];
         |                            ~~~~
   include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     511 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     512 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     518 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     524 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     525 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   In file included from net/dsa/user.c:8:
   In file included from include/linux/etherdevice.h:20:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:28:
   In file included from include/linux/dma-mapping.h:11:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:93:
   include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     548 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     561 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
         |                                                           ^
   include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
     102 | #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
         |                                                      ^
   In file included from net/dsa/user.c:8:
   In file included from include/linux/etherdevice.h:20:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:28:
   In file included from include/linux/dma-mapping.h:11:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:93:
   include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     574 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
         |                                                           ^
   include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
     115 | #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
         |                                                      ^
   In file included from net/dsa/user.c:8:
   In file included from include/linux/etherdevice.h:20:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:28:
   In file included from include/linux/dma-mapping.h:11:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:93:
   include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     585 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     595 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     605 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:693:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     693 |         readsb(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:701:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     701 |         readsw(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:709:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     709 |         readsl(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:718:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     718 |         writesb(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:727:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     727 |         writesw(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:736:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     736 |         writesl(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
>> net/dsa/user.c:1220:6: error: assigning to 'int' from incompatible type 'void'
    1220 |         ret = phylink_ethtool_get_wol(dp->pl, w);
         |             ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   16 warnings and 1 error generated.

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for MODVERSIONS
   Depends on [n]: MODULES [=y] && !COMPILE_TEST [=y]
   Selected by [y]:
   - RANDSTRUCT_FULL [=y] && (CC_HAS_RANDSTRUCT [=y] || GCC_PLUGINS [=n]) && MODULES [=y]


vim +1220 net/dsa/user.c

  1213	
  1214	static int dsa_user_set_wol(struct net_device *dev, struct ethtool_wolinfo *w)
  1215	{
  1216		struct dsa_port *dp = dsa_user_to_port(dev);
  1217		struct dsa_switch *ds = dp->ds;
  1218		int ret;
  1219	
> 1220		ret = phylink_ethtool_get_wol(dp->pl, w);
  1221	
  1222		if (ret != -EOPNOTSUPP)
  1223			return ret;
  1224	
  1225		if (ds->ops->set_wol)
  1226			return ds->ops->set_wol(ds, dp->index, w);
  1227	
  1228		return -EOPNOTSUPP;
  1229	}
  1230	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki