* Re: [PATCH v5 3/5] bnx2x: Eliminate duplicate barriers on weakly-ordered archs
From: Sinan Kaya @ 2018-03-23 16:31 UTC (permalink / raw)
To: David Miller
Cc: netdev, timur, sulrich, linux-arm-msm, linux-arm-kernel,
ariel.elior, everest-linux-l2, linux-kernel
In-Reply-To: <20180323.122035.1380806748695640531.davem@davemloft.net>
On 3/23/2018 12:20 PM, David Miller wrote:
> From: Sinan Kaya <okaya@codeaurora.org>
> Date: Thu, 22 Mar 2018 13:10:00 -0400
>
>> Code includes wmb() followed by writel(). writel() already has a
>> barrier on some architectures like arm64.
> ...
>> @@ -4155,7 +4155,7 @@ netdev_tx_t bnx2x_start_xmit(struct sk_buff *skb, struct net_device *dev)
>> txdata->tx_db.data.prod += nbd;
>> barrier();
>>
>> - DOORBELL(bp, txdata->cid, txdata->tx_db.raw);
>> + DOORBELL_RELAXED(bp, txdata->cid, txdata->tx_db.raw);
>>
>> mmiowb();
> ...
>> @@ -2592,7 +2592,7 @@ static int bnx2x_run_loopback(struct bnx2x *bp, int loopback_mode)
>>
>> txdata->tx_db.data.prod += 2;
>> barrier();
>> - DOORBELL(bp, txdata->cid, txdata->tx_db.raw);
>> + DOORBELL_RELAXED(bp, txdata->cid, txdata->tx_db.raw);
>
> These are compiler barriers being used here, not wmb().
>
> Look, if I can't see a clear:
>
> wmb()
> writel()
>
> sequence in the patch hunks, I am going to keep pushing back on
> these changes.
Sorry, you got me confused now.
If you look at the code closer, you'll see this.
wmb();
txdata->tx_db.data.prod += nbd;
barrier();
DOORBELL(bp, txdata->cid, txdata->tx_db.raw);
and you also asked me to rename DOORBELL to DOORBELL_RELAXED() to make
it obvious that we have a relaxed operator inside the macro.
Did I miss something?
of course, treating barrier() universally as a write barrier is wrong.
>
> Thank you.
>
--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* Re: [bpf-next V5 PATCH 02/15] xdp: introduce xdp_return_frame API and use in cpumap
From: Alexander Duyck @ 2018-03-23 16:35 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Netdev, BjörnTöpel, Karlsson, Magnus, Eugenia Emantayev,
Jason Wang, John Fastabend, Eran Ben Elisha, Saeed Mahameed,
Gal Pressman, Daniel Borkmann, Alexei Starovoitov, Tariq Toukan
In-Reply-To: <152180748903.20167.14050388865469930299.stgit@firesoul>
On Fri, Mar 23, 2018 at 5:18 AM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> Introduce an xdp_return_frame API, and convert over cpumap as
> the first user, given it have queued XDP frame structure to leverage.
>
> V3: Cleanup and remove C99 style comments, pointed out by Alex Duyck.
>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> ---
> include/net/xdp.h | 28 ++++++++++++++++++++++++
> kernel/bpf/cpumap.c | 60 +++++++++++++++++++++++++++++++--------------------
> net/core/xdp.c | 18 +++++++++++++++
> 3 files changed, 82 insertions(+), 24 deletions(-)
>
> diff --git a/include/net/xdp.h b/include/net/xdp.h
> index b2362ddfa694..15b546325e31 100644
> --- a/include/net/xdp.h
> +++ b/include/net/xdp.h
> @@ -33,16 +33,44 @@
> * also mandatory during RX-ring setup.
> */
>
> +enum mem_type {
> + MEM_TYPE_PAGE_SHARED = 0, /* Split-page refcnt based model */
> + MEM_TYPE_PAGE_ORDER0, /* Orig XDP full page model */
> + MEM_TYPE_MAX,
> +};
> +
> +struct xdp_mem_info {
> + u32 type; /* enum mem_type, but known size type */
Do you really need to make t his a full u32 value for something that
is likely never going to exceed a single digit value?
> + /* u32 id; will be added later in this patchset */
Wouldn't it be better to just hold off and add it then instead of
adding it as a comment?
> +};
> +
> struct xdp_rxq_info {
> struct net_device *dev;
> u32 queue_index;
> u32 reg_state;
> + struct xdp_mem_info mem;
> } ____cacheline_aligned; /* perf critical, avoid false-sharing */
>
> +
> +static inline
> +void xdp_return_frame(void *data, struct xdp_mem_info *mem)
> +{
> + if (mem->type == MEM_TYPE_PAGE_SHARED)
> + page_frag_free(data);
> +
> + if (mem->type == MEM_TYPE_PAGE_ORDER0) {
> + struct page *page = virt_to_page(data); /* Assumes order0 page*/
> +
> + put_page(page);
> + }
Actually page_frag_free would probably work for either one. Also is it
safe to assume that the page is order 0? Are the only users of
compound pages that support XDP also only supporting the page fragment
setup?
Also you probably don't need put_page. It might be better to use
__free_page if you are certain the pages are coming from the Rx path
of drivers and don't have any special destructors associated with
them.
> +}
> +
> int xdp_rxq_info_reg(struct xdp_rxq_info *xdp_rxq,
> struct net_device *dev, u32 queue_index);
> void xdp_rxq_info_unreg(struct xdp_rxq_info *xdp_rxq);
> void xdp_rxq_info_unused(struct xdp_rxq_info *xdp_rxq);
> bool xdp_rxq_info_is_reg(struct xdp_rxq_info *xdp_rxq);
> +int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
> + enum mem_type type, void *allocator);
>
> #endif /* __LINUX_NET_XDP_H__ */
> diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
> index a4bb0b34375a..3e4bbcbe3e86 100644
> --- a/kernel/bpf/cpumap.c
> +++ b/kernel/bpf/cpumap.c
> @@ -19,6 +19,7 @@
> #include <linux/bpf.h>
> #include <linux/filter.h>
> #include <linux/ptr_ring.h>
> +#include <net/xdp.h>
>
> #include <linux/sched.h>
> #include <linux/workqueue.h>
> @@ -137,27 +138,6 @@ static struct bpf_map *cpu_map_alloc(union bpf_attr *attr)
> return ERR_PTR(err);
> }
>
> -static void __cpu_map_queue_destructor(void *ptr)
> -{
> - /* The tear-down procedure should have made sure that queue is
> - * empty. See __cpu_map_entry_replace() and work-queue
> - * invoked cpu_map_kthread_stop(). Catch any broken behaviour
> - * gracefully and warn once.
> - */
> - if (WARN_ON_ONCE(ptr))
> - page_frag_free(ptr);
> -}
> -
> -static void put_cpu_map_entry(struct bpf_cpu_map_entry *rcpu)
> -{
> - if (atomic_dec_and_test(&rcpu->refcnt)) {
> - /* The queue should be empty at this point */
> - ptr_ring_cleanup(rcpu->queue, __cpu_map_queue_destructor);
> - kfree(rcpu->queue);
> - kfree(rcpu);
> - }
> -}
> -
> static void get_cpu_map_entry(struct bpf_cpu_map_entry *rcpu)
> {
> atomic_inc(&rcpu->refcnt);
> @@ -188,6 +168,10 @@ struct xdp_pkt {
> u16 len;
> u16 headroom;
> u16 metasize;
> + /* Lifetime of xdp_rxq_info is limited to NAPI/enqueue time,
> + * while mem info is valid on remote CPU.
> + */
> + struct xdp_mem_info mem;
> struct net_device *dev_rx;
> };
>
> @@ -213,6 +197,9 @@ static struct xdp_pkt *convert_to_xdp_pkt(struct xdp_buff *xdp)
> xdp_pkt->headroom = headroom - sizeof(*xdp_pkt);
> xdp_pkt->metasize = metasize;
>
> + /* rxq only valid until napi_schedule ends, convert to xdp_mem_info */
> + xdp_pkt->mem = xdp->rxq->mem;
> +
> return xdp_pkt;
> }
>
> @@ -265,6 +252,31 @@ static struct sk_buff *cpu_map_build_skb(struct bpf_cpu_map_entry *rcpu,
> return skb;
> }
>
> +static void __cpu_map_ring_cleanup(struct ptr_ring *ring)
> +{
> + /* The tear-down procedure should have made sure that queue is
> + * empty. See __cpu_map_entry_replace() and work-queue
> + * invoked cpu_map_kthread_stop(). Catch any broken behaviour
> + * gracefully and warn once.
> + */
> + struct xdp_pkt *xdp_pkt;
> +
> + while ((xdp_pkt = ptr_ring_consume(ring)))
> + if (WARN_ON_ONCE(xdp_pkt))
> + xdp_return_frame(xdp_pkt, &xdp_pkt->mem);
> +}
> +
> +static void put_cpu_map_entry(struct bpf_cpu_map_entry *rcpu)
> +{
> + if (atomic_dec_and_test(&rcpu->refcnt)) {
> + /* The queue should be empty at this point */
> + __cpu_map_ring_cleanup(rcpu->queue);
> + ptr_ring_cleanup(rcpu->queue, NULL);
> + kfree(rcpu->queue);
> + kfree(rcpu);
> + }
> +}
> +
> static int cpu_map_kthread_run(void *data)
> {
> struct bpf_cpu_map_entry *rcpu = data;
> @@ -307,7 +319,7 @@ static int cpu_map_kthread_run(void *data)
>
> skb = cpu_map_build_skb(rcpu, xdp_pkt);
> if (!skb) {
> - page_frag_free(xdp_pkt);
> + xdp_return_frame(xdp_pkt, &xdp_pkt->mem);
> continue;
> }
>
> @@ -604,13 +616,13 @@ static int bq_flush_to_queue(struct bpf_cpu_map_entry *rcpu,
> spin_lock(&q->producer_lock);
>
> for (i = 0; i < bq->count; i++) {
> - void *xdp_pkt = bq->q[i];
> + struct xdp_pkt *xdp_pkt = bq->q[i];
> int err;
>
> err = __ptr_ring_produce(q, xdp_pkt);
> if (err) {
> drops++;
> - page_frag_free(xdp_pkt); /* Free xdp_pkt */
> + xdp_return_frame(xdp_pkt->data, &xdp_pkt->mem);
> }
> processed++;
> }
> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index 097a0f74e004..9eee0c431126 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -71,3 +71,21 @@ bool xdp_rxq_info_is_reg(struct xdp_rxq_info *xdp_rxq)
> return (xdp_rxq->reg_state == REG_STATE_REGISTERED);
> }
> EXPORT_SYMBOL_GPL(xdp_rxq_info_is_reg);
> +
> +int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
> + enum mem_type type, void *allocator)
> +{
> + if (type >= MEM_TYPE_MAX)
> + return -EINVAL;
> +
> + xdp_rxq->mem.type = type;
> +
> + if (allocator)
> + return -EOPNOTSUPP;
> +
> + /* TODO: Allocate an ID that maps to allocator pointer
> + * See: https://www.kernel.org/doc/html/latest/core-api/idr.html
> + */
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(xdp_rxq_info_reg_mem_model);
>
^ permalink raw reply
* Re: [PATCH net-next] net: bridge: fix direct access to bridge vlan_enabled and use helper
From: David Miller @ 2018-03-23 16:41 UTC (permalink / raw)
To: nikolay; +Cc: netdev, roopa, bridge, 3chas3
In-Reply-To: <1521822426-18563-1-git-send-email-nikolay@cumulusnetworks.com>
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Date: Fri, 23 Mar 2018 18:27:06 +0200
> We need to use br_vlan_enabled() helper otherwise we'll break builds
> without bridge vlans:
> net/bridge//br_if.c: In function ‘br_mtu’:
> net/bridge//br_if.c:458:8: error: ‘const struct net_bridge’ has no
> member named ‘vlan_enabled’
> if (br->vlan_enabled)
> ^
> net/bridge//br_if.c:462:1: warning: control reaches end of non-void
> function [-Wreturn-type]
> }
> ^
> scripts/Makefile.build:324: recipe for target 'net/bridge//br_if.o'
> failed
>
> Fixes: 419d14af9e07 ("bridge: Allow max MTU when multiple VLANs present")
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Applied, thanks for responding to this issue so quickly.
^ permalink raw reply
* Re: [PATCH v5 3/5] bnx2x: Eliminate duplicate barriers on weakly-ordered archs
From: David Miller @ 2018-03-23 16:43 UTC (permalink / raw)
To: okaya
Cc: netdev, timur, sulrich, linux-arm-msm, linux-arm-kernel,
ariel.elior, everest-linux-l2, linux-kernel
In-Reply-To: <cb6a0522-2c53-b252-49db-5779d24963c6@codeaurora.org>
From: Sinan Kaya <okaya@codeaurora.org>
Date: Fri, 23 Mar 2018 12:31:12 -0400
> Sorry, you got me confused now.
>
> If you look at the code closer, you'll see this.
>
> wmb();
>
> txdata->tx_db.data.prod += nbd;
> barrier();
>
> DOORBELL(bp, txdata->cid, txdata->tx_db.raw);
>
> and you also asked me to rename DOORBELL to DOORBELL_RELAXED() to make
> it obvious that we have a relaxed operator inside the macro.
This still doesn't match the stated pattern.
wmb();
/* no other memory or I/O or IOMEM operation */
writel();
There is a write to a producer index there and then no non-compiler
barrier or any kind before the writel().
So, in fact, it might really need that implicit writel() barrier here!
^ permalink raw reply
* Re: [bpf-next V5 PATCH 03/15] ixgbe: use xdp_return_frame API
From: Alexander Duyck @ 2018-03-23 16:46 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Netdev, BjörnTöpel, Karlsson, Magnus, Eugenia Emantayev,
Jason Wang, John Fastabend, Eran Ben Elisha, Saeed Mahameed,
Gal Pressman, Daniel Borkmann, Alexei Starovoitov, Tariq Toukan
In-Reply-To: <152180749411.20167.16232784042872304317.stgit@firesoul>
On Fri, Mar 23, 2018 at 5:18 AM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> Extend struct ixgbe_tx_buffer to store the xdp_mem_info.
>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe.h | 1 +
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 6 ++++--
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> index c1e3a0039ea5..cbc20f199364 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> @@ -249,6 +249,7 @@ struct ixgbe_tx_buffer {
> DEFINE_DMA_UNMAP_ADDR(dma);
> DEFINE_DMA_UNMAP_LEN(len);
> u32 tx_flags;
> + struct xdp_mem_info xdp_mem;
Instead of increasing the size of this structure it might make more
sense to find free space somewhere in side of it.
One thing you could probably look at doing is making it so that the
gso_segs, protocol, and tx_flags fields could be put into an anonymous
structure that is then part of a union with the xdp_mem_info
structure. Then you would just have to tweak things slightly so that
the else section for the ring_is_xdp block pulls the code just above
it into that section so that those fields are not read.
You would need to flip the dma, length, and type field ordering but
that shouldn't be much of an issue and it should have no effect on
performance since they are all in the same 16 byte aligned block
anyway.
> };
>
> struct ixgbe_rx_buffer {
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 85369423452d..45520eb503ee 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -1207,7 +1207,7 @@ static bool ixgbe_clean_tx_irq(struct ixgbe_q_vector *q_vector,
>
> /* free the skb */
> if (ring_is_xdp(tx_ring))
> - page_frag_free(tx_buffer->data);
> + xdp_return_frame(tx_buffer->data, &tx_buffer->xdp_mem);
Based on my suggestion above this section would have:
total_packets++;
> else
> napi_consume_skb(tx_buffer->skb, napi_budget);
This section would pull the lines that read gso_segs and tx_flags down
into this else statement so that the fields go unused.
>
> @@ -5787,7 +5787,7 @@ static void ixgbe_clean_tx_ring(struct ixgbe_ring *tx_ring)
>
> /* Free all the Tx ring sk_buffs */
> if (ring_is_xdp(tx_ring))
> - page_frag_free(tx_buffer->data);
> + xdp_return_frame(tx_buffer->data, &tx_buffer->xdp_mem);
> else
> dev_kfree_skb_any(tx_buffer->skb);
>
> @@ -8351,6 +8351,8 @@ static int ixgbe_xmit_xdp_ring(struct ixgbe_adapter *adapter,
> dma_unmap_len_set(tx_buffer, len, len);
> dma_unmap_addr_set(tx_buffer, dma, dma);
> tx_buffer->data = xdp->data;
> + tx_buffer->xdp_mem = xdp->rxq->mem;
> +
It would work out better if you moved this up to the lines that are
setting the tx_buffer fields. Really you could probably pull the line
that is recording xdp->data up as well.
> tx_desc->read.buffer_addr = cpu_to_le64(dma);
>
> /* put descriptor type bits */
>
^ permalink raw reply
* [PATCH net-next v2 0/3] Drop NETDEV_UNREGISTER_FINAL (was unnamed)
From: Kirill Tkhai @ 2018-03-23 16:47 UTC (permalink / raw)
To: davem, Michal.Kalderon, Ariel.Elior, dledford, jgg, benve,
1dgoodell, daniel, jakub.kicinski, ast, edumazet, linux,
john.fastabend, brouer, dsahern, netdev, ktkhai
This series drops unused NETDEV_UNREGISTER_FINAL
after some preparations.
v2: New patch [2/3]. Use switch() in [1/3].
The first version was acked by Jason Gunthorpe,
and [1/3] was acked by David Ahern.
Since there are differences to v1, I haven't added
Acked-by tags of people. It would be nice, if you
fill OK to tag v2 too.
Thanks,
Kirill
---
Kirill Tkhai (3):
net: Make NETDEV_XXX commands enum { }
infiniband: Replace usnic_ib_netdev_event_to_string() with netdev_cmd_to_name()
net: Drop NETDEV_UNREGISTER_FINAL
drivers/infiniband/hw/qedr/main.c | 4 +-
drivers/infiniband/hw/usnic/usnic_ib_main.c | 28 ++---------
include/linux/netdevice.h | 68 ++++++++++++++-------------
include/rdma/ib_verbs.h | 4 +-
net/core/dev.c | 25 ++++++++--
5 files changed, 63 insertions(+), 66 deletions(-)
--
Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
^ permalink raw reply
* Re: [PATCH 0/2] bpf: Change print_bpf_insn interface
From: Daniel Borkmann @ 2018-03-23 16:47 UTC (permalink / raw)
To: Quentin Monnet, Jiri Olsa, Alexei Starovoitov; +Cc: lkml, netdev
In-Reply-To: <f5586965-1447-ba8e-a27c-398b221f18e3@netronome.com>
On 03/23/2018 02:34 PM, Quentin Monnet wrote:
> 2018-03-23 11:41 UTC+0100 ~ Jiri Olsa <jolsa@kernel.org>
>> hi,
>> this patchset removes struct bpf_verifier_env argument
>> from print_bpf_insn function (patch 1) and changes user
>> space bpftool user to use it that way (patch 2).
>>
>> thanks,
>> jirka
>>
>> ---
>> Jiri Olsa (2):
>> bpf: Remove struct bpf_verifier_env argument from print_bpf_insn
>> bpftool: Adjust to new print_bpf_insn interface
>>
>> kernel/bpf/disasm.c | 52 ++++++++++++++++++++++++++--------------------------
>> kernel/bpf/disasm.h | 5 +----
>> kernel/bpf/verifier.c | 44 +++++++++++++++++++++++++++-----------------
>> tools/bpf/bpftool/xlated_dumper.c | 12 ++++++------
>> 4 files changed, 60 insertions(+), 53 deletions(-)
>>
>
> Thanks, this version looks good to me. Please keep the "Reviewed-by"
> tags when resubmitting new versions of your patch sets :)
>
> For the series:
> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
Applied to bpf-next, thanks everyone!
^ permalink raw reply
* [PATCH net-next v2 3/3] net: Drop NETDEV_UNREGISTER_FINAL
From: Kirill Tkhai @ 2018-03-23 16:47 UTC (permalink / raw)
To: davem, Michal.Kalderon, Ariel.Elior, dledford, jgg, benve,
1dgoodell, daniel, jakub.kicinski, ast, edumazet, linux,
john.fastabend, brouer, dsahern, netdev, ktkhai
In-Reply-To: <152182320819.2767.12723786642825837986.stgit@localhost.localdomain>
Last user is gone after bdf5bd7f2132 "rds: tcp: remove
register_netdevice_notifier infrastructure.", so we can
remove this netdevice command. This allows to delete
rtnl_lock() in netdev_run_todo(), which is hot path for
net namespace unregistration.
dev_change_net_namespace() and netdev_wait_allrefs()
have rcu_barrier() before NETDEV_UNREGISTER_FINAL call,
and the source commits say they were introduced to
delemit the call with NETDEV_UNREGISTER, but this patch
leaves them on the places, since they require additional
analysis, whether we need in them for something else.
Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
drivers/infiniband/hw/qedr/main.c | 4 ++--
include/linux/netdevice.h | 1 -
include/rdma/ib_verbs.h | 4 ++--
net/core/dev.c | 7 -------
4 files changed, 4 insertions(+), 12 deletions(-)
diff --git a/drivers/infiniband/hw/qedr/main.c b/drivers/infiniband/hw/qedr/main.c
index db4bf97c0e15..eb32abb0099a 100644
--- a/drivers/infiniband/hw/qedr/main.c
+++ b/drivers/infiniband/hw/qedr/main.c
@@ -90,8 +90,8 @@ static struct net_device *qedr_get_netdev(struct ib_device *dev, u8 port_num)
dev_hold(qdev->ndev);
/* The HW vendor's device driver must guarantee
- * that this function returns NULL before the net device reaches
- * NETDEV_UNREGISTER_FINAL state.
+ * that this function returns NULL before the net device has finished
+ * NETDEV_UNREGISTER state.
*/
return qdev->ndev;
}
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index dd5a04c971d5..2a2d9cf50aa2 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2336,7 +2336,6 @@ enum netdev_cmd {
NETDEV_PRE_TYPE_CHANGE,
NETDEV_POST_TYPE_CHANGE,
NETDEV_POST_INIT,
- NETDEV_UNREGISTER_FINAL,
NETDEV_RELEASE,
NETDEV_NOTIFY_PEERS,
NETDEV_JOIN,
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 73b2387e3f74..a1c5e8320f86 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -2126,8 +2126,8 @@ struct ib_device {
* net device of device @device at port @port_num or NULL if such
* a net device doesn't exist. The vendor driver should call dev_hold
* on this net device. The HW vendor's device driver must guarantee
- * that this function returns NULL before the net device reaches
- * NETDEV_UNREGISTER_FINAL state.
+ * that this function returns NULL before the net device has finished
+ * NETDEV_UNREGISTER state.
*/
struct net_device *(*get_netdev)(struct ib_device *device,
u8 port_num);
diff --git a/net/core/dev.c b/net/core/dev.c
index eb8ff44f46d6..cb4d2ceb36fb 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1584,7 +1584,6 @@ const char *netdev_cmd_to_name(enum netdev_cmd cmd)
N(RESEND_IGMP) N(PRECHANGEMTU) N(CHANGEINFODATA) N(BONDING_INFO)
N(PRECHANGEUPPER) N(CHANGELOWERSTATE) N(UDP_TUNNEL_PUSH_INFO)
N(UDP_TUNNEL_DROP_INFO) N(CHANGE_TX_QUEUE_LEN)
- N(UNREGISTER_FINAL)
};
#undef N
return "UNKNOWN_NETDEV_EVENT";
@@ -8089,7 +8088,6 @@ static void netdev_wait_allrefs(struct net_device *dev)
rcu_barrier();
rtnl_lock();
- call_netdevice_notifiers(NETDEV_UNREGISTER_FINAL, dev);
if (test_bit(__LINK_STATE_LINKWATCH_PENDING,
&dev->state)) {
/* We must not have linkwatch events
@@ -8161,10 +8159,6 @@ void netdev_run_todo(void)
= list_first_entry(&list, struct net_device, todo_list);
list_del(&dev->todo_list);
- rtnl_lock();
- call_netdevice_notifiers(NETDEV_UNREGISTER_FINAL, dev);
- __rtnl_unlock();
-
if (unlikely(dev->reg_state != NETREG_UNREGISTERING)) {
pr_err("network todo '%s' but state %d\n",
dev->name, dev->reg_state);
@@ -8606,7 +8600,6 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char
*/
call_netdevice_notifiers(NETDEV_UNREGISTER, dev);
rcu_barrier();
- call_netdevice_notifiers(NETDEV_UNREGISTER_FINAL, dev);
new_nsid = peernet2id_alloc(dev_net(dev), net);
/* If there is an ifindex conflict assign a new one */
^ permalink raw reply related
* [PATCH net-next v2 2/3] infiniband: Replace usnic_ib_netdev_event_to_string() with netdev_cmd_to_name()
From: Kirill Tkhai @ 2018-03-23 16:47 UTC (permalink / raw)
To: davem, Michal.Kalderon, Ariel.Elior, dledford, jgg, benve,
1dgoodell, daniel, jakub.kicinski, ast, edumazet, linux,
john.fastabend, brouer, dsahern, netdev, ktkhai
In-Reply-To: <152182320819.2767.12723786642825837986.stgit@localhost.localdomain>
This function just calls netdev_cmd_to_name().
Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
drivers/infiniband/hw/usnic/usnic_ib_main.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/infiniband/hw/usnic/usnic_ib_main.c b/drivers/infiniband/hw/usnic/usnic_ib_main.c
index 5bf3b20eba25..ca5638091b55 100644
--- a/drivers/infiniband/hw/usnic/usnic_ib_main.c
+++ b/drivers/infiniband/hw/usnic/usnic_ib_main.c
@@ -95,11 +95,6 @@ void usnic_ib_log_vf(struct usnic_ib_vf *vf)
}
/* Start of netdev section */
-static inline const char *usnic_ib_netdev_event_to_string(unsigned long event)
-{
- return netdev_cmd_to_name(event);
-}
-
static void usnic_ib_qp_grp_modify_active_to_err(struct usnic_ib_dev *us_ibdev)
{
struct usnic_ib_ucontext *ctx;
@@ -172,7 +167,7 @@ static void usnic_ib_handle_usdev_event(struct usnic_ib_dev *us_ibdev,
ib_dispatch_event(&ib_event);
} else {
usnic_dbg("Ignoring %s on %s\n",
- usnic_ib_netdev_event_to_string(event),
+ netdev_cmd_to_name(event),
us_ibdev->ib_dev.name);
}
break;
@@ -209,7 +204,7 @@ static void usnic_ib_handle_usdev_event(struct usnic_ib_dev *us_ibdev,
break;
default:
usnic_dbg("Ignoring event %s on %s",
- usnic_ib_netdev_event_to_string(event),
+ netdev_cmd_to_name(event),
us_ibdev->ib_dev.name);
}
mutex_unlock(&us_ibdev->usdev_lock);
@@ -251,7 +246,7 @@ static int usnic_ib_handle_inet_event(struct usnic_ib_dev *us_ibdev,
switch (event) {
case NETDEV_DOWN:
usnic_info("%s via ip notifiers",
- usnic_ib_netdev_event_to_string(event));
+ netdev_cmd_to_name(event));
usnic_fwd_del_ipaddr(us_ibdev->ufdev);
usnic_ib_qp_grp_modify_active_to_err(us_ibdev);
ib_event.event = IB_EVENT_GID_CHANGE;
@@ -262,7 +257,7 @@ static int usnic_ib_handle_inet_event(struct usnic_ib_dev *us_ibdev,
case NETDEV_UP:
usnic_fwd_add_ipaddr(us_ibdev->ufdev, ifa->ifa_address);
usnic_info("%s via ip notifiers: ip %pI4",
- usnic_ib_netdev_event_to_string(event),
+ netdev_cmd_to_name(event),
&us_ibdev->ufdev->inaddr);
ib_event.event = IB_EVENT_GID_CHANGE;
ib_event.device = &us_ibdev->ib_dev;
@@ -271,7 +266,7 @@ static int usnic_ib_handle_inet_event(struct usnic_ib_dev *us_ibdev,
break;
default:
usnic_info("Ignoring event %s on %s",
- usnic_ib_netdev_event_to_string(event),
+ netdev_cmd_to_name(event),
us_ibdev->ib_dev.name);
}
mutex_unlock(&us_ibdev->usdev_lock);
^ permalink raw reply related
* Re: [PATCH net-next v2 0/3] Drop NETDEV_UNREGISTER_FINAL (was unnamed)
From: Jason Gunthorpe @ 2018-03-23 16:50 UTC (permalink / raw)
To: Kirill Tkhai
Cc: davem, Michal.Kalderon, Ariel.Elior, dledford, benve, 1dgoodell,
daniel, jakub.kicinski, ast, edumazet, linux, john.fastabend,
brouer, dsahern, netdev
In-Reply-To: <152182320819.2767.12723786642825837986.stgit@localhost.localdomain>
On Fri, Mar 23, 2018 at 07:47:09PM +0300, Kirill Tkhai wrote:
> This series drops unused NETDEV_UNREGISTER_FINAL
> after some preparations.
>
> v2: New patch [2/3]. Use switch() in [1/3].
>
> The first version was acked by Jason Gunthorpe,
> and [1/3] was acked by David Ahern.
>
> Since there are differences to v1, I haven't added
> Acked-by tags of people. It would be nice, if you
> fill OK to tag v2 too.
>
> Thanks,
> Kirill
>
> Kirill Tkhai (3):
> net: Make NETDEV_XXX commands enum { }
> infiniband: Replace usnic_ib_netdev_event_to_string() with netdev_cmd_to_name()
> net: Drop NETDEV_UNREGISTER_FINAL
>
> drivers/infiniband/hw/qedr/main.c | 4 +-
> drivers/infiniband/hw/usnic/usnic_ib_main.c | 28 ++---------
> include/linux/netdevice.h | 68 ++++++++++++++-------------
> include/rdma/ib_verbs.h | 4 +-
> net/core/dev.c | 25 ++++++++--
> 5 files changed, 63 insertions(+), 66 deletions(-)
All the infiniband stuff is OK to merge through netdev:
Acked-by: Jason Gunthorpe <jgg@mellanox>
Jason
^ permalink raw reply
* Re: [PATCH v5 3/5] bnx2x: Eliminate duplicate barriers on weakly-ordered archs
From: Sinan Kaya @ 2018-03-23 16:51 UTC (permalink / raw)
To: David Miller
Cc: netdev, timur, sulrich, linux-arm-msm, linux-arm-kernel,
ariel.elior, everest-linux-l2, linux-kernel
In-Reply-To: <20180323.124326.2170503491903886041.davem@davemloft.net>
On 3/23/2018 12:43 PM, David Miller wrote:
> From: Sinan Kaya <okaya@codeaurora.org>
> Date: Fri, 23 Mar 2018 12:31:12 -0400
>
>> Sorry, you got me confused now.
>>
>> If you look at the code closer, you'll see this.
>>
>> wmb();
>>
>> txdata->tx_db.data.prod += nbd;
>> barrier();
>>
>> DOORBELL(bp, txdata->cid, txdata->tx_db.raw);
>>
>> and you also asked me to rename DOORBELL to DOORBELL_RELAXED() to make
>> it obvious that we have a relaxed operator inside the macro.
>
> This still doesn't match the stated pattern.
I can certainly update the commit text for this or spin into its own
patch to make it obvious.
>
> wmb();
> /* no other memory or I/O or IOMEM operation */
> writel();
>
> There is a write to a producer index there and then no non-compiler
> barrier or any kind before the writel().
>
> So, in fact, it might really need that implicit writel() barrier here!
>
It could if txdata->tx_db was not a union. There is a data dependency
between txdata->tx_db.data.prod and txdata->tx_db.raw.
So, no reordering.
I can argue that barrier() here is useless in fact.
Anyhow, I'll spin this piece out of this patch so that we pay special
attention with a better description.
--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* Re: [PATCH net] ipv6: fix possible deadlock in rt6_age_examine_exception()
From: Wei Wang @ 2018-03-23 16:56 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Linux Kernel Network Developers, Eric Dumazet,
Martin KaFai Lau
In-Reply-To: <20180323145658.154636-1-edumazet@google.com>
On Fri, Mar 23, 2018 at 7:57 AM Eric Dumazet <edumazet@google.com> wrote:
> syzbot reported a LOCKDEP splat [1] in rt6_age_examine_exception()
> rt6_age_examine_exception() is called while rt6_exception_lock is held.
> This lock is the lower one in the lock hierarchy, thus we can not
> call dst_neigh_lookup() function, as it can fallback to neigh_create()
> We should instead do a pure RCU lookup. As a bonus we avoid
> a pair of atomic operations on neigh refcount.
> [1]
> WARNING: possible circular locking dependency detected
> 4.16.0-rc4+ #277 Not tainted
> syz-executor7/4015 is trying to acquire lock:
> (&ndev->lock){++--}, at: [<00000000416dce19>]
__ipv6_dev_mc_dec+0x45/0x350 net/ipv6/mcast.c:928
> but task is already holding lock:
> (&tbl->lock){++-.}, at: [<00000000b5cb1d65>] neigh_ifdown+0x3d/0x250
net/core/neighbour.c:292
> which lock already depends on the new lock.
> the existing dependency chain (in reverse order) is:
> -> #3 (&tbl->lock){++-.}:
> __raw_write_lock_bh include/linux/rwlock_api_smp.h:203 [inline]
> _raw_write_lock_bh+0x31/0x40 kernel/locking/spinlock.c:312
> __neigh_create+0x87e/0x1d90 net/core/neighbour.c:528
> neigh_create include/net/neighbour.h:315 [inline]
> ip6_neigh_lookup+0x9a7/0xba0 net/ipv6/route.c:228
> dst_neigh_lookup include/net/dst.h:405 [inline]
> rt6_age_examine_exception net/ipv6/route.c:1609 [inline]
> rt6_age_exceptions+0x381/0x660 net/ipv6/route.c:1645
> fib6_age+0xfb/0x140 net/ipv6/ip6_fib.c:2033
> fib6_clean_node+0x389/0x580 net/ipv6/ip6_fib.c:1919
> fib6_walk_continue+0x46c/0x8a0 net/ipv6/ip6_fib.c:1845
> fib6_walk+0x91/0xf0 net/ipv6/ip6_fib.c:1893
> fib6_clean_tree+0x1e6/0x340 net/ipv6/ip6_fib.c:1970
> __fib6_clean_all+0x1f4/0x3a0 net/ipv6/ip6_fib.c:1986
> fib6_clean_all net/ipv6/ip6_fib.c:1997 [inline]
> fib6_run_gc+0x16b/0x3c0 net/ipv6/ip6_fib.c:2053
> ndisc_netdev_event+0x3c2/0x4a0 net/ipv6/ndisc.c:1781
> notifier_call_chain+0x136/0x2c0 kernel/notifier.c:93
> __raw_notifier_call_chain kernel/notifier.c:394 [inline]
> raw_notifier_call_chain+0x2d/0x40 kernel/notifier.c:401
> call_netdevice_notifiers_info+0x32/0x70 net/core/dev.c:1707
> call_netdevice_notifiers net/core/dev.c:1725 [inline]
> __dev_notify_flags+0x262/0x430 net/core/dev.c:6960
> dev_change_flags+0xf5/0x140 net/core/dev.c:6994
> devinet_ioctl+0x126a/0x1ac0 net/ipv4/devinet.c:1080
> inet_ioctl+0x184/0x310 net/ipv4/af_inet.c:919
> sock_do_ioctl+0xef/0x390 net/socket.c:957
> sock_ioctl+0x36b/0x610 net/socket.c:1081
> vfs_ioctl fs/ioctl.c:46 [inline]
> do_vfs_ioctl+0x1b1/0x1520 fs/ioctl.c:686
> SYSC_ioctl fs/ioctl.c:701 [inline]
> SyS_ioctl+0x8f/0xc0 fs/ioctl.c:692
> do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
> entry_SYSCALL_64_after_hwframe+0x42/0xb7
> -> #2 (rt6_exception_lock){+.-.}:
> __raw_spin_lock_bh include/linux/spinlock_api_smp.h:135 [inline]
> _raw_spin_lock_bh+0x31/0x40 kernel/locking/spinlock.c:168
> spin_lock_bh include/linux/spinlock.h:315 [inline]
> rt6_flush_exceptions+0x21/0x210 net/ipv6/route.c:1367
> fib6_del_route net/ipv6/ip6_fib.c:1677 [inline]
> fib6_del+0x624/0x12c0 net/ipv6/ip6_fib.c:1761
> __ip6_del_rt+0xc7/0x120 net/ipv6/route.c:2980
> ip6_del_rt+0x132/0x1a0 net/ipv6/route.c:2993
> __ipv6_dev_ac_dec+0x3b1/0x600 net/ipv6/anycast.c:332
> ipv6_dev_ac_dec net/ipv6/anycast.c:345 [inline]
> ipv6_sock_ac_close+0x2b4/0x3e0 net/ipv6/anycast.c:200
> inet6_release+0x48/0x70 net/ipv6/af_inet6.c:433
> sock_release+0x8d/0x1e0 net/socket.c:594
> sock_close+0x16/0x20 net/socket.c:1149
> __fput+0x327/0x7e0 fs/file_table.c:209
> ____fput+0x15/0x20 fs/file_table.c:243
> task_work_run+0x199/0x270 kernel/task_work.c:113
> exit_task_work include/linux/task_work.h:22 [inline]
> do_exit+0x9bb/0x1ad0 kernel/exit.c:865
> do_group_exit+0x149/0x400 kernel/exit.c:968
> get_signal+0x73a/0x16d0 kernel/signal.c:2469
> do_signal+0x90/0x1e90 arch/x86/kernel/signal.c:809
> exit_to_usermode_loop+0x258/0x2f0 arch/x86/entry/common.c:162
> prepare_exit_to_usermode arch/x86/entry/common.c:196 [inline]
> syscall_return_slowpath arch/x86/entry/common.c:265 [inline]
> do_syscall_64+0x6ec/0x940 arch/x86/entry/common.c:292
> entry_SYSCALL_64_after_hwframe+0x42/0xb7
> -> #1 (&(&tb->tb6_lock)->rlock){+.-.}:
> __raw_spin_lock_bh include/linux/spinlock_api_smp.h:135 [inline]
> _raw_spin_lock_bh+0x31/0x40 kernel/locking/spinlock.c:168
> spin_lock_bh include/linux/spinlock.h:315 [inline]
> __ip6_ins_rt+0x56/0x90 net/ipv6/route.c:1007
> ip6_route_add+0x141/0x190 net/ipv6/route.c:2955
> addrconf_prefix_route+0x44f/0x620 net/ipv6/addrconf.c:2359
> fixup_permanent_addr net/ipv6/addrconf.c:3368 [inline]
> addrconf_permanent_addr net/ipv6/addrconf.c:3391 [inline]
> addrconf_notify+0x1ad2/0x2310 net/ipv6/addrconf.c:3460
> notifier_call_chain+0x136/0x2c0 kernel/notifier.c:93
> __raw_notifier_call_chain kernel/notifier.c:394 [inline]
> raw_notifier_call_chain+0x2d/0x40 kernel/notifier.c:401
> call_netdevice_notifiers_info+0x32/0x70 net/core/dev.c:1707
> call_netdevice_notifiers net/core/dev.c:1725 [inline]
> __dev_notify_flags+0x15d/0x430 net/core/dev.c:6958
> dev_change_flags+0xf5/0x140 net/core/dev.c:6994
> do_setlink+0xa22/0x3bb0 net/core/rtnetlink.c:2357
> rtnl_newlink+0xf37/0x1a50 net/core/rtnetlink.c:2965
> rtnetlink_rcv_msg+0x57f/0xb10 net/core/rtnetlink.c:4641
> netlink_rcv_skb+0x14b/0x380 net/netlink/af_netlink.c:2444
> rtnetlink_rcv+0x1c/0x20 net/core/rtnetlink.c:4659
> netlink_unicast_kernel net/netlink/af_netlink.c:1308 [inline]
> netlink_unicast+0x4c4/0x6b0 net/netlink/af_netlink.c:1334
> netlink_sendmsg+0xa4a/0xe60 net/netlink/af_netlink.c:1897
> sock_sendmsg_nosec net/socket.c:629 [inline]
> sock_sendmsg+0xca/0x110 net/socket.c:639
> ___sys_sendmsg+0x767/0x8b0 net/socket.c:2047
> __sys_sendmsg+0xe5/0x210 net/socket.c:2081
> SYSC_sendmsg net/socket.c:2092 [inline]
> SyS_sendmsg+0x2d/0x50 net/socket.c:2088
> do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
> entry_SYSCALL_64_after_hwframe+0x42/0xb7
> -> #0 (&ndev->lock){++--}:
> lock_acquire+0x1d5/0x580 kernel/locking/lockdep.c:3920
> __raw_write_lock_bh include/linux/rwlock_api_smp.h:203 [inline]
> _raw_write_lock_bh+0x31/0x40 kernel/locking/spinlock.c:312
> __ipv6_dev_mc_dec+0x45/0x350 net/ipv6/mcast.c:928
> ipv6_dev_mc_dec+0x110/0x1f0 net/ipv6/mcast.c:961
> pndisc_destructor+0x21a/0x340 net/ipv6/ndisc.c:392
> pneigh_ifdown net/core/neighbour.c:695 [inline]
> neigh_ifdown+0x149/0x250 net/core/neighbour.c:294
> rt6_disable_ip+0x537/0x700 net/ipv6/route.c:3874
> addrconf_ifdown+0x14b/0x14f0 net/ipv6/addrconf.c:3633
> addrconf_notify+0x5f8/0x2310 net/ipv6/addrconf.c:3557
> notifier_call_chain+0x136/0x2c0 kernel/notifier.c:93
> __raw_notifier_call_chain kernel/notifier.c:394 [inline]
> raw_notifier_call_chain+0x2d/0x40 kernel/notifier.c:401
> call_netdevice_notifiers_info+0x32/0x70 net/core/dev.c:1707
> call_netdevice_notifiers net/core/dev.c:1725 [inline]
> __dev_notify_flags+0x262/0x430 net/core/dev.c:6960
> dev_change_flags+0xf5/0x140 net/core/dev.c:6994
> devinet_ioctl+0x126a/0x1ac0 net/ipv4/devinet.c:1080
> inet_ioctl+0x184/0x310 net/ipv4/af_inet.c:919
> packet_ioctl+0x1ff/0x310 net/packet/af_packet.c:4066
> sock_do_ioctl+0xef/0x390 net/socket.c:957
> sock_ioctl+0x36b/0x610 net/socket.c:1081
> vfs_ioctl fs/ioctl.c:46 [inline]
> do_vfs_ioctl+0x1b1/0x1520 fs/ioctl.c:686
> SYSC_ioctl fs/ioctl.c:701 [inline]
> SyS_ioctl+0x8f/0xc0 fs/ioctl.c:692
> do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
> entry_SYSCALL_64_after_hwframe+0x42/0xb7
> other info that might help us debug this:
> Chain exists of:
> &ndev->lock --> rt6_exception_lock --> &tbl->lock
> Possible unsafe locking scenario:
> CPU0 CPU1
> ---- ----
> lock(&tbl->lock);
> lock(rt6_exception_lock);
> lock(&tbl->lock);
> lock(&ndev->lock);
> *** DEADLOCK ***
> 2 locks held by syz-executor7/4015:
> #0: (rtnl_mutex){+.+.}, at: [<00000000a2f16daa>] rtnl_lock+0x17/0x20
net/core/rtnetlink.c:74
> #1: (&tbl->lock){++-.}, at: [<00000000b5cb1d65>]
neigh_ifdown+0x3d/0x250 net/core/neighbour.c:292
> stack backtrace:
> CPU: 0 PID: 4015 Comm: syz-executor7 Not tainted 4.16.0-rc4+ #277
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:17 [inline]
> dump_stack+0x194/0x24d lib/dump_stack.c:53
> print_circular_bug.isra.38+0x2cd/0x2dc kernel/locking/lockdep.c:1223
> check_prev_add kernel/locking/lockdep.c:1863 [inline]
> check_prevs_add kernel/locking/lockdep.c:1976 [inline]
> validate_chain kernel/locking/lockdep.c:2417 [inline]
> __lock_acquire+0x30a8/0x3e00 kernel/locking/lockdep.c:3431
> lock_acquire+0x1d5/0x580 kernel/locking/lockdep.c:3920
> __raw_write_lock_bh include/linux/rwlock_api_smp.h:203 [inline]
> _raw_write_lock_bh+0x31/0x40 kernel/locking/spinlock.c:312
> __ipv6_dev_mc_dec+0x45/0x350 net/ipv6/mcast.c:928
> ipv6_dev_mc_dec+0x110/0x1f0 net/ipv6/mcast.c:961
> pndisc_destructor+0x21a/0x340 net/ipv6/ndisc.c:392
> pneigh_ifdown net/core/neighbour.c:695 [inline]
> neigh_ifdown+0x149/0x250 net/core/neighbour.c:294
> rt6_disable_ip+0x537/0x700 net/ipv6/route.c:3874
> addrconf_ifdown+0x14b/0x14f0 net/ipv6/addrconf.c:3633
> addrconf_notify+0x5f8/0x2310 net/ipv6/addrconf.c:3557
> notifier_call_chain+0x136/0x2c0 kernel/notifier.c:93
> __raw_notifier_call_chain kernel/notifier.c:394 [inline]
> raw_notifier_call_chain+0x2d/0x40 kernel/notifier.c:401
> call_netdevice_notifiers_info+0x32/0x70 net/core/dev.c:1707
> call_netdevice_notifiers net/core/dev.c:1725 [inline]
> __dev_notify_flags+0x262/0x430 net/core/dev.c:6960
> dev_change_flags+0xf5/0x140 net/core/dev.c:6994
> devinet_ioctl+0x126a/0x1ac0 net/ipv4/devinet.c:1080
> inet_ioctl+0x184/0x310 net/ipv4/af_inet.c:919
> packet_ioctl+0x1ff/0x310 net/packet/af_packet.c:4066
> sock_do_ioctl+0xef/0x390 net/socket.c:957
> sock_ioctl+0x36b/0x610 net/socket.c:1081
> vfs_ioctl fs/ioctl.c:46 [inline]
> do_vfs_ioctl+0x1b1/0x1520 fs/ioctl.c:686
> SYSC_ioctl fs/ioctl.c:701 [inline]
> SyS_ioctl+0x8f/0xc0 fs/ioctl.c:692
> do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
> entry_SYSCALL_64_after_hwframe+0x42/0xb7
> Fixes: c757faa8bfa2 ("ipv6: prepare fib6_age() for exception table")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Wei Wang <weiwan@google.com>
> Cc: Martin KaFai Lau <kafai@fb.com>
> ---
Nice fix. Thanks Eric.
Acked-by: Wei Wang <weiwan@google.com>
> net/ipv6/route.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index
b0d5c64e19780ce94feb112285ed1d85dbe07e9e..b33d057ac5eb2a85e19be59f0bceacf547cc9e59
100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -1626,11 +1626,10 @@ static void rt6_age_examine_exception(struct
rt6_exception_bucket *bucket,
> struct neighbour *neigh;
> __u8 neigh_flags = 0;
> - neigh = dst_neigh_lookup(&rt->dst, &rt->rt6i_gateway);
> - if (neigh) {
> + neigh = __ipv6_neigh_lookup_noref(rt->dst.dev,
&rt->rt6i_gateway);
> + if (neigh)
> neigh_flags = neigh->flags;
> - neigh_release(neigh);
> - }
> +
> if (!(neigh_flags & NTF_ROUTER)) {
> RT6_TRACE("purging route %p via non-router but
gateway\n",
> rt);
> @@ -1654,7 +1653,8 @@ void rt6_age_exceptions(struct rt6_info *rt,
> if (!rcu_access_pointer(rt->rt6i_exception_bucket))
> return;
> - spin_lock_bh(&rt6_exception_lock);
> + rcu_read_lock_bh();
> + spin_lock(&rt6_exception_lock);
> bucket = rcu_dereference_protected(rt->rt6i_exception_bucket,
> lockdep_is_held(&rt6_exception_lock));
> @@ -1668,7 +1668,8 @@ void rt6_age_exceptions(struct rt6_info *rt,
> bucket++;
> }
> }
> - spin_unlock_bh(&rt6_exception_lock);
> + spin_unlock(&rt6_exception_lock);
> + rcu_read_unlock_bh();
> }
> struct rt6_info *ip6_pol_route(struct net *net, struct fib6_table *table,
> --
> 2.17.0.rc0.231.g781580f067-goog
^ permalink raw reply
* Re: [PATCH net 0/3] mlxsw: Handle changes to MTU in GRE tunnels
From: David Miller @ 2018-03-23 16:56 UTC (permalink / raw)
To: idosch; +Cc: netdev, jiri, lucien.xin, mlxsw
In-Reply-To: <20180322175335.26232-1-idosch@mellanox.com>
From: Ido Schimmel <idosch@mellanox.com>
Date: Thu, 22 Mar 2018 19:53:32 +0200
> Petr says:
>
> When offloading GRE tunnels, the MTU setting is kept fixed after the
> initial offload even as the slow-path configuration changed. Worse: the
> offloaded MTU setting is actually just a transient value set at the time
> of NETDEV_REGISTER of the tunnel. As of commit ffc2b6ee4174 ("ip_gre:
> fix IFLA_MTU ignored on NEWLINK"), that transient value is zero, and
> unless there's e.g. a VRF migration that prompts re-offload, it stays at
> zero, and all GRE packets end up trapping.
>
> Thus, in patch #1, change the way the MTU is changed post-registration,
> so that the full event protocol is observed. That way the drivers get to
> see the change and have a chance to react.
>
> In the remaining two patches, implement support for MTU change in mlxsw
> driver.
Series applied, thanks.
^ permalink raw reply
* Re: [bpf-next V5 PATCH 10/15] xdp: rhashtable with allocator ID to pointer mapping
From: Alexander Duyck @ 2018-03-23 16:56 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Netdev, BjörnTöpel, Karlsson, Magnus, Eugenia Emantayev,
Jason Wang, John Fastabend, Eran Ben Elisha, Saeed Mahameed,
Gal Pressman, Daniel Borkmann, Alexei Starovoitov, Tariq Toukan
In-Reply-To: <152180752969.20167.10855856167552164450.stgit@firesoul>
On Fri, Mar 23, 2018 at 5:18 AM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> Use the IDA infrastructure for getting a cyclic increasing ID number,
> that is used for keeping track of each registered allocator per
> RX-queue xdp_rxq_info. Instead of using the IDR infrastructure, which
> uses a radix tree, use a dynamic rhashtable, for creating ID to
> pointer lookup table, because this is faster.
>
> The problem that is being solved here is that, the xdp_rxq_info
> pointer (stored in xdp_buff) cannot be used directly, as the
> guaranteed lifetime is too short. The info is needed on a
> (potentially) remote CPU during DMA-TX completion time . In an
> xdp_frame the xdp_mem_info is stored, when it got converted from an
> xdp_buff, which is sufficient for the simple page refcnt based recycle
> schemes.
>
> For more advanced allocators there is a need to store a pointer to the
> registered allocator. Thus, there is a need to guard the lifetime or
> validity of the allocator pointer, which is done through this
> rhashtable ID map to pointer. The removal and validity of of the
> allocator and helper struct xdp_mem_allocator is guarded by RCU. The
> allocator will be created by the driver, and registered with
> xdp_rxq_info_reg_mem_model().
>
> It is up-to debate who is responsible for freeing the allocator
> pointer or invoking the allocator destructor function. In any case,
> this must happen via RCU freeing.
>
> Use the IDA infrastructure for getting a cyclic increasing ID number,
> that is used for keeping track of each registered allocator per
> RX-queue xdp_rxq_info.
>
> V4: Per req of Jason Wang
> - Use xdp_rxq_info_reg_mem_model() in all drivers implementing
> XDP_REDIRECT, even-though it's not strictly necessary when
> allocator==NULL for type MEM_TYPE_PAGE_SHARED (given it's zero).
>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 9 +
> drivers/net/tun.c | 6 +
> drivers/net/virtio_net.c | 7 +
> include/net/xdp.h | 15 --
> net/core/xdp.c | 230 ++++++++++++++++++++++++-
> 5 files changed, 248 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 45520eb503ee..ff069597fccf 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -6360,7 +6360,7 @@ int ixgbe_setup_rx_resources(struct ixgbe_adapter *adapter,
> struct device *dev = rx_ring->dev;
> int orig_node = dev_to_node(dev);
> int ring_node = -1;
> - int size;
> + int size, err;
>
> size = sizeof(struct ixgbe_rx_buffer) * rx_ring->count;
>
> @@ -6397,6 +6397,13 @@ int ixgbe_setup_rx_resources(struct ixgbe_adapter *adapter,
> rx_ring->queue_index) < 0)
> goto err;
>
> + err = xdp_rxq_info_reg_mem_model(&rx_ring->xdp_rxq,
> + MEM_TYPE_PAGE_SHARED, NULL);
> + if (err) {
> + xdp_rxq_info_unreg(&rx_ring->xdp_rxq);
> + goto err;
> + }
> +
> rx_ring->xdp_prog = adapter->xdp_prog;
>
> return 0;
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 6750980d9f30..81fddf9cc58f 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -846,6 +846,12 @@ static int tun_attach(struct tun_struct *tun, struct file *file,
> tun->dev, tfile->queue_index);
> if (err < 0)
> goto out;
> + err = xdp_rxq_info_reg_mem_model(&tfile->xdp_rxq,
> + MEM_TYPE_PAGE_SHARED, NULL);
> + if (err < 0) {
> + xdp_rxq_info_unreg(&tfile->xdp_rxq);
> + goto out;
> + }
> err = 0;
> }
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 6c4220450506..48c86accd3b8 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1312,6 +1312,13 @@ static int virtnet_open(struct net_device *dev)
> if (err < 0)
> return err;
>
> + err = xdp_rxq_info_reg_mem_model(&vi->rq[i].xdp_rxq,
> + MEM_TYPE_PAGE_SHARED, NULL);
> + if (err < 0) {
> + xdp_rxq_info_unreg(&vi->rq[i].xdp_rxq);
> + return err;
> + }
> +
> virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi);
> virtnet_napi_tx_enable(vi, vi->sq[i].vq, &vi->sq[i].napi);
> }
> diff --git a/include/net/xdp.h b/include/net/xdp.h
> index bc0cb97e20dc..859aa9b737fe 100644
> --- a/include/net/xdp.h
> +++ b/include/net/xdp.h
> @@ -41,7 +41,7 @@ enum mem_type {
>
> struct xdp_mem_info {
> u32 type; /* enum mem_type, but known size type */
> - /* u32 id; will be added later in this patchset */
> + u32 id;
> };
>
> struct xdp_rxq_info {
> @@ -100,18 +100,7 @@ struct xdp_frame *convert_to_xdp_frame(struct xdp_buff *xdp)
> return xdp_frame;
> }
>
> -static inline
> -void xdp_return_frame(void *data, struct xdp_mem_info *mem)
> -{
> - if (mem->type == MEM_TYPE_PAGE_SHARED)
> - page_frag_free(data);
> -
> - if (mem->type == MEM_TYPE_PAGE_ORDER0) {
> - struct page *page = virt_to_page(data); /* Assumes order0 page*/
> -
> - put_page(page);
> - }
> -}
> +void xdp_return_frame(void *data, struct xdp_mem_info *mem);
>
> int xdp_rxq_info_reg(struct xdp_rxq_info *xdp_rxq,
> struct net_device *dev, u32 queue_index);
> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index 9eee0c431126..06a5b39491ad 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -5,6 +5,9 @@
> */
> #include <linux/types.h>
> #include <linux/mm.h>
> +#include <linux/slab.h>
> +#include <linux/idr.h>
> +#include <linux/rhashtable.h>
>
> #include <net/xdp.h>
>
> @@ -13,6 +16,99 @@
> #define REG_STATE_UNREGISTERED 0x2
> #define REG_STATE_UNUSED 0x3
>
> +DEFINE_IDA(mem_id_pool);
> +static DEFINE_MUTEX(mem_id_lock);
> +#define MEM_ID_MAX 0xFFFE
> +#define MEM_ID_MIN 1
> +static int mem_id_next = MEM_ID_MIN;
> +
> +static bool mem_id_init; /* false */
> +static struct rhashtable *mem_id_ht;
> +
> +struct xdp_mem_allocator {
> + struct xdp_mem_info mem;
> + void *allocator;
> + struct rhash_head node;
> + struct rcu_head rcu;
> +};
> +
> +static u32 xdp_mem_id_hashfn(const void *data, u32 len, u32 seed)
> +{
> + const u32 *k = data;
> + const u32 key = *k;
> +
> + BUILD_BUG_ON(FIELD_SIZEOF(struct xdp_mem_allocator, mem.id)
> + != sizeof(u32));
> +
> + /* Use cyclic increasing ID as direct hash key, see rht_bucket_index */
> + return key << RHT_HASH_RESERVED_SPACE;
> +}
> +
> +static int xdp_mem_id_cmp(struct rhashtable_compare_arg *arg,
> + const void *ptr)
> +{
> + const struct xdp_mem_allocator *xa = ptr;
> + u32 mem_id = *(u32 *)arg->key;
> +
> + return xa->mem.id != mem_id;
> +}
> +
> +static const struct rhashtable_params mem_id_rht_params = {
> + .nelem_hint = 64,
> + .head_offset = offsetof(struct xdp_mem_allocator, node),
> + .key_offset = offsetof(struct xdp_mem_allocator, mem.id),
> + .key_len = FIELD_SIZEOF(struct xdp_mem_allocator, mem.id),
> + .max_size = MEM_ID_MAX,
> + .min_size = 8,
> + .automatic_shrinking = true,
> + .hashfn = xdp_mem_id_hashfn,
> + .obj_cmpfn = xdp_mem_id_cmp,
> +};
> +
> +void __xdp_mem_allocator_rcu_free(struct rcu_head *rcu)
> +{
> + struct xdp_mem_allocator *xa;
> +
> + xa = container_of(rcu, struct xdp_mem_allocator, rcu);
> +
> + /* Allow this ID to be reused */
> + ida_simple_remove(&mem_id_pool, xa->mem.id);
> +
> + /* TODO: Depending on allocator type/pointer free resources */
> +
> + /* Poison memory */
> + xa->mem.id = 0xFFFF;
> + xa->mem.type = 0xF0F0;
> + xa->allocator = (void *)0xDEAD9001;
> +
> + kfree(xa);
> +}
> +
> +void __xdp_rxq_info_unreg_mem_model(struct xdp_rxq_info *xdp_rxq)
> +{
> + struct xdp_mem_allocator *xa;
> + int id = xdp_rxq->mem.id;
> + int err;
> +
> + if (id == 0)
> + return;
> +
> + mutex_lock(&mem_id_lock);
> +
> + xa = rhashtable_lookup(mem_id_ht, &id, mem_id_rht_params);
> + if (!xa) {
> + mutex_unlock(&mem_id_lock);
> + return;
> + }
> +
> + err = rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params);
> + WARN_ON(err);
> +
> + call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
> +
> + mutex_unlock(&mem_id_lock);
> +}
> +
> void xdp_rxq_info_unreg(struct xdp_rxq_info *xdp_rxq)
> {
> /* Simplify driver cleanup code paths, allow unreg "unused" */
> @@ -21,8 +117,14 @@ void xdp_rxq_info_unreg(struct xdp_rxq_info *xdp_rxq)
>
> WARN(!(xdp_rxq->reg_state == REG_STATE_REGISTERED), "Driver BUG");
>
> + __xdp_rxq_info_unreg_mem_model(xdp_rxq);
> +
> xdp_rxq->reg_state = REG_STATE_UNREGISTERED;
> xdp_rxq->dev = NULL;
> +
> + /* Reset mem info to defaults */
> + xdp_rxq->mem.id = 0;
> + xdp_rxq->mem.type = 0;
> }
> EXPORT_SYMBOL_GPL(xdp_rxq_info_unreg);
>
> @@ -72,20 +174,138 @@ bool xdp_rxq_info_is_reg(struct xdp_rxq_info *xdp_rxq)
> }
> EXPORT_SYMBOL_GPL(xdp_rxq_info_is_reg);
>
> +int __mem_id_init_hash_table(void)
> +{
> + struct rhashtable *rht;
> + int ret;
> +
> + if (unlikely(mem_id_init))
> + return 0;
> +
> + rht = kzalloc(sizeof(*rht), GFP_KERNEL);
> + if (!rht)
> + return -ENOMEM;
> +
> + ret = rhashtable_init(rht, &mem_id_rht_params);
> + if (ret < 0) {
> + kfree(rht);
> + return ret;
> + }
> + mem_id_ht = rht;
> + smp_mb(); /* mutex lock should provide enough pairing */
> + mem_id_init = true;
> +
> + return 0;
> +}
> +
> +/* Allocate a cyclic ID that maps to allocator pointer.
> + * See: https://www.kernel.org/doc/html/latest/core-api/idr.html
> + *
> + * Caller must lock mem_id_lock.
> + */
> +static int __mem_id_cyclic_get(gfp_t gfp)
> +{
> + int retries = 1;
> + int id;
> +
> +again:
> + id = ida_simple_get(&mem_id_pool, mem_id_next, MEM_ID_MAX, gfp);
> + if (id < 0) {
> + if (id == -ENOSPC) {
> + /* Cyclic allocator, reset next id */
> + if (retries--) {
> + mem_id_next = MEM_ID_MIN;
> + goto again;
> + }
> + }
> + return id; /* errno */
> + }
> + mem_id_next = id + 1;
> +
> + return id;
> +}
> +
> int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
> enum mem_type type, void *allocator)
> {
> + struct xdp_mem_allocator *xdp_alloc;
> + gfp_t gfp = GFP_KERNEL;
> + int id, errno, ret;
> + void *ptr;
> +
> + if (xdp_rxq->reg_state != REG_STATE_REGISTERED) {
> + WARN(1, "Missing register, driver bug");
> + return -EFAULT;
> + }
> +
> if (type >= MEM_TYPE_MAX)
> return -EINVAL;
>
> xdp_rxq->mem.type = type;
>
> - if (allocator)
> - return -EOPNOTSUPP;
> + if (!allocator)
> + return 0;
> +
> + /* Delay init of rhashtable to save memory if feature isn't used */
> + if (!mem_id_init) {
> + mutex_lock(&mem_id_lock);
> + ret = __mem_id_init_hash_table();
> + mutex_unlock(&mem_id_lock);
> + if (ret < 0) {
> + WARN_ON(1);
> + return ret;
> + }
> + }
> +
> + xdp_alloc = kzalloc(sizeof(*xdp_alloc), gfp);
> + if (!xdp_alloc)
> + return -ENOMEM;
> +
> + mutex_lock(&mem_id_lock);
> + id = __mem_id_cyclic_get(gfp);
> + if (id < 0) {
> + errno = id;
> + goto err;
> + }
> + xdp_rxq->mem.id = id;
> + xdp_alloc->mem = xdp_rxq->mem;
> + xdp_alloc->allocator = allocator;
> +
> + /* Insert allocator into ID lookup table */
> + ptr = rhashtable_insert_slow(mem_id_ht, &id, &xdp_alloc->node);
> + if (IS_ERR(ptr)) {
> + errno = PTR_ERR(ptr);
> + goto err;
> + }
> +
> + mutex_unlock(&mem_id_lock);
>
> - /* TODO: Allocate an ID that maps to allocator pointer
> - * See: https://www.kernel.org/doc/html/latest/core-api/idr.html
> - */
> return 0;
> +err:
> + mutex_unlock(&mem_id_lock);
> + kfree(xdp_alloc);
> + return errno;
> }
> EXPORT_SYMBOL_GPL(xdp_rxq_info_reg_mem_model);
> +
> +void xdp_return_frame(void *data, struct xdp_mem_info *mem)
> +{
> + struct xdp_mem_allocator *xa;
> +
> + rcu_read_lock();
> + if (mem->id)
> + xa = rhashtable_lookup(mem_id_ht, &mem->id, mem_id_rht_params);
> + rcu_read_unlock();
> +
> + if (mem->type == MEM_TYPE_PAGE_SHARED) {
> + page_frag_free(data);
> + return;
> + }
> +
> + if (mem->type == MEM_TYPE_PAGE_ORDER0) {
> + struct page *page = virt_to_page(data); /* Assumes order0 page*/
> +
> + put_page(page);
> + }
> +}
> +EXPORT_SYMBOL_GPL(xdp_return_frame);
>
I'm not sure what the point is of getting the xa value if it is not
going to be used. Also I would assume there are types that won't even
need the hash table lookup. I would prefer to see this bit held off on
until you have something that actually needs it.
^ permalink raw reply
* Re: [PATCH net-next v2] tc-testing: add selftests for 'bpf' action
From: David Miller @ 2018-03-23 16:59 UTC (permalink / raw)
To: dcaratti; +Cc: mrv, bjb, jhs, netdev
In-Reply-To: <678d3ddd35780ade1c67c33de84f548858f45f15.1521741967.git.dcaratti@redhat.com>
From: Davide Caratti <dcaratti@redhat.com>
Date: Thu, 22 Mar 2018 19:12:19 +0100
> Test d959: Add cBPF action with valid bytecode
> Test f84a: Add cBPF action with invalid bytecode
> Test e939: Add eBPF action with valid object-file
> Test 282d: Add eBPF action with invalid object-file
> Test d819: Replace cBPF bytecode and action control
> Test 6ae3: Delete cBPF action
> Test 3e0d: List cBPF actions
> Test 55ce: Flush BPF actions
> Test ccc3: Add cBPF action with duplicate index
> Test 89c7: Add cBPF action with invalid index
> Test 7ab9: Add cBPF action with cookie
>
> Changes since v1:
> - use index=2^32-1 in test ccc3, add tests 7a89, 89c7 (thanks Roman Mashak)
> - added test 282d
>
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>
Applied, thanks Davide.
^ permalink raw reply
* Re: [PATCH net-next] mlxsw: spectrum_span: Fix initialization of struct mlxsw_sp_span_parms
From: David Miller @ 2018-03-23 17:00 UTC (permalink / raw)
To: idosch; +Cc: netdev, petrm, jiri, mlxsw
In-Reply-To: <20180322181447.27554-1-idosch@mellanox.com>
From: Ido Schimmel <idosch@mellanox.com>
Date: Thu, 22 Mar 2018 20:14:47 +0200
> From: Petr Machata <petrm@mellanox.com>
>
> Since the first element of struct mlxsw_sp_span_parms is a pointer,
> to zero-initialize this structure the correct notation is not = {0}, but
> rather = {NULL}, as reported by sparse.
>
> Signed-off-by: Petr Machata <petrm@mellanox.com>
> Acked-by: Jiri Pirko <jiri@mellanox.com>
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 0/2] Converting pernet_operations (part #11)
From: David Miller @ 2018-03-23 17:01 UTC (permalink / raw)
To: ktkhai; +Cc: yoshfuji, dhowells, netdev
In-Reply-To: <152174314653.22875.17369758119928369672.stgit@localhost.localdomain>
From: Kirill Tkhai <ktkhai@virtuozzo.com>
Date: Thu, 22 Mar 2018 21:34:37 +0300
> this series continues to review and to convert pernet_operations
> to make them possible to be executed in parallel for several
> net namespaces at the same time.
>
> I thought last series was last, but there is one
> new pernet_operations came to kernel. This is
> udp_sysctl_ops, and here we convert it.
>
> Also, David Howells acked rxrpc_net_ops, so I resend
> the patch in case of it should be queued by patchwork:
>
> https://www.spinics.net/lists/netdev/msg490678.html
Series applied, thank you.
^ permalink raw reply
* Re: [bpf-next V5 PATCH 12/15] xdp: allow page_pool as an allocator type in xdp_return_frame
From: Alexander Duyck @ 2018-03-23 17:02 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Netdev, BjörnTöpel, Karlsson, Magnus, Eugenia Emantayev,
Jason Wang, John Fastabend, Eran Ben Elisha, Saeed Mahameed,
Gal Pressman, Daniel Borkmann, Alexei Starovoitov, Tariq Toukan
In-Reply-To: <152180753987.20167.7085747286116186761.stgit@firesoul>
On Fri, Mar 23, 2018 at 5:18 AM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> New allocator type MEM_TYPE_PAGE_POOL for page_pool usage.
>
> The registered allocator page_pool pointer is not available directly
> from xdp_rxq_info, but it could be (if needed). For now, the driver
> should keep separate track of the page_pool pointer, which it should
> use for RX-ring page allocation.
>
> As suggested by Saeed, to maintain a symmetric API it is the drivers
> responsibility to allocate/create and free/destroy the page_pool.
> Thus, after the driver have called xdp_rxq_info_unreg(), it is drivers
> responsibility to free the page_pool, but with a RCU free call. This
> is done easily via the page_pool helper page_pool_destroy_rcu() (which
> avoids touching any driver code during the RCU callback, which could
> happen after the driver have been unloaded).
>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> ---
> include/net/xdp.h | 3 +++
> net/core/xdp.c | 23 ++++++++++++++++++++---
> 2 files changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/include/net/xdp.h b/include/net/xdp.h
> index 859aa9b737fe..98b55eaf8fd7 100644
> --- a/include/net/xdp.h
> +++ b/include/net/xdp.h
> @@ -36,6 +36,7 @@
> enum mem_type {
> MEM_TYPE_PAGE_SHARED = 0, /* Split-page refcnt based model */
> MEM_TYPE_PAGE_ORDER0, /* Orig XDP full page model */
> + MEM_TYPE_PAGE_POOL,
> MEM_TYPE_MAX,
> };
>
> @@ -44,6 +45,8 @@ struct xdp_mem_info {
> u32 id;
> };
>
> +struct page_pool;
> +
> struct xdp_rxq_info {
> struct net_device *dev;
> u32 queue_index;
> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index 06a5b39491ad..fe8e87abc266 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -8,6 +8,7 @@
> #include <linux/slab.h>
> #include <linux/idr.h>
> #include <linux/rhashtable.h>
> +#include <net/page_pool.h>
>
> #include <net/xdp.h>
>
> @@ -27,7 +28,10 @@ static struct rhashtable *mem_id_ht;
>
> struct xdp_mem_allocator {
> struct xdp_mem_info mem;
> - void *allocator;
> + union {
> + void *allocator;
> + struct page_pool *page_pool;
> + };
> struct rhash_head node;
> struct rcu_head rcu;
> };
> @@ -74,7 +78,9 @@ void __xdp_mem_allocator_rcu_free(struct rcu_head *rcu)
> /* Allow this ID to be reused */
> ida_simple_remove(&mem_id_pool, xa->mem.id);
>
> - /* TODO: Depending on allocator type/pointer free resources */
> + /* Notice, driver is expected to free the *allocator,
> + * e.g. page_pool, and MUST also use RCU free.
> + */
>
> /* Poison memory */
> xa->mem.id = 0xFFFF;
> @@ -290,11 +296,21 @@ EXPORT_SYMBOL_GPL(xdp_rxq_info_reg_mem_model);
>
> void xdp_return_frame(void *data, struct xdp_mem_info *mem)
> {
> - struct xdp_mem_allocator *xa;
> + struct xdp_mem_allocator *xa = NULL;
>
> rcu_read_lock();
> if (mem->id)
> xa = rhashtable_lookup(mem_id_ht, &mem->id, mem_id_rht_params);
> +
> + if (mem->type == MEM_TYPE_PAGE_POOL) {
> + struct page *page = virt_to_head_page(data);
> +
> + if (xa)
> + page_pool_put_page(xa->page_pool, page);
> + else
> + put_page(page);
> + return;
> + }
So at this point there need to be a few changes.
I would suggest doing a switch based on mem->type instead of this mess
of if statements.
Also the PAGE_POOL is the only one that needs the reference to the
memory allocator. You should move all of the logic for accessing the
hash table into the section specific to the page pool with a
fall-through into the other paths.
> rcu_read_unlock();
>
> if (mem->type == MEM_TYPE_PAGE_SHARED) {
> @@ -306,6 +322,7 @@ void xdp_return_frame(void *data, struct xdp_mem_info *mem)
> struct page *page = virt_to_page(data); /* Assumes order0 page*/
>
> put_page(page);
> + return;
Why add a return here? This was the end of the function already wasn't it?
> }
> }
> EXPORT_SYMBOL_GPL(xdp_return_frame);
>
^ permalink raw reply
* Re: [PATCH v5 3/5] bnx2x: Eliminate duplicate barriers on weakly-ordered archs
From: David Miller @ 2018-03-23 17:04 UTC (permalink / raw)
To: okaya
Cc: netdev, timur, sulrich, linux-arm-msm, linux-arm-kernel,
ariel.elior, everest-linux-l2, linux-kernel
In-Reply-To: <4bd9ccd2-df8f-acad-2513-eefe065dc852@codeaurora.org>
From: Sinan Kaya <okaya@codeaurora.org>
Date: Fri, 23 Mar 2018 12:51:47 -0400
> It could if txdata->tx_db was not a union. There is a data dependency
> between txdata->tx_db.data.prod and txdata->tx_db.raw.
>
> So, no reordering.
I don't see it that way, the code requires that:
txdata->tx_db.data.prod += nbd;
is visible before the doorbell update.
barrier() doesn't provide that.
Neither does writel_relaxed(). However plain writel() does.
Therefore the code is only correct as-is, and your change potentially
adds a reordering problem.
^ permalink raw reply
* Re: [PATCH v2] net/mlx5: Fix use-after-free
From: David Miller @ 2018-03-23 17:06 UTC (permalink / raw)
To: gustavo
Cc: yuval.shaia, ilant, borisp, saeedm, matanb, leon, netdev,
linux-rdma, linux-kernel
In-Reply-To: <20180322184456.GA22259@embeddedgus>
From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
Date: Thu, 22 Mar 2018 13:44:56 -0500
> _rule_ is being freed and then dereferenced by accessing rule->ctx
>
> Fix this by copying the value returned by PTR_ERR(rule->ctx) into a local
> variable for its safe use after freeing _rule_
>
> Addresses-Coverity-ID: 1466041 ("Read from pointer after free")
> Fixes: 05564d0ae075 ("net/mlx5: Add flow-steering commands for FPGA IPSec implementation")
> Reviewed-by: Yuval Shaia <yuval.shaia@oracle.com>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
> Changes in v2:
> - Use a short subject prefix as suggested by Yuval Shaia.
> - Add Yuval's Reviewed-by.
Applied to net-next.
Thank you.
^ permalink raw reply
* Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
From: Florian Fainelli @ 2018-03-23 17:09 UTC (permalink / raw)
To: Jiri Pirko, Andrew Lunn
Cc: netdev, davem, idosch, jakub.kicinski, mlxsw, vivien.didelot,
michael.chan, ganeshgr, saeedm, simon.horman,
pieter.jansenvanvuuren, john.hurley, dirk.vandermerwe,
alexander.h.duyck, ogerlitz, dsahern, vijaya.guvva,
satananda.burla, raghu.vatsavayi, felix.manlunas, gospo,
sathya.perla, vasundhara-v.volam, tariqt, eranbe,
jeffrey.t.kirsher
In-Reply-To: <20180323144914.GA2125@nanopsycho>
On 03/23/2018 07:49 AM, Jiri Pirko wrote:
> Fri, Mar 23, 2018 at 02:30:02PM CET, andrew@lunn.ch wrote:
>> On Thu, Mar 22, 2018 at 11:55:14AM +0100, Jiri Pirko wrote:
>>> From: Jiri Pirko <jiri@mellanox.com>
>>>
>>> Set the attrs and allow to expose port flavour to user via devlink.
>>>
>>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>>> ---
>>> net/dsa/dsa2.c | 23 +++++++++++++++++++++++
>>> 1 file changed, 23 insertions(+)
>>>
>>> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
>>> index adf50fbc4c13..49453690696d 100644
>>> --- a/net/dsa/dsa2.c
>>> +++ b/net/dsa/dsa2.c
>>> @@ -270,7 +270,27 @@ static int dsa_port_setup(struct dsa_port *dp)
>>> case DSA_PORT_TYPE_UNUSED:
>>> break;
>>> case DSA_PORT_TYPE_CPU:
>>> + /* dp->index is used now as port_number. However
>>> + * CPU ports should have separate numbering
>>> + * independent from front panel port numbers.
>>> + */
>>> + devlink_port_attrs_set(&dp->devlink_port,
>>> + DEVLINK_PORT_FLAVOUR_CPU,
>>> + dp->index, false, 0);
>>> + err = dsa_port_link_register_of(dp);
>>> + if (err) {
>>> + dev_err(ds->dev, "failed to setup link for port %d.%d\n",
>>> + ds->index, dp->index);
>>> + return err;
>>> + }
>>
>> Ah, i get it. These used to be two case statements with one code
>> block. But you split them apart, so needed to duplicate the
>> dsa_port_link_register.
>>
>> Unfortunately, you forgot to add a 'break;', so it still falls
>> through, and overwrites the port flavour to DSA.
>
> ah, crap. Don't have hw to test this :/
> Will fix. Thanks!
You don't need hardware, there is drivers/net/dsa/dsa_loop.c which will
emulate a DSA switch. It won't create interconnect ports, since only one
switch can be created with the method chosen, but this would have helped
you catch the missing break since the "CPU" port would have been
displayed as "DSA" anyway.
If you need hardware, I am sure this can be somehow arranged. By that, I
mean something on which you can run upstream Linux on without out of
tree patches.
--
Florian
^ permalink raw reply
* Re: [bpf-next V5 PATCH 05/15] xdp: introduce a new xdp_frame type
From: Alexander Duyck @ 2018-03-23 17:11 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Netdev, BjörnTöpel, Karlsson, Magnus, Eugenia Emantayev,
Jason Wang, John Fastabend, Eran Ben Elisha, Saeed Mahameed,
Gal Pressman, Daniel Borkmann, Alexei Starovoitov, Tariq Toukan
In-Reply-To: <152180750428.20167.335753789470211325.stgit@firesoul>
On Fri, Mar 23, 2018 at 5:18 AM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> This is needed to convert drivers tuntap and virtio_net.
>
> This is a generalization of what is done inside cpumap, which will be
> converted later.
>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> ---
> include/net/xdp.h | 40 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/include/net/xdp.h b/include/net/xdp.h
> index 1ee154fe0be6..13f71a15c79f 100644
> --- a/include/net/xdp.h
> +++ b/include/net/xdp.h
> @@ -59,6 +59,46 @@ struct xdp_buff {
> struct xdp_rxq_info *rxq;
> };
>
> +struct xdp_frame {
> + void *data;
> + u16 len;
> + u16 headroom;
> + u16 metasize;
> + /* Lifetime of xdp_rxq_info is limited to NAPI/enqueue time,
> + * while mem info is valid on remote CPU.
> + */
> + struct xdp_mem_info mem;
> +};
> +
> +/* Convert xdp_buff to xdp_frame */
> +static inline
> +struct xdp_frame *convert_to_xdp_frame(struct xdp_buff *xdp)
> +{
> + struct xdp_frame *xdp_frame;
> + int metasize;
> + int headroom;
> +
> + /* Assure headroom is available for storing info */
> + headroom = xdp->data - xdp->data_hard_start;
> + metasize = xdp->data - xdp->data_meta;
> + metasize = metasize > 0 ? metasize : 0;
> + if (unlikely((headroom - metasize) < sizeof(*xdp_frame)))
> + return NULL;
> +
> + /* Store info in top of packet */
> + xdp_frame = xdp->data_hard_start;
> +
I thought the point of this stuff was supposed to be performance. This
is going to hurt. You are adding yet another cacheline to the number
of lines accessed to process the frame.
I would like to know how of a slow down is being added after you apply
this to the Tx path of ixgbe. I suspect there should be a noticeable
difference for your basic xmit loopback test.
> + xdp_frame->data = xdp->data;
> + xdp_frame->len = xdp->data_end - xdp->data;
> + xdp_frame->headroom = headroom - sizeof(*xdp_frame);
> + xdp_frame->metasize = metasize;
> +
> + /* rxq only valid until napi_schedule ends, convert to xdp_mem_info */
> + xdp_frame->mem = xdp->rxq->mem;
> +
> + return xdp_frame;
> +}
> +
> static inline
> void xdp_return_frame(void *data, struct xdp_mem_info *mem)
> {
>
^ permalink raw reply
* Re: [net-next 0/8] tipc: introduce 128-bit auto-configurable node id
From: David Miller @ 2018-03-23 17:12 UTC (permalink / raw)
To: jon.maloy; +Cc: netdev, tipc-discussion, mohan.krishna.ghanta.krishnamurthy
In-Reply-To: <1521747772-7727-1-git-send-email-jon.maloy@ericsson.com>
From: Jon Maloy <jon.maloy@ericsson.com>
Date: Thu, 22 Mar 2018 20:42:44 +0100
> We introduce a 128-bit free-format node identity as an alternative to
> the legacy <Zone.Cluster.Node> structured 32-bit node address.
>
> We also make configuration of this identity optional; if a bearer is
> enabled without a pre-configured node id it will be set automatically
> based on the used interface's MAC or IP address.
Looks good, series applied, thanks Jon.
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
^ permalink raw reply
* Re: [PATCH v5 3/5] bnx2x: Eliminate duplicate barriers on weakly-ordered archs
From: Sinan Kaya @ 2018-03-23 17:13 UTC (permalink / raw)
To: David Miller
Cc: netdev, timur, sulrich, linux-arm-msm, linux-arm-kernel,
ariel.elior, everest-linux-l2, linux-kernel
In-Reply-To: <20180323.130418.2223623186761161723.davem@davemloft.net>
On 3/23/2018 1:04 PM, David Miller wrote:
> From: Sinan Kaya <okaya@codeaurora.org>
> Date: Fri, 23 Mar 2018 12:51:47 -0400
>
>> It could if txdata->tx_db was not a union. There is a data dependency
>> between txdata->tx_db.data.prod and txdata->tx_db.raw.
>>
>> So, no reordering.
>
> I don't see it that way, the code requires that:
>
> txdata->tx_db.data.prod += nbd;
>
> is visible before the doorbell update.>
> barrier() doesn't provide that.
>
> Neither does writel_relaxed(). However plain writel() does.
Correct for some architectures including ARM but not correct universally.
writel() just guarantees register read/writes before and after to be
ordered when HW observes it.
writel() doesn't guarantee that the memory update is visible to the HW
on all architectures.
If you need memory update visibility, that barrier() should have been a
wmb()
A correct multi-arch pattern is
wmb()
writel_relaxed()
mmiowb()
We have decided to drop a similar patch on Infiniband due to incorrect
usage of barrier(). If you feel strongly about it, I can remove the
DOORBELL change.
>
> Therefore the code is only correct as-is, and your change potentially
> adds a reordering problem.
>
--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* Re: [PATCH v5 3/5] bnx2x: Eliminate duplicate barriers on weakly-ordered archs
From: David Miller @ 2018-03-23 17:16 UTC (permalink / raw)
To: okaya
Cc: netdev, timur, sulrich, linux-arm-msm, linux-arm-kernel,
ariel.elior, everest-linux-l2, linux-kernel
In-Reply-To: <b4553d26-b326-15be-dcac-12594ea7c5b5@codeaurora.org>
From: Sinan Kaya <okaya@codeaurora.org>
Date: Fri, 23 Mar 2018 13:13:46 -0400
> We have decided to drop a similar patch on Infiniband due to incorrect
> usage of barrier(). If you feel strongly about it, I can remove the
> DOORBELL change.
Either remove the DOORBELL change or properly adjust the barrier() to
be a wmb().
Be sure to mention this in the commit message, explicitly.
Thank you.
^ 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