* Re: [net 1/1] net/mlx5: Fix build break when CONFIG_SMP=n
From: Guenter Roeck @ 2018-05-15 20:11 UTC (permalink / raw)
To: Saeed Mahameed; +Cc: David S. Miller, netdev, Randy Dunlap, Thomas Gleixner
In-Reply-To: <20180514223810.21197-1-saeedm@mellanox.com>
On Mon, May 14, 2018 at 03:38:10PM -0700, Saeed Mahameed wrote:
> Avoid using the kernel's irq_descriptor and return IRQ vector affinity
> directly from the driver.
>
> This fixes the following build break when CONFIG_SMP=n
>
> include/linux/mlx5/driver.h: In function ‘mlx5_get_vector_affinity_hint’:
> include/linux/mlx5/driver.h:1299:13: error:
> ‘struct irq_desc’ has no member named ‘affinity_hint’
>
> Fixes: 6082d9c9c94a ("net/mlx5: Fix mlx5_get_vector_affinity function")
> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> CC: Randy Dunlap <rdunlap@infradead.org>
> CC: Guenter Roeck <linux@roeck-us.net>
> CC: Thomas Gleixner <tglx@linutronix.de>
> Tested-by: Israel Rukshin <israelr@mellanox.com>
Tested-by: Guenter Roeck <linux@roeck-us.net>
> ---
>
> For -stable v4.14
>
> include/linux/mlx5/driver.h | 12 +-----------
> 1 file changed, 1 insertion(+), 11 deletions(-)
>
> diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h
> index 2a156c5dfadd..d703774982ca 100644
> --- a/include/linux/mlx5/driver.h
> +++ b/include/linux/mlx5/driver.h
> @@ -1286,17 +1286,7 @@ enum {
> static inline const struct cpumask *
> mlx5_get_vector_affinity_hint(struct mlx5_core_dev *dev, int vector)
> {
> - struct irq_desc *desc;
> - unsigned int irq;
> - int eqn;
> - int err;
> -
> - err = mlx5_vector2eqn(dev, vector, &eqn, &irq);
> - if (err)
> - return NULL;
> -
> - desc = irq_to_desc(irq);
> - return desc->affinity_hint;
> + return dev->priv.irq_info[vector].mask;
> }
>
> #endif /* MLX5_DRIVER_H */
> --
> 2.17.0
>
^ permalink raw reply
* i40e - Is i40e_force_link_state doing the right thing ?
From: Chaitanya Lala @ 2018-05-15 20:15 UTC (permalink / raw)
To: netdev, mariusz.stachura, mitch.a.williams, andrewx.bowers,
jeffrey.t.kirsher
Hi,
I am trying to bring up a Intel XL710 4x10G Intel card using the
latest mainline top-of-tree.
The problem is that "ifconfig up" and "ifconfig down" do not take
effect at the link state level.
I tracked the problem down to i40e_force_link_state() when it is
called from i40e_down().
It calls i40e_force_link_state with "is_up" == false. In-turn it
calls, i40e_aq_set_link_restart_an(hw, true, NULL).
Should the second argument of i40e_aq_set_link_restart_an be "is_up"
vs the current "true"
i.e. i40e_aq_set_link_restart_an(hw, is_up, NULL). ? When I make this
change, the link state syncs-up with
the interface administrative state.
Is this a bug ?
Thanks,
Chaitanya
^ permalink raw reply
* Re: [PATCH net-next 2/2] pfifo_fast: drop unneeded additional lock on dequeue
From: Michael S. Tsirkin @ 2018-05-15 20:17 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev, David S. Miller, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
John Fastabend
In-Reply-To: <8a1740148995663939837bedb14f29716c7cf6f5.1526392746.git.pabeni@redhat.com>
On Tue, May 15, 2018 at 04:24:37PM +0200, Paolo Abeni wrote:
> After the previous patch, for NOLOCK qdiscs, q->seqlock is
> always held when the dequeue() is invoked, we can drop
> any additional locking to protect such operation.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> include/linux/skb_array.h | 5 +++++
> net/sched/sch_generic.c | 4 ++--
> 2 files changed, 7 insertions(+), 2 deletions(-)
Is the seqlock taken during qdisc_change_tx_queue_len?
We need to prevent that racing with dequeue.
> diff --git a/include/linux/skb_array.h b/include/linux/skb_array.h
> index a6b6e8bb3d7b..62d9b0a6329f 100644
> --- a/include/linux/skb_array.h
> +++ b/include/linux/skb_array.h
> @@ -97,6 +97,11 @@ static inline bool skb_array_empty_any(struct skb_array *a)
> return ptr_ring_empty_any(&a->ring);
> }
>
> +static inline struct sk_buff *__skb_array_consume(struct skb_array *a)
> +{
> + return __ptr_ring_consume(&a->ring);
> +}
> +
> static inline struct sk_buff *skb_array_consume(struct skb_array *a)
> {
> return ptr_ring_consume(&a->ring);
> diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
> index a126f16bc30b..760ab1b09f8b 100644
> --- a/net/sched/sch_generic.c
> +++ b/net/sched/sch_generic.c
> @@ -656,7 +656,7 @@ static struct sk_buff *pfifo_fast_dequeue(struct Qdisc *qdisc)
> if (__skb_array_empty(q))
> continue;
>
> - skb = skb_array_consume_bh(q);
> + skb = __skb_array_consume(q);
> }
> if (likely(skb)) {
> qdisc_qstats_cpu_backlog_dec(qdisc, skb);
> @@ -697,7 +697,7 @@ static void pfifo_fast_reset(struct Qdisc *qdisc)
> if (!q->ring.queue)
> continue;
>
> - while ((skb = skb_array_consume_bh(q)) != NULL)
> + while ((skb = __skb_array_consume(q)) != NULL)
> kfree_skb(skb);
> }
>
> --
> 2.14.3
^ permalink raw reply
* Re: [RFC PATCH bpf-next 11/12] i40e: implement AF_XDP zero-copy support for Rx
From: Alexander Duyck @ 2018-05-15 20:25 UTC (permalink / raw)
To: Björn Töpel
Cc: magnus.karlsson, Karlsson, Magnus, Duyck, Alexander H,
John Fastabend, Alexei Starovoitov, Jesper Dangaard Brouer,
Willem de Bruijn, Daniel Borkmann, Michael S. Tsirkin, Netdev,
Björn Töpel, michael.lundkvist, Brandeburg, Jesse,
Anjali Singhai Jain, qi.z.zhang, intel-wired-lan
In-Reply-To: <20180515190615.23099-12-bjorn.topel@gmail.com>
On Tue, May 15, 2018 at 12:06 PM, Björn Töpel <bjorn.topel@gmail.com> wrote:
> From: Björn Töpel <bjorn.topel@intel.com>
>
> A lot of things here. First we add support for the new
> XDP_SETUP_XSK_UMEM command in ndo_bpf. This allows the AF_XDP socket
> to pass a UMEM to the driver. The driver will then DMA map all the
> frames in the UMEM for the driver. Next, the Rx code will allocate
> frames from the UMEM fill queue, instead of the regular page
> allocator.
>
> Externally, for the rest of the XDP code, the driver the driver
> internal UMEM allocator will appear as a MEM_TYPE_ZERO_COPY.
>
> Keep in mind that having frames coming from userland requires some
> extra care taken when passing them to the regular kernel stack. In
> these cases the ZC frame must be copied.
>
> The commit also introduces a completely new clean_rx_irq/allocator
> functions for zero-copy, and means (functions pointers) to set
> allocators and clean_rx functions.
>
> Finally, a lot of this are *not* implemented here. To mention some:
>
> * No passing to the stack via XDP_PASS (clone/copy to skb).
> * No XDP redirect to other than sockets (convert_to_xdp_frame does not
> clone the frame yet).
>
> And yes, too much C&P and too big commit. :-)
>
> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
A few minor comments below.
> ---
> drivers/net/ethernet/intel/i40e/i40e.h | 20 ++
> drivers/net/ethernet/intel/i40e/i40e_main.c | 202 +++++++++++++-
> drivers/net/ethernet/intel/i40e/i40e_txrx.c | 400 ++++++++++++++++++++++++++--
> drivers/net/ethernet/intel/i40e/i40e_txrx.h | 30 ++-
> 4 files changed, 619 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
> index 7a80652e2500..e6ee6c9bf094 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
> @@ -786,6 +786,12 @@ struct i40e_vsi {
>
> /* VSI specific handlers */
> irqreturn_t (*irq_handler)(int irq, void *data);
> +
> + /* AF_XDP zero-copy */
> + struct xdp_umem **xsk_umems;
> + u16 num_xsk_umems_used;
> + u16 num_xsk_umems;
> +
> } ____cacheline_internodealigned_in_smp;
>
> struct i40e_netdev_priv {
> @@ -1090,6 +1096,20 @@ static inline bool i40e_enabled_xdp_vsi(struct i40e_vsi *vsi)
> return !!vsi->xdp_prog;
> }
>
> +static inline struct xdp_umem *i40e_xsk_umem(struct i40e_ring *ring)
> +{
> + bool xdp_on = i40e_enabled_xdp_vsi(ring->vsi);
> + int qid = ring->queue_index;
> +
> + if (ring_is_xdp(ring))
> + qid -= ring->vsi->alloc_queue_pairs;
> +
> + if (!ring->vsi->xsk_umems || !ring->vsi->xsk_umems[qid] || !xdp_on)
> + return NULL;
> +
> + return ring->vsi->xsk_umems[qid];
> +}
> +
> int i40e_create_queue_channel(struct i40e_vsi *vsi, struct i40e_channel *ch);
> int i40e_set_bw_limit(struct i40e_vsi *vsi, u16 seid, u64 max_tx_rate);
> int i40e_add_del_cloud_filter(struct i40e_vsi *vsi,
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index b4c23cf3979c..dc3d668a741e 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -5,6 +5,7 @@
> #include <linux/of_net.h>
> #include <linux/pci.h>
> #include <linux/bpf.h>
> +#include <net/xdp_sock.h>
>
> /* Local includes */
> #include "i40e.h"
> @@ -3054,6 +3055,9 @@ static int i40e_configure_tx_ring(struct i40e_ring *ring)
> i40e_status err = 0;
> u32 qtx_ctl = 0;
>
> + if (ring_is_xdp(ring))
> + ring->xsk_umem = i40e_xsk_umem(ring);
> +
> /* some ATR related tx ring init */
> if (vsi->back->flags & I40E_FLAG_FD_ATR_ENABLED) {
> ring->atr_sample_rate = vsi->back->atr_sample_rate;
> @@ -3163,13 +3167,31 @@ static int i40e_configure_rx_ring(struct i40e_ring *ring)
> struct i40e_hw *hw = &vsi->back->hw;
> struct i40e_hmc_obj_rxq rx_ctx;
> i40e_status err = 0;
> + int ret;
>
> bitmap_zero(ring->state, __I40E_RING_STATE_NBITS);
>
> /* clear the context structure first */
> memset(&rx_ctx, 0, sizeof(rx_ctx));
>
> - ring->rx_buf_len = vsi->rx_buf_len;
> + ring->xsk_umem = i40e_xsk_umem(ring);
> + if (ring->xsk_umem) {
> + ring->clean_rx_irq = i40e_clean_rx_irq_zc;
> + ring->alloc_rx_buffers = i40e_alloc_rx_buffers_zc;
> + ring->rx_buf_len = ring->xsk_umem->props.frame_size -
> + ring->xsk_umem->frame_headroom -
> + XDP_PACKET_HEADROOM;
> + ring->zca.free = i40e_zca_free;
> + ret = xdp_rxq_info_reg_mem_model(&ring->xdp_rxq,
> + MEM_TYPE_ZERO_COPY,
> + &ring->zca);
> + if (ret)
> + return ret;
> + } else {
> + ring->clean_rx_irq = i40e_clean_rx_irq;
> + ring->alloc_rx_buffers = i40e_alloc_rx_buffers;
> + ring->rx_buf_len = vsi->rx_buf_len;
> + }
>
> rx_ctx.dbuff = DIV_ROUND_UP(ring->rx_buf_len,
> BIT_ULL(I40E_RXQ_CTX_DBUFF_SHIFT));
> @@ -3225,7 +3247,7 @@ static int i40e_configure_rx_ring(struct i40e_ring *ring)
> ring->tail = hw->hw_addr + I40E_QRX_TAIL(pf_q);
> writel(0, ring->tail);
>
> - i40e_alloc_rx_buffers(ring, I40E_DESC_UNUSED(ring));
> + ring->alloc_rx_buffers(ring, I40E_DESC_UNUSED(ring));
>
> return 0;
> }
> @@ -12050,6 +12072,179 @@ static int i40e_queue_pair_enable(struct i40e_vsi *vsi, int queue_pair)
> return err;
> }
>
> +static int i40e_alloc_xsk_umems(struct i40e_vsi *vsi)
> +{
> + if (vsi->xsk_umems)
> + return 0;
> +
> + vsi->num_xsk_umems_used = 0;
> + vsi->num_xsk_umems = vsi->alloc_queue_pairs;
> + vsi->xsk_umems = kcalloc(vsi->num_xsk_umems, sizeof(*vsi->xsk_umems),
> + GFP_KERNEL);
> + if (!vsi->xsk_umems) {
> + vsi->num_xsk_umems = 0;
> + return -ENOMEM;
> + }
> +
> + return 0;
> +}
> +
> +static int i40e_add_xsk_umem(struct i40e_vsi *vsi, struct xdp_umem *umem,
> + u16 qid)
> +{
> + int err;
> +
> + err = i40e_alloc_xsk_umems(vsi);
> + if (err)
> + return err;
> +
> + vsi->xsk_umems[qid] = umem;
> + vsi->num_xsk_umems_used++;
> +
> + return 0;
> +}
> +
> +static void i40e_remove_xsk_umem(struct i40e_vsi *vsi, u16 qid)
> +{
> + vsi->xsk_umems[qid] = NULL;
> + vsi->num_xsk_umems_used--;
> +
> + if (vsi->num_xsk_umems == 0) {
> + kfree(vsi->xsk_umems);
> + vsi->xsk_umems = NULL;
> + vsi->num_xsk_umems = 0;
> + }
> +}
> +
> +static int i40e_xsk_umem_dma_map(struct i40e_vsi *vsi, struct xdp_umem *umem)
> +{
> + struct i40e_pf *pf = vsi->back;
> + struct device *dev;
> + unsigned int i, j;
> + dma_addr_t dma;
> +
> + dev = &pf->pdev->dev;
> +
> + for (i = 0; i < umem->props.nframes; i++) {
> + dma = dma_map_single_attrs(dev, umem->frames[i].addr,
> + umem->props.frame_size,
> + DMA_BIDIRECTIONAL, I40E_RX_DMA_ATTR);
> + if (dma_mapping_error(dev, dma))
> + goto out_unmap;
> +
> + umem->frames[i].dma = dma;
> + }
> +
> + return 0;
> +
> +out_unmap:
> + for (j = 0; j < i; j++) {
> + dma_unmap_single_attrs(dev, umem->frames[i].dma,
> + umem->props.frame_size,
> + DMA_BIDIRECTIONAL,
> + I40E_RX_DMA_ATTR);
> + umem->frames[i].dma = 0;
> + }
> +
> + return -1;
> +}
> +
> +static void i40e_xsk_umem_dma_unmap(struct i40e_vsi *vsi, struct xdp_umem *umem)
> +{
> + struct i40e_pf *pf = vsi->back;
> + struct device *dev;
> + unsigned int i;
> +
> + dev = &pf->pdev->dev;
> +
> + for (i = 0; i < umem->props.nframes; i++) {
> + dma_unmap_single_attrs(dev, umem->frames[i].dma,
> + umem->props.frame_size,
> + DMA_BIDIRECTIONAL,
> + I40E_RX_DMA_ATTR);
> +
> + umem->frames[i].dma = 0;
> + }
> +}
> +
> +static int i40e_xsk_umem_enable(struct i40e_vsi *vsi, struct xdp_umem *umem,
> + u16 qid)
> +{
> + bool if_running;
> + int err;
> +
> + if (vsi->type != I40E_VSI_MAIN)
> + return -EINVAL;
> +
> + if (qid >= vsi->num_queue_pairs)
> + return -EINVAL;
> +
> + if (vsi->xsk_umems && vsi->xsk_umems[qid])
> + return -EBUSY;
> +
> + err = i40e_xsk_umem_dma_map(vsi, umem);
> + if (err)
> + return err;
> +
> + if_running = netif_running(vsi->netdev) && i40e_enabled_xdp_vsi(vsi);
> +
> + if (if_running) {
> + err = i40e_queue_pair_disable(vsi, qid);
> + if (err)
> + return err;
> + }
> +
> + err = i40e_add_xsk_umem(vsi, umem, qid);
> + if (err)
> + return err;
> +
> + if (if_running) {
> + err = i40e_queue_pair_enable(vsi, qid);
> + if (err)
> + return err;
> + }
> +
> + return 0;
> +}
> +
> +static int i40e_xsk_umem_disable(struct i40e_vsi *vsi, u16 qid)
> +{
> + bool if_running;
> + int err;
> +
> + if (!vsi->xsk_umems || qid >= vsi->num_xsk_umems ||
> + !vsi->xsk_umems[qid])
> + return -EINVAL;
> +
> + if_running = netif_running(vsi->netdev) && i40e_enabled_xdp_vsi(vsi);
> +
> + if (if_running) {
> + err = i40e_queue_pair_disable(vsi, qid);
> + if (err)
> + return err;
> + }
> +
> + i40e_xsk_umem_dma_unmap(vsi, vsi->xsk_umems[qid]);
> + i40e_remove_xsk_umem(vsi, qid);
> +
> + if (if_running) {
> + err = i40e_queue_pair_enable(vsi, qid);
> + if (err)
> + return err;
> + }
> +
> + return 0;
> +}
> +
> +static int i40e_xsk_umem_setup(struct i40e_vsi *vsi, struct xdp_umem *umem,
> + u16 qid)
> +{
> + if (umem)
> + return i40e_xsk_umem_enable(vsi, umem, qid);
> +
> + return i40e_xsk_umem_disable(vsi, qid);
> +}
> +
> /**
> * i40e_xdp - implements ndo_bpf for i40e
> * @dev: netdevice
> @@ -12071,6 +12266,9 @@ static int i40e_xdp(struct net_device *dev,
> xdp->prog_attached = i40e_enabled_xdp_vsi(vsi);
> xdp->prog_id = vsi->xdp_prog ? vsi->xdp_prog->aux->id : 0;
> return 0;
> + case XDP_SETUP_XSK_UMEM:
> + return i40e_xsk_umem_setup(vsi, xdp->xsk.umem,
> + xdp->xsk.queue_id);
> default:
> return -EINVAL;
> }
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> index 5efa68de935b..f89ac524652c 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> @@ -5,6 +5,7 @@
> #include <net/busy_poll.h>
> #include <linux/bpf_trace.h>
> #include <net/xdp.h>
> +#include <net/xdp_sock.h>
> #include "i40e.h"
> #include "i40e_trace.h"
> #include "i40e_prototype.h"
> @@ -1373,31 +1374,35 @@ void i40e_clean_rx_ring(struct i40e_ring *rx_ring)
> }
>
> /* Free all the Rx ring sk_buffs */
> - for (i = 0; i < rx_ring->count; i++) {
> - struct i40e_rx_buffer *rx_bi = &rx_ring->rx_bi[i];
> + if (!rx_ring->xsk_umem) {
> + for (i = 0; i < rx_ring->count; i++) {
I'm not a fan of all this extra indenting. This could be much more
easily handled with just a goto and a label.
> + struct i40e_rx_buffer *rx_bi = &rx_ring->rx_bi[i];
>
> - if (!rx_bi->page)
> - continue;
> -
> - /* Invalidate cache lines that may have been written to by
> - * device so that we avoid corrupting memory.
> - */
> - dma_sync_single_range_for_cpu(rx_ring->dev,
> - rx_bi->dma,
> - rx_bi->page_offset,
> - rx_ring->rx_buf_len,
> - DMA_FROM_DEVICE);
> -
> - /* free resources associated with mapping */
> - dma_unmap_page_attrs(rx_ring->dev, rx_bi->dma,
> - i40e_rx_pg_size(rx_ring),
> - DMA_FROM_DEVICE,
> - I40E_RX_DMA_ATTR);
> -
> - __page_frag_cache_drain(rx_bi->page, rx_bi->pagecnt_bias);
> + if (!rx_bi->page)
> + continue;
>
> - rx_bi->page = NULL;
> - rx_bi->page_offset = 0;
> + /* Invalidate cache lines that may have been
> + * written to by device so that we avoid
> + * corrupting memory.
> + */
> + dma_sync_single_range_for_cpu(rx_ring->dev,
> + rx_bi->dma,
> + rx_bi->page_offset,
> + rx_ring->rx_buf_len,
> + DMA_FROM_DEVICE);
> +
> + /* free resources associated with mapping */
> + dma_unmap_page_attrs(rx_ring->dev, rx_bi->dma,
> + i40e_rx_pg_size(rx_ring),
> + DMA_FROM_DEVICE,
> + I40E_RX_DMA_ATTR);
> +
> + __page_frag_cache_drain(rx_bi->page,
> + rx_bi->pagecnt_bias);
> +
> + rx_bi->page = NULL;
> + rx_bi->page_offset = 0;
> + }
> }
>
> bi_size = sizeof(struct i40e_rx_buffer) * rx_ring->count;
> @@ -2214,8 +2219,6 @@ static struct sk_buff *i40e_run_xdp(struct i40e_ring *rx_ring,
> if (!xdp_prog)
> goto xdp_out;
>
> - prefetchw(xdp->data_hard_start); /* xdp_frame write */
> -
> act = bpf_prog_run_xdp(xdp_prog, xdp);
> switch (act) {
> case XDP_PASS:
> @@ -2284,7 +2287,7 @@ static inline void i40e_xdp_ring_update_tail(struct i40e_ring *xdp_ring)
> *
> * Returns amount of work completed
> **/
> -static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
> +int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
> {
> unsigned int total_rx_bytes = 0, total_rx_packets = 0;
> struct sk_buff *skb = rx_ring->skb;
> @@ -2426,6 +2429,349 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
> return failure ? budget : (int)total_rx_packets;
> }
>
How much of the code below is actually reused anywhere else? I would
almost be inclined to say that maybe the zero-copy path should be
moved to a new file since so much of this is being duplicated from the
original tx/rx code path. I can easily see this becoming confusing as
to which is which when a bug gets found and needs to be fixed.
> +static struct sk_buff *i40e_run_xdp_zc(struct i40e_ring *rx_ring,
> + struct xdp_buff *xdp)
> +{
> + int err, result = I40E_XDP_PASS;
> + struct i40e_ring *xdp_ring;
> + struct bpf_prog *xdp_prog;
> + u32 act;
> +
> + rcu_read_lock();
> + xdp_prog = READ_ONCE(rx_ring->xdp_prog);
> +
> + act = bpf_prog_run_xdp(xdp_prog, xdp);
> + switch (act) {
> + case XDP_PASS:
> + break;
> + case XDP_TX:
> + xdp_ring = rx_ring->vsi->xdp_rings[rx_ring->queue_index];
> + result = i40e_xmit_xdp_tx_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:
> + trace_xdp_exception(rx_ring->netdev, xdp_prog, act);
> + /* fallthrough -- handle aborts by dropping packet */
> + case XDP_DROP:
> + result = I40E_XDP_CONSUMED;
> + break;
> + }
> +
> + rcu_read_unlock();
> + return ERR_PTR(-result);
> +}
> +
> +static bool i40e_alloc_frame_zc(struct i40e_ring *rx_ring,
> + struct i40e_rx_buffer *bi)
> +{
> + struct xdp_umem *umem = rx_ring->xsk_umem;
> + void *addr = bi->addr;
> + u32 *id;
> +
> + if (addr) {
> + rx_ring->rx_stats.page_reuse_count++;
> + return true;
> + }
> +
> + id = xsk_umem_peek_id(umem);
> + if (unlikely(!id)) {
> + rx_ring->rx_stats.alloc_page_failed++;
> + return false;
> + }
> +
> + bi->dma = umem->frames[*id].dma + umem->frame_headroom +
> + XDP_PACKET_HEADROOM;
> + bi->addr = umem->frames[*id].addr + umem->frame_headroom +
> + XDP_PACKET_HEADROOM;
> + bi->id = *id;
> +
> + xsk_umem_discard_id(umem);
> + return true;
> +}
> +
> +bool i40e_alloc_rx_buffers_zc(struct i40e_ring *rx_ring, u16 cleaned_count)
> +{
> + u16 ntu = rx_ring->next_to_use;
> + union i40e_rx_desc *rx_desc;
> + struct i40e_rx_buffer *bi;
> +
> + rx_desc = I40E_RX_DESC(rx_ring, ntu);
> + bi = &rx_ring->rx_bi[ntu];
> +
> + do {
> + if (!i40e_alloc_frame_zc(rx_ring, bi))
> + goto no_buffers;
> +
> + /* sync the buffer for use by the device */
> + dma_sync_single_range_for_device(rx_ring->dev, bi->dma, 0,
> + rx_ring->rx_buf_len,
> + DMA_BIDIRECTIONAL);
> +
> + /* Refresh the desc even if buffer_addrs didn't change
> + * because each write-back erases this info.
> + */
> + rx_desc->read.pkt_addr = cpu_to_le64(bi->dma);
> +
> + rx_desc++;
> + bi++;
> + ntu++;
> + if (unlikely(ntu == rx_ring->count)) {
> + rx_desc = I40E_RX_DESC(rx_ring, 0);
> + bi = rx_ring->rx_bi;
> + ntu = 0;
> + }
> +
> + /* clear the status bits for the next_to_use descriptor */
> + rx_desc->wb.qword1.status_error_len = 0;
> +
> + cleaned_count--;
> + } while (cleaned_count);
> +
> + if (rx_ring->next_to_use != ntu)
> + i40e_release_rx_desc(rx_ring, ntu);
> +
> + return false;
> +
> +no_buffers:
> + if (rx_ring->next_to_use != ntu)
> + i40e_release_rx_desc(rx_ring, ntu);
> +
> + /* make sure to come back via polling to try again after
> + * allocation failure
> + */
> + return true;
> +}
> +
> +static struct i40e_rx_buffer *i40e_get_rx_buffer_zc(struct i40e_ring *rx_ring,
> + const unsigned int size)
> +{
> + struct i40e_rx_buffer *rx_buffer;
> +
> + rx_buffer = &rx_ring->rx_bi[rx_ring->next_to_clean];
> +
> + /* we are reusing so sync this buffer for CPU use */
> + dma_sync_single_range_for_cpu(rx_ring->dev,
> + rx_buffer->dma, 0,
> + size,
> + DMA_BIDIRECTIONAL);
> +
> + return rx_buffer;
> +}
> +
> +static void i40e_reuse_rx_buffer_zc(struct i40e_ring *rx_ring,
> + struct i40e_rx_buffer *old_buff)
> +{
> + struct i40e_rx_buffer *new_buff;
> + u16 nta = rx_ring->next_to_alloc;
> +
> + new_buff = &rx_ring->rx_bi[nta];
> +
> + /* update, and store next to alloc */
> + nta++;
> + rx_ring->next_to_alloc = (nta < rx_ring->count) ? nta : 0;
> +
> + /* transfer page from old buffer to new buffer */
> + new_buff->dma = old_buff->dma;
> + new_buff->addr = old_buff->addr;
> + new_buff->id = old_buff->id;
> +}
> +
> +/* Called from the XDP return API in NAPI context. */
> +void i40e_zca_free(struct zero_copy_allocator *alloc, unsigned long handle)
> +{
> + struct i40e_rx_buffer *new_buff;
> + struct i40e_ring *rx_ring;
> + u16 nta;
> +
> + rx_ring = container_of(alloc, struct i40e_ring, zca);
> + nta = rx_ring->next_to_alloc;
> +
> + new_buff = &rx_ring->rx_bi[nta];
> +
> + /* update, and store next to alloc */
> + nta++;
> + rx_ring->next_to_alloc = (nta < rx_ring->count) ? nta : 0;
> +
> + new_buff->dma = rx_ring->xsk_umem->frames[handle].dma;
> + new_buff->addr = rx_ring->xsk_umem->frames[handle].addr;
> + new_buff->id = (u32)handle;
> +}
> +
> +static struct sk_buff *i40e_zc_frame_to_skb(struct i40e_ring *rx_ring,
> + struct i40e_rx_buffer *rx_buffer,
> + struct xdp_buff *xdp)
> +{
> + // XXX implement alloc skb and copy
> + i40e_reuse_rx_buffer_zc(rx_ring, rx_buffer);
> + return NULL;
> +}
> +
> +static void i40e_clean_programming_status_zc(struct i40e_ring *rx_ring,
> + union i40e_rx_desc *rx_desc,
> + u64 qw)
> +{
> + struct i40e_rx_buffer *rx_buffer;
> + u32 ntc = rx_ring->next_to_clean;
> + u8 id;
> +
> + /* fetch, update, and store next to clean */
> + rx_buffer = &rx_ring->rx_bi[ntc++];
> + ntc = (ntc < rx_ring->count) ? ntc : 0;
> + rx_ring->next_to_clean = ntc;
> +
> + prefetch(I40E_RX_DESC(rx_ring, ntc));
> +
> + /* place unused page back on the ring */
> + i40e_reuse_rx_buffer_zc(rx_ring, rx_buffer);
> + rx_ring->rx_stats.page_reuse_count++;
> +
> + /* clear contents of buffer_info */
> + rx_buffer->addr = NULL;
> +
> + id = (qw & I40E_RX_PROG_STATUS_DESC_QW1_PROGID_MASK) >>
> + I40E_RX_PROG_STATUS_DESC_QW1_PROGID_SHIFT;
> +
> + if (id == I40E_RX_PROG_STATUS_DESC_FD_FILTER_STATUS)
> + i40e_fd_handle_status(rx_ring, rx_desc, id);
> +}
> +
> +int i40e_clean_rx_irq_zc(struct i40e_ring *rx_ring, int budget)
> +{
> + unsigned int total_rx_bytes = 0, total_rx_packets = 0;
> + u16 cleaned_count = I40E_DESC_UNUSED(rx_ring);
> + bool failure = false, xdp_xmit = false;
> + struct sk_buff *skb;
> + struct xdp_buff xdp;
> +
> + xdp.rxq = &rx_ring->xdp_rxq;
> +
> + while (likely(total_rx_packets < (unsigned int)budget)) {
> + struct i40e_rx_buffer *rx_buffer;
> + union i40e_rx_desc *rx_desc;
> + unsigned int size;
> + u16 vlan_tag;
> + u8 rx_ptype;
> + u64 qword;
> + u32 ntc;
> +
> + /* return some buffers to hardware, one at a time is too slow */
> + if (cleaned_count >= I40E_RX_BUFFER_WRITE) {
> + failure = failure ||
> + i40e_alloc_rx_buffers_zc(rx_ring,
> + cleaned_count);
> + cleaned_count = 0;
> + }
> +
> + rx_desc = I40E_RX_DESC(rx_ring, rx_ring->next_to_clean);
> +
> + /* status_error_len will always be zero for unused descriptors
> + * because it's cleared in cleanup, and overlaps with hdr_addr
> + * which is always zero because packet split isn't used, if the
> + * hardware wrote DD then the length will be non-zero
> + */
> + qword = le64_to_cpu(rx_desc->wb.qword1.status_error_len);
> +
> + /* This memory barrier is needed to keep us from reading
> + * any other fields out of the rx_desc until we have
> + * verified the descriptor has been written back.
> + */
> + dma_rmb();
> +
> + if (unlikely(i40e_rx_is_programming_status(qword))) {
> + i40e_clean_programming_status_zc(rx_ring, rx_desc,
> + qword);
> + cleaned_count++;
> + continue;
> + }
> + size = (qword & I40E_RXD_QW1_LENGTH_PBUF_MASK) >>
> + I40E_RXD_QW1_LENGTH_PBUF_SHIFT;
> + if (!size)
> + break;
> +
> + rx_buffer = i40e_get_rx_buffer_zc(rx_ring, size);
> +
> + /* retrieve a buffer from the ring */
> + xdp.data = rx_buffer->addr;
> + xdp_set_data_meta_invalid(&xdp);
> + xdp.data_hard_start = xdp.data - XDP_PACKET_HEADROOM;
> + xdp.data_end = xdp.data + size;
> + xdp.handle = rx_buffer->id;
> +
> + skb = i40e_run_xdp_zc(rx_ring, &xdp);
> +
> + if (IS_ERR(skb)) {
> + if (PTR_ERR(skb) == -I40E_XDP_TX)
> + xdp_xmit = true;
> + else
> + i40e_reuse_rx_buffer_zc(rx_ring, rx_buffer);
> + total_rx_bytes += size;
> + total_rx_packets++;
> + } else {
> + skb = i40e_zc_frame_to_skb(rx_ring, rx_buffer, &xdp);
> + if (!skb) {
> + rx_ring->rx_stats.alloc_buff_failed++;
> + break;
> + }
> + }
> +
> + rx_buffer->addr = NULL;
> + cleaned_count++;
> +
> + /* don't care about non-EOP frames in XDP mode */
> + ntc = rx_ring->next_to_clean + 1;
> + ntc = (ntc < rx_ring->count) ? ntc : 0;
> + rx_ring->next_to_clean = ntc;
> + prefetch(I40E_RX_DESC(rx_ring, ntc));
> +
> + if (i40e_cleanup_headers(rx_ring, skb, rx_desc)) {
> + skb = NULL;
> + continue;
> + }
> +
> + /* probably a little skewed due to removing CRC */
> + total_rx_bytes += skb->len;
> +
> + qword = le64_to_cpu(rx_desc->wb.qword1.status_error_len);
> + rx_ptype = (qword & I40E_RXD_QW1_PTYPE_MASK) >>
> + I40E_RXD_QW1_PTYPE_SHIFT;
> +
> + /* populate checksum, VLAN, and protocol */
> + i40e_process_skb_fields(rx_ring, rx_desc, skb, rx_ptype);
> +
> + vlan_tag = (qword & BIT(I40E_RX_DESC_STATUS_L2TAG1P_SHIFT)) ?
> + le16_to_cpu(rx_desc->wb.qword0.lo_dword.l2tag1) : 0;
> +
> + i40e_receive_skb(rx_ring, skb, vlan_tag);
> + skb = NULL;
> +
> + /* update budget accounting */
> + total_rx_packets++;
> + }
> +
> + if (xdp_xmit) {
> + struct i40e_ring *xdp_ring =
> + rx_ring->vsi->xdp_rings[rx_ring->queue_index];
> +
> + i40e_xdp_ring_update_tail(xdp_ring);
> + xdp_do_flush_map();
> + }
> +
> + u64_stats_update_begin(&rx_ring->syncp);
> + rx_ring->stats.packets += total_rx_packets;
> + rx_ring->stats.bytes += total_rx_bytes;
> + u64_stats_update_end(&rx_ring->syncp);
> + rx_ring->q_vector->rx.total_packets += total_rx_packets;
> + rx_ring->q_vector->rx.total_bytes += total_rx_bytes;
> +
> + /* guarantee a trip back through this routine if there was a failure */
> + return failure ? budget : (int)total_rx_packets;
> +}
> +
> static inline u32 i40e_buildreg_itr(const int type, u16 itr)
> {
> u32 val;
> @@ -2576,7 +2922,7 @@ int i40e_napi_poll(struct napi_struct *napi, int budget)
> budget_per_ring = max(budget/q_vector->num_ringpairs, 1);
>
> i40e_for_each_ring(ring, q_vector->rx) {
> - int cleaned = i40e_clean_rx_irq(ring, budget_per_ring);
> + int cleaned = ring->clean_rx_irq(ring, budget_per_ring);
>
> work_done += cleaned;
> /* if we clean as many as budgeted, we must not be done */
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.h b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
> index fdd2c55f03a6..9d5d9862e9f1 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
> @@ -296,13 +296,22 @@ struct i40e_tx_buffer {
>
> struct i40e_rx_buffer {
> dma_addr_t dma;
> - struct page *page;
> + union {
> + struct {
> + struct page *page;
> #if (BITS_PER_LONG > 32) || (PAGE_SIZE >= 65536)
> - __u32 page_offset;
> + __u32 page_offset;
> #else
> - __u16 page_offset;
> + __u16 page_offset;
> #endif
> - __u16 pagecnt_bias;
> + __u16 pagecnt_bias;
> + };
> + struct {
> + /* for umem */
> + void *addr;
> + u32 id;
> + };
> + };
> };
>
> struct i40e_queue_stats {
> @@ -344,6 +353,8 @@ enum i40e_ring_state_t {
> #define I40E_RX_SPLIT_TCP_UDP 0x4
> #define I40E_RX_SPLIT_SCTP 0x8
>
> +void i40e_zc_recycle(struct zero_copy_allocator *alloc, unsigned long handle);
> +
> /* struct that defines a descriptor ring, associated with a VSI */
> struct i40e_ring {
> struct i40e_ring *next; /* pointer to next ring in q_vector */
> @@ -414,6 +425,12 @@ struct i40e_ring {
>
> struct i40e_channel *ch;
> struct xdp_rxq_info xdp_rxq;
> +
> + int (*clean_rx_irq)(struct i40e_ring *, int);
> + bool (*alloc_rx_buffers)(struct i40e_ring *, u16);
> + struct xdp_umem *xsk_umem;
> +
> + struct zero_copy_allocator zca; /* ZC allocator anchor */
> } ____cacheline_internodealigned_in_smp;
>
> static inline bool ring_uses_build_skb(struct i40e_ring *ring)
> @@ -474,6 +491,7 @@ static inline unsigned int i40e_rx_pg_order(struct i40e_ring *ring)
> #define i40e_rx_pg_size(_ring) (PAGE_SIZE << i40e_rx_pg_order(_ring))
>
> bool i40e_alloc_rx_buffers(struct i40e_ring *rxr, u16 cleaned_count);
> +bool i40e_alloc_rx_buffers_zc(struct i40e_ring *rx_ring, u16 cleaned_count);
> netdev_tx_t i40e_lan_xmit_frame(struct sk_buff *skb, struct net_device *netdev);
> void i40e_clean_tx_ring(struct i40e_ring *tx_ring);
> void i40e_clean_rx_ring(struct i40e_ring *rx_ring);
> @@ -489,6 +507,9 @@ 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_frame *xdpf);
> void i40e_xdp_flush(struct net_device *dev);
> +int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget);
> +int i40e_clean_rx_irq_zc(struct i40e_ring *rx_ring, int budget);
> +void i40e_zca_free(struct zero_copy_allocator *alloc, unsigned long handle);
>
> /**
> * i40e_get_head - Retrieve head from head writeback
> @@ -575,4 +596,5 @@ static inline struct netdev_queue *txring_txq(const struct i40e_ring *ring)
> {
> return netdev_get_tx_queue(ring->netdev, ring->queue_index);
> }
> +
> #endif /* _I40E_TXRX_H_ */
> --
> 2.14.1
>
^ permalink raw reply
* RE: [PATCH v2 iproute2-next 1/3] rdma: update rdma_netlink.h to get new driver attributes
From: Steve Wise @ 2018-05-15 20:25 UTC (permalink / raw)
To: 'Doug Ledford', 'David Ahern', leon
Cc: stephen, netdev, linux-rdma
In-Reply-To: <eb6edd23b76a176b876002cc19c546a0669cf66f.camel@redhat.com>
> On Tue, 2018-05-15 at 13:37 -0600, David Ahern wrote:
> > On 5/14/18 9:42 AM, Steve Wise wrote:
> > > diff --git a/rdma/include/uapi/rdma/rdma_netlink.h
> b/rdma/include/uapi/rdma/rdma_netlink.h
> > > index 60416ed..40be0d8 100644
> > > --- a/rdma/include/uapi/rdma/rdma_netlink.h
> > > +++ b/rdma/include/uapi/rdma/rdma_netlink.h
> > > @@ -249,10 +249,22 @@ enum rdma_nldev_command {
> > > RDMA_NLDEV_NUM_OPS
> > > };
> > >
> > > +enum {
> > > + RDMA_NLDEV_ATTR_ENTRY_STRLEN = 16,
> > > +};
> > > +
> > > +enum rdma_nldev_print_type {
> > > + RDMA_NLDEV_PRINT_TYPE_UNSPEC,
> > > + RDMA_NLDEV_PRINT_TYPE_HEX,
> > > +};
> > > +
> > > enum rdma_nldev_attr {
> > > /* don't change the order or add anything between, this is ABI! */
> >
> > I asked this before and did not get a response. As the comment above
> > states with an emphasis (!) ...
Sorry David, I missed your question previously. ☹
> >
> > > RDMA_NLDEV_ATTR_UNSPEC,
> > >
> > > + /* Pad attribute for 64b alignment */
> > > + RDMA_NLDEV_ATTR_PAD = RDMA_NLDEV_ATTR_UNSPEC,
> > > +
> >
> > ... are you really adding new attributes in the middle?
>
> Not really. The new item is being explicitly set to the same value as
> the item above it. It therefore becomes two entries with the same enum
> value. The rest of the enum is all unchanged.
Correct.
The reason this was done was because a kernel had already been released where 64b nlattrs were being padded with 0 instead of defining an explicit pad attribute. Jason thought the kernel side should define an explicit PAD attribute and use it. To preserve the ABI we defined it but set it to 0 (aka ATTR_UNSPEC aka the first in the enum).
>
> > > /* Identifier for ib_device */
> > > RDMA_NLDEV_ATTR_DEV_INDEX, /* u32 */
> > >
> > > @@ -387,6 +399,20 @@ enum rdma_nldev_attr {
> > > RDMA_NLDEV_ATTR_RES_PD_ENTRY, /* nested table */
> > > RDMA_NLDEV_ATTR_RES_LOCAL_DMA_LKEY, /* u32 */
> > > RDMA_NLDEV_ATTR_RES_UNSAFE_GLOBAL_RKEY, /* u32 */
> > > + /*
> > > + * driver-specific attributes.
> > > + */
> > > + RDMA_NLDEV_ATTR_DRIVER, /* nested table */
> > > + RDMA_NLDEV_ATTR_DRIVER_ENTRY, /* nested table */
> > > + RDMA_NLDEV_ATTR_DRIVER_STRING, /* string */
> > > + /*
> > > + * u8 values from enum rdma_nldev_print_type
> > > + */
> > > + RDMA_NLDEV_ATTR_DRIVER_PRINT_TYPE, /* u8 */
> > > + RDMA_NLDEV_ATTR_DRIVER_S32, /* s32 */
> > > + RDMA_NLDEV_ATTR_DRIVER_U32, /* u32 */
> > > + RDMA_NLDEV_ATTR_DRIVER_S64, /* s64 */
> > > + RDMA_NLDEV_ATTR_DRIVER_U64, /* u64 */
> >
> > and again here.
> >
Ugh, this looks like a mistake maybe due to me rebasing and not noticing this commit added the name/index attrs.
5b2cc79de878 leonro@mellanox.com RDMA/nldev: Provide netdevice name and index
Both of these are in -next to be merged upstream together.
Should I do anything?
^ permalink raw reply
* RE: [PATCH v2 iproute2-next 1/3] rdma: update rdma_netlink.h to get new driver attributes
From: Steve Wise @ 2018-05-15 20:28 UTC (permalink / raw)
To: 'Doug Ledford', 'David Ahern', leon
Cc: stephen, netdev, linux-rdma
In-Reply-To: <39cd4caf34da44885021d8646699889ced482008.camel@redhat.com>
> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org <linux-rdma-
> owner@vger.kernel.org> On Behalf Of Doug Ledford
> Sent: Tuesday, May 15, 2018 2:53 PM
> To: David Ahern <dsahern@gmail.com>; Steve Wise
> <swise@opengridcomputing.com>; leon@kernel.org
> Cc: stephen@networkplumber.org; netdev@vger.kernel.org; linux-
> rdma@vger.kernel.org
> Subject: Re: [PATCH v2 iproute2-next 1/3] rdma: update rdma_netlink.h to
> get new driver attributes
>
> On Tue, 2018-05-15 at 13:37 -0600, David Ahern wrote:
> > On 5/14/18 9:42 AM, Steve Wise wrote:
> > > diff --git a/rdma/include/uapi/rdma/rdma_netlink.h
> b/rdma/include/uapi/rdma/rdma_netlink.h
> > > index 60416ed..40be0d8 100644
> > > --- a/rdma/include/uapi/rdma/rdma_netlink.h
> > > +++ b/rdma/include/uapi/rdma/rdma_netlink.h
> > >
> > > @@ -387,6 +399,20 @@ enum rdma_nldev_attr {
> > > RDMA_NLDEV_ATTR_RES_PD_ENTRY, /* nested table */
> > > RDMA_NLDEV_ATTR_RES_LOCAL_DMA_LKEY, /* u32 */
> > > RDMA_NLDEV_ATTR_RES_UNSAFE_GLOBAL_RKEY, /* u32 */
> > > + /*
> > > + * driver-specific attributes.
> > > + */
> > > + RDMA_NLDEV_ATTR_DRIVER, /* nested table */
> > > + RDMA_NLDEV_ATTR_DRIVER_ENTRY, /* nested table */
> > > + RDMA_NLDEV_ATTR_DRIVER_STRING, /* string */
> > > + /*
> > > + * u8 values from enum rdma_nldev_print_type
> > > + */
> > > + RDMA_NLDEV_ATTR_DRIVER_PRINT_TYPE, /* u8 */
> > > + RDMA_NLDEV_ATTR_DRIVER_S32, /* s32 */
> > > + RDMA_NLDEV_ATTR_DRIVER_U32, /* u32 */
> > > + RDMA_NLDEV_ATTR_DRIVER_S64, /* s64 */
> > > + RDMA_NLDEV_ATTR_DRIVER_U64, /* u64 */
> >
> > and again here.
>
> This chunk, however, is a problem. We'll need to fix that in the kernel
> and in this patch too.
I'll fix this series once I fix the kernel side. Doug, should I send a patch that basically moves the DRIVER attributes to the bottom?
Sorry about missing this!
Steve
^ permalink raw reply
* RE: [PATCH v2 iproute2-next 1/3] rdma: update rdma_netlink.h to get new driver attributes
From: Steve Wise @ 2018-05-15 20:29 UTC (permalink / raw)
To: 'David Ahern', 'Doug Ledford', leon
Cc: stephen, netdev, linux-rdma
In-Reply-To: <fd64ce39-a284-b09f-85b0-8cd7ba0d1207@gmail.com>
> -----Original Message-----
> From: David Ahern <dsahern@gmail.com>
> Sent: Tuesday, May 15, 2018 3:02 PM
> To: Doug Ledford <dledford@redhat.com>; Steve Wise
> <swise@opengridcomputing.com>; leon@kernel.org
> Cc: stephen@networkplumber.org; netdev@vger.kernel.org; linux-
> rdma@vger.kernel.org
> Subject: Re: [PATCH v2 iproute2-next 1/3] rdma: update rdma_netlink.h to
> get new driver attributes
>
> On 5/15/18 2:00 PM, Doug Ledford wrote:
> > I just sent an incremental fix to the list under separate cover. You
> > can squash that fix into Steve's patch and it should resolve the issue.
> > Or Steve can respin the set. Either way.
>
> Once the patch has been committed to the upstream repo, spin a new set
> with the correct header update.
Will do!
Steve.
^ permalink raw reply
* Re: i40e - Is i40e_force_link_state doing the right thing ?
From: Alexander Duyck @ 2018-05-15 20:31 UTC (permalink / raw)
To: Chaitanya Lala, intel-wired-lan
Cc: Netdev, mariusz.stachura, Mitch Williams, Bowers, AndrewX,
Jeff Kirsher
In-Reply-To: <CABOGejK8bnwU0iqO5-GRO8FX6tCaMYtS+UORPrug3E7LqeWg1Q@mail.gmail.com>
On Tue, May 15, 2018 at 1:15 PM, Chaitanya Lala
<chaitanya.lala@gmail.com> wrote:
> Hi,
>
> I am trying to bring up a Intel XL710 4x10G Intel card using the
> latest mainline top-of-tree.
> The problem is that "ifconfig up" and "ifconfig down" do not take
> effect at the link state level.
> I tracked the problem down to i40e_force_link_state() when it is
> called from i40e_down().
> It calls i40e_force_link_state with "is_up" == false. In-turn it
> calls, i40e_aq_set_link_restart_an(hw, true, NULL).
>
> Should the second argument of i40e_aq_set_link_restart_an be "is_up"
> vs the current "true"
> i.e. i40e_aq_set_link_restart_an(hw, is_up, NULL). ? When I make this
> change, the link state syncs-up with
> the interface administrative state.
>
> Is this a bug ?
>
> Thanks,
> Chaitanya
If you are calling i40e_down the assumption is you are bringing the
interface down, so as not to pass traffic. That is why the "is_up" is
set to false.
Could you provide the dmesg results for when you load the driver and
execute the "ifconfig up" and "ifconfig down" commands? That would
help us to understand what might be going on.
Thanks.
- Alex
^ permalink raw reply
* Re: [PATCH v2 iproute2-next 1/3] rdma: update rdma_netlink.h to get new driver attributes
From: Doug Ledford @ 2018-05-15 20:39 UTC (permalink / raw)
To: Steve Wise, 'David Ahern', leon; +Cc: stephen, netdev, linux-rdma
In-Reply-To: <004901d3ec8b$4b9ac5c0$e2d05140$@opengridcomputing.com>
[-- Attachment #1: Type: text/plain, Size: 2390 bytes --]
On Tue, 2018-05-15 at 15:28 -0500, Steve Wise wrote:
> > -----Original Message-----
> > From: linux-rdma-owner@vger.kernel.org <linux-rdma-
> > owner@vger.kernel.org> On Behalf Of Doug Ledford
> > Sent: Tuesday, May 15, 2018 2:53 PM
> > To: David Ahern <dsahern@gmail.com>; Steve Wise
> > <swise@opengridcomputing.com>; leon@kernel.org
> > Cc: stephen@networkplumber.org; netdev@vger.kernel.org; linux-
> > rdma@vger.kernel.org
> > Subject: Re: [PATCH v2 iproute2-next 1/3] rdma: update rdma_netlink.h to
> > get new driver attributes
> >
> > On Tue, 2018-05-15 at 13:37 -0600, David Ahern wrote:
> > > On 5/14/18 9:42 AM, Steve Wise wrote:
> > > > diff --git a/rdma/include/uapi/rdma/rdma_netlink.h
> >
> > b/rdma/include/uapi/rdma/rdma_netlink.h
> > > > index 60416ed..40be0d8 100644
> > > > --- a/rdma/include/uapi/rdma/rdma_netlink.h
> > > > +++ b/rdma/include/uapi/rdma/rdma_netlink.h
> > > >
> > > > @@ -387,6 +399,20 @@ enum rdma_nldev_attr {
> > > > RDMA_NLDEV_ATTR_RES_PD_ENTRY, /* nested table */
> > > > RDMA_NLDEV_ATTR_RES_LOCAL_DMA_LKEY, /* u32 */
> > > > RDMA_NLDEV_ATTR_RES_UNSAFE_GLOBAL_RKEY, /* u32 */
> > > > + /*
> > > > + * driver-specific attributes.
> > > > + */
> > > > + RDMA_NLDEV_ATTR_DRIVER, /* nested table */
> > > > + RDMA_NLDEV_ATTR_DRIVER_ENTRY, /* nested table */
> > > > + RDMA_NLDEV_ATTR_DRIVER_STRING, /* string */
> > > > + /*
> > > > + * u8 values from enum rdma_nldev_print_type
> > > > + */
> > > > + RDMA_NLDEV_ATTR_DRIVER_PRINT_TYPE, /* u8 */
> > > > + RDMA_NLDEV_ATTR_DRIVER_S32, /* s32 */
> > > > + RDMA_NLDEV_ATTR_DRIVER_U32, /* u32 */
> > > > + RDMA_NLDEV_ATTR_DRIVER_S64, /* s64 */
> > > > + RDMA_NLDEV_ATTR_DRIVER_U64, /* u64 */
> > >
> > > and again here.
> >
> > This chunk, however, is a problem. We'll need to fix that in the kernel
> > and in this patch too.
>
> I'll fix this series once I fix the kernel side. Doug, should I send a patch that basically moves the DRIVER attributes to the bottom?
>
> Sorry about missing this!
I missed it too. I knew about the first hunk, but thought this one was
at the end. Anyway, I've already pushed the fix to my wip/dl-for-next
branch, so you can get it there now.
--
Doug Ledford <dledford@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH net-next v3 0/7] Microsemi Ocelot Ethernet switch support
From: David Miller @ 2018-05-15 20:45 UTC (permalink / raw)
To: alexandre.belloni
Cc: Allan.Nielsen, razvan.stefanescu, po.liu, thomas.petazzoni,
andrew, f.fainelli, netdev, linux-kernel, linux-mips, jhogan
In-Reply-To: <20180514200500.2953-1-alexandre.belloni@bootlin.com>
From: Alexandre Belloni <alexandre.belloni@bootlin.com>
Date: Mon, 14 May 2018 22:04:53 +0200
> The ocelot dts changes are here for reference and should probably go
> through the MIPS tree once the bindings are accepted.
Ok, series applied to net-next minus patches #5 and #6.
Thank you.
^ permalink raw reply
* Re: Poor TCP performance with XPS enabled after scrubbing skb
From: Eric Dumazet @ 2018-05-15 21:08 UTC (permalink / raw)
To: Flavio Leitner, netdev; +Cc: Paolo Abeni
In-Reply-To: <20180515193128.GA11901@plex.lan>
On 05/15/2018 12:31 PM, Flavio Leitner wrote:
> Hi,
>
> There is a significant throughput issue (~50% drop) for a single TCP
> stream when the skb is scrubbed and XPS is enabled.
>
> If I turn CONFIG_XPS off, then the issue never happens and the test
> reaches line rate. The same happens if I echo 0 to tx-*/xps_cpus.
>
> It looks like that when the skb is scrubbed, there is no more reference
> to the struct sock,
And this is really the problem here, since it breaks back pressure (and TCP Small queues)
I am not sure why skb_orphan() is used in this scrubbing really.
^ permalink raw reply
* Re: [PATCH bpf-next v6 2/4] bpf: sockmap, add hash map support
From: Y Song @ 2018-05-15 21:09 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: John Fastabend, Alexei Starovoitov, netdev, David Miller
In-Reply-To: <cc2e3011-0309-842c-4dbc-d3e82d5fc2d0@iogearbox.net>
On Tue, May 15, 2018 at 12:01 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 05/14/2018 07:00 PM, John Fastabend wrote:
>> Sockmap is currently backed by an array and enforces keys to be
>> four bytes. This works well for many use cases and was originally
>> modeled after devmap which also uses four bytes keys. However,
>> this has become limiting in larger use cases where a hash would
>> be more appropriate. For example users may want to use the 5-tuple
>> of the socket as the lookup key.
>>
>> To support this add hash support.
>>
>> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
>> Acked-by: David S. Miller <davem@davemloft.net>
>> ---
>> include/linux/bpf.h | 8 +
>> include/linux/bpf_types.h | 1 +
>> include/uapi/linux/bpf.h | 52 ++++-
>> kernel/bpf/core.c | 1 +
>> kernel/bpf/sockmap.c | 494 ++++++++++++++++++++++++++++++++++++++++++++--
>> kernel/bpf/verifier.c | 14 +-
>> net/core/filter.c | 58 ++++++
>> 7 files changed, 610 insertions(+), 18 deletions(-)
>>
>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> index a38e474..ed0122b 100644
>> --- a/include/linux/bpf.h
>> +++ b/include/linux/bpf.h
>> @@ -668,6 +668,7 @@ static inline void bpf_map_offload_map_free(struct bpf_map *map)
>>
>> #if defined(CONFIG_STREAM_PARSER) && defined(CONFIG_BPF_SYSCALL) && defined(CONFIG_INET)
>> struct sock *__sock_map_lookup_elem(struct bpf_map *map, u32 key);
>> +struct sock *__sock_hash_lookup_elem(struct bpf_map *map, void *key);
>> int sock_map_prog(struct bpf_map *map, struct bpf_prog *prog, u32 type);
>> #else
>> static inline struct sock *__sock_map_lookup_elem(struct bpf_map *map, u32 key)
>> @@ -675,6 +676,12 @@ static inline struct sock *__sock_map_lookup_elem(struct bpf_map *map, u32 key)
>> return NULL;
>> }
>>
>> +static inline struct sock *__sock_hash_lookup_elem(struct bpf_map *map,
>> + void *key)
>> +{
>> + return NULL;
>> +}
>> +
>> static inline int sock_map_prog(struct bpf_map *map,
>> struct bpf_prog *prog,
>> u32 type)
>> @@ -724,6 +731,7 @@ static inline void __xsk_map_flush(struct bpf_map *map)
>> extern const struct bpf_func_proto bpf_get_stackid_proto;
>> extern const struct bpf_func_proto bpf_get_stack_proto;
>> extern const struct bpf_func_proto bpf_sock_map_update_proto;
>> +extern const struct bpf_func_proto bpf_sock_hash_update_proto;
>>
>> /* Shared helpers among cBPF and eBPF. */
>> void bpf_user_rnd_init_once(void);
>> diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
>> index d7df1b32..b67f879 100644
>> --- a/include/linux/bpf_types.h
>> +++ b/include/linux/bpf_types.h
>> @@ -47,6 +47,7 @@
>> BPF_MAP_TYPE(BPF_MAP_TYPE_DEVMAP, dev_map_ops)
>> #if defined(CONFIG_STREAM_PARSER) && defined(CONFIG_INET)
>> BPF_MAP_TYPE(BPF_MAP_TYPE_SOCKMAP, sock_map_ops)
>> +BPF_MAP_TYPE(BPF_MAP_TYPE_SOCKHASH, sock_hash_ops)
>> #endif
>> BPF_MAP_TYPE(BPF_MAP_TYPE_CPUMAP, cpu_map_ops)
>> #if defined(CONFIG_XDP_SOCKETS)
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index 02e4112..1205d86 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
>> @@ -118,6 +118,7 @@ enum bpf_map_type {
>> BPF_MAP_TYPE_SOCKMAP,
>> BPF_MAP_TYPE_CPUMAP,
>> BPF_MAP_TYPE_XSKMAP,
>> + BPF_MAP_TYPE_SOCKHASH,
>> };
>>
>> enum bpf_prog_type {
>> @@ -1855,6 +1856,52 @@ struct bpf_stack_build_id {
>> * Egress device index on success, 0 if packet needs to continue
>> * up the stack for further processing or a negative error in case
>> * of failure.
>> + * int bpf_sock_hash_update(struct bpf_sock_ops_kern *skops, struct bpf_map *map, void *key, u64 flags)
>
> When you rebase please fix this up properly next time and add a newline in between
> the helpers. I fixed this up while applying.
I guess the tools/include/uapi/linux/bpf.h may also need fixup to be
in sync with main bpf.h.
^ permalink raw reply
* Re: [PATCH 00/14] Modify action API for implementing lockless actions
From: Vlad Buslov @ 2018-05-15 21:21 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: netdev, davem, xiyou.wangcong, jiri, pablo, kadlec, fw, ast,
daniel, edumazet, keescook, linux-kernel, netfilter-devel,
coreteam, kliteyn
In-Reply-To: <97ccb118-a3dc-6eb0-b127-9c04adf9e2fd@mojatatu.com>
On Tue 15 May 2018 at 18:25, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> On 14/05/18 04:46 PM, Vlad Buslov wrote:
>>
>> On Mon 14 May 2018 at 18:03, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>>> On 14/05/18 10:27 AM, Vlad Buslov wrote:
>
>
>> Hello Jamal,
>>
>> I'm trying to run tdc, but keep getting following error even on clean
>> branch without my patches:
>
> Vlad, not sure if you saw my email:
> Apply Roman's patch and try again
>
> https://marc.info/?l=linux-netdev&m=152639369112020&w=2
>
> cheers,
> jamal
With patch applied I get following error:
Test 7d50: Add skbmod action to set destination mac
exit: 255 0
dst MAC address <11:22:33:44:55:66>
RTNETLINK answers: No such file or directory
We have an error talking to the kernel
-----> teardown stage *** Could not execute: "$TC actions flush action skbmod"
-----> teardown stage *** Error message: "RTNETLINK answers: Invalid argument
We have an error flushing
"
-----> teardown stage *** Aborting test run.
<_io.BufferedReader name=3> *** stdout ***
<_io.BufferedReader name=5> *** stderr ***
"-----> teardown stage" did not complete successfully
Exception <class '__main__.PluginMgrTestFail'> ('teardown', 'dst MAC address <11:22:33:44:55:66>\nRTNETLINK answers: No such file or directory\nWe have an error talking to the kernel\n', '"-----> teardown stage" did not complete successful
ly') (caught in test_runner, running test 87 7d50 Add skbmod action to set destination mac stage teardown)
---------------
traceback
File "./tdc.py", line 293, in test_runner
tap += run_one_test(pm, args, index, tidx)
File "./tdc.py", line 229, in run_one_test
prepare_env(args, pm, 'teardown', '-----> teardown stage', tidx['teardown'], procout)
File "./tdc.py", line 177, in prepare_env
'"{}" did not complete successfully'.format(prefix))
---------------
accumulated output for this test:
dst MAC address <11:22:33:44:55:66>
RTNETLINK answers: No such file or directory
We have an error talking to the kernel
---------------
Regards,
Vlad
^ permalink raw reply
* RE: i40e - Is i40e_force_link_state doing the right thing ?
From: Stachura, Mariusz @ 2018-05-15 21:24 UTC (permalink / raw)
To: Chaitanya Lala, netdev@vger.kernel.org, Williams, Mitch A,
Bowers, AndrewX, Kirsher, Jeffrey T
In-Reply-To: <CABOGejK8bnwU0iqO5-GRO8FX6tCaMYtS+UORPrug3E7LqeWg1Q@mail.gmail.com>
On Tue, May 15, 2018 at 1:15 PM, Chaitanya Lala <chaitanya.lala@gmail.com> wrote:
> Hi,
>
> I am trying to bring up a Intel XL710 4x10G Intel card using the
> latest mainline top-of-tree.
> The problem is that "ifconfig up" and "ifconfig down" do not take
> effect at the link state level.
> I tracked the problem down to i40e_force_link_state() when it is
> called from i40e_down().
> It calls i40e_force_link_state with "is_up" == false. In-turn it
> calls, i40e_aq_set_link_restart_an(hw, true, NULL).
>
> Should the second argument of i40e_aq_set_link_restart_an be "is_up"
> vs the current "true"
> i.e. i40e_aq_set_link_restart_an(hw, is_up, NULL). ? When I make this
> change, the link state syncs-up with the interface administrative
> state.
>
> Is this a bug ?
>
> Thanks,
> Chaitanya
Hello Chaitanya,
i40e_down() calls i40e_force_link_state with "is_up" == false only if interface's private flag "link-down-on-close" is set. By default the link is left up for manageability and VF traffic, user can use this flag to power down the interface on the link level. Does that work for you?
The command is:
"ethtool --set-priv-flags IFNAME link-down-on-close on" and then
"ifconfig IFNAME down"
Best regards!
Mariusz
--------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.
^ permalink raw reply
* Re: i40e - Is i40e_force_link_state doing the right thing ?
From: Chaitanya Lala @ 2018-05-15 21:47 UTC (permalink / raw)
To: Stachura, Mariusz
Cc: netdev@vger.kernel.org, Williams, Mitch A, Bowers, AndrewX,
Kirsher, Jeffrey T
In-Reply-To: <1F86EBA5C8668B468CEE1C33B96E862B36905DA1@IRSMSX104.ger.corp.intel.com>
Hi Mariusz, ...
On Tue, May 15, 2018 at 2:24 PM, Stachura, Mariusz
<mariusz.stachura@intel.com> wrote:
> On Tue, May 15, 2018 at 1:15 PM, Chaitanya Lala <chaitanya.lala@gmail.com> wrote:
>> Hi,
>>
>> I am trying to bring up a Intel XL710 4x10G Intel card using the
>> latest mainline top-of-tree.
>> The problem is that "ifconfig up" and "ifconfig down" do not take
>> effect at the link state level.
>> I tracked the problem down to i40e_force_link_state() when it is
>> called from i40e_down().
>> It calls i40e_force_link_state with "is_up" == false. In-turn it
>> calls, i40e_aq_set_link_restart_an(hw, true, NULL).
>>
>> Should the second argument of i40e_aq_set_link_restart_an be "is_up"
>> vs the current "true"
>> i.e. i40e_aq_set_link_restart_an(hw, is_up, NULL). ? When I make this
>> change, the link state syncs-up with the interface administrative
>> state.
>>
>> Is this a bug ?
>>
>> Thanks,
>> Chaitanya
>
> Hello Chaitanya,
>
> i40e_down() calls i40e_force_link_state with "is_up" == false only if interface's private flag "link-down-on-close" is set. By default the link is left up for manageability and VF traffic, user can use this flag to power down the interface on the link level. Does that work for you?
> The command is:
> "ethtool --set-priv-flags IFNAME link-down-on-close on" and then
This flag is _on_ in my setup and hencet i40e_force_link_state is
being called with is_up == false in my setup. The problem is that
irrespective of
value of "is_up" flag, i40e_force_link_state invokes
i40e_aq_set_link_restart_an with second argument (enable_link) as
"true". So i40e_aq_set_link_restart_an
is always trying to enable link even if is_up was false. Is that
correct behavior ?
I have pasted code with my annotations below marked with "//XXX".
/**
* i40e_force_link_state - Force the link status
* @pf: board private structure
* @is_up: whether the link state should be forced up or down
**/
static i40e_status i40e_force_link_state(struct i40e_pf *pf, bool
is_up) // XXX is_up was _false_ in my test case because I have
link-down-on-close "on"
{
struct i40e_aq_get_phy_abilities_resp abilities;
struct i40e_aq_set_phy_config config = {0};
struct i40e_hw *hw = &pf->hw;
i40e_status err;
u64 mask;
/* Get the current phy config */
err = i40e_aq_get_phy_capabilities(hw, false, false, &abilities,
NULL);
if (err) {
dev_err(&pf->pdev->dev,
"failed to get phy cap., ret = %s last_status = %s\n",
i40e_stat_str(hw, err),
i40e_aq_str(hw, hw->aq.asq_last_status));
return err;
}
/* If link needs to go up, but was not forced to go down,
* no need for a flap
*/
if (is_up && abilities.phy_type != 0)
return I40E_SUCCESS;
/* To force link we need to set bits for all supported PHY types,
* but there are now more than 32, so we need to split the bitmap
* across two fields.
*/
mask = I40E_PHY_TYPES_BITMASK;
config.phy_type = is_up ? cpu_to_le32((u32)(mask & 0xffffffff)) : 0;
config.phy_type_ext = is_up ? (u8)((mask >> 32) & 0xff) : 0;
/* Copy the old settings, except of phy_type */
config.abilities = abilities.abilities;
config.link_speed = abilities.link_speed;
config.eee_capability = abilities.eee_capability;
config.eeer = abilities.eeer_val;
config.low_power_ctrl = abilities.d3_lpan;
err = i40e_aq_set_phy_config(hw, &config, NULL);
if (err) {
dev_err(&pf->pdev->dev,
"set phy config ret = %s last_status = %s\n",
i40e_stat_str(&pf->hw, err),
i40e_aq_str(&pf->hw, pf->hw.aq.asq_last_status));
return err;
}
/* Update the link info */
err = i40e_update_link_info(hw);
if (err) {
/* Wait a little bit (on 40G cards it sometimes takes a really
* long time for link to come back from the atomic reset)
* and try once more
*/
msleep(1000);
i40e_update_link_info(hw);
}
i40e_aq_set_link_restart_an(hw, true, NULL); // XXX Second argument is
unconditionally true. Should it not be is_up instead of true ?
return I40E_SUCCESS;
}
Thanks,
Chaitanya
> "ifconfig IFNAME down"
>
> Best regards!
> Mariusz
> --------------------------------------------------------------------
>
> Intel Technology Poland sp. z o.o.
> ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
>
> Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
> przegladanie lub rozpowszechnianie jest zabronione.
> This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
> others is strictly prohibited.
^ permalink raw reply
* Re: [PATCH 00/14] Modify action API for implementing lockless actions
From: Jamal Hadi Salim @ 2018-05-15 21:49 UTC (permalink / raw)
To: Vlad Buslov
Cc: netdev, davem, xiyou.wangcong, jiri, pablo, kadlec, fw, ast,
daniel, edumazet, kliteyn
In-Reply-To: <vbfh8n81uoc.fsf@reg-r-vrt-018-180.mtr.labs.mlnx>
On 15/05/18 05:21 PM, Vlad Buslov wrote:
>
> On Tue 15 May 2018 at 18:25, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>> On 14/05/18 04:46 PM, Vlad Buslov wrote:
>>>
>>> On Mon 14 May 2018 at 18:03, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>>>> On 14/05/18 10:27 AM, Vlad Buslov wrote:
>>
>>
>>> Hello Jamal,
>>>
>>> I'm trying to run tdc, but keep getting following error even on clean
>>> branch without my patches:
>>
>> Vlad, not sure if you saw my email:
>> Apply Roman's patch and try again
>>
>> https://marc.info/?l=linux-netdev&m=152639369112020&w=2
>>
>> cheers,
>> jamal
>
> With patch applied I get following error:
>
> Test 7d50: Add skbmod action to set destination mac
> exit: 255 0
> dst MAC address <11:22:33:44:55:66>
> RTNETLINK answers: No such file or directory
> We have an error talking to the kernel
>
You may actually have broken something with your patches in this case.
Lucas - does this test pass on latest net-next?
cheers,
jamal
PS:- any reason for the big Cc? I have trimmed it down.
^ permalink raw reply
* [PATCH iproute2] Allow to configure /var/run/netns directory
From: Pavel Maltsev @ 2018-05-15 21:49 UTC (permalink / raw)
To: netdev, lorenzo, stephen, pavelm; +Cc: lorenzo, stephen, Pavel Maltsev
Currently NETNS_RUN_DIR is hardcoded and refers to /var/run/netns.
However, some systems (e.g. Android) doesn't have /var
which results in error attempts to create network namespaces on these
systems. This change makes NETNS_RUN_DIR configurable at build time
by allowing to pass environment variable to configre script.
For example: NETNS_RUN_DIR=/mnt/vendor/netns ./configure && make
Tested: verified that iproute2 with configuration mentioned above
creates namespaces in /mnt/vendor/netns
Signed-off-by: Pavel Maltsev <pavelm@google.com>
---
configure | 3 +++
include/namespace.h | 3 +++
2 files changed, 6 insertions(+)
diff --git a/configure b/configure
index 5ef5cd4c..a6a222da 100755
--- a/configure
+++ b/configure
@@ -213,6 +213,9 @@ EOF
echo "IP_CONFIG_SETNS:=y" >>$CONFIG
echo "yes"
echo "CFLAGS += -DHAVE_SETNS" >>$CONFIG
+ if [ -n "$NETNS_RUN_DIR" ]; then
+ echo "CFLAGS += -DNETNS_RUN_DIR=\\\"$NETNS_RUN_DIR\\\"" >>$CONFIG
+ fi
else
echo "no"
fi
diff --git a/include/namespace.h b/include/namespace.h
index aed7ce08..b8fb14df 100644
--- a/include/namespace.h
+++ b/include/namespace.h
@@ -8,7 +8,10 @@
#include <sys/syscall.h>
#include <errno.h>
+#ifndef NETNS_RUN_DIR
#define NETNS_RUN_DIR "/var/run/netns"
+#endif /* NETNS_RUN_DIR */
+
#define NETNS_ETC_DIR "/etc/netns"
#ifndef CLONE_NEWNET
--
2.17.0.441.gb46fe60e1d-goog
^ permalink raw reply related
* [PATCH net-next 0/2] of: mdio: Fall back to mdiobus_register() with np is NULL
From: Florian Fainelli @ 2018-05-15 21:59 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, David S. Miller,
Nicolas Ferre, Fugang Duan, Sergei Shtylyov, Giuseppe Cavallaro,
Alexandre Torgue, Jose Abreu, Grygorii Strashko, Woojung Huh,
Microchip Linux Driver Support, Rob Herring, Frank Rowand,
Antoine Tenart, Tobias Jordan, Russell King <r
Hi all,
This patch series updates of_mdiobus_register() such that when the device_node
argument is NULL, it calls mdiobus_register() directly. This is consistent with
the behavior of of_mdiobus_register() when CONFIG_OF=n.
I only converted the most obvious drivers, there are others that have a much
less obvious behavior and specifically attempt to deal with CONFIG_ACPI.
Florian Fainelli (2):
of: mdio: Fall back to mdiobus_register() with np is NULL
drivers: net: Remove device_node checks with of_mdiobus_register()
drivers/net/dsa/bcm_sf2.c | 8 ++------
drivers/net/dsa/mv88e6xxx/chip.c | 5 +----
drivers/net/ethernet/cadence/macb_main.c | 12 +++---------
drivers/net/ethernet/freescale/fec_main.c | 8 ++------
drivers/net/ethernet/marvell/mvmdio.c | 5 +----
drivers/net/ethernet/renesas/sh_eth.c | 11 +++--------
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 5 +----
drivers/net/ethernet/ti/davinci_mdio.c | 6 ++----
drivers/net/phy/mdio-gpio.c | 6 +-----
drivers/net/phy/mdio-mscc-miim.c | 6 +-----
drivers/net/usb/lan78xx.c | 7 ++-----
drivers/of/of_mdio.c | 6 ++++++
12 files changed, 25 insertions(+), 60 deletions(-)
--
2.14.1
^ permalink raw reply
* [PATCH net-next 1/2] of: mdio: Fall back to mdiobus_register() with np is NULL
From: Florian Fainelli @ 2018-05-15 21:59 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, David S. Miller,
Nicolas Ferre, Fugang Duan, Sergei Shtylyov, Giuseppe Cavallaro,
Alexandre Torgue, Jose Abreu, Grygorii Strashko, Woojung Huh,
Microchip Linux Driver Support, Rob Herring, Frank Rowand,
Antoine Tenart, Tobias Jordan, Russell King <r
In-Reply-To: <20180515215930.12115-1-f.fainelli@gmail.com>
When the Device Tree node specified is NULL, fall back to
mdiobus_register(). We have a number of drivers having a similar pattern
which is:
if (np)
of_mdiobus_register()
else
mdiobus_register()
so incorporate that behavior within the core of_mdiobus_register()
function. This is also consistent with the stub version that we defined
when CONFIG_OF=n.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/of/of_mdio.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 8c0c92712fc9..2341dbf675bf 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -204,6 +204,12 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
bool scanphys = false;
int addr, rc;
+ /* If the Device Tree node does not exist, fall back to traditional
+ * registration
+ */
+ if (!np)
+ return mdiobus_register(mdio);
+
/* Do not continue if the node is disabled */
if (!of_device_is_available(np))
return -ENODEV;
--
2.14.1
^ permalink raw reply related
* [PATCH net-next 2/2] drivers: net: Remove device_node checks with of_mdiobus_register()
From: Florian Fainelli @ 2018-05-15 21:59 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, David S. Miller,
Nicolas Ferre, Fugang Duan, Sergei Shtylyov, Giuseppe Cavallaro,
Alexandre Torgue, Jose Abreu, Grygorii Strashko, Woojung Huh,
Microchip Linux Driver Support, Rob Herring, Frank Rowand,
Antoine Tenart, Tobias Jordan, Russell King <r
In-Reply-To: <20180515215930.12115-1-f.fainelli@gmail.com>
A number of drivers have the following pattern:
if (np)
of_mdiobus_register()
else
mdiobus_register()
which the implementation of of_mdiobus_register() now takes care of.
Remove that pattern in drivers that strictly adhere to it.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/bcm_sf2.c | 8 ++------
drivers/net/dsa/mv88e6xxx/chip.c | 5 +----
drivers/net/ethernet/cadence/macb_main.c | 12 +++---------
drivers/net/ethernet/freescale/fec_main.c | 8 ++------
drivers/net/ethernet/marvell/mvmdio.c | 5 +----
drivers/net/ethernet/renesas/sh_eth.c | 11 +++--------
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 5 +----
drivers/net/ethernet/ti/davinci_mdio.c | 6 ++----
drivers/net/phy/mdio-gpio.c | 6 +-----
drivers/net/phy/mdio-mscc-miim.c | 6 +-----
drivers/net/usb/lan78xx.c | 7 ++-----
11 files changed, 19 insertions(+), 60 deletions(-)
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index ac621f44237a..02e8982519ce 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -450,12 +450,8 @@ static int bcm_sf2_mdio_register(struct dsa_switch *ds)
priv->slave_mii_bus->parent = ds->dev->parent;
priv->slave_mii_bus->phy_mask = ~priv->indir_phy_mask;
- if (dn)
- err = of_mdiobus_register(priv->slave_mii_bus, dn);
- else
- err = mdiobus_register(priv->slave_mii_bus);
-
- if (err)
+ err = of_mdiobus_register(priv->slave_mii_bus, dn);
+ if (err && dn)
of_node_put(dn);
return err;
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index b23c11d9f4b2..2bb3f03ee1cb 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2454,10 +2454,7 @@ static int mv88e6xxx_mdio_register(struct mv88e6xxx_chip *chip,
return err;
}
- if (np)
- err = of_mdiobus_register(bus, np);
- else
- err = mdiobus_register(bus);
+ err = of_mdiobus_register(bus, np);
if (err) {
dev_err(chip->dev, "Cannot register MDIO bus (%d)\n", err);
mv88e6xxx_g2_irq_mdio_free(chip, bus);
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index b4c9268100bb..3e93df5d4e3b 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -591,16 +591,10 @@ static int macb_mii_init(struct macb *bp)
dev_set_drvdata(&bp->dev->dev, bp->mii_bus);
np = bp->pdev->dev.of_node;
+ if (pdata)
+ bp->mii_bus->phy_mask = pdata->phy_mask;
- if (np) {
- err = of_mdiobus_register(bp->mii_bus, np);
- } else {
- if (pdata)
- bp->mii_bus->phy_mask = pdata->phy_mask;
-
- err = mdiobus_register(bp->mii_bus);
- }
-
+ err = of_mdiobus_register(bp->mii_bus, np);
if (err)
goto err_out_free_mdiobus;
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index d4604bc8eb5b..f3e43db0d6cb 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2052,13 +2052,9 @@ static int fec_enet_mii_init(struct platform_device *pdev)
fep->mii_bus->parent = &pdev->dev;
node = of_get_child_by_name(pdev->dev.of_node, "mdio");
- if (node) {
- err = of_mdiobus_register(fep->mii_bus, node);
+ err = of_mdiobus_register(fep->mii_bus, node);
+ if (node)
of_node_put(node);
- } else {
- err = mdiobus_register(fep->mii_bus);
- }
-
if (err)
goto err_out_free_mdiobus;
diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index 0495487f7b42..c5dac6bd2be4 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -348,10 +348,7 @@ static int orion_mdio_probe(struct platform_device *pdev)
goto out_mdio;
}
- if (pdev->dev.of_node)
- ret = of_mdiobus_register(bus, pdev->dev.of_node);
- else
- ret = mdiobus_register(bus);
+ ret = of_mdiobus_register(bus, pdev->dev.of_node);
if (ret < 0) {
dev_err(&pdev->dev, "Cannot register MDIO bus (%d)\n", ret);
goto out_mdio;
diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 5970d9e5ddf1..8dd41e08a6c6 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -3025,15 +3025,10 @@ static int sh_mdio_init(struct sh_eth_private *mdp,
pdev->name, pdev->id);
/* register MDIO bus */
- if (dev->of_node) {
- ret = of_mdiobus_register(mdp->mii_bus, dev->of_node);
- } else {
- if (pd->phy_irq > 0)
- mdp->mii_bus->irq[pd->phy] = pd->phy_irq;
-
- ret = mdiobus_register(mdp->mii_bus);
- }
+ if (pd->phy_irq > 0)
+ mdp->mii_bus->irq[pd->phy] = pd->phy_irq;
+ ret = of_mdiobus_register(mdp->mii_bus, dev->of_node);
if (ret)
goto out_free_bus;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index f5f37bfa1d58..5df1a608e566 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -233,10 +233,7 @@ int stmmac_mdio_register(struct net_device *ndev)
new_bus->phy_mask = mdio_bus_data->phy_mask;
new_bus->parent = priv->device;
- if (mdio_node)
- err = of_mdiobus_register(new_bus, mdio_node);
- else
- err = mdiobus_register(new_bus);
+ err = of_mdiobus_register(new_bus, mdio_node);
if (err != 0) {
dev_err(dev, "Cannot register the MDIO bus\n");
goto bus_register_fail;
diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
index 98a1c97fb95e..e720244e7f71 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -431,10 +431,8 @@ static int davinci_mdio_probe(struct platform_device *pdev)
*/
if (dev->of_node && of_get_child_count(dev->of_node)) {
data->skip_scan = true;
- ret = of_mdiobus_register(data->bus, dev->of_node);
- } else {
- ret = mdiobus_register(data->bus);
- }
+
+ ret = of_mdiobus_register(data->bus, dev->of_node);
if (ret)
goto bail_out;
diff --git a/drivers/net/phy/mdio-gpio.c b/drivers/net/phy/mdio-gpio.c
index b501221819e1..4e4c8daf44c3 100644
--- a/drivers/net/phy/mdio-gpio.c
+++ b/drivers/net/phy/mdio-gpio.c
@@ -179,11 +179,7 @@ static int mdio_gpio_probe(struct platform_device *pdev)
if (!new_bus)
return -ENODEV;
- if (pdev->dev.of_node)
- ret = of_mdiobus_register(new_bus, pdev->dev.of_node);
- else
- ret = mdiobus_register(new_bus);
-
+ ret = of_mdiobus_register(new_bus, pdev->dev.of_node);
if (ret)
mdio_gpio_bus_deinit(&pdev->dev);
diff --git a/drivers/net/phy/mdio-mscc-miim.c b/drivers/net/phy/mdio-mscc-miim.c
index 8c689ccfdbca..badbc99bedd3 100644
--- a/drivers/net/phy/mdio-mscc-miim.c
+++ b/drivers/net/phy/mdio-mscc-miim.c
@@ -151,11 +151,7 @@ static int mscc_miim_probe(struct platform_device *pdev)
}
}
- if (pdev->dev.of_node)
- ret = of_mdiobus_register(bus, pdev->dev.of_node);
- else
- ret = mdiobus_register(bus);
-
+ ret = of_mdiobus_register(bus, pdev->dev.of_node);
if (ret < 0) {
dev_err(&pdev->dev, "Cannot register MDIO bus (%d)\n", ret);
return ret;
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 91761436709a..8dff87ec6d99 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -1843,12 +1843,9 @@ static int lan78xx_mdio_init(struct lan78xx_net *dev)
}
node = of_get_child_by_name(dev->udev->dev.of_node, "mdio");
- if (node) {
- ret = of_mdiobus_register(dev->mdiobus, node);
+ ret = of_mdiobus_register(dev->mdiobus, node);
+ if (node)
of_node_put(node);
- } else {
- ret = mdiobus_register(dev->mdiobus);
- }
if (ret) {
netdev_err(dev->net, "can't register MDIO bus\n");
goto exit1;
--
2.14.1
^ permalink raw reply related
* Re: [PATCH 00/14] Modify action API for implementing lockless actions
From: Lucas Bates @ 2018-05-15 22:03 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Vlad Buslov, Linux Kernel Network Developers, David Miller,
Cong Wang, Jiri Pirko, pablo, kadlec, fw, ast, Daniel Borkmann,
edumazet, kliteyn
In-Reply-To: <2ee4066e-643a-f901-8926-7001f8699163@mojatatu.com>
On Tue, May 15, 2018 at 5:49 PM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>> Test 7d50: Add skbmod action to set destination mac
>> exit: 255 0
>> dst MAC address <11:22:33:44:55:66>
>> RTNETLINK answers: No such file or directory
>> We have an error talking to the kernel
>>
>
> You may actually have broken something with your patches in this case.
>
> Lucas - does this test pass on latest net-next?
Yes, 7d50 has been passing on our builds for at least the last month.
- Lucas
^ permalink raw reply
* Re: [PATCH 00/14] Modify action API for implementing lockless actions
From: Lucas Bates @ 2018-05-15 22:07 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Vlad Buslov, Linux Kernel Network Developers, David Miller,
Cong Wang, Jiri Pirko, pablo, kadlec, fw, ast, Daniel Borkmann,
edumazet, kliteyn
In-Reply-To: <CAMDBHY+QRKnQnmCx7hj_9wXmdvb1Qarnj5xJ3EaZssABoxG1bg@mail.gmail.com>
On Tue, May 15, 2018 at 6:03 PM, Lucas Bates <lucasb@mojatatu.com> wrote:
> On Tue, May 15, 2018 at 5:49 PM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>>> Test 7d50: Add skbmod action to set destination mac
>>> exit: 255 0
>>> dst MAC address <11:22:33:44:55:66>
>>> RTNETLINK answers: No such file or directory
>>> We have an error talking to the kernel
>>>
>>
>> You may actually have broken something with your patches in this case.
>>
>> Lucas - does this test pass on latest net-next?
>
> Yes, 7d50 has been passing on our builds for at least the last month.
Also, Vlad, you can look at the JSON to see the test case data, or run
tdc.py -s | less and search for the ID to see the commands being run.
I'm here if you need help using tdc.
^ permalink raw reply
* Re: [PATCH v2] {net, IB}/mlx5: Use 'kvfree()' for memory allocated by 'kvzalloc()'
From: Jason Gunthorpe @ 2018-05-15 22:21 UTC (permalink / raw)
To: Christophe JAILLET
Cc: saeedm, matanb, leon, dledford, davem, netdev, linux-rdma,
linux-kernel, kernel-janitors
In-Reply-To: <20180513070041.24246-1-christophe.jaillet@wanadoo.fr>
On Sun, May 13, 2018 at 09:00:41AM +0200, Christophe JAILLET wrote:
> When 'kvzalloc()' is used to allocate memory, 'kvfree()' must be used to
> free it.
>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> v1 -> v2: More places to update have been added to the patch
> ---
> drivers/infiniband/hw/mlx5/cq.c | 2 +-
> drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c | 2 +-
> drivers/net/ethernet/mellanox/mlx5/core/vport.c | 6 +++---
> 3 files changed, 5 insertions(+), 5 deletions(-)
I agree with Eric on the need for fixes lines in v3..
> diff --git a/drivers/infiniband/hw/mlx5/cq.c b/drivers/infiniband/hw/mlx5/cq.c
> index 77d257ec899b..6d52ea03574e 100644
> --- a/drivers/infiniband/hw/mlx5/cq.c
> +++ b/drivers/infiniband/hw/mlx5/cq.c
> @@ -849,7 +849,7 @@ static int create_cq_user(struct mlx5_ib_dev *dev, struct ib_udata *udata,
> return 0;
>
> err_cqb:
> - kfree(*cqb);
> + kvfree(*cqb);
For the infiniband part:
Acked-by: Jason Gunthorpe <jgg@mellanox.com>
Since this is mostly ethernet, can it go through netdev? thanks
Jason
^ permalink raw reply
* Re: [PATCH net-next 2/2] drivers: net: Remove device_node checks with of_mdiobus_register()
From: Grygorii Strashko @ 2018-05-15 22:57 UTC (permalink / raw)
To: Florian Fainelli, netdev
Cc: Andrew Lunn, Vivien Didelot, David S. Miller, Nicolas Ferre,
Fugang Duan, Sergei Shtylyov, Giuseppe Cavallaro,
Alexandre Torgue, Jose Abreu, Woojung Huh,
Microchip Linux Driver Support, Rob Herring, Frank Rowand,
Antoine Tenart, Tobias Jordan, Russell King, Geert Uytterhoeven,
Thomas
In-Reply-To: <20180515215930.12115-3-f.fainelli@gmail.com>
On 05/15/2018 04:59 PM, Florian Fainelli wrote:
> A number of drivers have the following pattern:
>
> if (np)
> of_mdiobus_register()
> else
> mdiobus_register()
>
> which the implementation of of_mdiobus_register() now takes care of.
> Remove that pattern in drivers that strictly adhere to it.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/net/dsa/bcm_sf2.c | 8 ++------
> drivers/net/dsa/mv88e6xxx/chip.c | 5 +----
> drivers/net/ethernet/cadence/macb_main.c | 12 +++---------
> drivers/net/ethernet/freescale/fec_main.c | 8 ++------
> drivers/net/ethernet/marvell/mvmdio.c | 5 +----
> drivers/net/ethernet/renesas/sh_eth.c | 11 +++--------
> drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 5 +----
> drivers/net/ethernet/ti/davinci_mdio.c | 6 ++----
> drivers/net/phy/mdio-gpio.c | 6 +-----
> drivers/net/phy/mdio-mscc-miim.c | 6 +-----
> drivers/net/usb/lan78xx.c | 7 ++-----
> 11 files changed, 19 insertions(+), 60 deletions(-)
>
...
> goto bus_register_fail;
> diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
> index 98a1c97fb95e..e720244e7f71 100644
> --- a/drivers/net/ethernet/ti/davinci_mdio.c
> +++ b/drivers/net/ethernet/ti/davinci_mdio.c
> @@ -431,10 +431,8 @@ static int davinci_mdio_probe(struct platform_device *pdev)
> */
> if (dev->of_node && of_get_child_count(dev->of_node)) {
It causes build error due to "{" above.
> data->skip_scan = true;
> - ret = of_mdiobus_register(data->bus, dev->of_node);
> - } else {
> - ret = mdiobus_register(data->bus);
> - }
> +
> + ret = of_mdiobus_register(data->bus, dev->of_node);
> if (ret)
> goto bail_out;
>
> diff --git a/drivers/net/phy/mdio-gpio.c b/drivers/net/phy/mdio-gpio.c
> index b501221819e1..4e4c8daf44c3 100644
> --- a/drivers/net/phy/mdio-gpio.c
> +++ b/drivers/net/phy/mdio-gpio.c
> @@ -179,11 +179,7 @@ static int mdio_gpio_probe(struct platform_device *pdev)
[...]
--
regards,
-grygorii
^ permalink raw reply
* Re: [PATCH net-next 2/2] drivers: net: Remove device_node checks with of_mdiobus_register()
From: Florian Fainelli @ 2018-05-15 22:59 UTC (permalink / raw)
To: Grygorii Strashko, netdev
Cc: Andrew Lunn, Vivien Didelot, David S. Miller, Nicolas Ferre,
Fugang Duan, Sergei Shtylyov, Giuseppe Cavallaro,
Alexandre Torgue, Jose Abreu, Woojung Huh,
Microchip Linux Driver Support, Rob Herring, Frank Rowand,
Antoine Tenart, Tobias Jordan, Russell King, Geert Uytterhoeven,
Thomas
In-Reply-To: <1112dc66-5300-89f8-8382-ceb7a024e933@ti.com>
On 05/15/2018 03:57 PM, Grygorii Strashko wrote:
>
>
> On 05/15/2018 04:59 PM, Florian Fainelli wrote:
>> A number of drivers have the following pattern:
>>
>> if (np)
>> of_mdiobus_register()
>> else
>> mdiobus_register()
>>
>> which the implementation of of_mdiobus_register() now takes care of.
>> Remove that pattern in drivers that strictly adhere to it.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> drivers/net/dsa/bcm_sf2.c | 8 ++------
>> drivers/net/dsa/mv88e6xxx/chip.c | 5 +----
>> drivers/net/ethernet/cadence/macb_main.c | 12 +++---------
>> drivers/net/ethernet/freescale/fec_main.c | 8 ++------
>> drivers/net/ethernet/marvell/mvmdio.c | 5 +----
>> drivers/net/ethernet/renesas/sh_eth.c | 11 +++--------
>> drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 5 +----
>> drivers/net/ethernet/ti/davinci_mdio.c | 6 ++----
>> drivers/net/phy/mdio-gpio.c | 6 +-----
>> drivers/net/phy/mdio-mscc-miim.c | 6 +-----
>> drivers/net/usb/lan78xx.c | 7 ++-----
>> 11 files changed, 19 insertions(+), 60 deletions(-)
>>
>
> ...
>
>> goto bus_register_fail;
>> diff --git a/drivers/net/ethernet/ti/davinci_mdio.c
>> b/drivers/net/ethernet/ti/davinci_mdio.c
>> index 98a1c97fb95e..e720244e7f71 100644
>> --- a/drivers/net/ethernet/ti/davinci_mdio.c
>> +++ b/drivers/net/ethernet/ti/davinci_mdio.c
>> @@ -431,10 +431,8 @@ static int davinci_mdio_probe(struct
>> platform_device *pdev)
>> */
>> if (dev->of_node && of_get_child_count(dev->of_node)) {
>
> It causes build error due to "{" above.
Humpf, shame on me for not enabling that driver, thanks for catching this!
>
>> data->skip_scan = true;
>> - ret = of_mdiobus_register(data->bus, dev->of_node);
>> - } else {
>> - ret = mdiobus_register(data->bus);
>> - }
>> +
>> + ret = of_mdiobus_register(data->bus, dev->of_node);
>> if (ret)
>> goto bail_out;
>> diff --git a/drivers/net/phy/mdio-gpio.c b/drivers/net/phy/mdio-gpio.c
>> index b501221819e1..4e4c8daf44c3 100644
>> --- a/drivers/net/phy/mdio-gpio.c
>> +++ b/drivers/net/phy/mdio-gpio.c
>> @@ -179,11 +179,7 @@ static int mdio_gpio_probe(struct platform_device
>> *pdev)
>
> [...]
>
--
Florian
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox