* Re: [PATCH net-next 2/2] net: bcmgenet: Add support for adaptive RX coalescing
From: Tal Gilboa @ 2018-03-26 21:23 UTC (permalink / raw)
To: Florian Fainelli, netdev
Cc: davem, jaedon.shin, pgynther, opendmb, michal.chan, gospo, saeedm
In-Reply-To: <20180323011933.29748-3-f.fainelli@gmail.com>
On 3/23/2018 4:19 AM, Florian Fainelli wrote:
> Unlike the moder modern SYSTEMPORT hardware, we do not have a
> configurable TDMA timeout, which limits us to implement adaptive RX
> interrupt coalescing only. We have each of our RX rings implement a
> bcmgenet_net_dim structure which holds an interrupt counter, number of
> packets, bytes, and a container for a net_dim instance.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/net/ethernet/broadcom/genet/bcmgenet.c | 109 +++++++++++++++++++++----
> drivers/net/ethernet/broadcom/genet/bcmgenet.h | 12 +++
> 2 files changed, 103 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index b1e35a9accf1..7db8edc643ec 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -603,6 +603,8 @@ static int bcmgenet_get_coalesce(struct net_device *dev,
> struct ethtool_coalesce *ec)
> {
> struct bcmgenet_priv *priv = netdev_priv(dev);
> + struct bcmgenet_rx_ring *ring;
> + unsigned int i;
>
> ec->tx_max_coalesced_frames =
> bcmgenet_tdma_ring_readl(priv, DESC_INDEX,
> @@ -613,15 +615,37 @@ static int bcmgenet_get_coalesce(struct net_device *dev,
> ec->rx_coalesce_usecs =
> bcmgenet_rdma_readl(priv, DMA_RING16_TIMEOUT) * 8192 / 1000;
>
> + for (i = 0; i < priv->hw_params->rx_queues; i++) {
> + ring = &priv->rx_rings[i];
> + ec->use_adaptive_rx_coalesce |= ring->dim.use_dim;
> + }
> + ring = &priv->rx_rings[DESC_INDEX];
> + ec->use_adaptive_rx_coalesce |= ring->dim.use_dim;
> +
> return 0;
> }
>
> +static void bcmgenet_set_rx_coalesce(struct bcmgenet_rx_ring *ring)
> +{
> + struct bcmgenet_priv *priv = ring->priv;
> + unsigned int i = ring->index;
> + u32 reg;
> +
> + bcmgenet_rdma_ring_writel(priv, i, ring->dim.coal_pkts,
> + DMA_MBUF_DONE_THRESH);
> +
> + reg = bcmgenet_rdma_readl(priv, DMA_RING0_TIMEOUT + i);
> + reg &= ~DMA_TIMEOUT_MASK;
> + reg |= DIV_ROUND_UP(ring->dim.coal_usecs * 1000, 8192);
> + bcmgenet_rdma_writel(priv, reg, DMA_RING0_TIMEOUT + i);
> +}
> +
Similar comments from path 1/2 apply here - wouldn't couple the genric
get_set_coalesce functions with dim.
> static int bcmgenet_set_coalesce(struct net_device *dev,
> struct ethtool_coalesce *ec)
> {
> struct bcmgenet_priv *priv = netdev_priv(dev);
> + struct bcmgenet_rx_ring *ring;
> unsigned int i;
> - u32 reg;
>
> /* Base system clock is 125Mhz, DMA timeout is this reference clock
> * divided by 1024, which yields roughly 8.192us, our maximum value
> @@ -641,7 +665,8 @@ static int bcmgenet_set_coalesce(struct net_device *dev,
> * transmitted, or when the ring is empty.
> */
> if (ec->tx_coalesce_usecs || ec->tx_coalesce_usecs_high ||
> - ec->tx_coalesce_usecs_irq || ec->tx_coalesce_usecs_low)
> + ec->tx_coalesce_usecs_irq || ec->tx_coalesce_usecs_low ||
> + ec->use_adaptive_tx_coalesce)
> return -EOPNOTSUPP;
>
> /* Program all TX queues with the same values, as there is no
> @@ -656,24 +681,26 @@ static int bcmgenet_set_coalesce(struct net_device *dev,
> DMA_MBUF_DONE_THRESH);
>
> for (i = 0; i < priv->hw_params->rx_queues; i++) {
> - bcmgenet_rdma_ring_writel(priv, i,
> - ec->rx_max_coalesced_frames,
> - DMA_MBUF_DONE_THRESH);
> -
> - reg = bcmgenet_rdma_readl(priv, DMA_RING0_TIMEOUT + i);
> - reg &= ~DMA_TIMEOUT_MASK;
> - reg |= DIV_ROUND_UP(ec->rx_coalesce_usecs * 1000, 8192);
> - bcmgenet_rdma_writel(priv, reg, DMA_RING0_TIMEOUT + i);
> + ring = &priv->rx_rings[i];
> + ring->dim.coal_usecs = ec->rx_coalesce_usecs;
> + ring->dim.coal_pkts = ec->rx_max_coalesced_frames;
> + if (!ec->use_adaptive_rx_coalesce && ring->dim.use_dim) {
> + ring->dim.coal_pkts = 1;
> + ring->dim.coal_usecs = 0;
> + }
> + ring->dim.use_dim = ec->use_adaptive_rx_coalesce;
> + bcmgenet_set_rx_coalesce(ring);
> }
>
> - bcmgenet_rdma_ring_writel(priv, DESC_INDEX,
> - ec->rx_max_coalesced_frames,
> - DMA_MBUF_DONE_THRESH);
> -
> - reg = bcmgenet_rdma_readl(priv, DMA_RING16_TIMEOUT);
> - reg &= ~DMA_TIMEOUT_MASK;
> - reg |= DIV_ROUND_UP(ec->rx_coalesce_usecs * 1000, 8192);
> - bcmgenet_rdma_writel(priv, reg, DMA_RING16_TIMEOUT);
> + ring = &priv->rx_rings[DESC_INDEX];
> + ring->dim.coal_usecs = ec->rx_coalesce_usecs;
> + ring->dim.coal_pkts = ec->rx_max_coalesced_frames;
> + if (!ec->use_adaptive_rx_coalesce && ring->dim.use_dim) {
> + ring->dim.coal_pkts = 1;
> + ring->dim.coal_usecs = 0;
> + }
> + ring->dim.use_dim = ec->use_adaptive_rx_coalesce;
> + bcmgenet_set_rx_coalesce(ring);
>
> return 0;
> }
> @@ -1713,6 +1740,7 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_rx_ring *ring,
> unsigned long dma_flag;
> int len;
> unsigned int rxpktprocessed = 0, rxpkttoprocess;
> + unsigned int bytes_processed = 0;
> unsigned int p_index, mask;
> unsigned int discards;
> unsigned int chksum_ok = 0;
> @@ -1832,6 +1860,8 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_rx_ring *ring,
> len -= ETH_FCS_LEN;
> }
>
> + bytes_processed += len;
> +
> /*Finish setting up the received SKB and send it to the kernel*/
> skb->protocol = eth_type_trans(skb, priv->dev);
> ring->packets++;
> @@ -1854,6 +1884,9 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_rx_ring *ring,
> bcmgenet_rdma_ring_writel(priv, ring->index, ring->c_index, RDMA_CONS_INDEX);
> }
>
> + ring->dim.bytes = bytes_processed;
> + ring->dim.packets = rxpktprocessed;
> +
> return rxpktprocessed;
> }
>
> @@ -1862,6 +1895,7 @@ static int bcmgenet_rx_poll(struct napi_struct *napi, int budget)
> {
> struct bcmgenet_rx_ring *ring = container_of(napi,
> struct bcmgenet_rx_ring, napi);
> + struct net_dim_sample dim_sample;
> unsigned int work_done;
>
> work_done = bcmgenet_desc_rx(ring, budget);
> @@ -1871,9 +1905,32 @@ static int bcmgenet_rx_poll(struct napi_struct *napi, int budget)
> ring->int_enable(ring);
> }
>
> + if (ring->dim.use_dim) {
> + net_dim_sample(ring->dim.event_ctr, ring->dim.packets,
> + ring->dim.bytes, &dim_sample);
> + net_dim(&ring->dim.dim, dim_sample);
> + }
> +
> return work_done;
> }
>
> +static void bcmgenet_dim_work(struct work_struct *work)
> +{
> + struct net_dim *dim = container_of(work, struct net_dim, work);
> + struct bcmgenet_net_dim *ndim =
> + container_of(dim, struct bcmgenet_net_dim, dim);
> + struct bcmgenet_rx_ring *ring =
> + container_of(ndim, struct bcmgenet_rx_ring, dim);
> + struct net_dim_cq_moder cur_profile =
> + net_dim_get_profile(dim->mode, dim->profile_ix);
> +
> + ring->dim.coal_usecs = cur_profile.usec;
> + ring->dim.coal_pkts = cur_profile.pkts;
> +
> + bcmgenet_set_rx_coalesce(ring);
> + dim->state = NET_DIM_START_MEASURE;
> +}
> +
> /* Assign skb to RX DMA descriptor. */
> static int bcmgenet_alloc_rx_buffers(struct bcmgenet_priv *priv,
> struct bcmgenet_rx_ring *ring)
> @@ -2022,6 +2079,16 @@ static void init_umac(struct bcmgenet_priv *priv)
> dev_dbg(kdev, "done init umac\n");
> }
>
> +static void bcmgenet_init_dim(struct bcmgenet_net_dim *dim,
> + void (*cb)(struct work_struct *work))
> +{
> + INIT_WORK(&dim->dim.work, cb);
> + dim->dim.mode = NET_DIM_CQ_PERIOD_MODE_START_FROM_EQE;
> + dim->event_ctr = 0;
> + dim->packets = 0;
> + dim->bytes = 0;
> +} > +
Similar comment from path 1/2 applies here - default values for
coal_usecs/pkts.
> /* Initialize a Tx ring along with corresponding hardware registers */
> static void bcmgenet_init_tx_ring(struct bcmgenet_priv *priv,
> unsigned int index, unsigned int size,
> @@ -2111,6 +2178,8 @@ static int bcmgenet_init_rx_ring(struct bcmgenet_priv *priv,
> if (ret)
> return ret;
>
> + bcmgenet_init_dim(&ring->dim, bcmgenet_dim_work);
> +
> /* Initialize Rx NAPI */
> netif_napi_add(priv->dev, &ring->napi, bcmgenet_rx_poll,
> NAPI_POLL_WEIGHT);
> @@ -2276,10 +2345,12 @@ static void bcmgenet_disable_rx_napi(struct bcmgenet_priv *priv)
> for (i = 0; i < priv->hw_params->rx_queues; ++i) {
> ring = &priv->rx_rings[i];
> napi_disable(&ring->napi);
> + cancel_work_sync(&ring->dim.dim.work);
> }
>
> ring = &priv->rx_rings[DESC_INDEX];
> napi_disable(&ring->napi);
> + cancel_work_sync(&ring->dim.dim.work);
> }
>
> static void bcmgenet_fini_rx_napi(struct bcmgenet_priv *priv)
> @@ -2557,6 +2628,7 @@ static irqreturn_t bcmgenet_isr1(int irq, void *dev_id)
> continue;
>
> rx_ring = &priv->rx_rings[index];
> + rx_ring->dim.event_ctr++;
>
> if (likely(napi_schedule_prep(&rx_ring->napi))) {
> rx_ring->int_disable(rx_ring);
> @@ -2601,6 +2673,7 @@ static irqreturn_t bcmgenet_isr0(int irq, void *dev_id)
>
> if (status & UMAC_IRQ_RXDMA_DONE) {
> rx_ring = &priv->rx_rings[DESC_INDEX];
> + rx_ring->dim.event_ctr++;
>
> if (likely(napi_schedule_prep(&rx_ring->napi))) {
> rx_ring->int_disable(rx_ring);
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
> index 3c50431ccd2a..22c41e0430fb 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
> @@ -16,6 +16,7 @@
> #include <linux/mii.h>
> #include <linux/if_vlan.h>
> #include <linux/phy.h>
> +#include <linux/net_dim.h>
>
> /* total number of Buffer Descriptors, same for Rx/Tx */
> #define TOTAL_DESC 256
> @@ -572,6 +573,16 @@ struct bcmgenet_tx_ring {
> struct bcmgenet_priv *priv;
> };
>
> +struct bcmgenet_net_dim {
> + u16 use_dim;
> + u16 event_ctr;
> + unsigned long packets;
> + unsigned long bytes;
> + u32 coal_usecs;
> + u32 coal_pkts;
> + struct net_dim dim;
> +};
> +
> struct bcmgenet_rx_ring {
> struct napi_struct napi; /* Rx NAPI struct */
> unsigned long bytes;
> @@ -586,6 +597,7 @@ struct bcmgenet_rx_ring {
> unsigned int cb_ptr; /* Rx ring initial CB ptr */
> unsigned int end_ptr; /* Rx ring end CB ptr */
> unsigned int old_discards;
> + struct bcmgenet_net_dim dim;
> void (*int_enable)(struct bcmgenet_rx_ring *);
> void (*int_disable)(struct bcmgenet_rx_ring *);
> struct bcmgenet_priv *priv;
>
^ permalink raw reply
* Re: [PATCH v5 bpf-next 06/10] tracepoint: compute num_args at build time
From: Mathieu Desnoyers @ 2018-03-26 21:27 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: rostedt, David S. Miller, Daniel Borkmann, Linus Torvalds,
Peter Zijlstra, netdev, kernel-team, linux-api, Frank Ch. Eigler
In-Reply-To: <17073efa-d833-7348-bef1-79376ad43bc6@fb.com>
----- On Mar 26, 2018, at 1:55 PM, Alexei Starovoitov ast@fb.com wrote:
[...]
>
> correct. this set deals with in-kernel tracepoints only.
> No attempt to do anything with tracepoints inside modules.
Please endeavor to handle in-module tracepoints properly, then we'll
be able to pursue a more constructive discussion.
[...]
> Also I hope you noticed that the patch is doing:
> +++ b/include/linux/tracepoint-defs.h
> @@ -33,6 +33,7 @@ struct tracepoint {
> int (*regfunc)(void);
> void (*unregfunc)(void);
> struct tracepoint_func __rcu *funcs;
> + u32 num_args;
> };
>
> To make sure that bpf programs are safe I need to do a static check
> in the verifier that programs don't access arguments beyond
> those specified by the tracepoint.
>
> That was mentioned in the commit log of patch 6 too:
> "
> compute number of arguments passed into tracepoint
> at compile time and store it as part of 'struct tracepoint'.
> The number is necessary to check safety of bpf program access that
> is coming in subsequent patch.
> "
This part of the patch and its associated changelog is fine
with me. Please submit it as a separate commit from the rest
of the tracepoint.{c,h} changes.
[...]
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information
From: Steve Wise @ 2018-03-26 21:34 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: David Ahern, leon, stephen, netdev, linux-rdma
In-Reply-To: <20180326211505.GF15554@ziepe.ca>
On 3/26/2018 4:15 PM, Jason Gunthorpe wrote:
> On Mon, Mar 26, 2018 at 09:30:41AM -0500, Steve Wise wrote:
>>
>> On 3/26/2018 9:17 AM, David Ahern wrote:
>>> On 2/27/18 9:07 AM, Steve Wise wrote:
>>>> diff --git a/rdma/rdma.h b/rdma/rdma.h
>>>> index 5809f70..e55205b 100644
>>>> +++ b/rdma/rdma.h
>>>> @@ -18,10 +18,12 @@
>>>> #include <libmnl/libmnl.h>
>>>> #include <rdma/rdma_netlink.h>
>>>> #include <time.h>
>>>> +#include <net/if_arp.h>
>>>>
>>>> #include "list.h"
>>>> #include "utils.h"
>>>> #include "json_writer.h"
>>>> +#include <rdma/rdma_cma.h>
>>>>
>>> did you forget to add rdma_cma.h? I don't see that file in my repo.
>> It is provided by the rdma-core package, upon which rdma tool now
>> depends for the rdma_port_space enum.
> It is a kernel bug that enum is not in an include/uapi/rdma header
>
> Fix it there and don't try to use rdma-core headers to get kernel ABI.
>
> Jason
I wish you'd commented on this just a little sooner. I just resent v3
of this series... with rdma_cma.h included. :)
How about the restrack/nldev code just translates the port space from
enum rdma_port_space to a new ABI enum, say nldev_rdma_port_space, that
i add to rdma_netlink.h? I'd hate to open the can of worms of trying to
split rdma_cma.h into uabi and no uabi headers. :(
Steve.
^ permalink raw reply
* Re: [PATCH net-next 1/2] net: systemport: Implement adaptive interrupt coalescing
From: Florian Fainelli @ 2018-03-26 21:36 UTC (permalink / raw)
To: Tal Gilboa, netdev
Cc: davem, jaedon.shin, pgynther, opendmb, michal.chan, gospo, saeedm
In-Reply-To: <58becbf1-baea-a679-ed32-b58fa5fd24bd@mellanox.com>
On 03/26/2018 02:22 PM, Tal Gilboa wrote:
> On 3/23/2018 4:19 AM, Florian Fainelli wrote:
>> Implement support for adaptive RX and TX interrupt coalescing using
>> net_dim. We have each of our TX ring and our single RX ring implement a
>> bcm_sysport_net_dim structure which holds an interrupt counter, number
>> of packets, bytes, and a container for a net_dim instance.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> drivers/net/ethernet/broadcom/bcmsysport.c | 141
>> ++++++++++++++++++++++++++---
>> drivers/net/ethernet/broadcom/bcmsysport.h | 14 +++
>> 2 files changed, 140 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c
>> b/drivers/net/ethernet/broadcom/bcmsysport.c
>> index f15a8fc6dfc9..5a5a726bafa4 100644
>> --- a/drivers/net/ethernet/broadcom/bcmsysport.c
>> +++ b/drivers/net/ethernet/broadcom/bcmsysport.c
>> @@ -15,6 +15,7 @@
>> #include <linux/module.h>
>> #include <linux/kernel.h>
>> #include <linux/netdevice.h>
>> +#include <linux/net_dim.h>
>
> I don't think you need this include. You already include net_dim in
> bcmsysport.h and include the bcmsysport.h here.
Indeed.
>
>> #include <linux/etherdevice.h>
>> #include <linux/platform_device.h>
>> #include <linux/of.h>
>> @@ -574,21 +575,55 @@ static int bcm_sysport_set_wol(struct net_device
>> *dev,
>> return 0;
>> }
>> +static void bcm_sysport_set_rx_coalesce(struct bcm_sysport_priv *priv)
>> +{
>> + u32 reg;
>> +
>> + reg = rdma_readl(priv, RDMA_MBDONE_INTR);
>> + reg &= ~(RDMA_INTR_THRESH_MASK |
>> + RDMA_TIMEOUT_MASK << RDMA_TIMEOUT_SHIFT);
>> + reg |= priv->dim.coal_pkts;
>> + reg |= DIV_ROUND_UP(priv->dim.coal_usecs * 1000, 8192) <<
>> + RDMA_TIMEOUT_SHIFT;
>> + rdma_writel(priv, reg, RDMA_MBDONE_INTR);
>> +}
>> +
>> +static void bcm_sysport_set_tx_coalesce(struct bcm_sysport_tx_ring
>> *ring)
>> +{
>> + struct bcm_sysport_priv *priv = ring->priv;
>> + u32 reg;
>> +
>> + reg = tdma_readl(priv, TDMA_DESC_RING_INTR_CONTROL(ring->index));
>> + reg &= ~(RING_INTR_THRESH_MASK |
>> + RING_TIMEOUT_MASK << RING_TIMEOUT_SHIFT);
>> + reg |= ring->dim.coal_pkts;
>> + reg |= DIV_ROUND_UP(ring->dim.coal_usecs * 1000, 8192) <<
>> + RING_TIMEOUT_SHIFT;
>> + tdma_writel(priv, reg, TDMA_DESC_RING_INTR_CONTROL(ring->index));
>> +}
>> +
>
> I wouldn't couple these functions with dim. This implies dim is always
> used. IMO, would be more clear to use a generic method which takes usecs
> and packets as an argument.
I did not want to create an additional structure for storing coalescing
parameters, but if you prefer I make this function take two parameters,
that sounds entirely reasonable.
>
>> static int bcm_sysport_get_coalesce(struct net_device *dev,
>> struct ethtool_coalesce *ec)
>> {
>> struct bcm_sysport_priv *priv = netdev_priv(dev);
>> + struct bcm_sysport_tx_ring *ring;
>> + unsigned int i;
>> u32 reg;
>> reg = tdma_readl(priv, TDMA_DESC_RING_INTR_CONTROL(0));
>> ec->tx_coalesce_usecs = (reg >> RING_TIMEOUT_SHIFT) * 8192 /
>> 1000;
>> ec->tx_max_coalesced_frames = reg & RING_INTR_THRESH_MASK;
>> + for (i = 0; i < dev->num_tx_queues; i++) {
>> + ring = &priv->tx_rings[i];
>> + ec->use_adaptive_tx_coalesce |= ring->dim.use_dim;
>> + }
>> reg = rdma_readl(priv, RDMA_MBDONE_INTR);
>> ec->rx_coalesce_usecs = (reg >> RDMA_TIMEOUT_SHIFT) * 8192 /
>> 1000;
>> ec->rx_max_coalesced_frames = reg & RDMA_INTR_THRESH_MASK;
>> + ec->use_adaptive_rx_coalesce = priv->dim.use_dim;
>> return 0;
>> }
>> @@ -597,8 +632,8 @@ static int bcm_sysport_set_coalesce(struct
>> net_device *dev,
>> struct ethtool_coalesce *ec)
>> {
>> struct bcm_sysport_priv *priv = netdev_priv(dev);
>> + struct bcm_sysport_tx_ring *ring;
>> unsigned int i;
>> - u32 reg;
>> /* Base system clock is 125Mhz, DMA timeout is this reference
>> clock
>> * divided by 1024, which yield roughly 8.192 us, our maximum
>> value has
>> @@ -615,22 +650,26 @@ static int bcm_sysport_set_coalesce(struct
>> net_device *dev,
>> return -EINVAL;
>> for (i = 0; i < dev->num_tx_queues; i++) {
>> - reg = tdma_readl(priv, TDMA_DESC_RING_INTR_CONTROL(i));
>> - reg &= ~(RING_INTR_THRESH_MASK |
>> - RING_TIMEOUT_MASK << RING_TIMEOUT_SHIFT);
>> - reg |= ec->tx_max_coalesced_frames;
>> - reg |= DIV_ROUND_UP(ec->tx_coalesce_usecs * 1000, 8192) <<
>> - RING_TIMEOUT_SHIFT;
>> - tdma_writel(priv, reg, TDMA_DESC_RING_INTR_CONTROL(i));
>> + ring = &priv->tx_rings[i];
>> + ring->dim.coal_pkts = ec->tx_max_coalesced_frames;
>> + ring->dim.coal_usecs = ec->tx_coalesce_usecs;
>> + if (!ec->use_adaptive_tx_coalesce && ring->dim.use_dim) {
>> + ring->dim.coal_pkts = 1;
>> + ring->dim.coal_usecs = 0;
>> + }
>> + ring->dim.use_dim = ec->use_adaptive_tx_coalesce;
>> + bcm_sysport_set_tx_coalesce(ring);
>> }
>
> If I understand correctly, if I disable dim, moderation is set to
> {usecs,packets}={0,1} regardless of the input from ethtool right?
Correct, these are the default coalescing parameters that the driver
sets. As mentioned before, since I am not storing any coalescing
parameters other than these two, there is no copy of what an user might
have previously provided, falling back to the defaults seemed reasonable.
> Doesn't this break the wanted behavior? As mentioned above, I would
> decouple dim from the set_tx/rx_coalesce() function. Also, when dim is
> enabled, why change dim.coal_pkts/usecs? They would just be overwritten
> in the next iteration of net_dim.
Indeed, that is not necessary.
>
>> - reg = rdma_readl(priv, RDMA_MBDONE_INTR);
>> - reg &= ~(RDMA_INTR_THRESH_MASK |
>> - RDMA_TIMEOUT_MASK << RDMA_TIMEOUT_SHIFT);
>> - reg |= ec->rx_max_coalesced_frames;
>> - reg |= DIV_ROUND_UP(ec->rx_coalesce_usecs * 1000, 8192) <<
>> - RDMA_TIMEOUT_SHIFT;
>> - rdma_writel(priv, reg, RDMA_MBDONE_INTR);
>> + priv->dim.coal_usecs = ec->rx_coalesce_usecs;
>> + priv->dim.coal_pkts = ec->rx_max_coalesced_frames;
>> +
>> + if (!ec->use_adaptive_rx_coalesce && priv->dim.use_dim) {
>> + priv->dim.coal_pkts = 1;
>> + priv->dim.coal_usecs = 0;
>> + }
>> + priv->dim.use_dim = ec->use_adaptive_rx_coalesce;
>> + bcm_sysport_set_rx_coalesce(priv);
>
> Same comment as above.
>
>> return 0;
>> }
>> @@ -709,6 +748,7 @@ static unsigned int bcm_sysport_desc_rx(struct
>> bcm_sysport_priv *priv,
>> struct bcm_sysport_stats64 *stats64 = &priv->stats64;
>> struct net_device *ndev = priv->netdev;
>> unsigned int processed = 0, to_process;
>> + unsigned int processed_bytes = 0;
>> struct bcm_sysport_cb *cb;
>> struct sk_buff *skb;
>> unsigned int p_index;
>> @@ -800,6 +840,7 @@ static unsigned int bcm_sysport_desc_rx(struct
>> bcm_sysport_priv *priv,
>> */
>> skb_pull(skb, sizeof(*rsb) + 2);
>> len -= (sizeof(*rsb) + 2);
>> + processed_bytes += len;
>> /* UniMAC may forward CRC */
>> if (priv->crc_fwd) {
>> @@ -824,6 +865,9 @@ static unsigned int bcm_sysport_desc_rx(struct
>> bcm_sysport_priv *priv,
>> priv->rx_read_ptr = 0;
>> }
>> + priv->dim.packets = processed;
>> + priv->dim.bytes = processed_bytes;
>> +
>> return processed;
>> }
>> @@ -900,6 +944,8 @@ static unsigned int
>> __bcm_sysport_tx_reclaim(struct bcm_sysport_priv *priv,
>> ring->packets += pkts_compl;
>> ring->bytes += bytes_compl;
>> u64_stats_update_end(&priv->syncp);
>> + ring->dim.packets = pkts_compl;
>> + ring->dim.bytes = bytes_compl;
>> ring->c_index = c_index;
>> @@ -945,6 +991,7 @@ static int bcm_sysport_tx_poll(struct
>> napi_struct *napi, int budget)
>> {
>> struct bcm_sysport_tx_ring *ring =
>> container_of(napi, struct bcm_sysport_tx_ring, napi);
>> + struct net_dim_sample dim_sample;
>> unsigned int work_done = 0;
>> work_done = bcm_sysport_tx_reclaim(ring->priv, ring);
>> @@ -961,6 +1008,12 @@ static int bcm_sysport_tx_poll(struct
>> napi_struct *napi, int budget)
>> return 0;
>> }
>> + if (ring->dim.use_dim) {
>> + net_dim_sample(ring->dim.event_ctr, ring->dim.packets,
>> + ring->dim.bytes, &dim_sample);
>> + net_dim(&ring->dim.dim, dim_sample);
>> + }
>> +
>> return budget;
>> }
>> @@ -976,6 +1029,7 @@ static int bcm_sysport_poll(struct napi_struct
>> *napi, int budget)
>> {
>> struct bcm_sysport_priv *priv =
>> container_of(napi, struct bcm_sysport_priv, napi);
>> + struct net_dim_sample dim_sample;
>> unsigned int work_done = 0;
>> work_done = bcm_sysport_desc_rx(priv, budget);
>> @@ -998,6 +1052,12 @@ static int bcm_sysport_poll(struct napi_struct
>> *napi, int budget)
>> intrl2_0_mask_clear(priv, INTRL2_0_RDMA_MBDONE);
>> }
>> + if (priv->dim.use_dim) {
>> + net_dim_sample(priv->dim.event_ctr, priv->dim.packets,
>> + priv->dim.bytes, &dim_sample);
>> + net_dim(&priv->dim.dim, dim_sample);
>> + }
>> +
>> return work_done;
>> }
>> @@ -1016,6 +1076,40 @@ static void
>> bcm_sysport_resume_from_wol(struct bcm_sysport_priv *priv)
>> netif_dbg(priv, wol, priv->netdev, "resumed from WOL\n");
>> }
>> +static void bcm_sysport_dim_work(struct work_struct *work)
>> +{
>> + struct net_dim *dim = container_of(work, struct net_dim, work);
>> + struct bcm_sysport_net_dim *ndim =
>> + container_of(dim, struct bcm_sysport_net_dim, dim);
>> + struct bcm_sysport_priv *priv =
>> + container_of(ndim, struct bcm_sysport_priv, dim);
>> + struct net_dim_cq_moder cur_profile =
>> + net_dim_get_profile(dim->mode, dim->profile_ix);
>> +
>> + priv->dim.coal_usecs = cur_profile.usec;
>> + priv->dim.coal_pkts = cur_profile.pkts;
>> +
>> + bcm_sysport_set_rx_coalesce(priv);
>> + dim->state = NET_DIM_START_MEASURE;
>> +}
>> +
>> +static void bcm_sysport_dim_tx_work(struct work_struct *work)
>> +{
>> + struct net_dim *dim = container_of(work, struct net_dim, work);
>> + struct bcm_sysport_net_dim *ndim =
>> + container_of(dim, struct bcm_sysport_net_dim, dim);
>> + struct bcm_sysport_tx_ring *ring =
>> + container_of(ndim, struct bcm_sysport_tx_ring, dim);
>> + struct net_dim_cq_moder cur_profile =
>> + net_dim_get_profile(dim->mode, dim->profile_ix);
>> +
>> + ring->dim.coal_usecs = cur_profile.usec;
>> + ring->dim.coal_pkts = cur_profile.pkts;
>> +
>> + bcm_sysport_set_tx_coalesce(ring);
>> + dim->state = NET_DIM_START_MEASURE;
>> +}
>> +
>> /* RX and misc interrupt routine */
>> static irqreturn_t bcm_sysport_rx_isr(int irq, void *dev_id)
>> {
>> @@ -1034,6 +1128,7 @@ static irqreturn_t bcm_sysport_rx_isr(int irq,
>> void *dev_id)
>> }
>> if (priv->irq0_stat & INTRL2_0_RDMA_MBDONE) {
>> + priv->dim.event_ctr++;
>> if (likely(napi_schedule_prep(&priv->napi))) {
>> /* disable RX interrupts */
>> intrl2_0_mask_set(priv, INTRL2_0_RDMA_MBDONE);
>> @@ -1061,6 +1156,7 @@ static irqreturn_t bcm_sysport_rx_isr(int irq,
>> void *dev_id)
>> continue;
>> txr = &priv->tx_rings[ring];
>> + txr->dim.event_ctr++;
>> if (likely(napi_schedule_prep(&txr->napi))) {
>> intrl2_0_mask_set(priv, ring_bit);
>> @@ -1093,6 +1189,7 @@ static irqreturn_t bcm_sysport_tx_isr(int irq,
>> void *dev_id)
>> continue;
>> txr = &priv->tx_rings[ring];
>> + txr->dim.event_ctr++;
>> if (likely(napi_schedule_prep(&txr->napi))) {
>> intrl2_1_mask_set(priv, BIT(ring));
>> @@ -1358,6 +1455,16 @@ static void bcm_sysport_adj_link(struct
>> net_device *dev)
>> phy_print_status(phydev);
>> }
>> +static void bcm_sysport_init_dim(struct bcm_sysport_net_dim *dim,
>> + void (*cb)(struct work_struct *work))
>> +{
>> + INIT_WORK(&dim->dim.work, cb);
>> + dim->dim.mode = NET_DIM_CQ_PERIOD_MODE_START_FROM_EQE;
>> + dim->event_ctr = 0;
>> + dim->packets = 0;
>> + dim->bytes = 0;
>> +}
>
> What about default values for coal_usecs/pkts? dim supports it through
> net_dim_get_def_profile(mode) function.
OK, thanks I did not know that.
--
Florian
^ permalink raw reply
* [net-next 01/10] i40e: move I40E_FLAG_FILTER_SYNC to a state bit
From: Jeff Kirsher @ 2018-03-26 21:40 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20180326214103.18218-1-jeffrey.t.kirsher@intel.com>
From: Jacob Keller <jacob.e.keller@intel.com>
The I40E_FLAG_FILTER_SYNC flag is modified during run time possibly when
the RTNL lock is not held. Thus, it should not be part of pf->flags, and
instead should be using atomic bit operations in the pf->state field.
Create a __I40E_MACVLAN_SYNC_PENDING state bit, and use it instead of
the old I40E_FLAG_FILTER_SYNC flag.
This is part of a larger effort to remove the need for cmpxchg64 in
i40e_set_priv_flags().
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e.h | 3 ++-
drivers/net/ethernet/intel/i40e/i40e_main.c | 16 +++++++++-------
2 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index 1d33a8b3ef54..52f99142244b 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -162,6 +162,7 @@ enum i40e_state_t {
__I40E_RESET_FAILED,
__I40E_PORT_SUSPENDED,
__I40E_VF_DISABLE,
+ __I40E_MACVLAN_SYNC_PENDING,
/* This must be last as it determines the size of the BITMAP */
__I40E_STATE_SIZE__,
};
@@ -516,7 +517,7 @@ struct i40e_pf {
#define I40E_FLAG_MSIX_ENABLED BIT_ULL(2)
#define I40E_FLAG_RSS_ENABLED BIT_ULL(3)
#define I40E_FLAG_VMDQ_ENABLED BIT_ULL(4)
-#define I40E_FLAG_FILTER_SYNC BIT_ULL(5)
+/* Gap for BIT_ULL(5) */
#define I40E_FLAG_SRIOV_ENABLED BIT_ULL(6)
#define I40E_FLAG_DCB_CAPABLE BIT_ULL(7)
#define I40E_FLAG_DCB_ENABLED BIT_ULL(8)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 536ed8e8a96f..7fc2c6d89637 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -1382,7 +1382,7 @@ struct i40e_mac_filter *i40e_add_filter(struct i40e_vsi *vsi,
hash_add(vsi->mac_filter_hash, &f->hlist, key);
vsi->flags |= I40E_VSI_FLAG_FILTER_CHANGED;
- vsi->back->flags |= I40E_FLAG_FILTER_SYNC;
+ set_bit(__I40E_MACVLAN_SYNC_PENDING, vsi->back->state);
}
/* If we're asked to add a filter that has been marked for removal, it
@@ -1432,7 +1432,7 @@ void __i40e_del_filter(struct i40e_vsi *vsi, struct i40e_mac_filter *f)
}
vsi->flags |= I40E_VSI_FLAG_FILTER_CHANGED;
- vsi->back->flags |= I40E_FLAG_FILTER_SYNC;
+ set_bit(__I40E_MACVLAN_SYNC_PENDING, vsi->state);
}
/**
@@ -1955,7 +1955,7 @@ static void i40e_set_rx_mode(struct net_device *netdev)
/* check for other flag changes */
if (vsi->current_netdev_flags != vsi->netdev->flags) {
vsi->flags |= I40E_VSI_FLAG_FILTER_CHANGED;
- vsi->back->flags |= I40E_FLAG_FILTER_SYNC;
+ set_bit(__I40E_MACVLAN_SYNC_PENDING, vsi->back->state);
}
}
@@ -2577,9 +2577,10 @@ static void i40e_sync_filters_subtask(struct i40e_pf *pf)
{
int v;
- if (!pf || !(pf->flags & I40E_FLAG_FILTER_SYNC))
+ if (!pf)
+ return;
+ if (!test_and_clear_bit(__I40E_MACVLAN_SYNC_PENDING, pf->state))
return;
- pf->flags &= ~I40E_FLAG_FILTER_SYNC;
for (v = 0; v < pf->num_alloc_vsi; v++) {
if (pf->vsi[v] &&
@@ -2588,7 +2589,8 @@ static void i40e_sync_filters_subtask(struct i40e_pf *pf)
if (ret) {
/* come back and try again later */
- pf->flags |= I40E_FLAG_FILTER_SYNC;
+ set_bit(__I40E_MACVLAN_SYNC_PENDING,
+ pf->state);
break;
}
}
@@ -12240,7 +12242,7 @@ static int i40e_add_vsi(struct i40e_vsi *vsi)
if (f_count) {
vsi->flags |= I40E_VSI_FLAG_FILTER_CHANGED;
- pf->flags |= I40E_FLAG_FILTER_SYNC;
+ set_bit(__I40E_MACVLAN_SYNC_PENDING, pf->state);
}
/* Update VSI BW information */
--
2.14.3
^ permalink raw reply related
* [net-next 00/10][pull request] 40GbE Intel Wired LAN Driver Updates 2018-03-26
From: Jeff Kirsher @ 2018-03-26 21:40 UTC (permalink / raw)
To: davem; +Cc: Jeff Kirsher, netdev, nhorman, sassmann, jogreene
This series contains updates to i40e only.
Jake provides several patches which remove the need for cmpxchg64(),
starting with moving I40E_FLAG_[UDP]_FILTER_SYNC from pf->flags to pf->state
since they are modified during run time possibly when the RTNL lock is not
held so they should be a state bits and not flags. Moved additional
"flags" which should be state fields, into pf->state. Ensure we hold
the RTNL lock for the entire sequence of preparing for reset and when
resuming, which will protect the flags related to interrupt scheme under
RTNL lock so that their modification is properly threaded. Finally,
cleanup the use of cmpxchg64() since it is no longer needed. Cleaned up
the holes in the feature flags created my moving some flags to the state
field.
Björn Töpel adds XDP_REDIRECT support as well as tweaking the page
counting for XDP_REDIRECT so that it will function properly.
The following are changes since commit 996bfed118748c128ad4b6c05c09fd2f5fdfa1b4:
Merge tag 'wireless-drivers-next-for-davem-2018-03-24' of git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 40GbE
Björn Töpel (2):
i40e: tweak page counting for XDP_REDIRECT
i40e: add support for XDP_REDIRECT
Jacob Keller (8):
i40e: move I40E_FLAG_FILTER_SYNC to a state bit
i40e: move I40E_FLAG_UDP_FILTER_SYNC to the state field
i40e: move AUTO_DISABLED flags into the state field
i40e: move I40E_FLAG_TEMP_LINK_POLLING to state field
i40e: move client flags into state bits
i40e: hold the RTNL lock while changing interrupt schemes
i40e: stop using cmpxchg flow in i40e_set_priv_flags()
i40e: re-number feature flags to remove gaps
drivers/net/ethernet/intel/i40e/i40e.h | 68 ++++++++--------
drivers/net/ethernet/intel/i40e/i40e_client.c | 7 +-
drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 23 ++----
drivers/net/ethernet/intel/i40e/i40e_main.c | 99 ++++++++++++-----------
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 104 +++++++++++++++++++------
drivers/net/ethernet/intel/i40e/i40e_txrx.h | 2 +
6 files changed, 181 insertions(+), 122 deletions(-)
--
2.14.3
^ permalink raw reply
* [net-next 04/10] i40e: move I40E_FLAG_TEMP_LINK_POLLING to state field
From: Jeff Kirsher @ 2018-03-26 21:40 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20180326214103.18218-1-jeffrey.t.kirsher@intel.com>
From: Jacob Keller <jacob.e.keller@intel.com>
This flag is modified outside of the RTNL lock and thus should not be
part of the pf->flags variable.
Use a state bit instead, so that we can use atomic bit operations.
This is part of a larger effort to remove cmpxchg64 in
i40e_set_priv_flags()
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e.h | 3 ++-
drivers/net/ethernet/intel/i40e/i40e_main.c | 7 +++----
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index 38633563dacd..1ffe802d489f 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -166,6 +166,7 @@ enum i40e_state_t {
__I40E_VF_DISABLE,
__I40E_MACVLAN_SYNC_PENDING,
__I40E_UDP_FILTER_SYNC_PENDING,
+ __I40E_TEMP_LINK_POLLING,
/* This must be last as it determines the size of the BITMAP */
__I40E_STATE_SIZE__,
};
@@ -534,7 +535,7 @@ struct i40e_pf {
#define I40E_FLAG_VEB_STATS_ENABLED BIT_ULL(17)
#define I40E_FLAG_LINK_POLLING_ENABLED BIT_ULL(18)
#define I40E_FLAG_TRUE_PROMISC_SUPPORT BIT_ULL(19)
-#define I40E_FLAG_TEMP_LINK_POLLING BIT_ULL(20)
+/* Gap for BIT_ULL(20) */
#define I40E_FLAG_LEGACY_RX BIT_ULL(21)
#define I40E_FLAG_PTP BIT_ULL(22)
#define I40E_FLAG_IWARP_ENABLED BIT_ULL(23)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index a478153818bc..307f5bf65708 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -8432,13 +8432,12 @@ static void i40e_link_event(struct i40e_pf *pf)
/* On success, disable temp link polling */
if (status == I40E_SUCCESS) {
- if (pf->flags & I40E_FLAG_TEMP_LINK_POLLING)
- pf->flags &= ~I40E_FLAG_TEMP_LINK_POLLING;
+ clear_bit(__I40E_TEMP_LINK_POLLING, pf->state);
} else {
/* Enable link polling temporarily until i40e_get_link_status
* returns I40E_SUCCESS
*/
- pf->flags |= I40E_FLAG_TEMP_LINK_POLLING;
+ set_bit(__I40E_TEMP_LINK_POLLING, pf->state);
dev_dbg(&pf->pdev->dev, "couldn't get link state, status: %d\n",
status);
return;
@@ -8490,7 +8489,7 @@ static void i40e_watchdog_subtask(struct i40e_pf *pf)
pf->service_timer_previous = jiffies;
if ((pf->flags & I40E_FLAG_LINK_POLLING_ENABLED) ||
- (pf->flags & I40E_FLAG_TEMP_LINK_POLLING))
+ test_bit(__I40E_TEMP_LINK_POLLING, pf->state))
i40e_link_event(pf);
/* Update the stats for active netdevs so the network stack
--
2.14.3
^ permalink raw reply related
* [net-next 03/10] i40e: move AUTO_DISABLED flags into the state field
From: Jeff Kirsher @ 2018-03-26 21:40 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20180326214103.18218-1-jeffrey.t.kirsher@intel.com>
From: Jacob Keller <jacob.e.keller@intel.com>
The two Flow Directory auto disable flags are used at run time to mark
when the flow director features needed to be disabled. Thus the flags
could change even when the RTNL lock is not held.
They also have some code constructions which really should be
test_and_set or test_and_clear using atomic bit operations.
Create new state fields to mark this, and stop including them in
pf->flags.
This is part of a larger effort to remove the need for cmpxchg64 in
i40e_set_priv_flags().
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e.h | 5 +++--
drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 4 ++--
drivers/net/ethernet/intel/i40e/i40e_main.c | 23 +++++++++--------------
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 21 +++++++++++++--------
4 files changed, 27 insertions(+), 26 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index f557c3c36080..38633563dacd 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -159,6 +159,8 @@ enum i40e_state_t {
__I40E_BAD_EEPROM,
__I40E_DOWN_REQUESTED,
__I40E_FD_FLUSH_REQUESTED,
+ __I40E_FD_ATR_AUTO_DISABLED,
+ __I40E_FD_SB_AUTO_DISABLED,
__I40E_RESET_FAILED,
__I40E_PORT_SUSPENDED,
__I40E_VF_DISABLE,
@@ -524,8 +526,7 @@ struct i40e_pf {
#define I40E_FLAG_DCB_ENABLED BIT_ULL(8)
#define I40E_FLAG_FD_SB_ENABLED BIT_ULL(9)
#define I40E_FLAG_FD_ATR_ENABLED BIT_ULL(10)
-#define I40E_FLAG_FD_SB_AUTO_DISABLED BIT_ULL(11)
-#define I40E_FLAG_FD_ATR_AUTO_DISABLED BIT_ULL(12)
+/* Gap for BIT_ULL(11) and BIT_ULL(12) */
#define I40E_FLAG_MFP_ENABLED BIT_ULL(13)
/* Gap for BIT_ULL(14) */
#define I40E_FLAG_HW_ATR_EVICT_ENABLED BIT_ULL(15)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 846a9d597e01..2a9c93091e3d 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -3951,7 +3951,7 @@ static int i40e_add_fdir_ethtool(struct i40e_vsi *vsi,
if (!(pf->flags & I40E_FLAG_FD_SB_ENABLED))
return -EOPNOTSUPP;
- if (pf->flags & I40E_FLAG_FD_SB_AUTO_DISABLED)
+ if (test_bit(__I40E_FD_SB_AUTO_DISABLED, pf->state))
return -ENOSPC;
if (test_bit(__I40E_RESET_RECOVERY_PENDING, pf->state) ||
@@ -4460,7 +4460,7 @@ static int i40e_set_priv_flags(struct net_device *dev, u32 flags)
/* Flush current ATR settings if ATR was disabled */
if ((changed_flags & I40E_FLAG_FD_ATR_ENABLED) &&
!(pf->flags & I40E_FLAG_FD_ATR_ENABLED)) {
- pf->flags |= I40E_FLAG_FD_ATR_AUTO_DISABLED;
+ set_bit(__I40E_FD_ATR_AUTO_DISABLED, pf->state);
set_bit(__I40E_FD_FLUSH_REQUESTED, pf->state);
}
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 3177e82059e6..a478153818bc 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -1083,13 +1083,13 @@ static void i40e_update_pf_stats(struct i40e_pf *pf)
&osd->rx_lpi_count, &nsd->rx_lpi_count);
if (pf->flags & I40E_FLAG_FD_SB_ENABLED &&
- !(pf->flags & I40E_FLAG_FD_SB_AUTO_DISABLED))
+ !test_bit(__I40E_FD_SB_AUTO_DISABLED, pf->state))
nsd->fd_sb_status = true;
else
nsd->fd_sb_status = false;
if (pf->flags & I40E_FLAG_FD_ATR_ENABLED &&
- !(pf->flags & I40E_FLAG_FD_ATR_AUTO_DISABLED))
+ !test_bit(__I40E_FD_ATR_AUTO_DISABLED, pf->state))
nsd->fd_atr_status = true;
else
nsd->fd_atr_status = false;
@@ -8144,12 +8144,10 @@ u32 i40e_get_global_fd_count(struct i40e_pf *pf)
**/
static void i40e_reenable_fdir_sb(struct i40e_pf *pf)
{
- if (pf->flags & I40E_FLAG_FD_SB_AUTO_DISABLED) {
- pf->flags &= ~I40E_FLAG_FD_SB_AUTO_DISABLED;
+ if (test_and_clear_bit(__I40E_FD_SB_AUTO_DISABLED, pf->state))
if ((pf->flags & I40E_FLAG_FD_SB_ENABLED) &&
(I40E_DEBUG_FD & pf->hw.debug_mask))
dev_info(&pf->pdev->dev, "FD Sideband/ntuple is being enabled since we have space in the table now\n");
- }
}
/**
@@ -8158,7 +8156,7 @@ static void i40e_reenable_fdir_sb(struct i40e_pf *pf)
**/
static void i40e_reenable_fdir_atr(struct i40e_pf *pf)
{
- if (pf->flags & I40E_FLAG_FD_ATR_AUTO_DISABLED) {
+ if (test_and_clear_bit(__I40E_FD_ATR_AUTO_DISABLED, pf->state)) {
/* ATR uses the same filtering logic as SB rules. It only
* functions properly if the input set mask is at the default
* settings. It is safe to restore the default input set
@@ -8168,7 +8166,6 @@ static void i40e_reenable_fdir_atr(struct i40e_pf *pf)
I40E_L3_SRC_MASK | I40E_L3_DST_MASK |
I40E_L4_SRC_MASK | I40E_L4_DST_MASK);
- pf->flags &= ~I40E_FLAG_FD_ATR_AUTO_DISABLED;
if ((pf->flags & I40E_FLAG_FD_ATR_ENABLED) &&
(I40E_DEBUG_FD & pf->hw.debug_mask))
dev_info(&pf->pdev->dev, "ATR is being enabled since we have space in the table and there are no conflicting ntuple rules\n");
@@ -8291,7 +8288,7 @@ static void i40e_fdir_flush_and_replay(struct i40e_pf *pf)
}
pf->fd_flush_timestamp = jiffies;
- pf->flags |= I40E_FLAG_FD_ATR_AUTO_DISABLED;
+ set_bit(__I40E_FD_ATR_AUTO_DISABLED, pf->state);
/* flush all filters */
wr32(&pf->hw, I40E_PFQF_CTL_1,
I40E_PFQF_CTL_1_CLEARFDTABLE_MASK);
@@ -8311,7 +8308,7 @@ static void i40e_fdir_flush_and_replay(struct i40e_pf *pf)
/* replay sideband filters */
i40e_fdir_filter_restore(pf->vsi[pf->lan_vsi]);
if (!disable_atr && !pf->fd_tcp4_filter_cnt)
- pf->flags &= ~I40E_FLAG_FD_ATR_AUTO_DISABLED;
+ clear_bit(__I40E_FD_ATR_AUTO_DISABLED, pf->state);
clear_bit(__I40E_FD_FLUSH_REQUESTED, pf->state);
if (I40E_DEBUG_FD & pf->hw.debug_mask)
dev_info(&pf->pdev->dev, "FD Filter table flushed and FD-SB replayed.\n");
@@ -11291,20 +11288,18 @@ bool i40e_set_ntuple(struct i40e_pf *pf, netdev_features_t features)
need_reset = true;
i40e_fdir_filter_exit(pf);
}
- pf->flags &= ~(I40E_FLAG_FD_SB_ENABLED |
- I40E_FLAG_FD_SB_AUTO_DISABLED);
+ pf->flags &= ~I40E_FLAG_FD_SB_ENABLED;
+ clear_bit(__I40E_FD_SB_AUTO_DISABLED, pf->state);
pf->flags |= I40E_FLAG_FD_SB_INACTIVE;
/* reset fd counters */
pf->fd_add_err = 0;
pf->fd_atr_cnt = 0;
/* if ATR was auto disabled it can be re-enabled. */
- if (pf->flags & I40E_FLAG_FD_ATR_AUTO_DISABLED) {
- pf->flags &= ~I40E_FLAG_FD_ATR_AUTO_DISABLED;
+ if (test_and_clear_bit(__I40E_FD_ATR_AUTO_DISABLED, pf->state))
if ((pf->flags & I40E_FLAG_FD_ATR_ENABLED) &&
(I40E_DEBUG_FD & pf->hw.debug_mask))
dev_info(&pf->pdev->dev, "ATR re-enabled.\n");
- }
}
return need_reset;
}
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 7ccd05bf4b06..797bcdd3504e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -336,7 +336,7 @@ static int i40e_add_del_fdir_tcpv4(struct i40e_vsi *vsi,
if ((pf->flags & I40E_FLAG_FD_ATR_ENABLED) &&
I40E_DEBUG_FD & pf->hw.debug_mask)
dev_info(&pf->pdev->dev, "Forcing ATR off, sideband rules for TCP/IPv4 flow being applied\n");
- pf->flags |= I40E_FLAG_FD_ATR_AUTO_DISABLED;
+ set_bit(__I40E_FD_ATR_AUTO_DISABLED, pf->state);
} else {
pf->fd_tcp4_filter_cnt--;
}
@@ -594,8 +594,14 @@ static void i40e_fd_handle_status(struct i40e_ring *rx_ring,
pf->fd_atr_cnt = i40e_get_current_atr_cnt(pf);
if ((rx_desc->wb.qword0.hi_dword.fd_id == 0) &&
- pf->flags & I40E_FLAG_FD_SB_AUTO_DISABLED) {
- pf->flags |= I40E_FLAG_FD_ATR_AUTO_DISABLED;
+ test_bit(__I40E_FD_SB_AUTO_DISABLED, pf->state)) {
+ /* These set_bit() calls aren't atomic with the
+ * test_bit() here, but worse case we potentially
+ * disable ATR and queue a flush right after SB
+ * support is re-enabled. That shouldn't cause an
+ * issue in practice
+ */
+ set_bit(__I40E_FD_ATR_AUTO_DISABLED, pf->state);
set_bit(__I40E_FD_FLUSH_REQUESTED, pf->state);
}
@@ -608,11 +614,10 @@ static void i40e_fd_handle_status(struct i40e_ring *rx_ring,
*/
if (fcnt_prog >= (fcnt_avail - I40E_FDIR_BUFFER_FULL_MARGIN)) {
if ((pf->flags & I40E_FLAG_FD_SB_ENABLED) &&
- !(pf->flags & I40E_FLAG_FD_SB_AUTO_DISABLED)) {
- pf->flags |= I40E_FLAG_FD_SB_AUTO_DISABLED;
+ !test_and_set_bit(__I40E_FD_SB_AUTO_DISABLED,
+ pf->state))
if (I40E_DEBUG_FD & pf->hw.debug_mask)
dev_warn(&pdev->dev, "FD filter space full, new ntuple rules will not be added\n");
- }
}
} else if (error == BIT(I40E_RX_PROG_STATUS_DESC_NO_FD_ENTRY_SHIFT)) {
if (I40E_DEBUG_FD & pf->hw.debug_mask)
@@ -2647,7 +2652,7 @@ static void i40e_atr(struct i40e_ring *tx_ring, struct sk_buff *skb,
if (!(pf->flags & I40E_FLAG_FD_ATR_ENABLED))
return;
- if (pf->flags & I40E_FLAG_FD_ATR_AUTO_DISABLED)
+ if (test_bit(__I40E_FD_ATR_AUTO_DISABLED, pf->state))
return;
/* if sampling is disabled do nothing */
@@ -2687,7 +2692,7 @@ static void i40e_atr(struct i40e_ring *tx_ring, struct sk_buff *skb,
th = (struct tcphdr *)(hdr.network + hlen);
/* Due to lack of space, no more new filters can be programmed */
- if (th->syn && (pf->flags & I40E_FLAG_FD_ATR_AUTO_DISABLED))
+ if (th->syn && test_bit(__I40E_FD_ATR_AUTO_DISABLED, pf->state))
return;
if (pf->flags & I40E_FLAG_HW_ATR_EVICT_ENABLED) {
/* HW ATR eviction will take care of removing filters on FIN
--
2.14.3
^ permalink raw reply related
* [net-next 02/10] i40e: move I40E_FLAG_UDP_FILTER_SYNC to the state field
From: Jeff Kirsher @ 2018-03-26 21:40 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20180326214103.18218-1-jeffrey.t.kirsher@intel.com>
From: Jacob Keller <jacob.e.keller@intel.com>
This flag is modified during run time, possibly even when the RTNL lock
is not held. Additionally it has a few places which should be using
test_and_set or test_and_clear atomic bit operations.
Create a new state bit __I40E_UDP_SYNC_PENDING and use it instead of the
ole I40E_FLAG_UDP_FILTER_SYNC flag.
This is part of a larger effort to remove the need for using cmpxchg64
in i40e_set_priv_flags.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e.h | 3 ++-
drivers/net/ethernet/intel/i40e/i40e_main.c | 10 ++++------
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index 52f99142244b..f557c3c36080 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -163,6 +163,7 @@ enum i40e_state_t {
__I40E_PORT_SUSPENDED,
__I40E_VF_DISABLE,
__I40E_MACVLAN_SYNC_PENDING,
+ __I40E_UDP_FILTER_SYNC_PENDING,
/* This must be last as it determines the size of the BITMAP */
__I40E_STATE_SIZE__,
};
@@ -526,7 +527,7 @@ struct i40e_pf {
#define I40E_FLAG_FD_SB_AUTO_DISABLED BIT_ULL(11)
#define I40E_FLAG_FD_ATR_AUTO_DISABLED BIT_ULL(12)
#define I40E_FLAG_MFP_ENABLED BIT_ULL(13)
-#define I40E_FLAG_UDP_FILTER_SYNC BIT_ULL(14)
+/* Gap for BIT_ULL(14) */
#define I40E_FLAG_HW_ATR_EVICT_ENABLED BIT_ULL(15)
#define I40E_FLAG_VEB_MODE_ENABLED BIT_ULL(16)
#define I40E_FLAG_VEB_STATS_ENABLED BIT_ULL(17)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 7fc2c6d89637..3177e82059e6 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -9721,7 +9721,7 @@ static void i40e_sync_udp_filters(struct i40e_pf *pf)
pf->pending_udp_bitmap |= BIT_ULL(i);
}
- pf->flags |= I40E_FLAG_UDP_FILTER_SYNC;
+ set_bit(__I40E_UDP_FILTER_SYNC_PENDING, pf->state);
}
/**
@@ -9735,11 +9735,9 @@ static void i40e_sync_udp_filters_subtask(struct i40e_pf *pf)
u16 port;
int i;
- if (!(pf->flags & I40E_FLAG_UDP_FILTER_SYNC))
+ if (!test_and_clear_bit(__I40E_UDP_FILTER_SYNC_PENDING, pf->state))
return;
- pf->flags &= ~I40E_FLAG_UDP_FILTER_SYNC;
-
for (i = 0; i < I40E_MAX_PF_UDP_OFFLOAD_PORTS; i++) {
if (pf->pending_udp_bitmap & BIT_ULL(i)) {
pf->pending_udp_bitmap &= ~BIT_ULL(i);
@@ -11439,7 +11437,7 @@ static void i40e_udp_tunnel_add(struct net_device *netdev,
/* New port: add it and mark its index in the bitmap */
pf->udp_ports[next_idx].port = port;
pf->pending_udp_bitmap |= BIT_ULL(next_idx);
- pf->flags |= I40E_FLAG_UDP_FILTER_SYNC;
+ set_bit(__I40E_UDP_FILTER_SYNC_PENDING, pf->state);
}
/**
@@ -11480,7 +11478,7 @@ static void i40e_udp_tunnel_del(struct net_device *netdev,
*/
pf->udp_ports[idx].port = 0;
pf->pending_udp_bitmap |= BIT_ULL(idx);
- pf->flags |= I40E_FLAG_UDP_FILTER_SYNC;
+ set_bit(__I40E_UDP_FILTER_SYNC_PENDING, pf->state);
return;
not_found:
--
2.14.3
^ permalink raw reply related
* [net-next 06/10] i40e: hold the RTNL lock while changing interrupt schemes
From: Jeff Kirsher @ 2018-03-26 21:40 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20180326214103.18218-1-jeffrey.t.kirsher@intel.com>
From: Jacob Keller <jacob.e.keller@intel.com>
When we suspend and resume, we need to clear and re-enable the interrupt
scheme. This was previously not done while holding the RTNL lock, which
could be problematic, because we are actually destroying and re-creating
queues.
Hold the RTNL lock for the entire sequence of preparing for reset, and
when resuming. This additionally protects the flags related to interrupt
scheme under RTNL lock so that their modification is properly threaded.
This is part of a larger effort to remove the need for cmpxchg64 in
i40e_set_priv_flags().
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 68b51761c509..5efd6d7bfa59 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -14348,7 +14348,13 @@ static int __maybe_unused i40e_suspend(struct device *dev)
if (pf->wol_en && (pf->hw_features & I40E_HW_WOL_MC_MAGIC_PKT_WAKE))
i40e_enable_mc_magic_wake(pf);
- i40e_prep_for_reset(pf, false);
+ /* Since we're going to destroy queues during the
+ * i40e_clear_interrupt_scheme() we should hold the RTNL lock for this
+ * whole section
+ */
+ rtnl_lock();
+
+ i40e_prep_for_reset(pf, true);
wr32(hw, I40E_PFPM_APM, (pf->wol_en ? I40E_PFPM_APM_APME_MASK : 0));
wr32(hw, I40E_PFPM_WUFC, (pf->wol_en ? I40E_PFPM_WUFC_MAG_MASK : 0));
@@ -14360,6 +14366,8 @@ static int __maybe_unused i40e_suspend(struct device *dev)
*/
i40e_clear_interrupt_scheme(pf);
+ rtnl_unlock();
+
return 0;
}
@@ -14377,6 +14385,11 @@ static int __maybe_unused i40e_resume(struct device *dev)
if (!test_bit(__I40E_SUSPENDED, pf->state))
return 0;
+ /* We need to hold the RTNL lock prior to restoring interrupt schemes,
+ * since we're going to be restoring queues
+ */
+ rtnl_lock();
+
/* We cleared the interrupt scheme when we suspended, so we need to
* restore it now to resume device functionality.
*/
@@ -14387,7 +14400,9 @@ static int __maybe_unused i40e_resume(struct device *dev)
}
clear_bit(__I40E_DOWN, pf->state);
- i40e_reset_and_rebuild(pf, false, false);
+ i40e_reset_and_rebuild(pf, false, true);
+
+ rtnl_unlock();
/* Clear suspended state last after everything is recovered */
clear_bit(__I40E_SUSPENDED, pf->state);
--
2.14.3
^ permalink raw reply related
* [net-next 05/10] i40e: move client flags into state bits
From: Jeff Kirsher @ 2018-03-26 21:40 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20180326214103.18218-1-jeffrey.t.kirsher@intel.com>
From: Jacob Keller <jacob.e.keller@intel.com>
The iWarp client flags are all potentially changed when the RTNL lock is
not held, so they should not be part of the pf->flags variable. Instead,
move them into the state field so that we can use atomic bit operations.
This is part of a larger effort to remove cmpxchg64 in
i40e_set_priv_flags()
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e.h | 7 ++++---
drivers/net/ethernet/intel/i40e/i40e_client.c | 7 +++----
drivers/net/ethernet/intel/i40e/i40e_main.c | 22 ++++++++++------------
3 files changed, 17 insertions(+), 19 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index 1ffe802d489f..67518013ca4d 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -167,6 +167,9 @@ enum i40e_state_t {
__I40E_MACVLAN_SYNC_PENDING,
__I40E_UDP_FILTER_SYNC_PENDING,
__I40E_TEMP_LINK_POLLING,
+ __I40E_CLIENT_SERVICE_REQUESTED,
+ __I40E_CLIENT_L2_CHANGE,
+ __I40E_CLIENT_RESET,
/* This must be last as it determines the size of the BITMAP */
__I40E_STATE_SIZE__,
};
@@ -539,9 +542,7 @@ struct i40e_pf {
#define I40E_FLAG_LEGACY_RX BIT_ULL(21)
#define I40E_FLAG_PTP BIT_ULL(22)
#define I40E_FLAG_IWARP_ENABLED BIT_ULL(23)
-#define I40E_FLAG_SERVICE_CLIENT_REQUESTED BIT_ULL(24)
-#define I40E_FLAG_CLIENT_L2_CHANGE BIT_ULL(25)
-#define I40E_FLAG_CLIENT_RESET BIT_ULL(26)
+/* Gap for BIT_ULL(24) through BIT_ULL(26) */
#define I40E_FLAG_LINK_DOWN_ON_CLOSE_ENABLED BIT_ULL(27)
#define I40E_FLAG_SOURCE_PRUNING_DISABLED BIT_ULL(28)
#define I40E_FLAG_TC_MQPRIO BIT_ULL(29)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_client.c b/drivers/net/ethernet/intel/i40e/i40e_client.c
index 999dea5a7c9e..d8ce4999864f 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_client.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_client.c
@@ -376,9 +376,8 @@ void i40e_client_subtask(struct i40e_pf *pf)
struct i40e_vsi *vsi = pf->vsi[pf->lan_vsi];
int ret = 0;
- if (!(pf->flags & I40E_FLAG_SERVICE_CLIENT_REQUESTED))
+ if (!test_and_clear_bit(__I40E_CLIENT_SERVICE_REQUESTED, pf->state))
return;
- pf->flags &= ~I40E_FLAG_SERVICE_CLIENT_REQUESTED;
cdev = pf->cinst;
/* If we're down or resetting, just bail */
@@ -459,7 +458,7 @@ int i40e_lan_add_device(struct i40e_pf *pf)
* added, we can schedule a subtask to go initiate the clients if
* they can be launched at probe time.
*/
- pf->flags |= I40E_FLAG_SERVICE_CLIENT_REQUESTED;
+ set_bit(__I40E_CLIENT_SERVICE_REQUESTED, pf->state);
i40e_service_event_schedule(pf);
out:
@@ -554,7 +553,7 @@ static void i40e_client_prepare(struct i40e_client *client)
pf = ldev->pf;
i40e_client_add_instance(pf);
/* Start the client subtask */
- pf->flags |= I40E_FLAG_SERVICE_CLIENT_REQUESTED;
+ set_bit(__I40E_CLIENT_SERVICE_REQUESTED, pf->state);
i40e_service_event_schedule(pf);
}
mutex_unlock(&i40e_device_mutex);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 307f5bf65708..68b51761c509 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -2634,8 +2634,8 @@ static int i40e_change_mtu(struct net_device *netdev, int new_mtu)
netdev->mtu = new_mtu;
if (netif_running(netdev))
i40e_vsi_reinit_locked(vsi);
- pf->flags |= (I40E_FLAG_SERVICE_CLIENT_REQUESTED |
- I40E_FLAG_CLIENT_L2_CHANGE);
+ set_bit(__I40E_CLIENT_SERVICE_REQUESTED, pf->state);
+ set_bit(__I40E_CLIENT_L2_CHANGE, pf->state);
return 0;
}
@@ -4722,9 +4722,9 @@ static void i40e_vsi_close(struct i40e_vsi *vsi)
i40e_vsi_free_tx_resources(vsi);
i40e_vsi_free_rx_resources(vsi);
vsi->current_netdev_flags = 0;
- pf->flags |= I40E_FLAG_SERVICE_CLIENT_REQUESTED;
+ set_bit(__I40E_CLIENT_SERVICE_REQUESTED, pf->state);
if (test_bit(__I40E_RESET_RECOVERY_PENDING, pf->state))
- pf->flags |= I40E_FLAG_CLIENT_RESET;
+ set_bit(__I40E_CLIENT_RESET, pf->state);
}
/**
@@ -6495,7 +6495,7 @@ static int i40e_up_complete(struct i40e_vsi *vsi)
/* On the next run of the service_task, notify any clients of the new
* opened netdev
*/
- pf->flags |= I40E_FLAG_SERVICE_CLIENT_REQUESTED;
+ set_bit(__I40E_CLIENT_SERVICE_REQUESTED, pf->state);
i40e_service_event_schedule(pf);
return 0;
@@ -8037,8 +8037,8 @@ static int i40e_handle_lldp_event(struct i40e_pf *pf,
i40e_service_event_schedule(pf);
} else {
i40e_pf_unquiesce_all_vsi(pf);
- pf->flags |= (I40E_FLAG_SERVICE_CLIENT_REQUESTED |
- I40E_FLAG_CLIENT_L2_CHANGE);
+ set_bit(__I40E_CLIENT_SERVICE_REQUESTED, pf->state);
+ set_bit(__I40E_CLIENT_L2_CHANGE, pf->state);
}
exit:
@@ -9785,17 +9785,15 @@ static void i40e_service_task(struct work_struct *work)
i40e_vc_process_vflr_event(pf);
i40e_watchdog_subtask(pf);
i40e_fdir_reinit_subtask(pf);
- if (pf->flags & I40E_FLAG_CLIENT_RESET) {
+ if (test_and_clear_bit(__I40E_CLIENT_RESET, pf->state)) {
/* Client subtask will reopen next time through. */
i40e_notify_client_of_netdev_close(pf->vsi[pf->lan_vsi], true);
- pf->flags &= ~I40E_FLAG_CLIENT_RESET;
} else {
i40e_client_subtask(pf);
- if (pf->flags & I40E_FLAG_CLIENT_L2_CHANGE) {
+ if (test_and_clear_bit(__I40E_CLIENT_L2_CHANGE,
+ pf->state))
i40e_notify_client_of_l2_param_changes(
pf->vsi[pf->lan_vsi]);
- pf->flags &= ~I40E_FLAG_CLIENT_L2_CHANGE;
- }
}
i40e_sync_filters_subtask(pf);
i40e_sync_udp_filters_subtask(pf);
--
2.14.3
^ permalink raw reply related
* [net-next 09/10] i40e: tweak page counting for XDP_REDIRECT
From: Jeff Kirsher @ 2018-03-26 21:41 UTC (permalink / raw)
To: davem
Cc: Björn Töpel, netdev, nhorman, sassmann, jogreene,
Jeff Kirsher
In-Reply-To: <20180326214103.18218-1-jeffrey.t.kirsher@intel.com>
From: Björn Töpel <bjorn.topel@intel.com>
This commit tweaks the page counting for XDP_REDIRECT to function
properly. XDP_REDIRECT support will be added in a future commit.
The current page counting scheme assumes that the reference count
cannot decrease until the received frame is sent to the upper layers
of the networking stack. This assumption does not hold for the
XDP_REDIRECT action, since a page (pointed out by xdp_buff) can have
its reference count decreased via the xdp_do_redirect call.
To work around that, we now start off by a large page count and then
don't allow a refcount less than two.
Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 797bcdd3504e..9c338cef8315 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -1588,9 +1588,8 @@ static bool i40e_alloc_mapped_page(struct i40e_ring *rx_ring,
bi->dma = dma;
bi->page = page;
bi->page_offset = i40e_rx_offset(rx_ring);
-
- /* initialize pagecnt_bias to 1 representing we fully own page */
- bi->pagecnt_bias = 1;
+ page_ref_add(page, USHRT_MAX - 1);
+ bi->pagecnt_bias = USHRT_MAX;
return true;
}
@@ -1956,8 +1955,8 @@ static bool i40e_can_reuse_rx_page(struct i40e_rx_buffer *rx_buffer)
* the pagecnt_bias and page count so that we fully restock the
* number of references the driver holds.
*/
- if (unlikely(!pagecnt_bias)) {
- page_ref_add(page, USHRT_MAX);
+ if (unlikely(pagecnt_bias == 1)) {
+ page_ref_add(page, USHRT_MAX - 1);
rx_buffer->pagecnt_bias = USHRT_MAX;
}
--
2.14.3
^ permalink raw reply related
* [net-next 08/10] i40e: re-number feature flags to remove gaps
From: Jeff Kirsher @ 2018-03-26 21:41 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20180326214103.18218-1-jeffrey.t.kirsher@intel.com>
From: Jacob Keller <jacob.e.keller@intel.com>
Remove the gaps created by the recent refactor of various feature flags
that have moved to the state field. Use only a u32 now that we have
fewer than 32 flags in the field.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e.h | 57 ++++++++++++++++------------------
1 file changed, 26 insertions(+), 31 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index 67518013ca4d..a44139c1de80 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -518,37 +518,32 @@ struct i40e_pf {
#define I40E_HW_RESTART_AUTONEG BIT(18)
#define I40E_HW_STOPPABLE_FW_LLDP BIT(19)
- u64 flags;
-#define I40E_FLAG_RX_CSUM_ENABLED BIT_ULL(0)
-#define I40E_FLAG_MSI_ENABLED BIT_ULL(1)
-#define I40E_FLAG_MSIX_ENABLED BIT_ULL(2)
-#define I40E_FLAG_RSS_ENABLED BIT_ULL(3)
-#define I40E_FLAG_VMDQ_ENABLED BIT_ULL(4)
-/* Gap for BIT_ULL(5) */
-#define I40E_FLAG_SRIOV_ENABLED BIT_ULL(6)
-#define I40E_FLAG_DCB_CAPABLE BIT_ULL(7)
-#define I40E_FLAG_DCB_ENABLED BIT_ULL(8)
-#define I40E_FLAG_FD_SB_ENABLED BIT_ULL(9)
-#define I40E_FLAG_FD_ATR_ENABLED BIT_ULL(10)
-/* Gap for BIT_ULL(11) and BIT_ULL(12) */
-#define I40E_FLAG_MFP_ENABLED BIT_ULL(13)
-/* Gap for BIT_ULL(14) */
-#define I40E_FLAG_HW_ATR_EVICT_ENABLED BIT_ULL(15)
-#define I40E_FLAG_VEB_MODE_ENABLED BIT_ULL(16)
-#define I40E_FLAG_VEB_STATS_ENABLED BIT_ULL(17)
-#define I40E_FLAG_LINK_POLLING_ENABLED BIT_ULL(18)
-#define I40E_FLAG_TRUE_PROMISC_SUPPORT BIT_ULL(19)
-/* Gap for BIT_ULL(20) */
-#define I40E_FLAG_LEGACY_RX BIT_ULL(21)
-#define I40E_FLAG_PTP BIT_ULL(22)
-#define I40E_FLAG_IWARP_ENABLED BIT_ULL(23)
-/* Gap for BIT_ULL(24) through BIT_ULL(26) */
-#define I40E_FLAG_LINK_DOWN_ON_CLOSE_ENABLED BIT_ULL(27)
-#define I40E_FLAG_SOURCE_PRUNING_DISABLED BIT_ULL(28)
-#define I40E_FLAG_TC_MQPRIO BIT_ULL(29)
-#define I40E_FLAG_FD_SB_INACTIVE BIT_ULL(30)
-#define I40E_FLAG_FD_SB_TO_CLOUD_FILTER BIT_ULL(31)
-#define I40E_FLAG_DISABLE_FW_LLDP BIT_ULL(32)
+ u32 flags;
+#define I40E_FLAG_RX_CSUM_ENABLED BIT(0)
+#define I40E_FLAG_MSI_ENABLED BIT(1)
+#define I40E_FLAG_MSIX_ENABLED BIT(2)
+#define I40E_FLAG_RSS_ENABLED BIT(3)
+#define I40E_FLAG_VMDQ_ENABLED BIT(4)
+#define I40E_FLAG_SRIOV_ENABLED BIT(5)
+#define I40E_FLAG_DCB_CAPABLE BIT(6)
+#define I40E_FLAG_DCB_ENABLED BIT(7)
+#define I40E_FLAG_FD_SB_ENABLED BIT(8)
+#define I40E_FLAG_FD_ATR_ENABLED BIT(9)
+#define I40E_FLAG_MFP_ENABLED BIT(10)
+#define I40E_FLAG_HW_ATR_EVICT_ENABLED BIT(11)
+#define I40E_FLAG_VEB_MODE_ENABLED BIT(12)
+#define I40E_FLAG_VEB_STATS_ENABLED BIT(13)
+#define I40E_FLAG_LINK_POLLING_ENABLED BIT(14)
+#define I40E_FLAG_TRUE_PROMISC_SUPPORT BIT(15)
+#define I40E_FLAG_LEGACY_RX BIT(16)
+#define I40E_FLAG_PTP BIT(17)
+#define I40E_FLAG_IWARP_ENABLED BIT(18)
+#define I40E_FLAG_LINK_DOWN_ON_CLOSE_ENABLED BIT(19)
+#define I40E_FLAG_SOURCE_PRUNING_DISABLED BIT(20)
+#define I40E_FLAG_TC_MQPRIO BIT(21)
+#define I40E_FLAG_FD_SB_INACTIVE BIT(22)
+#define I40E_FLAG_FD_SB_TO_CLOUD_FILTER BIT(23)
+#define I40E_FLAG_DISABLE_FW_LLDP BIT(24)
struct i40e_client_instance *cinst;
bool stat_offsets_loaded;
--
2.14.3
^ permalink raw reply related
* [net-next 07/10] i40e: stop using cmpxchg flow in i40e_set_priv_flags()
From: Jeff Kirsher @ 2018-03-26 21:41 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20180326214103.18218-1-jeffrey.t.kirsher@intel.com>
From: Jacob Keller <jacob.e.keller@intel.com>
Now that the only places which modify flags are either (a) during
initialization prior to creating a netdevice, or (b) while holding the
rtnl lock, we no longer need the cmpxchg64 call in i40e_set_priv_flags.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 19 +++++--------------
1 file changed, 5 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 2a9c93091e3d..b974482ff630 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -4436,21 +4436,12 @@ static int i40e_set_priv_flags(struct net_device *dev, u32 flags)
}
}
- /* Compare and exchange the new flags into place. If we failed, that
- * is if cmpxchg returns anything but the old value, this means that
- * something else has modified the flags variable since we copied it
- * originally. We'll just punt with an error and log something in the
- * message buffer.
- *
- * This is the point of no return for this function. We need to have
- * checked any discrepancies or misconfigurations and returned
- * EOPNOTSUPP before updating pf->flags here.
+ /* Now that we've checked to ensure that the new flags are valid, load
+ * them into place. Since we only modify flags either (a) during
+ * initialization or (b) while holding the RTNL lock, we don't need
+ * anything fancy here.
*/
- if (cmpxchg64(&pf->flags, orig_flags, new_flags) != orig_flags) {
- dev_warn(&pf->pdev->dev,
- "Unable to update pf->flags as it was modified by another thread...\n");
- return -EAGAIN;
- }
+ pf->flags = new_flags;
/* Process any additional changes needed as a result of flag changes.
* The changed_flags value reflects the list of bits that were
--
2.14.3
^ permalink raw reply related
* [net-next 10/10] i40e: add support for XDP_REDIRECT
From: Jeff Kirsher @ 2018-03-26 21:41 UTC (permalink / raw)
To: davem
Cc: Björn Töpel, netdev, nhorman, sassmann, jogreene,
Jeff Kirsher
In-Reply-To: <20180326214103.18218-1-jeffrey.t.kirsher@intel.com>
From: Björn Töpel <bjorn.topel@intel.com>
The driver now acts upon the XDP_REDIRECT return action. Two new ndos
are implemented, ndo_xdp_xmit and ndo_xdp_flush.
XDP_REDIRECT action enables XDP program to redirect frames to other
netdevs.
Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 2 +
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 74 +++++++++++++++++++++++++----
drivers/net/ethernet/intel/i40e/i40e_txrx.h | 2 +
3 files changed, 68 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 5efd6d7bfa59..16229998fb1e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -11815,6 +11815,8 @@ static const struct net_device_ops i40e_netdev_ops = {
.ndo_bridge_getlink = i40e_ndo_bridge_getlink,
.ndo_bridge_setlink = i40e_ndo_bridge_setlink,
.ndo_bpf = i40e_xdp,
+ .ndo_xdp_xmit = i40e_xdp_xmit,
+ .ndo_xdp_flush = i40e_xdp_flush,
};
/**
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 9c338cef8315..f174c72480ab 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -2214,7 +2214,7 @@ static int i40e_xmit_xdp_ring(struct xdp_buff *xdp,
static struct sk_buff *i40e_run_xdp(struct i40e_ring *rx_ring,
struct xdp_buff *xdp)
{
- int result = I40E_XDP_PASS;
+ int err, result = I40E_XDP_PASS;
struct i40e_ring *xdp_ring;
struct bpf_prog *xdp_prog;
u32 act;
@@ -2233,6 +2233,10 @@ static struct sk_buff *i40e_run_xdp(struct i40e_ring *rx_ring,
xdp_ring = rx_ring->vsi->xdp_rings[rx_ring->queue_index];
result = i40e_xmit_xdp_ring(xdp, xdp_ring);
break;
+ case XDP_REDIRECT:
+ err = xdp_do_redirect(rx_ring->netdev, xdp, xdp_prog);
+ result = !err ? I40E_XDP_TX : I40E_XDP_CONSUMED;
+ break;
default:
bpf_warn_invalid_xdp_action(act);
case XDP_ABORTED:
@@ -2268,6 +2272,15 @@ static void i40e_rx_buffer_flip(struct i40e_ring *rx_ring,
#endif
}
+static inline void i40e_xdp_ring_update_tail(struct i40e_ring *xdp_ring)
+{
+ /* Force memory writes to complete before letting h/w
+ * know there are new descriptors to fetch.
+ */
+ wmb();
+ writel_relaxed(xdp_ring->next_to_use, xdp_ring->tail);
+}
+
/**
* i40e_clean_rx_irq - Clean completed descriptors from Rx ring - bounce buf
* @rx_ring: rx descriptor ring to transact packets on
@@ -2402,16 +2415,11 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
}
if (xdp_xmit) {
- struct i40e_ring *xdp_ring;
-
- xdp_ring = rx_ring->vsi->xdp_rings[rx_ring->queue_index];
+ struct i40e_ring *xdp_ring =
+ rx_ring->vsi->xdp_rings[rx_ring->queue_index];
- /* Force memory writes to complete before letting h/w
- * know there are new descriptors to fetch.
- */
- wmb();
-
- writel(xdp_ring->next_to_use, xdp_ring->tail);
+ i40e_xdp_ring_update_tail(xdp_ring);
+ xdp_do_flush_map();
}
rx_ring->skb = skb;
@@ -3659,3 +3667,49 @@ netdev_tx_t i40e_lan_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
return i40e_xmit_frame_ring(skb, tx_ring);
}
+
+/**
+ * i40e_xdp_xmit - Implements ndo_xdp_xmit
+ * @dev: netdev
+ * @xdp: XDP buffer
+ *
+ * Returns Zero if sent, else an error code
+ **/
+int i40e_xdp_xmit(struct net_device *dev, struct xdp_buff *xdp)
+{
+ struct i40e_netdev_priv *np = netdev_priv(dev);
+ unsigned int queue_index = smp_processor_id();
+ struct i40e_vsi *vsi = np->vsi;
+ int err;
+
+ if (test_bit(__I40E_VSI_DOWN, vsi->state))
+ return -ENETDOWN;
+
+ if (!i40e_enabled_xdp_vsi(vsi) || queue_index >= vsi->num_queue_pairs)
+ return -ENXIO;
+
+ err = i40e_xmit_xdp_ring(xdp, vsi->xdp_rings[queue_index]);
+ if (err != I40E_XDP_TX)
+ return -ENOSPC;
+
+ return 0;
+}
+
+/**
+ * i40e_xdp_flush - Implements ndo_xdp_flush
+ * @dev: netdev
+ **/
+void i40e_xdp_flush(struct net_device *dev)
+{
+ struct i40e_netdev_priv *np = netdev_priv(dev);
+ unsigned int queue_index = smp_processor_id();
+ struct i40e_vsi *vsi = np->vsi;
+
+ if (test_bit(__I40E_VSI_DOWN, vsi->state))
+ return;
+
+ if (!i40e_enabled_xdp_vsi(vsi) || queue_index >= vsi->num_queue_pairs)
+ return;
+
+ i40e_xdp_ring_update_tail(vsi->xdp_rings[queue_index]);
+}
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.h b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
index 7f8220e65374..3043483ec426 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
@@ -510,6 +510,8 @@ u32 i40e_get_tx_pending(struct i40e_ring *ring, bool in_sw);
void i40e_detect_recover_hung(struct i40e_vsi *vsi);
int __i40e_maybe_stop_tx(struct i40e_ring *tx_ring, int size);
bool __i40e_chk_linearize(struct sk_buff *skb);
+int i40e_xdp_xmit(struct net_device *dev, struct xdp_buff *xdp);
+void i40e_xdp_flush(struct net_device *dev);
/**
* i40e_get_head - Retrieve head from head writeback
--
2.14.3
^ permalink raw reply related
* Re: [RFC PATCH 00/24] Introducing AF_XDP support
From: William Tu @ 2018-03-26 21:58 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Björn Töpel, magnus.karlsson, Alexander Duyck,
Alexander Duyck, John Fastabend, Alexei Starovoitov,
willemdebruijn.kernel, Daniel Borkmann,
Linux Kernel Network Developers, Björn Töpel,
michael.lundkvist, jesse.brandeburg, anjali.singhai,
jeffrey.b.shaw, ferruh.yigit, qi.z.zhang
In-Reply-To: <20180326183810.2ef4e29f@redhat.com>
Hi Jesper,
Thanks a lot for your prompt reply.
>> Hi,
>> I also did an evaluation of AF_XDP, however the performance isn't as
>> good as above.
>> I'd like to share the result and see if there are some tuning suggestions.
>>
>> System:
>> 16 core, Intel(R) Xeon(R) CPU E5-2440 v2 @ 1.90GHz
>> Intel 10G X540-AT2 ---> so I can only run XDP_SKB mode
>
> Hmmm, why is X540-AT2 not able to use XDP natively?
Because I'm only able to use ixgbe driver for this NIC,
and AF_XDP patch only has i40e support?
>
>> AF_XDP performance:
>> Benchmark XDP_SKB
>> rxdrop 1.27 Mpps
>> txpush 0.99 Mpps
>> l2fwd 0.85 Mpps
>
> Definitely too low...
>
I did another run, the rxdrop seems better.
Benchmark XDP_SKB
rxdrop 2.3 Mpps
txpush 1.05 Mpps
l2fwd 0.90 Mpps
> What is the performance if you drop packets via iptables?
>
> Command:
> $ iptables -t raw -I PREROUTING -p udp --dport 9 --j DROP
>
I did
# iptables -t raw -I PREROUTING -p udp -i enp10s0f0 -j DROP
# iptables -nvL -t raw; sleep 10; iptables -nvL -t raw
and I got 2.9Mpps.
>> NIC configuration:
>> the command
>> "ethtool -N p3p2 flow-type udp4 src-port 4242 dst-port 4242 action 16"
>> doesn't work on my ixgbe driver, so I use ntuple:
>>
>> ethtool -K enp10s0f0 ntuple on
>> ethtool -U enp10s0f0 flow-type udp4 src-ip 10.1.1.100 action 1
>> then
>> echo 1 > /proc/sys/net/core/bpf_jit_enable
>> ./xdpsock -i enp10s0f0 -r -S --queue=1
>>
>> I also take a look at perf result:
>> For rxdrop:
>> 86.56% xdpsock xdpsock [.] main
>> 9.22% xdpsock [kernel.vmlinux] [k] nmi
>> 4.23% xdpsock xdpsock [.] xq_enq
>
> It looks very strange that you see non-maskable interrupt's (NMI) being
> this high...
>
yes, that's weird. Looking at the perf annotate of nmi,
it shows 100% spent on nop instruction.
>
>> For l2fwd:
>> 20.81% xdpsock xdpsock [.] main
>> 10.64% xdpsock [kernel.vmlinux] [k] clflush_cache_range
>
> Oh, clflush_cache_range is being called!
I though clflush_cache_range is high because we have many smp_rmb, smp_wmb
in the xdpsock queue/ring management userspace code.
(perf shows that 75% of this 10.64% spent on mfence instruction.)
> Do your system use an IOMMU ?
>
Yes.
With CONFIG_INTEL_IOMMU=y
and I saw some related functions called (ex: intel_alloc_iova).
>> 8.46% xdpsock [kernel.vmlinux] [k] xsk_sendmsg
>> 6.72% xdpsock [kernel.vmlinux] [k] skb_set_owner_w
>> 5.89% xdpsock [kernel.vmlinux] [k] __domain_mapping
>> 5.74% xdpsock [kernel.vmlinux] [k] alloc_skb_with_frags
>> 4.62% xdpsock [kernel.vmlinux] [k] netif_skb_features
>> 3.96% xdpsock [kernel.vmlinux] [k] ___slab_alloc
>> 3.18% xdpsock [kernel.vmlinux] [k] nmi
>
> Again high count for NMI ?!?
>
> Maybe you just forgot to tell perf that you want it to decode the
> bpf_prog correctly?
>
> https://prototype-kernel.readthedocs.io/en/latest/bpf/troubleshooting.html#perf-tool-symbols
>
> Enable via:
> $ sysctl net/core/bpf_jit_kallsyms=1
>
> And use perf report (while BPF is STILL LOADED):
>
> $ perf report --kallsyms=/proc/kallsyms
>
> E.g. for emailing this you can use this command:
>
> $ perf report --sort cpu,comm,dso,symbol --kallsyms=/proc/kallsyms --no-children --stdio -g none | head -n 40
>
Thanks, I followed the steps, the result of l2fwd
# Total Lost Samples: 119
#
# Samples: 2K of event 'cycles:ppp'
# Event count (approx.): 25675705627
#
# Overhead CPU Command Shared Object Symbol
# ........ ... ....... .................. ..................................
#
10.48% 013 xdpsock xdpsock [.] main
9.77% 013 xdpsock [kernel.vmlinux] [k] clflush_cache_range
8.45% 013 xdpsock [kernel.vmlinux] [k] nmi
8.07% 013 xdpsock [kernel.vmlinux] [k] xsk_sendmsg
7.81% 013 xdpsock [kernel.vmlinux] [k] __domain_mapping
4.95% 013 xdpsock [kernel.vmlinux] [k] ixgbe_xmit_frame_ring
4.66% 013 xdpsock [kernel.vmlinux] [k] skb_store_bits
4.39% 013 xdpsock [kernel.vmlinux] [k] syscall_return_via_sysret
3.93% 013 xdpsock [kernel.vmlinux] [k] pfn_to_dma_pte
2.62% 013 xdpsock [kernel.vmlinux] [k] __intel_map_single
2.53% 013 xdpsock [kernel.vmlinux] [k] __alloc_skb
2.36% 013 xdpsock [kernel.vmlinux] [k] iommu_no_mapping
2.21% 013 xdpsock [kernel.vmlinux] [k] alloc_skb_with_frags
2.07% 013 xdpsock [kernel.vmlinux] [k] skb_set_owner_w
1.98% 013 xdpsock [kernel.vmlinux] [k] __kmalloc_node_track_caller
1.94% 013 xdpsock [kernel.vmlinux] [k] ksize
1.84% 013 xdpsock [kernel.vmlinux] [k] validate_xmit_skb_list
1.62% 013 xdpsock [kernel.vmlinux] [k] kmem_cache_alloc_node
1.48% 013 xdpsock [kernel.vmlinux] [k] __kmalloc_reserve.isra.37
1.21% 013 xdpsock xdpsock [.] xq_enq
1.08% 013 xdpsock [kernel.vmlinux] [k] intel_alloc_iova
And l2fwd under "perf stat" looks OK to me. There is little context
switches, cpu
is fully utilized, 1.17 insn per cycle seems ok.
Performance counter stats for 'CPU(s) 6':
10000.787420 cpu-clock (msec) # 1.000 CPUs
utilized
24 context-switches # 0.002 K/sec
0 cpu-migrations # 0.000 K/sec
0 page-faults # 0.000 K/sec
22,361,333,647 cycles # 2.236 GHz
13,458,442,838 stalled-cycles-frontend # 60.19% frontend
cycles idle
26,251,003,067 instructions # 1.17 insn per
cycle
# 0.51 stalled
cycles per insn
4,938,921,868 branches # 493.853 M/sec
7,591,739 branch-misses # 0.15% of all
branches
10.000835769 seconds time elapsed
Will continue investigate...
Thanks
William
^ permalink raw reply
* Re: [PATCH net-next 0/2] net: broadcom: Adaptive interrupt coalescing
From: Florian Fainelli @ 2018-03-26 22:04 UTC (permalink / raw)
To: Tal Gilboa, netdev
Cc: davem, jaedon.shin, pgynther, opendmb, michal.chan, gospo, saeedm
In-Reply-To: <a0263c2c-6fc6-45d4-9163-2a96b7d34492@mellanox.com>
On 03/26/2018 02:16 PM, Tal Gilboa wrote:
> On 3/23/2018 4:19 AM, Florian Fainelli wrote:
>> Hi all,
>>
>> This patch series adds adaptive interrupt coalescing for the Gigabit
>> Ethernet
>> drivers SYSTEMPORT and GENET.
>>
>> This really helps lower the interrupt count and system load, as
>> measured by
>> vmstat for a Gigabit TCP RX session:
>
> I don't see an improvement in system load, the opposite - 42% vs. 100%
> for SYSTEMPORT and 85% vs. 100% for GENET. Both with the same bandwidth.
Looks like I did not extract the correct data the load could spike in
both cases (with and without net_dim) up to 100, but averaged over the
transmission I see the following:
GENET without:
1 0 0 1169568 0 25556 0 0 0 0 130079 62795 2
86 13 0 0
GENET with:
1 0 0 1169536 0 25556 0 0 0 0 10566 10869 1
21 78 0 0
> Am I missing something? Talking about bandwidth, I would expect 941Mb/s
> (assuming this is TCP over IPv4). Do you know why the reduced interrupt
> rate doesn't improve bandwidth?
I am assuming that this comes down to a latency, still capturing some
pcap files to analyze the TCP session with wireshark and see if that is
indeed what is going on. The test machine is actually not that great
> Also, any effect on the client side (you
> mentioned enabling TX moderation for SYSTEMPORT)?
Yes, on SYSTEMPORT, being the TCP IPv4 client, I have the following:
SYSTEMPORT without:
2 0 0 191428 0 25748 0 0 0 0 86254 264 0 41
59 0 0
SYSTEMPORT with:
3 0 0 190176 0 25748 0 0 0 0 45485 31332 0
100 0 0 0
I don't get top to agree with these load results though but it looks
like we just have the CPU spinning more, does not look like a win.
Thanks a lot for taking a look at this Tal!
--
Florian
^ permalink raw reply
* [Patch net] llc: properly handle dev_queue_xmit() return value
From: Cong Wang @ 2018-03-26 22:08 UTC (permalink / raw)
To: netdev; +Cc: noamr, Cong Wang
llc_conn_send_pdu() pushes the skb into write queue and
calls llc_conn_send_pdus() to flush them out. However, the
status of dev_queue_xmit() is not returned to caller,
in this case, llc_conn_state_process().
llc_conn_state_process() needs hold the skb no matter
success or failure, because it still uses it after that,
therefore we should hold skb before dev_queue_xmit() when
that skb is the one being processed by llc_conn_state_process().
For other callers, they can just pass NULL and ignore
the return value as they are.
Reported-by: Noam Rathaus <noamr@beyondsecurity.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
include/net/llc_conn.h | 2 +-
net/llc/llc_c_ac.c | 15 +++++++++------
net/llc/llc_conn.c | 32 +++++++++++++++++++++++---------
3 files changed, 33 insertions(+), 16 deletions(-)
diff --git a/include/net/llc_conn.h b/include/net/llc_conn.h
index fe994d2e5286..5c40f118c0fa 100644
--- a/include/net/llc_conn.h
+++ b/include/net/llc_conn.h
@@ -103,7 +103,7 @@ void llc_sk_reset(struct sock *sk);
/* Access to a connection */
int llc_conn_state_process(struct sock *sk, struct sk_buff *skb);
-void llc_conn_send_pdu(struct sock *sk, struct sk_buff *skb);
+int llc_conn_send_pdu(struct sock *sk, struct sk_buff *skb);
void llc_conn_rtn_pdu(struct sock *sk, struct sk_buff *skb);
void llc_conn_resend_i_pdu_as_cmd(struct sock *sk, u8 nr, u8 first_p_bit);
void llc_conn_resend_i_pdu_as_rsp(struct sock *sk, u8 nr, u8 first_f_bit);
diff --git a/net/llc/llc_c_ac.c b/net/llc/llc_c_ac.c
index f59648018060..163121192aca 100644
--- a/net/llc/llc_c_ac.c
+++ b/net/llc/llc_c_ac.c
@@ -389,7 +389,7 @@ static int llc_conn_ac_send_i_cmd_p_set_0(struct sock *sk, struct sk_buff *skb)
llc_pdu_init_as_i_cmd(skb, 0, llc->vS, llc->vR);
rc = llc_mac_hdr_init(skb, llc->dev->dev_addr, llc->daddr.mac);
if (likely(!rc)) {
- llc_conn_send_pdu(sk, skb);
+ rc = llc_conn_send_pdu(sk, skb);
llc_conn_ac_inc_vs_by_1(sk, skb);
}
return rc;
@@ -916,7 +916,7 @@ static int llc_conn_ac_send_i_rsp_f_set_ackpf(struct sock *sk,
llc_pdu_init_as_i_cmd(skb, llc->ack_pf, llc->vS, llc->vR);
rc = llc_mac_hdr_init(skb, llc->dev->dev_addr, llc->daddr.mac);
if (likely(!rc)) {
- llc_conn_send_pdu(sk, skb);
+ rc = llc_conn_send_pdu(sk, skb);
llc_conn_ac_inc_vs_by_1(sk, skb);
}
return rc;
@@ -935,14 +935,17 @@ static int llc_conn_ac_send_i_rsp_f_set_ackpf(struct sock *sk,
int llc_conn_ac_send_i_as_ack(struct sock *sk, struct sk_buff *skb)
{
struct llc_sock *llc = llc_sk(sk);
+ int ret;
if (llc->ack_must_be_send) {
- llc_conn_ac_send_i_rsp_f_set_ackpf(sk, skb);
+ ret = llc_conn_ac_send_i_rsp_f_set_ackpf(sk, skb);
llc->ack_must_be_send = 0 ;
llc->ack_pf = 0;
- } else
- llc_conn_ac_send_i_cmd_p_set_0(sk, skb);
- return 0;
+ } else {
+ ret = llc_conn_ac_send_i_cmd_p_set_0(sk, skb);
+ }
+
+ return ret;
}
/**
diff --git a/net/llc/llc_conn.c b/net/llc/llc_conn.c
index 9177dbb16dce..110e32bcb399 100644
--- a/net/llc/llc_conn.c
+++ b/net/llc/llc_conn.c
@@ -30,7 +30,7 @@
#endif
static int llc_find_offset(int state, int ev_type);
-static void llc_conn_send_pdus(struct sock *sk);
+static int llc_conn_send_pdus(struct sock *sk, struct sk_buff *skb);
static int llc_conn_service(struct sock *sk, struct sk_buff *skb);
static int llc_exec_conn_trans_actions(struct sock *sk,
struct llc_conn_state_trans *trans,
@@ -193,11 +193,11 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb)
return rc;
}
-void llc_conn_send_pdu(struct sock *sk, struct sk_buff *skb)
+int llc_conn_send_pdu(struct sock *sk, struct sk_buff *skb)
{
/* queue PDU to send to MAC layer */
skb_queue_tail(&sk->sk_write_queue, skb);
- llc_conn_send_pdus(sk);
+ return llc_conn_send_pdus(sk, skb);
}
/**
@@ -255,7 +255,7 @@ void llc_conn_resend_i_pdu_as_cmd(struct sock *sk, u8 nr, u8 first_p_bit)
if (howmany_resend > 0)
llc->vS = (llc->vS + 1) % LLC_2_SEQ_NBR_MODULO;
/* any PDUs to re-send are queued up; start sending to MAC */
- llc_conn_send_pdus(sk);
+ llc_conn_send_pdus(sk, NULL);
out:;
}
@@ -296,7 +296,7 @@ void llc_conn_resend_i_pdu_as_rsp(struct sock *sk, u8 nr, u8 first_f_bit)
if (howmany_resend > 0)
llc->vS = (llc->vS + 1) % LLC_2_SEQ_NBR_MODULO;
/* any PDUs to re-send are queued up; start sending to MAC */
- llc_conn_send_pdus(sk);
+ llc_conn_send_pdus(sk, NULL);
out:;
}
@@ -340,12 +340,16 @@ int llc_conn_remove_acked_pdus(struct sock *sk, u8 nr, u16 *how_many_unacked)
/**
* llc_conn_send_pdus - Sends queued PDUs
* @sk: active connection
+ * @hold_skb: the skb held by caller, or NULL if does not care
*
- * Sends queued pdus to MAC layer for transmission.
+ * Sends queued pdus to MAC layer for transmission. When @hold_skb is
+ * NULL, always return 0. Otherwise, return 0 if @hold_skb is sent
+ * successfully, or 1 for failure.
*/
-static void llc_conn_send_pdus(struct sock *sk)
+static int llc_conn_send_pdus(struct sock *sk, struct sk_buff *hold_skb)
{
struct sk_buff *skb;
+ int ret = 0;
while ((skb = skb_dequeue(&sk->sk_write_queue)) != NULL) {
struct llc_pdu_sn *pdu = llc_pdu_sn_hdr(skb);
@@ -357,10 +361,20 @@ static void llc_conn_send_pdus(struct sock *sk)
skb_queue_tail(&llc_sk(sk)->pdu_unack_q, skb);
if (!skb2)
break;
- skb = skb2;
+ dev_queue_xmit(skb2);
+ } else {
+ bool is_target = skb == hold_skb;
+ int rc;
+
+ if (is_target)
+ skb_get(skb);
+ rc = dev_queue_xmit(skb);
+ if (is_target)
+ ret = rc;
}
- dev_queue_xmit(skb);
}
+
+ return ret;
}
/**
--
2.13.0
^ permalink raw reply related
* [PATCH bpf-next] bpf, tracing: unbreak lttng
From: Alexei Starovoitov @ 2018-03-26 22:08 UTC (permalink / raw)
To: davem
Cc: daniel, torvalds, peterz, rostedt, mathieu.desnoyers, netdev,
kernel-team, linux-api
for_each_kernel_tracepoint() is used by out-of-tree lttng module
and therefore cannot be changed.
Instead introduce kernel_tracepoint_find_by_name() to find
tracepoint by name.
Fixes: 9e9afbae6514 ("tracepoint: compute num_args at build time")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
include/linux/tracepoint.h | 14 ++++++++++----
kernel/bpf/syscall.c | 11 +----------
kernel/tracepoint.c | 36 ++++++++++++++++++++----------------
3 files changed, 31 insertions(+), 30 deletions(-)
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 2194e7c31484..035887dc4ed3 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -42,16 +42,22 @@ extern int
tracepoint_probe_unregister(struct tracepoint *tp, void *probe, void *data);
#ifdef CONFIG_TRACEPOINTS
-void *
-for_each_kernel_tracepoint(void *(*fct)(struct tracepoint *tp, void *priv),
+void
+for_each_kernel_tracepoint(void (*fct)(struct tracepoint *tp, void *priv),
void *priv);
+struct tracepoint *kernel_tracepoint_find_by_name(const char *name);
#else
-static inline void *
-for_each_kernel_tracepoint(void *(*fct)(struct tracepoint *tp, void *priv),
+static inline void
+for_each_kernel_tracepoint(void (*fct)(struct tracepoint *tp, void *priv),
void *priv)
{
return NULL;
}
+static inline struct tracepoint *
+kernel_tracepoint_find_by_name(const char *name)
+{
+ return NULL;
+}
#endif
#ifdef CONFIG_MODULES
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index ae8b43f1cee3..644311777d8e 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1334,15 +1334,6 @@ static const struct file_operations bpf_raw_tp_fops = {
.write = bpf_dummy_write,
};
-static void *__find_tp(struct tracepoint *tp, void *priv)
-{
- char *name = priv;
-
- if (!strcmp(tp->name, name))
- return tp;
- return NULL;
-}
-
#define BPF_RAW_TRACEPOINT_OPEN_LAST_FIELD raw_tracepoint.prog_fd
static int bpf_raw_tracepoint_open(const union bpf_attr *attr)
@@ -1358,7 +1349,7 @@ static int bpf_raw_tracepoint_open(const union bpf_attr *attr)
return -EFAULT;
tp_name[sizeof(tp_name) - 1] = 0;
- tp = for_each_kernel_tracepoint(__find_tp, tp_name);
+ tp = kernel_tracepoint_find_by_name(tp_name);
if (!tp)
return -ENOENT;
diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index 3f2dc5738c2b..764d02fbe782 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -502,22 +502,17 @@ static __init int init_tracepoints(void)
__initcall(init_tracepoints);
#endif /* CONFIG_MODULES */
-static void *for_each_tracepoint_range(struct tracepoint * const *begin,
- struct tracepoint * const *end,
- void *(*fct)(struct tracepoint *tp, void *priv),
- void *priv)
+static void for_each_tracepoint_range(struct tracepoint * const *begin,
+ struct tracepoint * const *end,
+ void (*fct)(struct tracepoint *tp, void *priv),
+ void *priv)
{
struct tracepoint * const *iter;
- void *ret;
if (!begin)
- return NULL;
- for (iter = begin; iter < end; iter++) {
- ret = fct(*iter, priv);
- if (ret)
- return ret;
- }
- return NULL;
+ return;
+ for (iter = begin; iter < end; iter++)
+ fct(*iter, priv);
}
/**
@@ -525,14 +520,23 @@ static void *for_each_tracepoint_range(struct tracepoint * const *begin,
* @fct: callback
* @priv: private data
*/
-void *for_each_kernel_tracepoint(void *(*fct)(struct tracepoint *tp, void *priv),
- void *priv)
+void for_each_kernel_tracepoint(void (*fct)(struct tracepoint *tp, void *priv),
+ void *priv)
{
- return for_each_tracepoint_range(__start___tracepoints_ptrs,
- __stop___tracepoints_ptrs, fct, priv);
+ for_each_tracepoint_range(__start___tracepoints_ptrs,
+ __stop___tracepoints_ptrs, fct, priv);
}
EXPORT_SYMBOL_GPL(for_each_kernel_tracepoint);
+struct tracepoint *kernel_tracepoint_find_by_name(const char *name)
+{
+ struct tracepoint * const *tp = __start___tracepoints_ptrs;
+
+ for (; tp < __stop___tracepoints_ptrs; tp++)
+ if (!strcmp((*tp)->name, name))
+ return *tp;
+ return NULL;
+}
#ifdef CONFIG_HAVE_SYSCALL_TRACEPOINTS
/* NB: reg/unreg are called while guarded with the tracepoints_mutex */
--
2.9.5
^ permalink raw reply related
* Re: [PATCH bpf-next] bpf, tracing: unbreak lttng
From: Steven Rostedt @ 2018-03-26 22:15 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: davem, daniel, torvalds, peterz, mathieu.desnoyers, netdev,
kernel-team, linux-api
In-Reply-To: <20180326220845.678423-1-ast@kernel.org>
On Mon, 26 Mar 2018 15:08:45 -0700
Alexei Starovoitov <ast@kernel.org> wrote:
> for_each_kernel_tracepoint() is used by out-of-tree lttng module
> and therefore cannot be changed.
> Instead introduce kernel_tracepoint_find_by_name() to find
> tracepoint by name.
>
> Fixes: 9e9afbae6514 ("tracepoint: compute num_args at build time")
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
I'm curious, why can't you rebase? The first patch was never acked.
-- Steve
^ permalink raw reply
* Re: [PATCH net-next 04/12] dt-bindings: net: dwmac-sun8i: Sort syscon compatibles by alphabetical order
From: Rob Herring @ 2018-03-26 22:23 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Maxime Ripard, Michael Turquette, Stephen Boyd,
Giuseppe Cavallaro, Mark Rutland, Mark Brown, linux-arm-kernel,
linux-clk, devicetree, netdev, Corentin Labbe, Icenowy Zheng
In-Reply-To: <20180317092857.4396-5-wens@csie.org>
On Sat, Mar 17, 2018 at 05:28:49PM +0800, Chen-Yu Tsai wrote:
> The A83T syscon compatible was appended to the syscon compatibles list,
> instead of inserted in to preserve the ordering.
>
> Move it to the proper place to keep the list sorted.
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
> Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH net] r8169: fix setting driver_data after register_netdev
From: Francois Romieu @ 2018-03-26 22:18 UTC (permalink / raw)
To: Andrew Lunn
Cc: Heiner Kallweit, Realtek linux nic maintainers, David Miller,
netdev@vger.kernel.org
In-Reply-To: <20180325232439.GD19365@lunn.ch>
Andrew Lunn <andrew@lunn.ch> :
[...]
> How about rtl8169_get_wol() and rtl8169_set_wol(). And
> rtl8169_get_ethtool_stats().
rtl8169_get_wol does not depend on dev->driver_data. Neither does
rtl8169_set_wol() nor rtl8169_get_ethtool_stats().
> Basically anything which makes use of run time power management
> could be invoked as soon as parts of register_netdev() have been
> called.
Ok, it can crash through rtl_open and check_link_status.
If rtl_open can be called that early, rtl_init_one::rtl8168_driver_start()
may also be executed a bit late.
--
Ueimor
^ permalink raw reply
* Re: [RFC PATCH v2] net: phy: Added device tree binding for dev-addr and dev-addr code check-up
From: Rob Herring @ 2018-03-26 22:25 UTC (permalink / raw)
To: Vicentiu Galanopulo
Cc: netdev, linux-kernel, mark.rutland, davem, marcel, devicetree,
madalin.bucur, alexandru.marginean
In-Reply-To: <20180323150522.9603-1-vicentiu.galanopulo@nxp.com>
On Fri, Mar 23, 2018 at 10:05:22AM -0500, Vicentiu Galanopulo wrote:
> Reason for this patch is that the Inphi PHY has a
> vendor specific address space for accessing the
> C45 MDIO registers - starting from 0x1e.
>
> A search of the dev-addr property is done in of_mdiobus_register.
> If the property is found in the PHY node,
> of_mdiobus_register_static_phy is called. This is a
> wrapper function for of_mdiobus_register_phy which finds the
> device in package based on dev-addr and fills devices_addrs:
> devices_addrs is a new field added to phy_c45_device_ids.
> This new field will store the dev-addr property on the same
> index where the device in package has been found.
> In order to have dev-addr in get_phy_c45_ids(), mdio_c45_ids is
> passed from of_mdio.c to phy_device.c as an external variable.
> In get_phy_device a copy of the mdio_c45_ids is done over the
> local c45_ids (wich are empty). After the copying, the c45_ids
> will also contain the static device found from dev-addr.
> Having dev-addr stored in devices_addrs, in get_phy_c45_ids(),
> when probing the identifiers, dev-addr can be extracted from
> devices_addrs and probed if devices_addrs[current_identifier]
> is not 0.
> This way changing the kernel API is avoided completely.
>
> As a plus to this patch, num_ids in get_phy_c45_ids,
> has the value 8 (ARRAY_SIZE(c45_ids->device_ids)),
> but the u32 *devs can store 32 devices in the bitfield.
> If a device is stored in *devs, in bits 32 to 9, it
> will not be found. This is the reason for changing
> in phy.h, the size of device_ids array.
>
> Signed-off-by: Vicentiu Galanopulo <vicentiu.galanopulo@nxp.com>
> ---
> Documentation/devicetree/bindings/net/phy.txt | 6 ++
Please split bindings to separate patch.
> drivers/net/phy/phy_device.c | 22 +++++-
> drivers/of/of_mdio.c | 98 ++++++++++++++++++++++++++-
> include/linux/phy.h | 5 +-
> 4 files changed, 125 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
> index d2169a5..82692e2 100644
> --- a/Documentation/devicetree/bindings/net/phy.txt
> +++ b/Documentation/devicetree/bindings/net/phy.txt
> @@ -61,6 +61,11 @@ Optional Properties:
> - reset-deassert-us: Delay after the reset was deasserted in microseconds.
> If this property is missing the delay will be skipped.
>
> +- dev-addr: If set, it indicates the device address of the PHY to be used
> + when accessing the C45 PHY registers over MDIO. It is used for vendor specific
> + register space addresses that do no conform to standard address for the MDIO
> + registers (e.g. MMD30)
This is a 2nd MDIO address, right? Can't you just append this to reg
property?
Rob
^ permalink raw reply
* Re: [PATCH net-next 4/8] dt-bindings: net: add DT bindings for Microsemi Ocelot Switch
From: Rob Herring @ 2018-03-26 22:25 UTC (permalink / raw)
To: Florian Fainelli
Cc: Alexandre Belloni, David S . Miller, Allan Nielsen,
razvan.stefanescu, po.liu, Thomas Petazzoni, Andrew Lunn, netdev,
devicetree, linux-kernel, linux-mips
In-Reply-To: <a5db6109-c5d9-f573-893c-f7d66c3168c2@gmail.com>
On Fri, Mar 23, 2018 at 02:11:35PM -0700, Florian Fainelli wrote:
> On 03/23/2018 01:11 PM, Alexandre Belloni wrote:
> > DT bindings for the Ethernet switch found on Microsemi Ocelot platforms.
> >
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> > ---
> > .../devicetree/bindings/net/mscc-ocelot.txt | 62 ++++++++++++++++++++++
> > 1 file changed, 62 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/net/mscc-ocelot.txt
> >
> > diff --git a/Documentation/devicetree/bindings/net/mscc-ocelot.txt b/Documentation/devicetree/bindings/net/mscc-ocelot.txt
> > new file mode 100644
> > index 000000000000..ee092a85b5a0
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/mscc-ocelot.txt
> > @@ -0,0 +1,62 @@
> > +Microsemi Ocelot network Switch
> > +===============================
> > +
> > +The Microsemi Ocelot network switch can be found on Microsemi SoCs (VSC7513,
> > +VSC7514)
> > +
> > +Required properties:
> > +- compatible: Should be "mscc,ocelot-switch"
> > +- reg: Must contain an (offset, length) pair of the register set for each
> > + entry in reg-names.
> > +- reg-names: Must include the following entries:
> > + - "sys"
> > + - "rew"
> > + - "qs"
> > + - "hsio"
> > + - "qsys"
> > + - "ana"
> > + - "portX" with X from 0 to the number of last port index available on that
> > + switch
> > +- interrupts: Should contain the switch interrupts for frame extraction and
> > + frame injection
> > +- interrupt-names: should contain the interrupt names: "xtr", "inj"
>
> You are not documenting the "ports" subnode(s).Please move the
> individual ports definition under a ports subnode, mainly for two reasons:
>
> - it makes it easy at the .dtsi level to have all ports disabled by default
>
> - this makes you strictly conforming to the DSA binding for Ethernet
> switches and this is good for consistency (both parsing code and just
> representation).
ports and port collide with the OF graph binding. It would be good if
this moved to ethernet-port(s) or similar.
Rob
^ permalink raw reply
* Re: [PATCH net-next 4/8] dt-bindings: net: add DT bindings for Microsemi Ocelot Switch
From: Rob Herring @ 2018-03-26 22:25 UTC (permalink / raw)
To: Alexandre Belloni
Cc: David S . Miller, Allan Nielsen, razvan.stefanescu, po.liu,
Thomas Petazzoni, Andrew Lunn, Florian Fainelli, netdev,
devicetree, linux-kernel, linux-mips
In-Reply-To: <20180323201117.8416-5-alexandre.belloni@bootlin.com>
On Fri, Mar 23, 2018 at 09:11:13PM +0100, Alexandre Belloni wrote:
> DT bindings for the Ethernet switch found on Microsemi Ocelot platforms.
>
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
> .../devicetree/bindings/net/mscc-ocelot.txt | 62 ++++++++++++++++++++++
> 1 file changed, 62 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/mscc-ocelot.txt
>
> diff --git a/Documentation/devicetree/bindings/net/mscc-ocelot.txt b/Documentation/devicetree/bindings/net/mscc-ocelot.txt
> new file mode 100644
> index 000000000000..ee092a85b5a0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/mscc-ocelot.txt
> @@ -0,0 +1,62 @@
> +Microsemi Ocelot network Switch
> +===============================
> +
> +The Microsemi Ocelot network switch can be found on Microsemi SoCs (VSC7513,
> +VSC7514)
What's the difference in these SoCs? You should probably have the
part#'s in the compatible string.
> +
> +Required properties:
> +- compatible: Should be "mscc,ocelot-switch"
> +- reg: Must contain an (offset, length) pair of the register set for each
> + entry in reg-names.
> +- reg-names: Must include the following entries:
> + - "sys"
> + - "rew"
> + - "qs"
> + - "hsio"
> + - "qsys"
> + - "ana"
> + - "portX" with X from 0 to the number of last port index available on that
> + switch
> +- interrupts: Should contain the switch interrupts for frame extraction and
> + frame injection
> +- interrupt-names: should contain the interrupt names: "xtr", "inj"
> +
> +Example:
> +
> + switch@1010000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "mscc,ocelot-switch";
> + reg = <0x1010000 0x10000>,
> + <0x1030000 0x10000>,
> + <0x1080000 0x100>,
> + <0x10d0000 0x10000>,
> + <0x11e0000 0x100>,
> + <0x11f0000 0x100>,
> + <0x1200000 0x100>,
> + <0x1210000 0x100>,
> + <0x1220000 0x100>,
> + <0x1230000 0x100>,
> + <0x1240000 0x100>,
> + <0x1250000 0x100>,
> + <0x1260000 0x100>,
> + <0x1270000 0x100>,
> + <0x1280000 0x100>,
> + <0x1800000 0x80000>,
> + <0x1880000 0x10000>;
> + reg-names = "sys", "rew", "qs", "hsio", "port0",
> + "port1", "port2", "port3", "port4", "port5",
> + "port6", "port7", "port8", "port9", "port10",
> + "qsys", "ana";
> + interrupts = <21 22>;
> + interrupt-names = "xtr", "inj";
> +
> + port0: port@0 {
> + reg = <0>;
> + phy-handle = <&phy0>;
> + };
> + port1: port@1 {
> + reg = <1>;
> + phy-handle = <&phy1>;
> + };
> + };
> --
> 2.16.2
>
^ 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