Netdev List
 help / color / mirror / Atom feed
* [PATCH net] net: dscc4: replace dev_kfree_skb_irq by dev_consume_skb_irq for drop profiles
From: Yang Wei @ 2019-02-05 16:07 UTC (permalink / raw)
  To: netdev; +Cc: romieu, davem, yang.wei9, albin_yang

From: Yang Wei <yang.wei9@zte.com.cn>

dev_consume_skb_irq() should be called in dscc4_tx_irq() when skb
xmit done. It makes drop profiles(dropwatch, perf) more friendly.

Signed-off-by: Yang Wei <yang.wei9@zte.com.cn>
---
 drivers/net/wan/dscc4.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wan/dscc4.c b/drivers/net/wan/dscc4.c
index c0b0f52..27decf8 100644
--- a/drivers/net/wan/dscc4.c
+++ b/drivers/net/wan/dscc4.c
@@ -1575,7 +1575,7 @@ static void dscc4_tx_irq(struct dscc4_pci_priv *ppriv,
 					dev->stats.tx_packets++;
 					dev->stats.tx_bytes += skb->len;
 				}
-				dev_kfree_skb_irq(skb);
+				dev_consume_skb_irq(skb);
 				dpriv->tx_skbuff[cur] = NULL;
 				++dpriv->tx_dirty;
 			} else {
-- 
2.7.4



^ permalink raw reply related

* Re: [PATCH bpf-next v2 0/4] Add RISC-V (RV64G) BPF JIT
From: Daniel Borkmann @ 2019-02-05 16:05 UTC (permalink / raw)
  To: bjorn.topel, linux-riscv, ast, netdev; +Cc: palmer, hch
In-Reply-To: <20190205124125.5553-1-bjorn.topel@gmail.com>

On 02/05/2019 01:41 PM, bjorn.topel@gmail.com wrote:
> From: Björn Töpel <bjorn.topel@gmail.com>
> 
> Hi!
> 
> This v2 series adds an RV64G BPF JIT to the kernel.
> 
> At the moment the RISC-V Linux port does not support
> CONFIG_HAVE_KPROBES (Patrick Stählin sent out an RFC last year), which
> means that CONFIG_BPF_EVENTS is not supported. Thus, no tests
> involving BPF_PROG_TYPE_TRACEPOINT, BPF_PROG_TYPE_PERF_EVENT,
> BPF_PROG_TYPE_KPROBE and BPF_PROG_TYPE_RAW_TRACEPOINT passes.
> 
> The implementation does not support "far branching" (>4KiB).
> 
> Test results:
>   # modprobe test_bpf
>   test_bpf: Summary: 378 PASSED, 0 FAILED, [366/366 JIT'ed]
> 
>   # echo 1 > /proc/sys/kernel/unprivileged_bpf_disabled
>   # ./test_verifier
>   ...
>   Summary: 761 PASSED, 507 SKIPPED, 2 FAILED
> 
> Note that "test_verifier" was run with one build with
> CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS=y and one without, otherwise
> many of the the tests that require unaligned access were skipped.
> 
> CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS=y:
>   # echo 1 > /proc/sys/kernel/unprivileged_bpf_disabled
>   # ./test_verifier | grep -c 'NOTE.*unknown align'
>   0
> 
> No CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS:
>   # echo 1 > /proc/sys/kernel/unprivileged_bpf_disabled
>   # ./test_verifier | grep -c 'NOTE.*unknown align'
>   59
> 
> The two failing test_verifier tests are:
>   "ld_abs: vlan + abs, test 1"
>   "ld_abs: jump around ld_abs"
> 
> This is due to that "far branching" involved in those tests.
> All tests where done on QEMU emulator version 3.1.50
> (v3.1.0-688-g8ae951fbc106). I'll test it on real hardware, when I get
> access to it.
> 
> I'm routing this patch via bpf-next/netdev mailing list (after a
> conversation with Palmer at FOSDEM), mainly because the other JITs
> went that path.
> 
> Again, thanks for all the comments!
> 
> Cheers,
> Björn
> 
> v1 -> v2:
> * Added JMP32 support. (Daniel)
> * Add RISC-V to Documentation/sysctl/net.txt. (Daniel)
> * Fixed seen_call() asymmetry. (Daniel)
> * Fixed broken bpf_flush_icache() range. (Daniel)
> * Added alignment annotations to some selftests.
> 
> RFCv1 -> v1:
> * Cleaned up the Kconfig and net/Makefile. (Christoph)
> * Removed the entry-stub and squashed the build/config changes to be
>   part of the JIT implementation. (Christoph)
> * Simplified the register tracking code. (Daniel)
> * Removed unused macros. (Daniel)
> * Added myself as maintainer and updated documentation. (Daniel)
> * Removed HAVE_EFFICIENT_UNALIGNED_ACCESS. (Christoph, Palmer)
> * Added tail-calls and cleaned up the code.
> 
> 
> Björn Töpel (4):
>   bpf, riscv: add BPF JIT for RV64G
>   MAINTAINERS: add RISC-V BPF JIT maintainer
>   bpf, doc: add RISC-V JIT to BPF documentation
>   selftests/bpf: add "any alignment" annotation for some tests
> 
>  Documentation/networking/filter.txt           |   16 +-
>  Documentation/sysctl/net.txt                  |    1 +
>  MAINTAINERS                                   |    6 +
>  arch/riscv/Kconfig                            |    1 +
>  arch/riscv/Makefile                           |    2 +-
>  arch/riscv/net/Makefile                       |    1 +
>  arch/riscv/net/bpf_jit_comp.c                 | 1602 +++++++++++++++++
>  .../selftests/bpf/verifier/ctx_sk_msg.c       |    1 +
>  .../testing/selftests/bpf/verifier/ctx_skb.c  |    1 +
>  tools/testing/selftests/bpf/verifier/jmp32.c  |   22 +
>  tools/testing/selftests/bpf/verifier/jset.c   |    2 +
>  .../selftests/bpf/verifier/spill_fill.c       |    1 +
>  .../selftests/bpf/verifier/spin_lock.c        |    2 +
>  .../selftests/bpf/verifier/value_ptr_arith.c  |    4 +
>  14 files changed, 1654 insertions(+), 8 deletions(-)
>  create mode 100644 arch/riscv/net/Makefile
>  create mode 100644 arch/riscv/net/bpf_jit_comp.c

Applied, thanks!

^ permalink raw reply

* Re: [PATCH btf v2 0/3] Add BTF types deduplication algorithm
From: Daniel Borkmann @ 2019-02-05 16:04 UTC (permalink / raw)
  To: Andrii Nakryiko, netdev, ast, yhs, acme, kernel-team, kafai,
	ecree, andrii.nakryiko
In-Reply-To: <20190205012946.1590917-1-andriin@fb.com>

On 02/05/2019 02:29 AM, Andrii Nakryiko wrote:
> This patch series adds BTF deduplication algorithm to libbpf. This algorithm
> allows to take BTF type information containing duplicate per-compilation unit
> information and reduce it to equivalent set of BTF types with no duplication without
> loss of information. It also deduplicates strings and removes those strings that
> are not referenced from any BTF type (and line information in .BTF.ext section,
> if any).
> 
> Algorithm also resolves struct/union forward declarations into concrete BTF types
> across multiple compilation units to facilitate better deduplication ratio. If
> undesired, this resolution can be disabled through specifying corresponding options.
> 
> When applied to BTF data emitted by pahole's DWARF->BTF converter, it reduces
> the overall size of .BTF section by about 65x, from about 112MB to 1.75MB, leaving
> only 29247 out of initial 3073497 BTF type descriptors.
> 
> Algorithm with minor differences and preliminary results before FUNC/FUNC_PROTO
> support is also described more verbosely at:
> https://facebookmicrosites.github.io/bpf/blog/2018/11/14/btf-enhancement.html
> 
> v1->v2:
> - rebase on latest bpf-next
> - err_log/elog -> pr_debug
> - btf__dedup, btf__get_strings, btf__get_nr_types listed under 0.0.2 version
> 
> Andrii Nakryiko (3):
>   btf: extract BTF type size calculation
>   btf: add BTF types deduplication algorithm
>   selftests/btf: add initial BTF dedup tests
> 
>  tools/lib/bpf/btf.c                    | 1851 +++++++++++++++++++++++-
>  tools/lib/bpf/btf.h                    |   10 +
>  tools/lib/bpf/libbpf.map               |    3 +
>  tools/testing/selftests/bpf/test_btf.c |  535 ++++++-
>  4 files changed, 2332 insertions(+), 67 deletions(-)

Applied, thanks!

^ permalink raw reply

* [PATCH net] net: tulip: de2104x: replace dev_kfree_skb_irq by dev_consume_skb_irq for drop profiles
From: Yang Wei @ 2019-02-05 16:03 UTC (permalink / raw)
  To: netdev, linux-parisc; +Cc: davem, yang.wei9, albin_yang

From: Yang Wei <yang.wei9@zte.com.cn>

dev_consume_skb_irq() should be called in de_tx() when skb xmit
done. It makes drop profiles(dropwatch, perf) more friendly.

Signed-off-by: Yang Wei <yang.wei9@zte.com.cn>
---
 drivers/net/ethernet/dec/tulip/de2104x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/dec/tulip/de2104x.c b/drivers/net/ethernet/dec/tulip/de2104x.c
index 13430f7..f1a2da1 100644
--- a/drivers/net/ethernet/dec/tulip/de2104x.c
+++ b/drivers/net/ethernet/dec/tulip/de2104x.c
@@ -585,7 +585,7 @@ static void de_tx (struct de_private *de)
 				netif_dbg(de, tx_done, de->dev,
 					  "tx done, slot %d\n", tx_tail);
 			}
-			dev_kfree_skb_irq(skb);
+			dev_consume_skb_irq(skb);
 		}
 
 next:
-- 
2.7.4



^ permalink raw reply related

* [PATCH net] net: defxx: replace dev_kfree_skb_irq by dev_consume_skb_irq for drop profiles
From: Yang Wei @ 2019-02-05 16:01 UTC (permalink / raw)
  To: netdev; +Cc: macro, davem, yang.wei9, albin_yang

From: Yang Wei <yang.wei9@zte.com.cn>

dev_consume_skb_irq() should be called in dfx_xmt_done() when skb
xmit done. It makes drop profiles(dropwatch, perf) more friendly.

Signed-off-by: Yang Wei <yang.wei9@zte.com.cn>
---
 drivers/net/fddi/defxx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/fddi/defxx.c b/drivers/net/fddi/defxx.c
index 38ac8ef..56b7791 100644
--- a/drivers/net/fddi/defxx.c
+++ b/drivers/net/fddi/defxx.c
@@ -3512,7 +3512,7 @@ static int dfx_xmt_done(DFX_board_t *bp)
 				 bp->descr_block_virt->xmt_data[comp].long_1,
 				 p_xmt_drv_descr->p_skb->len,
 				 DMA_TO_DEVICE);
-		dev_kfree_skb_irq(p_xmt_drv_descr->p_skb);
+		dev_consume_skb_irq(p_xmt_drv_descr->p_skb);
 
 		/*
 		 * Move to start of next packet by updating completion index
-- 
2.7.4



^ permalink raw reply related

* [PATCH 2/2] doc: add phylink documentation to the networking book
From: Russell King @ 2019-02-05 15:58 UTC (permalink / raw)
  To: linux-doc, netdev; +Cc: David S. Miller, Jonathan Corbet

Add some phylink documentation to the networking book detailing how
to convert network drivers from phylib to phylink.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
Version 2 adds the "Modes of operation" section, as it appears mvpp2 is
non-conformant (which is, unfortunately, causing problems in certain
circumstances.)

 Documentation/networking/index.rst       |   1 +
 Documentation/networking/sfp-phylink.rst | 268 +++++++++++++++++++++++++++++++
 2 files changed, 269 insertions(+)
 create mode 100644 Documentation/networking/sfp-phylink.rst

diff --git a/Documentation/networking/index.rst b/Documentation/networking/index.rst
index bd89dae8d578..ea81fc403b68 100644
--- a/Documentation/networking/index.rst
+++ b/Documentation/networking/index.rst
@@ -31,6 +31,7 @@ Linux Networking Documentation
    net_failover
    alias
    bridge
+   sfp-phylink
 
 .. only::  subproject
 
diff --git a/Documentation/networking/sfp-phylink.rst b/Documentation/networking/sfp-phylink.rst
new file mode 100644
index 000000000000..78a577c9d8a3
--- /dev/null
+++ b/Documentation/networking/sfp-phylink.rst
@@ -0,0 +1,268 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=======
+phylink
+=======
+
+Overview
+========
+
+phylink is a mechanism to support hot-pluggable networking modules
+without needing to re-initialise the adapter on hot-plug events.
+
+phylink supports conventional phylib-based setups, fixed link setups
+and SFP modules at present.
+
+Modes of operation
+==================
+
+phylink has several modes of operation, which depend on the firmware
+settings.
+
+1. PHY mode
+
+   In PHY mode, we use phylib to read the current link settings from
+   the PHY, and pass them to the MAC driver.  We expect the MAC driver
+   to configure exactly the modes that are specified without any
+   negotiation being enabled on the link.
+
+2. Fixed mode
+
+   Fixed mode is the same as PHY mode as far as the MAC driver is
+   concerned.
+
+3. In-band mode
+
+   In-band mode is used with 802.3z, SGMII and similar interface modes
+   are used, and we are expecting to use the and honor the in-band
+   negotiation or control word sent across the serdes channel.
+
+By example, what this means is that:
+
+.. code-block:: none
+
+  &eth {
+    phy = <&phy>;
+    phy-mode = "sgmii";
+  };
+
+does not use in-band SGMII signalling.  The PHY is expected to follow
+exactly the settings given to it in its :c:func:`mac_config` function.
+The link should be forced up or down appropriately in the
+:c:func:`mac_link_up` and :c:func:`mac_link_down` functions.
+
+.. code-block:: none
+
+  &eth {
+    managed = "in-band-status";
+    phy = <&phy>;
+    phy-mode = "sgmii";
+  };
+
+uses in-band mode, where results from the PHYs negotiation are passed
+to the MAC through the SGMII control word, and the MAC is expected to
+acknowledge the control word.  The :c:func:`mac_link_up` and
+:c:func:`mac_link_down` functions must not force the MAC side link
+up and down.
+
+Rough guide to converting a network driver to sfp/phylink
+=========================================================
+
+This guide briefly describes how to convert a network driver from
+phylib to the sfp/phylink support.  Please send patches to improve
+this documentation.
+
+1. Optionally split the network driver's phylib update function into
+   three parts dealing with link-down, link-up and reconfiguring the
+   MAC settings. This can be done as a separate preparation commit.
+
+   An example of this preparation can be found in git commit fc548b991fb0.
+
+2. Replace::
+
+	select FIXED_PHY
+	select PHYLIB
+
+   with::
+
+	select PHYLINK
+
+   in the driver's Kconfig stanza.
+
+3. Add::
+
+	#include <linux/phylink.h>
+
+   to the driver's list of header files.
+
+4. Add::
+
+	struct phylink *phylink;
+
+   to the driver's private data structure.  We shall refer to the
+   driver's private data pointer as ``priv`` below, and the driver's
+   private data structure as ``struct foo_priv``.
+
+5. Replace the following functions:
+
+   .. flat-table::
+    :header-rows: 1
+    :widths: 1 1
+    :stub-columns: 0
+
+    * - Original function
+      - Replacement function
+    * - phy_start(phydev)
+      - phylink_start(priv->phylink)
+    * - phy_stop(phydev)
+      - phylink_stop(priv->phylink)
+    * - phy_mii_ioctl(phydev, ifr, cmd)
+      - phylink_mii_ioctl(priv->phylink, ifr, cmd)
+    * - phy_ethtool_get_wol(phydev, wol)
+      - phylink_ethtool_get_wol(priv->phylink, wol)
+    * - phy_ethtool_set_wol(phydev, wol)
+      - phylink_ethtool_set_wol(priv->phylink, wol)
+    * - phy_disconnect(phydev)
+      - phylink_disconnect_phy(priv->phylink)
+
+   Please note that some of these functions must be called under the
+   rtnl lock, and will warn if not. This will normally be the case,
+   except if these are called from the driver suspend/resume paths.
+
+6. Add/replace ksettings get/set methods with:
+
+   .. code-block:: c
+
+    static int foo_ethtool_set_link_ksettings(struct net_device *dev,
+					     const struct ethtool_link_ksettings *cmd)
+    {
+	struct foo_priv *priv = netdev_priv(dev);
+
+	return phylink_ethtool_ksettings_set(priv->phylink, cmd);
+    }
+
+    static int foo_ethtool_get_link_ksettings(struct net_device *dev,
+					     struct ethtool_link_ksettings *cmd)
+    {
+	struct foo_priv *priv = netdev_priv(dev);
+
+	return phylink_ethtool_ksettings_get(priv->phylink, cmd);
+    }
+
+7. Replace the call to:
+
+	phy_dev = of_phy_connect(dev, node, link_func, flags, phy_interface)
+
+   and associated code with a call to:
+
+	err = phylink_of_phy_connect(priv->phylink, node, flags)
+
+   For the most part, ``flags`` can be zero, these flags are passed to
+   the of_phy_attach() inside this function call if a PHY is specified
+   in the DT node ``node``.
+
+   ``node`` should be the DT node which contains the network phy property,
+   fixed link properties, and will also contain the sfp property.
+
+   The setup of fixed links should also be removed; these are handled
+   natively by phylink.
+
+   of_phy_connect() was also passed a function pointer for link updates.
+   This function is replaced by a different form of MAC updates
+   described below in (8).
+
+   Manipulation of the PHY's supported/advertised happens within phylink
+   based on the validate callback, see below in (8).
+
+   Note that the driver no longer needs to store the ``phy_interface``,
+   and also note that ``phy_interface`` becomes a dynamic property,
+   just like the speed, duplex etc settings.
+
+   Finally, note that the MAC driver has no direct access to the PHY
+   anymore; that is because in the phylink model, the PHY can be
+   dynamic.
+
+8. Add a :c:type:`struct phylink_mac_ops <phylink_mac_ops>` instance to
+   the driver, which is a table of function pointers, and implement
+   these functions. The old link update function for
+   :c:func:`of_phy_connect` becomes three methods: :c:func:`mac_link_up`,
+   :c:func:`mac_link_down`, and :c:func:`mac_config`. If step 1 was
+   performed, then the functionality will have been split there.
+
+   It is important that if in-band negotiation is used,
+   :c:func:`mac_link_up` and :c:func:`mac_link_down` do not prevent the
+   in-band negotiation from completing, since these functions are called
+   when the in-band link state changes - otherwise the link will never
+   come up.
+
+   The :c:func:`validate` method should mask the supplied supported mask,
+   and ``state->advertising`` with the supported ethtool link modes.
+   These are the new ethtool link modes, so bitmask operations must be
+   used. For an example, see drivers/net/ethernet/marvell/mvneta.c.
+
+   The :c:func:`mac_link_state` method is used to read the link state
+   from the MAC, and report back the settings that the MAC is currently
+   using. This is particularly important for in-band negotiation
+   methods such as 1000base-X and SGMII.
+
+   The :c:func:`mac_config` method is used to update the MAC with the
+   requested state, and must avoid unnecessarily taking the link down
+   when making changes to the MAC configuration.  This means the
+   function should modify the state and only take the link down when
+   absolutely necessary to change the MAC configuration.  An example
+   of how to do this can be found in :c:func:`mvneta_mac_config` in
+   drivers/net/ethernet/marvell/mvneta.c.
+
+   For further information on these methods, please see the inline
+   documentation in :c:type:`struct phylink_mac_ops <phylink_mac_ops>`.
+
+9. Remove calls to of_parse_phandle() for the PHY,
+   of_phy_register_fixed_link() for fixed links etc from the probe
+   function, and replace with:
+
+   .. code-block:: c
+
+	struct phylink *phylink;
+
+	phylink = phylink_create(dev, node, phy_mode, &phylink_ops);
+	if (IS_ERR(phylink)) {
+		err = PTR_ERR(phylink);
+		fail probe;
+	}
+
+	priv->phylink = phylink;
+
+   and arrange to destroy the phylink in the probe failure path as
+   appropriate and the removal path too by calling:
+
+   .. code-block:: c
+
+	phylink_destroy(priv->phylink);
+
+10. Arrange for MAC link state interrupts to be forwarded into
+    phylink, via:
+
+    .. code-block:: c
+
+	phylink_mac_change(priv->phylink, link_is_up);
+
+    where ``link_is_up`` is true if the link is currently up or false
+    otherwise.
+
+11. Verify that the driver does not call::
+
+	netif_carrier_on()
+	netif_carrier_off()
+
+   as these will interfere with phylink's tracking of the link state,
+   and cause phylink to omit calls via the :c:func:`mac_link_up` and
+   :c:func:`mac_link_down` methods.
+
+Network drivers should call phylink_stop() and phylink_start() via their
+suspend/resume paths, which ensures that the appropriate
+:c:type:`struct phylink_mac_ops <phylink_mac_ops>` methods are called
+as necessary.
+
+For information describing the SFP cage in DT, please see the binding
+documentation in the kernel source tree
+``Documentation/devicetree/bindings/net/sff,sfp.txt``
-- 
2.7.4


^ permalink raw reply related

* [PATCH 1/2] net: phylink: update mac_config() documentation
From: Russell King @ 2019-02-05 15:58 UTC (permalink / raw)
  To: linux-doc, netdev

A detail for mac_config() had been missed in the documentation for the
method - it is expected that the method will update the MAC to the
settings, rather than completely reprogram the MAC on each call.
Update the documentation for this method for this detail.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 include/linux/phylink.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index 021fc6595856..606a121629a9 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -149,6 +149,13 @@ int mac_link_state(struct net_device *ndev,
  *   configuration word. Nothing is advertised by the MAC. The MAC is
  *   responsible for reading the configuration word and configuring
  *   itself accordingly.
+ *
+ * Implementations are expected to update the MAC to reflect the
+ * requested settings - i.o.w., if nothing has changed between two
+ * calls, no action is expected.  If only flow control settings have
+ * changed, flow control should be updated *without* taking the link
+ * down.  This "update" behaviour is critical to avoid bouncing the
+ * link up status.
  */
 void mac_config(struct net_device *ndev, unsigned int mode,
 		const struct phylink_link_state *state);
-- 
2.7.4


^ permalink raw reply related

* Re: [PATCH bpf-next] bpf: support SO_DEBUG in bpf_setsockopt()
From: Yafang Shao @ 2019-02-05 15:47 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Alexei Starovoitov, kafai, brakmo, ast, netdev, shaoyafang
In-Reply-To: <7d5c07bf-ec6c-3fac-d4f0-ff0bb7b5d174@iogearbox.net>

On Tue, Feb 5, 2019 at 4:23 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 02/04/2019 06:35 PM, Alexei Starovoitov wrote:
> > On Sun, Feb 03, 2019 at 04:15:07PM +0800, Yafang Shao wrote:
> >> Then we can enable/disable socket debugging without modifying user code.
> >> That is more convenient for debugging.
> >>
> >> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> >> ---
> >>  include/net/sock.h | 8 ++++++++
> >>  net/core/filter.c  | 3 +++
> >>  net/core/sock.c    | 8 --------
> >>  3 files changed, 11 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/include/net/sock.h b/include/net/sock.h
> >> index 2b229f7..8decee9 100644
> >> --- a/include/net/sock.h
> >> +++ b/include/net/sock.h
> >> @@ -1935,6 +1935,14 @@ static inline void sock_confirm_neigh(struct sk_buff *skb, struct neighbour *n)
> >>      }
> >>  }
> >>
> >> +static inline void sock_valbool_flag(struct sock *sk, int bit, int valbool)
> >> +{
> >> +    if (valbool)
> >> +            sock_set_flag(sk, bit);
> >> +    else
> >> +            sock_reset_flag(sk, bit);
> >> +}
> >> +
> >>  bool sk_mc_loop(struct sock *sk);
> >>
> >>  static inline bool sk_can_gso(const struct sock *sk)
> >> diff --git a/net/core/filter.c b/net/core/filter.c
> >> index 3a49f68..ce5da57 100644
> >> --- a/net/core/filter.c
> >> +++ b/net/core/filter.c
> >> @@ -4111,6 +4111,9 @@ static unsigned long bpf_xdp_copy(void *dst_buff, const void *src_buff,
> >>
> >>              /* Only some socketops are supported */
> >>              switch (optname) {
> >> +            case SO_DEBUG:
> >> +                    sock_valbool_flag(sk, SOCK_DBG, val);
> >> +                    break;
> >
> > I'm missing the point here.
> > This flag has any effect only when SOCK_DEBUGGING is set.
> > But it is off in distros.
> > Since it's for custom debug kernel only why bother with
> > setting the flag via bpf prog?
>
> +1, this seems like some ancient debugging interface. Back at last netconf
> there was a proposal [0] to have a tcp_stats(sk, TCP_MIB_...) API for MIBs
> counter such that this can be traced via BPF on a per socket basis, for
> example. Might be worthwhile to work into that direction instead and potentially
> get rid of the SOCK_DEBUG() statements and convert (where appropriate) to
> such an interface. Thoughts?
>
>   [0] page 14, http://vger.kernel.org/netconf2018_files/BrendanGregg_netconf2018.pdf

This proposal seems like a better solution.
I will think about it.
Thanks for your suggestion.

Thanks
Yafang

^ permalink raw reply

* Re: [PATCH bpf-next] bpf: support SO_DEBUG in bpf_setsockopt()
From: Yafang Shao @ 2019-02-05 15:44 UTC (permalink / raw)
  To: Alexei Starovoitov; +Cc: kafai, brakmo, ast, daniel, netdev, shaoyafang
In-Reply-To: <20190204173517.6o5v7yd5yn7pxjzi@ast-mbp.dhcp.thefacebook.com>

On Tue, Feb 5, 2019 at 1:35 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Sun, Feb 03, 2019 at 04:15:07PM +0800, Yafang Shao wrote:
> > Then we can enable/disable socket debugging without modifying user code.
> > That is more convenient for debugging.
> >
> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > ---
> >  include/net/sock.h | 8 ++++++++
> >  net/core/filter.c  | 3 +++
> >  net/core/sock.c    | 8 --------
> >  3 files changed, 11 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/net/sock.h b/include/net/sock.h
> > index 2b229f7..8decee9 100644
> > --- a/include/net/sock.h
> > +++ b/include/net/sock.h
> > @@ -1935,6 +1935,14 @@ static inline void sock_confirm_neigh(struct sk_buff *skb, struct neighbour *n)
> >       }
> >  }
> >
> > +static inline void sock_valbool_flag(struct sock *sk, int bit, int valbool)
> > +{
> > +     if (valbool)
> > +             sock_set_flag(sk, bit);
> > +     else
> > +             sock_reset_flag(sk, bit);
> > +}
> > +
> >  bool sk_mc_loop(struct sock *sk);
> >
> >  static inline bool sk_can_gso(const struct sock *sk)
> > diff --git a/net/core/filter.c b/net/core/filter.c
> > index 3a49f68..ce5da57 100644
> > --- a/net/core/filter.c
> > +++ b/net/core/filter.c
> > @@ -4111,6 +4111,9 @@ static unsigned long bpf_xdp_copy(void *dst_buff, const void *src_buff,
> >
> >               /* Only some socketops are supported */
> >               switch (optname) {
> > +             case SO_DEBUG:
> > +                     sock_valbool_flag(sk, SOCK_DBG, val);
> > +                     break;
>
> I'm missing the point here.
> This flag has any effect only when SOCK_DEBUGGING is set.
> But it is off in distros.

It's not off in distros. At least it is set in RHEL.
Pls. see the comment in include/net/sock.h,
    /* Define this to get the SOCK_DBG debugging facility. */
    #define SOCK_DEBUGGING

> Since it's for custom debug kernel only why bother with
> setting the flag via bpf prog?
>

^ permalink raw reply

* Re: Is advertising of 2500Mbps support must from phy device to set phy at 2500Mbps link speed
From: Andrew Lunn @ 2019-02-05 14:56 UTC (permalink / raw)
  To: abhijit; +Cc: netdev
In-Reply-To: <496b56c8-666a-7c5c-f3c6-0e709584ec0d@gmail.com>

On Tue, Feb 05, 2019 at 11:39:34AM +0530, abhijit wrote:
> Hi All,
> 
> We are using Ethernet MAC which has integrated Phy. This phy supports speed
> up to 10000Mbps. The phy has, 1000Base-X PCS(Physical Coding Sub-layer)
> followed by SerDes interface to support 10Mbps to 10000Mbps. Currently we
> are trying to get this phy at 2500Mbps. This device has 16 registers that
> corresponds to Clause 37, which can be used to advertise speed till
> 1000Mbps.
> So my question is,
> 1. Without phy advertising its capability of 2500Mbps, is there any way I
> can set phy speed at 2500Mbps?
> 2. I tried disabling auto-negotiation and setting speed at 2500Mbps with
> ethtool (ethtool -s eth0  speed 2500 autoneg off), but the ethtool reported
> this configuration as invalid?
> 3. At the end we are targeting print of "link up (2500/Full)"

Hi Abhijit

It all depends on what the PHY driver can do. It sounds like the PHY
driver does not support multi-gige speeds. So you probably need to
work on the PHY driver and add support for them.

What PHY is it?

     Andrew

^ permalink raw reply

* [PATCH] rhashtable: use irq-safe spinlock in rhashtable_rehash_table()
From: Johannes Berg @ 2019-02-05 14:37 UTC (permalink / raw)
  To: linux-wireless, netdev
  Cc: Jouni Malinen, Thomas Graf, Herbert Xu, Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

When an rhashtabl walk is done from irq/bh context, we rightfully
get a lockdep complaint saying that we could get a (soft-)IRQ in
the middle of a rehash. This happened e.g. in mac80211 as it does
a walk in soft-irq context.

Fix this by using irq-safe locking here. We don't need _irqsave()
as we know this will be called only in process context from the
workqueue. We could get away with _bh() but that seems a bit less
generic, though I'm not sure anyone would want to do a walk from
a real IRQ handler.

Reported-by: Jouni Malinen <j@w1.fi>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 lib/rhashtable.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 852ffa5160f1..ad3c1da15475 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -327,10 +327,10 @@ static int rhashtable_rehash_table(struct rhashtable *ht)
 	/* Publish the new table pointer. */
 	rcu_assign_pointer(ht->tbl, new_tbl);
 
-	spin_lock(&ht->lock);
+	spin_lock_irq(&ht->lock);
 	list_for_each_entry(walker, &old_tbl->walkers, list)
 		walker->tbl = NULL;
-	spin_unlock(&ht->lock);
+	spin_unlock_irq(&ht->lock);
 
 	/* Wait for readers. All new readers will see the new
 	 * table, and thus no references to the old table will
-- 
2.17.2


^ permalink raw reply related

* Re: [PATCH net-next v3 16/16] net: sched: unlock rules update API
From: Jiri Pirko @ 2019-02-05 13:31 UTC (permalink / raw)
  To: Vlad Buslov; +Cc: netdev, jhs, xiyou.wangcong, davem, ast, daniel
In-Reply-To: <20190204123301.4223-17-vladbu@mellanox.com>

Mon, Feb 04, 2019 at 01:33:01PM CET, vladbu@mellanox.com wrote:
>Register netlink protocol handlers for message types RTM_NEWTFILTER,
>RTM_DELTFILTER, RTM_GETTFILTER as unlocked. Set rtnl_held variable that
>tracks rtnl mutex state to be false by default.
>
>Introduce tcf_proto_is_unlocked() helper that is used to check
>tcf_proto_ops->flag to determine if ops can be called without taking rtnl
>lock. Manually lookup Qdisc, class and block in rule update handlers.
>Verify that both Qdisc ops and proto ops are unlocked before using any of
>their callbacks, and obtain rtnl lock otherwise.
>
>Signed-off-by: Vlad Buslov <vladbu@mellanox.com>

Acked-by: Jiri Pirko <jiri@mellanox.com>

^ permalink raw reply

* Re: [PATCH net-next v3 15/16] net: sched: refactor tcf_block_find() into standalone functions
From: Jiri Pirko @ 2019-02-05 13:30 UTC (permalink / raw)
  To: Vlad Buslov; +Cc: netdev, jhs, xiyou.wangcong, davem, ast, daniel
In-Reply-To: <20190204123301.4223-16-vladbu@mellanox.com>

Mon, Feb 04, 2019 at 01:33:00PM CET, vladbu@mellanox.com wrote:
>Refactor tcf_block_find() code into three standalone functions:
>- __tcf_qdisc_find() to lookup Qdisc and increment its reference counter.
>- __tcf_qdisc_cl_find() to lookup class.
>- __tcf_block_find() to lookup block and increment its reference counter.
>
>This change is necessary to allow netlink tc rule update handlers to call
>these functions directly in order to conditionally take rtnl lock
>according to Qdisc class ops flags before calling any of class ops
>functions.
>
>Signed-off-by: Vlad Buslov <vladbu@mellanox.com>

Acked-by: Jiri Pirko <jiri@mellanox.com>

^ permalink raw reply

* Re: [PATCH net-next v3 14/16] net: sched: add flags to Qdisc class ops struct
From: Jiri Pirko @ 2019-02-05 13:29 UTC (permalink / raw)
  To: Vlad Buslov; +Cc: netdev, jhs, xiyou.wangcong, davem, ast, daniel
In-Reply-To: <20190204123301.4223-15-vladbu@mellanox.com>

Mon, Feb 04, 2019 at 01:32:59PM CET, vladbu@mellanox.com wrote:
>Extend Qdisc_class_ops with flags. Create enum to hold possible class ops
>flag values. Add first class ops flags value QDISC_CLASS_OPS_DOIT_UNLOCKED
>to indicate that class ops functions can be called without taking rtnl
>lock.
>
>Signed-off-by: Vlad Buslov <vladbu@mellanox.com>

Acked-by: Jiri Pirko <jiri@mellanox.com>

^ permalink raw reply

* Re: [PATCH net-next v3 13/16] net: sched: extend proto ops to support unlocked classifiers
From: Jiri Pirko @ 2019-02-05 13:29 UTC (permalink / raw)
  To: Vlad Buslov; +Cc: netdev, jhs, xiyou.wangcong, davem, ast, daniel
In-Reply-To: <20190204123301.4223-14-vladbu@mellanox.com>

Mon, Feb 04, 2019 at 01:32:58PM CET, vladbu@mellanox.com wrote:
>Add 'rtnl_held' flag to tcf proto change, delete, destroy, dump, walk
>functions to track rtnl lock status. Extend users of these function in cls
>API to propagate rtnl lock status to them. This allows classifiers to
>obtain rtnl lock when necessary and to pass rtnl lock status to extensions
>and driver offload callbacks.
>
>Add flags field to tcf proto ops. Add flag value to indicate that
>classifier doesn't require rtnl lock.
>
>Signed-off-by: Vlad Buslov <vladbu@mellanox.com>

Acked-by: Jiri Pirko <jiri@mellanox.com>

^ permalink raw reply

* Re: [PATCH net-next v3] net: dsa: mv88e6xxx: Prevent suspend to RAM
From: Andrew Lunn @ 2019-02-05 13:27 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Vivien Didelot, Florian Fainelli, David S. Miller, netdev,
	linux-kernel, Thomas Petazzoni, Gregory Clement, Antoine Tenart,
	Maxime Chevallier, Nadav Haklai
In-Reply-To: <20190205110728.11451-1-miquel.raynal@bootlin.com>

On Tue, Feb 05, 2019 at 12:07:28PM +0100, Miquel Raynal wrote:
> On one hand, the mv88e6xxx driver has a work queue called in loop
> which will attempt register accesses after MDIO bus suspension, that
> entirely freezes the platform during suspend.
> 
> On the other hand, the DSA core is not ready yet to support suspend to
> RAM operation because so far there is no way to recover reliably the
> switch configuration.
> 
> To avoid the kernel to freeze when suspending with a switch driven by
> the mv88e6xxx driver, we choose to prevent the driver suspension and
> in the same way, the whole platform.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply

* Re: [PATCH net-next v3 12/16] net: sched: extend proto ops with 'put' callback
From: Jiri Pirko @ 2019-02-05 13:15 UTC (permalink / raw)
  To: Vlad Buslov; +Cc: netdev, jhs, xiyou.wangcong, davem, ast, daniel
In-Reply-To: <20190204123301.4223-13-vladbu@mellanox.com>

Mon, Feb 04, 2019 at 01:32:57PM CET, vladbu@mellanox.com wrote:
>Add optional tp->ops->put() API to be implemented for filter reference
>counting. This new function is called by cls API to release filter
>reference for filters returned by tp->ops->change() or tp->ops->get()
>functions. Implement tfilter_put() helper to call tp->ops->put() only for
>classifiers that implement it.
>
>Signed-off-by: Vlad Buslov <vladbu@mellanox.com>

Acked-by: Jiri Pirko <jiri@mellanox.com>

^ permalink raw reply

* Re: [PATCH] bpf: test_maps: Avoid possible out of bound access
From: Breno Leitao @ 2019-02-05 13:23 UTC (permalink / raw)
  To: Daniel Borkmann, netdev; +Cc: ast
In-Reply-To: <c42914fc-503f-abb9-b7e9-8f7360c9587a@iogearbox.net>

Hi Daniel,

On 2/4/19 6:57 PM, Daniel Borkmann wrote:
> There are couple more test_*() functions that need to be converted if we do
> the change to unsigned:
> 
> tools/testing/selftests/bpf/test_maps.c:48:static void test_hashmap(int task, void *data)
> tools/testing/selftests/bpf/test_maps.c:138:static void test_hashmap_sizes(int task, void *data)
> tools/testing/selftests/bpf/test_maps.c:158:static void test_hashmap_percpu(int task, void *data)
> tools/testing/selftests/bpf/test_maps.c:285:static void test_hashmap_walk(int task, void *data)
> tools/testing/selftests/bpf/test_maps.c:356:static void test_arraymap(int task, void *data)
> tools/testing/selftests/bpf/test_maps.c:411:static void test_arraymap_percpu(int task, void *data)
> tools/testing/selftests/bpf/test_maps.c:507:static void test_devmap(int task, void *data)
> tools/testing/selftests/bpf/test_maps.c:522:static void test_queuemap(int task, void *data)
> tools/testing/selftests/bpf/test_maps.c:580:static void test_stackmap(int task, void *data)
> tools/testing/selftests/bpf/test_maps.c:645:static void test_sockmap(int tasks, void *data)

That is a good idea. I haven't change them because the 'task' argument was
not being used at all, hence no GCC warning also. But due to the
__run_parallel() function parameter, I agree that we need to change them all.

Also, we need to change the test_update_delete(unsigned int fn, void *data)
function, which is also called through __run_parallel():

	run_parallel(TASKS, test_update_delete, data);

>>  {
>>  	struct bpf_map *bpf_map_rx, *bpf_map_tx, *bpf_map_msg, *bpf_map_break;
>>  	int map_fd_msg = 0, map_fd_rx = 0, map_fd_tx = 0, map_fd_break;
>> @@ -1261,7 +1261,7 @@ static void test_map_large(void)
>>  	printf("Fork %d tasks to '" #FN "'\n", N); \
>>  	__run_parallel(N, FN, DATA)
>>  
>> -static void __run_parallel(int tasks, void (*fn)(int task, void *data),
>> +static void __run_parallel(unsigned int tasks, void (*fn)(int task, void *data),
> 
> This would also need conversion to unsigned for the func arg above so that
> we don't type mismatch.

Ack!

I am sending a V2 soon. Than you!

^ permalink raw reply

* Re: [PATCH net-next v3 11/16] net: sched: track rtnl lock status when validating extensions
From: Jiri Pirko @ 2019-02-05 13:09 UTC (permalink / raw)
  To: Vlad Buslov; +Cc: netdev, jhs, xiyou.wangcong, davem, ast, daniel
In-Reply-To: <20190204123301.4223-12-vladbu@mellanox.com>

Mon, Feb 04, 2019 at 01:32:56PM CET, vladbu@mellanox.com wrote:
>Actions API is already updated to not rely on rtnl lock for
>synchronization. However, it need to be provided with rtnl status when
>called from classifiers API in order to be able to correctly release the
>lock when loading kernel module.
>
>Extend extension validation function with 'rtnl_held' flag which is passed
>to actions API. Add new 'rtnl_held' parameter to tcf_exts_validate() in cls
>API. No classifier is currently updated to support unlocked execution, so
>pass hardcoded 'true' flag parameter value.
>
>Signed-off-by: Vlad Buslov <vladbu@mellanox.com>

Acked-by: Jiri Pirko <jiri@mellanox.com>

^ permalink raw reply

* Re: [PATCH net-next v3 10/16] net: sched: prevent insertion of new classifiers during chain flush
From: Jiri Pirko @ 2019-02-05 13:08 UTC (permalink / raw)
  To: Vlad Buslov; +Cc: netdev, jhs, xiyou.wangcong, davem, ast, daniel
In-Reply-To: <20190204123301.4223-11-vladbu@mellanox.com>

Mon, Feb 04, 2019 at 01:32:55PM CET, vladbu@mellanox.com wrote:
>Extend tcf_chain with 'flushing' flag. Use the flag to prevent insertion of
>new classifier instances when chain flushing is in progress in order to
>prevent resource leak when tcf_proto is created by unlocked users
>concurrently.
>
>Return EAGAIN error from tcf_chain_tp_insert_unique() to restart
>tc_new_tfilter() and lookup the chain/proto again.
>
>Signed-off-by: Vlad Buslov <vladbu@mellanox.com>

Acked-by: Jiri Pirko <jiri@mellanox.com>

^ permalink raw reply

* Re: [PATCH net-next v3 09/16] net: sched: refactor tp insert/delete for concurrent execution
From: Jiri Pirko @ 2019-02-05 13:08 UTC (permalink / raw)
  To: Vlad Buslov; +Cc: netdev, jhs, xiyou.wangcong, davem, ast, daniel
In-Reply-To: <20190204123301.4223-10-vladbu@mellanox.com>

Mon, Feb 04, 2019 at 01:32:54PM CET, vladbu@mellanox.com wrote:
>Implement unique insertion function to atomically attach tcf_proto to chain
>after verifying that no other tcf proto with specified priority exists.
>Implement delete function that verifies that tp is actually empty before
>deleting it. Use these functions to refactor cls API to account for
>concurrent tp and rule update instead of relying on rtnl lock. Add new
>'deleting' flag to tcf proto. Use it to restart search when iterating over
>tp's on chain to prevent accessing potentially inval tp->next pointer.
>
>Extend tcf proto with spinlock that is intended to be used to protect its
>data from concurrent modification instead of relying on rtnl mutex. Use it
>to protect 'deleting' flag. Add lockdep macros to validate that lock is
>held when accessing protected fields.
>
>Signed-off-by: Vlad Buslov <vladbu@mellanox.com>

Acked-by: Jiri Pirko <jiri@mellanox.com>

^ permalink raw reply

* Re: [PATCH net-next v3 08/16] net: sched: traverse classifiers in chain with tcf_get_next_proto()
From: Jiri Pirko @ 2019-02-05 13:07 UTC (permalink / raw)
  To: Vlad Buslov; +Cc: netdev, jhs, xiyou.wangcong, davem, ast, daniel
In-Reply-To: <20190204123301.4223-9-vladbu@mellanox.com>

Mon, Feb 04, 2019 at 01:32:53PM CET, vladbu@mellanox.com wrote:
>All users of chain->filters_chain rely on rtnl lock and assume that no new
>classifier instances are added when traversing the list. Use
>tcf_get_next_proto() to traverse filters list without relying on rtnl
>mutex. This function iterates over classifiers by taking reference to
>current iterator classifier only and doesn't assume external
>synchronization of filters list.
>
>Signed-off-by: Vlad Buslov <vladbu@mellanox.com>

Acked-by: Jiri Pirko <jiri@mellanox.com>

^ permalink raw reply

* Re: [PATCH v3 2/2] netdev/phy: add MDIO bus multiplexer driven by a regmap
From: Andrew Lunn @ 2019-02-05 13:13 UTC (permalink / raw)
  To: Pankaj Bansal; +Cc: Florian Fainelli, netdev@vger.kernel.org
In-Reply-To: <20190205153014.3807-3-pankaj.bansal@nxp.com>

> diff --git a/include/linux/mdio-mux.h b/include/linux/mdio-mux.h
> index a5d58f221939..bb5d0e2774bf 100644
> --- a/include/linux/mdio-mux.h
> +++ b/include/linux/mdio-mux.h
> @@ -29,4 +29,38 @@ int mdio_mux_init(struct device *dev,
>  
>  void mdio_mux_uninit(void *mux_handle);
>  
> +#if defined(CONFIG_MDIO_BUS_MUX_REGMAP) || \
> +    defined(CONFIG_MDIO_BUS_MUX_REGMAP_MODULE)

Please use IS_ENABLED().

       Andrew

^ permalink raw reply

* Re: [PATCH v1] net: dsa: qca8k: implement DT-based ports <-> phy translation
From: Andrew Lunn @ 2019-02-05 13:09 UTC (permalink / raw)
  To: Christian Lamparter; +Cc: netdev, Florian Fainelli, Vivien Didelot
In-Reply-To: <24213401.HFHtFJUkrt@debian64>

> The trick here is that priv->bus is not the internal
> bus instead it's set to the SoC's (external) mii bus in qca8k_sw_probe()).
> So this isn't a slave bus! And as stated in the qca8k, the the external
> and internal PHY bus are not mapped 1:1.
> 
> >From what I can tell from the datasheet, the QCA8337N does have
> dedicated MDIO master control register which is what is
> needed here.

Ah, O.K, i was missing that bit. So yes, export an MDIO bus to Linux,
as the mv88e6xxx does.

   Andrew

^ permalink raw reply

* Re: [PATCH v1] net: dsa: qca8k: implement DT-based ports <-> phy translation
From: Christian Lamparter @ 2019-02-05 12:48 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev, Florian Fainelli, Vivien Didelot
In-Reply-To: <20190205024533.GB10838@lunn.ch>

On Tuesday, February 5, 2019 3:45:33 AM CET Andrew Lunn wrote:
> On Mon, Feb 04, 2019 at 10:35:55PM +0100, Christian Lamparter wrote:
> > The QCA8337 enumerates 5 PHYs on the MDC/MDIO access: PHY0-PHY4.
> > Based on the System Block Diagram in Section 1.2 of the
> > QCA8337's datasheet. These PHYs are internally connected
> > to MACs of PORT 1 - PORT 5. However, neither qca8k's slave
> > mdio access functions qca8k_phy_read()/qca8k_phy_write()
> > nor the dsa framework is set up for that.
> > 
> > This version of the patch uses the existing phy-handle
> > properties of each specified DSA Port in the DT to map
> > each PORT/MAC to its exposed PHY on the MDIO bus. This
> > is supported by the current binding document qca8k.txt
> > as well.
> 
> Hi Christian
> 
> Looking at Documentation/devicetree/bindings/net/dsa/qca8k.txt 
> 
> I think everything you need is already implemented. What problem do
> you actually have?

Thankfully, Florian's reply answered the question to me. The problem
has to do with the qca8k's slave qca8k_phy_read and qca8k_phy_write.
Here are the functions:

|qca8k_phy_read(struct dsa_switch *ds, int phy, int regnum)
|{
|	struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
|
|	return mdiobus_read(priv->bus, phy, regnum);
|}
|
|static int
|qca8k_phy_write(struct dsa_switch *ds, int phy, int regnum, u16 val)
|{
|	struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
|
|	return mdiobus_write(priv->bus, phy, regnum, val);
|}

The trick here is that priv->bus is not the internal
bus instead it's set to the SoC's (external) mii bus in qca8k_sw_probe()).
So this isn't a slave bus! And as stated in the qca8k, the the external
and internal PHY bus are not mapped 1:1.

From what I can tell from the datasheet, the QCA8337N does have
dedicated MDIO master control register which is what is
needed here. It's at 0x003C. So these mdiobus_read/writes on the
SoCs MDIO bus would either need to be converted to read and write
to 0x003c... Or the qca8k_phy_read() and qca8k_phy_write could be
dropped all together. I've tested it on the WPQ864 and driver
works without since it was never a real slave bus there to begin with.

Thanks,
Christian
---
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 7e97e620bd44..a26850c888cf 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -620,22 +620,6 @@ qca8k_adjust_link(struct dsa_switch *ds, int port, struct phy_device *phy)
 	qca8k_port_set_status(priv, port, 1);
 }
 
-static int
-qca8k_phy_read(struct dsa_switch *ds, int phy, int regnum)
-{
-	struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
-
-	return mdiobus_read(priv->bus, phy, regnum);
-}
-
-static int
-qca8k_phy_write(struct dsa_switch *ds, int phy, int regnum, u16 val)
-{
-	struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
-
-	return mdiobus_write(priv->bus, phy, regnum, val);
-}
-
 static void
 qca8k_get_strings(struct dsa_switch *ds, int port, u32 stringset, uint8_t *data)
 {
@@ -876,8 +860,6 @@ static const struct dsa_switch_ops qca8k_switch_ops = {
 	.setup			= qca8k_setup,
 	.adjust_link            = qca8k_adjust_link,
 	.get_strings		= qca8k_get_strings,
-	.phy_read		= qca8k_phy_read,
-	.phy_write		= qca8k_phy_write,
 	.get_ethtool_stats	= qca8k_get_ethtool_stats,
 	.get_sset_count		= qca8k_get_sset_count,
 	.get_mac_eee		= qca8k_get_mac_eee,








^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox