* Re: [net-next, PATCH, v2] net: netsec: Sync dma for device on buffer allocation
From: Ilias Apalodimas @ 2019-07-04 17:52 UTC (permalink / raw)
To: Jesper Dangaard Brouer; +Cc: netdev, jaswinder.singh, ard.biesheuvel, arnd
In-Reply-To: <20190704193944.5ef80468@carbon>
On Thu, Jul 04, 2019 at 07:39:44PM +0200, Jesper Dangaard Brouer wrote:
> On Thu, 4 Jul 2019 17:46:09 +0300
> Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote:
>
> > Quoting Arnd,
> >
> > We have to do a sync_single_for_device /somewhere/ before the
> > buffer is given to the device. On a non-cache-coherent machine with
> > a write-back cache, there may be dirty cache lines that get written back
> > after the device DMA's data into it (e.g. from a previous memset
> > from before the buffer got freed), so you absolutely need to flush any
> > dirty cache lines on it first.
> >
> > Since the coherency is configurable in this device make sure we cover
> > all configurations by explicitly syncing the allocated buffer for the
> > device before refilling it's descriptors
> >
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > ---
> >
> > Changes since V1:
> > - Make the code more readable
> >
> > drivers/net/ethernet/socionext/netsec.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
> > index 5544a722543f..ada7626bf3a2 100644
> > --- a/drivers/net/ethernet/socionext/netsec.c
> > +++ b/drivers/net/ethernet/socionext/netsec.c
> > @@ -727,21 +727,26 @@ static void *netsec_alloc_rx_data(struct netsec_priv *priv,
> > {
> >
> > struct netsec_desc_ring *dring = &priv->desc_ring[NETSEC_RING_RX];
> > + enum dma_data_direction dma_dir;
> > + dma_addr_t dma_start;
> > struct page *page;
> >
> > page = page_pool_dev_alloc_pages(dring->page_pool);
> > if (!page)
> > return NULL;
> >
> > + dma_start = page_pool_get_dma_addr(page);
> > /* We allocate the same buffer length for XDP and non-XDP cases.
> > * page_pool API will map the whole page, skip what's needed for
> > * network payloads and/or XDP
> > */
> > - *dma_handle = page_pool_get_dma_addr(page) + NETSEC_RXBUF_HEADROOM;
> > + *dma_handle = dma_start + NETSEC_RXBUF_HEADROOM;
> > /* Make sure the incoming payload fits in the page for XDP and non-XDP
> > * cases and reserve enough space for headroom + skb_shared_info
> > */
> > *desc_len = PAGE_SIZE - NETSEC_RX_BUF_NON_DATA;
> > + dma_dir = page_pool_get_dma_dir(dring->page_pool);
> > + dma_sync_single_for_device(priv->dev, dma_start, PAGE_SIZE, dma_dir);
>
> It's it costly to sync_for_device the entire page size?
>
> E.g. we already know that the head-room is not touched by device. And
> we actually want this head-room cache-hot for e.g. xdp_frame, thus it
> would be unfortunate if the head-room is explicitly evicted from the
> cache here.
>
> Even smarter, the driver could do the sync for_device, when it
> release/recycle page, as it likely know the exact length that was used
> by the packet.
It does sync for device when recycling takes place in XDP_TX with the correct
size.
I guess i can explicitly sync on the xdp_return_buff cases, and
netsec_setup_rx_dring() instead of the generic buffer allocation
I'll send a V3
Thanks!
/Ilias
^ permalink raw reply
* Re: "local" interfaces, in forwarding state, are mutually "blind", and fail to connect
From: Andrew Lunn @ 2019-07-04 17:46 UTC (permalink / raw)
To: James Feeney; +Cc: netdev
In-Reply-To: <a56eee49-49dc-1e61-19a4-6dfb6bd66f3e@nurealm.net>
On Thu, Jul 04, 2019 at 11:11:21AM -0600, James Feeney wrote:
> I have a question - maybe someone can point me in the right direction?
>
> When there exist two or more "local" interfaces
Hi James
What exactly do you mean by a 'local' interface? The IP address on the
interface has scope local?
Andrew
^ permalink raw reply
* Re: [net-next, PATCH, v2] net: netsec: Sync dma for device on buffer allocation
From: Jesper Dangaard Brouer @ 2019-07-04 17:39 UTC (permalink / raw)
To: Ilias Apalodimas; +Cc: brouer, netdev, jaswinder.singh, ard.biesheuvel, arnd
In-Reply-To: <1562251569-16506-1-git-send-email-ilias.apalodimas@linaro.org>
On Thu, 4 Jul 2019 17:46:09 +0300
Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote:
> Quoting Arnd,
>
> We have to do a sync_single_for_device /somewhere/ before the
> buffer is given to the device. On a non-cache-coherent machine with
> a write-back cache, there may be dirty cache lines that get written back
> after the device DMA's data into it (e.g. from a previous memset
> from before the buffer got freed), so you absolutely need to flush any
> dirty cache lines on it first.
>
> Since the coherency is configurable in this device make sure we cover
> all configurations by explicitly syncing the allocated buffer for the
> device before refilling it's descriptors
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>
> Changes since V1:
> - Make the code more readable
>
> drivers/net/ethernet/socionext/netsec.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
> index 5544a722543f..ada7626bf3a2 100644
> --- a/drivers/net/ethernet/socionext/netsec.c
> +++ b/drivers/net/ethernet/socionext/netsec.c
> @@ -727,21 +727,26 @@ static void *netsec_alloc_rx_data(struct netsec_priv *priv,
> {
>
> struct netsec_desc_ring *dring = &priv->desc_ring[NETSEC_RING_RX];
> + enum dma_data_direction dma_dir;
> + dma_addr_t dma_start;
> struct page *page;
>
> page = page_pool_dev_alloc_pages(dring->page_pool);
> if (!page)
> return NULL;
>
> + dma_start = page_pool_get_dma_addr(page);
> /* We allocate the same buffer length for XDP and non-XDP cases.
> * page_pool API will map the whole page, skip what's needed for
> * network payloads and/or XDP
> */
> - *dma_handle = page_pool_get_dma_addr(page) + NETSEC_RXBUF_HEADROOM;
> + *dma_handle = dma_start + NETSEC_RXBUF_HEADROOM;
> /* Make sure the incoming payload fits in the page for XDP and non-XDP
> * cases and reserve enough space for headroom + skb_shared_info
> */
> *desc_len = PAGE_SIZE - NETSEC_RX_BUF_NON_DATA;
> + dma_dir = page_pool_get_dma_dir(dring->page_pool);
> + dma_sync_single_for_device(priv->dev, dma_start, PAGE_SIZE, dma_dir);
It's it costly to sync_for_device the entire page size?
E.g. we already know that the head-room is not touched by device. And
we actually want this head-room cache-hot for e.g. xdp_frame, thus it
would be unfortunate if the head-room is explicitly evicted from the
cache here.
Even smarter, the driver could do the sync for_device, when it
release/recycle page, as it likely know the exact length that was used
by the packet.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH] tools bpftool: Fix json dump crash on powerpc
From: Quentin Monnet @ 2019-07-04 17:37 UTC (permalink / raw)
To: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann
Cc: Michael Petlan, netdev, bpf, Martin KaFai Lau
In-Reply-To: <20190704085856.17502-1-jolsa@kernel.org>
2019-07-04 10:58 UTC+0200 ~ Jiri Olsa <jolsa@kernel.org>
> Michael reported crash with by bpf program in json mode on powerpc:
>
> # bpftool prog -p dump jited id 14
> [{
> "name": "0xd00000000a9aa760",
> "insns": [{
> "pc": "0x0",
> "operation": "nop",
> "operands": [null
> ]
> },{
> "pc": "0x4",
> "operation": "nop",
> "operands": [null
> ]
> },{
> "pc": "0x8",
> "operation": "mflr",
> Segmentation fault (core dumped)
>
> The code is assuming char pointers in format, which is not always
> true at least for powerpc. Fixing this by dumping the whole string
> into buffer based on its format.
>
> Please note that libopcodes code does not check return values from
> fprintf callback, so there's no point to return error in case of
> allocation failure.
>
> Reported-by: Michael Petlan <mpetlan@redhat.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Looks good to me, thank you for the fix!
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
^ permalink raw reply
* Re: [PATCH v2 bpf-next] selftests/bpf: fix "alu with different scalars 1" on s390
From: Y Song @ 2019-07-04 17:28 UTC (permalink / raw)
To: Ilya Leoshkevich; +Cc: bpf, netdev
In-Reply-To: <20190704085224.65223-1-iii@linux.ibm.com>
On Thu, Jul 4, 2019 at 1:52 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> BPF_LDX_MEM is used to load the least significant byte of the retrieved
> test_val.index, however, on big-endian machines it ends up retrieving
> the most significant byte.
>
> Use the correct least significant byte offset on big-endian machines.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Acked-by: Yonghong Song <yhs@fb.com>
> ---
>
> v1->v2:
> - use __BYTE_ORDER instead of __BYTE_ORDER__.
>
> tools/testing/selftests/bpf/verifier/value_ptr_arith.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/verifier/value_ptr_arith.c b/tools/testing/selftests/bpf/verifier/value_ptr_arith.c
> index c3de1a2c9dc5..e5940c4e8b8f 100644
> --- a/tools/testing/selftests/bpf/verifier/value_ptr_arith.c
> +++ b/tools/testing/selftests/bpf/verifier/value_ptr_arith.c
> @@ -183,7 +183,11 @@
> BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
> BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1),
> BPF_EXIT_INSN(),
> +#if __BYTE_ORDER == __LITTLE_ENDIAN
> BPF_LDX_MEM(BPF_B, BPF_REG_1, BPF_REG_0, 0),
> +#else
> + BPF_LDX_MEM(BPF_B, BPF_REG_1, BPF_REG_0, sizeof(int) - 1),
> +#endif
> BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 0, 3),
> BPF_MOV64_IMM(BPF_REG_2, 0),
> BPF_MOV64_IMM(BPF_REG_3, 0x100000),
> --
> 2.21.0
>
^ permalink raw reply
* Re: [PATCH net-next V2] net: core: page_pool: add user refcnt and reintroduce page_pool_destroy
From: Ilias Apalodimas @ 2019-07-04 17:23 UTC (permalink / raw)
To: Jesper Dangaard Brouer, netdev, grygorii.strashko, jakub.kicinski,
daniel, john.fastabend, ast, linux-kernel, linux-omap
In-Reply-To: <20190704171354.GC2923@khorivan>
Hi Ivan,
>
> Have trouble with inet today...I will pick up it as my changes depend on it.
> And send probably in couple hours after verification.
Maybe you'd like to add your signed-off on this one since it was based on your
patchset?
Regards
/Ilias
^ permalink raw reply
* Re: [PATCH net-next V2] net: core: page_pool: add user refcnt and reintroduce page_pool_destroy
From: Ilias Apalodimas @ 2019-07-04 17:22 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: netdev, ivan.khoronzhuk, grygorii.strashko, jakub.kicinski,
daniel, john.fastabend, ast, linux-kernel, linux-omap
In-Reply-To: <156225871578.1603.6630229522953924907.stgit@firesoul>
Hi Jesper,
> Jesper recently removed page_pool_destroy() (from driver invocation) and
> moved shutdown and free of page_pool into xdp_rxq_info_unreg(), in-order to
> handle in-flight packets/pages. This created an asymmetry in drivers
> create/destroy pairs.
>
> This patch reintroduce page_pool_destroy and add page_pool user refcnt.
> This serves the purpose to simplify drivers error handling as driver now
> drivers always calls page_pool_destroy() and don't need to track if
> xdp_rxq_info_reg_mem_model() was unsuccessful.
>
> This could be used for a special cases where a single RX-queue (with a single
> page_pool) provides packets for two net_device'es, and thus needs to register
> the same page_pool twice with two xdp_rxq_info structures. This use-case is
> explicitly denied in this V2 patch, as it needs more discussion upstream.
>
> This patch is primarily to ease API usage for drivers. The recently merged
> netsec driver, actually have a bug in this area, which is solved by this API
> change.
>
> This patch is a modified version of Ivan Khoronzhu's original patch.
>
> Link: https://lore.kernel.org/netdev/20190625175948.24771-2-ivan.khoronzhuk@linaro.org/
> Fixes: 5c67bf0ec4d0 ("net: netsec: Use page_pool API")
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> ---
>
> I hope we can agree on this first step. Afterwards we can discuss, which is the
> best approach for Ivan's two netdev's with one RX-queue API change.
>
> drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 6 ++---
> drivers/net/ethernet/socionext/netsec.c | 8 ++----
> include/net/page_pool.h | 26 +++++++++++++++++++++
> net/core/page_pool.c | 8 ++++++
> net/core/xdp.c | 9 +++++++
> 5 files changed, 48 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index 1085040675ae..ce1c7a449eae 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -545,10 +545,8 @@ static int mlx5e_alloc_rq(struct mlx5e_channel *c,
> }
> err = xdp_rxq_info_reg_mem_model(&rq->xdp_rxq,
> MEM_TYPE_PAGE_POOL, rq->page_pool);
> - if (err) {
> - page_pool_free(rq->page_pool);
> + if (err)
> goto err_free;
> - }
>
> for (i = 0; i < wq_sz; i++) {
> if (rq->wq_type == MLX5_WQ_TYPE_LINKED_LIST_STRIDING_RQ) {
> @@ -613,6 +611,7 @@ static int mlx5e_alloc_rq(struct mlx5e_channel *c,
> if (rq->xdp_prog)
> bpf_prog_put(rq->xdp_prog);
> xdp_rxq_info_unreg(&rq->xdp_rxq);
> + page_pool_destroy(rq->page_pool);
> mlx5_wq_destroy(&rq->wq_ctrl);
>
> return err;
> @@ -643,6 +642,7 @@ static void mlx5e_free_rq(struct mlx5e_rq *rq)
> }
>
> xdp_rxq_info_unreg(&rq->xdp_rxq);
> + page_pool_destroy(rq->page_pool);
> mlx5_wq_destroy(&rq->wq_ctrl);
> }
>
> diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
> index 5544a722543f..43ab0ce90704 100644
> --- a/drivers/net/ethernet/socionext/netsec.c
> +++ b/drivers/net/ethernet/socionext/netsec.c
> @@ -1210,15 +1210,11 @@ static void netsec_uninit_pkt_dring(struct netsec_priv *priv, int id)
> }
> }
>
> - /* Rx is currently using page_pool
> - * since the pool is created during netsec_setup_rx_dring(), we need to
> - * free the pool manually if the registration failed
> - */
> + /* Rx is currently using page_pool */
> if (id == NETSEC_RING_RX) {
> if (xdp_rxq_info_is_reg(&dring->xdp_rxq))
> xdp_rxq_info_unreg(&dring->xdp_rxq);
> - else
> - page_pool_free(dring->page_pool);
> + page_pool_destroy(dring->page_pool);
> }
>
> memset(dring->desc, 0, sizeof(struct netsec_desc) * DESC_NUM);
> diff --git a/include/net/page_pool.h b/include/net/page_pool.h
> index ee9c871d2043..0a0984d8f5d5 100644
> --- a/include/net/page_pool.h
> +++ b/include/net/page_pool.h
> @@ -101,6 +101,12 @@ struct page_pool {
> struct ptr_ring ring;
>
> atomic_t pages_state_release_cnt;
> +
> + /* A page_pool is strictly tied to a single RX-queue being
> + * protected by NAPI, due to above pp_alloc_cache. This
> + * refcnt serves purpose is to simplify drivers error handling.
> + */
> + refcount_t user_cnt;
> };
>
> struct page *page_pool_alloc_pages(struct page_pool *pool, gfp_t gfp);
> @@ -134,6 +140,15 @@ static inline void page_pool_free(struct page_pool *pool)
> #endif
> }
>
> +/* Drivers use this instead of page_pool_free */
> +static inline void page_pool_destroy(struct page_pool *pool)
> +{
> + if (!pool)
> + return;
> +
> + page_pool_free(pool);
> +}
> +
> /* Never call this directly, use helpers below */
> void __page_pool_put_page(struct page_pool *pool,
> struct page *page, bool allow_direct);
> @@ -201,4 +216,15 @@ static inline bool is_page_pool_compiled_in(void)
> #endif
> }
>
> +static inline unsigned int page_pool_get(struct page_pool *pool)
> +{
> + refcount_inc(&pool->user_cnt);
> + return refcount_read(&pool->user_cnt);
> +}
> +
> +static inline bool page_pool_put(struct page_pool *pool)
> +{
> + return refcount_dec_and_test(&pool->user_cnt);
> +}
> +
> #endif /* _NET_PAGE_POOL_H */
> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> index b366f59885c1..3272dc7a8c81 100644
> --- a/net/core/page_pool.c
> +++ b/net/core/page_pool.c
> @@ -49,6 +49,9 @@ static int page_pool_init(struct page_pool *pool,
>
> atomic_set(&pool->pages_state_release_cnt, 0);
>
> + /* Driver calling page_pool_create() also call page_pool_destroy() */
> + refcount_set(&pool->user_cnt, 1);
> +
> if (pool->p.flags & PP_FLAG_DMA_MAP)
> get_device(pool->p.dev);
>
> @@ -70,6 +73,7 @@ struct page_pool *page_pool_create(const struct page_pool_params *params)
> kfree(pool);
> return ERR_PTR(err);
> }
> +
> return pool;
> }
> EXPORT_SYMBOL(page_pool_create);
> @@ -356,6 +360,10 @@ static void __warn_in_flight(struct page_pool *pool)
>
> void __page_pool_free(struct page_pool *pool)
> {
> + /* Only last user actually free/release resources */
> + if (!page_pool_put(pool))
> + return;
> +
> WARN(pool->alloc.count, "API usage violation");
> WARN(!ptr_ring_empty(&pool->ring), "ptr_ring is not empty");
>
> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index b29d7b513a18..57c2317d9ce5 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -370,6 +370,15 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
> goto err;
> }
>
> + if (type == MEM_TYPE_PAGE_POOL) {
> + if (page_pool_get(xdp_alloc->page_pool) > 2) {
> + WARN(1, "API misuse, argue to enable use-case");
> + page_pool_put(xdp_alloc->page_pool);
> + errno = -EINVAL;
> + goto err;
> + }
> + }
> +
> mutex_unlock(&mem_id_lock);
>
> trace_mem_connect(xdp_alloc, xdp_rxq);
>
For the netsec and page_pool part
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
^ permalink raw reply
* Re: [PATCH mlx5-next 4/5] net/mlx5: Introduce TLS TX offload hardware bits and structures
From: Saeed Mahameed @ 2019-07-04 17:21 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Saeed Mahameed, netdev@vger.kernel.org,
linux-rdma@vger.kernel.org, Eran Ben Elisha, Tariq Toukan
In-Reply-To: <20190704171519.GE7212@mtr-leonro.mtl.com>
On Thu, Jul 4, 2019 at 1:15 PM Leon Romanovsky <leon@kernel.org> wrote:
>
> On Thu, Jul 04, 2019 at 01:06:58PM -0400, Saeed Mahameed wrote:
> > On Wed, Jul 3, 2019 at 5:27 AM <leon@kernel.org> wrote:
> > >
> > > On Wed, Jul 03, 2019 at 07:39:32AM +0000, Saeed Mahameed wrote:
> > > > From: Eran Ben Elisha <eranbe@mellanox.com>
> > > >
> > > > Add TLS offload related IFC structs, layouts and enumerations.
> > > >
> > > > Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
> > > > Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
> > > > Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> > > > ---
> > > > include/linux/mlx5/device.h | 14 +++++
> > > > include/linux/mlx5/mlx5_ifc.h | 104 ++++++++++++++++++++++++++++++++--
> > > > 2 files changed, 114 insertions(+), 4 deletions(-)
> > >
> > > <...>
> > >
> > > > @@ -2725,7 +2739,8 @@ struct mlx5_ifc_traffic_counter_bits {
> > > >
> > > > struct mlx5_ifc_tisc_bits {
> > > > u8 strict_lag_tx_port_affinity[0x1];
> > > > - u8 reserved_at_1[0x3];
> > > > + u8 tls_en[0x1];
> > > > + u8 reserved_at_1[0x2];
> > >
> > > It should be reserved_at_2.
> > >
> >
> > it should be at_1.
>
> Why? See mlx5_ifc_flow_table_prop_layout_bits, mlx5_ifc_roce_cap_bits, e.t.c.
>
they are all at_1 .. so i don't really understand what you want from me,
Leon the code is good, please double check you comments..
> Thanks
>
> >
> > > Thanks
^ permalink raw reply
* "local" interfaces, in forwarding state, are mutually "blind", and fail to connect
From: James Feeney @ 2019-07-04 17:11 UTC (permalink / raw)
To: netdev
I have a question - maybe someone can point me in the right direction?
When there exist two or more "local" interfaces on the "host" system, where sysctl "net.ipv4.conf.<blah>.forwarding=1" has been set, and where each interface has an IP address on a different subnet, then, when a frame arrives at an interface, and is addressed to an IP address on some other "local" interface, that frame is never actually delivered to this other "local" interface, but instead, is "looped-back" at the connected "local" interface, and given a *fake* "source" IP address, as if the response had actually been generated by the other "local" interface.
When the incoming frame is of an ICMP echo request, and the outgoing response is an ICMP echo reply, this "fake source address" behavior seems to be of no consequence, except that an interface in a "down" state will still respond to ping. Parenthetically, then, what is the definition of the "down" state?
However, when the incoming request is, as an example, a domain query, addressed to the IP address of the daemon, and the domain daemon is only binding to the *other* "local" interface, then the domain request is never delivered to this other "local" interface, the one actually addressed, and the frame is never received by the daemon. The request fails, and there is no response.
At first glance this kernel behavior seems "broken". Should the frame not *actually* be delivered to this *other* "local" interface? Why should this "local"/"internal" packet routing fail? Is that "on purpose"?
Now, the simplest solution to the problem is to just have the daemon also bind to all of whichever "local" interfaces are meant to receive these domain requests, in this example. But still, I am wondering if this failure to actually deliver the frame, to the *other* "local" interface, that was actually addressed, is not some kind of improper behavior.
And then, a "follow-up" question, is there some otherwise simplistic manual reconfiguration of the kernel routing tables that would achieve the kind of behavior I would naively expect, that the incoming frame from one "local" interface would actually be delivered to the *other* "local" interface, that was *actually* addressed, so that the domain daemon, in this example, would be able to respond to the request, even when that daemon were only "listening" on the *other* "local" interface, the one addressed? And, without using a bridge interface, bridging these separate "local" interfaces, intended to be on different subnets?
Thanks
James
^ permalink raw reply
* Re: [PATCH mlx5-next 0/5] Mellanox, mlx5 low level updates 2019-07-02
From: Leon Romanovsky @ 2019-07-04 17:16 UTC (permalink / raw)
To: Saeed Mahameed; +Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org
In-Reply-To: <5c85e7cd688cc8727f421e4592304e66ccd018c7.camel@mellanox.com>
On Thu, Jul 04, 2019 at 05:10:25PM +0000, Saeed Mahameed wrote:
> On Wed, 2019-07-03 at 07:39 +0000, Saeed Mahameed wrote:
> > Hi All,
> >
> > This series includes some low level updates to mlx5 driver, required
> > for
> > shared mlx5-next branch.
> >
> > Tariq extends the WQE control fields names.
> > Eran adds the required HW definitions and structures for upcoming TLS
> > support.
> > Parav improves and refactors the E-Switch "function changed" handler.
> >
> > In case of no objections these patches will be applied to mlx5-next
> > and
> > will be sent later as pull request to both rdma-next and net-next
> > trees.
> >
> > Thanks,
> > Saeed.
>
> Applied to mlx5-next.
Saeed,
Please fix IFC, before you are pushing it out.
Thanks
>
^ permalink raw reply
* Re: [PATCH mlx5-next 4/5] net/mlx5: Introduce TLS TX offload hardware bits and structures
From: Leon Romanovsky @ 2019-07-04 17:15 UTC (permalink / raw)
To: Saeed Mahameed
Cc: Saeed Mahameed, netdev@vger.kernel.org,
linux-rdma@vger.kernel.org, Eran Ben Elisha, Tariq Toukan
In-Reply-To: <CALzJLG-em1w+Lgf2UutbG2Lzq8bx3zUqoLGx26H2_EXOuuk+jg@mail.gmail.com>
On Thu, Jul 04, 2019 at 01:06:58PM -0400, Saeed Mahameed wrote:
> On Wed, Jul 3, 2019 at 5:27 AM <leon@kernel.org> wrote:
> >
> > On Wed, Jul 03, 2019 at 07:39:32AM +0000, Saeed Mahameed wrote:
> > > From: Eran Ben Elisha <eranbe@mellanox.com>
> > >
> > > Add TLS offload related IFC structs, layouts and enumerations.
> > >
> > > Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
> > > Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
> > > Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> > > ---
> > > include/linux/mlx5/device.h | 14 +++++
> > > include/linux/mlx5/mlx5_ifc.h | 104 ++++++++++++++++++++++++++++++++--
> > > 2 files changed, 114 insertions(+), 4 deletions(-)
> >
> > <...>
> >
> > > @@ -2725,7 +2739,8 @@ struct mlx5_ifc_traffic_counter_bits {
> > >
> > > struct mlx5_ifc_tisc_bits {
> > > u8 strict_lag_tx_port_affinity[0x1];
> > > - u8 reserved_at_1[0x3];
> > > + u8 tls_en[0x1];
> > > + u8 reserved_at_1[0x2];
> >
> > It should be reserved_at_2.
> >
>
> it should be at_1.
Why? See mlx5_ifc_flow_table_prop_layout_bits, mlx5_ifc_roce_cap_bits, e.t.c.
Thanks
>
> > Thanks
^ permalink raw reply
* Re: [PATCH net-next V2] net: core: page_pool: add user refcnt and reintroduce page_pool_destroy
From: Ivan Khoronzhuk @ 2019-07-04 17:13 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: netdev, Ilias Apalodimas, grygorii.strashko, jakub.kicinski,
daniel, john.fastabend, ast, linux-kernel, linux-omap
In-Reply-To: <156225871578.1603.6630229522953924907.stgit@firesoul>
On Thu, Jul 04, 2019 at 06:47:07PM +0200, Jesper Dangaard Brouer wrote:
Have trouble with inet today...I will pick up it as my changes depend on it.
And send probably in couple hours after verification.
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply
* Re: [PATCH 1/1] tools/dtrace: initial implementation of DTrace
From: Brendan Gregg @ 2019-07-04 17:13 UTC (permalink / raw)
To: Kris Van Hees
Cc: netdev, bpf, dtrace-devel, LKML, Steven Rostedt, Masami Hiramatsu,
Arnaldo Carvalho de Melo, Alexei Starovoitov, Daniel Borkmann,
Peter Zijlstra, Chris Mason
In-Reply-To: <201907040314.x643EUoA017906@aserv0122.oracle.com>
On Wed, Jul 3, 2019 at 8:17 PM Kris Van Hees <kris.van.hees@oracle.com> wrote:
>
> This initial implementation of a tiny subset of DTrace functionality
> provides the following options:
>
> dtrace [-lvV] [-b bufsz] -s script
> -b set trace buffer size
> -l list probes (only works with '-s script' for now)
> -s enable or list probes for the specified BPF program
> -V report DTrace API version
>
> The patch comprises quite a bit of code due to DTrace requiring a few
> crucial components, even in its most basic form.
This patchset has moved from adding tests (which actually belong in
selftests, not tools/dtrace), to the start of adding a giant tracer to
the kernel code base.
First, in some ways you're doing this for me -- I've been the number
one user of DTrace for 15 years -- and thanks, but no thanks. I don't
need this anymore. I needed this 6 years ago, and I would have helped
you build it, but in the meantime Linux has built something better,
built from the ground up for BPF: bpftrace.
Second, you argued that DTrace was needed because of speculative
tracing (a feature I never used), as you had customers who wanted it.
Those customers aren't going to be happy with this initial tiny
implementation of DTrace -- this is really the start of adding a large
and complex tracer to the kernel.
We've all been working under the assumption that these user-space
tracers did not belong in the kernel, and so far that's been working
fine for us. Is it now open season for tracers in the kernel? Let's
have:
tools/bpftrace
tools/ply
tools/systemtap
tools/LTTng
tools/sysdig
tools/ktap
etc
Yes, that's ridiculous. If there's only going to be one, let's have
the best one, bpftrace. We'll offer a version that is GPL, C, and has
no dependencies.
But it would be news to us all that we're allowed to have even one.
There are more things that don't make sense about this, but I'll stop
here for now.
Brendan
^ permalink raw reply
* Re: [PATCH v6 net-next 1/5] xdp: allow same allocator usage
From: Ivan Khoronzhuk @ 2019-07-04 17:11 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: grygorii.strashko, davem, ast, linux-kernel, linux-omap,
xdp-newbies, ilias.apalodimas, netdev, daniel, jakub.kicinski,
john.fastabend
In-Reply-To: <20190704144144.5edd18eb@carbon>
On Thu, Jul 04, 2019 at 02:41:44PM +0200, Jesper Dangaard Brouer wrote:
>On Thu, 4 Jul 2019 13:22:40 +0300
>Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
>
>> On Wed, Jul 03, 2019 at 07:40:13PM +0200, Jesper Dangaard Brouer wrote:
>> >On Wed, 3 Jul 2019 13:18:59 +0300
>> >Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
>> >
>> >> First of all, it is an absolute requirement that each RX-queue have
>> >> their own page_pool object/allocator. And this change is intendant
>> >> to handle special case, where a single RX-queue can receive packets
>> >> from two different net_devices.
>> >>
>> >> In order to protect against using same allocator for 2 different rx
>> >> queues, add queue_index to xdp_mem_allocator to catch the obvious
>> >> mistake where queue_index mismatch, as proposed by Jesper Dangaard
>> >> Brouer.
>> >>
>> >> Adding this on xdp allocator level allows drivers with such dependency
>> >> change the allocators w/o modifications.
>> >>
>> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> >> ---
>> >> include/net/xdp_priv.h | 2 ++
>> >> net/core/xdp.c | 55 ++++++++++++++++++++++++++++++++++++++++++
>> >> 2 files changed, 57 insertions(+)
>> >>
>> >> diff --git a/include/net/xdp_priv.h b/include/net/xdp_priv.h
>> >> index 6a8cba6ea79a..9858a4057842 100644
>> >> --- a/include/net/xdp_priv.h
>> >> +++ b/include/net/xdp_priv.h
>> >> @@ -18,6 +18,8 @@ struct xdp_mem_allocator {
>> >> struct rcu_head rcu;
>> >> struct delayed_work defer_wq;
>> >> unsigned long defer_warn;
>> >> + unsigned long refcnt;
>> >> + u32 queue_index;
>> >> };
>> >
>> >I don't like this approach, because I think we need to extend struct
>> >xdp_mem_allocator with a net_device pointer, for doing dev_hold(), to
>> >correctly handle lifetime issues. (As I tried to explain previously).
>> >This will be much harder after this change, which is why I proposed the
>> >other patch.
>> My concern comes not from zero also.
>> It's partly continuation of not answered questions from here:
>> https://lwn.net/ml/netdev/20190625122822.GC6485@khorivan/
>>
>> "For me it's important to know only if it means that alloc.count is
>> freed at first call of __mem_id_disconnect() while shutdown.
>> The workqueue for the rest is connected only with ring cache protected
>> by ring lock and not supposed that alloc.count can be changed while
>> workqueue tries to shutdonwn the pool."
>
>Yes. The alloc.count is only freed on first call. I considered
>changing the shutdown API, to have two shutdown calls, where the call
>used from the work-queue will not have the loop emptying alloc.count,
>but instead have a WARN_ON(alloc.count), as it MUST be empty (once is
>code running from work-queue).
>
>> So patch you propose to leave works only because of luck, because fast
>> cache is cleared before workqueue is scheduled and no races between two
>> workqueues for fast cache later. I'm not really against this patch, but
>> I have to try smth better.
>
>It is not "luck". It does the correct thing as we never enter the
>while loop in __page_pool_request_shutdown() from a work-queue, but it
>is not obvious from the code. The not-so-nice thing is that two
>work-queue shutdowns will be racing with each-other, in the multi
>netdev use-case, but access to the ptr_ring is safe/locked.
So, having this, and being prudent to generic code changes, lets roll back
to idea from v.4:
https://lkml.org/lkml/2019/6/25/996
but use changes from following patch, reintroducing page destroy:
https://www.spinics.net/lists/netdev/msg583145.html
with appropriate small modifications for cpsw.
In case of some issue connected with it (not supposed), or two/more
allocators used by cpsw, or one more driver having such multi ndev
capabilities (supposed), would be nice to use this link as reference
and it can be base for similar modifications.
Unless Jesper disagrees with this ofc.
I will send v7 soon after verification is completed.
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply
* Re: [PATCH mlx5-next 0/5] Mellanox, mlx5 low level updates 2019-07-02
From: Saeed Mahameed @ 2019-07-04 17:10 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org
In-Reply-To: <20190703073909.14965-1-saeedm@mellanox.com>
On Wed, 2019-07-03 at 07:39 +0000, Saeed Mahameed wrote:
> Hi All,
>
> This series includes some low level updates to mlx5 driver, required
> for
> shared mlx5-next branch.
>
> Tariq extends the WQE control fields names.
> Eran adds the required HW definitions and structures for upcoming TLS
> support.
> Parav improves and refactors the E-Switch "function changed" handler.
>
> In case of no objections these patches will be applied to mlx5-next
> and
> will be sent later as pull request to both rdma-next and net-next
> trees.
>
> Thanks,
> Saeed.
Applied to mlx5-next.
^ permalink raw reply
* Re: [PATCH mlx5-next 4/5] net/mlx5: Introduce TLS TX offload hardware bits and structures
From: Saeed Mahameed @ 2019-07-04 17:06 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Saeed Mahameed, netdev@vger.kernel.org,
linux-rdma@vger.kernel.org, Eran Ben Elisha, Tariq Toukan
In-Reply-To: <20190703092735.GZ4727@mtr-leonro.mtl.com>
On Wed, Jul 3, 2019 at 5:27 AM <leon@kernel.org> wrote:
>
> On Wed, Jul 03, 2019 at 07:39:32AM +0000, Saeed Mahameed wrote:
> > From: Eran Ben Elisha <eranbe@mellanox.com>
> >
> > Add TLS offload related IFC structs, layouts and enumerations.
> >
> > Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
> > Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
> > Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> > ---
> > include/linux/mlx5/device.h | 14 +++++
> > include/linux/mlx5/mlx5_ifc.h | 104 ++++++++++++++++++++++++++++++++--
> > 2 files changed, 114 insertions(+), 4 deletions(-)
>
> <...>
>
> > @@ -2725,7 +2739,8 @@ struct mlx5_ifc_traffic_counter_bits {
> >
> > struct mlx5_ifc_tisc_bits {
> > u8 strict_lag_tx_port_affinity[0x1];
> > - u8 reserved_at_1[0x3];
> > + u8 tls_en[0x1];
> > + u8 reserved_at_1[0x2];
>
> It should be reserved_at_2.
>
it should be at_1.
> Thanks
^ permalink raw reply
* [PATCH net-next V2] net: core: page_pool: add user refcnt and reintroduce page_pool_destroy
From: Jesper Dangaard Brouer @ 2019-07-04 16:47 UTC (permalink / raw)
To: netdev, Ilias Apalodimas, ivan.khoronzhuk, Jesper Dangaard Brouer
Cc: grygorii.strashko, jakub.kicinski, daniel, john.fastabend, ast,
linux-kernel, linux-omap
Jesper recently removed page_pool_destroy() (from driver invocation) and
moved shutdown and free of page_pool into xdp_rxq_info_unreg(), in-order to
handle in-flight packets/pages. This created an asymmetry in drivers
create/destroy pairs.
This patch reintroduce page_pool_destroy and add page_pool user refcnt.
This serves the purpose to simplify drivers error handling as driver now
drivers always calls page_pool_destroy() and don't need to track if
xdp_rxq_info_reg_mem_model() was unsuccessful.
This could be used for a special cases where a single RX-queue (with a single
page_pool) provides packets for two net_device'es, and thus needs to register
the same page_pool twice with two xdp_rxq_info structures. This use-case is
explicitly denied in this V2 patch, as it needs more discussion upstream.
This patch is primarily to ease API usage for drivers. The recently merged
netsec driver, actually have a bug in this area, which is solved by this API
change.
This patch is a modified version of Ivan Khoronzhu's original patch.
Link: https://lore.kernel.org/netdev/20190625175948.24771-2-ivan.khoronzhuk@linaro.org/
Fixes: 5c67bf0ec4d0 ("net: netsec: Use page_pool API")
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
I hope we can agree on this first step. Afterwards we can discuss, which is the
best approach for Ivan's two netdev's with one RX-queue API change.
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 6 ++---
drivers/net/ethernet/socionext/netsec.c | 8 ++----
include/net/page_pool.h | 26 +++++++++++++++++++++
net/core/page_pool.c | 8 ++++++
net/core/xdp.c | 9 +++++++
5 files changed, 48 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 1085040675ae..ce1c7a449eae 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -545,10 +545,8 @@ static int mlx5e_alloc_rq(struct mlx5e_channel *c,
}
err = xdp_rxq_info_reg_mem_model(&rq->xdp_rxq,
MEM_TYPE_PAGE_POOL, rq->page_pool);
- if (err) {
- page_pool_free(rq->page_pool);
+ if (err)
goto err_free;
- }
for (i = 0; i < wq_sz; i++) {
if (rq->wq_type == MLX5_WQ_TYPE_LINKED_LIST_STRIDING_RQ) {
@@ -613,6 +611,7 @@ static int mlx5e_alloc_rq(struct mlx5e_channel *c,
if (rq->xdp_prog)
bpf_prog_put(rq->xdp_prog);
xdp_rxq_info_unreg(&rq->xdp_rxq);
+ page_pool_destroy(rq->page_pool);
mlx5_wq_destroy(&rq->wq_ctrl);
return err;
@@ -643,6 +642,7 @@ static void mlx5e_free_rq(struct mlx5e_rq *rq)
}
xdp_rxq_info_unreg(&rq->xdp_rxq);
+ page_pool_destroy(rq->page_pool);
mlx5_wq_destroy(&rq->wq_ctrl);
}
diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
index 5544a722543f..43ab0ce90704 100644
--- a/drivers/net/ethernet/socionext/netsec.c
+++ b/drivers/net/ethernet/socionext/netsec.c
@@ -1210,15 +1210,11 @@ static void netsec_uninit_pkt_dring(struct netsec_priv *priv, int id)
}
}
- /* Rx is currently using page_pool
- * since the pool is created during netsec_setup_rx_dring(), we need to
- * free the pool manually if the registration failed
- */
+ /* Rx is currently using page_pool */
if (id == NETSEC_RING_RX) {
if (xdp_rxq_info_is_reg(&dring->xdp_rxq))
xdp_rxq_info_unreg(&dring->xdp_rxq);
- else
- page_pool_free(dring->page_pool);
+ page_pool_destroy(dring->page_pool);
}
memset(dring->desc, 0, sizeof(struct netsec_desc) * DESC_NUM);
diff --git a/include/net/page_pool.h b/include/net/page_pool.h
index ee9c871d2043..0a0984d8f5d5 100644
--- a/include/net/page_pool.h
+++ b/include/net/page_pool.h
@@ -101,6 +101,12 @@ struct page_pool {
struct ptr_ring ring;
atomic_t pages_state_release_cnt;
+
+ /* A page_pool is strictly tied to a single RX-queue being
+ * protected by NAPI, due to above pp_alloc_cache. This
+ * refcnt serves purpose is to simplify drivers error handling.
+ */
+ refcount_t user_cnt;
};
struct page *page_pool_alloc_pages(struct page_pool *pool, gfp_t gfp);
@@ -134,6 +140,15 @@ static inline void page_pool_free(struct page_pool *pool)
#endif
}
+/* Drivers use this instead of page_pool_free */
+static inline void page_pool_destroy(struct page_pool *pool)
+{
+ if (!pool)
+ return;
+
+ page_pool_free(pool);
+}
+
/* Never call this directly, use helpers below */
void __page_pool_put_page(struct page_pool *pool,
struct page *page, bool allow_direct);
@@ -201,4 +216,15 @@ static inline bool is_page_pool_compiled_in(void)
#endif
}
+static inline unsigned int page_pool_get(struct page_pool *pool)
+{
+ refcount_inc(&pool->user_cnt);
+ return refcount_read(&pool->user_cnt);
+}
+
+static inline bool page_pool_put(struct page_pool *pool)
+{
+ return refcount_dec_and_test(&pool->user_cnt);
+}
+
#endif /* _NET_PAGE_POOL_H */
diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index b366f59885c1..3272dc7a8c81 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -49,6 +49,9 @@ static int page_pool_init(struct page_pool *pool,
atomic_set(&pool->pages_state_release_cnt, 0);
+ /* Driver calling page_pool_create() also call page_pool_destroy() */
+ refcount_set(&pool->user_cnt, 1);
+
if (pool->p.flags & PP_FLAG_DMA_MAP)
get_device(pool->p.dev);
@@ -70,6 +73,7 @@ struct page_pool *page_pool_create(const struct page_pool_params *params)
kfree(pool);
return ERR_PTR(err);
}
+
return pool;
}
EXPORT_SYMBOL(page_pool_create);
@@ -356,6 +360,10 @@ static void __warn_in_flight(struct page_pool *pool)
void __page_pool_free(struct page_pool *pool)
{
+ /* Only last user actually free/release resources */
+ if (!page_pool_put(pool))
+ return;
+
WARN(pool->alloc.count, "API usage violation");
WARN(!ptr_ring_empty(&pool->ring), "ptr_ring is not empty");
diff --git a/net/core/xdp.c b/net/core/xdp.c
index b29d7b513a18..57c2317d9ce5 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -370,6 +370,15 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
goto err;
}
+ if (type == MEM_TYPE_PAGE_POOL) {
+ if (page_pool_get(xdp_alloc->page_pool) > 2) {
+ WARN(1, "API misuse, argue to enable use-case");
+ page_pool_put(xdp_alloc->page_pool);
+ errno = -EINVAL;
+ goto err;
+ }
+ }
+
mutex_unlock(&mem_id_lock);
trace_mem_connect(xdp_alloc, xdp_rxq);
^ permalink raw reply related
* Re: [PATCH v2] rtl8xxxu: Fix wifi low signal strength issue of RTL8723BU
From: Jes Sorensen @ 2019-07-04 16:43 UTC (permalink / raw)
To: Chris Chiu, kvalo, davem; +Cc: linux-wireless, netdev, linux-kernel, linux
In-Reply-To: <20190704105528.74028-1-chiu@endlessm.com>
On 7/4/19 6:55 AM, Chris Chiu wrote:
> The WiFi tx power of RTL8723BU is extremely low after booting. So
> the WiFi scan gives very limited AP list and it always fails to
> connect to the selected AP. This module only supports 1x1 antenna
> and the antenna is switched to bluetooth due to some incorrect
> register settings.
>
> Compare with the vendor driver https://github.com/lwfinger/rtl8723bu,
> we realized that the 8723bu's enable_rf() does the same thing as
> rtw_btcoex_HAL_Initialize() in vendor driver. And it by default
> sets the antenna path to BTC_ANT_PATH_BT which we verified it's
> the cause of the wifi weak tx power. The vendor driver will set
> the antenna path to BTC_ANT_PATH_PTA in the consequent btcoexist
> mechanism, by the function halbtc8723b1ant_PsTdma.
>
> This commit hand over the antenna control to PTA(Packet Traffic
> Arbitration), which compares the weight of bluetooth/wifi traffic
> then determine whether to continue current wifi traffic or not.
> After PTA take control, The wifi signal will be back to normal and
> the bluetooth scan can also work at the same time. However, the
> btcoexist still needs to be handled under different circumstances.
> If there's a BT connection established, the wifi still fails to
> connect until BT disconnected.
>
> Signed-off-by: Chris Chiu <chiu@endlessm.com>
> ---
>
>
> Note:
> v2:
> - Replace BIT(11) with the descriptive definition
> - Meaningful comment for the REG_S0S1_PATH_SWITCH setting
>
>
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c | 11 ++++++++---
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 3 ++-
> 2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> index 3adb1d3d47ac..ceffe05bd65b 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> @@ -1525,7 +1525,7 @@ static void rtl8723b_enable_rf(struct rtl8xxxu_priv *priv)
> /*
> * WLAN action by PTA
> */
> - rtl8xxxu_write8(priv, REG_WLAN_ACT_CONTROL_8723B, 0x04);
> + rtl8xxxu_write8(priv, REG_WLAN_ACT_CONTROL_8723B, 0x0c);
>
> /*
> * BT select S0/S1 controlled by WiFi
> @@ -1568,9 +1568,14 @@ static void rtl8723b_enable_rf(struct rtl8xxxu_priv *priv)
> rtl8xxxu_gen2_h2c_cmd(priv, &h2c, sizeof(h2c.ant_sel_rsv));
>
> /*
> - * 0x280, 0x00, 0x200, 0x80 - not clear
> + * Different settings per different antenna position.
> + * Antenna Position: | Normal Inverse
> + * --------------------------------------------------
> + * Antenna switch to BT: | 0x280, 0x00
> + * Antenna switch to WiFi: | 0x0, 0x280
> + * Antenna switch to PTA: | 0x200, 0x80
> */
> - rtl8xxxu_write32(priv, REG_S0S1_PATH_SWITCH, 0x00);
> + rtl8xxxu_write32(priv, REG_S0S1_PATH_SWITCH, 0x80);
Per the documentation, shouldn't this be set to 0x200 then rather than 0x80?
We may need to put in place some code to detect whether we have normal
or inverse configuration of the dongle otherwise?
I really appreciate you're digging into this!
Cheers,
Jes
^ permalink raw reply
* [PATCH net] ipv4: Fix NULL pointer dereference in ipv4_neigh_lookup()
From: Ido Schimmel @ 2019-07-04 16:26 UTC (permalink / raw)
To: netdev; +Cc: davem, dsahern, jiri, shalomt, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
Both ip_neigh_gw4() and ip_neigh_gw6() can return either a valid pointer
or an error pointer, but the code currently checks that the pointer is
not NULL.
Fix this by checking that the pointer is not an error pointer, as this
can result in a NULL pointer dereference [1]. Specifically, I believe
that what happened is that ip_neigh_gw4() returned '-EINVAL'
(0xffffffffffffffea) to which the offset of 'refcnt' (0x70) was added,
which resulted in the address 0x000000000000005a.
[1]
BUG: KASAN: null-ptr-deref in refcount_inc_not_zero_checked+0x6e/0x180
Read of size 4 at addr 000000000000005a by task swapper/2/0
CPU: 2 PID: 0 Comm: swapper/2 Not tainted 5.2.0-rc6-custom-reg-179657-gaa32d89 #396
Hardware name: Mellanox Technologies Ltd. MSN2010/SA002610, BIOS 5.6.5 08/24/2017
Call Trace:
<IRQ>
dump_stack+0x73/0xbb
__kasan_report+0x188/0x1ea
kasan_report+0xe/0x20
refcount_inc_not_zero_checked+0x6e/0x180
ipv4_neigh_lookup+0x365/0x12c0
__neigh_update+0x1467/0x22f0
arp_process.constprop.6+0x82e/0x1f00
__netif_receive_skb_one_core+0xee/0x170
process_backlog+0xe3/0x640
net_rx_action+0x755/0xd90
__do_softirq+0x29b/0xae7
irq_exit+0x177/0x1c0
smp_apic_timer_interrupt+0x164/0x5e0
apic_timer_interrupt+0xf/0x20
</IRQ>
Fixes: 5c9f7c1dfc2e ("ipv4: Add helpers for neigh lookup for nexthop")
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reported-by: Shalom Toledo <shalomt@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
net/ipv4/route.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 8ea0735a6754..b2b35b38724d 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -447,7 +447,7 @@ static struct neighbour *ipv4_neigh_lookup(const struct dst_entry *dst,
n = ip_neigh_gw4(dev, pkey);
}
- if (n && !refcount_inc_not_zero(&n->refcnt))
+ if (!IS_ERR(n) && !refcount_inc_not_zero(&n->refcnt))
n = NULL;
rcu_read_unlock_bh();
--
2.20.1
^ permalink raw reply related
* Re: i.mx6ul with DSA in multi chip addressing mode - no MDIO access
From: Andrew Lunn @ 2019-07-04 15:53 UTC (permalink / raw)
To: Benjamin Beckmeyer; +Cc: netdev
In-Reply-To: <00b365da-9c7a-a78a-c10a-f031748e0af7@eks-engel.de>
> &mdio0 {
> interrupt-parent = <&gpio1>;
> interrupts = <3 IRQ_TYPE_LEVEL_HIGH>;
>
> switch0: switch0@2 {
> compatible = "marvell,mv88e6190";
> reg = <2>;
> pinctrl-0 = <&pinctrl_gpios>;
> reset-gpios = <&gpio4 16 GPIO_ACTIVE_LOW>;
> dsa,member = <0 0>;
This is wrong. The interrupt is a switch property, not an MDIO bus
property. So it belongs inside the switch node.
Andrew
^ permalink raw reply
* Re: [PATCH v1 net-next] net: stmmac: enable clause 45 mdio support
From: Andrew Lunn @ 2019-07-04 15:52 UTC (permalink / raw)
To: Voon, Weifeng
Cc: David S. Miller, Maxime Coquelin, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Jose Abreu, Giuseppe Cavallaro,
Florian Fainelli, Alexandre Torgue, biao huang, Ong, Boon Leong,
Kweh, Hock Leong
In-Reply-To: <D6759987A7968C4889FDA6FA91D5CBC8147388E0@PGSMSX103.gar.corp.intel.com>
> Yes, the top 16 bit of the data register only valid when C45 is enable.
> It contains the Register address which MDIO c45 frame intended for.
I think there is too much passing variables around by reference than
by value, to make this code easy to understand.
Maybe a better structure would be
static int stmmac_mdion_c45_read(struct stmmac_priv *priv, int phyaddr, int phyreg)
{
unsigned int reg_shift = priv->hw->mii.reg_shift;
unsigned int reg_mask = priv->hw->mii.reg_mask;
u32 mii_addr_val, mii_data_val;
mii_addr_val = MII_GMAC4_C45E |
((phyreg >> MII_DEVADDR_C45_SHIFT) << reg_shift) & reg_mask;
mii_data_val = (phyreg & MII_REGADDR_C45_MASK) << MII_GMAC4_REG_ADDR_SHIFT;
writel(mii_data_val, priv->ioaddr + priv->hw->mii_data);
writel(mii_addr_val, priv->ioaddr + priv->hw->mii_addrress);
return (int)readl(priv->ioaddr + mii_data) & MII_DATA_MASK;
}
static int stmmac_mdio_read(struct mii_bus *bus, int phyaddr, int phyreg)
{
...
if (readl_poll_timeout(priv->ioaddr + mii_address, v, !(v & MII_BUSY),
100, 10000))
return -EBUSY;
if (priv->plat->has_gmac4 && phyreg & MII_ADDR_C45)
return stmmac_mdio_c45_read(priv, phyaddr, phyreg);
Andrew
^ permalink raw reply
* RE: [PATCH net-next v2 3/3] net: stmmac: Introducing support for Page Pool
From: Jose Abreu @ 2019-07-04 15:40 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Jose Abreu
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org, Joao Pinto,
David S . Miller, Giuseppe Cavallaro, Alexandre Torgue,
Ilias Apalodimas, Arnd Bergmann
In-Reply-To: <20190704173758.6d985aa3@carbon>
From: Jesper Dangaard Brouer <brouer@redhat.com>
> This code is okay, but I would likely write it as:
>
> if (rx_q->page_pool) {
> page_pool_request_shutdown(rx_q->page_pool));
> page_pool_free(rx_q->page_pool);
> }
>
> Because (as you noticed) page_pool_free() have some API misuse checks,
> that will get triggered, and thus provide a warning of you forget to
> update this when driver evolves.
Yeah, makes sense. I will update and resend. Thanks for the review!
^ permalink raw reply
* Re: [PATCH net-next v2 3/3] net: stmmac: Introducing support for Page Pool
From: Jesper Dangaard Brouer @ 2019-07-04 15:37 UTC (permalink / raw)
To: Jose Abreu
Cc: linux-kernel, netdev, linux-stm32, linux-arm-kernel, Joao Pinto,
David S . Miller, Giuseppe Cavallaro, Alexandre Torgue,
Ilias Apalodimas, Arnd Bergmann, brouer
In-Reply-To: <fd2b12e6fc99f6064b0c04e1baae24328d16289f.1562252534.git.joabreu@synopsys.com>
On Thu, 4 Jul 2019 17:04:14 +0200
Jose Abreu <Jose.Abreu@synopsys.com> wrote:
> @@ -1498,8 +1480,9 @@ static void free_dma_rx_desc_resources(struct stmmac_priv *priv)
> sizeof(struct dma_extended_desc),
> rx_q->dma_erx, rx_q->dma_rx_phy);
>
> - kfree(rx_q->rx_skbuff_dma);
> - kfree(rx_q->rx_skbuff);
> + kfree(rx_q->buf_pool);
> + if (rx_q->page_pool && page_pool_request_shutdown(rx_q->page_pool))
> + page_pool_free(rx_q->page_pool);
> }
> }
This code is okay, but I would likely write it as:
if (rx_q->page_pool) {
page_pool_request_shutdown(rx_q->page_pool));
page_pool_free(rx_q->page_pool);
}
Because (as you noticed) page_pool_free() have some API misuse checks,
that will get triggered, and thus provide a warning of you forget to
update this when driver evolves.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* [PATCH 6/7] nfp: Use spinlock_t instead of struct spinlock
From: Sebastian Andrzej Siewior @ 2019-07-04 15:38 UTC (permalink / raw)
To: linux-kernel
Cc: tglx, Peter Zijlstra, Sebastian Andrzej Siewior, Jakub Kicinski,
David S. Miller, oss-drivers, netdev
In-Reply-To: <20190704153803.12739-1-bigeasy@linutronix.de>
For spinlocks the type spinlock_t should be used instead of "struct
spinlock".
Use spinlock_t for spinlock's definition.
Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: oss-drivers@netronome.com
Cc: netdev@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
drivers/net/ethernet/netronome/nfp/nfp_net.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net.h b/drivers/net/ethernet/netronome/nfp/nfp_net.h
index df9aff2684ed0..4690363fc5421 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net.h
@@ -392,7 +392,7 @@ struct nfp_net_r_vector {
struct {
struct tasklet_struct tasklet;
struct sk_buff_head queue;
- struct spinlock lock;
+ spinlock_t lock;
};
};
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net-next 3/3] net: stmmac: Introducing support for Page Pool
From: Jesper Dangaard Brouer @ 2019-07-04 15:33 UTC (permalink / raw)
To: Jose Abreu
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org, Joao Pinto,
David S . Miller, Giuseppe Cavallaro, Alexandre Torgue,
Maxime Coquelin, Maxime Ripard, Chen-Yu Tsai, brouer
In-Reply-To: <BYAPR12MB32692AA2F18A530D56383739D3FA0@BYAPR12MB3269.namprd12.prod.outlook.com>
On Thu, 4 Jul 2019 15:18:19 +0000
Jose Abreu <Jose.Abreu@synopsys.com> wrote:
> From: Jesper Dangaard Brouer <brouer@redhat.com>
>
> > You can just use page_pool_free() (p.s I'm working on reintroducing
> > page_pool_destroy wrapper). As you say, you will not have in-flight
> > frames/pages in this driver use-case.
>
> Well, if I remove the request_shutdown() it will trigger the "API usage
> violation" WARN ...
>
> I think this is due to alloc cache only be freed in request_shutdown(),
> or I'm having some leak :D
Sorry, for not being clear. You of-cause first have to call
page_pool_request_shutdown() and then call page_pool_free().
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ 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