* Re: [PATCH v4 net-next 1/4] net: core: page_pool: add user cnt preventing pool deletion
From: Ivan Khoronzhuk @ 2019-06-26 10:49 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: davem, grygorii.strashko, hawk, saeedm, leon, ast, linux-kernel,
linux-omap, xdp-newbies, ilias.apalodimas, netdev, daniel,
jakub.kicinski, john.fastabend
In-Reply-To: <20190626124216.494eee86@carbon>
On Wed, Jun 26, 2019 at 12:42:16PM +0200, Jesper Dangaard Brouer wrote:
>On Tue, 25 Jun 2019 20:59:45 +0300
>Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
>
>> Add user counter allowing to delete pool only when no users.
>> It doesn't prevent pool from flush, only prevents freeing the
>> pool instance. Helps when no need to delete the pool and now
>> it's user responsibility to free it by calling page_pool_free()
>> while destroying procedure. It also makes to use page_pool_free()
>> explicitly, not fully hidden in xdp unreg, which looks more
>> correct after page pool "create" routine.
>
>No, this is wrong.
below.
>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> ---
>> drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 8 +++++---
>> include/net/page_pool.h | 7 +++++++
>> net/core/page_pool.c | 7 +++++++
>> net/core/xdp.c | 3 +++
>> 4 files changed, 22 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>> index 5e40db8f92e6..cb028de64a1d 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,8 @@ 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);
>> + if (rq->page_pool)
>> + page_pool_free(rq->page_pool);
>> mlx5_wq_destroy(&rq->wq_ctrl);
>>
>> return err;
>> @@ -643,6 +643,8 @@ static void mlx5e_free_rq(struct mlx5e_rq *rq)
>> }
>>
>> xdp_rxq_info_unreg(&rq->xdp_rxq);
>> + if (rq->page_pool)
>> + page_pool_free(rq->page_pool);
>
>No, this is wrong. The hole point with the merged page_pool fixes
>patchset was that page_pool_free() needs to be delayed until no-more
>in-flight packets exist.
Probably it's not so obvious, but it's still delayed and deleted only
after no-more in-flight packets exist. Here question is only who is able
to do this first based on refcnt.
>
>
>> mlx5_wq_destroy(&rq->wq_ctrl);
>> }
>>
>> diff --git a/include/net/page_pool.h b/include/net/page_pool.h
>> index f07c518ef8a5..1ec838e9927e 100644
>> --- a/include/net/page_pool.h
>> +++ b/include/net/page_pool.h
>> @@ -101,6 +101,7 @@ struct page_pool {
>> struct ptr_ring ring;
>>
>> atomic_t pages_state_release_cnt;
>> + atomic_t user_cnt;
>> };
>>
>> struct page *page_pool_alloc_pages(struct page_pool *pool, gfp_t gfp);
>> @@ -183,6 +184,12 @@ static inline dma_addr_t page_pool_get_dma_addr(struct page *page)
>> return page->dma_addr;
>> }
>>
>> +/* used to prevent pool from deallocation */
>> +static inline void page_pool_get(struct page_pool *pool)
>> +{
>> + atomic_inc(&pool->user_cnt);
>> +}
>> +
>> static inline bool is_page_pool_compiled_in(void)
>> {
>> #ifdef CONFIG_PAGE_POOL
>> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
>> index b366f59885c1..169b0e3c870e 100644
>> --- a/net/core/page_pool.c
>> +++ b/net/core/page_pool.c
>> @@ -48,6 +48,7 @@ static int page_pool_init(struct page_pool *pool,
>> return -ENOMEM;
>>
>> atomic_set(&pool->pages_state_release_cnt, 0);
>> + atomic_set(&pool->user_cnt, 0);
>>
>> if (pool->p.flags & PP_FLAG_DMA_MAP)
>> get_device(pool->p.dev);
>> @@ -70,6 +71,8 @@ struct page_pool *page_pool_create(const struct page_pool_params *params)
>> kfree(pool);
>> return ERR_PTR(err);
>> }
>> +
>> + page_pool_get(pool);
>> return pool;
>> }
>> EXPORT_SYMBOL(page_pool_create);
>> @@ -356,6 +359,10 @@ static void __warn_in_flight(struct page_pool *pool)
>>
>> void __page_pool_free(struct page_pool *pool)
>> {
>> + /* free only if no users */
>> + if (!atomic_dec_and_test(&pool->user_cnt))
>> + 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 829377cc83db..04bdcd784d2e 100644
>> --- a/net/core/xdp.c
>> +++ b/net/core/xdp.c
>> @@ -372,6 +372,9 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
>>
>> mutex_unlock(&mem_id_lock);
>>
>> + if (type == MEM_TYPE_PAGE_POOL)
>> + page_pool_get(xdp_alloc->page_pool);
>> +
>> trace_mem_connect(xdp_alloc, xdp_rxq);
>> return 0;
>> err:
>
>
>
>--
>Best regards,
> Jesper Dangaard Brouer
> MSc.CS, Principal Kernel Engineer at Red Hat
> LinkedIn: http://www.linkedin.com/in/brouer
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply
* Re: [PATCH 2/2] net: stmmac: Fix crash observed if PHY does not support EEE
From: Thierry Reding @ 2019-06-26 10:45 UTC (permalink / raw)
To: Jon Hunter
Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, netdev,
linux-kernel, linux-tegra
In-Reply-To: <20190626102322.18821-2-jonathanh@nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 1236 bytes --]
On Wed, Jun 26, 2019 at 11:23:22AM +0100, Jon Hunter wrote:
> If the PHY does not support EEE mode, then a crash is observed when the
> ethernet interface is enabled. The crash occurs, because if the PHY does
> not support EEE, then although the EEE timer is never configured, it is
> still marked as enabled and so the stmmac ethernet driver is still
> trying to update the timer by calling mod_timer(). This triggers a BUG()
> in the mod_timer() because we are trying to update a timer when there is
> no callback function set because timer_setup() was never called for this
> timer.
>
> The problem is caused because we return true from the function
> stmmac_eee_init(), marking the EEE timer as enabled, even when we have
> not configured the EEE timer. Fix this by ensuring that we return false
> if the PHY does not support EEE and hence, 'eee_active' is not set.
>
> Fixes: 74371272f97f ("net: stmmac: Convert to phylink and remove phylib logic")
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
Thanks for hunting this down!
Tested-by: Thierry Reding <treding@nvidia.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] net: stmmac: Fix possible deadlock when disabling EEE support
From: Thierry Reding @ 2019-06-26 10:45 UTC (permalink / raw)
To: Jon Hunter
Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, netdev,
linux-kernel, linux-tegra
In-Reply-To: <20190626102322.18821-1-jonathanh@nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 721 bytes --]
On Wed, Jun 26, 2019 at 11:23:21AM +0100, Jon Hunter wrote:
> When stmmac_eee_init() is called to disable EEE support, then the timer
> for EEE support is stopped and we return from the function. Prior to
> stopping the timer, a mutex was acquired but in this case it is never
> released and so could cause a deadlock. Fix this by releasing the mutex
> prior to returning from stmmax_eee_init() when stopping the EEE timer.
>
> Fixes: 74371272f97f ("net: stmmac: Convert to phylink and remove phylib logic")
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 1 +
> 1 file changed, 1 insertion(+)
Tested-by: Thierry Reding <treding@nvidia.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v4 net-next 1/4] net: core: page_pool: add user cnt preventing pool deletion
From: Jesper Dangaard Brouer @ 2019-06-26 10:42 UTC (permalink / raw)
To: Ivan Khoronzhuk
Cc: davem, grygorii.strashko, hawk, saeedm, leon, ast, linux-kernel,
linux-omap, xdp-newbies, ilias.apalodimas, netdev, daniel,
jakub.kicinski, john.fastabend, brouer
In-Reply-To: <20190625175948.24771-2-ivan.khoronzhuk@linaro.org>
On Tue, 25 Jun 2019 20:59:45 +0300
Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
> Add user counter allowing to delete pool only when no users.
> It doesn't prevent pool from flush, only prevents freeing the
> pool instance. Helps when no need to delete the pool and now
> it's user responsibility to free it by calling page_pool_free()
> while destroying procedure. It also makes to use page_pool_free()
> explicitly, not fully hidden in xdp unreg, which looks more
> correct after page pool "create" routine.
No, this is wrong.
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 8 +++++---
> include/net/page_pool.h | 7 +++++++
> net/core/page_pool.c | 7 +++++++
> net/core/xdp.c | 3 +++
> 4 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index 5e40db8f92e6..cb028de64a1d 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,8 @@ 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);
> + if (rq->page_pool)
> + page_pool_free(rq->page_pool);
> mlx5_wq_destroy(&rq->wq_ctrl);
>
> return err;
> @@ -643,6 +643,8 @@ static void mlx5e_free_rq(struct mlx5e_rq *rq)
> }
>
> xdp_rxq_info_unreg(&rq->xdp_rxq);
> + if (rq->page_pool)
> + page_pool_free(rq->page_pool);
No, this is wrong. The hole point with the merged page_pool fixes
patchset was that page_pool_free() needs to be delayed until no-more
in-flight packets exist.
> mlx5_wq_destroy(&rq->wq_ctrl);
> }
>
> diff --git a/include/net/page_pool.h b/include/net/page_pool.h
> index f07c518ef8a5..1ec838e9927e 100644
> --- a/include/net/page_pool.h
> +++ b/include/net/page_pool.h
> @@ -101,6 +101,7 @@ struct page_pool {
> struct ptr_ring ring;
>
> atomic_t pages_state_release_cnt;
> + atomic_t user_cnt;
> };
>
> struct page *page_pool_alloc_pages(struct page_pool *pool, gfp_t gfp);
> @@ -183,6 +184,12 @@ static inline dma_addr_t page_pool_get_dma_addr(struct page *page)
> return page->dma_addr;
> }
>
> +/* used to prevent pool from deallocation */
> +static inline void page_pool_get(struct page_pool *pool)
> +{
> + atomic_inc(&pool->user_cnt);
> +}
> +
> static inline bool is_page_pool_compiled_in(void)
> {
> #ifdef CONFIG_PAGE_POOL
> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> index b366f59885c1..169b0e3c870e 100644
> --- a/net/core/page_pool.c
> +++ b/net/core/page_pool.c
> @@ -48,6 +48,7 @@ static int page_pool_init(struct page_pool *pool,
> return -ENOMEM;
>
> atomic_set(&pool->pages_state_release_cnt, 0);
> + atomic_set(&pool->user_cnt, 0);
>
> if (pool->p.flags & PP_FLAG_DMA_MAP)
> get_device(pool->p.dev);
> @@ -70,6 +71,8 @@ struct page_pool *page_pool_create(const struct page_pool_params *params)
> kfree(pool);
> return ERR_PTR(err);
> }
> +
> + page_pool_get(pool);
> return pool;
> }
> EXPORT_SYMBOL(page_pool_create);
> @@ -356,6 +359,10 @@ static void __warn_in_flight(struct page_pool *pool)
>
> void __page_pool_free(struct page_pool *pool)
> {
> + /* free only if no users */
> + if (!atomic_dec_and_test(&pool->user_cnt))
> + 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 829377cc83db..04bdcd784d2e 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -372,6 +372,9 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
>
> mutex_unlock(&mem_id_lock);
>
> + if (type == MEM_TYPE_PAGE_POOL)
> + page_pool_get(xdp_alloc->page_pool);
> +
> trace_mem_connect(xdp_alloc, xdp_rxq);
> return 0;
> err:
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* [PATCH v2 bpf-next] libbpf: fix max() type mismatch for 32bit
From: Ivan Khoronzhuk @ 2019-06-26 10:38 UTC (permalink / raw)
To: ast, netdev; +Cc: daniel, bpf, linux-kernel, Ivan Khoronzhuk
It fixes build error for 32bit caused by type mismatch
size_t/unsigned long.
Fixes: bf82927125dd ("libbpf: refactor map initialization")
Acked-by: Song Liu <songliubraving@fb.com>
Acked-by: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
tools/lib/bpf/libbpf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 68f45a96769f..5186b7710430 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -778,7 +778,7 @@ static struct bpf_map *bpf_object__add_map(struct bpf_object *obj)
if (obj->nr_maps < obj->maps_cap)
return &obj->maps[obj->nr_maps++];
- new_cap = max(4ul, obj->maps_cap * 3 / 2);
+ new_cap = max((size_t)4, obj->maps_cap * 3 / 2);
new_maps = realloc(obj->maps, new_cap * sizeof(*obj->maps));
if (!new_maps) {
pr_warning("alloc maps for object failed\n");
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net] ipv6: fix suspicious RCU usage in rt6_dump_route()
From: Stefano Brivio @ 2019-06-26 10:34 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S . Miller, netdev, Eric Dumazet, David Ahern
In-Reply-To: <20190626100528.218097-1-edumazet@google.com>
On Wed, 26 Jun 2019 03:05:28 -0700
Eric Dumazet <edumazet@google.com> wrote:
> syzbot reminded us that rt6_nh_dump_exceptions() needs to be called
> with rcu_read_lock()
>
> net/ipv6/route.c:1593 suspicious rcu_dereference_check() usage!
Thanks for fixing this too.
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
--
Stefano
^ permalink raw reply
* [PATCH 3/3 nf-next] netfilter: Flow table support for the bridge family
From: wenxu @ 2019-06-26 10:32 UTC (permalink / raw)
To: pablo, fw; +Cc: netfilter-devel, netdev
In-Reply-To: <1561545148-11978-1-git-send-email-wenxu@ucloud.cn>
From: wenxu <wenxu@ucloud.cn>
This patch adds the bridge flow table type, that implements the datapath
flow table to forward IPv4 traffic through bridge.
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
net/bridge/netfilter/Kconfig | 8 +++++
net/bridge/netfilter/Makefile | 1 +
net/bridge/netfilter/nf_flow_table_bridge.c | 46 +++++++++++++++++++++++++++++
3 files changed, 55 insertions(+)
create mode 100644 net/bridge/netfilter/nf_flow_table_bridge.c
diff --git a/net/bridge/netfilter/Kconfig b/net/bridge/netfilter/Kconfig
index f4fb0b9..cba5f71 100644
--- a/net/bridge/netfilter/Kconfig
+++ b/net/bridge/netfilter/Kconfig
@@ -33,6 +33,14 @@ config NF_CONNTRACK_BRIDGE
To compile it as a module, choose M here. If unsure, say N.
+config NF_FLOW_TABLE_BRIDGE
+ tristate "Netfilter flow table bridge module"
+ depends on NF_FLOW_TABLE && NF_CONNTRACK_BRIDGE
+ help
+ This option adds the flow table bridge support.
+
+ To compile it as a module, choose M here.
+
endif # NF_TABLES_BRIDGE
menuconfig BRIDGE_NF_EBTABLES
diff --git a/net/bridge/netfilter/Makefile b/net/bridge/netfilter/Makefile
index 9d77673..deb81e6 100644
--- a/net/bridge/netfilter/Makefile
+++ b/net/bridge/netfilter/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_NFT_BRIDGE_REJECT) += nft_reject_bridge.o
# connection tracking
obj-$(CONFIG_NF_CONNTRACK_BRIDGE) += nf_conntrack_bridge.o
+obj-$(CONFIG_NF_FLOW_TABLE_BRIDGE) += nf_flow_table_bridge.o
# packet logging
obj-$(CONFIG_NF_LOG_BRIDGE) += nf_log_bridge.o
diff --git a/net/bridge/netfilter/nf_flow_table_bridge.c b/net/bridge/netfilter/nf_flow_table_bridge.c
new file mode 100644
index 0000000..ad3220c
--- /dev/null
+++ b/net/bridge/netfilter/nf_flow_table_bridge.c
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/netfilter.h>
+#include <net/netfilter/nf_flow_table.h>
+#include <net/netfilter/nf_tables.h>
+
+static unsigned int
+nf_flow_offload_bridge_hook(void *priv, struct sk_buff *skb,
+ const struct nf_hook_state *state)
+{
+ switch (skb->protocol) {
+ case htons(ETH_P_IP):
+ return nf_flow_offload_ip_hook(priv, skb, state);
+ }
+
+ return NF_ACCEPT;
+}
+
+static struct nf_flowtable_type flowtable_bridge = {
+ .family = NFPROTO_BRIDGE,
+ .init = nf_flow_table_init,
+ .free = nf_flow_table_free,
+ .hook = nf_flow_offload_bridge_hook,
+ .owner = THIS_MODULE,
+};
+
+static int __init nf_flow_bridge_module_init(void)
+{
+ nft_register_flowtable_type(&flowtable_bridge);
+
+ return 0;
+}
+
+static void __exit nf_flow_bridge_module_exit(void)
+{
+ nft_unregister_flowtable_type(&flowtable_bridge);
+}
+
+module_init(nf_flow_bridge_module_init);
+module_exit(nf_flow_bridge_module_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("wenxu <wenxu@ucloud.cn>");
+MODULE_ALIAS_NF_FLOWTABLE(AF_BRIDGE);
--
1.8.3.1
^ permalink raw reply related
* [PATCH 1/3 nf-next] netfilter:nf_flow_table: Refactor flow_offload_tuple to support more offload method
From: wenxu @ 2019-06-26 10:32 UTC (permalink / raw)
To: pablo, fw; +Cc: netfilter-devel, netdev
From: wenxu <wenxu@ucloud.cn>
Add struct flow_offload_dst to support more offload method to replace
dst_cache which only work for route offload.
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
include/net/netfilter/nf_flow_table.h | 12 ++++++++++--
net/netfilter/nf_flow_table_core.c | 22 +++++++++++-----------
net/netfilter/nf_flow_table_ip.c | 4 ++--
net/netfilter/nft_flow_offload.c | 10 +++++-----
4 files changed, 28 insertions(+), 20 deletions(-)
diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h
index d8c1879..968be64 100644
--- a/include/net/netfilter/nf_flow_table.h
+++ b/include/net/netfilter/nf_flow_table.h
@@ -33,6 +33,10 @@ enum flow_offload_tuple_dir {
FLOW_OFFLOAD_DIR_MAX = IP_CT_DIR_MAX
};
+struct flow_offload_dst {
+ struct dst_entry *dst_cache;
+};
+
struct flow_offload_tuple {
union {
struct in_addr src_v4;
@@ -55,7 +59,7 @@ struct flow_offload_tuple {
u16 mtu;
- struct dst_entry *dst_cache;
+ struct flow_offload_dst dst;
};
struct flow_offload_tuple_rhash {
@@ -85,8 +89,12 @@ struct nf_flow_route {
} tuple[FLOW_OFFLOAD_DIR_MAX];
};
+struct nf_flow_data {
+ struct nf_flow_route route;
+};
+
struct flow_offload *flow_offload_alloc(struct nf_conn *ct,
- struct nf_flow_route *route);
+ struct nf_flow_data *data);
void flow_offload_free(struct flow_offload *flow);
int flow_offload_add(struct nf_flowtable *flow_table, struct flow_offload *flow);
diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
index e3d7972..125ce1c 100644
--- a/net/netfilter/nf_flow_table_core.c
+++ b/net/netfilter/nf_flow_table_core.c
@@ -24,13 +24,13 @@ struct flow_offload_entry {
static void
flow_offload_fill_dir(struct flow_offload *flow, struct nf_conn *ct,
- struct nf_flow_route *route,
+ struct nf_flow_data *data,
enum flow_offload_tuple_dir dir)
{
struct flow_offload_tuple *ft = &flow->tuplehash[dir].tuple;
struct nf_conntrack_tuple *ctt = &ct->tuplehash[dir].tuple;
- struct dst_entry *other_dst = route->tuple[!dir].dst;
- struct dst_entry *dst = route->tuple[dir].dst;
+ struct dst_entry *other_dst = date->route.tuple[!dir].dst;
+ struct dst_entry *dst = data->route.tuple[dir].dst;
ft->dir = dir;
@@ -57,7 +57,7 @@ struct flow_offload_entry {
}
struct flow_offload *
-flow_offload_alloc(struct nf_conn *ct, struct nf_flow_route *route)
+flow_offload_alloc(struct nf_conn *ct, struct nf_flow_data *data)
{
struct flow_offload_entry *entry;
struct flow_offload *flow;
@@ -72,16 +72,16 @@ struct flow_offload *
flow = &entry->flow;
- if (!dst_hold_safe(route->tuple[FLOW_OFFLOAD_DIR_ORIGINAL].dst))
+ if (!dst_hold_safe(data->route.tuple[FLOW_OFFLOAD_DIR_ORIGINAL].dst))
goto err_dst_cache_original;
- if (!dst_hold_safe(route->tuple[FLOW_OFFLOAD_DIR_REPLY].dst))
+ if (!dst_hold_safe(data->route.tuple[FLOW_OFFLOAD_DIR_REPLY].dst))
goto err_dst_cache_reply;
entry->ct = ct;
- flow_offload_fill_dir(flow, ct, route, FLOW_OFFLOAD_DIR_ORIGINAL);
- flow_offload_fill_dir(flow, ct, route, FLOW_OFFLOAD_DIR_REPLY);
+ flow_offload_fill_dir(flow, ct, data, FLOW_OFFLOAD_DIR_ORIGINAL);
+ flow_offload_fill_dir(flow, ct, data, FLOW_OFFLOAD_DIR_REPLY);
if (ct->status & IPS_SRC_NAT)
flow->flags |= FLOW_OFFLOAD_SNAT;
@@ -91,7 +91,7 @@ struct flow_offload *
return flow;
err_dst_cache_reply:
- dst_release(route->tuple[FLOW_OFFLOAD_DIR_ORIGINAL].dst);
+ dst_release(data->route.tuple[FLOW_OFFLOAD_DIR_ORIGINAL].dst);
err_dst_cache_original:
kfree(entry);
err_ct_refcnt:
@@ -139,8 +139,8 @@ void flow_offload_free(struct flow_offload *flow)
{
struct flow_offload_entry *e;
- dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.dst_cache);
- dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple.dst_cache);
+ dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.dst.dst_cache);
+ dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple.dst.dst_cache);
e = container_of(flow, struct flow_offload_entry, flow);
if (flow->flags & FLOW_OFFLOAD_DYING)
nf_ct_delete(e->ct, 0, 0);
diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
index 2413174..0016bb8 100644
--- a/net/netfilter/nf_flow_table_ip.c
+++ b/net/netfilter/nf_flow_table_ip.c
@@ -241,7 +241,7 @@ static bool nf_flow_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)
dir = tuplehash->tuple.dir;
flow = container_of(tuplehash, struct flow_offload, tuplehash[dir]);
- rt = (struct rtable *)flow->tuplehash[dir].tuple.dst_cache;
+ rt = (struct rtable *)flow->tuplehash[dir].tuple.dst.dst_cache;
outdev = rt->dst.dev;
if (unlikely(nf_flow_exceeds_mtu(skb, flow->tuplehash[dir].tuple.mtu)))
@@ -457,7 +457,7 @@ static int nf_flow_tuple_ipv6(struct sk_buff *skb, const struct net_device *dev,
dir = tuplehash->tuple.dir;
flow = container_of(tuplehash, struct flow_offload, tuplehash[dir]);
- rt = (struct rt6_info *)flow->tuplehash[dir].tuple.dst_cache;
+ rt = (struct rt6_info *)flow->tuplehash[dir].tuple.dst.dst_cache;
outdev = rt->dst.dev;
if (unlikely(nf_flow_exceeds_mtu(skb, flow->tuplehash[dir].tuple.mtu)))
diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
index aa5f571..cdb7c46 100644
--- a/net/netfilter/nft_flow_offload.c
+++ b/net/netfilter/nft_flow_offload.c
@@ -73,7 +73,7 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,
struct nft_flow_offload *priv = nft_expr_priv(expr);
struct nf_flowtable *flowtable = &priv->flowtable->data;
enum ip_conntrack_info ctinfo;
- struct nf_flow_route route;
+ struct nf_flow_data data;
struct flow_offload *flow;
enum ip_conntrack_dir dir;
bool is_tcp = false;
@@ -108,10 +108,10 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,
goto out;
dir = CTINFO2DIR(ctinfo);
- if (nft_flow_route(pkt, ct, &route, dir) < 0)
+ if (nft_flow_route(pkt, ct, &data.route, dir) < 0)
goto err_flow_route;
- flow = flow_offload_alloc(ct, &route);
+ flow = flow_offload_alloc(ct, &data);
if (!flow)
goto err_flow_alloc;
@@ -124,13 +124,13 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,
if (ret < 0)
goto err_flow_add;
- dst_release(route.tuple[!dir].dst);
+ dst_release(data.route.tuple[!dir].dst);
return;
err_flow_add:
flow_offload_free(flow);
err_flow_alloc:
- dst_release(route.tuple[!dir].dst);
+ dst_release(data.route.tuple[!dir].dst);
err_flow_route:
clear_bit(IPS_OFFLOAD_BIT, &ct->status);
out:
--
1.8.3.1
^ permalink raw reply related
* [PATCH 2/3 nf-next] netfilter:nf_flow_table: Support bridge type flow offload
From: wenxu @ 2019-06-26 10:32 UTC (permalink / raw)
To: pablo, fw; +Cc: netfilter-devel, netdev
In-Reply-To: <1561545148-11978-1-git-send-email-wenxu@ucloud.cn>
From: wenxu <wenxu@ucloud.cn>
With nf_conntrack_bridge function. The bridge family can do
conntrack it self. The flow offload function based on the
conntrack. So the flow in the bridge wih conntrack can be
offloaded.
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
include/net/netfilter/nf_flow_table.h | 30 +++++++++++-
net/netfilter/nf_flow_table_core.c | 53 ++++++++++++++++-----
net/netfilter/nf_flow_table_ip.c | 41 +++++++++++++---
net/netfilter/nft_flow_offload.c | 89 ++++++++++++++++++++++++++++++++---
4 files changed, 185 insertions(+), 28 deletions(-)
diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h
index 968be64..9a0cf27 100644
--- a/include/net/netfilter/nf_flow_table.h
+++ b/include/net/netfilter/nf_flow_table.h
@@ -33,8 +33,22 @@ enum flow_offload_tuple_dir {
FLOW_OFFLOAD_DIR_MAX = IP_CT_DIR_MAX
};
+enum flow_offload_tuple_type {
+ FLOW_OFFLOAD_TYPE_INET,
+ FLOW_OFFLOAD_TYPE_BRIDGE,
+};
+
+struct dst_br_port {
+ struct net_device *dev;
+ u16 dst_vlan_tag;
+};
+
struct flow_offload_dst {
- struct dst_entry *dst_cache;
+ enum flow_offload_tuple_type type;
+ union {
+ struct dst_entry *dst_cache;
+ struct dst_br_port dst_port;
+ };
};
struct flow_offload_tuple {
@@ -52,6 +66,7 @@ struct flow_offload_tuple {
};
int iifidx;
+ u16 vlan_tag;
u8 l3proto;
u8 l4proto;
@@ -89,8 +104,19 @@ struct nf_flow_route {
} tuple[FLOW_OFFLOAD_DIR_MAX];
};
+struct nf_flow_forward {
+ struct {
+ struct dst_br_port dst_port;
+ u16 vlan_tag;
+ } tuple[FLOW_OFFLOAD_DIR_MAX];
+};
+
struct nf_flow_data {
- struct nf_flow_route route;
+ enum flow_offload_tuple_type type;
+ union {
+ struct nf_flow_route route;
+ struct nf_flow_forward forward;
+ };
};
struct flow_offload *flow_offload_alloc(struct nf_conn *ct,
diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
index 125ce1c..19ee69c 100644
--- a/net/netfilter/nf_flow_table_core.c
+++ b/net/netfilter/nf_flow_table_core.c
@@ -29,16 +29,38 @@ struct flow_offload_entry {
{
struct flow_offload_tuple *ft = &flow->tuplehash[dir].tuple;
struct nf_conntrack_tuple *ctt = &ct->tuplehash[dir].tuple;
- struct dst_entry *other_dst = date->route.tuple[!dir].dst;
- struct dst_entry *dst = data->route.tuple[dir].dst;
+ struct dst_entry *other_dst;
+ struct dst_entry *dst;
+ struct dst_br_port other_dst_port;
+ struct dst_br_port dst_port;
+
+ if (data->type == FLOW_OFFLOAD_TYPE_BRIDGE) {
+ other_dst_port = data->forward.tuple[!dir].dst_port;
+ dst_port = data->forward.tuple[dir].dst_port;
+
+ ft->iifidx = other_dst_port.dev->ifindex;
+ ft->dst.dst_port = dst_port;
+ ft->vlan_tag = data->forward.tuple[dir].vlan_tag;
+ } else {
+ other_dst = data->route.tuple[!dir].dst;
+ dst = data->route.tuple[dir].dst;
+
+ ft->iifidx = other_dst->dev->ifindex;
+ ft->dst.dst_cache = dst;
+ }
+
+ ft->dst.type = data->type;
ft->dir = dir;
switch (ctt->src.l3num) {
case NFPROTO_IPV4:
ft->src_v4 = ctt->src.u3.in;
ft->dst_v4 = ctt->dst.u3.in;
- ft->mtu = ip_dst_mtu_maybe_forward(dst, true);
+ if (data->type == FLOW_OFFLOAD_TYPE_BRIDGE)
+ ft->mtu = dst_port.dev->mtu;
+ else
+ ft->mtu = ip_dst_mtu_maybe_forward(dst, true);
break;
case NFPROTO_IPV6:
ft->src_v6 = ctt->src.u3.in6;
@@ -51,9 +73,6 @@ struct flow_offload_entry {
ft->l4proto = ctt->dst.protonum;
ft->src_port = ctt->src.u.tcp.port;
ft->dst_port = ctt->dst.u.tcp.port;
-
- ft->iifidx = other_dst->dev->ifindex;
- ft->dst_cache = dst;
}
struct flow_offload *
@@ -72,11 +91,13 @@ struct flow_offload *
flow = &entry->flow;
- if (!dst_hold_safe(data->route.tuple[FLOW_OFFLOAD_DIR_ORIGINAL].dst))
- goto err_dst_cache_original;
+ if (data->type == FLOW_OFFLOAD_TYPE_INET) {
+ if (!dst_hold_safe(data->route.tuple[FLOW_OFFLOAD_DIR_ORIGINAL].dst))
+ goto err_dst_cache_original;
- if (!dst_hold_safe(data->route.tuple[FLOW_OFFLOAD_DIR_REPLY].dst))
- goto err_dst_cache_reply;
+ if (!dst_hold_safe(data->route.tuple[FLOW_OFFLOAD_DIR_REPLY].dst))
+ goto err_dst_cache_reply;
+ }
entry->ct = ct;
@@ -91,7 +112,8 @@ struct flow_offload *
return flow;
err_dst_cache_reply:
- dst_release(data->route.tuple[FLOW_OFFLOAD_DIR_ORIGINAL].dst);
+ if (data->type == FLOW_OFFLOAD_TYPE_INET)
+ dst_release(data->route.tuple[FLOW_OFFLOAD_DIR_ORIGINAL].dst);
err_dst_cache_original:
kfree(entry);
err_ct_refcnt:
@@ -139,8 +161,13 @@ void flow_offload_free(struct flow_offload *flow)
{
struct flow_offload_entry *e;
- dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.dst.dst_cache);
- dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple.dst.dst_cache);
+ if (flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.dst.type == FLOW_OFFLOAD_TYPE_INET) {
+ dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.dst.dst_cache);
+ dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple.dst.dst_cache);
+ } else {
+ dev_put(flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.dst.dst_port.dev);
+ dev_put(flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple.dst.dst_port.dev);
+ }
e = container_of(flow, struct flow_offload_entry, flow);
if (flow->flags & FLOW_OFFLOAD_DYING)
nf_ct_delete(e->ct, 0, 0);
diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
index 0016bb8..9af01ef 100644
--- a/net/netfilter/nf_flow_table_ip.c
+++ b/net/netfilter/nf_flow_table_ip.c
@@ -16,6 +16,8 @@
#include <linux/tcp.h>
#include <linux/udp.h>
+#include "../bridge/br_private.h"
+
static int nf_flow_state_check(struct flow_offload *flow, int proto,
struct sk_buff *skb, unsigned int thoff)
{
@@ -220,6 +222,7 @@ static bool nf_flow_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)
{
struct flow_offload_tuple_rhash *tuplehash;
struct nf_flowtable *flow_table = priv;
+ int family = flow_table->type->family;
struct flow_offload_tuple tuple = {};
enum flow_offload_tuple_dir dir;
struct flow_offload *flow;
@@ -228,6 +231,7 @@ static bool nf_flow_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)
unsigned int thoff;
struct iphdr *iph;
__be32 nexthop;
+ u16 vlan_tag;
if (skb->protocol != htons(ETH_P_IP))
return NF_ACCEPT;
@@ -235,14 +239,25 @@ static bool nf_flow_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)
if (nf_flow_tuple_ip(skb, state->in, &tuple) < 0)
return NF_ACCEPT;
+ if (family != NFPROTO_BRIDGE && family != NFPROTO_IPV4)
+ return NF_ACCEPT;
+
+ if (family == NFPROTO_BRIDGE && skb_vlan_tag_present(skb))
+ tuple.vlan_tag = skb_vlan_tag_get_id(skb);
+
tuplehash = flow_offload_lookup(flow_table, &tuple);
if (tuplehash == NULL)
return NF_ACCEPT;
dir = tuplehash->tuple.dir;
flow = container_of(tuplehash, struct flow_offload, tuplehash[dir]);
- rt = (struct rtable *)flow->tuplehash[dir].tuple.dst.dst_cache;
- outdev = rt->dst.dev;
+ if (family == NFPROTO_IPV4) {
+ rt = (struct rtable *)flow->tuplehash[dir].tuple.dst.dst_cache;
+ outdev = rt->dst.dev;
+ } else {
+ vlan_tag = flow->tuplehash[dir].tuple.dst.dst_port.dst_vlan_tag;
+ outdev = flow->tuplehash[dir].tuple.dst.dst_port.dev;
+ }
if (unlikely(nf_flow_exceeds_mtu(skb, flow->tuplehash[dir].tuple.mtu)))
return NF_ACCEPT;
@@ -258,13 +273,25 @@ static bool nf_flow_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)
return NF_DROP;
flow->timeout = (u32)jiffies + NF_FLOW_TIMEOUT;
- iph = ip_hdr(skb);
- ip_decrease_ttl(iph);
skb->dev = outdev;
- nexthop = rt_nexthop(rt, flow->tuplehash[!dir].tuple.src_v4.s_addr);
- skb_dst_set_noref(skb, &rt->dst);
- neigh_xmit(NEIGH_ARP_TABLE, outdev, &nexthop, skb);
+ if (family == NFPROTO_IPV4) {
+ iph = ip_hdr(skb);
+ ip_decrease_ttl(iph);
+
+ nexthop = rt_nexthop(rt, flow->tuplehash[!dir].tuple.src_v4.s_addr);
+ skb_dst_set_noref(skb, &rt->dst);
+ neigh_xmit(NEIGH_ARP_TABLE, outdev, &nexthop, skb);
+ } else {
+ const struct net_bridge_port *p;
+
+ if (vlan_tag && (p = br_port_get_rtnl_rcu(state->in)))
+ __vlan_hwaccel_put_tag(skb, p->br->vlan_proto, vlan_tag);
+ else
+ __vlan_hwaccel_clear_tag(skb);
+
+ br_dev_queue_push_xmit(state->net, state->sk, skb);
+ }
return NF_STOLEN;
}
diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
index cdb7c46..c88396a 100644
--- a/net/netfilter/nft_flow_offload.c
+++ b/net/netfilter/nft_flow_offload.c
@@ -14,6 +14,8 @@
#include <linux/netfilter/nf_conntrack_common.h>
#include <net/netfilter/nf_flow_table.h>
+#include "../bridge/br_private.h"
+
struct nft_flow_offload {
struct nft_flowtable *flowtable;
};
@@ -49,6 +51,58 @@ static int nft_flow_route(const struct nft_pktinfo *pkt,
return 0;
}
+static int nft_flow_forward(const struct nft_pktinfo *pkt,
+ const struct nf_conn *ct,
+ struct nf_flow_forward *forward,
+ enum ip_conntrack_dir dir)
+{
+ struct net_bridge_vlan_group *vg;
+ const struct net_bridge_port *p;
+ u16 vid = 0;
+
+ if (skb_vlan_tag_present(pkt->skb))
+ vid = skb_vlan_tag_get_id(pkt->skb);
+
+ forward->tuple[dir].dst_port.dst_vlan_tag = vid;
+ forward->tuple[!dir].vlan_tag = vid;
+ forward->tuple[dir].dst_port.dev = dev_get_by_index(dev_net(nft_out(pkt)),
+ nft_out(pkt)->ifindex);
+ forward->tuple[!dir].dst_port.dev = dev_get_by_index(dev_net(nft_in(pkt)),
+ nft_in(pkt)->ifindex);
+
+ rtnl_lock();
+ p = br_port_get_rtnl_rcu(nft_out(pkt));
+ if (p) {
+ if (!br_opt_get(p->br, BROPT_VLAN_ENABLED))
+ goto out;
+
+ if (!vid) {
+ vg = nbp_vlan_group_rcu(p);
+ vid = br_get_pvid(vg);
+ }
+
+ if (vid) {
+ struct bridge_vlan_info info;
+
+ if (br_vlan_get_info(nft_in(pkt), vid, &info) == 0 &&
+ info.flags & BRIDGE_VLAN_INFO_UNTAGGED)
+ vid = 0;
+ }
+ } else {
+ rtnl_unlock();
+ dev_put(forward->tuple[dir].dst_port.dev);
+ dev_put(forward->tuple[!dir].dst_port.dev);
+ return -ENOENT;
+ }
+
+out:
+ rtnl_unlock();
+ forward->tuple[!dir].dst_port.dst_vlan_tag = vid;
+ forward->tuple[dir].vlan_tag = vid;
+
+ return 0;
+}
+
static bool nft_flow_offload_skip(struct sk_buff *skb, int family)
{
if (skb_sec_path(skb))
@@ -61,6 +115,15 @@ static bool nft_flow_offload_skip(struct sk_buff *skb, int family)
if (unlikely(opt->optlen))
return true;
+ } else if (family == NFPROTO_BRIDGE) {
+ const struct iphdr *iph;
+
+ if (skb->protocol != htons(ETH_P_IP))
+ return true;
+
+ iph = ip_hdr(skb);
+ if (iph->ihl > 5)
+ return true;
}
return false;
@@ -76,11 +139,12 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,
struct nf_flow_data data;
struct flow_offload *flow;
enum ip_conntrack_dir dir;
+ int family = nft_pf(pkt);
bool is_tcp = false;
struct nf_conn *ct;
int ret;
- if (nft_flow_offload_skip(pkt->skb, nft_pf(pkt)))
+ if (nft_flow_offload_skip(pkt->skb, family))
goto out;
ct = nf_ct_get(pkt->skb, &ctinfo);
@@ -108,8 +172,15 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,
goto out;
dir = CTINFO2DIR(ctinfo);
- if (nft_flow_route(pkt, ct, &data.route, dir) < 0)
- goto err_flow_route;
+ if (family == NFPROTO_BRIDGE) {
+ data.type = FLOW_OFFLOAD_TYPE_BRIDGE;
+ if (nft_flow_forward(pkt, ct, &data.forward, dir) < 0)
+ goto err_flow_data;
+ } else {
+ data.type = FLOW_OFFLOAD_TYPE_INET;
+ if (nft_flow_route(pkt, ct, &data.route, dir) < 0)
+ goto err_flow_data;
+ }
flow = flow_offload_alloc(ct, &data);
if (!flow)
@@ -124,14 +195,20 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,
if (ret < 0)
goto err_flow_add;
- dst_release(data.route.tuple[!dir].dst);
+ if (family != NFPROTO_BRIDGE)
+ dst_release(data.route.tuple[!dir].dst);
return;
err_flow_add:
flow_offload_free(flow);
err_flow_alloc:
- dst_release(data.route.tuple[!dir].dst);
-err_flow_route:
+ if (family == NFPROTO_BRIDGE) {
+ dev_put(data.forward.tuple[dir].dst_port.dev);
+ dev_put(data.forward.tuple[!dir].dst_port.dev);
+ } else {
+ dst_release(data.route.tuple[!dir].dst);
+ }
+err_flow_data:
clear_bit(IPS_OFFLOAD_BIT, &ct->status);
out:
regs->verdict.code = NFT_BREAK;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH nf-next v2 1/2] netfilter: nft_meta: add NFT_META_BRI_PVID support
From: Pablo Neira Ayuso @ 2019-06-26 10:30 UTC (permalink / raw)
To: wenxu; +Cc: fw, netfilter-devel, netdev
In-Reply-To: <20190626090116.3qjmlnd5egeifozq@salvia>
On Wed, Jun 26, 2019 at 11:01:16AM +0200, Pablo Neira Ayuso wrote:
> On Thu, Jun 20, 2019 at 09:17:39AM +0800, wenxu@ucloud.cn wrote:
> > From: wenxu <wenxu@ucloud.cn>
> >
> > nft add table bridge firewall
> > nft add chain bridge firewall zones { type filter hook prerouting priority - 300 \; }
> > nft add rule bridge firewall zones counter ct zone set vlan id map { 100 : 1, 200 : 2 }
> >
> > As above set the bridge port with pvid, the received packet don't contain
> > the vlan tag which means the packet should belong to vlan 200 through pvid.
> > With this pacth user can get the pvid of bridge ports.
> >
> > So add the following rule for as the first rule in the chain of zones.
> >
> > nft add rule bridge firewall zones counter meta brvlan set meta brpvid
>
> Applied, thanks.
https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git/commit/?id=da4f10a4265b109fbdb77d5995236e0843e4a26d
This patch is applied.
2/2 is not, until we finish a bit of discussion on where to go.
Thanks.
^ permalink raw reply
* Re: [PATCH nf-next v2 2/2] netfilter: nft_meta: Add NFT_META_BRI_VLAN support
From: Pablo Neira Ayuso @ 2019-06-26 10:29 UTC (permalink / raw)
To: wenxu; +Cc: fw, netfilter-devel, netdev
In-Reply-To: <1560993460-25569-2-git-send-email-wenxu@ucloud.cn>
[-- Attachment #1: Type: text/plain, Size: 530 bytes --]
Could you add a NFT_META_BRI_VLAN_PROTO? Similar to patch 1/2, to
retrieve p->br->vlan_proto.
Then, add a generic way to set the vlan metadata. I'm attaching an
incomplete patch, so there is something like:
meta vlan set 0x88a8:20
to set q-in-q.
we could also add a shortcut for simple vlan case (no q-in-q), ie.
assuming protocol is 0x8100:
meta vlan set 20
Does this make sense to you?
And we have a way to set the meta vlan information from ingress to
then, which is something I also need here.
Thanks.
[-- Attachment #2: x.patch --]
[-- Type: text/x-diff, Size: 2341 bytes --]
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 8859535031e2..6ef2cc42924c 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -796,6 +796,7 @@ enum nft_exthdr_attributes {
* @NFT_META_IIFKIND: packet input interface kind name (dev->rtnl_link_ops->kind)
* @NFT_META_OIFKIND: packet output interface kind name (dev->rtnl_link_ops->kind)
* @NFT_META_BRI_PVID: packet input bridge port pvid
+ * @NFT_META_VLAN: packet vlan metadata
*/
enum nft_meta_keys {
NFT_META_LEN,
@@ -827,6 +828,7 @@ enum nft_meta_keys {
NFT_META_IIFKIND,
NFT_META_OIFKIND,
NFT_META_BRI_PVID,
+ NFT_META_VLAN,
};
/**
@@ -893,12 +895,14 @@ enum nft_hash_attributes {
* @NFTA_META_DREG: destination register (NLA_U32)
* @NFTA_META_KEY: meta data item to load (NLA_U32: nft_meta_keys)
* @NFTA_META_SREG: source register (NLA_U32)
+ * @NFTA_META_SREG2: source register (NLA_U32)
*/
enum nft_meta_attributes {
NFTA_META_UNSPEC,
NFTA_META_DREG,
NFTA_META_KEY,
NFTA_META_SREG,
+ NFTA_META_SREG2,
__NFTA_META_MAX
};
#define NFTA_META_MAX (__NFTA_META_MAX - 1)
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index 4f8116de70f8..dbbad7319183 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -28,7 +28,10 @@ struct nft_meta {
enum nft_meta_keys key:8;
union {
enum nft_registers dreg:8;
- enum nft_registers sreg:8;
+ struct {
+ enum nft_registers sreg:8;
+ enum nft_registers sreg2:8;
+ };
};
};
@@ -304,6 +307,17 @@ static void nft_meta_set_eval(const struct nft_expr *expr,
skb->secmark = value;
break;
#endif
+ case NFT_META_VLAN: {
+ u32 *sreg2 = ®s->data[meta->sreg2];
+ __be16 vlan_proto;
+ u16 vlan_tci;
+
+ vlan_tci = nft_reg_load16(sreg);
+ vlan_proto = nft_reg_load16(sreg2);
+
+ __vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
+ break;
+ }
default:
WARN_ON(1);
}
@@ -474,6 +488,13 @@ static int nft_meta_set_init(const struct nft_ctx *ctx,
case NFT_META_PKTTYPE:
len = sizeof(u8);
break;
+ case NFT_META_VLAN:
+ len = sizeof(u16);
+ priv->sreg2 = nft_parse_register(tb[NFTA_META_SREG2]);
+ err = nft_validate_register_load(priv->sreg2, len);
+ if (err < 0)
+ return err;
+ break;
default:
return -EOPNOTSUPP;
}
^ permalink raw reply related
* Re: [PATCH net] ipv4: fix suspicious RCU usage in fib_dump_info_fnhe()
From: Stefano Brivio @ 2019-06-26 10:28 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S . Miller, netdev, Eric Dumazet, David Ahern, syzbot
In-Reply-To: <20190626100450.217106-1-edumazet@google.com>
On Wed, 26 Jun 2019 03:04:50 -0700
Eric Dumazet <edumazet@google.com> wrote:
> sysbot reported that we lack appropriate rcu_read_lock()
> protection in fib_dump_info_fnhe()
Thanks for fixing this.
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index 6aee412a68bdd3c24a6a0eb9883e04b7a83998e0..59670fafcd2612b94c237cbe30109adb196cf3f0 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -2872,12 +2872,13 @@ int fib_dump_info_fnhe(struct sk_buff *skb, struct netlink_callback *cb,
> if (nhc->nhc_flags & RTNH_F_DEAD)
> continue;
>
> + rcu_read_lock();
> bucket = rcu_dereference(nhc->nhc_exceptions);
> - if (!bucket)
> - continue;
> -
> - err = fnhe_dump_bucket(net, skb, cb, table_id, bucket, genid,
> - fa_index, fa_start);
> + err = 0;
Could you perhaps move declaration and initialisation of 'err' outside
the block while at it? It looks a bit more readable at this point.
> + if (bucket)
> + err = fnhe_dump_bucket(net, skb, cb, table_id, bucket,
> + genid, fa_index, fa_start);
> + rcu_read_unlock();
> if (err)
> return err;
> }
Either way,
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
--
Stefano
^ permalink raw reply
* [PATCH 2/2] net: stmmac: Fix crash observed if PHY does not support EEE
From: Jon Hunter @ 2019-06-26 10:23 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu
Cc: netdev, linux-kernel, linux-tegra, Jon Hunter
In-Reply-To: <20190626102322.18821-1-jonathanh@nvidia.com>
If the PHY does not support EEE mode, then a crash is observed when the
ethernet interface is enabled. The crash occurs, because if the PHY does
not support EEE, then although the EEE timer is never configured, it is
still marked as enabled and so the stmmac ethernet driver is still
trying to update the timer by calling mod_timer(). This triggers a BUG()
in the mod_timer() because we are trying to update a timer when there is
no callback function set because timer_setup() was never called for this
timer.
The problem is caused because we return true from the function
stmmac_eee_init(), marking the EEE timer as enabled, even when we have
not configured the EEE timer. Fix this by ensuring that we return false
if the PHY does not support EEE and hence, 'eee_active' is not set.
Fixes: 74371272f97f ("net: stmmac: Convert to phylink and remove phylib logic")
Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 6c6c6ec3c781..8f5ebd51859e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -398,10 +398,12 @@ bool stmmac_eee_init(struct stmmac_priv *priv)
mutex_lock(&priv->lock);
/* Check if it needs to be deactivated */
- if (!priv->eee_active && priv->eee_enabled) {
- netdev_dbg(priv->dev, "disable EEE\n");
- del_timer_sync(&priv->eee_ctrl_timer);
- stmmac_set_eee_timer(priv, priv->hw, 0, tx_lpi_timer);
+ if (!priv->eee_active) {
+ if (priv->eee_enabled) {
+ netdev_dbg(priv->dev, "disable EEE\n");
+ del_timer_sync(&priv->eee_ctrl_timer);
+ stmmac_set_eee_timer(priv, priv->hw, 0, tx_lpi_timer);
+ }
mutex_unlock(&priv->lock);
return false;
}
--
1.9.1
^ permalink raw reply related
* [PATCH 1/2] net: stmmac: Fix possible deadlock when disabling EEE support
From: Jon Hunter @ 2019-06-26 10:23 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu
Cc: netdev, linux-kernel, linux-tegra, Jon Hunter
When stmmac_eee_init() is called to disable EEE support, then the timer
for EEE support is stopped and we return from the function. Prior to
stopping the timer, a mutex was acquired but in this case it is never
released and so could cause a deadlock. Fix this by releasing the mutex
prior to returning from stmmax_eee_init() when stopping the EEE timer.
Fixes: 74371272f97f ("net: stmmac: Convert to phylink and remove phylib logic")
Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index b628c697cee9..6c6c6ec3c781 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -402,6 +402,7 @@ bool stmmac_eee_init(struct stmmac_priv *priv)
netdev_dbg(priv->dev, "disable EEE\n");
del_timer_sync(&priv->eee_ctrl_timer);
stmmac_set_eee_timer(priv, priv->hw, 0, tx_lpi_timer);
+ mutex_unlock(&priv->lock);
return false;
}
--
1.9.1
^ permalink raw reply related
* BUG: unable to handle kernel paging request in tls_prots
From: syzbot @ 2019-06-26 10:17 UTC (permalink / raw)
To: ast, bpf, daniel, davem, edumazet, john.fastabend, kafai, kuznet,
linux-kernel, netdev, songliubraving, syzkaller-bugs, yhs,
yoshfuji
Hello,
syzbot found the following crash on:
HEAD commit: 904d88d7 qmi_wwan: Fix out-of-bounds read
git tree: net
console output: https://syzkaller.appspot.com/x/log.txt?x=14a8b865a00000
kernel config: https://syzkaller.appspot.com/x/.config?x=137ec2016ea3870d
dashboard link: https://syzkaller.appspot.com/bug?extid=4207c7f3a443366d8aa2
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15576c71a00000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+4207c7f3a443366d8aa2@syzkaller.appspotmail.com
BUG: unable to handle page fault for address: 000000004125973f
#PF: supervisor write access in kernel mode
#PF: error_code(0x0002) - not-present page
PGD 92e1a067 P4D 92e1a067 PUD 0
Thread overran stack, or stack corrupted
Oops: 0002 [#1] PREEMPT SMP KASAN
CPU: 1 PID: 9943 Comm: blkid Not tainted 5.2.0-rc5+ #62
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
RIP: 0010:tls_prots+0x1a8a/0x3520
Code: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 70 0d 0f 86 ff ff ff ff 40 5d <28> 86 ff ff ff
ff 60 6e 28 86 ff ff ff ff 00 de ed 85 ff ff ff ff
RSP: 0018:ffff888079bc7c10 EFLAGS: 00010082
RAX: ffff88808c5226c0 RBX: ffff88808cb36100 RCX: 1ffffffff116885c
RDX: 1ffff110118a44d8 RSI: 0000000041259740 RDI: ffff88808c523a70
RBP: ffff888079bc7c38 R08: ffff88808c5226c0 R09: ffffed1015d26c70
R10: ffffed1015d26c6f R11: ffff8880ae93637b R12: ffffffff860dd760
R13: ffffffff860dda75 R14: ffff888079bc7c08 R15: ffffffff8b1da9a0
FS: 00007fa341259740(0000) GS:ffff8880ae900000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000000004125973f CR3: 00000000907f6000 CR4: 00000000001406e0
Call Trace:
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
tcp_bpf_unhash net/ipv4/tcp_bpf.c:550 [inline]
tcp_bpf_unhash+0x315/0x390 net/ipv4/tcp_bpf.c:540
WARNING: kernel stack frame pointer at 00000000e809d5dc in blkid:9943 has
bad value 000000009844d018
unwind stack type:0 next_sp:000000005f927a44 mask:0x2 graph_idx:0
000000002401cb95: ffff888079bc7c68 (0xffff888079bc7c68)
00000000c24ed912: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
00000000db5b548d: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
00000000479aff76: ffff88808cb36100 (0xffff88808cb36100)
000000004473e5aa: 0000000000000000 ...
000000009d12a602: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
00000000ba44e25c: ffff888079bc7c98 (0xffff888079bc7c98)
00000000e1eb23e4: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
00000000f7d29ffb: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
00000000dc871c27: ffff88808cb36100 (0xffff88808cb36100)
000000003824545e: 0000000000000000 ...
000000008a182c7c: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
000000001c7a51e7: ffff888079bc7cc8 (0xffff888079bc7cc8)
0000000049da0237: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
000000009c31c0da: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
0000000024f3129d: ffff88808cb36100 (0xffff88808cb36100)
0000000040823c88: 0000000000000000 ...
000000004c33f42e: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
00000000fa38e5a4: ffff888079bc7cf8 (0xffff888079bc7cf8)
0000000030a46a60: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
00000000313cd10c: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
000000003feafbe9: ffff88808cb36100 (0xffff88808cb36100)
00000000b2d0344c: 0000000000000000 ...
00000000a54c1af0: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
00000000d4027d6c: ffff888079bc7d28 (0xffff888079bc7d28)
000000009ee98499: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
00000000cc716643: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
00000000f66028fc: ffff88808cb36100 (0xffff88808cb36100)
00000000f1a9f4a4: 0000000000000000 ...
0000000093e98749: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
0000000069764410: ffff888079bc7d58 (0xffff888079bc7d58)
0000000099cc4d5e: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
000000008452556b: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
000000009d210107: ffff88808cb36100 (0xffff88808cb36100)
000000000b3d1c61: 0000000000000000 ...
0000000029482aba: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
000000000080c2ff: ffff888079bc7d88 (0xffff888079bc7d88)
000000004e072bc8: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
00000000049c3c65: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
00000000b89e7d9b: ffff88808cb36100 (0xffff88808cb36100)
000000004f6d4a66: 0000000000000000 ...
0000000062d2c1b7: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
0000000064dd81c9: ffff888079bc7db8 (0xffff888079bc7db8)
000000003089fca0: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
0000000080e7f1f9: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
00000000487c3ec3: ffff88808cb36100 (0xffff88808cb36100)
00000000291b99a5: 0000000000000000 ...
00000000879c3ace: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
0000000088f16e91: ffff888079bc7de8 (0xffff888079bc7de8)
00000000feaf9900: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
00000000699d8a79: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
00000000ea800749: ffff88808cb36100 (0xffff88808cb36100)
000000002c99629d: 0000000000000000 ...
00000000f89aef64: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
000000008a8c9aec: ffff888079bc7e18 (0xffff888079bc7e18)
0000000050ff6d78: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
000000008764d630: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
0000000091e33368: ffff88808cb36100 (0xffff88808cb36100)
00000000e1a0ace5: 0000000000000000 ...
00000000cbc751ab: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
00000000d4f9acf4: ffff888079bc7e48 (0xffff888079bc7e48)
0000000081fc1715: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
00000000aa6f1bc2: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
00000000d2a9e7ce: ffff88808cb36100 (0xffff88808cb36100)
000000006f96fab3: 0000000000000000 ...
00000000aeaeead6: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
00000000f35f95f5: ffff888079bc7e78 (0xffff888079bc7e78)
000000008c846b2f: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
000000001c6a7237: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
00000000799c1d50: ffff88808cb36100 (0xffff88808cb36100)
00000000447f9e03: 0000000000000000 ...
00000000ec08ab6f: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
0000000079b88671: ffff888079bc7ea8 (0xffff888079bc7ea8)
0000000057ceab90: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
00000000a3af49c4: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
00000000c8233266: ffff88808cb36100 (0xffff88808cb36100)
00000000a4346829: 0000000000000000 ...
00000000a77b30e0: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
000000009289d9ef: ffff888079bc7ed8 (0xffff888079bc7ed8)
0000000093a94b95: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
00000000bba52dcd: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
000000004ba72af3: ffff88808cb36100 (0xffff88808cb36100)
000000002713380e: 0000000000000000 ...
0000000082b4f5fe: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
00000000fd331aa8: ffff888079bc7f08 (0xffff888079bc7f08)
00000000a125cb7a: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
00000000b176b5fe: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
0000000021b424c8: ffff88808cb36100 (0xffff88808cb36100)
00000000c8aca3a7: 0000000000000000 ...
00000000ae0e04d5: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
00000000218dfe2f: ffff888079bc7f38 (0xffff888079bc7f38)
000000006f81db8c: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
000000005415d20a: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
00000000cd8b4d0d: ffff88808cb36100 (0xffff88808cb36100)
0000000028dde223: 0000000000000000 ...
00000000f1bac7a7: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
00000000819b4e09: ffff888079bc7f68 (0xffff888079bc7f68)
0000000040faf187: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
00000000d40fc060: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
000000002efea550: ffff88808cb36100 (0xffff88808cb36100)
0000000056721274: 0000000000000000 ...
000000008c6a3097: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
0000000039ab3cf0: ffff888079bc7f98 (0xffff888079bc7f98)
00000000b825c1f0: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
000000005d65596f: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
00000000a0cc451f: ffff88808cb36100 (0xffff88808cb36100)
00000000e7f05b41: 0000000000000000 ...
0000000049404ce5: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
000000001a81ddbb: ffff888079bc7fc8 (0xffff888079bc7fc8)
000000003afd376f: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
0000000067da2263: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
00000000951815ed: ffff88808cb36100 (0xffff88808cb36100)
00000000eb36eac9: 0000000000000000 ...
0000000084ea5e52: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
00000000e809d5dc: ffff888079bc7ff8 (0xffff888079bc7ff8)
00000000e50d63ae: ffffffff860dda75 (tcp_bpf_unhash+0x315/0x390)
00000000d90e11b6: ffffffff860dd760 (tcp_bpf_recvmsg+0xa40/0xa40)
000000000a06acca: ffff88808cb36100 (0xffff88808cb36100)
000000006c7f5c3c: 0000000000000000 ...
00000000e157dcd0: ffffffff8b1da9a0 (tls_prots+0x1a80/0x3520)
000000009844d018: ffff888079bc8028 (0xffff888079bc8028)
Modules linked in:
CR2: 000000004125973f
---[ end trace 79fdcdc2ec39f3e0 ]---
RIP: 0010:tls_prots+0x1a8a/0x3520
Code: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 70 0d 0f 86 ff ff ff ff 40 5d <28> 86 ff ff ff
ff 60 6e 28 86 ff ff ff ff 00 de ed 85 ff ff ff ff
RSP: 0018:ffff888079bc7c10 EFLAGS: 00010082
RAX: ffff88808c5226c0 RBX: ffff88808cb36100 RCX: 1ffffffff116885c
RDX: 1ffff110118a44d8 RSI: 0000000041259740 RDI: ffff88808c523a70
RBP: ffff888079bc7c38 R08: ffff88808c5226c0 R09: ffffed1015d26c70
R10: ffffed1015d26c6f R11: ffff8880ae93637b R12: ffffffff860dd760
R13: ffffffff860dda75 R14: ffff888079bc7c08 R15: ffffffff8b1da9a0
FS: 00007fa341259740(0000) GS:ffff8880ae900000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000000004125973f CR3: 00000000907f6000 CR4: 00000000001406e0
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* Re: [PATCH net] ipv6: fix suspicious RCU usage in rt6_dump_route()
From: Eric Dumazet @ 2019-06-26 10:09 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, Eric Dumazet, Stefano Brivio, David Ahern
In-Reply-To: <20190626100528.218097-1-edumazet@google.com>
On Wed, Jun 26, 2019 at 12:05 PM Eric Dumazet <edumazet@google.com> wrote:
>
> syzbot reminded us that rt6_nh_dump_exceptions() needs to be called
> with rcu_read_lock()
Same remark : This is for net-next tree.
^ permalink raw reply
* Re: [PATCH net] ipv4: fix suspicious RCU usage in fib_dump_info_fnhe()
From: Eric Dumazet @ 2019-06-26 10:09 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Eric Dumazet, Stefano Brivio, David Ahern, syzbot
In-Reply-To: <20190626100450.217106-1-edumazet@google.com>
On Wed, Jun 26, 2019 at 12:04 PM Eric Dumazet <edumazet@google.com> wrote:
>
> sysbot reported that we lack appropriate rcu_read_lock()
> protection in fib_dump_info_fnhe()
This is for net-next tree, sorry for the confusion.
^ permalink raw reply
* [PATCH net] ipv6: fix suspicious RCU usage in rt6_dump_route()
From: Eric Dumazet @ 2019-06-26 10:05 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Eric Dumazet, Eric Dumazet, Stefano Brivio, David Ahern
syzbot reminded us that rt6_nh_dump_exceptions() needs to be called
with rcu_read_lock()
net/ipv6/route.c:1593 suspicious rcu_dereference_check() usage!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
2 locks held by syz-executor609/8966:
#0: 00000000b7dbe288 (rtnl_mutex){+.+.}, at: netlink_dump+0xe7/0xfb0 net/netlink/af_netlink.c:2199
#1: 00000000f2d87c21 (&(&tb->tb6_lock)->rlock){+...}, at: spin_lock_bh include/linux/spinlock.h:343 [inline]
#1: 00000000f2d87c21 (&(&tb->tb6_lock)->rlock){+...}, at: fib6_dump_table.isra.0+0x37e/0x570 net/ipv6/ip6_fib.c:533
stack backtrace:
CPU: 0 PID: 8966 Comm: syz-executor609 Not tainted 5.2.0-rc5+ #43
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x172/0x1f0 lib/dump_stack.c:113
lockdep_rcu_suspicious+0x153/0x15d kernel/locking/lockdep.c:5250
fib6_nh_get_excptn_bucket+0x18e/0x1b0 net/ipv6/route.c:1593
rt6_nh_dump_exceptions+0x45/0x4d0 net/ipv6/route.c:5541
rt6_dump_route+0x904/0xc50 net/ipv6/route.c:5640
fib6_dump_node+0x168/0x280 net/ipv6/ip6_fib.c:467
fib6_walk_continue+0x4a9/0x8e0 net/ipv6/ip6_fib.c:1986
fib6_walk+0x9d/0x100 net/ipv6/ip6_fib.c:2034
fib6_dump_table.isra.0+0x38a/0x570 net/ipv6/ip6_fib.c:534
inet6_dump_fib+0x93c/0xb00 net/ipv6/ip6_fib.c:624
rtnl_dump_all+0x295/0x490 net/core/rtnetlink.c:3445
netlink_dump+0x558/0xfb0 net/netlink/af_netlink.c:2244
__netlink_dump_start+0x5b1/0x7d0 net/netlink/af_netlink.c:2352
netlink_dump_start include/linux/netlink.h:226 [inline]
rtnetlink_rcv_msg+0x73d/0xb00 net/core/rtnetlink.c:5182
netlink_rcv_skb+0x177/0x450 net/netlink/af_netlink.c:2477
rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5237
netlink_unicast_kernel net/netlink/af_netlink.c:1302 [inline]
netlink_unicast+0x531/0x710 net/netlink/af_netlink.c:1328
netlink_sendmsg+0x8ae/0xd70 net/netlink/af_netlink.c:1917
sock_sendmsg_nosec net/socket.c:646 [inline]
sock_sendmsg+0xd7/0x130 net/socket.c:665
sock_write_iter+0x27c/0x3e0 net/socket.c:994
call_write_iter include/linux/fs.h:1872 [inline]
new_sync_write+0x4d3/0x770 fs/read_write.c:483
__vfs_write+0xe1/0x110 fs/read_write.c:496
vfs_write+0x20c/0x580 fs/read_write.c:558
ksys_write+0x14f/0x290 fs/read_write.c:611
__do_sys_write fs/read_write.c:623 [inline]
__se_sys_write fs/read_write.c:620 [inline]
__x64_sys_write+0x73/0xb0 fs/read_write.c:620
do_syscall_64+0xfd/0x680 arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4401b9
Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ffc8e134978 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 00000000004401b9
RDX: 000000000000001c RSI: 0000000020000000 RDI: 00
Fixes: 1e47b4837f3b ("ipv6: Dump route exceptions if requested")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Stefano Brivio <sbrivio@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
---
net/ipv6/route.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index be5e65c97652d0c34d209f85c8295d6faf871990..c59e97cf9d25da3084098572896178b71fb28fe6 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -5632,6 +5632,7 @@ int rt6_dump_route(struct fib6_info *rt, void *p_arg, unsigned int skip)
.count = 0 };
int err;
+ rcu_read_lock();
if (rt->nh) {
err = nexthop_for_each_fib6_nh(rt->nh,
rt6_nh_dump_exceptions,
@@ -5639,6 +5640,7 @@ int rt6_dump_route(struct fib6_info *rt, void *p_arg, unsigned int skip)
} else {
err = rt6_nh_dump_exceptions(rt->fib6_nh, &w);
}
+ rcu_read_unlock();
if (err)
return count += w.count;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH net] ipv4: fix suspicious RCU usage in fib_dump_info_fnhe()
From: Eric Dumazet @ 2019-06-26 10:04 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Eric Dumazet, Eric Dumazet, Stefano Brivio, David Ahern,
syzbot
sysbot reported that we lack appropriate rcu_read_lock()
protection in fib_dump_info_fnhe()
net/ipv4/route.c:2875 suspicious rcu_dereference_check() usage!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
1 lock held by syz-executor609/8966:
#0: 00000000b7dbe288 (rtnl_mutex){+.+.}, at: netlink_dump+0xe7/0xfb0 net/netlink/af_netlink.c:2199
stack backtrace:
CPU: 0 PID: 8966 Comm: syz-executor609 Not tainted 5.2.0-rc5+ #43
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x172/0x1f0 lib/dump_stack.c:113
lockdep_rcu_suspicious+0x153/0x15d kernel/locking/lockdep.c:5250
fib_dump_info_fnhe+0x9d9/0x1080 net/ipv4/route.c:2875
fn_trie_dump_leaf net/ipv4/fib_trie.c:2141 [inline]
fib_table_dump+0x64a/0xd00 net/ipv4/fib_trie.c:2175
inet_dump_fib+0x83c/0xa90 net/ipv4/fib_frontend.c:1004
rtnl_dump_all+0x295/0x490 net/core/rtnetlink.c:3445
netlink_dump+0x558/0xfb0 net/netlink/af_netlink.c:2244
__netlink_dump_start+0x5b1/0x7d0 net/netlink/af_netlink.c:2352
netlink_dump_start include/linux/netlink.h:226 [inline]
rtnetlink_rcv_msg+0x73d/0xb00 net/core/rtnetlink.c:5182
netlink_rcv_skb+0x177/0x450 net/netlink/af_netlink.c:2477
rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5237
netlink_unicast_kernel net/netlink/af_netlink.c:1302 [inline]
netlink_unicast+0x531/0x710 net/netlink/af_netlink.c:1328
netlink_sendmsg+0x8ae/0xd70 net/netlink/af_netlink.c:1917
sock_sendmsg_nosec net/socket.c:646 [inline]
sock_sendmsg+0xd7/0x130 net/socket.c:665
sock_write_iter+0x27c/0x3e0 net/socket.c:994
call_write_iter include/linux/fs.h:1872 [inline]
new_sync_write+0x4d3/0x770 fs/read_write.c:483
__vfs_write+0xe1/0x110 fs/read_write.c:496
vfs_write+0x20c/0x580 fs/read_write.c:558
ksys_write+0x14f/0x290 fs/read_write.c:611
__do_sys_write fs/read_write.c:623 [inline]
__se_sys_write fs/read_write.c:620 [inline]
__x64_sys_write+0x73/0xb0 fs/read_write.c:620
do_syscall_64+0xfd/0x680 arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4401b9
Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ffc8e134978 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 00000000004401b9
RDX: 000000000000001c RSI: 0000000020000000 RDI: 0000000000000003
RBP: 00000000006ca018 R08: 00000000004002c8 R09: 00000000004002c8
R10: 0000000000000010 R11: 0000000000000246 R12: 0000000000401a40
R13: 0000000000401ad0 R14: 0000000000000000 R15: 0000000000000000
Fixes: ee28906fd7a1 ("ipv4: Dump route exceptions if requested")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Stefano Brivio <sbrivio@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
---
net/ipv4/route.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 6aee412a68bdd3c24a6a0eb9883e04b7a83998e0..59670fafcd2612b94c237cbe30109adb196cf3f0 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2872,12 +2872,13 @@ int fib_dump_info_fnhe(struct sk_buff *skb, struct netlink_callback *cb,
if (nhc->nhc_flags & RTNH_F_DEAD)
continue;
+ rcu_read_lock();
bucket = rcu_dereference(nhc->nhc_exceptions);
- if (!bucket)
- continue;
-
- err = fnhe_dump_bucket(net, skb, cb, table_id, bucket, genid,
- fa_index, fa_start);
+ err = 0;
+ if (bucket)
+ err = fnhe_dump_bucket(net, skb, cb, table_id, bucket,
+ genid, fa_index, fa_start);
+ rcu_read_unlock();
if (err)
return err;
}
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* Re: KASAN: use-after-free Write in validate_chain
From: syzbot @ 2019-06-26 10:04 UTC (permalink / raw)
To: ast, daniel, john.fastabend, linux-kernel, netdev, syzkaller-bugs
In-Reply-To: <0000000000000c4e3e058bd5008d@google.com>
syzbot has bisected this bug to:
commit e9db4ef6bf4ca9894bb324c76e01b8f1a16b2650
Author: John Fastabend <john.fastabend@gmail.com>
Date: Sat Jun 30 13:17:47 2018 +0000
bpf: sockhash fix omitted bucket lock in sock_close
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1555b795a00000
start commit: abf02e29 Merge tag 'pm-5.2-rc6' of git://git.kernel.org/pu..
git tree: upstream
final crash: https://syzkaller.appspot.com/x/report.txt?x=1755b795a00000
console output: https://syzkaller.appspot.com/x/log.txt?x=1355b795a00000
kernel config: https://syzkaller.appspot.com/x/.config?x=28ec3437a5394ee0
dashboard link: https://syzkaller.appspot.com/bug?extid=55c548ad445cef6063ab
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=128ce13aa00000
Reported-by: syzbot+55c548ad445cef6063ab@syzkaller.appspotmail.com
Fixes: e9db4ef6bf4c ("bpf: sockhash fix omitted bucket lock in sock_close")
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
^ permalink raw reply
* Re: [RFC PATCH v4 net-next 06/11] net: ethernet: ti: introduce cpsw switchdev based driver part 1 - dual-emac
From: Ivan Khoronzhuk @ 2019-06-26 9:58 UTC (permalink / raw)
To: Grygorii Strashko
Cc: netdev, Ilias Apalodimas, Andrew Lunn, David S . Miller,
Jiri Pirko, Florian Fainelli, Sekhar Nori, linux-kernel,
linux-omap, Murali Karicheri, Ivan Vecera, Rob Herring,
devicetree
In-Reply-To: <20190621181314.20778-7-grygorii.strashko@ti.com>
Hi Grygorii
Too much code, but I've tried pass thru.
Probably expectation the devlink to be reviewed, but several
common replies that should be reflected in non RFC v.
On Fri, Jun 21, 2019 at 09:13:09PM +0300, Grygorii Strashko wrote:
>From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>
>Part 1:
> Introduce basic CPSW dual_mac driver (cpsw_new.c) which is operating in
>dual-emac mode by default, thus working as 2 individual network interfaces.
>Main differences from legacy CPSW driver are:
>
> - optimized promiscuous mode: The P0_UNI_FLOOD (both ports) is enabled in
>addition to ALLMULTI (current port) instead of ALE_BYPASS. So, Ports in
>promiscuous mode will keep possibility of mcast and vlan filtering, which
>is provides significant benefits when ports are joined to the same bridge,
>but without enabling "switch" mode, or to different bridges.
> - learning disabled on ports as it make not too much sense for
> segregated ports - no forwarding in HW.
> - enabled basic support for devlink.
>
> devlink dev show
> platform/48484000.ethernet_switch
>
> devlink dev param show
> platform/48484000.ethernet_switch:
> name ale_bypass type driver-specific
> values:
> cmode runtime value false
>
> - "ale_bypass" devlink driver parameter allows to enable
>ALE_CONTROL(4).BYPASS mode for debug purposes.
> - updated DT bindings.
>
>Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>---
> drivers/net/ethernet/ti/Kconfig | 19 +-
> drivers/net/ethernet/ti/Makefile | 2 +
> drivers/net/ethernet/ti/cpsw_new.c | 1555 +++++++++++++++++++++++++++
> drivers/net/ethernet/ti/cpsw_priv.c | 8 +-
> drivers/net/ethernet/ti/cpsw_priv.h | 12 +-
> 5 files changed, 1591 insertions(+), 5 deletions(-)
> create mode 100644 drivers/net/ethernet/ti/cpsw_new.c
>
[...]
>+
>+static void cpsw_rx_handler(void *token, int len, int status)
>+{
>+ struct sk_buff *skb = token;
>+ struct cpsw_common *cpsw;
>+ struct net_device *ndev;
>+ struct sk_buff *new_skb;
>+ struct cpsw_priv *priv;
>+ struct cpdma_chan *ch;
>+ int ret = 0, port;
>+
>+ ndev = skb->dev;
>+ cpsw = ndev_to_cpsw(ndev);
>+
>+ port = CPDMA_RX_SOURCE_PORT(status);
>+ if (port) {
>+ ndev = cpsw->slaves[--port].ndev;
>+ skb->dev = ndev;
>+ }
>+
>+ if (unlikely(status < 0) || unlikely(!netif_running(ndev))) {
>+ /* In dual emac mode check for all interfaces */
>+ if (cpsw->usage_count && status >= 0) {
>+ /* The packet received is for the interface which
>+ * is already down and the other interface is up
>+ * and running, instead of freeing which results
>+ * in reducing of the number of rx descriptor in
>+ * DMA engine, requeue skb back to cpdma.
>+ */
>+ new_skb = skb;
>+ goto requeue;
>+ }
>+
>+ /* the interface is going down, skbs are purged */
>+ dev_kfree_skb_any(skb);
>+ return;
>+ }
>+
>+ priv = netdev_priv(ndev);
>+
>+ new_skb = netdev_alloc_skb_ip_align(ndev, cpsw->rx_packet_max);
>+ if (new_skb) {
>+ skb_copy_queue_mapping(new_skb, skb);
>+ skb_put(skb, len);
>+ if (status & CPDMA_RX_VLAN_ENCAP)
>+ cpsw_rx_vlan_encap(skb);
>+ if (priv->rx_ts_enabled)
>+ cpts_rx_timestamp(cpsw->cpts, skb);
>+ skb->protocol = eth_type_trans(skb, ndev);
>+ netif_receive_skb(skb);
>+ ndev->stats.rx_bytes += len;
>+ ndev->stats.rx_packets++;
>+ /* CPDMA stores skb in internal CPPI RAM (SRAM) which belongs
>+ * to DEV MMIO space. Kmemleak does not scan IO memory and so
>+ * reports memory leaks.
>+ * see commit 254a49d5139a ('drivers: net: cpsw: fix kmemleak
>+ * false-positive reports for sk buffers') for details.
>+ */
>+ kmemleak_not_leak(new_skb);
>+ } else {
>+ ndev->stats.rx_dropped++;
>+ new_skb = skb;
>+ }
>+
>+requeue:
----
>+ if (netif_dormant(ndev)) {
>+ dev_kfree_skb_any(new_skb);
>+ return;
>+ }
---
drop above, no need any more
>+
>+ ch = cpsw->rxv[skb_get_queue_mapping(new_skb)].ch;
>+ ret = cpdma_chan_submit(ch, new_skb, new_skb->data,
>+ skb_tailroom(new_skb), 0);
>+ if (WARN_ON(ret < 0))
>+ dev_kfree_skb_any(new_skb);
Here were a little changes last time, looks like:
ret = cpdma_chan_submit(ch, new_skb, new_skb->data,
skb_tailroom(new_skb), 0);
if (ret < 0) {
WARN_ON(ret == -ENOMEM);
dev_kfree_skb_any(new_skb);
}
>+}
>+
[...]
>+
>+static void cpsw_init_host_port(struct cpsw_priv *priv)
>+{
>+ struct cpsw_common *cpsw = priv->cpsw;
>+ u32 control_reg;
>+
>+ /* soft reset the controller and initialize ale */
>+ soft_reset("cpsw", &cpsw->regs->soft_reset);
>+ cpsw_ale_start(cpsw->ale);
>+
>+ /* switch to vlan unaware mode */
>+ cpsw_ale_control_set(cpsw->ale, HOST_PORT_NUM, ALE_VLAN_AWARE,
>+ CPSW_ALE_VLAN_AWARE);
>+ control_reg = readl(&cpsw->regs->control);
>+ control_reg |= CPSW_VLAN_AWARE | CPSW_RX_VLAN_ENCAP;
>+ writel(control_reg, &cpsw->regs->control);
>+
>+ /* setup host port priority mapping */
>+ writel_relaxed(CPDMA_TX_PRIORITY_MAP,
>+ &cpsw->host_port_regs->cpdma_tx_pri_map);
>+ writel_relaxed(0, &cpsw->host_port_regs->cpdma_rx_chan_map);
----
>+
>+ /* disable priority elevation */
>+ writel_relaxed(0, &cpsw->regs->ptype);
>+
>+ /* enable statistics collection only on all ports */
>+ writel_relaxed(0x7, &cpsw->regs->stat_port_en);
>+
>+ /* Enable internal fifo flow control */
>+ writel(0x7, &cpsw->regs->flow_control);
---
Would be nice to do the same in old driver.
I mean moving it from ndo_open
Also were thoughts about this.
>+
>+ cpsw_init_host_port_dual_mac(priv);
>+
>+ cpsw_ale_control_set(cpsw->ale, HOST_PORT_NUM,
>+ ALE_PORT_STATE, ALE_PORT_STATE_FORWARD);
>+}
>+
[...]
>+
>+static int cpsw_ndo_open(struct net_device *ndev)
>+{
>+ struct cpsw_priv *priv = netdev_priv(ndev);
>+ struct cpsw_common *cpsw = priv->cpsw;
>+ int ret;
>+
>+ cpsw_info(priv, ifdown, "starting ndev\n");
>+ ret = pm_runtime_get_sync(cpsw->dev);
>+ if (ret < 0) {
>+ pm_runtime_put_noidle(cpsw->dev);
>+ return ret;
>+ }
>+
>+ netif_carrier_off(ndev);
>+
>+ /* Notify the stack of the actual queue counts. */
>+ ret = netif_set_real_num_tx_queues(ndev, cpsw->tx_ch_num);
>+ if (ret) {
>+ dev_err(priv->dev, "cannot set real number of tx queues\n");
>+ goto err_cleanup;
>+ }
>+
>+ ret = netif_set_real_num_rx_queues(ndev, cpsw->rx_ch_num);
>+ if (ret) {
>+ dev_err(priv->dev, "cannot set real number of rx queues\n");
>+ goto err_cleanup;
>+ }
>+
>+ /* Initialize host and slave ports */
>+ if (!cpsw->usage_count)
>+ cpsw_init_host_port(priv);
>+ cpsw_slave_open(&cpsw->slaves[priv->emac_port - 1], priv);
>+
>+ /* initialize shared resources for every ndev */
>+ if (!cpsw->usage_count) {
>+ ret = cpsw_fill_rx_channels(priv);
>+ if (ret < 0)
>+ goto err_cleanup;
>+
>+ if (cpts_register(cpsw->cpts))
>+ dev_err(priv->dev, "error registering cpts device\n");
>+
>+ napi_enable(&cpsw->napi_rx);
>+ napi_enable(&cpsw->napi_tx);
>+
>+ if (cpsw->tx_irq_disabled) {
>+ cpsw->tx_irq_disabled = false;
>+ enable_irq(cpsw->irqs_table[1]);
>+ }
>+
>+ if (cpsw->rx_irq_disabled) {
>+ cpsw->rx_irq_disabled = false;
>+ enable_irq(cpsw->irqs_table[0]);
>+ }
>+ }
>+
>+ cpsw_restore(priv);
>+
>+ /* Enable Interrupt pacing if configured */
>+ if (cpsw->coal_intvl != 0) {
>+ struct ethtool_coalesce coal;
>+
>+ coal.rx_coalesce_usecs = cpsw->coal_intvl;
>+ cpsw_set_coalesce(ndev, &coal);
>+ }
>+
>+ cpdma_ctlr_start(cpsw->dma);
>+ cpsw_intr_enable(cpsw);
>+ cpsw->usage_count++;
>+
>+ return 0;
>+
>+err_cleanup:
>+ cpdma_ctlr_stop(cpsw->dma);
Here were a little changes also:
Now looks like:
if (!cpsw->usage_count) {
cpdma_ctlr_stop(cpsw->dma)
}
>+ cpsw_slave_stop(&cpsw->slaves[priv->emac_port - 1], priv);
>+ pm_runtime_put_sync(cpsw->dev);
>+ netif_carrier_off(priv->ndev);
>+ return ret;
>+}
>+
[...]
>+
>+static netdev_tx_t cpsw_ndo_start_xmit(struct sk_buff *skb,
>+ struct net_device *ndev)
>+{
>+ struct cpsw_priv *priv = netdev_priv(ndev);
>+ struct cpsw_common *cpsw = priv->cpsw;
>+ struct cpts *cpts = cpsw->cpts;
>+ struct netdev_queue *txq;
>+ struct cpdma_chan *txch;
>+ int ret, q_idx;
>+
>+ if (skb_padto(skb, CPSW_MIN_PACKET_SIZE)) {
>+ cpsw_err(priv, tx_err, "packet pad failed\n");
>+ ndev->stats.tx_dropped++;
>+ return NET_XMIT_DROP;
>+ }
>+
>+ if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP &&
>+ priv->tx_ts_enabled && cpts_can_timestamp(cpts, skb))
>+ skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
>+
>+ q_idx = skb_get_queue_mapping(skb);
>+ if (q_idx >= cpsw->tx_ch_num)
>+ q_idx = q_idx % cpsw->tx_ch_num;
>+
>+ txch = cpsw->txv[q_idx].ch;
>+ txq = netdev_get_tx_queue(ndev, q_idx);
>+ skb_tx_timestamp(skb);
>+ ret = cpdma_chan_submit(txch, skb, skb->data, skb->len,
>+ priv->emac_port);
------------
>+ if (unlikely(ret != 0)) {
>+ cpsw_err(priv, tx_err, "desc submit failed\n");
>+ goto fail;
>+ }
>+
>+ /* If there is no more tx desc left free then we need to
>+ * tell the kernel to stop sending us tx frames.
>+ */
>+ if (unlikely(!cpdma_check_free_tx_desc(txch))) {
>+ netif_tx_stop_queue(txq);
>+
>+ /* Barrier, so that stop_queue visible to other cpus */
>+ smp_mb__after_atomic();
>+
>+ if (cpdma_check_free_tx_desc(txch))
>+ netif_tx_wake_queue(txq);
>+ }
>+
>+ return NETDEV_TX_OK;
>+fail:
>+ ndev->stats.tx_dropped++;
>+ netif_tx_stop_queue(txq);
>+
>+ /* Barrier, so that stop_queue visible to other cpus */
>+ smp_mb__after_atomic();
>+
>+ if (cpdma_check_free_tx_desc(txch))
>+ netif_tx_wake_queue(txq);
>+
>+ return NETDEV_TX_BUSY;
>+}
-------------
I have a proposition, here were no reason lastly, but now maybe
it can be optimized a little. Smth like, replace above on:
if (unlikely(ret != 0)) {
cpsw_err(priv, tx_err, "desc submit failed\n");
ndev->stats.tx_dropped++;
ret = NETDEV_TX_BUSY;
goto fail
}
ret = NETDEV_TX_OK;
/* If there is no more tx desc left free then we need to
* tell the kernel to stop sending us tx frames.
*/
if (unlikely(cpdma_check_free_tx_desc(txch)))
return ret;
fail:
netif_tx_stop_queue(txq);
/* Barrier, so that stop_queue visible to other cpus */
smp_mb__after_atomic();
if (cpdma_check_free_tx_desc(txch))
netif_tx_wake_queue(txq);
return ret;
Result: minus 6 lines and less dupes.
>+
>+
[...]
>+static int cpsw_probe(struct platform_device *pdev)
>+{
>+ const struct soc_device_attribute *soc;
>+ struct device *dev = &pdev->dev;
>+ struct resource *ss_res;
>+ struct cpsw_common *cpsw;
>+ struct gpio_descs *mode;
>+ void __iomem *ss_regs;
>+ int ret = 0, ch;
>+ struct clk *clk;
>+ int irq;
>+
>+ cpsw = devm_kzalloc(dev, sizeof(struct cpsw_common), GFP_KERNEL);
>+ if (!cpsw)
>+ return -ENOMEM;
>+
>+ cpsw_slave_index = cpsw_slave_index_priv;
>+
>+ cpsw->dev = dev;
>+
>+ cpsw->slaves = devm_kcalloc(dev,
>+ CPSW_SLAVE_PORTS_NUM,
>+ sizeof(struct cpsw_slave),
>+ GFP_KERNEL);
>+ if (!cpsw->slaves)
>+ return -ENOMEM;
>+
>+ mode = devm_gpiod_get_array_optional(dev, "mode", GPIOD_OUT_LOW);
>+ if (IS_ERR(mode)) {
>+ ret = PTR_ERR(mode);
>+ dev_err(dev, "gpio request failed, ret %d\n", ret);
>+ return ret;
>+ }
>+
>+ clk = devm_clk_get(dev, "fck");
>+ if (IS_ERR(clk)) {
>+ ret = PTR_ERR(clk);
>+ dev_err(dev, "fck is not found %d\n", ret);
>+ return ret;
>+ }
>+ cpsw->bus_freq_mhz = clk_get_rate(clk) / 1000000;
>+
>+ ss_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>+ ss_regs = devm_ioremap_resource(dev, ss_res);
>+ if (IS_ERR(ss_regs)) {
>+ ret = PTR_ERR(ss_regs);
>+ return ret;
>+ }
>+ cpsw->regs = ss_regs;
>+
>+ irq = platform_get_irq_byname(pdev, "rx");
>+ if (irq < 0)
>+ return irq;
>+ cpsw->irqs_table[0] = irq;
>+
>+ irq = platform_get_irq_byname(pdev, "tx");
>+ if (irq < 0)
>+ return irq;
>+ cpsw->irqs_table[1] = irq;
>+
>+ platform_set_drvdata(pdev, cpsw);
set cpsw, but below ndev
>+ /* This may be required here for child devices. */
>+ pm_runtime_enable(dev);
>+
>+ /* Need to enable clocks with runtime PM api to access module
>+ * registers
>+ */
>+ ret = pm_runtime_get_sync(dev);
>+ if (ret < 0) {
>+ pm_runtime_put_noidle(dev);
>+ pm_runtime_disable(dev);
>+ return ret;
>+ }
>+
>+ ret = cpsw_probe_dt(cpsw);
>+ if (ret)
>+ goto clean_dt_ret;
>+
>+ soc = soc_device_match(cpsw_soc_devices);
>+ if (soc)
>+ cpsw->quirk_irq = 1;
>+
>+ cpsw->rx_packet_max = rx_packet_max;
>+ cpsw->descs_pool_size = descs_pool_size;
>+
>+ ret = cpsw_init_common(cpsw, ss_regs, ale_ageout,
>+ (u32 __force)ss_res->start + CPSW2_BD_OFFSET,
>+ descs_pool_size);
>+ if (ret)
>+ goto clean_dt_ret;
>+
>+ cpsw->wr_regs = cpsw->version == CPSW_VERSION_1 ?
>+ ss_regs + CPSW1_WR_OFFSET :
>+ ss_regs + CPSW2_WR_OFFSET;
>+
>+ ch = cpsw->quirk_irq ? 0 : 7;
>+ cpsw->txv[0].ch = cpdma_chan_create(cpsw->dma, ch, cpsw_tx_handler, 0);
>+ if (IS_ERR(cpsw->txv[0].ch)) {
>+ dev_err(dev, "error initializing tx dma channel\n");
>+ ret = PTR_ERR(cpsw->txv[0].ch);
>+ goto clean_cpts;
>+ }
>+
>+ cpsw->rxv[0].ch = cpdma_chan_create(cpsw->dma, 0, cpsw_rx_handler, 1);
>+ if (IS_ERR(cpsw->rxv[0].ch)) {
>+ dev_err(dev, "error initializing rx dma channel\n");
>+ ret = PTR_ERR(cpsw->rxv[0].ch);
>+ goto clean_cpts;
>+ }
>+ cpsw_split_res(cpsw);
>+
>+ /* setup netdevs */
>+ ret = cpsw_create_ports(cpsw);
>+ if (ret)
>+ goto clean_unregister_netdev;
>+
>+ /* Grab RX and TX IRQs. Note that we also have RX_THRESHOLD and
>+ * MISC IRQs which are always kept disabled with this driver so
>+ * we will not request them.
>+ *
>+ * If anyone wants to implement support for those, make sure to
>+ * first request and append them to irqs_table array.
>+ */
>+
>+ ret = devm_request_irq(dev, cpsw->irqs_table[0], cpsw_rx_interrupt,
>+ 0, dev_name(dev), cpsw);
>+ if (ret < 0) {
>+ dev_err(dev, "error attaching irq (%d)\n", ret);
>+ goto clean_unregister_netdev;
>+ }
>+
>+ ret = devm_request_irq(dev, cpsw->irqs_table[1], cpsw_tx_interrupt,
>+ 0, dev_name(dev), cpsw);
>+ if (ret < 0) {
>+ dev_err(dev, "error attaching irq (%d)\n", ret);
>+ goto clean_unregister_netdev;
>+ }
>+
>+ ret = cpsw_register_devlink(cpsw);
>+ if (ret)
>+ goto clean_unregister_netdev;
>+
>+ dev_notice(dev, "initialized (regs %pa, pool size %d) hw_ver:%08X %d.%d (%d)\n",
>+ &ss_res->start, descs_pool_size,
>+ cpsw->version, CPSW_MAJOR_VERSION(cpsw->version),
>+ CPSW_MINOR_VERSION(cpsw->version),
>+ CPSW_RTL_VERSION(cpsw->version));
>+
>+ pm_runtime_put(dev);
>+
>+ return 0;
>+
>+clean_unregister_netdev:
>+ cpsw_unregister_ports(cpsw);
>+clean_cpts:
>+ cpts_release(cpsw->cpts);
>+ cpdma_ctlr_destroy(cpsw->dma);
>+clean_dt_ret:
>+ cpsw_remove_dt(cpsw);
>+ pm_runtime_put_sync(dev);
>+ pm_runtime_disable(dev);
>+ return ret;
>+}
>+
>+static int cpsw_remove(struct platform_device *pdev)
>+{
>+ struct net_device *ndev = platform_get_drvdata(pdev);
now it's cpsw, as in probe?
>+ struct cpsw_common *cpsw = ndev_to_cpsw(ndev);
>+ int ret;
>+
>+ ret = pm_runtime_get_sync(&pdev->dev);
>+ if (ret < 0) {
>+ pm_runtime_put_noidle(&pdev->dev);
>+ return ret;
>+ }
>+
>+ cpsw_unregister_devlink(cpsw);
>+ cpsw_unregister_ports(cpsw);
>+
>+ cpts_release(cpsw->cpts);
>+ cpdma_ctlr_destroy(cpsw->dma);
>+ cpsw_remove_dt(cpsw);
>+ pm_runtime_put_sync(&pdev->dev);
>+ pm_runtime_disable(&pdev->dev);
>+ return 0;
>+}
>+
[...]
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply
* RE: [EXT] [PATCH net-next 05/16] qlge: Remove rx_ring.sbq_buf_size
From: Manish Chopra @ 2019-06-26 9:36 UTC (permalink / raw)
To: Benjamin Poirier, GR-Linux-NIC-Dev, netdev@vger.kernel.org
In-Reply-To: <20190617074858.32467-5-bpoirier@suse.com>
> -----Original Message-----
> From: Benjamin Poirier <bpoirier@suse.com>
> Sent: Monday, June 17, 2019 1:19 PM
> To: Manish Chopra <manishc@marvell.com>; GR-Linux-NIC-Dev <GR-Linux-
> NIC-Dev@marvell.com>; netdev@vger.kernel.org
> Subject: [EXT] [PATCH net-next 05/16] qlge: Remove rx_ring.sbq_buf_size
>
> External Email
>
> ----------------------------------------------------------------------
> Tx rings have sbq_buf_size = 0 but there's no case where the code actually
> tests on that value. We can remove sbq_buf_size and use a constant instead.
>
Seems relevant to RX ring, not the TX ring ?
^ permalink raw reply
* RE: [PATCH net-next 04/16] qlge: Remove bq_desc.maplen
From: Manish Chopra @ 2019-06-26 9:31 UTC (permalink / raw)
To: Benjamin Poirier, GR-Linux-NIC-Dev, netdev@vger.kernel.org
In-Reply-To: <20190617074858.32467-4-bpoirier@suse.com>
> -----Original Message-----
> From: Benjamin Poirier <bpoirier@suse.com>
> Sent: Monday, June 17, 2019 1:19 PM
> To: Manish Chopra <manishc@marvell.com>; GR-Linux-NIC-Dev <GR-Linux-
> NIC-Dev@marvell.com>; netdev@vger.kernel.org
> Subject: [PATCH net-next 04/16] qlge: Remove bq_desc.maplen
>
> The size of the mapping is known statically in all cases, there's no need to save
> it at runtime. Remove this member.
>
> Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
> ---
> drivers/net/ethernet/qlogic/qlge/qlge.h | 1 -
> drivers/net/ethernet/qlogic/qlge/qlge_main.c | 43 +++++++-------------
> 2 files changed, 15 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/net/ethernet/qlogic/qlge/qlge.h
> b/drivers/net/ethernet/qlogic/qlge/qlge.h
> index ba61b4559dd6..f32da8c7679f 100644
> --- a/drivers/net/ethernet/qlogic/qlge/qlge.h
> +++ b/drivers/net/ethernet/qlogic/qlge/qlge.h
> @@ -1373,7 +1373,6 @@ struct bq_desc {
> __le64 *addr;
> u32 index;
> DEFINE_DMA_UNMAP_ADDR(mapaddr);
> - DEFINE_DMA_UNMAP_LEN(maplen);
> };
>
> #define QL_TXQ_IDX(qdev, skb) (smp_processor_id()%(qdev->tx_ring_count))
> diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> index 9df06ad3fb93..25dbaa9cc55d 100644
> --- a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> +++ b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> @@ -1108,8 +1108,6 @@ static void ql_update_lbq(struct ql_adapter *qdev,
> struct rx_ring *rx_ring)
> map = lbq_desc->p.pg_chunk.map +
> lbq_desc->p.pg_chunk.offset;
> dma_unmap_addr_set(lbq_desc, mapaddr, map);
> - dma_unmap_len_set(lbq_desc, maplen,
> - qdev->lbq_buf_size);
> *lbq_desc->addr = cpu_to_le64(map);
>
> pci_dma_sync_single_for_device(qdev->pdev, map,
> @@ -1177,8 +1175,6 @@ static void ql_update_sbq(struct ql_adapter *qdev,
> struct rx_ring *rx_ring)
> return;
> }
> dma_unmap_addr_set(sbq_desc, mapaddr,
> map);
> - dma_unmap_len_set(sbq_desc, maplen,
> - rx_ring->sbq_buf_size);
> *sbq_desc->addr = cpu_to_le64(map);
> }
>
> @@ -1598,14 +1594,14 @@ static void ql_process_mac_rx_skb(struct
> ql_adapter *qdev,
>
> pci_dma_sync_single_for_cpu(qdev->pdev,
> dma_unmap_addr(sbq_desc, mapaddr),
> - dma_unmap_len(sbq_desc, maplen),
> + rx_ring->sbq_buf_size,
> PCI_DMA_FROMDEVICE);
>
> skb_put_data(new_skb, skb->data, length);
>
> pci_dma_sync_single_for_device(qdev->pdev,
> dma_unmap_addr(sbq_desc, mapaddr),
> - dma_unmap_len(sbq_desc, maplen),
> + rx_ring->sbq_buf_size,
> PCI_DMA_FROMDEVICE);
> skb = new_skb;
>
> @@ -1727,8 +1723,7 @@ static struct sk_buff *ql_build_rx_skb(struct
> ql_adapter *qdev,
> sbq_desc = ql_get_curr_sbuf(rx_ring);
> pci_unmap_single(qdev->pdev,
> dma_unmap_addr(sbq_desc, mapaddr),
> - dma_unmap_len(sbq_desc, maplen),
> - PCI_DMA_FROMDEVICE);
> + rx_ring->sbq_buf_size,
> PCI_DMA_FROMDEVICE);
> skb = sbq_desc->p.skb;
> ql_realign_skb(skb, hdr_len);
> skb_put(skb, hdr_len);
> @@ -1758,19 +1753,15 @@ static struct sk_buff *ql_build_rx_skb(struct
> ql_adapter *qdev,
> */
> sbq_desc = ql_get_curr_sbuf(rx_ring);
> pci_dma_sync_single_for_cpu(qdev->pdev,
> - dma_unmap_addr
> - (sbq_desc, mapaddr),
> - dma_unmap_len
> - (sbq_desc, maplen),
> +
> dma_unmap_addr(sbq_desc,
> + mapaddr),
> + rx_ring->sbq_buf_size,
> PCI_DMA_FROMDEVICE);
> skb_put_data(skb, sbq_desc->p.skb->data, length);
> pci_dma_sync_single_for_device(qdev->pdev,
> - dma_unmap_addr
> - (sbq_desc,
> - mapaddr),
> - dma_unmap_len
> - (sbq_desc,
> - maplen),
> +
> dma_unmap_addr(sbq_desc,
> + mapaddr),
> + rx_ring->sbq_buf_size,
> PCI_DMA_FROMDEVICE);
> } else {
> netif_printk(qdev, rx_status, KERN_DEBUG, qdev-
> >ndev, @@ -1781,10 +1772,8 @@ static struct sk_buff *ql_build_rx_skb(struct
> ql_adapter *qdev,
> ql_realign_skb(skb, length);
> skb_put(skb, length);
> pci_unmap_single(qdev->pdev,
> - dma_unmap_addr(sbq_desc,
> - mapaddr),
> - dma_unmap_len(sbq_desc,
> - maplen),
> + dma_unmap_addr(sbq_desc,
> mapaddr),
> + rx_ring->sbq_buf_size,
> PCI_DMA_FROMDEVICE);
> sbq_desc->p.skb = NULL;
> }
> @@ -1822,9 +1811,8 @@ static struct sk_buff *ql_build_rx_skb(struct
> ql_adapter *qdev,
> return NULL;
> }
> pci_unmap_page(qdev->pdev,
> - dma_unmap_addr(lbq_desc,
> - mapaddr),
> - dma_unmap_len(lbq_desc, maplen),
> + dma_unmap_addr(lbq_desc, mapaddr),
> + qdev->lbq_buf_size,
> PCI_DMA_FROMDEVICE);
> skb_reserve(skb, NET_IP_ALIGN);
> netif_printk(qdev, rx_status, KERN_DEBUG, qdev-
> >ndev, @@ -1858,8 +1846,7 @@ static struct sk_buff *ql_build_rx_skb(struct
> ql_adapter *qdev,
> sbq_desc = ql_get_curr_sbuf(rx_ring);
> pci_unmap_single(qdev->pdev,
> dma_unmap_addr(sbq_desc, mapaddr),
> - dma_unmap_len(sbq_desc, maplen),
> - PCI_DMA_FROMDEVICE);
> + rx_ring->sbq_buf_size,
> PCI_DMA_FROMDEVICE);
> if (!(ib_mac_rsp->flags4 & IB_MAC_IOCB_RSP_HS)) {
> /*
> * This is an non TCP/UDP IP frame, so @@ -2820,7
> +2807,7 @@ static void ql_free_sbq_buffers(struct ql_adapter *qdev, struct
> rx_ring *rx_ring
> if (sbq_desc->p.skb) {
> pci_unmap_single(qdev->pdev,
> dma_unmap_addr(sbq_desc,
> mapaddr),
> - dma_unmap_len(sbq_desc, maplen),
> + rx_ring->sbq_buf_size,
> PCI_DMA_FROMDEVICE);
> dev_kfree_skb(sbq_desc->p.skb);
> sbq_desc->p.skb = NULL;
> --
> 2.21.0
Acked-by: Manish Chopra <manishc@marvell.com>
^ permalink raw reply
* Re: [PATCH net-next] can: dev: call netif_carrier_off() in register_candev()
From: Rasmus Villemoes @ 2019-06-26 9:31 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
Rasmus Villemoes, linux-can@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CA+FuTSeHhz1kntLyeUfAB4ZbtYjO1=Ornwse-yQbPwo5c-_2=g@mail.gmail.com>
On 24/06/2019 19.26, Willem de Bruijn wrote:
> On Mon, Jun 24, 2019 at 4:34 AM Rasmus Villemoes
> <rasmus.villemoes@prevas.dk> wrote:
>>
>> CONFIG_CAN_LEDS is deprecated. When trying to use the generic netdev
>> trigger as suggested, there's a small inconsistency with the link
>> property: The LED is on initially, stays on when the device is brought
>> up, and then turns off (as expected) when the device is brought down.
>>
>> Make sure the LED always reflects the state of the CAN device.
>>
>> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
>
> Should this target net?
No, I think this should go through the CAN tree. Perhaps I've
misunderstood when to use the net-next prefix - is that only for things
that should be applied directly to the net-next tree? If so, sorry.
> Regardless of CONFIG_CAN_LEDS deprecation,
> this is already not initialized properly if that CONFIG is disabled
> and a can_led_event call at device probe is a noop.
I'm not sure I understand this part. The CONFIG_CAN_LEDS support for
showing the state of the interface is implemented via hooking into the
ndo_open/ndo_stop callbacks, and does not look at or touch the
__LINK_STATE_NOCARRIER bit at all.
Other than via the netdev LED trigger I don't think one can even observe
the slightly odd initial state of the __LINK_STATE_NOCARRIER bit for CAN
devices, which is why I framed this as a fix purely to allow the netdev
trigger to be a closer drop-in replacement for CONFIG_CAN_LEDS.
Rasmus
^ permalink raw reply
* RE: [EXT] [PATCH net-next 03/16] qlge: Deduplicate lbq_buf_size
From: Manish Chopra @ 2019-06-26 9:24 UTC (permalink / raw)
To: Benjamin Poirier, GR-Linux-NIC-Dev, netdev@vger.kernel.org
In-Reply-To: <20190617074858.32467-3-bpoirier@suse.com>
> -----Original Message-----
> From: Benjamin Poirier <bpoirier@suse.com>
> Sent: Monday, June 17, 2019 1:19 PM
> To: Manish Chopra <manishc@marvell.com>; GR-Linux-NIC-Dev <GR-Linux-
> NIC-Dev@marvell.com>; netdev@vger.kernel.org
> Subject: [EXT] [PATCH net-next 03/16] qlge: Deduplicate lbq_buf_size
>
> External Email
>
> ----------------------------------------------------------------------
> lbq_buf_size is duplicated to every rx_ring structure whereas lbq_buf_order is
> present once in the ql_adapter structure. All rings use the same buf size, keep
> only one copy of it. Also factor out the calculation of lbq_buf_size instead of
> having two copies.
>
> Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
> ---
> drivers/net/ethernet/qlogic/qlge/qlge.h | 2 +-
> drivers/net/ethernet/qlogic/qlge/qlge_dbg.c | 2 +-
> drivers/net/ethernet/qlogic/qlge/qlge_main.c | 61 +++++++++-----------
> 3 files changed, 28 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/net/ethernet/qlogic/qlge/qlge.h
> b/drivers/net/ethernet/qlogic/qlge/qlge.h
> index 0a156a95e981..ba61b4559dd6 100644
> --- a/drivers/net/ethernet/qlogic/qlge/qlge.h
> +++ b/drivers/net/ethernet/qlogic/qlge/qlge.h
> @@ -1433,7 +1433,6 @@ struct rx_ring {
> /* Large buffer queue elements. */
> u32 lbq_len; /* entry count */
> u32 lbq_size; /* size in bytes of queue */
> - u32 lbq_buf_size;
> void *lbq_base;
> dma_addr_t lbq_base_dma;
> void *lbq_base_indirect;
> @@ -2108,6 +2107,7 @@ struct ql_adapter {
> struct rx_ring rx_ring[MAX_RX_RINGS];
> struct tx_ring tx_ring[MAX_TX_RINGS];
> unsigned int lbq_buf_order;
> + u32 lbq_buf_size;
>
> int rx_csum;
> u32 default_rx_queue;
> diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_dbg.c
> b/drivers/net/ethernet/qlogic/qlge/qlge_dbg.c
> index 31389ab8bdf7..46599d74c6fb 100644
> --- a/drivers/net/ethernet/qlogic/qlge/qlge_dbg.c
> +++ b/drivers/net/ethernet/qlogic/qlge/qlge_dbg.c
> @@ -1630,6 +1630,7 @@ void ql_dump_qdev(struct ql_adapter *qdev)
> DUMP_QDEV_FIELD(qdev, "0x%08x", xg_sem_mask);
> DUMP_QDEV_FIELD(qdev, "0x%08x", port_link_up);
> DUMP_QDEV_FIELD(qdev, "0x%08x", port_init);
> + DUMP_QDEV_FIELD(qdev, "%u", lbq_buf_size);
> }
> #endif
>
> @@ -1774,7 +1775,6 @@ void ql_dump_rx_ring(struct rx_ring *rx_ring)
> pr_err("rx_ring->lbq_curr_idx = %d\n", rx_ring->lbq_curr_idx);
> pr_err("rx_ring->lbq_clean_idx = %d\n", rx_ring->lbq_clean_idx);
> pr_err("rx_ring->lbq_free_cnt = %d\n", rx_ring->lbq_free_cnt);
> - pr_err("rx_ring->lbq_buf_size = %d\n", rx_ring->lbq_buf_size);
>
> pr_err("rx_ring->sbq_base = %p\n", rx_ring->sbq_base);
> pr_err("rx_ring->sbq_base_dma = %llx\n", diff --git
> a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> index 038a6bfc79c7..9df06ad3fb93 100644
> --- a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> +++ b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> @@ -995,15 +995,14 @@ static struct bq_desc *ql_get_curr_lchunk(struct
> ql_adapter *qdev,
> struct bq_desc *lbq_desc = ql_get_curr_lbuf(rx_ring);
>
> pci_dma_sync_single_for_cpu(qdev->pdev,
> - dma_unmap_addr(lbq_desc,
> mapaddr),
> - rx_ring->lbq_buf_size,
> - PCI_DMA_FROMDEVICE);
> + dma_unmap_addr(lbq_desc, mapaddr),
> + qdev->lbq_buf_size,
> PCI_DMA_FROMDEVICE);
>
> /* If it's the last chunk of our master page then
> * we unmap it.
> */
> - if ((lbq_desc->p.pg_chunk.offset + rx_ring->lbq_buf_size)
> - == ql_lbq_block_size(qdev))
> + if (lbq_desc->p.pg_chunk.offset + qdev->lbq_buf_size ==
> + ql_lbq_block_size(qdev))
> pci_unmap_page(qdev->pdev,
> lbq_desc->p.pg_chunk.map,
> ql_lbq_block_size(qdev),
> @@ -1074,11 +1073,11 @@ static int ql_get_next_chunk(struct ql_adapter
> *qdev, struct rx_ring *rx_ring,
> /* Adjust the master page chunk for next
> * buffer get.
> */
> - rx_ring->pg_chunk.offset += rx_ring->lbq_buf_size;
> + rx_ring->pg_chunk.offset += qdev->lbq_buf_size;
> if (rx_ring->pg_chunk.offset == ql_lbq_block_size(qdev)) {
> rx_ring->pg_chunk.page = NULL;
> } else {
> - rx_ring->pg_chunk.va += rx_ring->lbq_buf_size;
> + rx_ring->pg_chunk.va += qdev->lbq_buf_size;
> get_page(rx_ring->pg_chunk.page);
> }
> return 0;
> @@ -1110,12 +1109,12 @@ static void ql_update_lbq(struct ql_adapter
> *qdev, struct rx_ring *rx_ring)
> lbq_desc->p.pg_chunk.offset;
> dma_unmap_addr_set(lbq_desc, mapaddr, map);
> dma_unmap_len_set(lbq_desc, maplen,
> - rx_ring->lbq_buf_size);
> + qdev->lbq_buf_size);
> *lbq_desc->addr = cpu_to_le64(map);
>
> pci_dma_sync_single_for_device(qdev->pdev, map,
> - rx_ring->lbq_buf_size,
> - PCI_DMA_FROMDEVICE);
> + qdev->lbq_buf_size,
> + PCI_DMA_FROMDEVICE);
> clean_idx++;
> if (clean_idx == rx_ring->lbq_len)
> clean_idx = 0;
> @@ -1880,8 +1879,7 @@ static struct sk_buff *ql_build_rx_skb(struct
> ql_adapter *qdev,
> }
> do {
> lbq_desc = ql_get_curr_lchunk(qdev, rx_ring);
> - size = (length < rx_ring->lbq_buf_size) ? length :
> - rx_ring->lbq_buf_size;
> + size = min(length, qdev->lbq_buf_size);
>
> netif_printk(qdev, rx_status, KERN_DEBUG, qdev-
> >ndev,
> "Adding page %d to skb for %d bytes.\n",
> @@ -2776,12 +2774,12 @@ static int ql_alloc_tx_resources(struct ql_adapter
> *qdev,
>
> static void ql_free_lbq_buffers(struct ql_adapter *qdev, struct rx_ring
> *rx_ring) {
> - unsigned int last_offset = ql_lbq_block_size(qdev) -
> - rx_ring->lbq_buf_size;
> + unsigned int last_offset;
> struct bq_desc *lbq_desc;
>
> uint32_t curr_idx, clean_idx;
>
> + last_offset = ql_lbq_block_size(qdev) - qdev->lbq_buf_size;
> curr_idx = rx_ring->lbq_curr_idx;
> clean_idx = rx_ring->lbq_clean_idx;
> while (curr_idx != clean_idx) {
> @@ -3149,8 +3147,8 @@ static int ql_start_rx_ring(struct ql_adapter *qdev,
> struct rx_ring *rx_ring)
> } while (page_entries < MAX_DB_PAGES_PER_BQ(rx_ring-
> >lbq_len));
> cqicb->lbq_addr =
> cpu_to_le64(rx_ring->lbq_base_indirect_dma);
> - bq_len = (rx_ring->lbq_buf_size == 65536) ? 0 :
> - (u16) rx_ring->lbq_buf_size;
> + bq_len = (qdev->lbq_buf_size == 65536) ? 0 :
> + (u16)qdev->lbq_buf_size;
> cqicb->lbq_buf_size = cpu_to_le16(bq_len);
> bq_len = (rx_ring->lbq_len == 65536) ? 0 :
> (u16) rx_ring->lbq_len;
> @@ -4048,16 +4046,21 @@ static int qlge_close(struct net_device *ndev)
> return 0;
> }
>
> +static void qlge_set_lb_size(struct ql_adapter *qdev) {
> + if (qdev->ndev->mtu <= 1500)
> + qdev->lbq_buf_size = LARGE_BUFFER_MIN_SIZE;
> + else
> + qdev->lbq_buf_size = LARGE_BUFFER_MAX_SIZE;
> + qdev->lbq_buf_order = get_order(qdev->lbq_buf_size); }
> +
> static int ql_configure_rings(struct ql_adapter *qdev) {
> int i;
> struct rx_ring *rx_ring;
> struct tx_ring *tx_ring;
> int cpu_cnt = min(MAX_CPUS, (int)num_online_cpus());
> - unsigned int lbq_buf_len = (qdev->ndev->mtu > 1500) ?
> - LARGE_BUFFER_MAX_SIZE : LARGE_BUFFER_MIN_SIZE;
> -
> - qdev->lbq_buf_order = get_order(lbq_buf_len);
>
> /* In a perfect world we have one RSS ring for each CPU
> * and each has it's own vector. To do that we ask for @@ -4105,7
> +4108,6 @@ static int ql_configure_rings(struct ql_adapter *qdev)
> rx_ring->lbq_len = NUM_LARGE_BUFFERS;
> rx_ring->lbq_size =
> rx_ring->lbq_len * sizeof(__le64);
> - rx_ring->lbq_buf_size = (u16)lbq_buf_len;
> rx_ring->sbq_len = NUM_SMALL_BUFFERS;
> rx_ring->sbq_size =
> rx_ring->sbq_len * sizeof(__le64); @@ -4121,7
> +4123,6 @@ static int ql_configure_rings(struct ql_adapter *qdev)
> rx_ring->cq_len * sizeof(struct ql_net_rsp_iocb);
> rx_ring->lbq_len = 0;
> rx_ring->lbq_size = 0;
> - rx_ring->lbq_buf_size = 0;
> rx_ring->sbq_len = 0;
> rx_ring->sbq_size = 0;
> rx_ring->sbq_buf_size = 0;
> @@ -4140,6 +4141,7 @@ static int qlge_open(struct net_device *ndev)
> if (err)
> return err;
>
> + qlge_set_lb_size(qdev);
> err = ql_configure_rings(qdev);
> if (err)
> return err;
> @@ -4161,9 +4163,7 @@ static int qlge_open(struct net_device *ndev)
>
> static int ql_change_rx_buffers(struct ql_adapter *qdev) {
> - struct rx_ring *rx_ring;
> - int i, status;
> - u32 lbq_buf_len;
> + int status;
>
> /* Wait for an outstanding reset to complete. */
> if (!test_bit(QL_ADAPTER_UP, &qdev->flags)) { @@ -4186,16 +4186,7
> @@ static int ql_change_rx_buffers(struct ql_adapter *qdev)
> if (status)
> goto error;
>
> - /* Get the new rx buffer size. */
> - lbq_buf_len = (qdev->ndev->mtu > 1500) ?
> - LARGE_BUFFER_MAX_SIZE : LARGE_BUFFER_MIN_SIZE;
> - qdev->lbq_buf_order = get_order(lbq_buf_len);
> -
> - for (i = 0; i < qdev->rss_ring_count; i++) {
> - rx_ring = &qdev->rx_ring[i];
> - /* Set the new size. */
> - rx_ring->lbq_buf_size = lbq_buf_len;
> - }
> + qlge_set_lb_size(qdev);
>
> status = ql_adapter_up(qdev);
> if (status)
> --
> 2.21.0
Not sure if this change is really required, I think fields relevant to rx_ring should be present in the rx_ring structure.
There are various other fields like "lbq_len" and "lbq_size" which would be same for all rx rings but still under the relevant rx_ring structure.
^ 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