Netdev List
 help / color / mirror / Atom feed
* [PATCH v3 bpf-next 9/9] selftests/bpf: convert existing tracepoint tests to new APIs
From: Andrii Nakryiko @ 2019-06-28  5:53 UTC (permalink / raw)
  To: andrii.nakryiko, ast, daniel, netdev, bpf, sdf, kernel-team
  Cc: Andrii Nakryiko
In-Reply-To: <20190628055303.1249758-1-andriin@fb.com>

Convert some existing tests that attach to tracepoints to use
bpf_program__attach_tracepoint API instead.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Reviewed-by: Stanislav Fomichev <sdf@google.com>
---
 .../bpf/prog_tests/stacktrace_build_id.c      | 50 ++++---------------
 .../selftests/bpf/prog_tests/stacktrace_map.c | 43 ++++------------
 .../bpf/prog_tests/stacktrace_map_raw_tp.c    | 15 ++++--
 3 files changed, 31 insertions(+), 77 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c
index 3aab2b083c71..768883d838ea 100644
--- a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c
+++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c
@@ -4,11 +4,13 @@
 void test_stacktrace_build_id(void)
 {
 	int control_map_fd, stackid_hmap_fd, stackmap_fd, stack_amap_fd;
+	const char *prog_name = "tracepoint/random/urandom_read";
 	const char *file = "./test_stacktrace_build_id.o";
-	int bytes, efd, err, pmu_fd, prog_fd, stack_trace_len;
-	struct perf_event_attr attr = {};
+	int err, prog_fd, stack_trace_len;
 	__u32 key, previous_key, val, duration = 0;
+	struct bpf_program *prog;
 	struct bpf_object *obj;
+	struct bpf_link *link = NULL;
 	char buf[256];
 	int i, j;
 	struct bpf_stack_build_id id_offs[PERF_MAX_STACK_DEPTH];
@@ -20,42 +22,14 @@ void test_stacktrace_build_id(void)
 	if (CHECK(err, "prog_load", "err %d errno %d\n", err, errno))
 		goto out;
 
-	/* Get the ID for the sched/sched_switch tracepoint */
-	snprintf(buf, sizeof(buf),
-		 "/sys/kernel/debug/tracing/events/random/urandom_read/id");
-	efd = open(buf, O_RDONLY, 0);
-	if (CHECK(efd < 0, "open", "err %d errno %d\n", efd, errno))
+	prog = bpf_object__find_program_by_title(obj, prog_name);
+	if (CHECK(!prog, "find_prog", "prog '%s' not found\n", prog_name))
 		goto close_prog;
 
-	bytes = read(efd, buf, sizeof(buf));
-	close(efd);
-	if (CHECK(bytes <= 0 || bytes >= sizeof(buf),
-		  "read", "bytes %d errno %d\n", bytes, errno))
+	link = bpf_program__attach_tracepoint(prog, "random", "urandom_read");
+	if (CHECK(IS_ERR(link), "attach_tp", "err %ld\n", PTR_ERR(link)))
 		goto close_prog;
 
-	/* Open the perf event and attach bpf progrram */
-	attr.config = strtol(buf, NULL, 0);
-	attr.type = PERF_TYPE_TRACEPOINT;
-	attr.sample_type = PERF_SAMPLE_RAW | PERF_SAMPLE_CALLCHAIN;
-	attr.sample_period = 1;
-	attr.wakeup_events = 1;
-	pmu_fd = syscall(__NR_perf_event_open, &attr, -1 /* pid */,
-			 0 /* cpu 0 */, -1 /* group id */,
-			 0 /* flags */);
-	if (CHECK(pmu_fd < 0, "perf_event_open", "err %d errno %d\n",
-		  pmu_fd, errno))
-		goto close_prog;
-
-	err = ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0);
-	if (CHECK(err, "perf_event_ioc_enable", "err %d errno %d\n",
-		  err, errno))
-		goto close_pmu;
-
-	err = ioctl(pmu_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
-	if (CHECK(err, "perf_event_ioc_set_bpf", "err %d errno %d\n",
-		  err, errno))
-		goto disable_pmu;
-
 	/* find map fds */
 	control_map_fd = bpf_find_map(__func__, obj, "control_map");
 	if (CHECK(control_map_fd < 0, "bpf_find_map control_map",
@@ -133,8 +107,7 @@ void test_stacktrace_build_id(void)
 	 * try it one more time.
 	 */
 	if (build_id_matches < 1 && retry--) {
-		ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE);
-		close(pmu_fd);
+		bpf_link__destroy(link);
 		bpf_object__close(obj);
 		printf("%s:WARN:Didn't find expected build ID from the map, retrying\n",
 		       __func__);
@@ -152,10 +125,7 @@ void test_stacktrace_build_id(void)
 	      "err %d errno %d\n", err, errno);
 
 disable_pmu:
-	ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE);
-
-close_pmu:
-	close(pmu_fd);
+	bpf_link__destroy(link);
 
 close_prog:
 	bpf_object__close(obj);
diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c
index 2bfd50a0d6d1..fc539335c5b3 100644
--- a/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c
+++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c
@@ -4,50 +4,26 @@
 void test_stacktrace_map(void)
 {
 	int control_map_fd, stackid_hmap_fd, stackmap_fd, stack_amap_fd;
+	const char *prog_name = "tracepoint/sched/sched_switch";
+	int err, prog_fd, stack_trace_len;
 	const char *file = "./test_stacktrace_map.o";
-	int bytes, efd, err, pmu_fd, prog_fd, stack_trace_len;
-	struct perf_event_attr attr = {};
 	__u32 key, val, duration = 0;
+	struct bpf_program *prog;
 	struct bpf_object *obj;
-	char buf[256];
+	struct bpf_link *link;
 
 	err = bpf_prog_load(file, BPF_PROG_TYPE_TRACEPOINT, &obj, &prog_fd);
 	if (CHECK(err, "prog_load", "err %d errno %d\n", err, errno))
 		return;
 
-	/* Get the ID for the sched/sched_switch tracepoint */
-	snprintf(buf, sizeof(buf),
-		 "/sys/kernel/debug/tracing/events/sched/sched_switch/id");
-	efd = open(buf, O_RDONLY, 0);
-	if (CHECK(efd < 0, "open", "err %d errno %d\n", efd, errno))
+	prog = bpf_object__find_program_by_title(obj, prog_name);
+	if (CHECK(!prog, "find_prog", "prog '%s' not found\n", prog_name))
 		goto close_prog;
 
-	bytes = read(efd, buf, sizeof(buf));
-	close(efd);
-	if (bytes <= 0 || bytes >= sizeof(buf))
+	link = bpf_program__attach_tracepoint(prog, "sched", "sched_switch");
+	if (CHECK(IS_ERR(link), "attach_tp", "err %ld\n", PTR_ERR(link)))
 		goto close_prog;
 
-	/* Open the perf event and attach bpf progrram */
-	attr.config = strtol(buf, NULL, 0);
-	attr.type = PERF_TYPE_TRACEPOINT;
-	attr.sample_type = PERF_SAMPLE_RAW | PERF_SAMPLE_CALLCHAIN;
-	attr.sample_period = 1;
-	attr.wakeup_events = 1;
-	pmu_fd = syscall(__NR_perf_event_open, &attr, -1 /* pid */,
-			 0 /* cpu 0 */, -1 /* group id */,
-			 0 /* flags */);
-	if (CHECK(pmu_fd < 0, "perf_event_open", "err %d errno %d\n",
-		  pmu_fd, errno))
-		goto close_prog;
-
-	err = ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0);
-	if (err)
-		goto disable_pmu;
-
-	err = ioctl(pmu_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
-	if (err)
-		goto disable_pmu;
-
 	/* find map fds */
 	control_map_fd = bpf_find_map(__func__, obj, "control_map");
 	if (control_map_fd < 0)
@@ -96,8 +72,7 @@ void test_stacktrace_map(void)
 disable_pmu:
 	error_cnt++;
 disable_pmu_noerr:
-	ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE);
-	close(pmu_fd);
+	bpf_link__destroy(link);
 close_prog:
 	bpf_object__close(obj);
 }
diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_map_raw_tp.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_map_raw_tp.c
index 1f8387d80fd7..fbfa8e76cf63 100644
--- a/tools/testing/selftests/bpf/prog_tests/stacktrace_map_raw_tp.c
+++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_map_raw_tp.c
@@ -3,18 +3,25 @@
 
 void test_stacktrace_map_raw_tp(void)
 {
+	const char *prog_name = "tracepoint/sched/sched_switch";
 	int control_map_fd, stackid_hmap_fd, stackmap_fd;
 	const char *file = "./test_stacktrace_map.o";
-	int efd, err, prog_fd;
 	__u32 key, val, duration = 0;
+	int err, prog_fd;
+	struct bpf_program *prog;
 	struct bpf_object *obj;
+	struct bpf_link *link = NULL;
 
 	err = bpf_prog_load(file, BPF_PROG_TYPE_RAW_TRACEPOINT, &obj, &prog_fd);
 	if (CHECK(err, "prog_load raw tp", "err %d errno %d\n", err, errno))
 		return;
 
-	efd = bpf_raw_tracepoint_open("sched_switch", prog_fd);
-	if (CHECK(efd < 0, "raw_tp_open", "err %d errno %d\n", efd, errno))
+	prog = bpf_object__find_program_by_title(obj, prog_name);
+	if (CHECK(!prog, "find_prog", "prog '%s' not found\n", prog_name))
+		goto close_prog;
+
+	link = bpf_program__attach_raw_tracepoint(prog, "sched_switch");
+	if (CHECK(IS_ERR(link), "attach_raw_tp", "err %ld\n", PTR_ERR(link)))
 		goto close_prog;
 
 	/* find map fds */
@@ -55,5 +62,7 @@ void test_stacktrace_map_raw_tp(void)
 close_prog:
 	error_cnt++;
 close_prog_noerr:
+	if (!IS_ERR_OR_NULL(link))
+		bpf_link__destroy(link);
 	bpf_object__close(obj);
 }
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH] netfilter: Fix remainder of pseudo-header protocol 0
From: He Zhe @ 2019-06-28  6:03 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: kadlec, fw, davem, netfilter-devel, coreteam, netdev,
	linux-kernel
In-Reply-To: <20190627184903.atdcwk4wnfaayyer@salvia>



On 6/28/19 2:49 AM, Pablo Neira Ayuso wrote:
> On Mon, Jun 24, 2019 at 11:17:38AM +0800, zhe.he@windriver.com wrote:
>> From: He Zhe <zhe.he@windriver.com>
>>
>> Since v5.1-rc1, some types of packets do not get unreachable reply with the
>> following iptables setting. Fox example,
>>
>> $ iptables -A INPUT -p icmp --icmp-type 8 -j REJECT
>> $ ping 127.0.0.1 -c 1
>> PING 127.0.0.1 (127.0.0.1) 56(84) bytes of data.
>> — 127.0.0.1 ping statistics —
>> 1 packets transmitted, 0 received, 100% packet loss, time 0ms
>>
>> We should have got the following reply from command line, but we did not.
>> From 127.0.0.1 icmp_seq=1 Destination Port Unreachable
>>
>> Yi Zhao reported it and narrowed it down to:
>> 7fc38225363d ("netfilter: reject: skip csum verification for protocols that don't support it"),
>>
>> This is because nf_ip_checksum still expects pseudo-header protocol type 0 for
>> packets that are of neither TCP or UDP, and thus ICMP packets are mistakenly
>> treated as TCP/UDP.
>>
>> This patch corrects the conditions in nf_ip_checksum and all other places that
>> still call it with protocol 0.
> Looking at 7fc38225363dd8f19e667ad7c77b63bc4a5c065d, I wonder this can
> be fixed while simplifying it...
>
> I think nf_reject_verify_csum() is useless?
>
> In your patch, now you explicitly check for IPPROTO_TCP and
> IPPROTO_UDP to validate the checksum.

Thanks for your review.

I suppose the two main points of 7fc38225363d are valid and I was trying to
align with them and fix them:
1) Skip csum verification for protocols that don't support it.
2) Remove the protocol 0 used to indicate non-TCP/UDP packets, and use actual
   types instead to be clear.

1) uses nf_reject_verify_csum to skip those that should be skipped and leaves
the protocols that support csum to the rest of the logic including
nf_ip_checksum. But 2) removes the "0" transition from the rest of the
logic and thus causes this issue. So I add the explicit check against TCP/UDP to
nf_ip_checksum. And nf_reject_verify_csum is still useful.

Zhe

>


^ permalink raw reply

* Re: [PATCH 2/3 nf-next] netfilter:nf_flow_table: Support bridge type flow offload
From: Florian Westphal @ 2019-06-28  6:06 UTC (permalink / raw)
  To: wenxu; +Cc: Pablo Neira Ayuso, Florian Westphal, netfilter-devel, netdev
In-Reply-To: <b2a48653-9f30-18a9-d0e1-eaa940a361a9@ucloud.cn>

wenxu <wenxu@ucloud.cn> wrote:
> ns21 iperf to 10.0.0.8 with dport 22 in ns22
> first time with OFFLOAD enable
> 
> nft add flowtable bridge firewall fb2 { hook ingress priority 0 \; devices = { veth21, veth22 } \; }
> nft add chain bridge firewall ftb-all {type filter hook forward priority 0 \; policy accept \; }
> nft add rule bridge firewall ftb-all counter ct zone 2 ip protocol tcp flow offload @fb2
> 
> # iperf -c 10.0.0.8 -p 22 -t 60 -i2
[..]
> [  3]  0.0-60.0 sec   353 GBytes  50.5 Gbits/sec
> 
> The second time on any offload:
> # iperf -c 10.0.0.8 -p 22 -t 60 -i2
> [  3]  0.0-60.0 sec   271 GBytes  38.8 Gbits/sec

Wow, this is pretty impressive.  Do you have numbers without
offload and no connection tracking?

Is this with CONFIG_RETPOLINE=y (just curious)?

^ 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-28  6:35 UTC (permalink / raw)
  To: Ivan Khoronzhuk
  Cc: davem, grygorii.strashko, saeedm, leon, ast, linux-kernel,
	linux-omap, ilias.apalodimas, netdev, daniel, jakub.kicinski,
	john.fastabend, brouer
In-Reply-To: <20190627220245.GA3269@khorivan>

On Fri, 28 Jun 2019 01:02:47 +0300
Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:

> Hi Jesper, thanks you remember about it.
> 
> >
> >I don't think that "create" and "free" routines paring looks "more
> >correct" together.
> >
> >Maybe we can scale back your solution(?), via creating a page_pool_get()
> >and page_pool_put() API that can be used by your driver, to keep the
> >page_pool object after a xdp_rxq_info_unreg() call.  Then you can use
> >it for two xdp_rxq_info structs, and call page_pool_put() after you
> >have unregistered both.
> >
> >The API would basically be:
> >
> >diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> >index b366f59885c1..691ddacfb5a6 100644
> >--- a/net/core/page_pool.c
> >+++ b/net/core/page_pool.c
> >@@ -357,6 +357,10 @@ static void __warn_in_flight(struct page_pool *pool)
> > void __page_pool_free(struct page_pool *pool)
> > {
> >        WARN(pool->alloc.count, "API usage violation");
> >+
> >+       if (atomic_read(&pool->user_cnt) != 0)
> >+               return;
> >+
> >        WARN(!ptr_ring_empty(&pool->ring), "ptr_ring is not empty");
> >
> >        /* Can happen due to forced shutdown */
> >@@ -372,6 +376,19 @@ void __page_pool_free(struct page_pool *pool)
> > }
> > EXPORT_SYMBOL(__page_pool_free);
> >
> >+void page_pool_put(struct page_pool *pool)
> >+{
> >+       if (!atomic_dec_and_test(&pool->user_cnt))
> >+               __page_pool_free(pool);
> >+}
> >+EXPORT_SYMBOL(page_pool_put);
> >+
> >+void page_pool_get(struct page_pool *pool)
> >+{
> >+       atomic_inc(&pool->user_cnt);
> >+}
> >+EXPORT_SYMBOL(page_pool_get);
> >+  
> 
> I have another solution that doesn't touch page pool and adds modifications
> to xdp allocator. As for me it looks better and work wider, I don't need to
> think about this in the driver also.
> 
> It's supposed allocator works as before, no any changes to mlx5 and
> page_pool API and its usage and seems like fits your requirements.
> It still supposes that allocator runs under same napi softirq but allows
> to reuse allocator.
> 
> I have not verified yet, but looks like:
> 
> diff --git a/include/net/xdp_priv.h b/include/net/xdp_priv.h
> index 6a8cba6ea79a..995b21da2f27 100644
> --- a/include/net/xdp_priv.h
> +++ b/include/net/xdp_priv.h
> @@ -18,6 +18,7 @@ struct xdp_mem_allocator {
>  	struct rcu_head rcu;
>  	struct delayed_work defer_wq;
>  	unsigned long defer_warn;
> +	unsigned long refcnt;
>  };
>  
>  #endif /* __LINUX_NET_XDP_PRIV_H__ */
> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index f98ab6b98674..6239483e3793 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -98,6 +98,12 @@ static bool __mem_id_disconnect(int id, bool force)
>  		WARN(1, "Request remove non-existing id(%d), driver bug?", id);
>  		return true;
>  	}
> +
> +	if (--xa->refcnt) {
> +		mutex_unlock(&mem_id_lock);
> +		return true;

This doesn't work.  This function __mem_id_disconnect() can be called
multiple times. E.g. if there are in-flight packets.


> +	}
> +
>  	xa->disconnect_cnt++;
>  
>  	/* Detects in-flight packet-pages for page_pool */
> @@ -312,6 +318,33 @@ static bool __is_supported_mem_type(enum xdp_mem_type type)
>  	return true;
>  }
>  
> +static struct xdp_mem_allocator *xdp_allocator_get(void *allocator)
> +{
> +	struct xdp_mem_allocator *xae, *xa == NULL;
> +	struct rhashtable_iter iter;
> +
> +	mutex_lock(&mem_id_lock);
> +	rhashtable_walk_enter(mem_id_ht, &iter);
> +	do {
> +		rhashtable_walk_start(&iter);
> +
> +		while ((xae = rhashtable_walk_next(&iter)) && !IS_ERR(xae)) {
> +			if (xae->allocator == allocator) {
> +				xae->refcnt++;
> +				xa = xae;
> +				break;
> +			}
> +		}
> +
> +		rhashtable_walk_stop(&iter);
> +
> +	} while (xae == ERR_PTR(-EAGAIN));
> +	rhashtable_walk_exit(&iter);
> +	mutex_unlock(&mem_id_lock);
> +
> +	return xa;
> +}
> +
>  int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
>  			       enum xdp_mem_type type, void *allocator)
>  {
> @@ -347,6 +380,9 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
>  		}
>  	}
>  
> +	if (xdp_allocator_get(allocator))
> +		return 0;
> +
>  	xdp_alloc = kzalloc(sizeof(*xdp_alloc), gfp);
>  	if (!xdp_alloc)
>  		return -ENOMEM;
> @@ -360,6 +396,7 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
>  	xdp_rxq->mem.id = id;
>  	xdp_alloc->mem  = xdp_rxq->mem;
>  	xdp_alloc->allocator = allocator;
> +	xdp_alloc->refcnt = 1;
>  
>  	/* Insert allocator into ID lookup table */
>  	ptr = rhashtable_insert_slow(mem_id_ht, &id, &xdp_alloc->node);
> 
> 



-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: [RFC, PATCH 2/2, net-next] net: netsec: add XDP support
From: Ilias Apalodimas @ 2019-06-28  6:54 UTC (permalink / raw)
  To: Maciej Fijalkowski
  Cc: netdev, jaswinder.singh, ard.biesheuvel, bjorn.topel,
	magnus.karlsson, brouer, daniel, ast, makita.toshiaki,
	jakub.kicinski, john.fastabend, davem
In-Reply-To: <20190627161816.0000645a@gmail.com>

Hi Maciej Fijalkowski,

[...]
> > +	tx_ctrl.cksum_offload_flag = false;
> > +	tx_ctrl.tcp_seg_offload_flag = false;
> > +	tx_ctrl.tcp_seg_len = 0;
> 
> Aren't these three lines redundant? tx_ctrl is zero initialized.
> 
Yea i think i can remove those

> > +
> > +	tx_desc.dma_addr = dma_handle;
> > +	tx_desc.addr = xdpf->data;
> > +	tx_desc.len = xdpf->len;
> > +
> > +	netsec_set_tx_de(priv, tx_ring, &tx_ctrl, &tx_desc, xdpf);
> > +
> > +	return NETSEC_XDP_TX;
> > +}
> > +
> > +static u32 netsec_xdp_xmit_back(struct netsec_priv *priv, struct xdp_buff *xdp)
> > +{
> > +	struct netsec_desc_ring *tx_ring = &priv->desc_ring[NETSEC_RING_TX];
> > +	struct xdp_frame *xdpf = convert_to_xdp_frame(xdp);
> > +	u32 ret;
> > +
> > +	if (unlikely(!xdpf))
> > +		return NETSEC_XDP_CONSUMED;
> > +
> > +	spin_lock(&tx_ring->lock);
> > +	ret = netsec_xdp_queue_one(priv, xdpf, false);
> > +	spin_unlock(&tx_ring->lock);
> > +
> > +	return ret;
> > +}
> > +
> > +static u32 netsec_run_xdp(struct netsec_priv *priv, struct bpf_prog *prog,
> > +			  struct xdp_buff *xdp)
> > +{
> > +	u32 ret = NETSEC_XDP_PASS;
> > +	int err;
> > +	u32 act;
> > +
> > +	rcu_read_lock();
> > +	act = bpf_prog_run_xdp(prog, xdp);
> > +
> > +	switch (act) {
> > +	case XDP_PASS:
> > +		ret = NETSEC_XDP_PASS;
> > +		break;
> > +	case XDP_TX:
> > +		ret = netsec_xdp_xmit_back(priv, xdp);
> > +		if (ret != NETSEC_XDP_TX)
> > +			xdp_return_buff(xdp);
> > +		break;
> > +	case XDP_REDIRECT:
> > +		err = xdp_do_redirect(priv->ndev, xdp, prog);
> > +		if (!err) {
> > +			ret = NETSEC_XDP_REDIR;
> > +		} else {
> > +			ret = NETSEC_XDP_CONSUMED;
> > +			xdp_return_buff(xdp);
> > +		}
> > +		break;
> > +	default:
> > +		bpf_warn_invalid_xdp_action(act);
> > +		/* fall through */
> > +	case XDP_ABORTED:
> > +		trace_xdp_exception(priv->ndev, prog, act);
> > +		/* fall through -- handle aborts by dropping packet */
> > +	case XDP_DROP:
> > +		ret = NETSEC_XDP_CONSUMED;
> > +		xdp_return_buff(xdp);
> > +		break;
> > +	}
> > +
> > +	rcu_read_unlock();
> > +
> > +	return ret;
> > +}
> > +
> >  static int netsec_process_rx(struct netsec_priv *priv, int budget)
> >  {
> >  	struct netsec_desc_ring *dring = &priv->desc_ring[NETSEC_RING_RX];
> > +	struct bpf_prog *xdp_prog = READ_ONCE(priv->xdp_prog);
> 
> Reading BPF prog should be RCU protected. There might be a case where RCU
> callback that destroys BPF prog is executed during the bottom half handling and
> you have the PREEMPT_RCU=y in your kernel config. I've just rephrased Brenden's
> words here, so for further info, see:
> 
> https://lore.kernel.org/netdev/20160904042958.8594-1-bblanco@plumgrid.com/
> 
> So either expand the RCU section or read prog pointer per each frame, under the
> lock, as it seems that currently we have these two schemes in drivers that
> support XDP.
> 
Thanks, i'll fix it

Cheers
/Ilias

^ permalink raw reply

* [PATCH v4] net: netfilter: Fix rpfilter dropping vrf packets by mistake
From: Miaohe Lin @ 2019-06-28  9:06 UTC (permalink / raw)
  To: pablo, kadlec, fw, davem, kuznet, yoshfuji, netfilter-devel,
	coreteam, netdev, linux-kernel
  Cc: linmiaohe, mingfangsen

When firewalld is enabled with ipv4/ipv6 rpfilter, vrf
ipv4/ipv6 packets will be dropped. Vrf device will pass
through netfilter hook twice. One with enslaved device
and another one with l3 master device. So in device may
dismatch witch out device because out device is always
enslaved device.So failed with the check of the rpfilter
and drop the packets by mistake.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 net/ipv4/netfilter/ipt_rpfilter.c  | 1 +
 net/ipv6/netfilter/ip6t_rpfilter.c | 8 +++++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/netfilter/ipt_rpfilter.c b/net/ipv4/netfilter/ipt_rpfilter.c
index 59031670b16a..cc23f1ce239c 100644
--- a/net/ipv4/netfilter/ipt_rpfilter.c
+++ b/net/ipv4/netfilter/ipt_rpfilter.c
@@ -78,6 +78,7 @@ static bool rpfilter_mt(const struct sk_buff *skb, struct xt_action_param *par)
 	flow.flowi4_mark = info->flags & XT_RPFILTER_VALID_MARK ? skb->mark : 0;
 	flow.flowi4_tos = RT_TOS(iph->tos);
 	flow.flowi4_scope = RT_SCOPE_UNIVERSE;
+	flow.flowi4_oif = l3mdev_master_ifindex_rcu(xt_in(par));
 
 	return rpfilter_lookup_reverse(xt_net(par), &flow, xt_in(par), info->flags) ^ invert;
 }
diff --git a/net/ipv6/netfilter/ip6t_rpfilter.c b/net/ipv6/netfilter/ip6t_rpfilter.c
index 6bcaf7357183..3c4a1772c15f 100644
--- a/net/ipv6/netfilter/ip6t_rpfilter.c
+++ b/net/ipv6/netfilter/ip6t_rpfilter.c
@@ -55,6 +55,10 @@ static bool rpfilter_lookup_reverse6(struct net *net, const struct sk_buff *skb,
 	if (rpfilter_addr_linklocal(&iph->saddr)) {
 		lookup_flags |= RT6_LOOKUP_F_IFACE;
 		fl6.flowi6_oif = dev->ifindex;
+	/* Set flowi6_oif for vrf devices to lookup route in l3mdev domain. */
+	} else if (netif_is_l3_master(dev) || netif_is_l3_slave(dev)) {
+		lookup_flags |= FLOWI_FLAG_SKIP_NH_OIF;
+		fl6.flowi6_oif = dev->ifindex;
 	} else if ((flags & XT_RPFILTER_LOOSE) == 0)
 		fl6.flowi6_oif = dev->ifindex;
 
@@ -70,7 +74,9 @@ static bool rpfilter_lookup_reverse6(struct net *net, const struct sk_buff *skb,
 		goto out;
 	}
 
-	if (rt->rt6i_idev->dev == dev || (flags & XT_RPFILTER_LOOSE))
+	if (rt->rt6i_idev->dev == dev ||
+	    l3mdev_master_ifindex_rcu(rt->rt6i_idev->dev) == dev->ifindex ||
+	    (flags & XT_RPFILTER_LOOSE))
 		ret = true;
  out:
 	ip6_rt_put(rt);
-- 
2.21.GIT


^ permalink raw reply related

* Re: [PATCH bpf-next v5 1/3] devmap/cpumap: Use flush list instead of bitmap
From: Toke Høiland-Jørgensen @ 2019-06-28  7:14 UTC (permalink / raw)
  To: Daniel Borkmann, netdev
  Cc: Jesper Dangaard Brouer, Alexei Starovoitov, David Miller,
	Jonathan Lemon
In-Reply-To: <ff82dde7-8f31-1ab5-65b8-5e2d5ca5f680@iogearbox.net>

Daniel Borkmann <daniel@iogearbox.net> writes:

> On 06/23/2019 04:17 AM, Toke Høiland-Jørgensen wrote:
>> From: Toke Høiland-Jørgensen <toke@redhat.com>
>> 
>> The socket map uses a linked list instead of a bitmap to keep track of
>> which entries to flush. Do the same for devmap and cpumap, as this means we
>> don't have to care about the map index when enqueueing things into the
>> map (and so we can cache the map lookup).
>> 
>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> [...]
>> +static int bq_flush_to_queue(struct xdp_bulk_queue *bq, bool in_napi_ctx)
>>  {
>> +	struct bpf_cpu_map_entry *rcpu = bq->obj;
>>  	unsigned int processed = 0, drops = 0;
>>  	const int to_cpu = rcpu->cpu;
>>  	struct ptr_ring *q;
>> @@ -621,6 +630,9 @@ static int bq_flush_to_queue(struct bpf_cpu_map_entry *rcpu,
>>  	bq->count = 0;
>>  	spin_unlock(&q->producer_lock);
>>  
>> +	__list_del(bq->flush_node.prev, bq->flush_node.next);
>> +	bq->flush_node.prev = NULL;
>
> Given this and below is a bit non-standard way of using list API, maybe add
> these as inline helpers to include/linux/list.h to make sure anyone changing
> list API semantics doesn't overlook these in future?

Sure, can do.

-Toke

^ permalink raw reply

* Re: XDP multi-buffer incl. jumbo-frames (Was: [RFC V1 net-next 1/1] net: ena: implement XDP drop support)
From: Eelco Chaudron @ 2019-06-28  7:14 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: Machulsky, Zorik, Jubran, Samih, davem, netdev, Woodhouse, David,
	Matushevsky, Alexander, Bshara, Saeed, Wilson, Matt,
	Liguori, Anthony, Bshara, Nafea, Tzalik, Guy, Belgazal, Netanel,
	Saidi, Ali, Herrenschmidt, Benjamin, Kiyanovski, Arthur,
	Daniel Borkmann, Toke Høiland-Jørgensen,
	Ilias Apalodimas, Alexei Starovoitov, Jakub Kicinski, xdp-newbies
In-Reply-To: <20190626103829.5360ef2d@carbon>



On 26 Jun 2019, at 10:38, Jesper Dangaard Brouer wrote:

> On Tue, 25 Jun 2019 03:19:22 +0000
> "Machulsky, Zorik" <zorik@amazon.com> wrote:
>
>> On 6/23/19, 7:21 AM, "Jesper Dangaard Brouer" <brouer@redhat.com> 
>> wrote:
>>
>>     On Sun, 23 Jun 2019 10:06:49 +0300 <sameehj@amazon.com> wrote:
>>
>>     > This commit implements the basic functionality of drop/pass 
>> logic in the
>>     > ena driver.
>>
>>     Usually we require a driver to implement all the XDP return 
>> codes,
>>     before we accept it.  But as Daniel and I discussed with Zorik 
>> during
>>     NetConf[1], we are going to make an exception and accept the 
>> driver
>>     if you also implement XDP_TX.
>>
>>     As we trust that Zorik/Amazon will follow and implement 
>> XDP_REDIRECT
>>     later, given he/you wants AF_XDP support which requires 
>> XDP_REDIRECT.
>>
>> Jesper, thanks for your comments and very helpful discussion during
>> NetConf! That's the plan, as we agreed. From our side I would like to
>> reiterate again the importance of multi-buffer support by xdp frame.
>> We would really prefer not to see our MTU shrinking because of xdp
>> support.
>
> Okay we really need to make a serious attempt to find a way to support
> multi-buffer packets with XDP. With the important criteria of not
> hurting performance of the single-buffer per packet design.
>
> I've created a design document[2], that I will update based on our
> discussions: [2] 
> https://github.com/xdp-project/xdp-project/blob/master/areas/core/xdp-multi-buffer01-design.org
>
> The use-case that really convinced me was Eric's packet header-split.
>
>
> Lets refresh: Why XDP don't have multi-buffer support:
>
> XDP is designed for maximum performance, which is why certain 
> driver-level
> use-cases were not supported, like multi-buffer packets (like 
> jumbo-frames).
> As it e.g. complicated the driver RX-loop and memory model handling.
>
> The single buffer per packet design, is also tied into eBPF 
> Direct-Access
> (DA) to packet data, which can only be allowed if the packet memory is 
> in
> contiguous memory.  This DA feature is essential for XDP performance.
>
>
> One way forward is to define that XDP only get access to the first
> packet buffer, and it cannot see subsequent buffers.  For XDP_TX and
> XDP_REDIRECT to work then XDP still need to carry pointers (plus
> len+offset) to the other buffers, which is 16 bytes per extra buffer.


I’ve seen various network processor HW designs, and they normally get 
the first x bytes (128 - 512) which they can manipulate 
(append/prepend/insert/modify/delete).

There are designs where they can “page in” the additional fragments 
but it’s expensive as it requires additional memory transfers. But the 
majority do not care (cannot change) the remaining fragments. Can also 
not think of a reason why you might want to remove something at the end 
of the frame (thinking about routing/forwarding needs here).

If we do want XDP to access other fragments we could do this through a 
helper which swaps the packet context?

//Eelco


^ permalink raw reply

* Re: [PATCH bpf-next v5 2/3] bpf_xdp_redirect_map: Perform map lookup in eBPF helper
From: Toke Høiland-Jørgensen @ 2019-06-28  7:17 UTC (permalink / raw)
  To: Daniel Borkmann, netdev
  Cc: Jesper Dangaard Brouer, Alexei Starovoitov, David Miller,
	Jonathan Lemon
In-Reply-To: <04a5da1d-6d0e-5963-4622-20cb54285926@iogearbox.net>

Daniel Borkmann <daniel@iogearbox.net> writes:

> On 06/23/2019 04:17 AM, Toke Høiland-Jørgensen wrote:
>> From: Toke Høiland-Jørgensen <toke@redhat.com>
>> 
>> The bpf_redirect_map() helper used by XDP programs doesn't return any
>> indication of whether it can successfully redirect to the map index it was
>> given. Instead, BPF programs have to track this themselves, leading to
>> programs using duplicate maps to track which entries are populated in the
>> devmap.
>> 
>> This patch fixes this by moving the map lookup into the bpf_redirect_map()
>> helper, which makes it possible to return failure to the eBPF program. The
>> lower bits of the flags argument is used as the return code, which means
>> that existing users who pass a '0' flag argument will get XDP_ABORTED.
>> 
>> With this, a BPF program can check the return code from the helper call and
>> react by, for instance, substituting a different redirect. This works for
>> any type of map used for redirect.
>> 
>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
>
> Overall series looks good to me. Just very small things inline here & in the
> other two patches:
>
> [...]
>> @@ -3750,9 +3742,16 @@ BPF_CALL_3(bpf_xdp_redirect_map, struct bpf_map *, map, u32, ifindex,
>>  {
>>  	struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
>>  
>> -	if (unlikely(flags))
>> +	/* Lower bits of the flags are used as return code on lookup failure */
>> +	if (unlikely(flags > XDP_TX))
>>  		return XDP_ABORTED;
>>  
>> +	ri->item = __xdp_map_lookup_elem(map, ifindex);
>> +	if (unlikely(!ri->item)) {
>> +		WRITE_ONCE(ri->map, NULL);
>
> This WRITE_ONCE() is not needed. We never set it before at this point.

You mean the WRITE_ONCE() wrapper is not needed, or the set-to-NULL is
not needed? The reason I added it is in case an eBPF program calls the
helper twice before returning, where the first lookup succeeds but the
second fails; in that case we want to clear the ->map pointer, no?

-Toke

^ permalink raw reply

* Re: [PATCH RFC net-next 1/5] net: dsa: mt7530: Convert to PHYLINK API
From: Daniel Santos @ 2019-06-28  7:16 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: René van Dorst, sean.wang, f.fainelli, davem, matthias.bgg,
	vivien.didelot, frank-w, netdev, linux-mediatek, linux-mips
In-Reply-To: <20190627192806.GQ27733@lunn.ch>

Hello Andrew,

On 6/27/19 2:28 PM, Andrew Lunn wrote:
>>> Looking at the data sheet page, you want FORCE_MODE_Pn set. You never
>>> want the MAC directly talking to the PHY. Bad things will happen.
>> So what exactly do you mean by the MAC directly talking to the PHY?  Do
>> you mean setting speed, duplex, etc. via the MAC registers instead of
>> via MDIO to the MII registers of the PHY?
> The data sheet implies the MAC hardware performs reads on the PHY to
> get the status, and then uses that to configure the MAC. This is often
> called PHY polling. The MAC hardware however has no idea what the PHY
> driver is doing. The MAC does not take the PHY mutex. So the PHY
> driver might be doing something at the same time the MAC hardware
> polls the PHY, and it gets odd results. Some PHYs have multiple pages,
> and for example reading the temperature sensor means swapping
> pages. If the MAC hardware was to poll while the sensor is being read,
> it would not get the link status, it would read some random
> temperature register.
>
> So you want the PHY driver to read the results of auto-neg and it then
> tell the MAC the results, so the MAC can be configured correctly.

Thank you, this is very helpful!  I finally understand why these
settings are in two different places. :)  Unfortunately this driver (in
OpenWRT) does a lot of "magic" during init to registers that I don't
have documentation for, but I see where auto-polling can be disabled now.

>>> Then use FORCE_RX_FC_Pn and FORCE_TX_Pn to reflect phydev->pause and
>>> phydev->asym_pause.
>>>
>>> The same idea applies when using phylink.
>>>
>>>     Andrew
>> You're help is greatly appreciated here.  Admittedly, I'm also trying to
>> get this working in the now deprecated swconfig for a 3.18 kernel that's
>> in production.
> I'm not very familiar with swconfig. Is there software driving the
> PHY? If not, it is then safe for the MAC hardware to poll the PHY.
>
>      Andrew

swconfig is an netlink-based interface the OpenWRT team developed for
configuring switches before DSA was converted into a vendor-neutral
interface.  Now that DSA does what swconfig was designed for it has been
deprecated, although (to my knowledge) we don't yet have DSA for all
devices that OpenWRT supports.

Daniel

^ permalink raw reply

* pull-request: wireless-drivers 2019-06-28
From: Kalle Valo @ 2019-06-28  7:18 UTC (permalink / raw)
  To: David Miller; +Cc: linux-wireless, netdev, linux-kernel

Hi Dave,

here's a pull request to net tree for 5.2, more info below. Please let
me know if there are any problems.

Kalle

The following changes since commit c356dc4b540edd6c02b409dd8cf3208ba2804c38:

  Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2019-06-21 22:23:35 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git tags/wireless-drivers-for-davem-2019-06-28

for you to fetch changes up to 2a92b08b18553c101115423bd34963b1a59a45a3:

  mt76: usb: fix rx A-MSDU support (2019-06-27 19:48:36 +0300)

----------------------------------------------------------------
wireless-drivers fixes for 5.2

Hopefully the last set of fixes for 5.2. Nothing special this around,
only small fixes and support for new cards.

iwlwifi

* add new cards for 22000 series and smaller fixes

wl18xx

* fix a clang warning about unused variables

mwifiex

* properly handle small vendor IEs (a regression from the recent
  security fix)

ath

* fix few SPDX tags

mt76

* fix A-MSDU aggregation which got broken in v5.2-rc1

----------------------------------------------------------------
Brian Norris (1):
      mwifiex: Don't abort on small, spec-compliant vendor IEs

Ihab Zhaika (3):
      iwlwifi: add new cards for 22000 and fix struct name
      iwlwifi: add new cards for 22000 and change wrong structs
      iwlwifi: change 0x02F0 fw from qu to quz

Kalle Valo (1):
      ath: fix SPDX tags

Lorenzo Bianconi (1):
      mt76: usb: fix rx A-MSDU support

Nathan Huckleberry (1):
      wl18xx: Fix Wunused-const-variable

Oren Givon (1):
      iwlwifi: add support for hr1 RF ID

 drivers/net/wireless/ath/Kconfig                 |   2 +-
 drivers/net/wireless/ath/Makefile                |   2 +-
 drivers/net/wireless/ath/ar5523/Kconfig          |   2 +-
 drivers/net/wireless/ath/ar5523/Makefile         |   2 +-
 drivers/net/wireless/ath/ath10k/Kconfig          |   2 +-
 drivers/net/wireless/ath/ath5k/Kconfig           |   2 +-
 drivers/net/wireless/ath/ath5k/Makefile          |   2 +-
 drivers/net/wireless/ath/ath6kl/Kconfig          |   2 +-
 drivers/net/wireless/ath/ath6kl/trace.h          |   2 +-
 drivers/net/wireless/ath/ath9k/Kconfig           |   2 +-
 drivers/net/wireless/ath/ath9k/Makefile          |   2 +-
 drivers/net/wireless/ath/wcn36xx/Kconfig         |   2 +-
 drivers/net/wireless/ath/wcn36xx/Makefile        |   2 +-
 drivers/net/wireless/ath/wil6210/Kconfig         |   2 +-
 drivers/net/wireless/ath/wil6210/Makefile        |   2 +-
 drivers/net/wireless/intel/iwlwifi/cfg/22000.c   | 144 +++++++++++++-
 drivers/net/wireless/intel/iwlwifi/iwl-config.h  |  14 +-
 drivers/net/wireless/intel/iwlwifi/iwl-csr.h     |   1 +
 drivers/net/wireless/intel/iwlwifi/pcie/drv.c    | 241 ++++++++++++-----------
 drivers/net/wireless/intel/iwlwifi/pcie/trans.c  |  17 +-
 drivers/net/wireless/marvell/mwifiex/fw.h        |  12 +-
 drivers/net/wireless/marvell/mwifiex/scan.c      |  18 +-
 drivers/net/wireless/marvell/mwifiex/sta_ioctl.c |   4 +-
 drivers/net/wireless/marvell/mwifiex/wmm.c       |   2 +-
 drivers/net/wireless/mediatek/mt76/mt76.h        |   1 +
 drivers/net/wireless/mediatek/mt76/usb.c         |  46 ++++-
 drivers/net/wireless/ti/wl18xx/main.c            |  38 ----
 27 files changed, 364 insertions(+), 204 deletions(-)

^ permalink raw reply

* Re: [PATCH bpf-next v5 2/3] bpf_xdp_redirect_map: Perform map lookup in eBPF helper
From: Toke Høiland-Jørgensen @ 2019-06-28  7:23 UTC (permalink / raw)
  To: Daniel Borkmann, netdev
  Cc: Jesper Dangaard Brouer, Alexei Starovoitov, David Miller,
	Jonathan Lemon
In-Reply-To: <d0d5dbe3-d2bf-2284-b2a3-667c77487125@iogearbox.net>

Daniel Borkmann <daniel@iogearbox.net> writes:

> On 06/23/2019 04:17 AM, Toke Høiland-Jørgensen wrote:
>> From: Toke Høiland-Jørgensen <toke@redhat.com>
>> 
>> The bpf_redirect_map() helper used by XDP programs doesn't return any
>> indication of whether it can successfully redirect to the map index it was
>> given. Instead, BPF programs have to track this themselves, leading to
>> programs using duplicate maps to track which entries are populated in the
>> devmap.
>> 
>> This patch fixes this by moving the map lookup into the bpf_redirect_map()
>> helper, which makes it possible to return failure to the eBPF program. The
>> lower bits of the flags argument is used as the return code, which means
>> that existing users who pass a '0' flag argument will get XDP_ABORTED.
>> 
>> With this, a BPF program can check the return code from the helper call and
>> react by, for instance, substituting a different redirect. This works for
>> any type of map used for redirect.
>> 
>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> [...]
>> diff --git a/net/core/filter.c b/net/core/filter.c
>> index 183bf4d8e301..a6779e1cc1b8 100644
>> --- a/net/core/filter.c
>> +++ b/net/core/filter.c
>> @@ -3605,17 +3605,13 @@ static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
>>  			       struct bpf_redirect_info *ri)
>>  {
>>  	u32 index = ri->ifindex;
>> -	void *fwd = NULL;
>> +	void *fwd = ri->item;
>>  	int err;
>>  
>>  	ri->ifindex = 0;
>> +	ri->item = NULL;
>>  	WRITE_ONCE(ri->map, NULL);
>>  
>> -	fwd = __xdp_map_lookup_elem(map, index);
>> -	if (unlikely(!fwd)) {
>> -		err = -EINVAL;
>> -		goto err;
>> -	}
>
> If you look at the _trace_xdp_redirect{,_err}(), we should also get rid of the
> extra NULL test in devmap_ifindex() which is not under tracepoint static key.

ACK, will add.

-Toke

^ permalink raw reply

* [PATCH net-next v2] net: stmmac: Fix case when PHY handle is not present
From: Jose Abreu @ 2019-06-28  7:25 UTC (permalink / raw)
  To: linux-kernel, netdev
  Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
	Alexandre Torgue, Sergei Shtylyov

Some DT bindings do not have the PHY handle. Let's fallback to manually
discovery in case phylink_of_phy_connect() fails.

Changes from v1:
	- Fixup comment style (Sergei)

Fixes: 74371272f97f ("net: stmmac: Convert to phylink and remove phylib logic")
Reported-by: Katsuhiro Suzuki <katsuhiro@katsuster.net>
Tested-by: Katsuhiro Suzuki <katsuhiro@katsuster.net>
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 8f5ebd51859e..91f24b63ea16 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -953,9 +953,13 @@ static int stmmac_init_phy(struct net_device *dev)
 
 	node = priv->plat->phylink_node;
 
-	if (node) {
+	if (node)
 		ret = phylink_of_phy_connect(priv->phylink, node, 0);
-	} else {
+
+	/* Some DT bindings do not set-up the PHY handle. Let's try to
+	 * manually parse it
+	 */
+	if (!node || ret) {
 		int addr = priv->plat->phy_addr;
 		struct phy_device *phydev;
 
-- 
2.7.4


^ permalink raw reply related

* RE: [PATCH net-next 10/10] net: stmmac: Try to get C45 PHY if everything else fails
From: Jose Abreu @ 2019-06-28  7:27 UTC (permalink / raw)
  To: Andrew Lunn, Jose Abreu
  Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org, Joao Pinto,
	David S . Miller, Giuseppe Cavallaro, Alexandre Torgue
In-Reply-To: <20190627140316.GF31189@lunn.ch>

From: Andrew Lunn <andrew@lunn.ch>

> On Thu, Jun 27, 2019 at 01:33:59PM +0000, Jose Abreu wrote:
> > From: Andrew Lunn <andrew@lunn.ch>
> > 
> > > There have been some drivers gaining patches for ACPI. That is
> > > probably the better long term solution, ask ACPI where is the PHY and
> > > what MDIO protocol to use to talk to it.
> > 
> > Hmmm, I'm not sure this is going to work that way ...
> > 
> > My setup is a PCI EP which is hot-pluggable and as far as I know ACPI 
> > has only static content (????)
> 
> I've wanted to improve the PHY probe code for a while. I was thinking
> we should add a flag to the MDIO bus driver structure indicating it
> can do C45. When that flag is present, we should also scan the bus for
> C45 devices, and register them as well.
> 
> With that in place, i think your problem goes away. Architecturally, i
> think it is wrong that a MAC driver is registering PHY devices. The
> MDIO core should do this.

Ok, I will drop this patch from the series until we come up with a 
better solution.

^ permalink raw reply

* [PATCH net-next v2 08/10] net: stmmac: Update RX Tail Pointer to last free entry
From: Jose Abreu @ 2019-06-28  7:29 UTC (permalink / raw)
  To: linux-kernel, netdev
  Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
	Alexandre Torgue
In-Reply-To: <cover.1561706800.git.joabreu@synopsys.com>

Update the RX Tail Pointer to the last available SKB entry.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 620dd387e3b1..a629b3144a0d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3347,6 +3347,8 @@ static inline void stmmac_rx_refill(struct stmmac_priv *priv, u32 queue)
 		entry = STMMAC_GET_ENTRY(entry, DMA_RX_SIZE);
 	}
 	rx_q->dirty_rx = entry;
+	rx_q->rx_tail_addr = rx_q->dma_rx_phy +
+			    (rx_q->dirty_rx * sizeof(struct dma_desc));
 	stmmac_set_rx_tail_ptr(priv, priv->ioaddr, rx_q->rx_tail_addr, queue);
 }
 
-- 
2.7.4


^ permalink raw reply related

* [PATCH net-next v2 10/10] net: stmmac: Update Kconfig entry
From: Jose Abreu @ 2019-06-28  7:29 UTC (permalink / raw)
  To: linux-kernel, netdev
  Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
	Alexandre Torgue
In-Reply-To: <cover.1561706800.git.joabreu@synopsys.com>

We support more speeds now. Update the Kconfig entry.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
 drivers/net/ethernet/stmicro/stmmac/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index c43e2da4e7e3..2acb999b7f63 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config STMMAC_ETH
-	tristate "STMicroelectronics 10/100/1000/EQOS Ethernet driver"
+	tristate "STMicroelectronics 10/100/1000/EQOS/2500/5000/10000 Ethernet driver"
 	depends on HAS_IOMEM && HAS_DMA
 	select MII
 	select PHYLINK
-- 
2.7.4


^ permalink raw reply related

* [PATCH net-next v2 06/10] net: stmmac: Do not disable interrupts when cleaning TX
From: Jose Abreu @ 2019-06-28  7:29 UTC (permalink / raw)
  To: linux-kernel, netdev
  Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
	Alexandre Torgue
In-Reply-To: <cover.1561706800.git.joabreu@synopsys.com>

This is a performance killer and anyways the interrupts are being
disabled by RX NAPI so no need to disable them again.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 4a5941caaadc..e8f3e76889c8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2061,10 +2061,8 @@ static int stmmac_napi_check(struct stmmac_priv *priv, u32 chan)
 		napi_schedule_irqoff(&ch->rx_napi);
 	}
 
-	if ((status & handle_tx) && (chan < priv->plat->tx_queues_to_use)) {
-		stmmac_disable_dma_irq(priv, priv->ioaddr, chan);
+	if ((status & handle_tx) && (chan < priv->plat->tx_queues_to_use))
 		napi_schedule_irqoff(&ch->tx_napi);
-	}
 
 	return status;
 }
@@ -3570,8 +3568,8 @@ static int stmmac_napi_poll_tx(struct napi_struct *napi, int budget)
 	work_done = stmmac_tx_clean(priv, DMA_TX_SIZE, chan);
 	work_done = min(work_done, budget);
 
-	if (work_done < budget && napi_complete_done(napi, work_done))
-		stmmac_enable_dma_irq(priv, priv->ioaddr, chan);
+	if (work_done < budget)
+		napi_complete_done(napi, work_done);
 
 	/* Force transmission restart */
 	tx_q = &priv->tx_queue[chan];
-- 
2.7.4


^ permalink raw reply related

* Re: [RFC] longer netdev names proposal
From: Jiri Pirko @ 2019-06-28  7:29 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, davem, jakub.kicinski, sthemmin, mlxsw
In-Reply-To: <26b73332-9ea0-9d2c-9185-9de522c72bb9@gmail.com>

Thu, Jun 27, 2019 at 07:14:31PM CEST, dsahern@gmail.com wrote:
>On 6/27/19 3:43 AM, Jiri Pirko wrote:
>> Hi all.
>> 
>> In the past, there was repeatedly discussed the IFNAMSIZ (16) limit for
>> netdevice name length. Now when we have PF and VF representors
>> with port names like "pfXvfY", it became quite common to hit this limit:
>> 0123456789012345
>> enp131s0f1npf0vf6
>> enp131s0f1npf0vf22
>
>QinQ (stacked vlans) is another example.

There are more usecases for this, yes.


>
>> 
>> Since IFLA_NAME is just a string, I though it might be possible to use
>> it to carry longer names as it is. However, the userspace tools, like
>> iproute2, are doing checks before print out. So for example in output of
>> "ip addr" when IFLA_NAME is longer than IFNAMSIZE, the netdevice is
>> completely avoided.
>> 
>> So here is a proposal that might work:
>> 1) Add a new attribute IFLA_NAME_EXT that could carry names longer than
>>    IFNAMSIZE, say 64 bytes. The max size should be only defined in kernel,
>>    user should be prepared for any string size.
>> 2) Add a file in sysfs that would indicate that NAME_EXT is supported by
>>    the kernel.
>
>no sysfs files.
>
>Johannes added infrastructure to retrieve the policy. That is a more
>flexible and robust option for determining what the kernel supports.

Sure, udev can query rtnetlink. I just proposed it as an option, anyway,
it's implementation detail.


>
>
>> 3) Udev is going to look for the sysfs indication file. In case when
>>    kernel supports long names, it will do rename to longer name, setting
>>    IFLA_NAME_EXT. If not, it does what it does now - fail.
>> 4) There are two cases that can happen during rename:
>>    A) The name is shorter than IFNAMSIZ
>>       -> both IFLA_NAME and IFLA_NAME_EXT would contain the same string:
>>          original IFLA_NAME     = eth0
>>          original IFLA_NAME_EXT = eth0
>>          renamed  IFLA_NAME     = enp5s0f1npf0vf1
>>          renamed  IFLA_NAME_EXT = enp5s0f1npf0vf1
>>    B) The name is longer tha IFNAMSIZ
>>       -> IFLA_NAME would contain the original one, IFLA_NAME_EXT would 
>>          contain the new one:
>>          original IFLA_NAME     = eth0
>>          original IFLA_NAME_EXT = eth0
>>          renamed  IFLA_NAME     = eth0
>>          renamed  IFLA_NAME_EXT = enp131s0f1npf0vf22
>
>so kernel side there will be 2 names for the same net_device?

Yes. However, updated tools (which would be eventually all) are going to
show only the ext one.



>
>> 
>> This would allow the old tools to work with "eth0" and the new
>> tools would work with "enp131s0f1npf0vf22". In sysfs, there would
>> be symlink from one name to another.
>
>I would prefer a solution that does not rely on sysfs hooks.

Please note that this /sys/class/net/ifacename dirs are already created.
What I propose is to have symlink from ext to the short name or vice
versa. The solution really does not "rely" on this...


>
>>       
>> Also, there might be a warning added to kernel if someone works
>> with IFLA_NAME that the userspace tool should be upgraded.
>
>that seems like spam and confusion for the first few years of a new api.

Spam? warn_once?


>
>> 
>> Eventually, only IFLA_NAME_EXT is going to be used by everyone.
>> 
>> I'm aware there are other places where similar new attribute
>> would have to be introduced too (ip rule for example).
>> I'm not saying this is a simple work.
>> 
>> Question is what to do with the ioctl api (get ifindex etc). I would
>> probably leave it as is and push tools to use rtnetlink instead.
>
>The ioctl API is going to be a limiter here. ifconfig is still quite
>prevalent and net-snmp still uses ioctl (as just 2 common examples).
>snmp showing one set of names and rtnetlink s/w showing another is going
>to be really confusing.

I don't see other way though, do you? The ioctl names are unextendable :/


^ permalink raw reply

* [PATCH net-next v2 05/10] net: stmmac: Add the missing speeds that XGMAC supports
From: Jose Abreu @ 2019-06-28  7:29 UTC (permalink / raw)
  To: linux-kernel, netdev
  Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
	Alexandre Torgue
In-Reply-To: <cover.1561706800.git.joabreu@synopsys.com>

XGMAC supports following speeds:
	- 10G XGMII
	- 5G XGMII
	- 2.5G XGMII
	- 2.5G GMII
	- 1G GMII
	- 100M MII
	- 10M MII

Add them to the stmmac driver.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
 drivers/net/ethernet/stmicro/stmmac/common.h       |  6 +-
 drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h     | 10 ++-
 .../net/ethernet/stmicro/stmmac/dwxgmac2_core.c    | 14 ++--
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 79 ++++++++++++++++++----
 4 files changed, 86 insertions(+), 23 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 14d8f6d7cb9a..9e8f4dbdcc22 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -392,8 +392,12 @@ struct mac_link {
 	u32 speed100;
 	u32 speed1000;
 	u32 speed2500;
-	u32 speed10000;
 	u32 duplex;
+	struct {
+		u32 speed2500;
+		u32 speed5000;
+		u32 speed10000;
+	} xgmii;
 };
 
 struct mii_regs {
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
index c9e802faabb5..29db9b538874 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
@@ -15,10 +15,14 @@
 /* MAC Registers */
 #define XGMAC_TX_CONFIG			0x00000000
 #define XGMAC_CONFIG_SS_OFF		29
-#define XGMAC_CONFIG_SS_MASK		GENMASK(30, 29)
+#define XGMAC_CONFIG_SS_MASK		GENMASK(31, 29)
 #define XGMAC_CONFIG_SS_10000		(0x0 << XGMAC_CONFIG_SS_OFF)
-#define XGMAC_CONFIG_SS_2500		(0x2 << XGMAC_CONFIG_SS_OFF)
-#define XGMAC_CONFIG_SS_1000		(0x3 << XGMAC_CONFIG_SS_OFF)
+#define XGMAC_CONFIG_SS_2500_GMII	(0x2 << XGMAC_CONFIG_SS_OFF)
+#define XGMAC_CONFIG_SS_1000_GMII	(0x3 << XGMAC_CONFIG_SS_OFF)
+#define XGMAC_CONFIG_SS_100_MII		(0x4 << XGMAC_CONFIG_SS_OFF)
+#define XGMAC_CONFIG_SS_5000		(0x5 << XGMAC_CONFIG_SS_OFF)
+#define XGMAC_CONFIG_SS_2500		(0x6 << XGMAC_CONFIG_SS_OFF)
+#define XGMAC_CONFIG_SS_10_MII		(0x7 << XGMAC_CONFIG_SS_OFF)
 #define XGMAC_CONFIG_SARC		GENMASK(22, 20)
 #define XGMAC_CONFIG_SARC_SHIFT		20
 #define XGMAC_CONFIG_JD			BIT(16)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
index bfa7d6913fd4..0a32c96a7854 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
@@ -36,7 +36,7 @@ static void dwxgmac2_core_init(struct mac_device_info *hw,
 
 		switch (hw->ps) {
 		case SPEED_10000:
-			tx |= hw->link.speed10000;
+			tx |= hw->link.xgmii.speed10000;
 			break;
 		case SPEED_2500:
 			tx |= hw->link.speed2500;
@@ -381,11 +381,13 @@ int dwxgmac2_setup(struct stmmac_priv *priv)
 		mac->mcast_bits_log2 = ilog2(mac->multicast_filter_bins);
 
 	mac->link.duplex = 0;
-	mac->link.speed10 = 0;
-	mac->link.speed100 = 0;
-	mac->link.speed1000 = XGMAC_CONFIG_SS_1000;
-	mac->link.speed2500 = XGMAC_CONFIG_SS_2500;
-	mac->link.speed10000 = XGMAC_CONFIG_SS_10000;
+	mac->link.speed10 = XGMAC_CONFIG_SS_10_MII;
+	mac->link.speed100 = XGMAC_CONFIG_SS_100_MII;
+	mac->link.speed1000 = XGMAC_CONFIG_SS_1000_GMII;
+	mac->link.speed2500 = XGMAC_CONFIG_SS_2500_GMII;
+	mac->link.xgmii.speed2500 = XGMAC_CONFIG_SS_2500;
+	mac->link.xgmii.speed5000 = XGMAC_CONFIG_SS_5000;
+	mac->link.xgmii.speed10000 = XGMAC_CONFIG_SS_10000;
 	mac->link.speed_mask = XGMAC_CONFIG_SS_MASK;
 
 	mac->mii.addr = XGMAC_MDIO_ADDR;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 40408a067400..4a5941caaadc 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -805,14 +805,43 @@ static void stmmac_validate(struct phylink_config *config,
 			    struct phylink_link_state *state)
 {
 	struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev));
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(mac_supported) = { 0, };
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
 	int tx_cnt = priv->plat->tx_queues_to_use;
 	int max_speed = priv->plat->max_speed;
 
+	phylink_set(mac_supported, 10baseT_Half);
+	phylink_set(mac_supported, 10baseT_Full);
+	phylink_set(mac_supported, 100baseT_Half);
+	phylink_set(mac_supported, 100baseT_Full);
+
+	phylink_set(mac_supported, Autoneg);
+	phylink_set(mac_supported, Pause);
+	phylink_set(mac_supported, Asym_Pause);
+	phylink_set_port_modes(mac_supported);
+
+	if (priv->plat->has_gmac ||
+	    priv->plat->has_gmac4 ||
+	    priv->plat->has_xgmac) {
+		phylink_set(mac_supported, 1000baseT_Half);
+		phylink_set(mac_supported, 1000baseT_Full);
+		phylink_set(mac_supported, 1000baseKX_Full);
+	}
+
 	/* Cut down 1G if asked to */
 	if ((max_speed > 0) && (max_speed < 1000)) {
 		phylink_set(mask, 1000baseT_Full);
 		phylink_set(mask, 1000baseX_Full);
+	} else if (priv->plat->has_xgmac) {
+		phylink_set(mac_supported, 2500baseT_Full);
+		phylink_set(mac_supported, 5000baseT_Full);
+		phylink_set(mac_supported, 10000baseSR_Full);
+		phylink_set(mac_supported, 10000baseLR_Full);
+		phylink_set(mac_supported, 10000baseER_Full);
+		phylink_set(mac_supported, 10000baseLRM_Full);
+		phylink_set(mac_supported, 10000baseT_Full);
+		phylink_set(mac_supported, 10000baseKX4_Full);
+		phylink_set(mac_supported, 10000baseKR_Full);
 	}
 
 	/* Half-Duplex can only work with single queue */
@@ -822,7 +851,12 @@ static void stmmac_validate(struct phylink_config *config,
 		phylink_set(mask, 1000baseT_Half);
 	}
 
-	bitmap_andnot(supported, supported, mask, __ETHTOOL_LINK_MODE_MASK_NBITS);
+	bitmap_and(supported, supported, mac_supported,
+		   __ETHTOOL_LINK_MODE_MASK_NBITS);
+	bitmap_andnot(supported, supported, mask,
+		      __ETHTOOL_LINK_MODE_MASK_NBITS);
+	bitmap_and(state->advertising, state->advertising, mac_supported,
+		   __ETHTOOL_LINK_MODE_MASK_NBITS);
 	bitmap_andnot(state->advertising, state->advertising, mask,
 		      __ETHTOOL_LINK_MODE_MASK_NBITS);
 }
@@ -842,18 +876,37 @@ static void stmmac_mac_config(struct phylink_config *config, unsigned int mode,
 	ctrl = readl(priv->ioaddr + MAC_CTRL_REG);
 	ctrl &= ~priv->hw->link.speed_mask;
 
-	switch (state->speed) {
-	case SPEED_1000:
-		ctrl |= priv->hw->link.speed1000;
-		break;
-	case SPEED_100:
-		ctrl |= priv->hw->link.speed100;
-		break;
-	case SPEED_10:
-		ctrl |= priv->hw->link.speed10;
-		break;
-	default:
-		return;
+	if (state->interface == PHY_INTERFACE_MODE_USXGMII) {
+		switch (state->speed) {
+		case SPEED_10000:
+			ctrl |= priv->hw->link.xgmii.speed10000;
+			break;
+		case SPEED_5000:
+			ctrl |= priv->hw->link.xgmii.speed5000;
+			break;
+		case SPEED_2500:
+			ctrl |= priv->hw->link.xgmii.speed2500;
+			break;
+		default:
+			return;
+		}
+	} else {
+		switch (state->speed) {
+		case SPEED_2500:
+			ctrl |= priv->hw->link.speed2500;
+			break;
+		case SPEED_1000:
+			ctrl |= priv->hw->link.speed1000;
+			break;
+		case SPEED_100:
+			ctrl |= priv->hw->link.speed100;
+			break;
+		case SPEED_10:
+			ctrl |= priv->hw->link.speed10;
+			break;
+		default:
+			return;
+		}
 	}
 
 	priv->speed = state->speed;
-- 
2.7.4


^ permalink raw reply related

* [PATCH net-next v2 04/10] net: stmmac: dwxgmac: Fix the undefined burst setting
From: Jose Abreu @ 2019-06-28  7:29 UTC (permalink / raw)
  To: linux-kernel, netdev
  Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
	Alexandre Torgue
In-Reply-To: <cover.1561706800.git.joabreu@synopsys.com>

Undefined burst shall only be set if pdata asks to.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
index 42448973c36e..b739673f8842 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
@@ -91,11 +91,11 @@ static void dwxgmac2_dma_axi(void __iomem *ioaddr, struct stmmac_axi *axi)
 	value |= (axi->axi_rd_osr_lmt << XGMAC_RD_OSR_LMT_SHIFT) &
 		XGMAC_RD_OSR_LMT;
 
+	if (!axi->axi_fb)
+		value |= XGMAC_UNDEF;
+
 	value &= ~XGMAC_BLEN;
 	for (i = 0; i < AXI_BLEN; i++) {
-		if (axi->axi_blen[i])
-			value &= ~XGMAC_UNDEF;
-
 		switch (axi->axi_blen[i]) {
 		case 256:
 			value |= XGMAC_BLEN256;
-- 
2.7.4


^ permalink raw reply related

* [PATCH net-next v2 00/10] net: stmmac: 10GbE using XGMAC
From: Jose Abreu @ 2019-06-28  7:29 UTC (permalink / raw)
  To: linux-kernel, netdev
  Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
	Alexandre Torgue

Support for 10Gb Link using XGMAC core plus some performance tweaks.

Tested in a PCI based setup.

iperf3 TCP results:
	TSO ON, MTU=1500, TX Queues = 1, RX Queues = 1, Flow Control ON
	Pinned CPU (-A), Zero-Copy (-Z)

[ ID] Interval           Transfer     Bitrate         Retr
[  5]   0.00-600.00 sec   643 GBytes  9.21 Gbits/sec    1             sender
[  5]   0.00-600.00 sec   643 GBytes  9.21 Gbits/sec                  receiver

Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>

Jose Abreu (10):
  net: stmmac: dwxgmac: Enable EDMA by default
  net: stmmac: Do not try to enable PHY EEE if MAC does not support it
  net: stmmac: Decrease default RX Watchdog value
  net: stmmac: dwxgmac: Fix the undefined burst setting
  net: stmmac: Add the missing speeds that XGMAC supports
  net: stmmac: Do not disable interrupts when cleaning TX
  net: stmmac: Enable support for > 32 Bits addressing in XGMAC
  net: stmmac: Update RX Tail Pointer to last free entry
  net: stmmac: Only disable interrupts if NAPI is scheduled
  net: stmmac: Update Kconfig entry

 drivers/net/ethernet/stmicro/stmmac/Kconfig        |   2 +-
 drivers/net/ethernet/stmicro/stmmac/common.h       |   9 +-
 drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h     |  16 ++-
 .../net/ethernet/stmicro/stmmac/dwxgmac2_core.c    |  14 +-
 .../net/ethernet/stmicro/stmmac/dwxgmac2_descs.c   |   4 +-
 drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c |  27 +++-
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 160 ++++++++++++++++-----
 7 files changed, 176 insertions(+), 56 deletions(-)

-- 
2.7.4


^ permalink raw reply

* [PATCH net-next v2 03/10] net: stmmac: Decrease default RX Watchdog value
From: Jose Abreu @ 2019-06-28  7:29 UTC (permalink / raw)
  To: linux-kernel, netdev
  Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
	Alexandre Torgue
In-Reply-To: <cover.1561706800.git.joabreu@synopsys.com>

For performance reasons decrease the default RX Watchdog value for the
minimum allowed.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
 drivers/net/ethernet/stmicro/stmmac/common.h      | 2 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index ad9e9368535d..14d8f6d7cb9a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -246,7 +246,7 @@ struct stmmac_safety_stats {
 
 /* Max/Min RI Watchdog Timer count value */
 #define MAX_DMA_RIWT		0xff
-#define MIN_DMA_RIWT		0x20
+#define MIN_DMA_RIWT		0x10
 /* Tx coalesce parameters */
 #define STMMAC_COAL_TX_TIMER	1000
 #define STMMAC_MAX_COAL_TX_TICK	100000
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 0b1900bf4e9e..40408a067400 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2516,9 +2516,9 @@ static int stmmac_hw_setup(struct net_device *dev, bool init_ptp)
 	priv->tx_lpi_timer = STMMAC_DEFAULT_TWT_LS;
 
 	if (priv->use_riwt) {
-		ret = stmmac_rx_watchdog(priv, priv->ioaddr, MAX_DMA_RIWT, rx_cnt);
+		ret = stmmac_rx_watchdog(priv, priv->ioaddr, MIN_DMA_RIWT, rx_cnt);
 		if (!ret)
-			priv->rx_riwt = MAX_DMA_RIWT;
+			priv->rx_riwt = MIN_DMA_RIWT;
 	}
 
 	if (priv->hw->pcs)
-- 
2.7.4


^ permalink raw reply related

* [PATCH net-next v2 07/10] net: stmmac: Enable support for > 32 Bits addressing in XGMAC
From: Jose Abreu @ 2019-06-28  7:29 UTC (permalink / raw)
  To: linux-kernel, netdev
  Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
	Alexandre Torgue, Andrew Lunn
In-Reply-To: <cover.1561706800.git.joabreu@synopsys.com>

Currently, stmmac only supports 32 bits addressing for SKB. Enable the
support for upto 48 bits addressing in XGMAC core.

This avoids the use of bounce buffers and increases performance.

Changes from v1:
	- Fallback to 32 bits in failure (Andrew)

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Cc: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/ethernet/stmicro/stmmac/common.h       |  1 +
 drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h     |  2 +
 .../net/ethernet/stmicro/stmmac/dwxgmac2_descs.c   |  4 +-
 drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 19 +++++++-
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 55 +++++++++++++++++-----
 5 files changed, 66 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 9e8f4dbdcc22..2403a65167b2 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -351,6 +351,7 @@ struct dma_features {
 	unsigned int frpsel;
 	unsigned int frpbs;
 	unsigned int frpes;
+	unsigned int addr64;
 };
 
 /* GMAC TX FIFO is 8K, Rx FIFO is 16K */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
index 29db9b538874..9a9792527530 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
@@ -87,6 +87,7 @@
 #define XGMAC_HWFEAT_GMIISEL		BIT(1)
 #define XGMAC_HW_FEATURE1		0x00000120
 #define XGMAC_HWFEAT_TSOEN		BIT(18)
+#define XGMAC_HWFEAT_ADDR64		GENMASK(15, 14)
 #define XGMAC_HWFEAT_TXFIFOSIZE		GENMASK(10, 6)
 #define XGMAC_HWFEAT_RXFIFOSIZE		GENMASK(4, 0)
 #define XGMAC_HW_FEATURE2		0x00000124
@@ -172,6 +173,7 @@
 #define XGMAC_EN_LPI			BIT(15)
 #define XGMAC_LPI_XIT_PKT		BIT(14)
 #define XGMAC_AAL			BIT(12)
+#define XGMAC_EAME			BIT(11)
 #define XGMAC_BLEN			GENMASK(7, 1)
 #define XGMAC_BLEN256			BIT(7)
 #define XGMAC_BLEN128			BIT(6)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c
index 98fa471da7c0..c4c45402b8f8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c
@@ -242,8 +242,8 @@ static void dwxgmac2_get_addr(struct dma_desc *p, unsigned int *addr)
 
 static void dwxgmac2_set_addr(struct dma_desc *p, dma_addr_t addr)
 {
-	p->des0 = cpu_to_le32(addr);
-	p->des1 = 0;
+	p->des0 = cpu_to_le32(lower_32_bits(addr));
+	p->des1 = cpu_to_le32(upper_32_bits(addr));
 }
 
 static void dwxgmac2_clear(struct dma_desc *p)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
index b739673f8842..229c58758cbd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
@@ -27,7 +27,7 @@ static void dwxgmac2_dma_init(void __iomem *ioaddr,
 	if (dma_cfg->aal)
 		value |= XGMAC_AAL;
 
-	writel(value, ioaddr + XGMAC_DMA_SYSBUS_MODE);
+	writel(value | XGMAC_EAME, ioaddr + XGMAC_DMA_SYSBUS_MODE);
 }
 
 static void dwxgmac2_dma_init_chan(void __iomem *ioaddr,
@@ -361,6 +361,23 @@ static void dwxgmac2_get_hw_feature(void __iomem *ioaddr,
 	/* MAC HW feature 1 */
 	hw_cap = readl(ioaddr + XGMAC_HW_FEATURE1);
 	dma_cap->tsoen = (hw_cap & XGMAC_HWFEAT_TSOEN) >> 18;
+
+	dma_cap->addr64 = (hw_cap & XGMAC_HWFEAT_ADDR64) >> 14;
+	switch (dma_cap->addr64) {
+	case 0:
+		dma_cap->addr64 = 32;
+		break;
+	case 1:
+		dma_cap->addr64 = 40;
+		break;
+	case 2:
+		dma_cap->addr64 = 48;
+		break;
+	default:
+		dma_cap->addr64 = 32;
+		break;
+	}
+
 	dma_cap->tx_fifo_size =
 		128 << ((hw_cap & XGMAC_HWFEAT_TXFIFOSIZE) >> 6);
 	dma_cap->rx_fifo_size =
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index e8f3e76889c8..620dd387e3b1 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2772,7 +2772,7 @@ static int stmmac_release(struct net_device *dev)
  *  This function fills descriptor and request new descriptors according to
  *  buffer length to fill
  */
-static void stmmac_tso_allocator(struct stmmac_priv *priv, unsigned int des,
+static void stmmac_tso_allocator(struct stmmac_priv *priv, dma_addr_t des,
 				 int total_len, bool last_segment, u32 queue)
 {
 	struct stmmac_tx_queue *tx_q = &priv->tx_queue[queue];
@@ -2783,11 +2783,18 @@ static void stmmac_tso_allocator(struct stmmac_priv *priv, unsigned int des,
 	tmp_len = total_len;
 
 	while (tmp_len > 0) {
+		dma_addr_t curr_addr;
+
 		tx_q->cur_tx = STMMAC_GET_ENTRY(tx_q->cur_tx, DMA_TX_SIZE);
 		WARN_ON(tx_q->tx_skbuff[tx_q->cur_tx]);
 		desc = tx_q->dma_tx + tx_q->cur_tx;
 
-		desc->des0 = cpu_to_le32(des + (total_len - tmp_len));
+		curr_addr = des + (total_len - tmp_len);
+		if (priv->dma_cap.addr64 <= 32)
+			desc->des0 = cpu_to_le32(curr_addr);
+		else
+			stmmac_set_desc_addr(priv, desc, curr_addr);
+
 		buff_size = tmp_len >= TSO_MAX_BUFF_SIZE ?
 			    TSO_MAX_BUFF_SIZE : tmp_len;
 
@@ -2833,11 +2840,12 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)
 	struct stmmac_priv *priv = netdev_priv(dev);
 	int nfrags = skb_shinfo(skb)->nr_frags;
 	u32 queue = skb_get_queue_mapping(skb);
-	unsigned int first_entry, des;
+	unsigned int first_entry;
 	struct stmmac_tx_queue *tx_q;
 	int tmp_pay_len = 0;
 	u32 pay_len, mss;
 	u8 proto_hdr_len;
+	dma_addr_t des;
 	int i;
 
 	tx_q = &priv->tx_queue[queue];
@@ -2894,14 +2902,19 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)
 	tx_q->tx_skbuff_dma[first_entry].buf = des;
 	tx_q->tx_skbuff_dma[first_entry].len = skb_headlen(skb);
 
-	first->des0 = cpu_to_le32(des);
+	if (priv->dma_cap.addr64 <= 32) {
+		first->des0 = cpu_to_le32(des);
 
-	/* Fill start of payload in buff2 of first descriptor */
-	if (pay_len)
-		first->des1 = cpu_to_le32(des + proto_hdr_len);
+		/* Fill start of payload in buff2 of first descriptor */
+		if (pay_len)
+			first->des1 = cpu_to_le32(des + proto_hdr_len);
 
-	/* If needed take extra descriptors to fill the remaining payload */
-	tmp_pay_len = pay_len - TSO_MAX_BUFF_SIZE;
+		/* If needed take extra descriptors to fill the remaining payload */
+		tmp_pay_len = pay_len - TSO_MAX_BUFF_SIZE;
+	} else {
+		stmmac_set_desc_addr(priv, first, des);
+		tmp_pay_len = pay_len;
+	}
 
 	stmmac_tso_allocator(priv, des, tmp_pay_len, (nfrags == 0), queue);
 
@@ -3031,12 +3044,12 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
 	int i, csum_insertion = 0, is_jumbo = 0;
 	u32 queue = skb_get_queue_mapping(skb);
 	int nfrags = skb_shinfo(skb)->nr_frags;
-	int entry;
-	unsigned int first_entry;
 	struct dma_desc *desc, *first;
 	struct stmmac_tx_queue *tx_q;
+	unsigned int first_entry;
 	unsigned int enh_desc;
-	unsigned int des;
+	dma_addr_t des;
+	int entry;
 
 	tx_q = &priv->tx_queue[queue];
 
@@ -4316,6 +4329,24 @@ int stmmac_dvr_probe(struct device *device,
 		priv->tso = true;
 		dev_info(priv->device, "TSO feature enabled\n");
 	}
+
+	if (priv->dma_cap.addr64) {
+		ret = dma_set_mask_and_coherent(device,
+				DMA_BIT_MASK(priv->dma_cap.addr64));
+		if (!ret) {
+			dev_info(priv->device, "Using %d bits DMA width\n",
+				 priv->dma_cap.addr64);
+		} else {
+			ret = dma_set_mask_and_coherent(device, DMA_BIT_MASK(32));
+			if (ret) {
+				dev_err(priv->device, "Failed to set DMA Mask\n");
+				goto error_hw_init;
+			}
+
+			priv->dma_cap.addr64 = 32;
+		}
+	}
+
 	ndev->features |= ndev->hw_features | NETIF_F_HIGHDMA;
 	ndev->watchdog_timeo = msecs_to_jiffies(watchdog);
 #ifdef STMMAC_VLAN_TAG_USED
-- 
2.7.4


^ permalink raw reply related

* [PATCH net-next v2 09/10] net: stmmac: Only disable interrupts if NAPI is scheduled
From: Jose Abreu @ 2019-06-28  7:29 UTC (permalink / raw)
  To: linux-kernel, netdev
  Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
	Alexandre Torgue
In-Reply-To: <cover.1561706800.git.joabreu@synopsys.com>

Only disable the interrupts if RX NAPI gets to be scheduled. Also,
schedule the TX NAPI only when the interrupts are disabled.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index a629b3144a0d..3425d4dda03d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2053,12 +2053,12 @@ static int stmmac_napi_check(struct stmmac_priv *priv, u32 chan)
 						 &priv->xstats, chan);
 	struct stmmac_channel *ch = &priv->channel[chan];
 
-	if (status)
-		status |= handle_rx | handle_tx;
-
 	if ((status & handle_rx) && (chan < priv->plat->rx_queues_to_use)) {
-		stmmac_disable_dma_irq(priv, priv->ioaddr, chan);
-		napi_schedule_irqoff(&ch->rx_napi);
+		if (napi_schedule_prep(&ch->rx_napi)) {
+			stmmac_disable_dma_irq(priv, priv->ioaddr, chan);
+			__napi_schedule_irqoff(&ch->rx_napi);
+			status |= handle_tx;
+		}
 	}
 
 	if ((status & handle_tx) && (chan < priv->plat->tx_queues_to_use))
-- 
2.7.4


^ permalink raw reply related

* [PATCH net-next v2 01/10] net: stmmac: dwxgmac: Enable EDMA by default
From: Jose Abreu @ 2019-06-28  7:29 UTC (permalink / raw)
  To: linux-kernel, netdev
  Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
	Alexandre Torgue
In-Reply-To: <cover.1561706800.git.joabreu@synopsys.com>

Enable the EDMA feature by default which gives higher performance.

Changes from v1:
	- Do not use magic values (David)

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h     | 4 ++++
 drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
index b8296eb41011..c9e802faabb5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
@@ -177,6 +177,10 @@
 #define XGMAC_BLEN8			BIT(2)
 #define XGMAC_BLEN4			BIT(1)
 #define XGMAC_UNDEF			BIT(0)
+#define XGMAC_TX_EDMA_CTRL		0x00003040
+#define XGMAC_TDPS			GENMASK(29, 0)
+#define XGMAC_RX_EDMA_CTRL		0x00003044
+#define XGMAC_RDPS			GENMASK(29, 0)
 #define XGMAC_DMA_CH_CONTROL(x)		(0x00003100 + (0x80 * (x)))
 #define XGMAC_PBLx8			BIT(16)
 #define XGMAC_DMA_CH_TX_CONTROL(x)	(0x00003104 + (0x80 * (x)))
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
index 7861a938420a..42448973c36e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
@@ -122,6 +122,8 @@ static void dwxgmac2_dma_axi(void __iomem *ioaddr, struct stmmac_axi *axi)
 	}
 
 	writel(value, ioaddr + XGMAC_DMA_SYSBUS_MODE);
+	writel(XGMAC_TDPS, ioaddr + XGMAC_TX_EDMA_CTRL);
+	writel(XGMAC_RDPS, ioaddr + XGMAC_RX_EDMA_CTRL);
 }
 
 static void dwxgmac2_dma_rx_mode(void __iomem *ioaddr, int mode,
-- 
2.7.4


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox