Netdev List
 help / color / mirror / Atom feed
* [net-next 07/11] net/mlx5e: Add missing static function annotation
From: Saeed Mahameed @ 2019-02-19 22:19 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Ariel Almog, Leon Romanovsky, Saeed Mahameed
In-Reply-To: <20190219221957.2899-1-saeedm@mellanox.com>

From: Leon Romanovsky <leonro@mellanox.com>

Compilation with W=1 produces following warning:

drivers/net/ethernet/mellanox/mlx5/core/en/monitor_stats.c:69:6:
warning: no previous prototype for _mlx5e_monitor_counter_start_ [-Wmissing-prototypes]
 void mlx5e_monitor_counter_start(struct mlx5e_priv *priv)
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~

Avoid it by declaring mlx5e_monitor_counter_start() as a static function.

Fixes: 5c7e8bbb0257 ("net/mlx5e: Use monitor counters for update stats")
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en/monitor_stats.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/monitor_stats.c b/drivers/net/ethernet/mellanox/mlx5/core/en/monitor_stats.c
index 2ce420851e77..7cd5b02e0f10 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/monitor_stats.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/monitor_stats.c
@@ -66,7 +66,7 @@ static int mlx5e_monitor_event_handler(struct notifier_block *nb,
 	return NOTIFY_OK;
 }
 
-void mlx5e_monitor_counter_start(struct mlx5e_priv *priv)
+static void mlx5e_monitor_counter_start(struct mlx5e_priv *priv)
 {
 	MLX5_NB_INIT(&priv->monitor_counters_nb, mlx5e_monitor_event_handler,
 		     MONITOR_COUNTER);
-- 
2.20.1


^ permalink raw reply related

* [net-next 09/11] net/mlx5e: Remove unused variable ‘esw’
From: Saeed Mahameed @ 2019-02-19 22:19 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Ariel Almog, Saeed Mahameed
In-Reply-To: <20190219221957.2899-1-saeedm@mellanox.com>

Fix the following compiler warning:

drivers/net/ethernet/mellanox/mlx5/core/en_tc.c:2770:
warning: unused variable ‘esw’ [-Wunused-variable]

Fixes: 1cd3ab86b713 ("net/mlx5e: Introduce mlx5e_flow_esw_attr_init() helper")
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index 0b3cd3f7f18a..b38986e18dd7 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -2767,7 +2767,6 @@ __mlx5e_add_fdb_flow(struct mlx5e_priv *priv,
 {
 	struct flow_rule *rule = tc_cls_flower_offload_flow_rule(f);
 	struct netlink_ext_ack *extack = f->common.extack;
-	struct mlx5_eswitch *esw = priv->mdev->priv.eswitch;
 	struct mlx5e_tc_flow_parse_attr *parse_attr;
 	struct mlx5e_tc_flow *flow;
 	int attr_size, err;
-- 
2.20.1


^ permalink raw reply related

* [net-next 08/11] net/mlx5: Delete unused FPGA QPN variable
From: Saeed Mahameed @ 2019-02-19 22:19 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Ariel Almog, Leon Romanovsky, Saeed Mahameed
In-Reply-To: <20190219221957.2899-1-saeedm@mellanox.com>

From: Leon Romanovsky <leonro@mellanox.com>

fpga_qpn was assigned but never used and compilation with W=1
produced the following warning:

drivers/net/ethernet/mellanox/mlx5/core/fpga/core.c: In function _mlx5_fpga_event_:
drivers/net/ethernet/mellanox/mlx5/core/fpga/core.c:320:6: warning:
variable _fpga_qpn_ set but not used [-Wunused-but-set-variable]
  u32 fpga_qpn;
      ^~~~~~~~

Fixes: 98db16bab59f ("net/mlx5: FPGA, Handle QP error event")
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/fpga/core.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fpga/core.c b/drivers/net/ethernet/mellanox/mlx5/core/fpga/core.c
index 27c5f6c7d36a..d046d1ec2a86 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fpga/core.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fpga/core.c
@@ -317,7 +317,6 @@ static int mlx5_fpga_event(struct mlx5_fpga_device *fdev,
 	const char *event_name;
 	bool teardown = false;
 	unsigned long flags;
-	u32 fpga_qpn;
 	u8 syndrome;
 
 	switch (event) {
@@ -328,7 +327,6 @@ static int mlx5_fpga_event(struct mlx5_fpga_device *fdev,
 	case MLX5_EVENT_TYPE_FPGA_QP_ERROR:
 		syndrome = MLX5_GET(fpga_qp_error_event, data, syndrome);
 		event_name = mlx5_fpga_qp_syndrome_to_string(syndrome);
-		fpga_qpn = MLX5_GET(fpga_qp_error_event, data, fpga_qpn);
 		break;
 	default:
 		return NOTIFY_DONE;
-- 
2.20.1


^ permalink raw reply related

* [net-next 10/11] net/mlx5: E-Switch, Fix the warning on vport index out of range
From: Saeed Mahameed @ 2019-02-19 22:19 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Ariel Almog, Bodong Wang, Saeed Mahameed
In-Reply-To: <20190219221957.2899-1-saeedm@mellanox.com>

From: Bodong Wang <bodong@mellanox.com>

When eswitch gets vport data structure, the index should not be out
of the range of the vport array. Driver mistakenly used vport number
to check the range.

Fixes: 22b8ddc86bf4 ("net/mlx5: E-Switch, Assign a different position for uplink rep and vport")
Signed-off-by: Bodong Wang <bodong@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/eswitch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
index e18af31336e6..d4f6859bf58c 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
@@ -87,7 +87,7 @@ static struct mlx5_vport *mlx5_eswitch_get_vport(struct mlx5_eswitch *esw,
 {
 	u16 idx = mlx5_eswitch_vport_num_to_index(esw, vport_num);
 
-	WARN_ON(vport_num > esw->total_vports - 1);
+	WARN_ON(idx > esw->total_vports - 1);
 	return &esw->vports[idx];
 }
 
-- 
2.20.1


^ permalink raw reply related

* [net-next 11/11] net/mlx5: E-Switch, Disable esw manager vport correctly
From: Saeed Mahameed @ 2019-02-19 22:19 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Ariel Almog, Bodong Wang, Eli Britstein, Or Gerlitz,
	Saeed Mahameed
In-Reply-To: <20190219221957.2899-1-saeedm@mellanox.com>

From: Bodong Wang <bodong@mellanox.com>

When disabling vport, relevant vport configurations will be cleaned
up. These cleanups should be done to the vports which had these configs
applied at vport enablement. As esw manager vport didn't have such
vport config applied, cleanup should not touch it.

Fixes: de9e6a8136c5 ("net/mlx5: E-Switch, Properly refer to host PF vport as other vport")
Signed-off-by: Bodong Wang <bodong@mellanox.com>
Reported-by: Eli Britstein <elibr@mellanox.com>
Reviewed-by: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/eswitch.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
index d4f6859bf58c..6cb9710f76d5 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
@@ -1606,7 +1606,8 @@ static void esw_disable_vport(struct mlx5_eswitch *esw,
 	esw_vport_change_handle_locked(vport);
 	vport->enabled_events = 0;
 	esw_vport_disable_qos(esw, vport_num);
-	if (esw->mode == SRIOV_LEGACY) {
+	if (esw->manager_vport != vport_num &&
+	    esw->mode == SRIOV_LEGACY) {
 		mlx5_modify_vport_admin_state(esw->dev,
 					      MLX5_VPORT_STATE_OP_MOD_ESW_VPORT,
 					      vport_num, 1,
-- 
2.20.1


^ permalink raw reply related

* linux-next: Fixes tag needs some work in the net-next tree
From: Stephen Rothwell @ 2019-02-19 22:31 UTC (permalink / raw)
  To: David Miller, Networking
  Cc: Linux Next Mailing List, Linux Kernel Mailing List, Vinod Koul,
	Andrew Lunn

[-- Attachment #1: Type: text/plain, Size: 365 bytes --]

Hi all,

In commit

  a968b5e9d587 ("net: dsa: qca8k: Enable delay for RGMII_ID mode")

Fixes tag

  Fixes: 40269aa9f40a ("net: dsa: qca8k: disable delay for RGMII mode")

has these problem(s):

  - Target SHA1 does not exist

Did you mean:

  Fixes: 5ecdd77c61c8 ("net: dsa: qca8k: disable delay for RGMII mode")

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH bpf-next 7/9] bpf: Sample NRM BPF program to limit egress bw
From: Lawrence Brakmo @ 2019-02-19 22:31 UTC (permalink / raw)
  To: Eric Dumazet, netdev; +Cc: Martin Lau, Alexei Starovoitov, Daniel Borkmann
In-Reply-To: <b390abb6-71e7-2c96-96f6-8fe372fc4500@gmail.com>

On 2/19/19, 10:29 AM, "netdev-owner@vger.kernel.org on behalf of Eric Dumazet" <netdev-owner@vger.kernel.org on behalf of eric.dumazet@gmail.com> wrote:
    
    
    On 02/18/2019 09:38 PM, brakmo wrote:
    
    > +
    > +static __always_inline void get_nrm_pkt_info(struct bpf_sock *sk,
    > +					     struct nrm_pkt_info *pkti)
    > +{
    > +	if (sk->family == AF_INET6 || sk->family == AF_INET) {
    > +		pkti->is_ip = true;
    > +		pkti->is_tcp = (sk->protocol == IPPROTO_TCP);
    > +		if (pkti->is_tcp) {
    > +			struct bpf_tcp_sock *tp;
    > +
    > +			tp = bpf_tcp_sock(sk);
    > +			if (tp)
    > +				pkti->ecn = tp->ecn_flags & TCP_ECN_OK;
    > +			else
    > +				pkti->ecn = 0;
    > +		} else {
    > +			pkti->ecn = 0;
    > +		}
    > +	} else {
    > +		pkti->is_ip = false;
    > +		pkti->is_tcp = false;
    > +		pkti->ecn = 0;
    > +	}
    > +}
    > 
    
    This looks very strange.
    
    ECN capability is per packet, and does not need access to the original
    TCP socket really.

    We definitely can use ECN with UDP packets.
    
    IMO this sample looks like a work in progress.

This sample NRM program focuses on TCP, I should have made that more explicit. I originally was checking the ECN bits on the packet, but someone pointed out that since I was focusing on TCP, it would be more efficient to just look at the TCP state (saving having to read the packet header). However, your point is correct in that I could be wrongly marking packets that I should not mark, such as pure ACKs. I will go back to the previous version that looks at the ECN bits in the header and not limit ECN marking to just TCP.
    
    EDT model allows to implement full shaping (not only virtual one)
    by twaking/advancing skb->tstamp 
    (see commit f11216b24219a bpf: add skb->tstamp r/w access from tc clsact and cg skb progs)
    
I have a version of an NRM BPF program that uses EDT, so I know what you mean. However, I decided to send it as a separate patch (including an ingress sample program).

    Implementing shaping with the need of accessing TCP sockets seems a layering violation,
    a lot of things can go wrong with this model.
    For instance, you wont be able to offload this.

On the other hand, there are also advantages to having access to the TCP socket. For example, one has more tools to affect the TCP rate (like the proposed helper bpf_tcp_enter_cwr() which does not require dropping the packet). We will need more experience to fully understand the tradeoffs between the various implementations of rate limiting and/or shaping.

Thank you for your feedback.

    
    


^ permalink raw reply

* [GIT] Networking
From: David Miller @ 2019-02-19 22:33 UTC (permalink / raw)
  To: torvalds; +Cc: akpm, netdev, linux-kernel


1) Fix suspend and resume in mt76x0u USB driver, from Stanislaw Gruszka.

2) Missing memory barriers in xsk, from Magnus Karlsson.

3) rhashtable fixes in mac80211 from Herbert Xu.

4) 32-bit MIPS eBPF JIT fixes from Paul Burton.

5) Fix for_each_netdev_feature() on big endian, from Hauke Mehrtens.

6) GSO validation fixes from Willem de Bruijn.

7) Endianness fix for dwmac4 timestamp handling, from Alexandre
   Torgue.

8) More strict checks in tcp_v4_err(), from Eric Dumazet.

9) af_alg_release should NULL out the sk after the sock_put(),
   from Mao Wenan.

10) Missing unlock in mac80211 mesh error path, from Wei Yongjun.

11) Missing device put in hns driver, from Salil Mehta.

Please pull, thanks a lot!

The following changes since commit 24f0a48743a256bdec1bcb80708bc309da4aa261:

  Merge tag 'for-linus-20190215' of git://git.kernel.dk/linux-block (2019-02-15 09:12:28 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 

for you to fetch changes up to 1765f5dcd00963e33f1b8a4e0f34061fbc0e2f7f:

  sky2: Increase D3 delay again (2019-02-19 14:16:41 -0800)

----------------------------------------------------------------
Alexander Duyck (2):
      mm: Use fixed constant in page_frag_alloc instead of size + 1
      net: Do not allocate page fragments that are not skb aligned

Alexandre Torgue (1):
      net: stmmac: handle endianness in dwmac4_get_timestamp

Alexei Starovoitov (1):
      bpf: fix lockdep false positive in stackmap

Alexey Khoroshilov (1):
      net: mv643xx_eth: disable clk on error path in mv643xx_eth_shared_probe()

Andrea Claudi (1):
      ipvs: fix warning on unused variable

Beniamino Galvani (1):
      qmi_wwan: apply SET_DTR quirk to Sierra WP7607

Björn Töpel (1):
      xsk: do not remove umem from netdevice on fall-back to copy-mode

Colin Ian King (1):
      net/mlx4_en: fix spelling mistake: "quiting" -> "quitting"

David Chen (1):
      r8152: Add support for MAC address pass through on RTL8153-BD

David S. Miller (9):
      Merge tag 'mac80211-for-davem-2019-02-15' of git://git.kernel.org/.../jberg/mac80211
      Merge branch 'net-dsa-b53-VLAN-and-L2-fixes'
      net: Add header for usage of fls64()
      Merge git://git.kernel.org/.../bpf/bpf
      Merge branch 'tcp-fix-possible-crash-in-tcp_v4_err'
      Merge branch 'netdev-page_frag_alloc-fixes'
      Merge branch 'qed-iWARP'
      Merge tag 'wireless-drivers-for-davem-2019-02-18' of git://git.kernel.org/.../kvalo/wireless-drivers
      Merge git://git.kernel.org/.../pablo/nf

Davidlohr Bueso (1):
      xsk: share the mmap_sem for page pinning

Eric Dumazet (2):
      tcp: clear icsk_backoff in tcp_write_queue_purge()
      tcp: tcp_v4_err() should be more careful

Florian Fainelli (5):
      net: dsa: b53: Fix default VLAN ID
      net: dsa: b53: Properly account for VLAN filtering
      net: systemport: Fix reception of BPDUs
      net: dsa: bcm_sf2: Do not assume DSA master supports WoL
      net: dsa: b53: Do not program CPU port's PVID

Hauke Mehrtens (1):
      net: Fix for_each_netdev_feature on Big endian

Herbert Xu (2):
      mac80211: Use linked list instead of rhashtable walk for mesh tables
      mac80211: Free mpath object when rhashtable insertion fails

Jason Wang (1):
      vhost: correctly check the return value of translate_desc() in log_used()

Jose Abreu (1):
      net: stmmac: Fix a race in EEE enable callback

Kai-Heng Feng (1):
      sky2: Increase D3 delay again

Lorenzo Bianconi (1):
      net: ip6_gre: initialize erspan_ver just for erspan tunnels

Magnus Karlsson (1):
      xsk: add missing smp_rmb() in xsk_mmap

Mao Wenan (1):
      net: crypto set sk to NULL when af_alg_release.

Martin KaFai Lau (1):
      bpf: Fix narrow load on a bpf_sock returned from sk_lookup()

Michal Kalderon (2):
      qed: Fix iWARP buffer size provided for syn packet processing.
      qed: Fix iWARP syn packet mac address validation.

Murali Karicheri (1):
      net: netcp: Fix ethss driver probe issue

Pablo Neira Ayuso (1):
      netfilter: nf_tables: fix flush after rule deletion in the same batch

Paul Burton (2):
      MIPS: eBPF: Always return sign extended 32b values
      MIPS: eBPF: Remove REG_32BIT_ZERO_EX

Paul Kocialkowski (1):
      net: phy: xgmiitorgmii: Support generic PHY status read

Petr Machata (1):
      mlxsw: __mlxsw_sp_port_headroom_set(): Fix a use of local variable

Petr Vorel (1):
      doc: Mention MSG_ZEROCOPY implementation for UDP

Rakesh Pillai (1):
      mac80211: Restore vif beacon interval if start ap fails

Salil Mehta (1):
      net: hns: Fixes the missing put_device in positive leg for roce reset

Stanislaw Gruszka (1):
      mt76x0u: fix suspend/resume

Wei Yongjun (1):
      mac80211: mesh: fix missing unlock on error in table_path_del()

Willem de Bruijn (2):
      bpf: only adjust gso_size on bytestream protocols
      net: validate untrusted gso packets without csum offload

 Documentation/networking/msg_zerocopy.rst            |   2 +-
 arch/mips/net/ebpf_jit.c                             |  24 +++++++-------
 crypto/af_alg.c                                      |   4 ++-
 drivers/net/dsa/b53/b53_common.c                     |  90 ++++++++++++++++++++++++++++++++++++++++-----------
 drivers/net/dsa/b53/b53_priv.h                       |   3 ++
 drivers/net/dsa/bcm_sf2.c                            |  10 +++---
 drivers/net/ethernet/broadcom/bcmsysport.c           |   4 +++
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c   |   3 ++
 drivers/net/ethernet/marvell/mv643xx_eth.c           |   7 +++-
 drivers/net/ethernet/marvell/sky2.c                  |   2 +-
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c       |   2 +-
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c       |  12 ++++---
 drivers/net/ethernet/qlogic/qed/qed_iwarp.c          |  21 ++++++++----
 drivers/net/ethernet/qlogic/qed/qed_iwarp.h          |   1 -
 drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c   |   9 ++++--
 drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c |  22 +++++++------
 drivers/net/ethernet/ti/netcp_core.c                 |   2 +-
 drivers/net/phy/xilinx_gmii2rgmii.c                  |   5 ++-
 drivers/net/usb/qmi_wwan.c                           |   4 +--
 drivers/net/usb/r8152.c                              |   5 +--
 drivers/net/wireless/mediatek/mt76/mt76x0/usb.c      |  46 ++++++++++++++++----------
 drivers/vhost/vhost.c                                |   2 +-
 include/linux/netdev_features.h                      |  24 ++++++++++++--
 include/linux/skbuff.h                               |   8 ++++-
 include/linux/virtio_net.h                           |   9 ++++++
 kernel/bpf/stackmap.c                                |   8 ++++-
 kernel/bpf/verifier.c                                |  11 ++++---
 mm/page_alloc.c                                      |   8 ++---
 net/core/dev.c                                       |   4 +--
 net/core/filter.c                                    |  12 +++----
 net/core/skbuff.c                                    |   4 +++
 net/ipv4/tcp.c                                       |   2 +-
 net/ipv4/tcp_ipv4.c                                  |   5 ++-
 net/ipv6/ip6_gre.c                                   |  34 ++++++++++++--------
 net/mac80211/cfg.c                                   |   6 +++-
 net/mac80211/mesh.h                                  |   6 ++++
 net/mac80211/mesh_pathtbl.c                          | 157 +++++++++++++++++++++++++++--------------------------------------------------------------
 net/netfilter/ipvs/ip_vs_ctl.c                       |   3 +-
 net/netfilter/nf_tables_api.c                        |   3 ++
 net/xdp/xdp_umem.c                                   |  11 ++++---
 net/xdp/xsk.c                                        |   4 +++
 41 files changed, 355 insertions(+), 244 deletions(-)

^ permalink raw reply

* Zdravstvujte vas interesuyut klientskie bazy dannyh?
From: netdev @ 2019-02-19 17:55 UTC (permalink / raw)
  To: netdev

Zdravstvujte vas interesuyut klientskie bazy dannyh?



^ permalink raw reply

* Re: [PATCH net-next v2 2/3] net: dsa: mv88e6xxx: add support for bridge flags
From: Russell King - ARM Linux admin @ 2019-02-19 22:52 UTC (permalink / raw)
  To: Vivien Didelot
  Cc: Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S. Miller,
	netdev
In-Reply-To: <20190219145627.GB27596@t480s.localdomain>

On Tue, Feb 19, 2019 at 02:56:27PM -0500, Vivien Didelot wrote:
> Hi Russell,
> 
> On Tue, 19 Feb 2019 19:10:16 +0000, Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
> > > True, let's stick with ops->port_egress_flood(ds, port, bool uc, bool mc).
> > > I do not think that it is necessary to add support for BR_BCAST_FLOOD yet,
> > > we can extend this routine later if we need to.
> > > 
> > > Your dsa_port_bridge_flags() core function can notify the understood
> > > features. This will allow us to scope the support of the bridge flags in
> > > the core, and preventing the drivers to do that themselves.
> > 
> > So, if we have ops->port_egress_flood, then we tell bridge that
> > we support BR_FLOOD | BR_MCAST_FLOOD, irrespective of whether the
> > bridge actually supports both?
> 
> I would say so yes. If a driver implements port_egress_flood(), this means
> its switch device supports both BR_FLOOD | BR_MCAST_FLOOD.
> 
> I have one concern though. The documentation of mcast_flood for bridge(8)
> says that this flag "controls whether a given port will *be flooded* with
> [unknown] multicast traffic". From this I understand allowing this port to
> *receive* frames with unknown destination addresses. But with mv88e6xxx, we
> program whether the port is allowed to egress a frame that has an unknown
> destination address. Otherwise, it will not go out this port.
> 
> Am I mistaken? If I understood correctly, is it safe to assume it is the
> same thing we are implementing here?

Please look at the net/bridge code to resolve questions such as this.
The relevant code is net/bridge/br_forward.c::br_flood():

void br_flood(struct net_bridge *br, struct sk_buff *skb,
              enum br_pkt_type pkt_type, bool local_rcv, bool local_orig)
{
...
        list_for_each_entry_rcu(p, &br->port_list, list) {
                /* Do not flood unicast traffic to ports that turn it off, nor
                 * other traffic if flood off, except for traffic we originate
                 */
                switch (pkt_type) {
                case BR_PKT_UNICAST:
                        if (!(p->flags & BR_FLOOD))
                                continue;
                        break;
                case BR_PKT_MULTICAST:
                        if (!(p->flags & BR_MCAST_FLOOD) && skb->dev != br->dev)                                continue;
                        break;
                case BR_PKT_BROADCAST:
                        if (!(p->flags & BR_BCAST_FLOOD) && skb->dev != br->dev)                                continue;
                        break;
                }
...
                prev = maybe_deliver(prev, p, skb, local_orig);
        }

So, BR_FLOOD, BR_MCAST_FLOOD and BR_BCAST_FLOOD control whether the
packet of type pkt_type being flooded on the bridge egresses from
port p, where p is each port attached to the bridge.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply

* Re: [PATCH bpf-next 1/9] bpf: Add bpf helper bpf_tcp_enter_cwr
From: Lawrence Brakmo @ 2019-02-19 22:59 UTC (permalink / raw)
  To: Daniel Borkmann, netdev
  Cc: Martin Lau, Alexei Starovoitov, Daniel Borkmann --cc=Kernel Team
In-Reply-To: <c885d84e-9f4d-9cbe-8391-7bff46567118@iogearbox.net>


On 2/19/19, 2:24 AM, "Daniel Borkmann" <daniel@iogearbox.net> wrote:

    On 02/19/2019 06:38 AM, brakmo wrote:
    > This patch adds a new bpf helper BPF_FUNC_tcp_enter_cwr
    > "int bpf_tcp_enter_cwr(struct bpf_tcp_sock *tp)".
    > It is added to BPF_PROG_TYPE_CGROUP_SKB typed bpf_prog
    > which currently can be attached to the ingress and egress
    > path.
    > 
    > This helper makes a tcp_sock enter CWR state.  It can be used
    > by a bpf_prog to manage egress network bandwidth limit per
    > cgroupv2.  A later patch will have a sample program to
    > show how it can be used to limit bandwidth usage per cgroupv2.
    > 
    > Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
    > Signed-off-by: Martin KaFai Lau <kafai@fb.com>
    > ---
    >  include/linux/bpf.h      |  1 +
    >  include/uapi/linux/bpf.h |  9 ++++++++-
    >  kernel/bpf/verifier.c    |  4 ++++
    >  net/core/filter.c        | 14 ++++++++++++++
    >  4 files changed, 27 insertions(+), 1 deletion(-)
    > 
    > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
    > index de18227b3d95..525628c913c9 100644
    > --- a/include/linux/bpf.h
    > +++ b/include/linux/bpf.h
    > @@ -195,6 +195,7 @@ enum bpf_arg_type {
    >  	ARG_PTR_TO_SOCKET,	/* pointer to bpf_sock */
    >  	ARG_PTR_TO_SPIN_LOCK,	/* pointer to bpf_spin_lock */
    >  	ARG_PTR_TO_SOCK_COMMON,	/* pointer to sock_common */
    > +	ARG_PTR_TO_TCP_SOCK,    /* pointer to tcp_sock */
    >  };
    >  
    >  /* type of values returned from helper functions */
    > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
    > index bcdd2474eee7..9e9f4f1a0370 100644
    > --- a/include/uapi/linux/bpf.h
    > +++ b/include/uapi/linux/bpf.h
    > @@ -2359,6 +2359,12 @@ union bpf_attr {
    >   *	Return
    >   *		A **struct bpf_tcp_sock** pointer on success, or NULL in
    >   *		case of failure.
    > + *
    > + * int bpf_tcp_enter_cwr(struct bpf_tcp_sock *tp)
    > + *    Description
    > + *        Make a tcp_sock enter CWR state.
    > + *    Return
    > + *        0
    >   */
    >  #define __BPF_FUNC_MAPPER(FN)		\
    >  	FN(unspec),			\
    > @@ -2457,7 +2463,8 @@ union bpf_attr {
    >  	FN(spin_lock),			\
    >  	FN(spin_unlock),		\
    >  	FN(sk_fullsock),		\
    > -	FN(tcp_sock),
    > +	FN(tcp_sock),			\
    > +	FN(tcp_enter_cwr),
    >  
    >  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
    >   * function eBPF program intends to call
    > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
    > index 1b9496c41383..95fb385c6f3c 100644
    > --- a/kernel/bpf/verifier.c
    > +++ b/kernel/bpf/verifier.c
    > @@ -2424,6 +2424,10 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
    >  			return -EFAULT;
    >  		}
    >  		meta->ptr_id = reg->id;
    > +	} else if (arg_type == ARG_PTR_TO_TCP_SOCK) {
    > +		expected_type = PTR_TO_TCP_SOCK;
    > +		if (type != expected_type)
    > +			goto err_type;
    >  	} else if (arg_type == ARG_PTR_TO_SPIN_LOCK) {
    >  		if (meta->func_id == BPF_FUNC_spin_lock) {
    >  			if (process_spin_lock(env, regno, true))
    > diff --git a/net/core/filter.c b/net/core/filter.c
    > index b584cb42a803..f51c4a781844 100644
    > --- a/net/core/filter.c
    > +++ b/net/core/filter.c
    > @@ -5426,6 +5426,18 @@ static const struct bpf_func_proto bpf_tcp_sock_proto = {
    >  	.arg1_type	= ARG_PTR_TO_SOCK_COMMON,
    >  };
    >  
    > +BPF_CALL_1(bpf_tcp_enter_cwr, struct tcp_sock *, tp)
    > +{
    > +	tcp_enter_cwr((struct sock *)tp);
    
    Is it safe to call in every case, meaning do we always have a icsk_ca_ops
    assigned (e.g. pre-4whs completion)?

The helper, bpf_tcp_enter_cwr, can only be called for an skb belonging to a full tcp socket. The icsk_ca_ops field is initialized by tcp_init_sock, so this should not be an issue. However, it could be called before icsk_ca_ops->init() has been called, so it is probably better to check that the tcp sock is in the established state in the bpf helper.
    
    > +	return 0;
    > +}
    > +
    > +static const struct bpf_func_proto bpf_tcp_enter_cwr_proto = {
    > +	.func        = bpf_tcp_enter_cwr,
    > +	.gpl_only    = false,
    > +	.ret_type    = RET_INTEGER,
    > +	.arg1_type    = ARG_PTR_TO_TCP_SOCK,
    > +};
    >  #endif /* CONFIG_INET */
    >  
    >  bool bpf_helper_changes_pkt_data(void *func)
    > @@ -5585,6 +5597,8 @@ cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
    >  #ifdef CONFIG_INET
    >  	case BPF_FUNC_tcp_sock:
    >  		return &bpf_tcp_sock_proto;
    > +	case BPF_FUNC_tcp_enter_cwr:
    > +		return &bpf_tcp_enter_cwr_proto;
    >  #endif
    >  	default:
    >  		return sk_filter_func_proto(func_id, prog);
    > 
    
    


^ permalink raw reply

* Re: [PATCH iproute2, v2] ip-rule: fix json key "to_tbl" for unspecific rule action
From: Stephen Hemminger @ 2019-02-19 23:22 UTC (permalink / raw)
  To: Thomas Haller; +Cc: netdev
In-Reply-To: <20190219205019.30757-1-thaller@redhat.com>

On Tue, 19 Feb 2019 21:50:19 +0100
Thomas Haller <thaller@redhat.com> wrote:

> The key should not be called "to_tbl" because it is exactly
> not a FR_ACT_TO_TBL action. Change it to "action".
> 
>     # ip rule add blackhole
>     # ip -j rule | python -m json.tool
>     ...
>     {
>         "priority": 0,
>         "src": "all",
>         "to_tbl": "blackhole"
>     },
> 
> This is an API break of JSON output as it was added in v4.17.0.
> Still change it as the API is relatively new and unstable.
> 
> Fixes: 0dd4ccc56c0e ("iprule: add json support")
> 
> Signed-off-by: Thomas Haller <thaller@redhat.com>

Applied, thanks.


^ permalink raw reply

* Re: [RFC iproute2 v2] ip route: get: allow zero-length subnet mask
From: Stephen Hemminger @ 2019-02-19 23:22 UTC (permalink / raw)
  To: Luca Boccassi; +Cc: netdev, Clément Hertling
In-Reply-To: <20190214232918.6185-1-bluca@debian.org>

On Thu, 14 Feb 2019 23:29:18 +0000
Luca Boccassi <bluca@debian.org> wrote:

> A /0 subnet mask is theoretically valid, but ip route get doesn't allow
> it:
> 
> $ ip route get 1.0.0.0/0
> need at least a destination address
> 
> Change the check and remember whether we found an address or not, since
> according to the documentation it's a mandatory parameter.
> 
> $ ip/ip route get 1.0.0.0/0
> 1.0.0.0 via 192.168.1.1 dev eth0 src 192.168.1.91 uid 1000
>     cache
> 
> Reported-by: Clément Hertling <wxcafe@wxcafe.net>
> Signed-off-by: Luca Boccassi <bluca@debian.org>
> ---

Applied

^ permalink raw reply

* Re: [RFC iproute2] devlink: add support for updating device flash
From: Stephen Hemminger @ 2019-02-19 23:28 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: davem, jiri, netdev, oss-drivers, mkubecek, andrew
In-Reply-To: <20190211065923.22670-5-jakub.kicinski@netronome.com>

On Sun, 10 Feb 2019 22:59:23 -0800
Jakub Kicinski <jakub.kicinski@netronome.com> wrote:

> Add new command for updating flash of devices via devlink API.
> Example:
> 
> $ cp flash-boot.bin /lib/firmware/
> $ devlink dev flash pci/0000:05:00.0 file flash-boot.bin
> 
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>

This is targeted at iproute2-next since it needs stuff from net-next.


^ permalink raw reply

* Re: [PATCH net-next v2 2/3] net: dsa: mv88e6xxx: add support for bridge flags
From: Russell King - ARM Linux admin @ 2019-02-19 23:34 UTC (permalink / raw)
  To: Vivien Didelot
  Cc: Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S. Miller,
	netdev
In-Reply-To: <20190219170059.h5j552kuplvdaqol@shell.armlinux.org.uk>

On Tue, Feb 19, 2019 at 05:00:59PM +0000, Russell King - ARM Linux admin wrote:
> I've just changed my last patch to set these modes from
> dsa_port_bridge_join() and dsa_port_bridge_leave(), and while testing,
> I notice this on the ZII rev B board:
> 
> At boot (without anything connected to any of the switch ports):
> 
> br0: port 1(lan0) entered blocking state
> br0: port 1(lan0) entered disabled state
> device lan0 entered promiscuous mode
> device eth1 entered promiscuous mode
> br0: port 2(lan1) entered blocking state
> br0: port 2(lan1) entered disabled state
> device lan1 entered promiscuous mode
> ...
> 
> I then removed lan0 from the bridge:
> 
> device lan0 left promiscuous mode
> br0: port 1(lan0) entered disabled state
> 
> and then added it back:
> 
> br0: port 1(lan0) entered blocking state
> br0: port 1(lan0) entered disabled state
> device lan0 entered promiscuous mode
> 
> Now, you'd expect lan0 and lan1 to be configured the same at this
> point, and the same as it was before lan0 was removed from the bridge?
> lan0 is port 0, lan1 is port 1 on this switch - and the register debug
> says:
> 
>     GLOBAL GLOBAL2 SERDES     0    1    2    3    4    5    6
>  0:  c800       0    1140  500f 500f 500f 500f 500f 4e07 4d04
> ...
>  4:  40a8     258     1e0   43c  43d  43d   7c  430  53f 373f
> 
> Note that port 0 is in disabled state, but port 1 and 2 are in
> blocking state... but wait, the kernel printed a message saying it was
> in disabled state!
> 
> If I do the same for lan1, port 1 above changed from 0x43d to 0x433 as
> expected, and then returns to 0x43c.
> 
> It looks like DSA isn't always in sync with bridge as per port state.

Okay, the problem is what we do when we up the port.

When the port is added to the bridge device, and it's down, the bridge
code sets the STP state to "disabled".

Then when we up the interface, dsa_slave_open() calls dsa_port_enable(),
which then decides to change the STP state on its own without reference
to the state assigned by net/bridge:

int dsa_port_enable(struct dsa_port *dp, struct phy_device *phy)
{
        u8 stp_state = dp->bridge_dev ? BR_STATE_BLOCKING : BR_STATE_FORWARDING;
...
        dsa_port_set_state_now(dp, stp_state);
...
}

I can understand setting the state to BR_STATE_FORWARDING for
stand-alone ports, but why for bridged ports when the bridge code has
already taken care of configuring the STP state of the port?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply

* [PATCH net] ipvlan: disallow userns cap_net_admin to change global mode/flags
From: Daniel Borkmann @ 2019-02-19 23:15 UTC (permalink / raw)
  To: davem; +Cc: maheshb, m, netdev, Daniel Borkmann

When running Docker with userns isolation e.g. --userns-remap="default"
and spawning up some containers with CAP_NET_ADMIN under this realm, I
noticed that link changes on ipvlan slave device inside that container
can affect all devices from this ipvlan group which are in other net
namespaces where the container should have no permission to make changes
to, such as the init netns, for example.

This effectively allows to undo ipvlan private mode and switch globally to
bridge mode where slaves can communicate directly without going through
hostns, or it allows to switch between global operation mode (l2/l3/l3s)
for everyone bound to the given ipvlan master device. libnetwork plugin
here is creating an ipvlan master and ipvlan slave in hostns and a slave
each that is moved into the container's netns upon creation event.

* In hostns:

  # ip -d a
  [...]
  8: cilium_host@bond0: <BROADCAST,MULTICAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default qlen 1000
     link/ether 0c:c4:7a:e1:3d:cc brd ff:ff:ff:ff:ff:ff promiscuity 0 minmtu 68 maxmtu 65535
     ipvlan  mode l3 bridge numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
     inet 10.41.0.1/32 scope link cilium_host
       valid_lft forever preferred_lft forever
  [...]

* Spawn container & change ipvlan mode setting inside of it:

  # docker run -dt --cap-add=NET_ADMIN --network cilium-net --name client -l app=test cilium/netperf
  9fff485d69dcb5ce37c9e33ca20a11ccafc236d690105aadbfb77e4f4170879c

  # docker exec -ti client ip -d a
  [...]
  10: cilium0@if4: <BROADCAST,MULTICAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default qlen 1000
      link/ether 0c:c4:7a:e1:3d:cc brd ff:ff:ff:ff:ff:ff promiscuity 0 minmtu 68 maxmtu 65535
      ipvlan  mode l3 bridge numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
      inet 10.41.197.43/32 brd 10.41.197.43 scope global cilium0
         valid_lft forever preferred_lft forever

  # docker exec -ti client ip link change link cilium0 name cilium0 type ipvlan mode l2

  # docker exec -ti client ip -d a
  [...]
  10: cilium0@if4: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default qlen 1000
      link/ether 0c:c4:7a:e1:3d:cc brd ff:ff:ff:ff:ff:ff promiscuity 0 minmtu 68 maxmtu 65535
      ipvlan  mode l2 bridge numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
      inet 10.41.197.43/32 brd 10.41.197.43 scope global cilium0
         valid_lft forever preferred_lft forever

* In hostns (mode switched to l2):

  # ip -d a
  [...]
  8: cilium_host@bond0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default qlen 1000
      link/ether 0c:c4:7a:e1:3d:cc brd ff:ff:ff:ff:ff:ff promiscuity 0 minmtu 68 maxmtu 65535
      ipvlan  mode l2 bridge numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
      inet 10.41.0.1/32 scope link cilium_host
         valid_lft forever preferred_lft forever
  [...]

Same l3 -> l2 switch would also happen by creating another slave inside
the container's network namespace when specifying the existing cilium0
link to derive the actual (bond0) master:

  # docker exec -ti client ip link add link cilium0 name cilium1 type ipvlan mode l2

  # docker exec -ti client ip -d a
  [...]
  2: cilium1@if4: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN group default qlen 1000
      link/ether 0c:c4:7a:e1:3d:cc brd ff:ff:ff:ff:ff:ff promiscuity 0 minmtu 68 maxmtu 65535
      ipvlan  mode l2 bridge numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
  10: cilium0@if4: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default qlen 1000
      link/ether 0c:c4:7a:e1:3d:cc brd ff:ff:ff:ff:ff:ff promiscuity 0 minmtu 68 maxmtu 65535
      ipvlan  mode l2 bridge numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
      inet 10.41.197.43/32 brd 10.41.197.43 scope global cilium0
         valid_lft forever preferred_lft forever

* In hostns:

  # ip -d a
  [...]
  8: cilium_host@bond0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default qlen 1000
      link/ether 0c:c4:7a:e1:3d:cc brd ff:ff:ff:ff:ff:ff promiscuity 0 minmtu 68 maxmtu 65535
      ipvlan  mode l2 bridge numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
      inet 10.41.0.1/32 scope link cilium_host
         valid_lft forever preferred_lft forever
  [...]

One way to mitigate it is to check CAP_NET_ADMIN permissions of
the ipvlan master device's ns, and only then allow to change
mode or flags for all devices bound to it. Above two cases are
then disallowed after the patch.

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 drivers/net/ipvlan/ipvlan_main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index 7cdac77..07e41c4 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -499,6 +499,8 @@ static int ipvlan_nl_changelink(struct net_device *dev,
 
 	if (!data)
 		return 0;
+	if (!ns_capable(dev_net(ipvlan->phy_dev)->user_ns, CAP_NET_ADMIN))
+		return -EPERM;
 
 	if (data[IFLA_IPVLAN_MODE]) {
 		u16 nmode = nla_get_u16(data[IFLA_IPVLAN_MODE]);
@@ -601,6 +603,8 @@ int ipvlan_link_new(struct net *src_net, struct net_device *dev,
 		struct ipvl_dev *tmp = netdev_priv(phy_dev);
 
 		phy_dev = tmp->phy_dev;
+		if (!ns_capable(dev_net(phy_dev)->user_ns, CAP_NET_ADMIN))
+			return -EPERM;
 	} else if (!netif_is_ipvlan_port(phy_dev)) {
 		/* Exit early if the underlying link is invalid or busy */
 		if (phy_dev->type != ARPHRD_ETHER ||
-- 
2.7.4


^ permalink raw reply related

* [PATCH iproute2] bridge: make mcast_flood description consistent
From: Vivien Didelot @ 2019-02-19 23:47 UTC (permalink / raw)
  To: netdev; +Cc: stephen, Russell King, Vivien Didelot

This patch simply changes the description of the mcast_flood flag
with "flood" instead of "be flooded with" to avoid confusion, and be
consistent with the description of the flooding flag, which "Controls
whether a given port will *flood* unicast traffic for which there is
no FDB entry."

Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
---
 man/man8/bridge.8     | 2 +-
 man/man8/ip-link.8.in | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/man/man8/bridge.8 b/man/man8/bridge.8
index 72210f62..03b33d34 100644
--- a/man/man8/bridge.8
+++ b/man/man8/bridge.8
@@ -361,7 +361,7 @@ switch.
 
 .TP
 .BR "mcast_flood on " or " mcast_flood off "
-Controls whether a given port will be flooded with multicast traffic for which there is no MDB entry. By default this flag is on.
+Controls whether a given port will flood multicast traffic for which there is no MDB entry. By default this flag is on.
 
 .TP
 .BR "neigh_suppress on " or " neigh_suppress off "
diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index 5132f514..cef489a4 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -2155,7 +2155,7 @@ queries.
 option above.
 
 .BR mcast_flood " { " on " | " off " }"
-- controls whether a given port will be flooded with multicast traffic for which there is no MDB entry.
+- controls whether a given port will flood multicast traffic for which there is no MDB entry.
 
 .BI group_fwd_mask " MASK "
 - set the group forward mask. This is the bitmask that is applied to decide whether to forward incoming frames destined to link-local addresses, ie addresses of the form 01:80:C2:00:00:0X (defaults to 0, ie the bridge does not forward any link-local frames coming on this port).
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH bpf-next 3/9] bpf: add bpf helper bpf_skb_set_ecn
From: Lawrence Brakmo @ 2019-02-19 23:53 UTC (permalink / raw)
  To: Daniel Borkmann, netdev
  Cc: Martin Lau, Alexei Starovoitov, Daniel Borkmann --cc=Kernel Team
In-Reply-To: <e6522869-d79b-5da1-d44a-06f80dc72257@iogearbox.net>


On 2/19/19, 2:52 AM, "netdev-owner@vger.kernel.org on behalf of Daniel Borkmann" <netdev-owner@vger.kernel.org on behalf of daniel@iogearbox.net> wrote:

    On 02/19/2019 06:38 AM, brakmo wrote:
    > This patch adds a new bpf helper BPF_FUNC_skb_set_ecn
    > "int bpf_skb_set_Ecn(struct sk_buff *skb)". It is added to
    > BPF_PROG_TYPE_CGROUP_SKB typed bpf_prog which currently can
    > be attached to the ingress and egress path. This type of
    > bpf_prog cannot modify the skb directly.
    > 
    > This helper is used to set the ECN bits (2) of the IPv6 or IPv4
    > header in skb. It can be used by a bpf_prog to manage egress
    > network bandwdith limit per cgroupv2 by inducing an ECN
    > response in the TCP sender (when the packet is ECN enabled).
    > This works best when using DCTCP.
    > 
    > Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
    > ---
    >  include/uapi/linux/bpf.h | 10 +++++++++-
    >  net/core/filter.c        | 29 +++++++++++++++++++++++++++++
    >  2 files changed, 38 insertions(+), 1 deletion(-)
    > 
    > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
    > index 9e9f4f1a0370..5daf404511f7 100644
    > --- a/include/uapi/linux/bpf.h
    > +++ b/include/uapi/linux/bpf.h
    > @@ -2365,6 +2365,13 @@ union bpf_attr {
    >   *        Make a tcp_sock enter CWR state.
    >   *    Return
    >   *        0
    > + *
    > + * int bpf_skb_set_ecn(struct sk_buf *skb, int val)
    
    Nit: BPF_CALL_2() has u32 val

Thanks!
    
    > + *	Description
    > + *		Sets ECN bits (2) of IP header. Works with IPv6 and IPv4.
    > + *		val should be one of 0, 1, 2, 3.
    > + *	Return
    > + *		-EINVAL on error (e.g. val > 3), 0 otherwise.
    >   */
    >  #define __BPF_FUNC_MAPPER(FN)		\
    >  	FN(unspec),			\
    > @@ -2464,7 +2471,8 @@ union bpf_attr {
    >  	FN(spin_unlock),		\
    >  	FN(sk_fullsock),		\
    >  	FN(tcp_sock),			\
    > -	FN(tcp_enter_cwr),
    > +	FN(tcp_enter_cwr),		\
    > +	FN(skb_set_ecn),
    >  
    >  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
    >   * function eBPF program intends to call
    > diff --git a/net/core/filter.c b/net/core/filter.c
    > index f51c4a781844..275acfb2117d 100644
    > --- a/net/core/filter.c
    > +++ b/net/core/filter.c
    > @@ -5438,6 +5438,33 @@ static const struct bpf_func_proto bpf_tcp_enter_cwr_proto = {
    >  	.ret_type    = RET_INTEGER,
    >  	.arg1_type    = ARG_PTR_TO_TCP_SOCK,
    >  };
    > +
    > +BPF_CALL_2(bpf_skb_set_ecn, struct sk_buff *, skb, u32, val)
    > +{
    > +	struct ipv6hdr *ip6h = ipv6_hdr(skb);
    > +
    > +	if ((val & ~0x3) != 0)
    
    Nit: INET_ECN_MASK

Thanks!
    
    > +		return -EINVAL;
    > +
    > +	if (ip6h->version == 6) {
    > +		ip6h->flow_lbl[0] = (ip6h->flow_lbl[0] & ~0x30) | (val << 4);
    > +		return 0;
    > +	} else if (ip6h->version == 4) {
    > +		struct iphdr *ip4h = (struct iphdr *)ip6h;
    > +
    > +		ip4h->tos = (ip4h->tos & ~0x3) | val;
    > +		return 0;
    > +	}
    
    Couldn't this be done as native BPF code via direct packet access instead?
    Afaik, skb->data should most likely points to network header for the hooks
    and skb->protocol should be one of ETH_P_IP{,V6}, no?

Cgroup skb bpf programs do not have write access to packet data. I originally was doing what you propose, by adding write support and changing the ecn value in the bpf program, but Alexei felt that could create problems. Hence this approach.
    
    Aside from this, don't we also have cloned skbs here (in particular from
    TCP side)?
    
    Looking at cg_skb_verifier_ops ... it seems there also a bug in the current
    code, namely that if we have a direct packet write, we don't make the skb
    writable; at that point skb->data is not private. The cg_skb_is_valid_access()
    allows to fetch PTR_TO_PACKET{,_END}, so we need a fix like the below for -bpf:
    
    diff --git a/net/core/filter.c b/net/core/filter.c
    index f7d0004fc160..34fe6da0a236 100644
    --- a/net/core/filter.c
    +++ b/net/core/filter.c
    @@ -5796,6 +5796,12 @@ static bool sk_filter_is_valid_access(int off, int size,
            return bpf_skb_is_valid_access(off, size, type, prog, info);
     }
    
    +static int cg_skb_prologue(struct bpf_insn *insn_buf, bool direct_write,
    +                          const struct bpf_prog *prog)
    +{
    +       return bpf_unclone_prologue(insn_buf, direct_write, prog, 0);
    +}
    +
     static bool cg_skb_is_valid_access(int off, int size,
                                       enum bpf_access_type type,
                                       const struct bpf_prog *prog,
    @@ -7595,6 +7601,7 @@ const struct bpf_verifier_ops cg_skb_verifier_ops = {
            .get_func_proto         = cg_skb_func_proto,
            .is_valid_access        = cg_skb_is_valid_access,
            .convert_ctx_access     = bpf_convert_ctx_access,
    +       .gen_prologue           = cg_skb_prologue,
     };
    
     const struct bpf_prog_ops cg_skb_prog_ops = {
    
    > +	return -EINVAL;
    > +}
    > +
    > +static const struct bpf_func_proto bpf_skb_set_ecn_proto = {
    > +	.func		= bpf_skb_set_ecn,
    > +	.gpl_only	= false,
    > +	.ret_type	= RET_INTEGER,
    > +	.arg1_type	= ARG_PTR_TO_CTX,
    > +	.arg2_type	= ARG_ANYTHING,
    > +};
    >  #endif /* CONFIG_INET */
    >  
    >  bool bpf_helper_changes_pkt_data(void *func)
    > @@ -5599,6 +5626,8 @@ cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
    >  		return &bpf_tcp_sock_proto;
    >  	case BPF_FUNC_tcp_enter_cwr:
    >  		return &bpf_tcp_enter_cwr_proto;
    > +	case BPF_FUNC_skb_set_ecn:
    > +		return &bpf_skb_set_ecn_proto;
    >  #endif
    >  	default:
    >  		return sk_filter_func_proto(func_id, prog);
    > 

    
    


^ permalink raw reply

* Re: [PATCH net-next v2 2/3] net: dsa: mv88e6xxx: add support for bridge flags
From: Florian Fainelli @ 2019-02-19 23:53 UTC (permalink / raw)
  To: Russell King - ARM Linux admin, Vivien Didelot
  Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, netdev
In-Reply-To: <20190219233408.th2wukhmtxdmeivj@shell.armlinux.org.uk>

On 2/19/19 3:34 PM, Russell King - ARM Linux admin wrote:
> On Tue, Feb 19, 2019 at 05:00:59PM +0000, Russell King - ARM Linux admin wrote:
>> I've just changed my last patch to set these modes from
>> dsa_port_bridge_join() and dsa_port_bridge_leave(), and while testing,
>> I notice this on the ZII rev B board:
>>
>> At boot (without anything connected to any of the switch ports):
>>
>> br0: port 1(lan0) entered blocking state
>> br0: port 1(lan0) entered disabled state
>> device lan0 entered promiscuous mode
>> device eth1 entered promiscuous mode
>> br0: port 2(lan1) entered blocking state
>> br0: port 2(lan1) entered disabled state
>> device lan1 entered promiscuous mode
>> ...
>>
>> I then removed lan0 from the bridge:
>>
>> device lan0 left promiscuous mode
>> br0: port 1(lan0) entered disabled state
>>
>> and then added it back:
>>
>> br0: port 1(lan0) entered blocking state
>> br0: port 1(lan0) entered disabled state
>> device lan0 entered promiscuous mode
>>
>> Now, you'd expect lan0 and lan1 to be configured the same at this
>> point, and the same as it was before lan0 was removed from the bridge?
>> lan0 is port 0, lan1 is port 1 on this switch - and the register debug
>> says:
>>
>>     GLOBAL GLOBAL2 SERDES     0    1    2    3    4    5    6
>>  0:  c800       0    1140  500f 500f 500f 500f 500f 4e07 4d04
>> ...
>>  4:  40a8     258     1e0   43c  43d  43d   7c  430  53f 373f
>>
>> Note that port 0 is in disabled state, but port 1 and 2 are in
>> blocking state... but wait, the kernel printed a message saying it was
>> in disabled state!
>>
>> If I do the same for lan1, port 1 above changed from 0x43d to 0x433 as
>> expected, and then returns to 0x43c.
>>
>> It looks like DSA isn't always in sync with bridge as per port state.
> 
> Okay, the problem is what we do when we up the port.
> 
> When the port is added to the bridge device, and it's down, the bridge
> code sets the STP state to "disabled".
> 
> Then when we up the interface, dsa_slave_open() calls dsa_port_enable(),
> which then decides to change the STP state on its own without reference
> to the state assigned by net/bridge:
> 
> int dsa_port_enable(struct dsa_port *dp, struct phy_device *phy)
> {
>         u8 stp_state = dp->bridge_dev ? BR_STATE_BLOCKING : BR_STATE_FORWARDING;
> ...
>         dsa_port_set_state_now(dp, stp_state);
> ...
> }
> 
> I can understand setting the state to BR_STATE_FORWARDING for
> stand-alone ports, but why for bridged ports when the bridge code has
> already taken care of configuring the STP state of the port?

There was no reason for doing that in commit
b73adef67765b72f2a0d01ef15aff9d784dc85da ("net: dsa: integrate with
SWITCHDEV for HW bridging") other than copying what rocker had done
(which served as model back then), and which got changed the next day in
rocker with: e47172ab7e4176883077b454286bbd5b87b5f488 ("rocker: put port
in FORWADING state after leaving bridge")

Good catch!
--
Florian

^ permalink raw reply

* [PATCH v2 net-next 0/4] net: dsa: microchip: add MIB counters support
From: Tristram.Ha @ 2019-02-19 23:56 UTC (permalink / raw)
  To: Sergio Paracuellos, Andrew Lunn, Florian Fainelli, Pavel Machek
  Cc: Tristram Ha, UNGLinuxDriver, netdev

From: Tristram Ha <Tristram.Ha@microchip.com>

This series of patches is to modify the KSZ9477 DSA driver to read MIB
counters periodically to avoid overflow.

The MIB counters should be read only when there is link.  Otherwise it is
a waste of time as hardware never increases the counters.

Functions are added to check the port link status so that MIB counters
read call is used efficiently.

v2
- Create macro similar to readx_poll_timeout to use with switch
- Create ksz_port_cleanup function so that variables like on_ports and
  live_ports can be updated inside it.

v1
- Use readx_poll_timeout
- Do not clear MIB counters when port is enabled
- Do not advertise 1000 half-duplex mode when port is enabled
- Do not use freeze function as MIB counters may miss counts

Tristram Ha (4):
  net: dsa: microchip: prepare PHY for proper advertisement
  net: dsa: microchip: add MIB counter reading support
  net: dsa: microchip: get port link status
  net: dsa: microchip: remove unnecessary include headers

 drivers/net/dsa/microchip/ksz9477.c    | 137 +++++++++++++++++----------
 drivers/net/dsa/microchip/ksz_common.c | 163 ++++++++++++++++++++++++++++++++-
 drivers/net/dsa/microchip/ksz_common.h |  27 +++++-
 drivers/net/dsa/microchip/ksz_priv.h   |  14 +--
 4 files changed, 284 insertions(+), 57 deletions(-)

-- 
1.9.1


^ permalink raw reply

* [PATCH v2 net-next 2/4] net: dsa: microchip: add MIB counter reading support
From: Tristram.Ha @ 2019-02-19 23:57 UTC (permalink / raw)
  To: Sergio Paracuellos, Andrew Lunn, Florian Fainelli, Pavel Machek
  Cc: Tristram Ha, UNGLinuxDriver, netdev
In-Reply-To: <1550620623-13036-1-git-send-email-Tristram.Ha@microchip.com>

From: Tristram Ha <Tristram.Ha@microchip.com>

Add background MIB counter reading support.

Port MIB counters should only be read when there is link.  Otherwise it is
a waste of time as hardware never increases those counters.  There are
exceptions as some switches keep track of dropped counts no matter waht.

Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
---
 drivers/net/dsa/microchip/ksz9477.c    | 118 ++++++++++++++++++++------------
 drivers/net/dsa/microchip/ksz_common.c | 121 +++++++++++++++++++++++++++++++++
 drivers/net/dsa/microchip/ksz_common.h |  24 ++++++-
 drivers/net/dsa/microchip/ksz_priv.h   |   9 ++-
 4 files changed, 224 insertions(+), 48 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 4573b6e..e2d74c7 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -18,8 +18,8 @@
 #include <net/switchdev.h>
 
 #include "ksz_priv.h"
-#include "ksz_common.h"
 #include "ksz9477_reg.h"
+#include "ksz_common.h"
 
 static const struct {
 	int index;
@@ -259,6 +259,71 @@ static int ksz9477_reset_switch(struct ksz_device *dev)
 	return 0;
 }
 
+static void ksz9477_r_mib_cnt(struct ksz_device *dev, int port, u16 addr,
+			      u64 *cnt)
+{
+	struct ksz_port *p = &dev->ports[port];
+	u32 data;
+	int ret;
+
+	/* retain the flush/freeze bit */
+	data = p->freeze ? MIB_COUNTER_FLUSH_FREEZE : 0;
+	data |= MIB_COUNTER_READ;
+	data |= (addr << MIB_COUNTER_INDEX_S);
+	ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4, data);
+
+	ret = ksz_pread_poll_timeout(ksz_pread32, dev, port,
+				     REG_PORT_MIB_CTRL_STAT__4, data,
+				     !(data & MIB_COUNTER_READ), 10, 1000);
+
+	/* failed to read MIB. get out of loop */
+	if (ret < 0) {
+		dev_dbg(dev->dev, "Failed to get MIB\n");
+		return;
+	}
+
+	/* count resets upon read */
+	ksz_pread32(dev, port, REG_PORT_MIB_DATA, &data);
+	*cnt += data;
+}
+
+static void ksz9477_r_mib_pkt(struct ksz_device *dev, int port, u16 addr,
+			      u64 *dropped, u64 *cnt)
+{
+	addr = ksz9477_mib_names[addr].index;
+	ksz9477_r_mib_cnt(dev, port, addr, cnt);
+}
+
+static void ksz9477_freeze_mib(struct ksz_device *dev, int port, bool freeze)
+{
+	u32 val = freeze ? MIB_COUNTER_FLUSH_FREEZE : 0;
+	struct ksz_port *p = &dev->ports[port];
+
+	/* enable/disable the port for flush/freeze function */
+	mutex_lock(&p->mib.cnt_mutex);
+	ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4, val);
+
+	/* used by MIB counter reading code to know freeze is enabled */
+	p->freeze = freeze;
+	mutex_unlock(&p->mib.cnt_mutex);
+}
+
+static void ksz9477_port_init_cnt(struct ksz_device *dev, int port)
+{
+	struct ksz_port_mib *mib = &dev->ports[port].mib;
+
+	/* flush all enabled port MIB counters */
+	mutex_lock(&mib->cnt_mutex);
+	ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4,
+		     MIB_COUNTER_FLUSH_FREEZE);
+	ksz_write8(dev, REG_SW_MAC_CTRL_6, SW_MIB_COUNTER_FLUSH);
+	ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4, 0);
+	mutex_unlock(&mib->cnt_mutex);
+
+	mib->cnt_ptr = 0;
+	memset(mib->counters, 0, dev->mib_cnt * sizeof(u64));
+}
+
 static enum dsa_tag_protocol ksz9477_get_tag_protocol(struct dsa_switch *ds,
 						      int port)
 {
@@ -342,47 +407,6 @@ static void ksz9477_get_strings(struct dsa_switch *ds, int port,
 	}
 }
 
-static void ksz_get_ethtool_stats(struct dsa_switch *ds, int port,
-				  uint64_t *buf)
-{
-	struct ksz_device *dev = ds->priv;
-	int i;
-	u32 data;
-	int timeout;
-
-	mutex_lock(&dev->stats_mutex);
-
-	for (i = 0; i < TOTAL_SWITCH_COUNTER_NUM; i++) {
-		data = MIB_COUNTER_READ;
-		data |= ((ksz9477_mib_names[i].index & 0xFF) <<
-			MIB_COUNTER_INDEX_S);
-		ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4, data);
-
-		timeout = 1000;
-		do {
-			ksz_pread32(dev, port, REG_PORT_MIB_CTRL_STAT__4,
-				    &data);
-			usleep_range(1, 10);
-			if (!(data & MIB_COUNTER_READ))
-				break;
-		} while (timeout-- > 0);
-
-		/* failed to read MIB. get out of loop */
-		if (!timeout) {
-			dev_dbg(dev->dev, "Failed to get MIB\n");
-			break;
-		}
-
-		/* count resets upon read */
-		ksz_pread32(dev, port, REG_PORT_MIB_DATA, &data);
-
-		dev->mib_value[i] += (uint64_t)data;
-		buf[i] = dev->mib_value[i];
-	}
-
-	mutex_unlock(&dev->stats_mutex);
-}
-
 static void ksz9477_cfg_port_member(struct ksz_device *dev, int port,
 				    u8 member)
 {
@@ -1151,9 +1175,14 @@ static int ksz9477_setup(struct dsa_switch *ds)
 	/* queue based egress rate limit */
 	ksz_cfg(dev, REG_SW_MAC_CTRL_5, SW_OUT_RATE_LIMIT_QUEUE_BASED, true);
 
+	/* enable global MIB counter freeze function */
+	ksz_cfg(dev, REG_SW_MAC_CTRL_6, SW_MIB_COUNTER_FREEZE, true);
+
 	/* start switch */
 	ksz_cfg(dev, REG_SW_OPERATION, SW_START, true);
 
+	ksz_init_mib_timer(dev);
+
 	return 0;
 }
 
@@ -1287,6 +1316,7 @@ static int ksz9477_switch_init(struct ksz_device *dev)
 	if (!dev->ports)
 		return -ENOMEM;
 	for (i = 0; i < dev->mib_port_cnt; i++) {
+		mutex_init(&dev->ports[i].mib.cnt_mutex);
 		dev->ports[i].mib.counters =
 			devm_kzalloc(dev->dev,
 				     sizeof(u64) *
@@ -1311,6 +1341,10 @@ static void ksz9477_switch_exit(struct ksz_device *dev)
 	.flush_dyn_mac_table = ksz9477_flush_dyn_mac_table,
 	.phy_setup = ksz9477_phy_setup,
 	.port_setup = ksz9477_port_setup,
+	.r_mib_cnt = ksz9477_r_mib_cnt,
+	.r_mib_pkt = ksz9477_r_mib_pkt,
+	.freeze_mib = ksz9477_freeze_mib,
+	.port_init_cnt = ksz9477_port_init_cnt,
 	.shutdown = ksz9477_reset_switch,
 	.detect = ksz9477_switch_detect,
 	.init = ksz9477_switch_init,
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 6f72842..9270570 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -40,6 +40,101 @@ void ksz_update_port_member(struct ksz_device *dev, int port)
 }
 EXPORT_SYMBOL_GPL(ksz_update_port_member);
 
+static void port_r_cnt(struct ksz_device *dev, int port)
+{
+	struct ksz_port_mib *mib = &dev->ports[port].mib;
+	u64 *dropped;
+
+	/* Some ports may not have MIB counters before SWITCH_COUNTER_NUM. */
+	while (mib->cnt_ptr < dev->reg_mib_cnt) {
+		dev->dev_ops->r_mib_cnt(dev, port, mib->cnt_ptr,
+					&mib->counters[mib->cnt_ptr]);
+		++mib->cnt_ptr;
+	}
+
+	/* last one in storage */
+	dropped = &mib->counters[dev->mib_cnt];
+
+	/* Some ports may not have MIB counters after SWITCH_COUNTER_NUM. */
+	while (mib->cnt_ptr < dev->mib_cnt) {
+		dev->dev_ops->r_mib_pkt(dev, port, mib->cnt_ptr,
+					dropped, &mib->counters[mib->cnt_ptr]);
+		++mib->cnt_ptr;
+	}
+	mib->cnt_ptr = 0;
+}
+
+static void ksz_mib_read_work(struct work_struct *work)
+{
+	struct ksz_device *dev = container_of(work, struct ksz_device,
+					      mib_read);
+	struct ksz_port_mib *mib;
+	struct ksz_port *p;
+	int i;
+
+	for (i = 0; i < dev->mib_port_cnt; i++) {
+		p = &dev->ports[i];
+
+		/* Only read MIB counters when the port is told to do. */
+		if (!p->read)
+			continue;
+		mib = &p->mib;
+		mutex_lock(&mib->cnt_mutex);
+		port_r_cnt(dev, i);
+		mutex_unlock(&mib->cnt_mutex);
+	}
+}
+
+static void mib_monitor(struct timer_list *t)
+{
+	struct ksz_device *dev = from_timer(dev, t, mib_read_timer);
+	const struct dsa_port *dp;
+	struct net_device *netdev;
+	struct ksz_port_mib *mib;
+	struct ksz_port *p;
+	int i;
+
+	mod_timer(&dev->mib_read_timer, jiffies + dev->mib_read_interval);
+
+	/* Check which port needs to read MIB counters. */
+	for (i = 0; i < dev->mib_port_cnt; i++) {
+		p = &dev->ports[i];
+		if (!p->on)
+			continue;
+		dp = dsa_to_port(dev->ds, i);
+		netdev = dp->slave;
+
+		mib = &p->mib;
+		mutex_lock(&mib->cnt_mutex);
+
+		/* Read only dropped counters when link is not up. */
+		if (netdev && netdev->phydev && !netdev->phydev->link)
+			mib->cnt_ptr = dev->reg_mib_cnt;
+		mutex_unlock(&mib->cnt_mutex);
+		p->read = true;
+	}
+	schedule_work(&dev->mib_read);
+}
+
+void ksz_init_mib_timer(struct ksz_device *dev)
+{
+	int i;
+
+	/* Read MIB counters every 30 seconds to avoid overflow. */
+	dev->mib_read_interval = msecs_to_jiffies(30000);
+
+	INIT_WORK(&dev->mib_read, ksz_mib_read_work);
+	timer_setup(&dev->mib_read_timer, mib_monitor, 0);
+
+	for (i = 0; i < dev->mib_port_cnt; i++)
+		dev->dev_ops->port_init_cnt(dev, i);
+
+	/* Start the timer 2 seconds later. */
+	dev->mib_read_timer.expires = jiffies + msecs_to_jiffies(2000);
+	add_timer(&dev->mib_read_timer);
+}
+EXPORT_SYMBOL_GPL(ksz_init_mib_timer);
+
 int ksz_phy_read16(struct dsa_switch *ds, int addr, int reg)
 {
 	struct ksz_device *dev = ds->priv;
@@ -72,6 +167,26 @@ int ksz_sset_count(struct dsa_switch *ds, int port, int sset)
 }
 EXPORT_SYMBOL_GPL(ksz_sset_count);
 
+void ksz_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *buf)
+{
+	const struct dsa_port *dp = dsa_to_port(ds, port);
+	struct ksz_device *dev = ds->priv;
+	struct net_device *netdev;
+	struct ksz_port_mib *mib;
+
+	mib = &dev->ports[port].mib;
+	mutex_lock(&mib->cnt_mutex);
+
+	/* Only read dropped counters if no link. */
+	netdev = dp->slave;
+	if (netdev && netdev->phydev && !netdev->phydev->link)
+		mib->cnt_ptr = dev->reg_mib_cnt;
+	port_r_cnt(dev, port);
+	memcpy(buf, mib->counters, dev->mib_cnt * sizeof(u64));
+	mutex_unlock(&mib->cnt_mutex);
+}
+EXPORT_SYMBOL_GPL(ksz_get_ethtool_stats);
+
 int ksz_port_bridge_join(struct dsa_switch *ds, int port,
 			 struct net_device *br)
 {
@@ -339,6 +454,12 @@ int ksz_switch_register(struct ksz_device *dev,
 
 void ksz_switch_remove(struct ksz_device *dev)
 {
+	/* timer started */
+	if (dev->mib_read_timer.expires) {
+		del_timer_sync(&dev->mib_read_timer);
+		flush_work(&dev->mib_read);
+	}
+
 	dev->dev_ops->exit(dev);
 	dsa_unregister_switch(dev->ds);
 
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 2dd832d..0b0ed3d 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -1,19 +1,21 @@
 /* SPDX-License-Identifier: GPL-2.0
  * Microchip switch driver common header
  *
- * Copyright (C) 2017-2018 Microchip Technology Inc.
+ * Copyright (C) 2017-2019 Microchip Technology Inc.
  */
 
 #ifndef __KSZ_COMMON_H
 #define __KSZ_COMMON_H
 
 void ksz_update_port_member(struct ksz_device *dev, int port);
+void ksz_init_mib_timer(struct ksz_device *dev);
 
 /* Common DSA access functions */
 
 int ksz_phy_read16(struct dsa_switch *ds, int addr, int reg);
 int ksz_phy_write16(struct dsa_switch *ds, int addr, int reg, u16 val);
 int ksz_sset_count(struct dsa_switch *ds, int port, int sset);
+void ksz_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *buf);
 int ksz_port_bridge_join(struct dsa_switch *ds, int port,
 			 struct net_device *br);
 void ksz_port_bridge_leave(struct dsa_switch *ds, int port,
@@ -211,4 +213,24 @@ static void ksz_port_cfg(struct ksz_device *dev, int port, int offset, u8 bits,
 	ksz_write8(dev, addr, data);
 }
 
+/* Modify from readx_poll_timeout in iopoll.h. */
+#define ksz_pread_poll_timeout(op, dev, p, addr, val, cond, sleep_us, \
+			       timeout_us) \
+({ \
+	ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
+	might_sleep_if(sleep_us); \
+	for (;;) { \
+		op(dev, p, addr, &(val)); \
+		if (cond) \
+			break; \
+		if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \
+			op(dev, p, addr, &(val)); \
+			break; \
+		} \
+		if (sleep_us) \
+			usleep_range((sleep_us >> 2) + 1, sleep_us); \
+	} \
+	(cond) ? 0 : -ETIMEDOUT; \
+})
+
 #endif
diff --git a/drivers/net/dsa/microchip/ksz_priv.h b/drivers/net/dsa/microchip/ksz_priv.h
index 0fdc58b..1d2d98f 100644
--- a/drivers/net/dsa/microchip/ksz_priv.h
+++ b/drivers/net/dsa/microchip/ksz_priv.h
@@ -14,8 +14,6 @@
 #include <linux/etherdevice.h>
 #include <net/dsa.h>
 
-#include "ksz9477_reg.h"
-
 struct ksz_io_ops;
 
 struct vlan_table {
@@ -23,6 +21,7 @@ struct vlan_table {
 };
 
 struct ksz_port_mib {
+	struct mutex cnt_mutex;		/* structure access */
 	u8 cnt_ptr;
 	u64 *counters;
 };
@@ -38,7 +37,8 @@ struct ksz_port {
 	u32 fiber:1;			/* port is fiber */
 	u32 sgmii:1;			/* port is SGMII */
 	u32 force:1;
-	u32 link_just_down:1;		/* link just goes down */
+	u32 read:1;			/* read MIB counters in background */
+	u32 freeze:1;			/* MIB counter freeze is enabled */
 
 	struct ksz_port_mib mib;
 };
@@ -79,8 +79,6 @@ struct ksz_device {
 
 	struct vlan_table *vlan_cache;
 
-	u64 mib_value[TOTAL_SWITCH_COUNTER_NUM];
-
 	u8 *txbuf;
 
 	struct ksz_port *ports;
@@ -153,6 +151,7 @@ struct ksz_dev_ops {
 			  u64 *cnt);
 	void (*r_mib_pkt)(struct ksz_device *dev, int port, u16 addr,
 			  u64 *dropped, u64 *cnt);
+	void (*freeze_mib)(struct ksz_device *dev, int port, bool freeze);
 	void (*port_init_cnt)(struct ksz_device *dev, int port);
 	int (*shutdown)(struct ksz_device *dev);
 	int (*detect)(struct ksz_device *dev);
-- 
1.9.1


^ permalink raw reply related

* [PATCH v2 net-next 3/4] net: dsa: microchip: get port link status
From: Tristram.Ha @ 2019-02-19 23:57 UTC (permalink / raw)
  To: Sergio Paracuellos, Andrew Lunn, Florian Fainelli, Pavel Machek
  Cc: Tristram Ha, UNGLinuxDriver, netdev
In-Reply-To: <1550620623-13036-1-git-send-email-Tristram.Ha@microchip.com>

From: Tristram Ha <Tristram.Ha@microchip.com>

Get port link status to know whether to read MIB counters when the link
is going down.  Add port_cleanup function to read MIB counters the last
time as after the port is disabled the PHY is also powered down.

Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
---
 drivers/net/dsa/microchip/ksz9477.c    |  2 ++
 drivers/net/dsa/microchip/ksz_common.c | 39 ++++++++++++++++++++++++++++++++--
 drivers/net/dsa/microchip/ksz_common.h |  3 +++
 drivers/net/dsa/microchip/ksz_priv.h   |  1 +
 4 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index e2d74c7..6ad28e2 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -1191,6 +1191,7 @@ static int ksz9477_setup(struct dsa_switch *ds)
 	.setup			= ksz9477_setup,
 	.phy_read		= ksz9477_phy_read16,
 	.phy_write		= ksz9477_phy_write16,
+	.adjust_link		= ksz_adjust_link,
 	.port_enable		= ksz_enable_port,
 	.port_disable		= ksz_disable_port,
 	.get_strings		= ksz9477_get_strings,
@@ -1340,6 +1341,7 @@ static void ksz9477_switch_exit(struct ksz_device *dev)
 	.cfg_port_member = ksz9477_cfg_port_member,
 	.flush_dyn_mac_table = ksz9477_flush_dyn_mac_table,
 	.phy_setup = ksz9477_phy_setup,
+	.port_cleanup = ksz_port_cleanup,
 	.port_setup = ksz9477_port_setup,
 	.r_mib_cnt = ksz9477_r_mib_cnt,
 	.r_mib_pkt = ksz9477_r_mib_pkt,
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 9270570..c4bb67f 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -20,6 +20,22 @@
 
 #include "ksz_priv.h"
 
+void ksz_port_cleanup(struct ksz_device *dev, int port)
+{
+	/* Read all MIB counters when the link is going down. */
+	if (dev->live_ports & (1 << port)) {
+		struct ksz_port *p = &dev->ports[port];
+
+		p->read = true;
+		schedule_work(&dev->mib_read);
+	}
+
+	/* Common code for port cleanup. */
+	dev->on_ports &= ~(1 << port);
+	dev->live_ports &= ~(1 << port);
+}
+EXPORT_SYMBOL_GPL(ksz_port_cleanup);
+
 void ksz_update_port_member(struct ksz_device *dev, int port)
 {
 	struct ksz_port *p;
@@ -156,6 +172,26 @@ int ksz_phy_write16(struct dsa_switch *ds, int addr, int reg, u16 val)
 }
 EXPORT_SYMBOL_GPL(ksz_phy_write16);
 
+void ksz_adjust_link(struct dsa_switch *ds, int port,
+		     struct phy_device *phydev)
+{
+	struct ksz_device *dev = ds->priv;
+	struct ksz_port *p = &dev->ports[port];
+
+	if (!phydev->link) {
+		/* Read all MIB counters when the link is going down. */
+		if (dev->live_ports & (1 << port)) {
+			p->read = true;
+			schedule_work(&dev->mib_read);
+		}
+		dev->live_ports &= ~(1 << port);
+	} else {
+		/* Remember which port is connected and active. */
+		dev->live_ports |= (1 << port) & dev->on_ports;
+	}
+}
+EXPORT_SYMBOL_GPL(ksz_adjust_link);
+
 int ksz_sset_count(struct dsa_switch *ds, int port, int sset)
 {
 	struct ksz_device *dev = ds->priv;
@@ -367,8 +403,7 @@ void ksz_disable_port(struct dsa_switch *ds, int port, struct phy_device *phy)
 {
 	struct ksz_device *dev = ds->priv;
 
-	dev->on_ports &= ~(1 << port);
-	dev->live_ports &= ~(1 << port);
+	dev->dev_ops->port_cleanup(dev, port);
 
 	/* port_stp_state_set() will be called after to disable the port so
 	 * there is no need to do anything.
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 0b0ed3d..ebe6f8e 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -7,6 +7,7 @@
 #ifndef __KSZ_COMMON_H
 #define __KSZ_COMMON_H
 
+void ksz_port_cleanup(struct ksz_device *dev, int port);
 void ksz_update_port_member(struct ksz_device *dev, int port);
 void ksz_init_mib_timer(struct ksz_device *dev);
 
@@ -14,6 +15,8 @@
 
 int ksz_phy_read16(struct dsa_switch *ds, int addr, int reg);
 int ksz_phy_write16(struct dsa_switch *ds, int addr, int reg, u16 val);
+void ksz_adjust_link(struct dsa_switch *ds, int port,
+		     struct phy_device *phydev);
 int ksz_sset_count(struct dsa_switch *ds, int port, int sset);
 void ksz_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *buf);
 int ksz_port_bridge_join(struct dsa_switch *ds, int port,
diff --git a/drivers/net/dsa/microchip/ksz_priv.h b/drivers/net/dsa/microchip/ksz_priv.h
index 1d2d98f..a1d84c1 100644
--- a/drivers/net/dsa/microchip/ksz_priv.h
+++ b/drivers/net/dsa/microchip/ksz_priv.h
@@ -137,6 +137,7 @@ struct ksz_dev_ops {
 	void (*flush_dyn_mac_table)(struct ksz_device *dev, int port);
 	void (*phy_setup)(struct ksz_device *dev, int port,
 			  struct phy_device *phy);
+	void (*port_cleanup)(struct ksz_device *dev, int port);
 	void (*port_setup)(struct ksz_device *dev, int port, bool cpu_port);
 	void (*r_phy)(struct ksz_device *dev, u16 phy, u16 reg, u16 *val);
 	void (*w_phy)(struct ksz_device *dev, u16 phy, u16 reg, u16 val);
-- 
1.9.1


^ permalink raw reply related

* [PATCH v2 net-next 4/4] net: dsa: microchip: remove unnecessary include headers
From: Tristram.Ha @ 2019-02-19 23:57 UTC (permalink / raw)
  To: Sergio Paracuellos, Andrew Lunn, Florian Fainelli, Pavel Machek
  Cc: Tristram Ha, UNGLinuxDriver, netdev
In-Reply-To: <1550620623-13036-1-git-send-email-Tristram.Ha@microchip.com>

From: Tristram Ha <Tristram.Ha@microchip.com>

Remove unnecessary header include.

Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/microchip/ksz9477.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 6ad28e2..a9465c1 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -5,14 +5,10 @@
  * Copyright (C) 2017-2019 Microchip Technology Inc.
  */
 
-#include <linux/delay.h>
-#include <linux/export.h>
-#include <linux/gpio.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/platform_data/microchip-ksz.h>
 #include <linux/phy.h>
-#include <linux/etherdevice.h>
 #include <linux/if_bridge.h>
 #include <net/dsa.h>
 #include <net/switchdev.h>
-- 
1.9.1


^ permalink raw reply related

* [PATCH v2 net-next 1/4] net: dsa: microchip: prepare PHY for proper advertisement
From: Tristram.Ha @ 2019-02-19 23:57 UTC (permalink / raw)
  To: Sergio Paracuellos, Andrew Lunn, Florian Fainelli, Pavel Machek
  Cc: Tristram Ha, UNGLinuxDriver, netdev
In-Reply-To: <1550620623-13036-1-git-send-email-Tristram.Ha@microchip.com>

From: Tristram Ha <Tristram.Ha@microchip.com>

Prepare PHY for proper advertisement as sometimes the PHY in the switch
has its own problems even though it may share the PHY id from regular PHY
but the fixes in the PHY driver do not apply.

Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
---
 drivers/net/dsa/microchip/ksz9477.c    | 13 ++++++++++++-
 drivers/net/dsa/microchip/ksz_common.c |  3 ++-
 drivers/net/dsa/microchip/ksz_priv.h   |  4 +++-
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 674d77e..4573b6e 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -2,7 +2,7 @@
 /*
  * Microchip KSZ9477 switch driver main logic
  *
- * Copyright (C) 2017-2018 Microchip Technology Inc.
+ * Copyright (C) 2017-2019 Microchip Technology Inc.
  */
 
 #include <linux/delay.h>
@@ -966,6 +966,16 @@ static void ksz9477_port_mirror_del(struct dsa_switch *ds, int port,
 			     PORT_MIRROR_SNIFFER, false);
 }
 
+static void ksz9477_phy_setup(struct ksz_device *dev, int port,
+			      struct phy_device *phy)
+{
+	if (port < dev->phy_port_cnt) {
+		/* The MAC actually cannot run in 1000 half-duplex mode. */
+		phy_remove_link_mode(phy,
+				     ETHTOOL_LINK_MODE_1000baseT_Half_BIT);
+	}
+}
+
 static void ksz9477_port_setup(struct ksz_device *dev, int port, bool cpu_port)
 {
 	u8 data8;
@@ -1299,6 +1309,7 @@ static void ksz9477_switch_exit(struct ksz_device *dev)
 	.get_port_addr = ksz9477_get_port_addr,
 	.cfg_port_member = ksz9477_cfg_port_member,
 	.flush_dyn_mac_table = ksz9477_flush_dyn_mac_table,
+	.phy_setup = ksz9477_phy_setup,
 	.port_setup = ksz9477_port_setup,
 	.shutdown = ksz9477_reset_switch,
 	.detect = ksz9477_switch_detect,
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 8a5111f..6f72842 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -2,7 +2,7 @@
 /*
  * Microchip switch driver main logic
  *
- * Copyright (C) 2017-2018 Microchip Technology Inc.
+ * Copyright (C) 2017-2019 Microchip Technology Inc.
  */
 
 #include <linux/delay.h>
@@ -238,6 +238,7 @@ int ksz_enable_port(struct dsa_switch *ds, int port, struct phy_device *phy)
 
 	/* setup slave port */
 	dev->dev_ops->port_setup(dev, port, false);
+	dev->dev_ops->phy_setup(dev, port, phy);
 
 	/* port_stp_state_set() will be called after to enable the port so
 	 * there is no need to do anything.
diff --git a/drivers/net/dsa/microchip/ksz_priv.h b/drivers/net/dsa/microchip/ksz_priv.h
index 60b4901..0fdc58b 100644
--- a/drivers/net/dsa/microchip/ksz_priv.h
+++ b/drivers/net/dsa/microchip/ksz_priv.h
@@ -2,7 +2,7 @@
  *
  * Microchip KSZ series switch common definitions
  *
- * Copyright (C) 2017-2018 Microchip Technology Inc.
+ * Copyright (C) 2017-2019 Microchip Technology Inc.
  */
 
 #ifndef __KSZ_PRIV_H
@@ -137,6 +137,8 @@ struct ksz_dev_ops {
 	u32 (*get_port_addr)(int port, int offset);
 	void (*cfg_port_member)(struct ksz_device *dev, int port, u8 member);
 	void (*flush_dyn_mac_table)(struct ksz_device *dev, int port);
+	void (*phy_setup)(struct ksz_device *dev, int port,
+			  struct phy_device *phy);
 	void (*port_setup)(struct ksz_device *dev, int port, bool cpu_port);
 	void (*r_phy)(struct ksz_device *dev, u16 phy, u16 reg, u16 *val);
 	void (*w_phy)(struct ksz_device *dev, u16 phy, u16 reg, u16 val);
-- 
1.9.1


^ permalink raw reply related

* Re: [PATCH iproute2] bridge: make mcast_flood description consistent
From: Russell King - ARM Linux admin @ 2019-02-20  0:05 UTC (permalink / raw)
  To: Vivien Didelot; +Cc: netdev, stephen
In-Reply-To: <20190219234738.17009-1-vivien.didelot@gmail.com>

On Tue, Feb 19, 2019 at 06:47:38PM -0500, Vivien Didelot wrote:
> This patch simply changes the description of the mcast_flood flag
> with "flood" instead of "be flooded with" to avoid confusion, and be
> consistent with the description of the flooding flag, which "Controls
> whether a given port will *flood* unicast traffic for which there is
> no FDB entry."

Hi Vivien,

I'm not sure if it's in the current iproute2, but there is a
discrepency between the arguments for 'bridge' stated in the man page
and the description thereof:

       bridge link set dev DEV  [ cost COST ] [ priority PRIO ] [ state STATE
...
               } ] [ learning_sync { on | off } ] [ flood { on | off } ] [
                                                    ^^^^^^^^^^^^^^^^^^

vs

       flooding on or flooding off
              Controls whether a given port will flood unicast traffic for
              which there is no FDB entry. By default this flag is on.

vs the command actually accepting "flood" not "flooding".  I spotted
that in iproute2-4.20.0.  I haven't had a chance to generate a patch
that yet and work out how to submit it, but thanks for leading the
way!

> 
> Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
> ---
>  man/man8/bridge.8     | 2 +-
>  man/man8/ip-link.8.in | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/man/man8/bridge.8 b/man/man8/bridge.8
> index 72210f62..03b33d34 100644
> --- a/man/man8/bridge.8
> +++ b/man/man8/bridge.8
> @@ -361,7 +361,7 @@ switch.
>  
>  .TP
>  .BR "mcast_flood on " or " mcast_flood off "
> -Controls whether a given port will be flooded with multicast traffic for which there is no MDB entry. By default this flag is on.
> +Controls whether a given port will flood multicast traffic for which there is no MDB entry. By default this flag is on.
>  
>  .TP
>  .BR "neigh_suppress on " or " neigh_suppress off "
> diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
> index 5132f514..cef489a4 100644
> --- a/man/man8/ip-link.8.in
> +++ b/man/man8/ip-link.8.in
> @@ -2155,7 +2155,7 @@ queries.
>  option above.
>  
>  .BR mcast_flood " { " on " | " off " }"
> -- controls whether a given port will be flooded with multicast traffic for which there is no MDB entry.
> +- controls whether a given port will flood multicast traffic for which there is no MDB entry.
>  
>  .BI group_fwd_mask " MASK "
>  - set the group forward mask. This is the bitmask that is applied to decide whether to forward incoming frames destined to link-local addresses, ie addresses of the form 01:80:C2:00:00:0X (defaults to 0, ie the bridge does not forward any link-local frames coming on this port).
> -- 
> 2.20.1
> 
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply


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