* Re: [PATCH net-next] net: dsa: Add support for 64-bit statistics
From: Florian Fainelli @ 2017-08-01 22:42 UTC (permalink / raw)
To: netdev; +Cc: davem, Andrew Lunn, Vivien Didelot, open list
In-Reply-To: <20170801220036.11355-1-f.fainelli@gmail.com>
On 08/01/2017 03:00 PM, Florian Fainelli wrote:
> DSA slave network devices maintain a pair of bytes and packets counters
> for each directions, but these are not 64-bit capable. Re-use
> pcpu_sw_netstats which contains exactly what we need for that purpose
> and update the code path to report 64-bit capable statistics.
Humm, I do see a long time for ifconfig/ethtool -S to complete after
having transfered over 250GiB of data, the locking looks correct to me
in that statistics for the RX path are updated from softIRQ context
(napi_gro_receive/netif_receive_skb) but maybe I did misunderstand
whether the softirq/IRQ context applies to the producer and not the
reader...
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> net/dsa/dsa.c | 8 ++++++--
> net/dsa/dsa_priv.h | 2 ++
> net/dsa/slave.c | 38 +++++++++++++++++++++++++++++++-------
> 3 files changed, 39 insertions(+), 9 deletions(-)
>
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index a55e2e4087a4..0ba842c08dd3 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -190,6 +190,7 @@ static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
> {
> struct dsa_switch_tree *dst = dev->dsa_ptr;
> struct sk_buff *nskb = NULL;
> + struct dsa_slave_priv *p;
>
> if (unlikely(dst == NULL)) {
> kfree_skb(skb);
> @@ -207,12 +208,15 @@ static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
> }
>
> skb = nskb;
> + p = netdev_priv(skb->dev);
> skb_push(skb, ETH_HLEN);
> skb->pkt_type = PACKET_HOST;
> skb->protocol = eth_type_trans(skb, skb->dev);
>
> - skb->dev->stats.rx_packets++;
> - skb->dev->stats.rx_bytes += skb->len;
> + u64_stats_update_begin(&p->stats64.syncp);
> + p->stats64.rx_packets++;
> + p->stats64.rx_bytes += skb->len;
> + u64_stats_update_end(&p->stats64.syncp);
>
> netif_receive_skb(skb);
>
> diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
> index 55982cc39b24..7aa0656296c2 100644
> --- a/net/dsa/dsa_priv.h
> +++ b/net/dsa/dsa_priv.h
> @@ -77,6 +77,8 @@ struct dsa_slave_priv {
> struct sk_buff * (*xmit)(struct sk_buff *skb,
> struct net_device *dev);
>
> + struct pcpu_sw_netstats stats64;
> +
> /* DSA port data, such as switch, port index, etc. */
> struct dsa_port *dp;
>
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 9507bd38cf04..65f3cef85976 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -354,8 +354,10 @@ static netdev_tx_t dsa_slave_xmit(struct sk_buff *skb, struct net_device *dev)
> struct dsa_slave_priv *p = netdev_priv(dev);
> struct sk_buff *nskb;
>
> - dev->stats.tx_packets++;
> - dev->stats.tx_bytes += skb->len;
> + u64_stats_update_begin(&p->stats64.syncp);
> + p->stats64.tx_packets++;
> + p->stats64.tx_bytes += skb->len;
> + u64_stats_update_end(&p->stats64.syncp);
>
> /* Transmit function may have to reallocate the original SKB,
> * in which case it must have freed it. Only free it here on error.
> @@ -594,11 +596,15 @@ static void dsa_slave_get_ethtool_stats(struct net_device *dev,
> {
> struct dsa_slave_priv *p = netdev_priv(dev);
> struct dsa_switch *ds = p->dp->ds;
> -
> - data[0] = dev->stats.tx_packets;
> - data[1] = dev->stats.tx_bytes;
> - data[2] = dev->stats.rx_packets;
> - data[3] = dev->stats.rx_bytes;
> + unsigned int start;
> +
> + do {
> + start = u64_stats_fetch_begin_irq(&p->stats64.syncp);
> + data[0] = p->stats64.tx_packets;
> + data[1] = p->stats64.tx_bytes;
> + data[2] = p->stats64.rx_packets;
> + data[3] = p->stats64.rx_bytes;
> + } while (u64_stats_fetch_retry_irq(&p->stats64.syncp, start));
> if (ds->ops->get_ethtool_stats)
> ds->ops->get_ethtool_stats(ds, p->dp->index, data + 4);
> }
> @@ -861,6 +867,22 @@ static int dsa_slave_setup_tc(struct net_device *dev, u32 handle,
> }
> }
>
> +static void dsa_slave_get_stats64(struct net_device *dev,
> + struct rtnl_link_stats64 *stats)
> +{
> + struct dsa_slave_priv *p = netdev_priv(dev);
> + unsigned int start;
> +
> + netdev_stats_to_stats64(stats, &dev->stats);
> + do {
> + start = u64_stats_fetch_begin_irq(&p->stats64.syncp);
> + stats->tx_packets = p->stats64.tx_packets;
> + stats->tx_bytes = p->stats64.tx_bytes;
> + stats->rx_packets = p->stats64.rx_packets;
> + stats->rx_bytes = p->stats64.rx_bytes;
> + } while (u64_stats_fetch_retry_irq(&p->stats64.syncp, start));
> +}
> +
> void dsa_cpu_port_ethtool_init(struct ethtool_ops *ops)
> {
> ops->get_sset_count = dsa_cpu_port_get_sset_count;
> @@ -936,6 +958,7 @@ static const struct net_device_ops dsa_slave_netdev_ops = {
> .ndo_bridge_dellink = switchdev_port_bridge_dellink,
> .ndo_get_phys_port_name = dsa_slave_get_phys_port_name,
> .ndo_setup_tc = dsa_slave_setup_tc,
> + .ndo_get_stats64 = dsa_slave_get_stats64,
> };
>
> static const struct switchdev_ops dsa_slave_switchdev_ops = {
> @@ -1171,6 +1194,7 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
> slave_dev->vlan_features = master->vlan_features;
>
> p = netdev_priv(slave_dev);
> + u64_stats_init(&p->stats64.syncp);
> p->dp = &ds->ports[port];
> INIT_LIST_HEAD(&p->mall_tc_list);
> p->xmit = dst->tag_ops->xmit;
>
--
Florian
^ permalink raw reply
* admin
From: administrador @ 2017-08-01 18:17 UTC (permalink / raw)
To: Recipients
ATENCIÓN;
Su buzón ha superado el límite de almacenamiento, que es de 5 GB definidos por el administrador, quien actualmente está ejecutando en 10.9GB, no puede ser capaz de enviar o recibir correo nuevo hasta que vuelva a validar su buzón de correo electrónico. Para revalidar su buzón de correo, envíe la siguiente información a continuación:
nombre:
Nombre de usuario:
contraseña:
Confirmar contraseña:
E-mail:
teléfono:
Si usted no puede revalidar su buzón, el buzón se deshabilitará!
Disculpa las molestias.
Código de verificación: es: 006524
Correo Soporte Técnico © 2017
¡gracias
Sistemas administrador
^ permalink raw reply
* Re: [PATCH] Cipso: cipso_v4_optptr enter infinite loop
From: David Miller @ 2017-08-01 22:31 UTC (permalink / raw)
To: yujuan.qi; +Cc: pmoore, casey, netdev, linux-mediatek, linux-kernel, ryder.lee
In-Reply-To: <1501471381-12808-1-git-send-email-yujuan.qi@mediatek.com>
From: Yujuan Qi <yujuan.qi@mediatek.com>
Date: Mon, 31 Jul 2017 11:23:01 +0800
> From: "yujuan.qi" <yujuan.qi@mediatek.com>
>
> in for(),if((optlen > 0) && (optptr[1] == 0)), enter infinite loop.
>
> Test: receive a packet which the ip length > 20 and the first byte of ip option is 0, produce this issue
>
> Signed-off-by: yujuan.qi <yujuan.qi@mediatek.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] drivers/net/wan/z85230.c: Use designated initializers
From: David Miller @ 2017-08-01 22:29 UTC (permalink / raw)
To: keescook; +Cc: linux-kernel, netdev
In-Reply-To: <20170731013117.GA67801@beast>
From: Kees Cook <keescook@chromium.org>
Date: Sun, 30 Jul 2017 18:31:17 -0700
> In preparation for the randstruct gcc plugin performing randomization of
> structures that are entirely function pointers, use designated initializers
> so the compiler doesn't get angry.
>
> Reported-by: kbuild test robot <fengguang.wu@intel.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> This is a prerequisite for the future randstruct fptr randomization. I'd
> prefer to carry this in my gcc-plugin tree for v4.14 with an Ack from
> someone on net-dev, or if possible, have it applied to v4.13 via net-dev.
Please queue this up into the gcc-plugin tree then:
Acked-by: David S. Miller <davem@davemloft.net>
It isn't a bug fix so I would only have put it into 'net-next' rather
than 'net'.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next 0/3] net: Infrastructure changes for [kz]proxy
From: David Miller @ 2017-08-01 22:26 UTC (permalink / raw)
To: tom; +Cc: netdev, rohit, john.fastabend
In-Reply-To: <20170728232243.3040-1-tom@quantonium.net>
From: Tom Herbert <tom@quantonium.net>
Date: Fri, 28 Jul 2017 16:22:40 -0700
> This patch set contains some general infrastructure enhancements that
> will be used by kernel proxy and zero proxy.
>
> The changes are:
> - proto_ops: Add locked versions of sendmsg and sendpage
> - skb_send_sock: Allow sending and skb on a socket within the
> kernel
> - Generalize strparser. Allow it to be used in other contexts than
> just in the read_sock path. This will be used in the transmit
> path of zero proxy.
>
> Some nice future work (which I've been discussing with John Fastabend)
> will be to make some of the related functions to allow gifting of skbs
> We should be able to do that with skb_send_sock and strp_process. I'd
> also like this feature in the read_sock callbeck.
>
> Tested: Ran modified kernel without incident. Tested new functionality
> using zero proxy (in development).
Series applied, thanks Tom.
^ permalink raw reply
* Re: [PATCH v4 0/4] net: ethernet: ti: cpts: fix tx timestamping timeout
From: David Miller @ 2017-08-01 22:24 UTC (permalink / raw)
To: grygorii.strashko
Cc: netdev, richardcochran, nsekhar, linux-kernel, linux-omap,
w-kwok2, ivan.khoronzhuk, john.stultz, tglx
In-Reply-To: <20170728223005.21420-1-grygorii.strashko@ti.com>
From: Grygorii Strashko <grygorii.strashko@ti.com>
Date: Fri, 28 Jul 2017 17:30:01 -0500
> With the low Ethernet connection speed cpdma notification about packet
> processing can be received before CPTS TX timestamp event, which is set
> when packet actually left CPSW while cpdma notification is sent when packet
> pushed in CPSW fifo. As result, when connection is slow and CPU is fast
> enough TX timestamping is not working properly.
> Issue was discovered using timestamping tool on am57x boards with Ethernet link
> speed forced to 100M and on am335x-evm with Ethernet link speed forced to 10M.
>
> Patch3 - This series fixes it by introducing TX SKB queue to store PTP SKBs for
> which Ethernet Transmit Event hasn't been received yet and then re-check this
> queue with new Ethernet Transmit Events by scheduling CPTS overflow
> work more often until TX SKB queue is not empty.
>
> Patch 1,2 - As CPTS overflow work is time critical task it important to ensure
> that its scheduling is not delayed. Unfortunately, There could be significant
> delay in CPTS work schedule under high system load and on -RT which could cause
> CPTS misbehavior due to internal counter overflow and there is no way to tune
> CPTS overflow work execution policy and priority manually. The kthread_worker
> can be used instead of workqueues, as it creates separate named kthread for
> each worker and its its execution policy and priority can be configured
> using chrt tool. Instead of modifying CPTS driver itself it was proposed to
> it was proposed to add PTP auxiliary worker to the PHC subsystem [1], so
> other drivers can benefit from this feature also.
>
> [1] https://www.spinics.net/lists/netdev/msg445392.html
Series applied to 'net', thanks.
^ permalink raw reply
* Re: [PATCH v6 net-next] net: systemport: Support 64bit statistics
From: Florian Fainelli @ 2017-08-01 22:21 UTC (permalink / raw)
To: Jianming.qiao, davem, eric.dumazet, netdev
In-Reply-To: <1501550293-20443-1-git-send-email-jqiaoulk@gmail.com>
On 07/31/2017 06:18 PM, Jianming.qiao wrote:
> When using Broadcom Systemport device in 32bit Platform, ifconfig can
> only report up to 4G tx,rx status, which will be wrapped to 0 when the
> number of incoming or outgoing packets exceeds 4G, only taking
> around 2 hours in busy network environment (such as streaming).
> Therefore, it makes hard for network diagnostic tool to get reliable
> statistical result, so the patch is used to add 64bit support for
> Broadcom Systemport device in 32bit Platform.
>
> Signed-off-by: Jianming.qiao <kiki-good@hotmail.com>
> ---
> drivers/net/ethernet/broadcom/bcmsysport.c | 68 ++++++++++++++++++++----------
> drivers/net/ethernet/broadcom/bcmsysport.h | 9 +++-
> 2 files changed, 52 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
> index 5333601..bb3cc7a 100644
> --- a/drivers/net/ethernet/broadcom/bcmsysport.c
> +++ b/drivers/net/ethernet/broadcom/bcmsysport.c
> @@ -662,6 +662,7 @@ static int bcm_sysport_alloc_rx_bufs(struct bcm_sysport_priv *priv)
> static unsigned int bcm_sysport_desc_rx(struct bcm_sysport_priv *priv,
> unsigned int budget)
> {
> + struct bcm_sysport_stats *stats64 = &priv->stats64;
> struct net_device *ndev = priv->netdev;
> unsigned int processed = 0, to_process;
> struct bcm_sysport_cb *cb;
> @@ -765,6 +766,10 @@ static unsigned int bcm_sysport_desc_rx(struct bcm_sysport_priv *priv,
> skb->protocol = eth_type_trans(skb, ndev);
> ndev->stats.rx_packets++;
> ndev->stats.rx_bytes += len;
> + u64_stats_update_begin(&stats64->syncp);
> + stats64->rx_packets++;
> + stats64->rx_bytes += len;
> + u64_stats_update_end(&stats64->syncp);
>
> napi_gro_receive(&priv->napi, skb);
> next:
> @@ -787,17 +792,15 @@ static void bcm_sysport_tx_reclaim_one(struct bcm_sysport_tx_ring *ring,
> struct device *kdev = &priv->pdev->dev;
>
> if (cb->skb) {
> - ring->bytes += cb->skb->len;
> *bytes_compl += cb->skb->len;
> dma_unmap_single(kdev, dma_unmap_addr(cb, dma_addr),
> dma_unmap_len(cb, dma_len),
> DMA_TO_DEVICE);
> - ring->packets++;
> (*pkts_compl)++;
> bcm_sysport_free_cb(cb);
> /* SKB fragment */
> } else if (dma_unmap_addr(cb, dma_addr)) {
> - ring->bytes += dma_unmap_len(cb, dma_len);
> + *bytes_compl += dma_unmap_len(cb, dma_len);
> dma_unmap_page(kdev, dma_unmap_addr(cb, dma_addr),
> dma_unmap_len(cb, dma_len), DMA_TO_DEVICE);
> dma_unmap_addr_set(cb, dma_addr, 0);
> @@ -808,9 +811,10 @@ static void bcm_sysport_tx_reclaim_one(struct bcm_sysport_tx_ring *ring,
> static unsigned int __bcm_sysport_tx_reclaim(struct bcm_sysport_priv *priv,
> struct bcm_sysport_tx_ring *ring)
> {
> - struct net_device *ndev = priv->netdev;
> unsigned int c_index, last_c_index, last_tx_cn, num_tx_cbs;
> + struct bcm_sysport_stats *stats64 = &priv->stats64;
> unsigned int pkts_compl = 0, bytes_compl = 0;
> + struct net_device *ndev = priv->netdev;
> struct bcm_sysport_cb *cb;
> u32 hw_ind;
>
> @@ -849,6 +853,11 @@ static unsigned int __bcm_sysport_tx_reclaim(struct bcm_sysport_priv *priv,
> last_c_index &= (num_tx_cbs - 1);
> }
>
> + u64_stats_update_begin(&stats64->syncp);
> + ring->packets += pkts_compl;
> + ring->bytes += bytes_compl;
> + u64_stats_update_end(&stats64->syncp);
> +
> ring->c_index = c_index;
>
> netif_dbg(priv, tx_done, ndev,
> @@ -1671,24 +1680,6 @@ static int bcm_sysport_change_mac(struct net_device *dev, void *p)
> return 0;
> }
>
> -static struct net_device_stats *bcm_sysport_get_nstats(struct net_device *dev)
> -{
> - struct bcm_sysport_priv *priv = netdev_priv(dev);
> - unsigned long tx_bytes = 0, tx_packets = 0;
> - struct bcm_sysport_tx_ring *ring;
> - unsigned int q;
> -
> - for (q = 0; q < dev->num_tx_queues; q++) {
> - ring = &priv->tx_rings[q];
> - tx_bytes += ring->bytes;
> - tx_packets += ring->packets;
> - }
> -
> - dev->stats.tx_bytes = tx_bytes;
> - dev->stats.tx_packets = tx_packets;
> - return &dev->stats;
> -}
> -
> static void bcm_sysport_netif_start(struct net_device *dev)
> {
> struct bcm_sysport_priv *priv = netdev_priv(dev);
> @@ -1923,6 +1914,37 @@ static int bcm_sysport_stop(struct net_device *dev)
> return 0;
> }
>
> +static void bcm_sysport_get_stats64(struct net_device *dev,
> + struct rtnl_link_stats64 *stats)
> +{
> + struct bcm_sysport_priv *priv = netdev_priv(dev);
> + struct bcm_sysport_stats *stats64 = &priv->stats64;
> + struct bcm_sysport_tx_ring *ring;
> + u64 tx_packets = 0, tx_bytes = 0;
> + unsigned int start;
> + unsigned int q;
> +
> + netdev_stats_to_stats64(stats, &dev->stats);
> +
> + for (q = 0; q < dev->num_tx_queues; q++) {
> + ring = &priv->tx_rings[q];
> + do {
> + start = u64_stats_fetch_begin_irq(&stats64->syncp);
> + tx_bytes = ring->bytes;
> + tx_packets = ring->packets;
> + } while (u64_stats_fetch_retry_irq(&stats64->syncp, start));
> +
> + stats->tx_bytes += tx_bytes;
> + stats->tx_packets += tx_packets;
> + }
> +
> + do {
> + start = u64_stats_fetch_begin_irq(&stats64->syncp);
> + stats->rx_packets = stats64->rx_packets;
> + stats->rx_bytes = stats64->rx_bytes;
> + } while (u64_stats_fetch_retry_irq(&stats64->syncp, start));
> +}
> +
> static const struct ethtool_ops bcm_sysport_ethtool_ops = {
> .get_drvinfo = bcm_sysport_get_drvinfo,
> .get_msglevel = bcm_sysport_get_msglvl,
> @@ -1950,7 +1972,7 @@ static int bcm_sysport_stop(struct net_device *dev)
> #ifdef CONFIG_NET_POLL_CONTROLLER
> .ndo_poll_controller = bcm_sysport_poll_controller,
> #endif
> - .ndo_get_stats = bcm_sysport_get_nstats,
> + .ndo_get_stats64 = bcm_sysport_get_stats64,
> };
>
> #define REV_FMT "v%2x.%02x"
> diff --git a/drivers/net/ethernet/broadcom/bcmsysport.h b/drivers/net/ethernet/broadcom/bcmsysport.h
> index 77a51c1..c03a176 100644
> --- a/drivers/net/ethernet/broadcom/bcmsysport.h
> +++ b/drivers/net/ethernet/broadcom/bcmsysport.h
> @@ -657,6 +657,9 @@ struct bcm_sysport_stats {
> enum bcm_sysport_stat_type type;
> /* reg offset from UMAC base for misc counters */
> u16 reg_offset;
> + u64 rx_packets;
> + u64 rx_bytes;
> + struct u64_stats_sync syncp;
Also this should be moved outside of the bcm_sysport_stats structure
because bcm_sysport_stats describe the HW MIB counters, and there are as
many BCM_SYSPORT_STATS_LEN which can be quite a lot. By adding one
bcm_sysport_stats at the end, you are wasting a few bytes because you
are not using the 4 members above, likewise, when declaring the array of
statistics, you are wasting as many bytes as what rx_packets, rx_bytes
and syncp represent for each MIB counter.
> };
>
> /* Software house keeping helper structure */
> @@ -693,8 +696,8 @@ struct bcm_sysport_tx_ring {
> struct bcm_sysport_cb *cbs; /* Transmit control blocks */
> struct dma_desc *desc_cpu; /* CPU view of the descriptor */
> struct bcm_sysport_priv *priv; /* private context backpointer */
> - unsigned long packets; /* packets statistics */
> - unsigned long bytes; /* bytes statistics */
> + u64 packets; /* packets statistics */
> + u64 bytes; /* bytes statistics */
> };
>
> /* Driver private structure */
> @@ -743,5 +746,7 @@ struct bcm_sysport_priv {
>
> /* Ethtool */
> u32 msg_enable;
> + /* 64bit stats on 32bit/64bit Machine */
> + struct bcm_sysport_stats stats64;
> };
> #endif /* __BCM_SYSPORT_H */
>
--
Florian
^ permalink raw reply
* net-next virtio_net merge...
From: David Miller @ 2017-08-01 22:12 UTC (permalink / raw)
To: mst; +Cc: netdev, jasowang
I just merged net into net-next and one of the conflicts had to do with
the truesize bug fix in 'net' conflicting with the changes in 'net-next'
which encode the headroom into the contexts for mergeable buffers.
I did my best to resolve this, but if you two would take a closer
look and test out the result I would really appreciate it!
Thanks.
^ permalink raw reply
* Re: [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default
From: Florian Fainelli @ 2017-08-01 22:08 UTC (permalink / raw)
To: Timur Tabi, David S. Miller, netdev
In-Reply-To: <ec4dedc0-d117-4fa7-9226-1aba61bf2e7e@codeaurora.org>
On 08/01/2017 03:02 PM, Timur Tabi wrote:
> On 08/01/2017 04:55 PM, Florian Fainelli wrote:
>> This is not specific to your EMAC, a lot of adapters have this problem
>> actually.
>>
>> I wonder if it would make sense to reach for a broader solution where we
>> could have a networking stack panic/oops notifier which will actively
>> clean up the active network devices' RX queue(s) and if tx_pause was
>> enabled, disable it. We could have drivers announce themselves as
>> needing this either via NETIF_F_* feature bit or some other private flag.
>
> Unfortunately, the problem occurs only when Linux hangs, to the point
> where the driver's interrupt handlers are blocked. The RX queue is 256
> entries, and the processor has 48 cores, so the EMAC is never going to
> send pause frames in any real-world situation.
>
> The only time I've seen pause frames sent out is in the lab when I halt
> the cores with a hardware debugger, and only if I have enough network
> traffic that the EMAC picks up.
The size and scale of your system makes it so but imagine e.g: a single
core ~ 1Ghz @ 1Gbits/sec system having the same problems, here you are
quite likely to see the system under panic flooding the network.
Then again your patch is fine and can be revised at any time a broader
facility is offered, I just felt like we actually have a good way with
reasonably driver-agnostic code to possibly deal with that problem.
Implementing such a solution would not be a -stable backport candidate
though....
--
Florian
^ permalink raw reply
* Re: Please merge net into net-next
From: David Miller @ 2017-08-01 22:08 UTC (permalink / raw)
To: ecree; +Cc: netdev
In-Reply-To: <7b79c842-4349-0537-5644-b4b2974d1668@solarflare.com>
From: Edward Cree <ecree@solarflare.com>
Date: Mon, 31 Jul 2017 18:25:16 +0100
> Could you please merge net into net-next, so I can rebase my BPF patches?
> Otherwise there's likely to be merge conflicts with the BPF_SUB fix.
This has now been done.
^ permalink raw reply
* Re: [PATCH 2/2] net: qcom/emac: add software control for pause frame mode
From: Florian Fainelli @ 2017-08-01 22:06 UTC (permalink / raw)
To: Timur Tabi, David S. Miller, netdev
In-Reply-To: <9e7c0589-d163-a735-bb4c-87bc216164d2@codeaurora.org>
On 08/01/2017 03:00 PM, Timur Tabi wrote:
> On 08/01/2017 04:51 PM, Florian Fainelli wrote:
>
>> A few adapters (bcmgenet, bcmsysport) support configuring the pause
>> quanta so it would not be inconceivable to try to update
>> ethtool_pauseparam to include additional fields such as:
>
> Wouldn't this require a change to the user space tool?
It would yes.
>
>> - number of pause frames to send where we define an arbitrary high
>> default value (e.g: 0xffff), N < 0xffff is something drivers can test
>> for whether they support it, and 0 is only valid if pause is already
>> disabled
>>
>> - pause quanta (16-bits)
>>
>> Private flags are not usually that great and there could be more
>> adapters capable of doing the same pause frame number configuration, but
>> since there is no available knob it's hard to know.
>
> Well, for the EMAC, the quanta in this case would be either 1 or
> infinite. For other devices, it could be any combination of values. In
> a future revision of the hardware, we might support a variable quanta.
> And I suspect that some devices measure the quanta in time, not count.
OK, either way is fine and there should be ways to convert back and
forth without too much trouble.
>
> How would the user know what the acceptable values are? If I set the
> quanta to 10 via user space, and my driver truncates that to 1, I don't
> think that would be acceptable.
There can be several ways to deal with that, we can either ask for a
strict rejection before committing the changes to the HW, or we can have
the HW round up/down to the nearest supported values and an user would
call get_pauseparam() to see which value ended-up being used.
Anyway food for thought, David decides what gets merged ;)
--
Florian
^ permalink raw reply
* [PATCH net-next] liquidio: set sriov_totalvfs correctly
From: Felix Manlunas @ 2017-08-01 22:05 UTC (permalink / raw)
To: davem; +Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla
From: Derek Chickles <derek.chickles@cavium.com>
The file /sys/devices/pci000.../sriov_totalvfs is showing a wrong value.
Fix it by calling pci_sriov_set_totalvfs() to set the total number of VFs
available after calculations for the number of PF and VF queues are made.
Signed-off-by: Derek Chickles <derek.chickles@cavium.com>
Signed-off-by: Raghu Vatsavayi <raghu.vatsavayi@cavium.com>
Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
---
drivers/net/ethernet/cavium/liquidio/lio_main.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c
index 1d8fefa..39a8dca 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
@@ -1825,6 +1825,11 @@ static int octeon_chip_specific_setup(struct octeon_device *oct)
case OCTEON_CN23XX_PCIID_PF:
oct->chip_id = OCTEON_CN23XX_PF_VID;
ret = setup_cn23xx_octeon_pf_device(oct);
+#ifdef CONFIG_PCI_IOV
+ if (!ret)
+ pci_sriov_set_totalvfs(oct->pci_dev,
+ oct->sriov_info.max_vfs);
+#endif
s = "CN23XX";
break;
^ permalink raw reply related
* Re: [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default
From: Timur Tabi @ 2017-08-01 22:02 UTC (permalink / raw)
To: Florian Fainelli, David S. Miller, netdev
In-Reply-To: <1af0df70-c8a9-f327-abba-9c977c4dfdc9@gmail.com>
On 08/01/2017 04:55 PM, Florian Fainelli wrote:
> This is not specific to your EMAC, a lot of adapters have this problem
> actually.
>
> I wonder if it would make sense to reach for a broader solution where we
> could have a networking stack panic/oops notifier which will actively
> clean up the active network devices' RX queue(s) and if tx_pause was
> enabled, disable it. We could have drivers announce themselves as
> needing this either via NETIF_F_* feature bit or some other private flag.
Unfortunately, the problem occurs only when Linux hangs, to the point
where the driver's interrupt handlers are blocked. The RX queue is 256
entries, and the processor has 48 cores, so the EMAC is never going to
send pause frames in any real-world situation.
The only time I've seen pause frames sent out is in the lab when I halt
the cores with a hardware debugger, and only if I have enough network
traffic that the EMAC picks up.
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH net-next] net: dsa: Add support for 64-bit statistics
From: Florian Fainelli @ 2017-08-01 22:00 UTC (permalink / raw)
To: netdev; +Cc: davem, Florian Fainelli, Andrew Lunn, Vivien Didelot, open list
DSA slave network devices maintain a pair of bytes and packets counters
for each directions, but these are not 64-bit capable. Re-use
pcpu_sw_netstats which contains exactly what we need for that purpose
and update the code path to report 64-bit capable statistics.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
net/dsa/dsa.c | 8 ++++++--
net/dsa/dsa_priv.h | 2 ++
net/dsa/slave.c | 38 +++++++++++++++++++++++++++++++-------
3 files changed, 39 insertions(+), 9 deletions(-)
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index a55e2e4087a4..0ba842c08dd3 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -190,6 +190,7 @@ static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
{
struct dsa_switch_tree *dst = dev->dsa_ptr;
struct sk_buff *nskb = NULL;
+ struct dsa_slave_priv *p;
if (unlikely(dst == NULL)) {
kfree_skb(skb);
@@ -207,12 +208,15 @@ static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
}
skb = nskb;
+ p = netdev_priv(skb->dev);
skb_push(skb, ETH_HLEN);
skb->pkt_type = PACKET_HOST;
skb->protocol = eth_type_trans(skb, skb->dev);
- skb->dev->stats.rx_packets++;
- skb->dev->stats.rx_bytes += skb->len;
+ u64_stats_update_begin(&p->stats64.syncp);
+ p->stats64.rx_packets++;
+ p->stats64.rx_bytes += skb->len;
+ u64_stats_update_end(&p->stats64.syncp);
netif_receive_skb(skb);
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 55982cc39b24..7aa0656296c2 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -77,6 +77,8 @@ struct dsa_slave_priv {
struct sk_buff * (*xmit)(struct sk_buff *skb,
struct net_device *dev);
+ struct pcpu_sw_netstats stats64;
+
/* DSA port data, such as switch, port index, etc. */
struct dsa_port *dp;
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 9507bd38cf04..65f3cef85976 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -354,8 +354,10 @@ static netdev_tx_t dsa_slave_xmit(struct sk_buff *skb, struct net_device *dev)
struct dsa_slave_priv *p = netdev_priv(dev);
struct sk_buff *nskb;
- dev->stats.tx_packets++;
- dev->stats.tx_bytes += skb->len;
+ u64_stats_update_begin(&p->stats64.syncp);
+ p->stats64.tx_packets++;
+ p->stats64.tx_bytes += skb->len;
+ u64_stats_update_end(&p->stats64.syncp);
/* Transmit function may have to reallocate the original SKB,
* in which case it must have freed it. Only free it here on error.
@@ -594,11 +596,15 @@ static void dsa_slave_get_ethtool_stats(struct net_device *dev,
{
struct dsa_slave_priv *p = netdev_priv(dev);
struct dsa_switch *ds = p->dp->ds;
-
- data[0] = dev->stats.tx_packets;
- data[1] = dev->stats.tx_bytes;
- data[2] = dev->stats.rx_packets;
- data[3] = dev->stats.rx_bytes;
+ unsigned int start;
+
+ do {
+ start = u64_stats_fetch_begin_irq(&p->stats64.syncp);
+ data[0] = p->stats64.tx_packets;
+ data[1] = p->stats64.tx_bytes;
+ data[2] = p->stats64.rx_packets;
+ data[3] = p->stats64.rx_bytes;
+ } while (u64_stats_fetch_retry_irq(&p->stats64.syncp, start));
if (ds->ops->get_ethtool_stats)
ds->ops->get_ethtool_stats(ds, p->dp->index, data + 4);
}
@@ -861,6 +867,22 @@ static int dsa_slave_setup_tc(struct net_device *dev, u32 handle,
}
}
+static void dsa_slave_get_stats64(struct net_device *dev,
+ struct rtnl_link_stats64 *stats)
+{
+ struct dsa_slave_priv *p = netdev_priv(dev);
+ unsigned int start;
+
+ netdev_stats_to_stats64(stats, &dev->stats);
+ do {
+ start = u64_stats_fetch_begin_irq(&p->stats64.syncp);
+ stats->tx_packets = p->stats64.tx_packets;
+ stats->tx_bytes = p->stats64.tx_bytes;
+ stats->rx_packets = p->stats64.rx_packets;
+ stats->rx_bytes = p->stats64.rx_bytes;
+ } while (u64_stats_fetch_retry_irq(&p->stats64.syncp, start));
+}
+
void dsa_cpu_port_ethtool_init(struct ethtool_ops *ops)
{
ops->get_sset_count = dsa_cpu_port_get_sset_count;
@@ -936,6 +958,7 @@ static const struct net_device_ops dsa_slave_netdev_ops = {
.ndo_bridge_dellink = switchdev_port_bridge_dellink,
.ndo_get_phys_port_name = dsa_slave_get_phys_port_name,
.ndo_setup_tc = dsa_slave_setup_tc,
+ .ndo_get_stats64 = dsa_slave_get_stats64,
};
static const struct switchdev_ops dsa_slave_switchdev_ops = {
@@ -1171,6 +1194,7 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
slave_dev->vlan_features = master->vlan_features;
p = netdev_priv(slave_dev);
+ u64_stats_init(&p->stats64.syncp);
p->dp = &ds->ports[port];
INIT_LIST_HEAD(&p->mall_tc_list);
p->xmit = dst->tag_ops->xmit;
--
2.9.3
^ permalink raw reply related
* Re: [PATCH 2/2] net: qcom/emac: add software control for pause frame mode
From: Timur Tabi @ 2017-08-01 22:00 UTC (permalink / raw)
To: Florian Fainelli, David S. Miller, netdev
In-Reply-To: <293ae5e0-c161-0c00-e40c-2583734aa2d4@gmail.com>
On 08/01/2017 04:51 PM, Florian Fainelli wrote:
> A few adapters (bcmgenet, bcmsysport) support configuring the pause
> quanta so it would not be inconceivable to try to update
> ethtool_pauseparam to include additional fields such as:
Wouldn't this require a change to the user space tool?
> - number of pause frames to send where we define an arbitrary high
> default value (e.g: 0xffff), N < 0xffff is something drivers can test
> for whether they support it, and 0 is only valid if pause is already
> disabled
>
> - pause quanta (16-bits)
>
> Private flags are not usually that great and there could be more
> adapters capable of doing the same pause frame number configuration, but
> since there is no available knob it's hard to know.
Well, for the EMAC, the quanta in this case would be either 1 or
infinite. For other devices, it could be any combination of values. In
a future revision of the hardware, we might support a variable quanta.
And I suspect that some devices measure the quanta in time, not count.
How would the user know what the acceptable values are? If I set the
quanta to 10 via user space, and my driver truncates that to 1, I don't
think that would be acceptable.
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* Re: [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default
From: Florian Fainelli @ 2017-08-01 21:55 UTC (permalink / raw)
To: Timur Tabi, David S. Miller, netdev
In-Reply-To: <1501623460-3575-2-git-send-email-timur@codeaurora.org>
On 08/01/2017 02:37 PM, Timur Tabi wrote:
> The EMAC has a curious qwirk when RX flow control is enabled and the
> kernel hangs. With the kernel hung, the EMAC's RX queue soon fills.
> If RX flow control is enabled, the EMAC will then send a non-stop
> stream of pause frames until the system is reset. The EMAC does not
> have a built-in watchdog.
>
> In various tests, the pause frame stream sometimes overloads nearby
> switches, effectively disabling the network. Since the RX queue is
> large and the host processor is more than capable of handling incoming
> packets quickly, the only time the EMAC will send any pause frames is
> when the kernel is hung and unrecoverable.
This is not specific to your EMAC, a lot of adapters have this problem
actually.
I wonder if it would make sense to reach for a broader solution where we
could have a networking stack panic/oops notifier which will actively
clean up the active network devices' RX queue(s) and if tx_pause was
enabled, disable it. We could have drivers announce themselves as
needing this either via NETIF_F_* feature bit or some other private flag.
>
> To avoid all these problems, we disable flow control autonegotiation
> by default, and only enable receiving pause frames.
>
> Cc: stable@vger.kernel.org # 4.11.x
> Signed-off-by: Timur Tabi <timur@codeaurora.org>
> ---
> drivers/net/ethernet/qualcomm/emac/emac.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/qualcomm/emac/emac.c b/drivers/net/ethernet/qualcomm/emac/emac.c
> index 60850bfa3d32..475c0ea29235 100644
> --- a/drivers/net/ethernet/qualcomm/emac/emac.c
> +++ b/drivers/net/ethernet/qualcomm/emac/emac.c
> @@ -441,8 +441,13 @@ static void emac_init_adapter(struct emac_adapter *adpt)
> /* others */
> adpt->preamble = EMAC_PREAMBLE_DEF;
>
> - /* default to automatic flow control */
> - adpt->automatic = true;
> + /* Disable transmission of pause frames by default, to avoid the
> + * risk of a pause frame flood that can occur if the kernel hangs.
> + * We still want to be able to respond to them, however.
> + */
> + adpt->automatic = false;
> + adpt->tx_flow_control = false;
> + adpt->rx_flow_control = true;
> }
>
> /* Get the clock */
>
--
Florian
^ permalink raw reply
* Re: sysctl, argument parsing, possible bug
From: Cong Wang @ 2017-08-01 21:54 UTC (permalink / raw)
To: Massimo Sala
Cc: Luis R. Rodriguez, Kees Cook, LKML,
Linux Kernel Network Developers
In-Reply-To: <CAKv20-VFcdZti=4a6QzVJH0wAYGpqo2umktRPr19tv+btSZY_w@mail.gmail.com>
On Tue, Aug 1, 2017 at 2:34 PM, Massimo Sala <massimo.sala.71@gmail.com> wrote:
> Do you confirm it is a sysctl parsing bug ?
>
> Bosybox handles these cases, so I think also standalone sysctl have to.
>
> Or at least someone must update sysctl docs / MAN about this.
>
Maybe, sysctl seems (I never look into it) just to interpret dot as
a separator, unless it reads from the /proc/sys/ directory, it can
not know eth0.100 is actually a netdev name.
^ permalink raw reply
* Re: [PATCH 2/2] net: qcom/emac: add software control for pause frame mode
From: Florian Fainelli @ 2017-08-01 21:51 UTC (permalink / raw)
To: Timur Tabi, David S. Miller, netdev
In-Reply-To: <1501623460-3575-3-git-send-email-timur@codeaurora.org>
On 08/01/2017 02:37 PM, Timur Tabi wrote:
> The EMAC has the option of sending only a single pause frame when
> flow control is enabled and the RX queue is full. Although sending
> only one pause frame has little value, this would allow admins to
> enable automatic flow control without having to worry about the EMAC
> flooding nearby switches with pause frames if the kernel hangs.
>
> The option is enabled by using the single-pause-mode private flag.
A few adapters (bcmgenet, bcmsysport) support configuring the pause
quanta so it would not be inconceivable to try to update
ethtool_pauseparam to include additional fields such as:
- number of pause frames to send where we define an arbitrary high
default value (e.g: 0xffff), N < 0xffff is something drivers can test
for whether they support it, and 0 is only valid if pause is already
disabled
- pause quanta (16-bits)
Private flags are not usually that great and there could be more
adapters capable of doing the same pause frame number configuration, but
since there is no available knob it's hard to know.
>
> Signed-off-by: Timur Tabi <timur@codeaurora.org>
> ---
> drivers/net/ethernet/qualcomm/emac/emac-ethtool.c | 30 +++++++++++++++++++++++
> drivers/net/ethernet/qualcomm/emac/emac-mac.c | 22 +++++++++++++++++
> drivers/net/ethernet/qualcomm/emac/emac.c | 3 +++
> drivers/net/ethernet/qualcomm/emac/emac.h | 3 +++
> 4 files changed, 58 insertions(+)
>
> diff --git a/drivers/net/ethernet/qualcomm/emac/emac-ethtool.c b/drivers/net/ethernet/qualcomm/emac/emac-ethtool.c
> index bbe24639aa5a..c8c6231b87f3 100644
> --- a/drivers/net/ethernet/qualcomm/emac/emac-ethtool.c
> +++ b/drivers/net/ethernet/qualcomm/emac/emac-ethtool.c
> @@ -88,6 +88,8 @@ static void emac_set_msglevel(struct net_device *netdev, u32 data)
> static int emac_get_sset_count(struct net_device *netdev, int sset)
> {
> switch (sset) {
> + case ETH_SS_PRIV_FLAGS:
> + return 1;
> case ETH_SS_STATS:
> return EMAC_STATS_LEN;
> default:
> @@ -100,6 +102,10 @@ static void emac_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
> unsigned int i;
>
> switch (stringset) {
> + case ETH_SS_PRIV_FLAGS:
> + strcpy(data, "single-pause-mode");
> + break;
> +
> case ETH_SS_STATS:
> for (i = 0; i < EMAC_STATS_LEN; i++) {
> strlcpy(data, emac_ethtool_stat_strings[i],
> @@ -230,6 +236,27 @@ static int emac_get_regs_len(struct net_device *netdev)
> return EMAC_MAX_REG_SIZE * sizeof(u32);
> }
>
> +#define EMAC_PRIV_ENABLE_SINGLE_PAUSE BIT(0)
> +
> +static int emac_set_priv_flags(struct net_device *netdev, u32 flags)
> +{
> + struct emac_adapter *adpt = netdev_priv(netdev);
> +
> + adpt->single_pause_mode = !!(flags & EMAC_PRIV_ENABLE_SINGLE_PAUSE);
> +
> + if (netif_running(netdev))
> + return emac_reinit_locked(adpt);
> +
> + return 0;
> +}
> +
> +static u32 emac_get_priv_flags(struct net_device *netdev)
> +{
> + struct emac_adapter *adpt = netdev_priv(netdev);
> +
> + return adpt->single_pause_mode ? EMAC_PRIV_ENABLE_SINGLE_PAUSE : 0;
> +}
> +
> static const struct ethtool_ops emac_ethtool_ops = {
> .get_link_ksettings = phy_ethtool_get_link_ksettings,
> .set_link_ksettings = phy_ethtool_set_link_ksettings,
> @@ -253,6 +280,9 @@ static int emac_get_regs_len(struct net_device *netdev)
>
> .get_regs_len = emac_get_regs_len,
> .get_regs = emac_get_regs,
> +
> + .set_priv_flags = emac_set_priv_flags,
> + .get_priv_flags = emac_get_priv_flags,
> };
>
> void emac_set_ethtool_ops(struct net_device *netdev)
> diff --git a/drivers/net/ethernet/qualcomm/emac/emac-mac.c b/drivers/net/ethernet/qualcomm/emac/emac-mac.c
> index bcd4708b3745..0ea3ca09c689 100644
> --- a/drivers/net/ethernet/qualcomm/emac/emac-mac.c
> +++ b/drivers/net/ethernet/qualcomm/emac/emac-mac.c
> @@ -551,6 +551,28 @@ static void emac_mac_start(struct emac_adapter *adpt)
> mac &= ~(HUGEN | VLAN_STRIP | TPAUSE | SIMR | HUGE | MULTI_ALL |
> DEBUG_MODE | SINGLE_PAUSE_MODE);
>
> + /* Enable single-pause-frame mode if requested.
> + *
> + * If enabled, the EMAC will send a single pause frame when the RX
> + * queue is full. This normally leads to packet loss because
> + * the pause frame disables the remote MAC only for 33ms (the quanta),
> + * and then the remote MAC continues sending packets even though
> + * the RX queue is still full.
> + *
> + * If disabled, the EMAC sends a pause frame every 31ms until the RX
> + * queue is no longer full. Normally, this is the preferred
> + * method of operation. However, when the system is hung (e.g.
> + * cores are halted), the EMAC interrupt handler is never called
> + * and so the RX queue fills up quickly and stays full. The resuling
> + * non-stop "flood" of pause frames sometimes has the effect of
> + * disabling nearby switches. In some cases, other nearby switches
> + * are also affected, shutting down the entire network.
This explanation would have more value in your previous commit message,
especially since you are targeting it as a bugfix.
> + *
> + * The user can enable or disable single-pause-frame mode
> + * via ethtool.
> + */
> + mac |= adpt->single_pause_mode ? SINGLE_PAUSE_MODE : 0;
> +
> writel_relaxed(csr1, adpt->csr + EMAC_EMAC_WRAPPER_CSR1);
>
> writel_relaxed(mac, adpt->base + EMAC_MAC_CTRL);
> diff --git a/drivers/net/ethernet/qualcomm/emac/emac.c b/drivers/net/ethernet/qualcomm/emac/emac.c
> index 475c0ea29235..6f5e858ffbf3 100644
> --- a/drivers/net/ethernet/qualcomm/emac/emac.c
> +++ b/drivers/net/ethernet/qualcomm/emac/emac.c
> @@ -448,6 +448,9 @@ static void emac_init_adapter(struct emac_adapter *adpt)
> adpt->automatic = false;
> adpt->tx_flow_control = false;
> adpt->rx_flow_control = true;
> +
> + /* Disable single-pause-frame mode by default */
> + adpt->single_pause_mode = false;
> }
>
> /* Get the clock */
> diff --git a/drivers/net/ethernet/qualcomm/emac/emac.h b/drivers/net/ethernet/qualcomm/emac/emac.h
> index 8ee4ec6aef2e..d7c9f44209d4 100644
> --- a/drivers/net/ethernet/qualcomm/emac/emac.h
> +++ b/drivers/net/ethernet/qualcomm/emac/emac.h
> @@ -363,6 +363,9 @@ struct emac_adapter {
> bool tx_flow_control;
> bool rx_flow_control;
>
> + /* True == use single-pause-frame mode. */
> + bool single_pause_mode;
> +
> /* Ring parameter */
> u8 tpd_burst;
> u8 rfd_burst;
>
--
Florian
^ permalink raw reply
* Re: [PATCH V4 net 2/2] net: fix tcp reset packet flowlabel for ipv6
From: Shaohua Li @ 2017-08-01 21:42 UTC (permalink / raw)
To: Cong Wang
Cc: Linux Kernel Network Developers, David Miller, Shaohua Li,
Eric Dumazet, Florent Fourcot
In-Reply-To: <CAM_iQpWiw6CJOr2+zvxdoCht1DRERmZfZCCZiE36_6RiumEOVA@mail.gmail.com>
On Tue, Aug 01, 2017 at 02:17:58PM -0700, Cong Wang wrote:
> On Mon, Jul 31, 2017 at 4:00 PM, Shaohua Li <shli@kernel.org> wrote:
> > On Mon, Jul 31, 2017 at 03:35:02PM -0700, Cong Wang wrote:
> >> On Mon, Jul 31, 2017 at 3:19 PM, Shaohua Li <shli@kernel.org> wrote:
> >> > static inline __be32 ip6_make_flowlabel(struct net *net, struct sk_buff *skb,
> >> > __be32 flowlabel, bool autolabel,
> >> > - struct flowi6 *fl6)
> >> > + struct flowi6 *fl6, u32 hash)
> >> > {
> >> > - u32 hash;
> >> > -
> >> > /* @flowlabel may include more than a flow label, eg, the traffic class.
> >> > * Here we want only the flow label value.
> >> > */
> >> > @@ -788,7 +786,8 @@ static inline __be32 ip6_make_flowlabel(struct net *net, struct sk_buff *skb,
> >> > net->ipv6.sysctl.auto_flowlabels != IP6_AUTO_FLOW_LABEL_FORCED))
> >> > return flowlabel;
> >> >
> >> > - hash = skb_get_hash_flowi6(skb, fl6);
> >> > + if (skb)
> >> > + hash = skb_get_hash_flowi6(skb, fl6);
> >>
> >>
> >> Why not just move skb_get_hash_flowi6() to its caller?
> >> This check is not necessary. If you don't want to touch
> >> existing callers, you can just introduce a wrapper:
> >>
> >>
> >> static inline __be32 ip6_make_flowlabel(struct net *net, struct sk_buff *skb,
> >> __be32 flowlabel, bool autolabel,
> >> struct flowi6 *fl6)
> >> {
> >> u32 hash = skb_get_hash_flowi6(skb, fl6);
> >> return __ip6_make_flowlabel(net, flowlabel, autolabel, hash);
> >> }
> >
> > this will always call skb_get_hash_flowi6 for the fast path even auto flowlabel
> > is disabled. I thought we should avoid this.
>
> Yeah, but you can move the check out too,
> something like:
Is this really better? I don't see any point. I'd use my original patch other
than this one. that said, there are just several lines of code, brutally
'abstract' them into a function doesn't make the code better.
> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
> index 6eac5cf8f1e6..18ffa824c00a 100644
> --- a/include/net/ipv6.h
> +++ b/include/net/ipv6.h
> @@ -771,31 +771,22 @@ static inline void
> iph_to_flow_copy_v6addrs(struct flow_keys *flow,
>
> #define IP6_DEFAULT_AUTO_FLOW_LABELS IP6_AUTO_FLOW_LABEL_OPTOUT
>
> -static inline __be32 ip6_make_flowlabel(struct net *net, struct sk_buff *skb,
> - __be32 flowlabel, bool autolabel,
> - struct flowi6 *fl6)
> +static inline bool ip6_need_flowlabel(struct net *net, __be32
> flowlabel, bool autolabel)
> {
> - u32 hash;
> -
> /* @flowlabel may include more than a flow label, eg, the traffic class.
> * Here we want only the flow label value.
> */
> - flowlabel &= IPV6_FLOWLABEL_MASK;
> -
> - if (flowlabel ||
> + if ((flowlabel & IPV6_FLOWLABEL_MASK) ||
> net->ipv6.sysctl.auto_flowlabels == IP6_AUTO_FLOW_LABEL_OFF ||
> (!autolabel &&
> net->ipv6.sysctl.auto_flowlabels != IP6_AUTO_FLOW_LABEL_FORCED))
> - return flowlabel;
> -
> - hash = skb_get_hash_flowi6(skb, fl6);
> + return false;
>
> - /* Since this is being sent on the wire obfuscate hash a bit
> - * to minimize possbility that any useful information to an
> - * attacker is leaked. Only lower 20 bits are relevant.
> - */
> - rol32(hash, 16);
> + return true;
> +}
>
> +static inline __be32 __ip6_make_flowlabel(struct net *net, __be32
> flowlabel, u32 hash)
> +{
> flowlabel = (__force __be32)hash & IPV6_FLOWLABEL_MASK;
>
> if (net->ipv6.sysctl.flowlabel_state_ranges)
> @@ -804,6 +795,19 @@ static inline __be32 ip6_make_flowlabel(struct
> net *net, struct sk_buff *skb,
> return flowlabel;
> }
>
> +static inline __be32 ip6_make_flowlabel(struct net *net, struct sk_buff *skb,
> + __be32 flowlabel, bool autolabel,
> + struct flowi6 *fl6)
> +{
> + u32 hash;
> +
> + if (!ip6_need_flowlabel(net, flowlabel, autolabel))
> + return flowlabel & IPV6_FLOWLABEL_MASK;
> +
> + hash = skb_get_hash_flowi6(skb, fl6);
> + return __ip6_make_flowlabel(net, flowlabel, hash);
> +}
> +
> static inline int ip6_default_np_autolabel(struct net *net)
> {
> switch (net->ipv6.sysctl.auto_flowlabels) {
^ permalink raw reply
* [PATCH 2/2] net: qcom/emac: add software control for pause frame mode
From: Timur Tabi @ 2017-08-01 21:37 UTC (permalink / raw)
To: David S. Miller, netdev; +Cc: timur
In-Reply-To: <1501623460-3575-1-git-send-email-timur@codeaurora.org>
The EMAC has the option of sending only a single pause frame when
flow control is enabled and the RX queue is full. Although sending
only one pause frame has little value, this would allow admins to
enable automatic flow control without having to worry about the EMAC
flooding nearby switches with pause frames if the kernel hangs.
The option is enabled by using the single-pause-mode private flag.
Signed-off-by: Timur Tabi <timur@codeaurora.org>
---
drivers/net/ethernet/qualcomm/emac/emac-ethtool.c | 30 +++++++++++++++++++++++
drivers/net/ethernet/qualcomm/emac/emac-mac.c | 22 +++++++++++++++++
drivers/net/ethernet/qualcomm/emac/emac.c | 3 +++
drivers/net/ethernet/qualcomm/emac/emac.h | 3 +++
4 files changed, 58 insertions(+)
diff --git a/drivers/net/ethernet/qualcomm/emac/emac-ethtool.c b/drivers/net/ethernet/qualcomm/emac/emac-ethtool.c
index bbe24639aa5a..c8c6231b87f3 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-ethtool.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac-ethtool.c
@@ -88,6 +88,8 @@ static void emac_set_msglevel(struct net_device *netdev, u32 data)
static int emac_get_sset_count(struct net_device *netdev, int sset)
{
switch (sset) {
+ case ETH_SS_PRIV_FLAGS:
+ return 1;
case ETH_SS_STATS:
return EMAC_STATS_LEN;
default:
@@ -100,6 +102,10 @@ static void emac_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
unsigned int i;
switch (stringset) {
+ case ETH_SS_PRIV_FLAGS:
+ strcpy(data, "single-pause-mode");
+ break;
+
case ETH_SS_STATS:
for (i = 0; i < EMAC_STATS_LEN; i++) {
strlcpy(data, emac_ethtool_stat_strings[i],
@@ -230,6 +236,27 @@ static int emac_get_regs_len(struct net_device *netdev)
return EMAC_MAX_REG_SIZE * sizeof(u32);
}
+#define EMAC_PRIV_ENABLE_SINGLE_PAUSE BIT(0)
+
+static int emac_set_priv_flags(struct net_device *netdev, u32 flags)
+{
+ struct emac_adapter *adpt = netdev_priv(netdev);
+
+ adpt->single_pause_mode = !!(flags & EMAC_PRIV_ENABLE_SINGLE_PAUSE);
+
+ if (netif_running(netdev))
+ return emac_reinit_locked(adpt);
+
+ return 0;
+}
+
+static u32 emac_get_priv_flags(struct net_device *netdev)
+{
+ struct emac_adapter *adpt = netdev_priv(netdev);
+
+ return adpt->single_pause_mode ? EMAC_PRIV_ENABLE_SINGLE_PAUSE : 0;
+}
+
static const struct ethtool_ops emac_ethtool_ops = {
.get_link_ksettings = phy_ethtool_get_link_ksettings,
.set_link_ksettings = phy_ethtool_set_link_ksettings,
@@ -253,6 +280,9 @@ static int emac_get_regs_len(struct net_device *netdev)
.get_regs_len = emac_get_regs_len,
.get_regs = emac_get_regs,
+
+ .set_priv_flags = emac_set_priv_flags,
+ .get_priv_flags = emac_get_priv_flags,
};
void emac_set_ethtool_ops(struct net_device *netdev)
diff --git a/drivers/net/ethernet/qualcomm/emac/emac-mac.c b/drivers/net/ethernet/qualcomm/emac/emac-mac.c
index bcd4708b3745..0ea3ca09c689 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-mac.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac-mac.c
@@ -551,6 +551,28 @@ static void emac_mac_start(struct emac_adapter *adpt)
mac &= ~(HUGEN | VLAN_STRIP | TPAUSE | SIMR | HUGE | MULTI_ALL |
DEBUG_MODE | SINGLE_PAUSE_MODE);
+ /* Enable single-pause-frame mode if requested.
+ *
+ * If enabled, the EMAC will send a single pause frame when the RX
+ * queue is full. This normally leads to packet loss because
+ * the pause frame disables the remote MAC only for 33ms (the quanta),
+ * and then the remote MAC continues sending packets even though
+ * the RX queue is still full.
+ *
+ * If disabled, the EMAC sends a pause frame every 31ms until the RX
+ * queue is no longer full. Normally, this is the preferred
+ * method of operation. However, when the system is hung (e.g.
+ * cores are halted), the EMAC interrupt handler is never called
+ * and so the RX queue fills up quickly and stays full. The resuling
+ * non-stop "flood" of pause frames sometimes has the effect of
+ * disabling nearby switches. In some cases, other nearby switches
+ * are also affected, shutting down the entire network.
+ *
+ * The user can enable or disable single-pause-frame mode
+ * via ethtool.
+ */
+ mac |= adpt->single_pause_mode ? SINGLE_PAUSE_MODE : 0;
+
writel_relaxed(csr1, adpt->csr + EMAC_EMAC_WRAPPER_CSR1);
writel_relaxed(mac, adpt->base + EMAC_MAC_CTRL);
diff --git a/drivers/net/ethernet/qualcomm/emac/emac.c b/drivers/net/ethernet/qualcomm/emac/emac.c
index 475c0ea29235..6f5e858ffbf3 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac.c
@@ -448,6 +448,9 @@ static void emac_init_adapter(struct emac_adapter *adpt)
adpt->automatic = false;
adpt->tx_flow_control = false;
adpt->rx_flow_control = true;
+
+ /* Disable single-pause-frame mode by default */
+ adpt->single_pause_mode = false;
}
/* Get the clock */
diff --git a/drivers/net/ethernet/qualcomm/emac/emac.h b/drivers/net/ethernet/qualcomm/emac/emac.h
index 8ee4ec6aef2e..d7c9f44209d4 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac.h
+++ b/drivers/net/ethernet/qualcomm/emac/emac.h
@@ -363,6 +363,9 @@ struct emac_adapter {
bool tx_flow_control;
bool rx_flow_control;
+ /* True == use single-pause-frame mode. */
+ bool single_pause_mode;
+
/* Ring parameter */
u8 tpd_burst;
u8 rfd_burst;
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply related
* [PATCH 1/2] [for 4.13] net: qcom/emac: disable flow control autonegotiation by default
From: Timur Tabi @ 2017-08-01 21:37 UTC (permalink / raw)
To: David S. Miller, netdev; +Cc: timur
In-Reply-To: <1501623460-3575-1-git-send-email-timur@codeaurora.org>
The EMAC has a curious qwirk when RX flow control is enabled and the
kernel hangs. With the kernel hung, the EMAC's RX queue soon fills.
If RX flow control is enabled, the EMAC will then send a non-stop
stream of pause frames until the system is reset. The EMAC does not
have a built-in watchdog.
In various tests, the pause frame stream sometimes overloads nearby
switches, effectively disabling the network. Since the RX queue is
large and the host processor is more than capable of handling incoming
packets quickly, the only time the EMAC will send any pause frames is
when the kernel is hung and unrecoverable.
To avoid all these problems, we disable flow control autonegotiation
by default, and only enable receiving pause frames.
Cc: stable@vger.kernel.org # 4.11.x
Signed-off-by: Timur Tabi <timur@codeaurora.org>
---
drivers/net/ethernet/qualcomm/emac/emac.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/emac/emac.c b/drivers/net/ethernet/qualcomm/emac/emac.c
index 60850bfa3d32..475c0ea29235 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac.c
@@ -441,8 +441,13 @@ static void emac_init_adapter(struct emac_adapter *adpt)
/* others */
adpt->preamble = EMAC_PREAMBLE_DEF;
- /* default to automatic flow control */
- adpt->automatic = true;
+ /* Disable transmission of pause frames by default, to avoid the
+ * risk of a pause frame flood that can occur if the kernel hangs.
+ * We still want to be able to respond to them, however.
+ */
+ adpt->automatic = false;
+ adpt->tx_flow_control = false;
+ adpt->rx_flow_control = true;
}
/* Get the clock */
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply related
* [PATCH 0/2] net: qcom/emac: fixes for pause frame floods
From: Timur Tabi @ 2017-08-01 21:37 UTC (permalink / raw)
To: David S. Miller, netdev; +Cc: timur
The first patch is for 4.13. It's changes the default behavior of the
EMAC driver so that it doesn't send pause frames unless the user
enables them.
The second patch is for 4.14, but it can be applied to 4.13 if you
want. It adds the ability for the user to enable a special "single
pause frame" mode that could be useful in some situations.
Timur Tabi (2):
[for 4.13] net: qcom/emac: disable flow control autonegotiation by
default
net: qcom/emac: add software control for pause frame mode
drivers/net/ethernet/qualcomm/emac/emac-ethtool.c | 30 +++++++++++++++++++++++
drivers/net/ethernet/qualcomm/emac/emac-mac.c | 22 +++++++++++++++++
drivers/net/ethernet/qualcomm/emac/emac.c | 12 +++++++--
drivers/net/ethernet/qualcomm/emac/emac.h | 3 +++
4 files changed, 65 insertions(+), 2 deletions(-)
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* Re: sysctl, argument parsing, possible bug
From: Massimo Sala @ 2017-08-01 21:34 UTC (permalink / raw)
To: Cong Wang, Luis R. Rodriguez, Kees Cook
Cc: LKML, Linux Kernel Network Developers
In-Reply-To: <CAM_iQpXtHACnD8kmmHd4bcg+Khy5Y0K=HMmc08avTSHov4zN1A@mail.gmail.com>
Do you confirm it is a sysctl parsing bug ?
Bosybox handles these cases, so I think also standalone sysctl have to.
Or at least someone must update sysctl docs / MAN about this.
Best regards, Sala
On 01/08/2017, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Tue, Aug 1, 2017 at 1:47 PM, Massimo Sala <massimo.sala.71@gmail.com>
> wrote:
>> cat /proc/sys/net/ipv4/conf/eth0.100/forwarding
>> 0
>>
>> sysctl net.ipv4.conf.eth0.100.forwarding
>> error: "net.ipv4.conf.eth0.100.forwarding" is an unknown key
>>
>
> Use echo instead, sysctl doesn't understand eth0.100
> is a netdev name, sigh.
>
^ permalink raw reply
* Re: [PATCH net-next v2 01/11] net: dsa: PHY device is mandatory for EEE
From: Florian Fainelli @ 2017-08-01 21:31 UTC (permalink / raw)
To: Vivien Didelot, netdev
Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn, John Crispin
In-Reply-To: <20170801203241.22294-2-vivien.didelot@savoirfairelinux.com>
On 08/01/2017 01:32 PM, Vivien Didelot wrote:
> The port's PHY and MAC are both implied in EEE. The current code does
> not call the PHY operations if the related device is NULL. Change that
> by returning -ENODEV if there's no PHY device attached to the interface.
>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next v2 06/11] net: dsa: bcm_sf2: remove unneeded supported flags
From: Florian Fainelli @ 2017-08-01 21:29 UTC (permalink / raw)
To: Vivien Didelot, netdev
Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn, John Crispin
In-Reply-To: <20170801203241.22294-7-vivien.didelot@savoirfairelinux.com>
On 08/01/2017 01:32 PM, Vivien Didelot wrote:
> The SF2 driver is masking the supported bitfield of its private copy of
> the ports' ethtool_eee structures. It is used nowhere, thus remove it.
>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox