Netdev List
 help / color / mirror / Atom feed
* 3% Loan Offer Apply Now
From: Mr. Passy Ben @ 2018-09-09 15:10 UTC (permalink / raw)
  To: Recipients

ARE YOU IN NEED OF LOAN @3% INTEREST RATE FOR BUSINESS AND PRIVATE
PURPOSES? IF YES:
FILL AND RETURN
Name:===
Amount needed:===
Duration:==
country:===
Purpose:===
Mobile number

^ permalink raw reply

* Re: kernels > v4.12 oops/crash with ipsec-traffic: bisected to b838d5e1c5b6e57b10ec8af2268824041e3ea911: ipv4: mark DST_NOGC and remove the operation of dst_free()
From: Wolfgang Walter @ 2018-09-10 10:46 UTC (permalink / raw)
  To: Kristian Evensen
  Cc: Steffen Klassert, Network Development, weiwan, Tobias Hommel,
	edumazet
In-Reply-To: <CAKfDRXg7EJeoO--QFFEJLtSPqRVngy4etSQHjSkQwNnhc4RFRA@mail.gmail.com>

Am Montag, 10. September 2018, 10:18:47 schrieb Kristian Evensen:
> Hi,
> 
> Thanks everyone for all the effort in debugging this issue.
> 
> On Mon, Sep 10, 2018 at 8:39 AM Steffen Klassert
> 
> <steffen.klassert@secunet.com> wrote:
> > The easy fix that could be backported to stable would be
> > to check skb->dst for NULL and drop the packet in that case.
> 
> Thought I should just chime in and say that we deployed this
> work-around when we started observing the error back in June. Since
> then we have not seen any crashes. Also, we have instrumented some of
> our kernels to count the number of times the error is hit (overall +
> consecutive). Compared to the overall number of packets, the error
> happens very rarely. With our workloads, we on average see the error
> once every couple of days.
> 

Would you mind send us yout patch (with the accounting) so that we can check 
how often that happens here?

Regards,
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts

^ permalink raw reply

* Re: [PATCH] net: ipv4: Use BUG_ON directly instead of a if condition followed by BUG
From: Jiri Benc @ 2018-09-10 15:39 UTC (permalink / raw)
  To: zhong jiang; +Cc: davem, edumazet, kuznet, yoshfuji, netdev, linux-kernel
In-Reply-To: <1536590282-23899-1-git-send-email-zhongjiang@huawei.com>

On Mon, 10 Sep 2018 22:38:02 +0800, zhong jiang wrote:
> +			BUG(skb_copy_bits(skb, ptr, &tmp, 1));

I doubt this builds.

 Jiri

^ permalink raw reply

* Re: [net-next, PATCH 2/2, v1] net: socionext: add AF_XDP support
From: Toshiaki Makita @ 2018-09-10 10:56 UTC (permalink / raw)
  To: Ilias Apalodimas, netdev, jaswinder.singh
  Cc: ard.biesheuvel, masami.hiramatsu, arnd, mykyta.iziumtsev,
	bjorn.topel, magnus.karlsson
In-Reply-To: <1536567880-15097-3-git-send-email-ilias.apalodimas@linaro.org>

On 2018/09/10 17:24, Ilias Apalodimas wrote:
> Add basic AF_XDP support without zero-copy
> 
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
...
> @@ -707,6 +731,26 @@ static int netsec_process_rx(struct netsec_priv *priv, int budget)
>  		if (unlikely(!buf_addr))
>  			break;
>  
> +		if (xdp_prog) {
> +			xdp_result = netsec_run_xdp(desc, priv, xdp_prog,
> +						    pkt_len);
> +			if (xdp_result != NETSEC_XDP_PASS) {
> +				xdp_flush |= xdp_result & NETSEC_XDP_REDIR;
> +
> +				dma_unmap_single_attrs(priv->dev,
> +						       desc->dma_addr,
> +						       desc->len, DMA_TO_DEVICE,
> +						       DMA_ATTR_SKIP_CPU_SYNC);
> +
> +				desc->len = desc_len;
> +				desc->dma_addr = dma_handle;
> +				desc->addr = buf_addr;
> +				netsec_rx_fill(priv, idx, 1);
> +				nsetsec_adv_desc(&dring->tail);
> +			}
> +			continue;

Continue even on XDP_PASS? Is this really correct?

Also seems there is no handling of adjust_head/tail for XDP_PASS case.

> +		}
> +
>  		skb = build_skb(desc->addr, desc->len);
>  		if (unlikely(!skb)) {
>  			dma_unmap_single(priv->dev, dma_handle, desc_len,
> @@ -740,6 +784,9 @@ static int netsec_process_rx(struct netsec_priv *priv, int budget)
>  		nsetsec_adv_desc(&dring->tail);
>  	}
>  
> +	if (xdp_flush & NETSEC_XDP_REDIR)
> +		xdp_do_flush_map();
> +
>  	return done;
>  }
...
> +static u32 netsec_run_xdp(struct netsec_desc *desc, struct netsec_priv *priv,
> +			  struct bpf_prog *prog, u16 len)
> +
> +{
> +	struct netsec_desc_ring *dring = &priv->desc_ring[NETSEC_RING_RX];
> +	struct xdp_buff xdp;
> +	u32 ret = NETSEC_XDP_PASS;
> +	int err;
> +	u32 act;
> +
> +	xdp.data_hard_start = desc->addr;
> +	xdp.data = desc->addr;

There is no headroom. REDIRECT using devmap/cpumap will fail due to
this. Generally we need XDP_PACKET_HEADROOM.

> +	xdp_set_data_meta_invalid(&xdp);
> +	xdp.data_end = xdp.data + len;
> +	xdp.rxq = &dring->xdp_rxq;
> +
> +	rcu_read_lock();
> +	act = bpf_prog_run_xdp(prog, &xdp);
> +
> +	switch (act) {
> +	case XDP_PASS:
> +		ret = NETSEC_XDP_PASS;
> +		break;
> +	case XDP_TX:
> +		ret = netsec_xmit_xdp(priv, &xdp, desc);
> +		break;
> +	case XDP_REDIRECT:
> +		err = xdp_do_redirect(priv->ndev, &xdp, prog);
> +		if (!err) {
> +			ret = NETSEC_XDP_REDIR;
> +		} else {
> +			ret = NETSEC_XDP_CONSUMED;
> +			xdp_return_buff(&xdp);
> +		}
> +		break;
> +	default:
> +		bpf_warn_invalid_xdp_action(act);
> +		/* fall through */
> +	case XDP_ABORTED:
> +		trace_xdp_exception(priv->ndev, prog, act);
> +		/* fall through -- handle aborts by dropping packet */
> +	case XDP_DROP:
> +		ret = NETSEC_XDP_CONSUMED;
> +		break;
> +	}
> +
> +	rcu_read_unlock();
> +
> +	return ret;
> +}
> +
> +static int netsec_xdp_setup(struct netsec_priv *priv, struct bpf_prog *prog)
> +{
> +	struct net_device *dev = priv->ndev;
> +	struct bpf_prog *old_prog;
> +
> +	/* For now just support only the usual MTU sized frames */
> +	if (prog && dev->mtu > 1500) {
> +		netdev_warn(dev, "Jumbo frames not yet supported with XDP\n");

Why not using extack?

> +		return -EOPNOTSUPP;
> +	}
> +

-- 
Toshiaki Makita

^ permalink raw reply

* 3% Loan Offer Apply Now
From: Mr. Passy Ben @ 2018-09-09 17:00 UTC (permalink / raw)
  To: Recipients

ARE YOU IN NEED OF LOAN @3% INTEREST RATE FOR BUSINESS AND PRIVATE
PURPOSES? IF YES:
FILL AND RETURN
Name:===
Amount needed:===
Duration:==
country:===
Purpose:===
Mobile number

^ permalink raw reply

* Re: [net-next, PATCH 2/2, v1] net: socionext: add AF_XDP support
From: Ilias Apalodimas @ 2018-09-10 11:20 UTC (permalink / raw)
  To: Toshiaki Makita
  Cc: netdev, jaswinder.singh, ard.biesheuvel, masami.hiramatsu, arnd,
	mykyta.iziumtsev, bjorn.topel, magnus.karlsson
In-Reply-To: <8bfd8219-acea-8b63-b6be-d17a7e3b6e24@lab.ntt.co.jp>

On Mon, Sep 10, 2018 at 07:56:49PM +0900, Toshiaki Makita wrote:
> On 2018/09/10 17:24, Ilias Apalodimas wrote:
> > Add basic AF_XDP support without zero-copy
> > 
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > ---
> ...
> > @@ -707,6 +731,26 @@ static int netsec_process_rx(struct netsec_priv *priv, int budget)
> >  		if (unlikely(!buf_addr))
> >  			break;
> >  
> > +		if (xdp_prog) {
> > +			xdp_result = netsec_run_xdp(desc, priv, xdp_prog,
> > +						    pkt_len);
> > +			if (xdp_result != NETSEC_XDP_PASS) {
> > +				xdp_flush |= xdp_result & NETSEC_XDP_REDIR;
> > +
> > +				dma_unmap_single_attrs(priv->dev,
> > +						       desc->dma_addr,
> > +						       desc->len, DMA_TO_DEVICE,
> > +						       DMA_ATTR_SKIP_CPU_SYNC);
> > +
> > +				desc->len = desc_len;
> > +				desc->dma_addr = dma_handle;
> > +				desc->addr = buf_addr;
> > +				netsec_rx_fill(priv, idx, 1);
> > +				nsetsec_adv_desc(&dring->tail);
> > +			}
> > +			continue;
> 
> Continue even on XDP_PASS? Is this really correct?
> 
> Also seems there is no handling of adjust_head/tail for XDP_PASS case.
No continue is wrong there, thanks for the catch. This should return the packet
to the network stack. I'll fix it on v2.

> 
> > +		}
> > +
> >  		skb = build_skb(desc->addr, desc->len);
> >  		if (unlikely(!skb)) {
> >  			dma_unmap_single(priv->dev, dma_handle, desc_len,
> > @@ -740,6 +784,9 @@ static int netsec_process_rx(struct netsec_priv *priv, int budget)
> >  		nsetsec_adv_desc(&dring->tail);
> >  	}
> >  
> > +	if (xdp_flush & NETSEC_XDP_REDIR)
> > +		xdp_do_flush_map();
> > +
> >  	return done;
> >  }
> ...
> > +static u32 netsec_run_xdp(struct netsec_desc *desc, struct netsec_priv *priv,
> > +			  struct bpf_prog *prog, u16 len)
> > +
> > +{
> > +	struct netsec_desc_ring *dring = &priv->desc_ring[NETSEC_RING_RX];
> > +	struct xdp_buff xdp;
> > +	u32 ret = NETSEC_XDP_PASS;
> > +	int err;
> > +	u32 act;
> > +
> > +	xdp.data_hard_start = desc->addr;
> > +	xdp.data = desc->addr;
> 
> There is no headroom. REDIRECT using devmap/cpumap will fail due to
> this. Generally we need XDP_PACKET_HEADROOM.
> 
Ok, i'll modify the patch to include XDP_PACKET_HEADROOM
> > +	xdp_set_data_meta_invalid(&xdp);
> > +	xdp.data_end = xdp.data + len;
> > +	xdp.rxq = &dring->xdp_rxq;
> > +
> > +	rcu_read_lock();
> > +	act = bpf_prog_run_xdp(prog, &xdp);
> > +
> > +	switch (act) {
> > +	case XDP_PASS:
> > +		ret = NETSEC_XDP_PASS;
> > +		break;
> > +	case XDP_TX:
> > +		ret = netsec_xmit_xdp(priv, &xdp, desc);
> > +		break;
> > +	case XDP_REDIRECT:
> > +		err = xdp_do_redirect(priv->ndev, &xdp, prog);
> > +		if (!err) {
> > +			ret = NETSEC_XDP_REDIR;
> > +		} else {
> > +			ret = NETSEC_XDP_CONSUMED;
> > +			xdp_return_buff(&xdp);
> > +		}
> > +		break;
> > +	default:
> > +		bpf_warn_invalid_xdp_action(act);
> > +		/* fall through */
> > +	case XDP_ABORTED:
> > +		trace_xdp_exception(priv->ndev, prog, act);
> > +		/* fall through -- handle aborts by dropping packet */
> > +	case XDP_DROP:
> > +		ret = NETSEC_XDP_CONSUMED;
> > +		break;
> > +	}
> > +
> > +	rcu_read_unlock();
> > +
> > +	return ret;
> > +}
> > +
> > +static int netsec_xdp_setup(struct netsec_priv *priv, struct bpf_prog *prog)
> > +{
> > +	struct net_device *dev = priv->ndev;
> > +	struct bpf_prog *old_prog;
> > +
> > +	/* For now just support only the usual MTU sized frames */
> > +	if (prog && dev->mtu > 1500) {
> > +		netdev_warn(dev, "Jumbo frames not yet supported with XDP\n");
> 
> Why not using extack?
> 
Wasn't aware that this should use the extack for errors. I just saw the same
check on an existing driver and thought that was the way to go
> > +		return -EOPNOTSUPP;
> > +	}
> > +

Thanks for having a look!

/Ilias

^ permalink raw reply

* Re: [PATCH net 00/13] cls_u32 cleanups and fixes.
From: Jamal Hadi Salim @ 2018-09-10 11:30 UTC (permalink / raw)
  To: Al Viro; +Cc: netdev, Cong Wang, Jiri Pirko
In-Reply-To: <20180909141538.GG19965@ZenIV.linux.org.uk>

On 2018-09-09 10:15 a.m., Al Viro wrote:

[..]

> Umm...  Interesting - TCA_U32_SEL is not the only thing that
> gets ignored there; TCA_U32_MARK gets the same treatment.
> And then there's a lovely question what to do with n->pf -
> it's an array of n->sel.nkeys counters, and apparently we
> want (at least in common cases) to avoid resetting those.

> *If* we declare that ->nkeys mismatch means failure, it's
> all relatively easy to implement.  Alternatively, we could
> declare that selector change means resetting the stats.
> Preferences?

I am now conflicted. I have sample scripts which showed
that at one point that worked. All of them seem to be
indicating the nkeys and stats never changed. So that
could be a starting point; however, if a policy changes
the match tuples then it is no longer the same rule imo.

Note: you can change the "actions" - of which the most
primitive is "set classid x:y" without changing what is
being matched. i.e changing the classid in that example
would work.

cheers,
jamal

^ permalink raw reply

* Re: [net-next, v2, 1/2] net: stmmac: Rework coalesce timer and fix multi-queue races
From: Neil Armstrong @ 2018-09-10 11:43 UTC (permalink / raw)
  To: Jose Abreu, netdev
  Cc: Jerome Brunet, Martin Blumenstingl, David S. Miller, Joao Pinto,
	Giuseppe Cavallaro, Alexandre Torgue
In-Reply-To: <c54230d2dc7e2740c039e0f5e8f306a7544bbe5e.1536570319.git.joabreu@synopsys.com>

Hi Jose,

On 10/09/2018 11:14, Jose Abreu wrote:
> This follows David Miller advice and tries to fix coalesce timer in
> multi-queue scenarios.
> 
> We are now using per-queue coalesce values and per-queue TX timer.
> 
> Coalesce timer default values was changed to 1ms and the coalesce frames
> to 25.
> 
> Tested in B2B setup between XGMAC2 and GMAC5.
> 
> Signed-off-by: Jose Abreu <joabreu@synopsys.com>
> Cc: Jerome Brunet <jbrunet@baylibre.com>
> Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Joao Pinto <jpinto@synopsys.com>
> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> Cc: Alexandre Torgue <alexandre.torgue@st.com>
> ---
> Jerome,

Jerome is in holidays...

> 
> Can you please test if this one is okay ?


I tested this patch on top of 4.18.7 with the previous patch (4ae0169fd1b3) reverted.

The TX or RX stopped a few seconds after iperf3 started :
(iperf3 is running in server mode on the Amlogic A113D board)

$ iperf3 -c 10.1.4.95
Connecting to host 10.1.4.95, port 5201
[  4] local 10.1.2.12 port 39906 connected to 10.1.4.95 port 5201
[ ID] Interval           Transfer     Bandwidth       Retr  Cwnd
[  4]   0.00-1.00   sec  56.2 MBytes   472 Mbits/sec    0    660 KBytes
[  4]   1.00-2.00   sec  56.2 MBytes   472 Mbits/sec    0    660 KBytes
[  4]   2.00-3.00   sec  38.8 MBytes   325 Mbits/sec    1   1.41 KBytes
[  4]   3.00-4.00   sec  0.00 Bytes  0.00 bits/sec    1   1.41 KBytes
[  4]   4.00-5.00   sec  0.00 Bytes  0.00 bits/sec    1   1.41 KBytes
^C[  4]   5.00-5.61   sec  0.00 Bytes  0.00 bits/sec    0   1.41 KBytes
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bandwidth       Retr
[  4]   0.00-5.61   sec   151 MBytes   226 Mbits/sec    3             sender
[  4]   0.00-5.61   sec  0.00 Bytes  0.00 bits/sec                  receiver
iperf3: interrupt - the client has terminated

$ iperf3 -c 10.1.4.95 -R
Connecting to host 10.1.4.95, port 5201
Reverse mode, remote host 10.1.4.95 is sending
[  4] local 10.1.2.12 port 39894 connected to 10.1.4.95 port 5201
[ ID] Interval           Transfer     Bandwidth
[  4]   0.00-1.00   sec  82.2 MBytes   690 Mbits/sec
[  4]   1.00-2.00   sec  82.6 MBytes   693 Mbits/sec
[  4]   2.00-3.00   sec  24.2 MBytes   203 Mbits/sec
[  4]   3.00-4.00   sec  0.00 Bytes  0.00 bits/sec
[  4]   4.00-5.00   sec  0.00 Bytes  0.00 bits/sec
^C[  4]   5.00-5.53   sec  0.00 Bytes  0.00 bits/sec
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bandwidth
[  4]   0.00-5.53   sec  0.00 Bytes  0.00 bits/sec                  sender
[  4]   0.00-5.53   sec   189 MBytes   287 Mbits/sec                  receiver
iperf3: interrupt - the client has terminated

These works for hours without this patch applied.

Neil

> 
> Thanks and Best Regards,
> Jose Miguel Abreu
> ---
>  drivers/net/ethernet/stmicro/stmmac/common.h      |   4 +-
>  drivers/net/ethernet/stmicro/stmmac/stmmac.h      |   6 +-
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 207 ++++++++++++++--------
>  3 files changed, 135 insertions(+), 82 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
> index 1854f270ad66..b1b305f8f414 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
> @@ -258,10 +258,10 @@ struct stmmac_safety_stats {
>  #define MAX_DMA_RIWT		0xff
>  #define MIN_DMA_RIWT		0x20
>  /* Tx coalesce parameters */
> -#define STMMAC_COAL_TX_TIMER	40000
> +#define STMMAC_COAL_TX_TIMER	1000
>  #define STMMAC_MAX_COAL_TX_TICK	100000
>  #define STMMAC_TX_MAX_FRAMES	256
> -#define STMMAC_TX_FRAMES	64
> +#define STMMAC_TX_FRAMES	25
>  
>  /* Packets types */
>  enum packets_types {
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> index c0a855b7ab3b..957030cfb833 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> @@ -48,6 +48,9 @@ struct stmmac_tx_info {
>  
>  /* Frequently used values are kept adjacent for cache effect */
>  struct stmmac_tx_queue {
> +	u32 tx_count_frames;
> +	int tx_timer_active;
> +	struct timer_list txtimer;
>  	u32 queue_index;
>  	struct stmmac_priv *priv_data;
>  	struct dma_extended_desc *dma_etx ____cacheline_aligned_in_smp;
> @@ -59,6 +62,7 @@ struct stmmac_tx_queue {
>  	dma_addr_t dma_tx_phy;
>  	u32 tx_tail_addr;
>  	u32 mss;
> +	struct napi_struct napi ____cacheline_aligned_in_smp;
>  };
>  
>  struct stmmac_rx_queue {
> @@ -109,14 +113,12 @@ struct stmmac_pps_cfg {
>  
>  struct stmmac_priv {
>  	/* Frequently used values are kept adjacent for cache effect */
> -	u32 tx_count_frames;
>  	u32 tx_coal_frames;
>  	u32 tx_coal_timer;
>  
>  	int tx_coalesce;
>  	int hwts_tx_en;
>  	bool tx_path_in_lpi_mode;
> -	struct timer_list txtimer;
>  	bool tso;
>  
>  	unsigned int dma_buf_sz;
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 9f458bb16f2a..9809c2b319fe 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -148,6 +148,7 @@ static void stmmac_verify_args(void)
>  static void stmmac_disable_all_queues(struct stmmac_priv *priv)
>  {
>  	u32 rx_queues_cnt = priv->plat->rx_queues_to_use;
> +	u32 tx_queues_cnt = priv->plat->tx_queues_to_use;
>  	u32 queue;
>  
>  	for (queue = 0; queue < rx_queues_cnt; queue++) {
> @@ -155,6 +156,12 @@ static void stmmac_disable_all_queues(struct stmmac_priv *priv)
>  
>  		napi_disable(&rx_q->napi);
>  	}
> +
> +	for (queue = 0; queue < tx_queues_cnt; queue++) {
> +		struct stmmac_tx_queue *tx_q = &priv->tx_queue[queue];
> +
> +		napi_disable(&tx_q->napi);
> +	}
>  }
>  
>  /**
> @@ -164,6 +171,7 @@ static void stmmac_disable_all_queues(struct stmmac_priv *priv)
>  static void stmmac_enable_all_queues(struct stmmac_priv *priv)
>  {
>  	u32 rx_queues_cnt = priv->plat->rx_queues_to_use;
> +	u32 tx_queues_cnt = priv->plat->tx_queues_to_use;
>  	u32 queue;
>  
>  	for (queue = 0; queue < rx_queues_cnt; queue++) {
> @@ -171,6 +179,12 @@ static void stmmac_enable_all_queues(struct stmmac_priv *priv)
>  
>  		napi_enable(&rx_q->napi);
>  	}
> +
> +	for (queue = 0; queue < tx_queues_cnt; queue++) {
> +		struct stmmac_tx_queue *tx_q = &priv->tx_queue[queue];
> +
> +		napi_enable(&tx_q->napi);
> +	}
>  }
>  
>  /**
> @@ -1843,7 +1857,8 @@ static void stmmac_dma_operation_mode(struct stmmac_priv *priv)
>   * @queue: TX queue index
>   * Description: it reclaims the transmit resources after transmission completes.
>   */
> -static void stmmac_tx_clean(struct stmmac_priv *priv, u32 queue)
> +static int stmmac_tx_clean(struct stmmac_priv *priv, int limit, u32 queue,
> +			   bool *more)
>  {
>  	struct stmmac_tx_queue *tx_q = &priv->tx_queue[queue];
>  	unsigned int bytes_compl = 0, pkts_compl = 0;
> @@ -1851,10 +1866,13 @@ static void stmmac_tx_clean(struct stmmac_priv *priv, u32 queue)
>  
>  	netif_tx_lock(priv->dev);
>  
> +	if (more)
> +		*more = false;
> +
>  	priv->xstats.tx_clean++;
>  
>  	entry = tx_q->dirty_tx;
> -	while (entry != tx_q->cur_tx) {
> +	while ((entry != tx_q->cur_tx) && (pkts_compl < limit)) {
>  		struct sk_buff *skb = tx_q->tx_skbuff[entry];
>  		struct dma_desc *p;
>  		int status;
> @@ -1937,7 +1955,13 @@ static void stmmac_tx_clean(struct stmmac_priv *priv, u32 queue)
>  		stmmac_enable_eee_mode(priv);
>  		mod_timer(&priv->eee_ctrl_timer, STMMAC_LPI_T(eee_timer));
>  	}
> +
> +	if (more && (tx_q->dirty_tx != tx_q->cur_tx))
> +		*more = true;
> +
>  	netif_tx_unlock(priv->dev);
> +
> +	return pkts_compl;
>  }
>  
>  /**
> @@ -2020,6 +2044,34 @@ static bool stmmac_safety_feat_interrupt(struct stmmac_priv *priv)
>  	return false;
>  }
>  
> +static int stmmac_napi_check(struct stmmac_priv *priv, u32 chan)
> +{
> +	int status = stmmac_dma_interrupt_status(priv, priv->ioaddr,
> +						 &priv->xstats, chan);
> +
> +	if ((status & handle_rx) && (chan < priv->plat->rx_queues_to_use)) {
> +		struct stmmac_rx_queue *rx_q = &priv->rx_queue[chan];
> +
> +		if (likely(napi_schedule_prep(&rx_q->napi))) {
> +			stmmac_disable_dma_irq(priv, priv->ioaddr, chan);
> +			__napi_schedule(&rx_q->napi);
> +		}
> +	} else {
> +		status &= ~handle_rx;
> +	}
> +
> +	if ((status & handle_tx) && (chan < priv->plat->tx_queues_to_use)) {
> +		struct stmmac_tx_queue *tx_q = &priv->tx_queue[chan];
> +
> +		if (likely(napi_schedule_prep(&tx_q->napi)))
> +			__napi_schedule(&tx_q->napi);
> +	} else {
> +		status &= ~handle_tx;
> +	}
> +
> +	return status;
> +}
> +
>  /**
>   * stmmac_dma_interrupt - DMA ISR
>   * @priv: driver private structure
> @@ -2034,57 +2086,14 @@ static void stmmac_dma_interrupt(struct stmmac_priv *priv)
>  	u32 channels_to_check = tx_channel_count > rx_channel_count ?
>  				tx_channel_count : rx_channel_count;
>  	u32 chan;
> -	bool poll_scheduled = false;
>  	int status[max_t(u32, MTL_MAX_TX_QUEUES, MTL_MAX_RX_QUEUES)];
>  
>  	/* Make sure we never check beyond our status buffer. */
>  	if (WARN_ON_ONCE(channels_to_check > ARRAY_SIZE(status)))
>  		channels_to_check = ARRAY_SIZE(status);
>  
> -	/* Each DMA channel can be used for rx and tx simultaneously, yet
> -	 * napi_struct is embedded in struct stmmac_rx_queue rather than in a
> -	 * stmmac_channel struct.
> -	 * Because of this, stmmac_poll currently checks (and possibly wakes)
> -	 * all tx queues rather than just a single tx queue.
> -	 */
>  	for (chan = 0; chan < channels_to_check; chan++)
> -		status[chan] = stmmac_dma_interrupt_status(priv, priv->ioaddr,
> -				&priv->xstats, chan);
> -
> -	for (chan = 0; chan < rx_channel_count; chan++) {
> -		if (likely(status[chan] & handle_rx)) {
> -			struct stmmac_rx_queue *rx_q = &priv->rx_queue[chan];
> -
> -			if (likely(napi_schedule_prep(&rx_q->napi))) {
> -				stmmac_disable_dma_irq(priv, priv->ioaddr, chan);
> -				__napi_schedule(&rx_q->napi);
> -				poll_scheduled = true;
> -			}
> -		}
> -	}
> -
> -	/* If we scheduled poll, we already know that tx queues will be checked.
> -	 * If we didn't schedule poll, see if any DMA channel (used by tx) has a
> -	 * completed transmission, if so, call stmmac_poll (once).
> -	 */
> -	if (!poll_scheduled) {
> -		for (chan = 0; chan < tx_channel_count; chan++) {
> -			if (status[chan] & handle_tx) {
> -				/* It doesn't matter what rx queue we choose
> -				 * here. We use 0 since it always exists.
> -				 */
> -				struct stmmac_rx_queue *rx_q =
> -					&priv->rx_queue[0];
> -
> -				if (likely(napi_schedule_prep(&rx_q->napi))) {
> -					stmmac_disable_dma_irq(priv,
> -							priv->ioaddr, chan);
> -					__napi_schedule(&rx_q->napi);
> -				}
> -				break;
> -			}
> -		}
> -	}
> +		status[chan] = stmmac_napi_check(priv, chan);
>  
>  	for (chan = 0; chan < tx_channel_count; chan++) {
>  		if (unlikely(status[chan] & tx_hard_error_bump_tc)) {
> @@ -2241,13 +2250,11 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
>   */
>  static void stmmac_tx_timer(struct timer_list *t)
>  {
> -	struct stmmac_priv *priv = from_timer(priv, t, txtimer);
> -	u32 tx_queues_count = priv->plat->tx_queues_to_use;
> -	u32 queue;
> +	struct stmmac_tx_queue *tx_q = from_timer(tx_q, t, txtimer);
>  
> -	/* let's scan all the tx queues */
> -	for (queue = 0; queue < tx_queues_count; queue++)
> -		stmmac_tx_clean(priv, queue);
> +	if (likely(napi_schedule_prep(&tx_q->napi)))
> +		__napi_schedule(&tx_q->napi);
> +	tx_q->tx_timer_active = 0;
>  }
>  
>  /**
> @@ -2260,11 +2267,17 @@ static void stmmac_tx_timer(struct timer_list *t)
>   */
>  static void stmmac_init_tx_coalesce(struct stmmac_priv *priv)
>  {
> +	u32 tx_channel_count = priv->plat->tx_queues_to_use;
> +	u32 chan;
> +
>  	priv->tx_coal_frames = STMMAC_TX_FRAMES;
>  	priv->tx_coal_timer = STMMAC_COAL_TX_TIMER;
> -	timer_setup(&priv->txtimer, stmmac_tx_timer, 0);
> -	priv->txtimer.expires = STMMAC_COAL_TIMER(priv->tx_coal_timer);
> -	add_timer(&priv->txtimer);
> +
> +	for (chan = 0; chan < tx_channel_count; chan++) {
> +		struct stmmac_tx_queue *tx_q = &priv->tx_queue[chan];
> +
> +		timer_setup(&tx_q->txtimer, stmmac_tx_timer, 0);
> +	}
>  }
>  
>  static void stmmac_set_rings_length(struct stmmac_priv *priv)
> @@ -2592,6 +2605,7 @@ static void stmmac_hw_teardown(struct net_device *dev)
>  static int stmmac_open(struct net_device *dev)
>  {
>  	struct stmmac_priv *priv = netdev_priv(dev);
> +	u32 chan;
>  	int ret;
>  
>  	stmmac_check_ether_addr(priv);
> @@ -2688,7 +2702,9 @@ static int stmmac_open(struct net_device *dev)
>  	if (dev->phydev)
>  		phy_stop(dev->phydev);
>  
> -	del_timer_sync(&priv->txtimer);
> +	for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++)
> +		del_timer_sync(&priv->tx_queue[chan].txtimer);
> +
>  	stmmac_hw_teardown(dev);
>  init_error:
>  	free_dma_desc_resources(priv);
> @@ -2708,6 +2724,7 @@ static int stmmac_open(struct net_device *dev)
>  static int stmmac_release(struct net_device *dev)
>  {
>  	struct stmmac_priv *priv = netdev_priv(dev);
> +	u32 chan;
>  
>  	if (priv->eee_enabled)
>  		del_timer_sync(&priv->eee_ctrl_timer);
> @@ -2722,7 +2739,8 @@ static int stmmac_release(struct net_device *dev)
>  
>  	stmmac_disable_all_queues(priv);
>  
> -	del_timer_sync(&priv->txtimer);
> +	for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++)
> +		del_timer_sync(&priv->tx_queue[chan].txtimer);
>  
>  	/* Free the IRQ lines */
>  	free_irq(dev->irq, dev);
> @@ -2936,14 +2954,11 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)
>  	priv->xstats.tx_tso_nfrags += nfrags;
>  
>  	/* Manage tx mitigation */
> -	priv->tx_count_frames += nfrags + 1;
> -	if (likely(priv->tx_coal_frames > priv->tx_count_frames)) {
> -		mod_timer(&priv->txtimer,
> -			  STMMAC_COAL_TIMER(priv->tx_coal_timer));
> -	} else {
> -		priv->tx_count_frames = 0;
> +	tx_q->tx_count_frames += nfrags + 1;
> +	if (priv->tx_coal_frames <= tx_q->tx_count_frames) {
>  		stmmac_set_tx_ic(priv, desc);
>  		priv->xstats.tx_set_ic_bit++;
> +		tx_q->tx_count_frames = 0;
>  	}
>  
>  	skb_tx_timestamp(skb);
> @@ -2994,6 +3009,12 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)
>  
>  	stmmac_set_tx_tail_ptr(priv, priv->ioaddr, tx_q->tx_tail_addr, queue);
>  
> +	if (priv->tx_coal_timer && !tx_q->tx_timer_active) {
> +		tx_q->tx_timer_active = 1;
> +		mod_timer(&tx_q->txtimer,
> +				STMMAC_COAL_TIMER(priv->tx_coal_timer));
> +	}
> +
>  	return NETDEV_TX_OK;
>  
>  dma_map_err:
> @@ -3146,14 +3167,11 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
>  	 * This approach takes care about the fragments: desc is the first
>  	 * element in case of no SG.
>  	 */
> -	priv->tx_count_frames += nfrags + 1;
> -	if (likely(priv->tx_coal_frames > priv->tx_count_frames)) {
> -		mod_timer(&priv->txtimer,
> -			  STMMAC_COAL_TIMER(priv->tx_coal_timer));
> -	} else {
> -		priv->tx_count_frames = 0;
> +	tx_q->tx_count_frames += nfrags + 1;
> +	if (priv->tx_coal_frames <= tx_q->tx_count_frames) {
>  		stmmac_set_tx_ic(priv, desc);
>  		priv->xstats.tx_set_ic_bit++;
> +		tx_q->tx_count_frames = 0;
>  	}
>  
>  	skb_tx_timestamp(skb);
> @@ -3199,8 +3217,15 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
>  	netdev_tx_sent_queue(netdev_get_tx_queue(dev, queue), skb->len);
>  
>  	stmmac_enable_dma_transmission(priv, priv->ioaddr);
> +
>  	stmmac_set_tx_tail_ptr(priv, priv->ioaddr, tx_q->tx_tail_addr, queue);
>  
> +	if (priv->tx_coal_timer && !tx_q->tx_timer_active) {
> +		tx_q->tx_timer_active = 1;
> +		mod_timer(&tx_q->txtimer,
> +				STMMAC_COAL_TIMER(priv->tx_coal_timer));
> +	}
> +
>  	return NETDEV_TX_OK;
>  
>  dma_map_err:
> @@ -3514,27 +3539,41 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
>   *  Description :
>   *  To look at the incoming frames and clear the tx resources.
>   */
> -static int stmmac_poll(struct napi_struct *napi, int budget)
> +static int stmmac_rx_poll(struct napi_struct *napi, int budget)
>  {
>  	struct stmmac_rx_queue *rx_q =
>  		container_of(napi, struct stmmac_rx_queue, napi);
>  	struct stmmac_priv *priv = rx_q->priv_data;
> -	u32 tx_count = priv->plat->tx_queues_to_use;
>  	u32 chan = rx_q->queue_index;
>  	int work_done = 0;
> -	u32 queue;
>  
>  	priv->xstats.napi_poll++;
>  
> -	/* check all the queues */
> -	for (queue = 0; queue < tx_count; queue++)
> -		stmmac_tx_clean(priv, queue);
> -
>  	work_done = stmmac_rx(priv, budget, rx_q->queue_index);
> +	if (work_done < budget && napi_complete_done(napi, work_done))
> +		stmmac_enable_dma_irq(priv, priv->ioaddr, chan);
> +
> +	return work_done;
> +}
> +
> +static int stmmac_tx_poll(struct napi_struct *napi, int budget)
> +{
> +	struct stmmac_tx_queue *tx_q =
> +		container_of(napi, struct stmmac_tx_queue, napi);
> +	struct stmmac_priv *priv = tx_q->priv_data;
> +	u32 chan = tx_q->queue_index;
> +	int work_done = 0;
> +	bool more;
> +
> +	priv->xstats.napi_poll++;
> +
> +	work_done = stmmac_tx_clean(priv, budget, chan, &more);
>  	if (work_done < budget) {
>  		napi_complete_done(napi, work_done);
> -		stmmac_enable_dma_irq(priv, priv->ioaddr, chan);
> +		if (more)
> +			napi_reschedule(napi);
>  	}
> +
>  	return work_done;
>  }
>  
> @@ -4325,10 +4364,17 @@ int stmmac_dvr_probe(struct device *device,
>  	for (queue = 0; queue < priv->plat->rx_queues_to_use; queue++) {
>  		struct stmmac_rx_queue *rx_q = &priv->rx_queue[queue];
>  
> -		netif_napi_add(ndev, &rx_q->napi, stmmac_poll,
> +		netif_napi_add(ndev, &rx_q->napi, stmmac_rx_poll,
>  			       (8 * priv->plat->rx_queues_to_use));
>  	}
>  
> +	for (queue = 0; queue < priv->plat->tx_queues_to_use; queue++) {
> +		struct stmmac_tx_queue *tx_q = &priv->tx_queue[queue];
> +
> +		netif_napi_add(ndev, &tx_q->napi, stmmac_tx_poll,
> +			       (8 * priv->plat->tx_queues_to_use));
> +	}
> +
>  	mutex_init(&priv->lock);
>  
>  	/* If a specific clk_csr value is passed from the platform
> @@ -4377,6 +4423,11 @@ int stmmac_dvr_probe(struct device *device,
>  
>  		netif_napi_del(&rx_q->napi);
>  	}
> +	for (queue = 0; queue < priv->plat->tx_queues_to_use; queue++) {
> +		struct stmmac_tx_queue *tx_q = &priv->tx_queue[queue];
> +
> +		netif_napi_del(&tx_q->napi);
> +	}
>  error_hw_init:
>  	destroy_workqueue(priv->wq);
>  error_wq:
> 

^ permalink raw reply

* Re: [PATCH] r8169: Clear RTL_FLAG_TASK_*_PENDING when clearing RTL_FLAG_TASK_ENABLED
From: Florian Fainelli @ 2018-09-10 16:42 UTC (permalink / raw)
  To: Kai-Heng Feng, nic_swsd; +Cc: davem, netdev, linux-kernel, Heiner Kallweit
In-Reply-To: <20180910063436.2326-1-kai.heng.feng@canonical.com>

On 09/09/2018 11:34 PM, Kai-Heng Feng wrote:
> After system suspend, sometimes the r8169 doesn't work when ethernet
> cable gets pluggued.
> 
> This issue happens because rtl_reset_work() doesn't get called from
> rtl8169_runtime_resume(), after system suspend.
> 
> In rtl_task(), RTL_FLAG_TASK_* only gets cleared if this condition is
> met:
> if (!netif_running(dev) ||
>     !test_bit(RTL_FLAG_TASK_ENABLED, tp->wk.flags))
>     ...
> 
> If RTL_FLAG_TASK_ENABLED was cleared during system suspend while
> RTL_FLAG_TASK_RESET_PENDING was set, the next rtl_schedule_task() won't
> schedule task as the flag is still there.
> 
> So in addition to clearing RTL_FLAG_TASK_ENABLED, also clears other
> flags.
> 
> Cc: Heiner Kallweit <hkallweit1@gmail.com>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
>  drivers/net/ethernet/realtek/r8169.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
> index b08d51bf7a20..20593245ef53 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -6648,6 +6648,7 @@ static int rtl8169_close(struct net_device *dev)
>  {
>  	struct rtl8169_private *tp = netdev_priv(dev);
>  	struct pci_dev *pdev = tp->pci_dev;
> +	int i;
>  
>  	pm_runtime_get_sync(&pdev->dev);
>  
> @@ -6655,7 +6656,9 @@ static int rtl8169_close(struct net_device *dev)
>  	rtl8169_update_counters(tp);
>  
>  	rtl_lock_work(tp);
> -	clear_bit(RTL_FLAG_TASK_ENABLED, tp->wk.flags);
> +	/* Clear all task flags */
> +	for (i = 0; i < RTL_FLAG_MAX; i++)
> +		clear_bit(i, tp->wk.flags);

bitmap_zero(&tp->wk.,flagsm RTL_FLAG_MAX)

>  
>  	rtl8169_down(dev);
>  	rtl_unlock_work(tp);
> @@ -6828,6 +6831,7 @@ rtl8169_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
>  static void rtl8169_net_suspend(struct net_device *dev)
>  {
>  	struct rtl8169_private *tp = netdev_priv(dev);
> +	int i;
>  
>  	if (!netif_running(dev))
>  		return;
> @@ -6838,7 +6842,10 @@ static void rtl8169_net_suspend(struct net_device *dev)
>  
>  	rtl_lock_work(tp);
>  	napi_disable(&tp->napi);
> -	clear_bit(RTL_FLAG_TASK_ENABLED, tp->wk.flags);
> +	/* Clear all task flags */
> +	for (i = 0; i < RTL_FLAG_MAX; i++)
> +		clear_bit(i, tp->wk.flags);

Likewise.

> +
>  	rtl_unlock_work(tp);
>  
>  	rtl_pll_power_down(tp);
> 


-- 
Florian

^ permalink raw reply

* Re: [PATCH iproute2 3/3] bridge: fix vlan show formatting
From: Tobias Jungel @ 2018-09-10 11:52 UTC (permalink / raw)
  To: Stephen Hemminger, netdev
In-Reply-To: <20180906153057.5379-3-stephen@networkplumber.org>

Hi Stephen,

I just have seen this patch, hence please ignore the "bridge: Correct
json output" I sent. This one solves the issue in a way more elegant
manor.

I tested the JSON output in this series and it works as intended.

Thanks


Reviewed-by: Tobias Jungel <tobias.jungel@bisdn.de>


On Thu, 2018-09-06 at 16:30 +0100, Stephen Hemminger wrote:
> The output of vlan show was broken previous change to use json_print.
> Clean the code up and return to original format.
> 
> Note: the JSON syntax has changed to make the bridge vlan
> show more like other outputs (e.g. ip -j li show).
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  bridge/br_common.h |  2 +-
>  bridge/link.c      |  6 ++---
>  bridge/vlan.c      | 61 +++++++++++++++++++++++++++-----------------
> --
>  3 files changed, 40 insertions(+), 29 deletions(-)
> 
> diff --git a/bridge/br_common.h b/bridge/br_common.h
> index 2f1cb8fd9f3d..69665fde32b6 100644
> --- a/bridge/br_common.h
> +++ b/bridge/br_common.h
> @@ -6,7 +6,7 @@
>  #define MDB_RTR_RTA(r) \
>  		((struct rtattr *)(((char *)(r)) +
> RTA_ALIGN(sizeof(__u32))))
>  
> -extern void print_vlan_info(FILE *fp, struct rtattr *tb);
> +extern void print_vlan_info(struct rtattr *tb, int ifindex);

nitpick, this does not apply to master.

>  extern int print_linkinfo(const struct sockaddr_nl *who,
>  			  struct nlmsghdr *n,
>  			  void *arg);
> diff --git a/bridge/link.c b/bridge/link.c
> index 8d89aca2e638..a5ee9a5c58e6 100644
> --- a/bridge/link.c
> +++ b/bridge/link.c
> @@ -161,7 +161,7 @@ static void print_protinfo(FILE *fp, struct
> rtattr *attr)
>   * This is reported by HW devices that have some bridging
>   * capabilities.
>   */
> -static void print_af_spec(FILE *fp, struct rtattr *attr)
> +static void print_af_spec(struct rtattr *attr, int ifindex)
>  {
>  	struct rtattr *aftb[IFLA_BRIDGE_MAX+1];
>  
> @@ -174,7 +174,7 @@ static void print_af_spec(FILE *fp, struct rtattr
> *attr)
>  		return;
>  
>  	if (aftb[IFLA_BRIDGE_VLAN_INFO])
> -		print_vlan_info(fp, aftb[IFLA_BRIDGE_VLAN_INFO]);
> +		print_vlan_info(aftb[IFLA_BRIDGE_VLAN_INFO], ifindex);
>  }
>  
>  int print_linkinfo(const struct sockaddr_nl *who,
> @@ -229,7 +229,7 @@ int print_linkinfo(const struct sockaddr_nl *who,
>  		print_protinfo(fp, tb[IFLA_PROTINFO]);
>  
>  	if (tb[IFLA_AF_SPEC])
> -		print_af_spec(fp, tb[IFLA_AF_SPEC]);
> +		print_af_spec(tb[IFLA_AF_SPEC], ifi->ifi_index);
>  
>  	print_string(PRINT_FP, NULL, "%s", "\n");
>  	close_json_object();
> diff --git a/bridge/vlan.c b/bridge/vlan.c
> index 19a36b804069..bdce55ae4e14 100644
> --- a/bridge/vlan.c
> +++ b/bridge/vlan.c
> @@ -252,10 +252,18 @@ static int filter_vlan_check(__u16 vid, __u16
> flags)
>  	return 1;
>  }
>  
> -static void print_vlan_port(FILE *fp, int ifi_index)
> +static void open_vlan_port(int ifi_index)
>  {
> -	print_string(PRINT_ANY, NULL, "%s",
> +	open_json_object(NULL);
> +	print_string(PRINT_ANY, "ifname", "%s",
>  		     ll_index_to_name(ifi_index));
> +	open_json_array(PRINT_JSON, "vlans");
> +}
> +
> +static void close_vlan_port(void)
> +{
> +	close_json_array(PRINT_JSON, NULL);
> +	close_json_object();
>  }
>  
>  static void print_range(const char *name, __u16 start, __u16 id)
> @@ -278,7 +286,7 @@ static void print_vlan_tunnel_info(FILE *fp,
> struct rtattr *tb, int ifindex)
>  	__u32 last_tunid_start = 0;
>  
>  	if (!filter_vlan)
> -		print_vlan_port(fp, ifindex);
> +		open_vlan_port(ifindex);
>  
>  	open_json_array(PRINT_JSON, "tunnel");
>  	for (i = RTA_DATA(list); RTA_OK(i, rem); i = RTA_NEXT(i, rem))
> {
> @@ -323,18 +331,20 @@ static void print_vlan_tunnel_info(FILE *fp,
> struct rtattr *tb, int ifindex)
>  			continue;
>  
>  		if (filter_vlan)
> -			print_vlan_port(fp, ifindex);
> +			open_vlan_port(ifindex);
>  
>  		open_json_object(NULL);
>  		print_range("vlan", last_vid_start, tunnel_vid);
>  		print_range("tunid", last_tunid_start, tunnel_id);
>  		close_json_object();
>  
> -		if (!is_json_context())
> -			fprintf(fp, "\n");
> -
> +		print_string(PRINT_FP, NULL, "%s", _SL_);
> +		if (filter_vlan)
> +			close_vlan_port();
>  	}
> -	close_json_array(PRINT_JSON, NULL);
> +
> +	if (!filter_vlan)
> +		close_vlan_port();
>  }
>  
>  static int print_vlan_tunnel(const struct sockaddr_nl *who,
> @@ -421,8 +431,8 @@ static int print_vlan(const struct sockaddr_nl
> *who,
>  		return 0;
>  	}
>  
> -	print_vlan_port(fp, ifm->ifi_index);
> -	print_vlan_info(fp, tb[IFLA_AF_SPEC]);
> +	print_vlan_info(tb[IFLA_AF_SPEC], ifm->ifi_index);
> +	print_string(PRINT_FP, NULL, "%s", _SL_);
>  
>  	fflush(fp);
>  	return 0;
> @@ -430,11 +440,16 @@ static int print_vlan(const struct sockaddr_nl
> *who,
>  
>  static void print_vlan_flags(__u16 flags)
>  {
> +	if (flags == 0)
> +		return;
> +
> +	open_json_array(PRINT_JSON, "flags");
>  	if (flags & BRIDGE_VLAN_INFO_PVID)
> -		print_null(PRINT_ANY, "pvid", " %s", "PVID");
> +		print_string(PRINT_ANY, NULL, " %s", "PVID");
>  
>  	if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
> -		print_null(PRINT_ANY, "untagged", " %s", "untagged");
> +		print_string(PRINT_ANY, NULL, " %s", "Egress
> Untagged");
> +	close_json_array(PRINT_JSON, NULL);
>  }
>  
>  static void print_one_vlan_stats(const struct bridge_vlan_xstats
> *vstats)
> @@ -461,6 +476,7 @@ static void print_vlan_stats_attr(struct rtattr
> *attr, int ifindex)
>  {
>  	struct rtattr *brtb[LINK_XSTATS_TYPE_MAX+1];
>  	struct rtattr *i, *list;
> +	const char *ifname;
>  	int rem;
>  
>  	parse_rtattr(brtb, LINK_XSTATS_TYPE_MAX, RTA_DATA(attr),
> @@ -471,13 +487,12 @@ static void print_vlan_stats_attr(struct rtattr
> *attr, int ifindex)
>  	list = brtb[LINK_XSTATS_TYPE_BRIDGE];
>  	rem = RTA_PAYLOAD(list);
>  
> -	open_json_object(NULL);
> +	ifname = ll_index_to_name(ifindex);
> +	open_json_object(ifname);
>  
> -	print_color_string(PRINT_ANY, COLOR_IFNAME,
> -			   "dev", "%-16s",
> -			   ll_index_to_name(ifindex));
> +	print_color_string(PRINT_FP, COLOR_IFNAME,
> +			   NULL, "%-16s", ifname);
>  
> -	open_json_array(PRINT_JSON, "xstats");
>  	for (i = RTA_DATA(list); RTA_OK(i, rem); i = RTA_NEXT(i, rem))
> {
>  		const struct bridge_vlan_xstats *vstats = RTA_DATA(i);
>  
> @@ -494,7 +509,6 @@ static void print_vlan_stats_attr(struct rtattr
> *attr, int ifindex)
>  
>  		print_one_vlan_stats(vstats);
>  	}
> -	close_json_array(PRINT_ANY, "\n");
>  	close_json_object();
>  
>  }
> @@ -623,16 +637,13 @@ static int vlan_show(int argc, char **argv)
>  	return 0;
>  }
>  
> -void print_vlan_info(FILE *fp, struct rtattr *tb)
> +void print_vlan_info(struct rtattr *tb, int ifindex)
>  {
>  	struct rtattr *i, *list = tb;
>  	int rem = RTA_PAYLOAD(list);
>  	__u16 last_vid_start = 0;
>  
> -	if (!is_json_context())
> -		fprintf(fp, "%s", _SL_);
> -
> -	open_json_array(PRINT_JSON, "vlan");
> +	open_vlan_port(ifindex);
>  
>  	for (i = RTA_DATA(list); RTA_OK(i, rem); i = RTA_NEXT(i, rem))
> {
>  		struct bridge_vlan_info *vinfo;
> @@ -656,9 +667,9 @@ void print_vlan_info(FILE *fp, struct rtattr *tb)
>  
>  		print_vlan_flags(vinfo->flags);
>  		close_json_object();
> +		print_string(PRINT_FP, NULL, "%s", _SL_);
>  	}
> -
> -	close_json_array(PRINT_ANY, "\n");
> +	close_vlan_port();
>  }
>  
>  int do_vlan(int argc, char **argv)

^ permalink raw reply

* Re: [PATCH][next] cxgb4: remove redundant assignment to vlan_cmd.dropnovlan_fm
From: David Miller @ 2018-09-10 17:01 UTC (permalink / raw)
  To: colin.king; +Cc: ganeshgr, netdev, kernel-janitors, linux-kernel
In-Reply-To: <20180910121722.15524-1-colin.king@canonical.com>

From: Colin King <colin.king@canonical.com>
Date: Mon, 10 Sep 2018 13:17:22 +0100

> From: Colin Ian King <colin.king@canonical.com>
> 
> A recent commit updated vlan_cmd.dropnovlan_fm but failed to remove
> the older assignment.  Fix this by removing the former redundant
> assignment.
> 
> Detected by CoverityScan, CID#1473290 ("Unused value")
> 
> Fixes: a89cdd8e7c74 ("cxgb4: impose mandatory VLAN usage when non-zero TAG ID")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Applied, thanks.

^ permalink raw reply

* [PATCH][next] cxgb4: remove redundant assignment to vlan_cmd.dropnovlan_fm
From: Colin King @ 2018-09-10 12:17 UTC (permalink / raw)
  To: Ganesh Goudar, David S . Miller, netdev; +Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

A recent commit updated vlan_cmd.dropnovlan_fm but failed to remove
the older assignment.  Fix this by removing the former redundant
assignment.

Detected by CoverityScan, CID#1473290 ("Unused value")

Fixes: a89cdd8e7c74 ("cxgb4: impose mandatory VLAN usage when non-zero TAG ID")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/net/ethernet/chelsio/cxgb4/t4_hw.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
index 6f1bd7bf1a11..c28a1d8b7f33 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
@@ -10209,7 +10209,6 @@ int t4_set_vlan_acl(struct adapter *adap, unsigned int mbox, unsigned int vf,
 					 FW_ACL_VLAN_CMD_VFN_V(vf));
 	vlan_cmd.en_to_len16 = cpu_to_be32(enable | FW_LEN16(vlan_cmd));
 	/* Drop all packets that donot match vlan id */
-	vlan_cmd.dropnovlan_fm = FW_ACL_VLAN_CMD_FM_F;
 	vlan_cmd.dropnovlan_fm = (enable
 				  ? (FW_ACL_VLAN_CMD_DROPNOVLAN_F |
 				     FW_ACL_VLAN_CMD_FM_F) : 0);
-- 
2.17.1

^ permalink raw reply related

* Offloaded u32 classifier tables WAS (Re: [PATCH net 00/13] cls_u32 cleanups and fixes.
From: Jamal Hadi Salim @ 2018-09-10 12:25 UTC (permalink / raw)
  To: Al Viro
  Cc: netdev, Cong Wang, Jiri Pirko, Rahul Lakkireddy, Kumar A S,
	Sridhar Samudrala, Alexander Duyck, Amritha Nambiar, Jose.Abreu,
	peppe.cavallaro, alexandre.torgue, Jeff Kirsher,
	Anjali Singhai Jain
In-Reply-To: <20180909154827.GH19965@ZenIV.linux.org.uk>

On 2018-09-09 11:48 a.m., Al Viro wrote:

>
> BTW, shouldn't we issue u32_clear_hw_hnode() every time
> we destroy an hnode?  It's done on u32_delete(), it's
> done (for root ht) on u32_destroy(), but it's not done
> for any other hnodes when you remove the entire (not shared)
> filter.  Looks fishy...
> 

What you are saying makes sense, but that doesnt seem
like a new thing.
All hardware offload examples I have seen use root tables
only[1]. So I am not sure if they use any other tables
although the intel hardware at least seems very capable.
Look at ixgbe_main.c for example. Theres an explicit assumption
that root is always 0x800 but oresence of

+Cc some of the NIC vendor folks..



cheers,
jamal

[1]
Here's a script posted by someone at Intel(Sridhar?) a while back
that adds 2 filters, one with skip-sw and the other with skip-hw
flag.

---
    # add ingress qdisc
    tc qdisc add dev p4p1 ingress

    # enable hw tc offload.
    ethtool -K p4p1 hw-tc-offload on

    # add u32 filter with skip-sw flag.
    tc filter add dev p4p1 parent ffff: protocol ip prio 99 \
       handle 800:0:1 u32 ht 800: flowid 800:1 \
       skip-sw \
       match ip src 192.168.1.0/24 \
       action drop

    # add u32 filter with skip-hw flag.
    tc filter add dev p4p1 parent ffff: protocol ip prio 99 \
       handle 800:0:2 u32 ht 800: flowid 800:2 \
       skip-hw \
       match ip src 192.168.2.0/24 \
       action drop
----

^ permalink raw reply

* Re: Offloaded u32 classifier tables WAS (Re: [PATCH net 00/13] cls_u32 cleanups and fixes.
From: Jamal Hadi Salim @ 2018-09-10 12:31 UTC (permalink / raw)
  To: Al Viro
  Cc: netdev, Cong Wang, Jiri Pirko, Rahul Lakkireddy, Kumar A S,
	Sridhar Samudrala, Alexander Duyck, Amritha Nambiar, Jose.Abreu,
	peppe.cavallaro, alexandre.torgue, Jeff Kirsher,
	Anjali Singhai Jain
In-Reply-To: <3c72ac9e-12d6-0244-d380-5803607c1dae@mojatatu.com>

On 2018-09-10 8:25 a.m., Jamal Hadi Salim wrote:
> On 2018-09-09 11:48 a.m., Al Viro wrote:
> 
>>
>> BTW, shouldn't we issue u32_clear_hw_hnode() every time
>> we destroy an hnode?  It's done on u32_delete(), it's
>> done (for root ht) on u32_destroy(), but it's not done
>> for any other hnodes when you remove the entire (not shared)
>> filter.  Looks fishy...
>>
> 
> What you are saying makes sense, but that doesnt seem
> like a new thing.
> All hardware offload examples I have seen use root tables
> only[1]. So I am not sure if they use any other tables
> although the intel hardware at least seems very capable.
> Look at ixgbe_main.c for example. Theres an explicit assumption
> that root is always 0x800 but oresence of
>

"..but presence of other tables can be handled"

cheers,
jamal

^ permalink raw reply

* libbpf build broken on musl libc (Alpine Linux)
From: Arnaldo Carvalho de Melo @ 2018-09-10 17:29 UTC (permalink / raw)
  To: Daniel Borkmann, Thomas Richter
  Cc: Jakub Kicinski, Hendrik Brueckner, Linux Kernel Mailing List,
	Linux Networking Development Mailing List

Hi guys,

	Ingo updated tip/perf/core and this made me notice that perf is
not building on systems using !glibc, like Alpine Linux, that uses musl
libc. This ends up as:

  # dm
   1 alpine:3.4                    : FAIL gcc (Alpine 5.3.0) 5.3.0
   2 alpine:3.5                    : FAIL gcc (Alpine 6.2.1) 6.2.1 20160822
   3 alpine:3.6                    : FAIL gcc (Alpine 6.3.0) 6.3.0
   4 alpine:3.7                    : FAIL gcc (Alpine 6.4.0) 6.4.0
   5 alpine:3.8                    : FAIL gcc (Alpine 6.4.0) 6.4.0
   6 alpine:edge                   : FAIL gcc (Alpine 6.4.0) 6.4.0
   7 amazonlinux:1                 : Ok   gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-28)
   8 amazonlinux:2                 : Ok   gcc (GCC) 7.3.1 20180303 (Red Hat 7.3.1-5)
   9 android-ndk:r12b-arm          : Ok   arm-linux-androideabi-gcc (GCC) 4.9.x 20150123 (prerelease)
  10 android-ndk:r15c-arm          : Ok   arm-linux-androideabi-gcc (GCC) 4.9.x 20150123 (prerelease)
  11 centos:5                      : Ok   gcc (GCC) 4.1.2 20080704 (Red Hat 4.1.2-55)
  12 centos:6                      : Ok   gcc (GCC) 4.4.7 20120313 (Red Hat 4.4.7-23)
  13 centos:7                      : Ok   gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-28)
  14 debian:7                      : Ok   gcc (Debian 4.7.2-5) 4.7.2
  15 debian:8                      : Ok   gcc (Debian 4.9.2-10+deb8u1) 4.9.2
  17 debian:9                      : Ok   gcc (Debian 6.3.0-18+deb9u1) 6.3.0 20170516
  18 debian:experimental           : Ok   gcc (Debian 8.2.0-4) 8.2.0
<still building on the other containers, 65 in total>

  CC       /tmp/build/perf/libbpf.o
  CC       /tmp/build/perf/event-plugin.o
  CC       /tmp/build/perf/parse-options.o
libbpf.c: In function 'bpf_object__elf_init':
libbpf.c:472:15: error: initialization makes pointer from integer without a cast [-Werror=int-conversion]
    char *cp = strerror_r(errno, errmsg, sizeof(errmsg));
               ^
libbpf.c: In function 'bpf_object__elf_collect':
libbpf.c:813:16: error: initialization makes pointer from integer without a cast [-Werror=int-conversion]
     char *cp = strerror_r(-err, errmsg,
                ^
libbpf.c: In function 'bpf_object__create_maps':
libbpf.c:1143:7: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]
    cp = strerror_r(errno, errmsg, sizeof(errmsg));
       ^
libbpf.c:1158:7: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]
    cp = strerror_r(errno, errmsg, sizeof(errmsg));
       ^
libbpf.c: In function 'load_program':
libbpf.c:1342:5: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]
  cp = strerror_r(errno, errmsg, sizeof(errmsg));
     ^
libbpf.c: In function 'check_path':
libbpf.c:1657:6: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]
   cp = strerror_r(errno, errmsg, sizeof(errmsg));
      ^
libbpf.c: In function 'bpf_program__pin_instance':
libbpf.c:1693:6: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]
   cp = strerror_r(errno, errmsg, sizeof(errmsg));
      ^
libbpf.c: In function 'make_dir':
libbpf.c:1711:6: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]
   cp = strerror_r(-err, errmsg, sizeof(errmsg));
      ^
libbpf.c: In function 'bpf_map__pin':
libbpf.c:1773:6: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]
   cp = strerror_r(errno, errmsg, sizeof(errmsg));
      ^
  CC       /tmp/build/perf/trace-seq.o
  CC       /tmp/build/perf/parse-filter.o
  CC       /tmp/build/perf/parse-utils.o
cc1: all warnings being treated as errors
mv: can't rename '/tmp/build/perf/.libbpf.o.tmp': No such file or directory

This is handled in tools/perf/ by using tools/lib/str_error_r.c, that
was introduced with the cset at the end of this message.

After lunch I'll work on a patch to fix this, 

Thanks,

- Arnaldo

commit c8b5f2c96d1bf6cefcbe12f67dce0b892fe20512
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
Date:   Wed Jul 6 11:56:20 2016 -0300

    tools: Introduce str_error_r()
    
    The tools so far have been using the strerror_r() GNU variant, that
    returns a string, be it the buffer passed or something else.
    
    But that, besides being tricky in cases where we expect that the
    function using strerror_r() returns the error formatted in a provided
    buffer (we have to check if it returned something else and copy that
    instead), breaks the build on systems not using glibc, like Alpine
    Linux, where musl libc is used.
    
    So, introduce yet another wrapper, str_error_r(), that has the GNU
    interface, but uses the portable XSI variant of strerror_r(), so that
    users rest asured that the provided buffer is used and it is what is
    returned.
    
    Cc: Adrian Hunter <adrian.hunter@intel.com>
    Cc: David Ahern <dsahern@gmail.com>
    Cc: Jiri Olsa <jolsa@kernel.org>
    Cc: Namhyung Kim <namhyung@kernel.org>
    Cc: Wang Nan <wangnan0@huawei.com>
    Link: http://lkml.kernel.org/n/tip-d4t42fnf48ytlk8rjxs822tf@git.kernel.org
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/include/linux/string.h b/tools/include/linux/string.h
index e26223f1f287..b466d0228b57 100644
--- a/tools/include/linux/string.h
+++ b/tools/include/linux/string.h
@@ -12,4 +12,6 @@ int strtobool(const char *s, bool *res);
 extern size_t strlcpy(char *dest, const char *src, size_t size);
 #endif
 
+char *str_error_r(int errnum, char *buf, size_t buflen);
<SNIP>

^ permalink raw reply related

* Re: [PATCH v2 net] MIPS: lantiq: dma: add dev pointer
From: Andrew Lunn @ 2018-09-10 12:45 UTC (permalink / raw)
  To: Hauke Mehrtens
  Cc: davem, netdev, f.fainelli, john, linux-mips, hauke.mehrtens,
	paul.burton
In-Reply-To: <20180909192623.14998-1-hauke@hauke-m.de>

On Sun, Sep 09, 2018 at 09:26:23PM +0200, Hauke Mehrtens wrote:
> dma_zalloc_coherent() now crashes if no dev pointer is given.
> Add a dev pointer to the ltq_dma_channel structure and fill it in the
> driver using it.
> 
> This fixes a bug introduced in kernel 4.19.
> 
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> ---
> 
> no changes since v1.
> 
> This should go into kernel 4.19 and I have some other patches adding new 
> features for kernel 4.20 which are depending on this, so I would prefer 
> if this goes through the net tree. 

Hi Hauke

Is this a build time dependency, or a runtime dependency?

What we don't want to do is add the switch driver to net-next and find
it does not compile because this change is not in net-next yet.

   Andrew

^ permalink raw reply

* Re: [net-next, v2, 1/2] net: stmmac: Rework coalesce timer and fix multi-queue races
From: Jose Abreu @ 2018-09-10 12:52 UTC (permalink / raw)
  To: Neil Armstrong, Jose Abreu, netdev
  Cc: Jerome Brunet, Martin Blumenstingl, David S. Miller, Joao Pinto,
	Giuseppe Cavallaro, Alexandre Torgue
In-Reply-To: <ff2f8f89-134e-b7f8-1a40-d87f371be303@baylibre.com>

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

Hi Neil,

On 10-09-2018 12:43, Neil Armstrong wrote:
> Hi Jose,
>
> On 10/09/2018 11:14, Jose Abreu wrote:
>> This follows David Miller advice and tries to fix coalesce timer in
>> multi-queue scenarios.
>>
>> We are now using per-queue coalesce values and per-queue TX timer.
>>
>> Coalesce timer default values was changed to 1ms and the coalesce frames
>> to 25.
>>
>> Tested in B2B setup between XGMAC2 and GMAC5.
>>
>> Signed-off-by: Jose Abreu <joabreu@synopsys.com>
>> Cc: Jerome Brunet <jbrunet@baylibre.com>
>> Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>> Cc: David S. Miller <davem@davemloft.net>
>> Cc: Joao Pinto <jpinto@synopsys.com>
>> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
>> Cc: Alexandre Torgue <alexandre.torgue@st.com>
>> ---
>> Jerome,
> Jerome is in holidays...
>
>> Can you please test if this one is okay ?
>
> I tested this patch on top of 4.18.7 with the previous patch (4ae0169fd1b3) reverted.
>
> The TX or RX stopped a few seconds after iperf3 started :
> (iperf3 is running in server mode on the Amlogic A113D board)
>
> $ iperf3 -c 10.1.4.95
> Connecting to host 10.1.4.95, port 5201
> [  4] local 10.1.2.12 port 39906 connected to 10.1.4.95 port 5201
> [ ID] Interval           Transfer     Bandwidth       Retr  Cwnd
> [  4]   0.00-1.00   sec  56.2 MBytes   472 Mbits/sec    0    660 KBytes
> [  4]   1.00-2.00   sec  56.2 MBytes   472 Mbits/sec    0    660 KBytes
> [  4]   2.00-3.00   sec  38.8 MBytes   325 Mbits/sec    1   1.41 KBytes
> [  4]   3.00-4.00   sec  0.00 Bytes  0.00 bits/sec    1   1.41 KBytes
> [  4]   4.00-5.00   sec  0.00 Bytes  0.00 bits/sec    1   1.41 KBytes
> ^C[  4]   5.00-5.61   sec  0.00 Bytes  0.00 bits/sec    0   1.41 KBytes
> - - - - - - - - - - - - - - - - - - - - - - - - -
> [ ID] Interval           Transfer     Bandwidth       Retr
> [  4]   0.00-5.61   sec   151 MBytes   226 Mbits/sec    3             sender
> [  4]   0.00-5.61   sec  0.00 Bytes  0.00 bits/sec                  receiver
> iperf3: interrupt - the client has terminated
>
> $ iperf3 -c 10.1.4.95 -R
> Connecting to host 10.1.4.95, port 5201
> Reverse mode, remote host 10.1.4.95 is sending
> [  4] local 10.1.2.12 port 39894 connected to 10.1.4.95 port 5201
> [ ID] Interval           Transfer     Bandwidth
> [  4]   0.00-1.00   sec  82.2 MBytes   690 Mbits/sec
> [  4]   1.00-2.00   sec  82.6 MBytes   693 Mbits/sec
> [  4]   2.00-3.00   sec  24.2 MBytes   203 Mbits/sec
> [  4]   3.00-4.00   sec  0.00 Bytes  0.00 bits/sec
> [  4]   4.00-5.00   sec  0.00 Bytes  0.00 bits/sec
> ^C[  4]   5.00-5.53   sec  0.00 Bytes  0.00 bits/sec
> - - - - - - - - - - - - - - - - - - - - - - - - -
> [ ID] Interval           Transfer     Bandwidth
> [  4]   0.00-5.53   sec  0.00 Bytes  0.00 bits/sec                  sender
> [  4]   0.00-5.53   sec   189 MBytes   287 Mbits/sec                  receiver
> iperf3: interrupt - the client has terminated
>
> These works for hours without this patch applied.

Damn... I'm able to replicate the issue if I turn SMP off but
it's not consistent ...

Can you please try attached follow-up patch ? It's been running
for 10min now and I'm getting ~700Mbps on the same GMAC as you
have (3.71).

Thanks and Best Regards,
Jose Miguel Abreu

>
> Neil
>
>> Thanks and Best Regards,
>> Jose Miguel Abreu
>> ---
>>  drivers/net/ethernet/stmicro/stmmac/common.h      |   4 +-
>>  drivers/net/ethernet/stmicro/stmmac/stmmac.h      |   6 +-
>>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 207 ++++++++++++++--------
>>  3 files changed, 135 insertions(+), 82 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
>> index 1854f270ad66..b1b305f8f414 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
>> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
>> @@ -258,10 +258,10 @@ struct stmmac_safety_stats {
>>  #define MAX_DMA_RIWT		0xff
>>  #define MIN_DMA_RIWT		0x20
>>  /* Tx coalesce parameters */
>> -#define STMMAC_COAL_TX_TIMER	40000
>> +#define STMMAC_COAL_TX_TIMER	1000
>>  #define STMMAC_MAX_COAL_TX_TICK	100000
>>  #define STMMAC_TX_MAX_FRAMES	256
>> -#define STMMAC_TX_FRAMES	64
>> +#define STMMAC_TX_FRAMES	25
>>  
>>  /* Packets types */
>>  enum packets_types {
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
>> index c0a855b7ab3b..957030cfb833 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
>> @@ -48,6 +48,9 @@ struct stmmac_tx_info {
>>  
>>  /* Frequently used values are kept adjacent for cache effect */
>>  struct stmmac_tx_queue {
>> +	u32 tx_count_frames;
>> +	int tx_timer_active;
>> +	struct timer_list txtimer;
>>  	u32 queue_index;
>>  	struct stmmac_priv *priv_data;
>>  	struct dma_extended_desc *dma_etx ____cacheline_aligned_in_smp;
>> @@ -59,6 +62,7 @@ struct stmmac_tx_queue {
>>  	dma_addr_t dma_tx_phy;
>>  	u32 tx_tail_addr;
>>  	u32 mss;
>> +	struct napi_struct napi ____cacheline_aligned_in_smp;
>>  };
>>  
>>  struct stmmac_rx_queue {
>> @@ -109,14 +113,12 @@ struct stmmac_pps_cfg {
>>  
>>  struct stmmac_priv {
>>  	/* Frequently used values are kept adjacent for cache effect */
>> -	u32 tx_count_frames;
>>  	u32 tx_coal_frames;
>>  	u32 tx_coal_timer;
>>  
>>  	int tx_coalesce;
>>  	int hwts_tx_en;
>>  	bool tx_path_in_lpi_mode;
>> -	struct timer_list txtimer;
>>  	bool tso;
>>  
>>  	unsigned int dma_buf_sz;
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> index 9f458bb16f2a..9809c2b319fe 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> @@ -148,6 +148,7 @@ static void stmmac_verify_args(void)
>>  static void stmmac_disable_all_queues(struct stmmac_priv *priv)
>>  {
>>  	u32 rx_queues_cnt = priv->plat->rx_queues_to_use;
>> +	u32 tx_queues_cnt = priv->plat->tx_queues_to_use;
>>  	u32 queue;
>>  
>>  	for (queue = 0; queue < rx_queues_cnt; queue++) {
>> @@ -155,6 +156,12 @@ static void stmmac_disable_all_queues(struct stmmac_priv *priv)
>>  
>>  		napi_disable(&rx_q->napi);
>>  	}
>> +
>> +	for (queue = 0; queue < tx_queues_cnt; queue++) {
>> +		struct stmmac_tx_queue *tx_q = &priv->tx_queue[queue];
>> +
>> +		napi_disable(&tx_q->napi);
>> +	}
>>  }
>>  
>>  /**
>> @@ -164,6 +171,7 @@ static void stmmac_disable_all_queues(struct stmmac_priv *priv)
>>  static void stmmac_enable_all_queues(struct stmmac_priv *priv)
>>  {
>>  	u32 rx_queues_cnt = priv->plat->rx_queues_to_use;
>> +	u32 tx_queues_cnt = priv->plat->tx_queues_to_use;
>>  	u32 queue;
>>  
>>  	for (queue = 0; queue < rx_queues_cnt; queue++) {
>> @@ -171,6 +179,12 @@ static void stmmac_enable_all_queues(struct stmmac_priv *priv)
>>  
>>  		napi_enable(&rx_q->napi);
>>  	}
>> +
>> +	for (queue = 0; queue < tx_queues_cnt; queue++) {
>> +		struct stmmac_tx_queue *tx_q = &priv->tx_queue[queue];
>> +
>> +		napi_enable(&tx_q->napi);
>> +	}
>>  }
>>  
>>  /**
>> @@ -1843,7 +1857,8 @@ static void stmmac_dma_operation_mode(struct stmmac_priv *priv)
>>   * @queue: TX queue index
>>   * Description: it reclaims the transmit resources after transmission completes.
>>   */
>> -static void stmmac_tx_clean(struct stmmac_priv *priv, u32 queue)
>> +static int stmmac_tx_clean(struct stmmac_priv *priv, int limit, u32 queue,
>> +			   bool *more)
>>  {
>>  	struct stmmac_tx_queue *tx_q = &priv->tx_queue[queue];
>>  	unsigned int bytes_compl = 0, pkts_compl = 0;
>> @@ -1851,10 +1866,13 @@ static void stmmac_tx_clean(struct stmmac_priv *priv, u32 queue)
>>  
>>  	netif_tx_lock(priv->dev);
>>  
>> +	if (more)
>> +		*more = false;
>> +
>>  	priv->xstats.tx_clean++;
>>  
>>  	entry = tx_q->dirty_tx;
>> -	while (entry != tx_q->cur_tx) {
>> +	while ((entry != tx_q->cur_tx) && (pkts_compl < limit)) {
>>  		struct sk_buff *skb = tx_q->tx_skbuff[entry];
>>  		struct dma_desc *p;
>>  		int status;
>> @@ -1937,7 +1955,13 @@ static void stmmac_tx_clean(struct stmmac_priv *priv, u32 queue)
>>  		stmmac_enable_eee_mode(priv);
>>  		mod_timer(&priv->eee_ctrl_timer, STMMAC_LPI_T(eee_timer));
>>  	}
>> +
>> +	if (more && (tx_q->dirty_tx != tx_q->cur_tx))
>> +		*more = true;
>> +
>>  	netif_tx_unlock(priv->dev);
>> +
>> +	return pkts_compl;
>>  }
>>  
>>  /**
>> @@ -2020,6 +2044,34 @@ static bool stmmac_safety_feat_interrupt(struct stmmac_priv *priv)
>>  	return false;
>>  }
>>  
>> +static int stmmac_napi_check(struct stmmac_priv *priv, u32 chan)
>> +{
>> +	int status = stmmac_dma_interrupt_status(priv, priv->ioaddr,
>> +						 &priv->xstats, chan);
>> +
>> +	if ((status & handle_rx) && (chan < priv->plat->rx_queues_to_use)) {
>> +		struct stmmac_rx_queue *rx_q = &priv->rx_queue[chan];
>> +
>> +		if (likely(napi_schedule_prep(&rx_q->napi))) {
>> +			stmmac_disable_dma_irq(priv, priv->ioaddr, chan);
>> +			__napi_schedule(&rx_q->napi);
>> +		}
>> +	} else {
>> +		status &= ~handle_rx;
>> +	}
>> +
>> +	if ((status & handle_tx) && (chan < priv->plat->tx_queues_to_use)) {
>> +		struct stmmac_tx_queue *tx_q = &priv->tx_queue[chan];
>> +
>> +		if (likely(napi_schedule_prep(&tx_q->napi)))
>> +			__napi_schedule(&tx_q->napi);
>> +	} else {
>> +		status &= ~handle_tx;
>> +	}
>> +
>> +	return status;
>> +}
>> +
>>  /**
>>   * stmmac_dma_interrupt - DMA ISR
>>   * @priv: driver private structure
>> @@ -2034,57 +2086,14 @@ static void stmmac_dma_interrupt(struct stmmac_priv *priv)
>>  	u32 channels_to_check = tx_channel_count > rx_channel_count ?
>>  				tx_channel_count : rx_channel_count;
>>  	u32 chan;
>> -	bool poll_scheduled = false;
>>  	int status[max_t(u32, MTL_MAX_TX_QUEUES, MTL_MAX_RX_QUEUES)];
>>  
>>  	/* Make sure we never check beyond our status buffer. */
>>  	if (WARN_ON_ONCE(channels_to_check > ARRAY_SIZE(status)))
>>  		channels_to_check = ARRAY_SIZE(status);
>>  
>> -	/* Each DMA channel can be used for rx and tx simultaneously, yet
>> -	 * napi_struct is embedded in struct stmmac_rx_queue rather than in a
>> -	 * stmmac_channel struct.
>> -	 * Because of this, stmmac_poll currently checks (and possibly wakes)
>> -	 * all tx queues rather than just a single tx queue.
>> -	 */
>>  	for (chan = 0; chan < channels_to_check; chan++)
>> -		status[chan] = stmmac_dma_interrupt_status(priv, priv->ioaddr,
>> -				&priv->xstats, chan);
>> -
>> -	for (chan = 0; chan < rx_channel_count; chan++) {
>> -		if (likely(status[chan] & handle_rx)) {
>> -			struct stmmac_rx_queue *rx_q = &priv->rx_queue[chan];
>> -
>> -			if (likely(napi_schedule_prep(&rx_q->napi))) {
>> -				stmmac_disable_dma_irq(priv, priv->ioaddr, chan);
>> -				__napi_schedule(&rx_q->napi);
>> -				poll_scheduled = true;
>> -			}
>> -		}
>> -	}
>> -
>> -	/* If we scheduled poll, we already know that tx queues will be checked.
>> -	 * If we didn't schedule poll, see if any DMA channel (used by tx) has a
>> -	 * completed transmission, if so, call stmmac_poll (once).
>> -	 */
>> -	if (!poll_scheduled) {
>> -		for (chan = 0; chan < tx_channel_count; chan++) {
>> -			if (status[chan] & handle_tx) {
>> -				/* It doesn't matter what rx queue we choose
>> -				 * here. We use 0 since it always exists.
>> -				 */
>> -				struct stmmac_rx_queue *rx_q =
>> -					&priv->rx_queue[0];
>> -
>> -				if (likely(napi_schedule_prep(&rx_q->napi))) {
>> -					stmmac_disable_dma_irq(priv,
>> -							priv->ioaddr, chan);
>> -					__napi_schedule(&rx_q->napi);
>> -				}
>> -				break;
>> -			}
>> -		}
>> -	}
>> +		status[chan] = stmmac_napi_check(priv, chan);
>>  
>>  	for (chan = 0; chan < tx_channel_count; chan++) {
>>  		if (unlikely(status[chan] & tx_hard_error_bump_tc)) {
>> @@ -2241,13 +2250,11 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
>>   */
>>  static void stmmac_tx_timer(struct timer_list *t)
>>  {
>> -	struct stmmac_priv *priv = from_timer(priv, t, txtimer);
>> -	u32 tx_queues_count = priv->plat->tx_queues_to_use;
>> -	u32 queue;
>> +	struct stmmac_tx_queue *tx_q = from_timer(tx_q, t, txtimer);
>>  
>> -	/* let's scan all the tx queues */
>> -	for (queue = 0; queue < tx_queues_count; queue++)
>> -		stmmac_tx_clean(priv, queue);
>> +	if (likely(napi_schedule_prep(&tx_q->napi)))
>> +		__napi_schedule(&tx_q->napi);
>> +	tx_q->tx_timer_active = 0;
>>  }
>>  
>>  /**
>> @@ -2260,11 +2267,17 @@ static void stmmac_tx_timer(struct timer_list *t)
>>   */
>>  static void stmmac_init_tx_coalesce(struct stmmac_priv *priv)
>>  {
>> +	u32 tx_channel_count = priv->plat->tx_queues_to_use;
>> +	u32 chan;
>> +
>>  	priv->tx_coal_frames = STMMAC_TX_FRAMES;
>>  	priv->tx_coal_timer = STMMAC_COAL_TX_TIMER;
>> -	timer_setup(&priv->txtimer, stmmac_tx_timer, 0);
>> -	priv->txtimer.expires = STMMAC_COAL_TIMER(priv->tx_coal_timer);
>> -	add_timer(&priv->txtimer);
>> +
>> +	for (chan = 0; chan < tx_channel_count; chan++) {
>> +		struct stmmac_tx_queue *tx_q = &priv->tx_queue[chan];
>> +
>> +		timer_setup(&tx_q->txtimer, stmmac_tx_timer, 0);
>> +	}
>>  }
>>  
>>  static void stmmac_set_rings_length(struct stmmac_priv *priv)
>> @@ -2592,6 +2605,7 @@ static void stmmac_hw_teardown(struct net_device *dev)
>>  static int stmmac_open(struct net_device *dev)
>>  {
>>  	struct stmmac_priv *priv = netdev_priv(dev);
>> +	u32 chan;
>>  	int ret;
>>  
>>  	stmmac_check_ether_addr(priv);
>> @@ -2688,7 +2702,9 @@ static int stmmac_open(struct net_device *dev)
>>  	if (dev->phydev)
>>  		phy_stop(dev->phydev);
>>  
>> -	del_timer_sync(&priv->txtimer);
>> +	for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++)
>> +		del_timer_sync(&priv->tx_queue[chan].txtimer);
>> +
>>  	stmmac_hw_teardown(dev);
>>  init_error:
>>  	free_dma_desc_resources(priv);
>> @@ -2708,6 +2724,7 @@ static int stmmac_open(struct net_device *dev)
>>  static int stmmac_release(struct net_device *dev)
>>  {
>>  	struct stmmac_priv *priv = netdev_priv(dev);
>> +	u32 chan;
>>  
>>  	if (priv->eee_enabled)
>>  		del_timer_sync(&priv->eee_ctrl_timer);
>> @@ -2722,7 +2739,8 @@ static int stmmac_release(struct net_device *dev)
>>  
>>  	stmmac_disable_all_queues(priv);
>>  
>> -	del_timer_sync(&priv->txtimer);
>> +	for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++)
>> +		del_timer_sync(&priv->tx_queue[chan].txtimer);
>>  
>>  	/* Free the IRQ lines */
>>  	free_irq(dev->irq, dev);
>> @@ -2936,14 +2954,11 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)
>>  	priv->xstats.tx_tso_nfrags += nfrags;
>>  
>>  	/* Manage tx mitigation */
>> -	priv->tx_count_frames += nfrags + 1;
>> -	if (likely(priv->tx_coal_frames > priv->tx_count_frames)) {
>> -		mod_timer(&priv->txtimer,
>> -			  STMMAC_COAL_TIMER(priv->tx_coal_timer));
>> -	} else {
>> -		priv->tx_count_frames = 0;
>> +	tx_q->tx_count_frames += nfrags + 1;
>> +	if (priv->tx_coal_frames <= tx_q->tx_count_frames) {
>>  		stmmac_set_tx_ic(priv, desc);
>>  		priv->xstats.tx_set_ic_bit++;
>> +		tx_q->tx_count_frames = 0;
>>  	}
>>  
>>  	skb_tx_timestamp(skb);
>> @@ -2994,6 +3009,12 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)
>>  
>>  	stmmac_set_tx_tail_ptr(priv, priv->ioaddr, tx_q->tx_tail_addr, queue);
>>  
>> +	if (priv->tx_coal_timer && !tx_q->tx_timer_active) {
>> +		tx_q->tx_timer_active = 1;
>> +		mod_timer(&tx_q->txtimer,
>> +				STMMAC_COAL_TIMER(priv->tx_coal_timer));
>> +	}
>> +
>>  	return NETDEV_TX_OK;
>>  
>>  dma_map_err:
>> @@ -3146,14 +3167,11 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
>>  	 * This approach takes care about the fragments: desc is the first
>>  	 * element in case of no SG.
>>  	 */
>> -	priv->tx_count_frames += nfrags + 1;
>> -	if (likely(priv->tx_coal_frames > priv->tx_count_frames)) {
>> -		mod_timer(&priv->txtimer,
>> -			  STMMAC_COAL_TIMER(priv->tx_coal_timer));
>> -	} else {
>> -		priv->tx_count_frames = 0;
>> +	tx_q->tx_count_frames += nfrags + 1;
>> +	if (priv->tx_coal_frames <= tx_q->tx_count_frames) {
>>  		stmmac_set_tx_ic(priv, desc);
>>  		priv->xstats.tx_set_ic_bit++;
>> +		tx_q->tx_count_frames = 0;
>>  	}
>>  
>>  	skb_tx_timestamp(skb);
>> @@ -3199,8 +3217,15 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
>>  	netdev_tx_sent_queue(netdev_get_tx_queue(dev, queue), skb->len);
>>  
>>  	stmmac_enable_dma_transmission(priv, priv->ioaddr);
>> +
>>  	stmmac_set_tx_tail_ptr(priv, priv->ioaddr, tx_q->tx_tail_addr, queue);
>>  
>> +	if (priv->tx_coal_timer && !tx_q->tx_timer_active) {
>> +		tx_q->tx_timer_active = 1;
>> +		mod_timer(&tx_q->txtimer,
>> +				STMMAC_COAL_TIMER(priv->tx_coal_timer));
>> +	}
>> +
>>  	return NETDEV_TX_OK;
>>  
>>  dma_map_err:
>> @@ -3514,27 +3539,41 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
>>   *  Description :
>>   *  To look at the incoming frames and clear the tx resources.
>>   */
>> -static int stmmac_poll(struct napi_struct *napi, int budget)
>> +static int stmmac_rx_poll(struct napi_struct *napi, int budget)
>>  {
>>  	struct stmmac_rx_queue *rx_q =
>>  		container_of(napi, struct stmmac_rx_queue, napi);
>>  	struct stmmac_priv *priv = rx_q->priv_data;
>> -	u32 tx_count = priv->plat->tx_queues_to_use;
>>  	u32 chan = rx_q->queue_index;
>>  	int work_done = 0;
>> -	u32 queue;
>>  
>>  	priv->xstats.napi_poll++;
>>  
>> -	/* check all the queues */
>> -	for (queue = 0; queue < tx_count; queue++)
>> -		stmmac_tx_clean(priv, queue);
>> -
>>  	work_done = stmmac_rx(priv, budget, rx_q->queue_index);
>> +	if (work_done < budget && napi_complete_done(napi, work_done))
>> +		stmmac_enable_dma_irq(priv, priv->ioaddr, chan);
>> +
>> +	return work_done;
>> +}
>> +
>> +static int stmmac_tx_poll(struct napi_struct *napi, int budget)
>> +{
>> +	struct stmmac_tx_queue *tx_q =
>> +		container_of(napi, struct stmmac_tx_queue, napi);
>> +	struct stmmac_priv *priv = tx_q->priv_data;
>> +	u32 chan = tx_q->queue_index;
>> +	int work_done = 0;
>> +	bool more;
>> +
>> +	priv->xstats.napi_poll++;
>> +
>> +	work_done = stmmac_tx_clean(priv, budget, chan, &more);
>>  	if (work_done < budget) {
>>  		napi_complete_done(napi, work_done);
>> -		stmmac_enable_dma_irq(priv, priv->ioaddr, chan);
>> +		if (more)
>> +			napi_reschedule(napi);
>>  	}
>> +
>>  	return work_done;
>>  }
>>  
>> @@ -4325,10 +4364,17 @@ int stmmac_dvr_probe(struct device *device,
>>  	for (queue = 0; queue < priv->plat->rx_queues_to_use; queue++) {
>>  		struct stmmac_rx_queue *rx_q = &priv->rx_queue[queue];
>>  
>> -		netif_napi_add(ndev, &rx_q->napi, stmmac_poll,
>> +		netif_napi_add(ndev, &rx_q->napi, stmmac_rx_poll,
>>  			       (8 * priv->plat->rx_queues_to_use));
>>  	}
>>  
>> +	for (queue = 0; queue < priv->plat->tx_queues_to_use; queue++) {
>> +		struct stmmac_tx_queue *tx_q = &priv->tx_queue[queue];
>> +
>> +		netif_napi_add(ndev, &tx_q->napi, stmmac_tx_poll,
>> +			       (8 * priv->plat->tx_queues_to_use));
>> +	}
>> +
>>  	mutex_init(&priv->lock);
>>  
>>  	/* If a specific clk_csr value is passed from the platform
>> @@ -4377,6 +4423,11 @@ int stmmac_dvr_probe(struct device *device,
>>  
>>  		netif_napi_del(&rx_q->napi);
>>  	}
>> +	for (queue = 0; queue < priv->plat->tx_queues_to_use; queue++) {
>> +		struct stmmac_tx_queue *tx_q = &priv->tx_queue[queue];
>> +
>> +		netif_napi_del(&tx_q->napi);
>> +	}
>>  error_hw_init:
>>  	destroy_workqueue(priv->wq);
>>  error_wq:
>>


[-- Attachment #2: 0001-fixup_coalesce.patch --]
[-- Type: text/x-patch, Size: 3982 bytes --]

>From 35a5e33f4edcc663511d615e61a1ea119dfc77ee Mon Sep 17 00:00:00 2001
Message-Id: <35a5e33f4edcc663511d615e61a1ea119dfc77ee.1536583587.git.joabreu@synopsys.com>
From: Jose Abreu <joabreu@synopsys.com>
Date: Mon, 10 Sep 2018 14:46:09 +0200
Subject: [PATCH] fixup_coalesce

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 39 +++++++++++++----------
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 97268769186e..7875e81966fb 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1872,7 +1872,7 @@ static int stmmac_tx_clean(struct stmmac_priv *priv, int limit, u32 queue,
 	priv->xstats.tx_clean++;
 
 	entry = tx_q->dirty_tx;
-	while ((entry != tx_q->cur_tx) && (pkts_compl < limit)) {
+	while (entry != tx_q->cur_tx) {
 		struct sk_buff *skb = tx_q->tx_skbuff[entry];
 		struct dma_desc *p;
 		int status;
@@ -2241,6 +2241,17 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
 	return ret;
 }
 
+static void stmmac_tx_timer_arm(struct stmmac_priv *priv, u32 queue)
+{
+	struct stmmac_tx_queue *tx_q = &priv->tx_queue[queue];
+
+	if (tx_q->tx_timer_active)
+		return;
+
+	mod_timer(&tx_q->txtimer, STMMAC_COAL_TIMER(priv->tx_coal_timer));
+	tx_q->tx_timer_active = true;
+}
+
 /**
  * stmmac_tx_timer - mitigation sw timer for tx.
  * @data: data pointer
@@ -2250,10 +2261,10 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
 static void stmmac_tx_timer(struct timer_list *t)
 {
 	struct stmmac_tx_queue *tx_q = from_timer(tx_q, t, txtimer);
+	struct stmmac_priv *priv = tx_q->priv_data;
 
-	if (likely(napi_schedule_prep(&tx_q->napi)))
-		__napi_schedule(&tx_q->napi);
-	tx_q->tx_timer_active = 0;
+	stmmac_tx_clean(priv, ~0, tx_q->queue_index, NULL);
+	tx_q->tx_timer_active = false;
 }
 
 /**
@@ -2852,6 +2863,9 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)
 	/* Compute header lengths */
 	proto_hdr_len = skb_transport_offset(skb) + tcp_hdrlen(skb);
 
+	/* Start coalesce timer earlier in case TX Queue is stopped */
+	stmmac_tx_timer_arm(priv, queue);
+
 	/* Desc availability based on threshold should be enough safe */
 	if (unlikely(stmmac_tx_avail(priv, queue) <
 		(((skb->len - proto_hdr_len) / TSO_MAX_BUFF_SIZE + 1)))) {
@@ -3009,12 +3023,6 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)
 	tx_q->tx_tail_addr = tx_q->dma_tx_phy + (tx_q->cur_tx * sizeof(*desc));
 	stmmac_set_tx_tail_ptr(priv, priv->ioaddr, tx_q->tx_tail_addr, queue);
 
-	if (priv->tx_coal_timer && !tx_q->tx_timer_active) {
-		tx_q->tx_timer_active = 1;
-		mod_timer(&tx_q->txtimer,
-				STMMAC_COAL_TIMER(priv->tx_coal_timer));
-	}
-
 	return NETDEV_TX_OK;
 
 dma_map_err:
@@ -3054,6 +3062,9 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
 			return stmmac_tso_xmit(skb, dev);
 	}
 
+	/* Start coalesce timer earlier in case TX Queue is stopped */
+	stmmac_tx_timer_arm(priv, queue);
+
 	if (unlikely(stmmac_tx_avail(priv, queue) < nfrags + 1)) {
 		if (!netif_tx_queue_stopped(netdev_get_tx_queue(dev, queue))) {
 			netif_tx_stop_queue(netdev_get_tx_queue(priv->dev,
@@ -3221,12 +3232,6 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
 	tx_q->tx_tail_addr = tx_q->dma_tx_phy + (tx_q->cur_tx * sizeof(*desc));
 	stmmac_set_tx_tail_ptr(priv, priv->ioaddr, tx_q->tx_tail_addr, queue);
 
-	if (priv->tx_coal_timer && !tx_q->tx_timer_active) {
-		tx_q->tx_timer_active = 1;
-		mod_timer(&tx_q->txtimer,
-				STMMAC_COAL_TIMER(priv->tx_coal_timer));
-	}
-
 	return NETDEV_TX_OK;
 
 dma_map_err:
@@ -3575,7 +3580,7 @@ static int stmmac_tx_poll(struct napi_struct *napi, int budget)
 			napi_reschedule(napi);
 	}
 
-	return work_done;
+	return min(work_done, budget);
 }
 
 /**
-- 
2.7.4


^ permalink raw reply related

* Re: [PATCH v3 net-next 3/6] dt-bindings: net: Add lantiq,xrx200-net DT bindings
From: Andrew Lunn @ 2018-09-10 12:53 UTC (permalink / raw)
  To: Hauke Mehrtens
  Cc: davem, netdev, vivien.didelot, f.fainelli, john, linux-mips, dev,
	hauke.mehrtens, devicetree
In-Reply-To: <20180909201647.32727-4-hauke@hauke-m.de>

On Sun, Sep 09, 2018 at 10:16:44PM +0200, Hauke Mehrtens wrote:
> This adds the binding for the PMAC core between the CPU and the GSWIP
> switch found on the xrx200 / VR9 Lantiq / Intel SoC.
> 
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> Cc: devicetree@vger.kernel.org
> ---
>  .../devicetree/bindings/net/lantiq,xrx200-net.txt   | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/lantiq,xrx200-net.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/lantiq,xrx200-net.txt b/Documentation/devicetree/bindings/net/lantiq,xrx200-net.txt
> new file mode 100644
> index 000000000000..8a2fe5200cdc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/lantiq,xrx200-net.txt
> @@ -0,0 +1,21 @@
> +Lantiq xRX200 GSWIP PMAC Ethernet driver
> +==================================
> +
> +Required properties:
> +
> +- compatible	: "lantiq,xrx200-net" for the PMAC of the embedded
> +		: GSWIP in the xXR200
> +- reg		: memory range of the PMAC core inside of the GSWIP core
> +- interrupts	: TX and RX DMA interrupts. Use interrupt-names "tx" for
> +		: the TX interrupt and "rx" for the RX interrupt.
> +
> +Example:
> +
> +eth0: eth@E10B308 {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	compatible = "lantiq,xrx200-net";
> +	reg = <0xE10B308 0x30>;

Hi Hauke

This binding itself looks fine. I just find this address range a bit
odd. What are 0xe10b300-0xe10b307 used for? Are all 0x30 bytes used in
the range? The address range ending at 0xe10b338 seems a bit
odd. 0xe10b33f would be more typical.

I'm asking because it can be messy when you find out you need to
change the address range, and not break backwards compatibility.

     Andrew

^ permalink raw reply

* Re: [net-next, v2, 1/2] net: stmmac: Rework coalesce timer and fix multi-queue races
From: Jose Abreu @ 2018-09-10 12:55 UTC (permalink / raw)
  To: Neil Armstrong, Jose Abreu, netdev
  Cc: Jerome Brunet, Martin Blumenstingl, David S. Miller, Joao Pinto,
	Giuseppe Cavallaro, Alexandre Torgue
In-Reply-To: <e3492330-7ab6-254a-4f6d-a422ae4d9acb@synopsys.com>

On 10-09-2018 13:52, Jose Abreu wrote:
>
> Can you please try attached follow-up patch ? 

Oh, please apply the whole series otherwise this will not apply
cleanly.

Thanks and Best Regards,
Jose Miguel Abreu

^ permalink raw reply

* [PATCH v2] r8169: Clear RTL_FLAG_TASK_*_PENDING when clearing RTL_FLAG_TASK_ENABLED
From: Kai-Heng Feng @ 2018-09-10 17:51 UTC (permalink / raw)
  To: nic_swsd; +Cc: davem, netdev, linux-kernel, Kai-Heng Feng, Heiner Kallweit

After system suspend, sometimes the r8169 doesn't work when ethernet
cable gets pluggued.

This issue happens because rtl_reset_work() doesn't get called from
rtl8169_runtime_resume(), after system suspend.

In rtl_task(), RTL_FLAG_TASK_* only gets cleared if this condition is
met:
if (!netif_running(dev) ||
    !test_bit(RTL_FLAG_TASK_ENABLED, tp->wk.flags))
    ...

If RTL_FLAG_TASK_ENABLED was cleared during system suspend while
RTL_FLAG_TASK_RESET_PENDING was set, the next rtl_schedule_task() won't
schedule task as the flag is still there.

So in addition to clearing RTL_FLAG_TASK_ENABLED, also clears other
flags.

Cc: Heiner Kallweit <hkallweit1@gmail.com>
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
v2:
- Use bitmap_zero(), suggested by Florian Fainelli.
- Assign zero to first enum member in rtl_flag, just in case.

 drivers/net/ethernet/realtek/r8169.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index b08d51bf7a20..a1bc6404d1d4 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -631,7 +631,7 @@ struct rtl8169_tc_offsets {
 };
 
 enum rtl_flag {
-	RTL_FLAG_TASK_ENABLED,
+	RTL_FLAG_TASK_ENABLED = 0,
 	RTL_FLAG_TASK_SLOW_PENDING,
 	RTL_FLAG_TASK_RESET_PENDING,
 	RTL_FLAG_MAX
@@ -6655,7 +6655,8 @@ static int rtl8169_close(struct net_device *dev)
 	rtl8169_update_counters(tp);
 
 	rtl_lock_work(tp);
-	clear_bit(RTL_FLAG_TASK_ENABLED, tp->wk.flags);
+	/* Clear all task flags */
+	bitmap_zero(tp->wk.flags, RTL_FLAG_MAX);
 
 	rtl8169_down(dev);
 	rtl_unlock_work(tp);
@@ -6838,7 +6839,9 @@ static void rtl8169_net_suspend(struct net_device *dev)
 
 	rtl_lock_work(tp);
 	napi_disable(&tp->napi);
-	clear_bit(RTL_FLAG_TASK_ENABLED, tp->wk.flags);
+	/* Clear all task flags */
+	bitmap_zero(tp->wk.flags, RTL_FLAG_MAX);
+
 	rtl_unlock_work(tp);
 
 	rtl_pll_power_down(tp);
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH] can: rcar_can: convert to SPDX identifiers
From: Simon Horman @ 2018-09-10 13:11 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Wolfgang Grandegger, Marc Kleine-Budde, Linux-Renesas, Linux-Net,
	linux-can
In-Reply-To: <877ejyqdhy.wl-kuninori.morimoto.gx@renesas.com>

On Fri, Sep 07, 2018 at 02:00:44AM +0000, Kuninori Morimoto wrote:
> 
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> This patch updates license to use SPDX-License-Identifier
> instead of verbose license text.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

^ permalink raw reply

* Re: [PATCH v3 net-next 6/6] net: dsa: Add Lantiq / Intel DSA driver for vrx200
From: Andrew Lunn @ 2018-09-10 13:27 UTC (permalink / raw)
  To: Hauke Mehrtens
  Cc: davem, netdev, vivien.didelot, f.fainelli, john, linux-mips, dev,
	hauke.mehrtens, devicetree
In-Reply-To: <20180909202039.471-1-hauke@hauke-m.de>

On Sun, Sep 09, 2018 at 10:20:39PM +0200, Hauke Mehrtens wrote:
> +static void gswip_phylink_validate(struct dsa_switch *ds, int port,
> +				   unsigned long *supported,
> +				   struct phylink_link_state *state)
> +{
> +	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> +
> +	switch (port) {
> +	case 0:
> +	case 1:
> +		if (!phy_interface_mode_is_rgmii(state->interface) &&
> +		    state->interface != PHY_INTERFACE_MODE_MII &&
> +		    state->interface != PHY_INTERFACE_MODE_REVMII &&
> +		    state->interface != PHY_INTERFACE_MODE_RMII) {
> +			bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
> +			dev_err(ds->dev,
> +			"Unsupported interface: %d\n", state->interface);
> +			return;
> +		}
> +		break;
> +	case 2:
> +	case 3:
> +	case 4:
> +		if (state->interface != PHY_INTERFACE_MODE_INTERNAL) {
> +			bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
> +			dev_err(ds->dev,
> +			"Unsupported interface: %d\n", state->interface);
> +			return;
> +		}
> +		break;
> +	case 5:
> +		if (!phy_interface_mode_is_rgmii(state->interface) &&
> +		    state->interface != PHY_INTERFACE_MODE_INTERNAL) {
> +			bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
> +			dev_err(ds->dev,
> +			"Unsupported interface: %d\n", state->interface);
> +			return;

Hi Hauke

Minor nit. You have the same thing repeated three times. Maybe change
it to a goto out; and have the error block only once at the out:
label.

> +static int gswip_gphy_fw_list(struct gswip_priv *priv,
> +			      struct device_node *gphy_fw_list_np, u32 version)
> +{
> +	struct device *dev = priv->dev;
> +	struct device_node *gphy_fw_np;
> +	const struct of_device_id *match;
> +	int err;
> +	int i = 0;
> +
> +	/* The The VRX200 rev 1.1 uses the GSWIP 2.0 and needs the older

Double The.

> +
> +	/* bring up the mdio bus */
> +	mdio_np = of_find_compatible_node(pdev->dev.of_node, NULL,
> +					  "lantiq,xrx200-mdio");
> +	if (mdio_np) {
> +		err = gswip_mdio(priv, mdio_np);
> +		if (err) {
> +			dev_err(dev, "mdio probe failed\n");
> +			goto gphy_fw;
> +		}
> +	}
> +
> +	err = dsa_register_switch(priv->ds);
> +	if (err) {
> +		dev_err(dev, "dsa switch register failed: %i\n", err);
> +		goto mdio_bus;
> +	}

> +	if (priv->ds->dst->cpu_dp->index != priv->hw_info->cpu_port) {

I think that can be simplified to

        if (!dsa_is_cpu_port(ds, priv->hw_info->cpu_port))

which is probably more readable.

Florian was also considering that we should move this test into the
DSA core. But for the moment, doing it here is O.K.

This is otherwise looking good.

Thanks

    Andrew

^ permalink raw reply

* Re: [PATCH net-next] virtio_net: ethtool tx napi configuration
From: Willem de Bruijn @ 2018-09-10 13:35 UTC (permalink / raw)
  To: Jason Wang
  Cc: Network Development, David Miller, caleb.raitto,
	Michael S. Tsirkin, Jon Olson (Google Drive), Willem de Bruijn
In-Reply-To: <cbc42263-0f2d-bf76-5842-e241dc1c97fb@redhat.com>

On Mon, Sep 10, 2018 at 2:01 AM Jason Wang <jasowang@redhat.com> wrote:
>
>
>
> On 2018年09月10日 06:44, Willem de Bruijn wrote:
> > From: Willem de Bruijn <willemb@google.com>
> >
> > Implement ethtool .set_coalesce (-C) and .get_coalesce (-c) handlers.
> > Interrupt moderation is currently not supported, so these accept and
> > display the default settings of 0 usec and 1 frame.
> >
> > Toggle tx napi through a bit in tx-frames. So as to not interfere
> > with possible future interrupt moderation, use bit 10, well outside
> > the reasonable range of real interrupt moderation values.
> >
> > Changes are not atomic. The tx IRQ, napi BH and transmit path must
> > be quiesced when switching modes. Only allow changing this setting
> > when the device is down.
>
> I cook a fixup, and it looks works in my setup:
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index b320b6b14749..9181c3f2f832 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2204,10 +2204,17 @@ static int virtnet_set_coalesce(struct
> net_device *dev,
>                  return -EINVAL;
>
>          if (napi_weight ^ vi->sq[0].napi.weight) {
> -               if (dev->flags & IFF_UP)
> -                       return -EBUSY;
> -               for (i = 0; i < vi->max_queue_pairs; i++)
> +               for (i = 0; i < vi->max_queue_pairs; i++) {
> +                       struct netdev_queue *txq =
> +                              netdev_get_tx_queue(vi->dev, i);
> +
> + virtnet_napi_tx_disable(&vi->sq[i].napi);
> +                       __netif_tx_lock_bh(txq);
>                          vi->sq[i].napi.weight = napi_weight;
> +                       __netif_tx_unlock_bh(txq);
> +                       virtnet_napi_tx_enable(vi, vi->sq[i].vq,
> + &vi->sq[i].napi);
> +               }
>          }
>
>          return 0;

Thanks! It passes my simple stress test, too. Which consists of two
concurrent loops, one toggling the ethtool option, another running
TCP_RR.

> The only left case is the speculative tx polling in RX NAPI. I think we
> don't need to care in this case since it was not a must for correctness.

As long as the txq lock is held that will be a noop, anyway. The other
concurrent action is skb_xmit_done. It looks correct to me, but need
to think about it a bit. The tricky transition is coming out of napi without
having >= 2 + MAX_SKB_FRAGS clean descriptors. If the queue is
stopped it may deadlock transmission in no-napi mode.

> >
> > Link: https://patchwork.ozlabs.org/patch/948149/
> > Suggested-by: Jason Wang <jasowang@redhat.com>
> > Signed-off-by: Willem de Bruijn <willemb@google.com>
> > ---
> >   drivers/net/virtio_net.c | 52 ++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 52 insertions(+)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 765920905226..b320b6b14749 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -66,6 +66,8 @@ DECLARE_EWMA(pkt_len, 0, 64)
> >
> >   #define VIRTNET_DRIVER_VERSION "1.0.0"
> >
> > +static const u32 ethtool_coalesce_napi_mask = (1UL << 10);
> > +
> >   static const unsigned long guest_offloads[] = {
> >       VIRTIO_NET_F_GUEST_TSO4,
> >       VIRTIO_NET_F_GUEST_TSO6,
> > @@ -2181,6 +2183,54 @@ static int virtnet_get_link_ksettings(struct net_device *dev,
> >       return 0;
> >   }
> >
> > +static int virtnet_set_coalesce(struct net_device *dev,
> > +                             struct ethtool_coalesce *ec)
> > +{
> > +     const struct ethtool_coalesce ec_default = {
> > +             .cmd = ETHTOOL_SCOALESCE,
> > +             .rx_max_coalesced_frames = 1,
>
> I think rx part is no necessary.

The definition of ethtool_coalesce has:

"* It is illegal to set both usecs and max_frames to zero as this
 * would cause interrupts to never be generated.  To disable
 * coalescing, set usecs = 0 and max_frames = 1."

I'd rather not diverge from this prescribed behavior unless there's
a strong reason.

On the related point in the other thread:

> Rethink about this, how about something like:
>
> - UINT_MAX: no tx interrupt
> - other value: tx interrupt with possible interrupt moderation

Okay, that will be simpler to configure.

^ permalink raw reply

* Re: [PATCH can-next] can: ucan: remove set but not used variable 'udev'
From: Martin Elshuber @ 2018-09-10 13:41 UTC (permalink / raw)
  Cc: linux-can, netdev, kernel-janitors
In-Reply-To: <1535507214-145191-1-git-send-email-yuehaibing@huawei.com>


[-- Attachment #1.1: Type: text/plain, Size: 1043 bytes --]

Thank you for the fix

Reviewed-by: Martin Elshuber <martin.elshuber@theobroma-systems.com>

> Fixes gcc '-Wunused-but-set-variable' warning:
> 
> drivers/net/can/usb/ucan.c: In function 'ucan_disconnect':
> drivers/net/can/usb/ucan.c:1578:21: warning:
>  variable 'udev' set but not used [-Wunused-but-set-variable]
>   struct usb_device *udev;
> 
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
>  drivers/net/can/usb/ucan.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/net/can/usb/ucan.c b/drivers/net/can/usb/ucan.c
> index 0678a38..c9fd83e 100644
> --- a/drivers/net/can/usb/ucan.c
> +++ b/drivers/net/can/usb/ucan.c
> @@ -1575,11 +1575,8 @@ static int ucan_probe(struct usb_interface *intf,
>  /* disconnect the device */
>  static void ucan_disconnect(struct usb_interface *intf)
>  {
> -	struct usb_device *udev;
>  	struct ucan_priv *up = usb_get_intfdata(intf);
>  
> -	udev = interface_to_usbdev(intf);
> -
>  	usb_set_intfdata(intf, NULL);
>  
>  	if (up) {
> 



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

^ permalink raw reply

* Re: [net-next, v2, 1/2] net: stmmac: Rework coalesce timer and fix multi-queue races
From: Neil Armstrong @ 2018-09-10 13:46 UTC (permalink / raw)
  To: Jose Abreu, netdev
  Cc: Jerome Brunet, Martin Blumenstingl, David S. Miller, Joao Pinto,
	Giuseppe Cavallaro, Alexandre Torgue
In-Reply-To: <ffed5dd1-cded-eab3-c7b3-4502a3b85095@synopsys.com>

hi Jose,

On 10/09/2018 14:55, Jose Abreu wrote:
> On 10-09-2018 13:52, Jose Abreu wrote:
>>
>> Can you please try attached follow-up patch ? 
> 
> Oh, please apply the whole series otherwise this will not apply
> cleanly.

Indeed, it helps!

With the fixups, it fails later, around 15s instead of 3, in RX and TX.

Neil

> 
> Thanks and Best Regards,
> Jose Miguel Abreu
> 
> 

^ 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