netdev - phylink: flow control on fixed-link not working.

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-next>] [day] [month] [year] [list]

Message-ID: <20190717213111.Horde.nir2D5kAJww569fjh8BZgZm@www.vdorst.com>
Date:   Wed, 17 Jul 2019 21:31:11 +0000
From:   René van Dorst <opensource@...rst.com>
To:     netdev@...r.kernel.org
Cc:     Russell King <rmk+kernel@....linux.org.uk>
Subject: phylink: flow control on fixed-link not working.

Hi,

I am trying to enable flow control/pause on PHYLINK and fixed-link.

My setup SOC mac (mt7621) <-> RGMII <-> SWITCH mac (mt7530).

It seems that in fixed-link mode all the flow control/pause bits are  
cleared in
phylink_parse_fixedlink(). If I read phylink_parse_fixedlink() [0] correctly,
I see that pl->link_config.advertising is AND with pl->supprted which has only
the PHY_SETTING() modes bits set. pl->link_config.advertising is losing Pause
bits. pl->link_config.advertising is used in phylink_resolve_flow() to set the
MLO_PAUSE_RX/TX BITS.

I think this is an error.
Because in phylink_start() see this part [1].

  /* Apply the link configuration to the MAC when starting. This allows
   * a fixed-link to start with the correct parameters, and also
   * ensures that we set the appropriate advertisement for Serdes links.
   */
  phylink_resolve_flow(pl, &pl->link_config);
  phylink_mac_config(pl, &pl->link_config);

If I add a this hacky patch below, flow control is enabled on the fixed-link.
         if (s) {
                 __set_bit(s->bit, pl->supported);
+               if (phylink_test(pl->link_config.advertising, Pause))
+                       phylink_set(pl->supported, Pause);
         } else {

So is phylink_parse_fixedlink() broken or should it handled in a other way?

Greats,

René

[0]:  
https://elixir.bootlin.com/linux/latest/source/drivers/net/phy/phylink.c#L196
[1]:  
https://elixir.bootlin.com/linux/latest/source/drivers/net/phy/phylink.c#L897